From fe07070eff35fb60a71d9fc2195399fa324c870f Mon Sep 17 00:00:00 2001 From: Vincent Pelletier <vincent@nexedi.com> Date: Fri, 14 Dec 2012 19:14:55 +0100 Subject: [PATCH] Move selection checksum computation next to its verification Allows getting current selection's MD5 even outside ListBox's renderer. Signed-off-by: Julien Muchembled <jm@nexedi.com> --- .../erp5_core/Base_callDialogMethod.xml | 1 - .../portal_skins/erp5_core/Folder_delete.xml | 33 ++++++----------- product/ERP5/tests/testERP5Core.py | 36 ++++++++----------- product/ERP5Form/ListBox.py | 21 ++++------- product/ERP5Form/Tool/SelectionTool.py | 23 +++++++++--- 5 files changed, 49 insertions(+), 65 deletions(-) diff --git a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_callDialogMethod.xml b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_callDialogMethod.xml index 09b9024e14..22c2b445a8 100644 --- a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_callDialogMethod.xml +++ b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_callDialogMethod.xml @@ -129,7 +129,6 @@ if dialog_method == \'Base_createRelation\':\n if dialog_method == \'Folder_delete\':\n return context.Folder_delete(form_id=kw[\'form_id\'],\n selection_name=kw[\'selection_name\'],\n - uids=kw[\'listbox_uid\'],\n md5_object_uid_list=kw[\'md5_object_uid_list\'])\n \n \n diff --git a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Folder_delete.xml b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Folder_delete.xml index e310e8c9c8..b598f11263 100644 --- a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Folder_delete.xml +++ b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Folder_delete.xml @@ -57,24 +57,13 @@ from Products.CMFCore.WorkflowCore import WorkflowException\n \n portal = context.getPortalObject()\n Base_translateString = portal.Base_translateString\n +REQUEST = portal.REQUEST\n \n -selected_uids = portal.portal_selections.updateSelectionCheckedUidList(\n - selection_name,listbox_uid,uids)\n uids = portal.portal_selections.getSelectionCheckedUidsFor(selection_name)\n -\n -error = portal.portal_selections.selectionHasChanged(md5_object_uid_list,\n - uids)\n -\n -REQUEST=context.REQUEST\n -qs = \'\'\n -ret_url = context.absolute_url() + \'/\' + form_id\n -if error:\n +if portal.portal_selections.selectionHasChanged(md5_object_uid_list, uids):\n message = Base_translateString("Sorry, your selection has changed.")\n - qs = \'?portal_status_message=%s\' % message\n -elif uids is not None:\n +elif uids:\n # Check if there is some related objets.\n - object_used = 0\n -\n object_list = [x.getObject() for x in context.Folder_getDeleteObjectList(uid=uids)]\n object_used = sum([x.getRelationCountForDeletion() and 1 for x in object_list])\n \n @@ -84,7 +73,6 @@ elif uids is not None:\n else:\n message = Base_translateString("Sorry, ${count} items are in use.",\n mapping={\'count\': repr(object_used)})\n - qs = \'?portal_status_message=%s\' % message \n else:\n \n # Do not delete objects which have a workflow history \n @@ -106,7 +94,7 @@ elif uids is not None:\n \n # Remove some objects\n try:\n - if object_to_remove_list != []:\n + if object_to_remove_list:\n if context.portal_type == \'Preference\':\n # Templates inside preference are not indexed, so we cannot pass\n # uids= to manage_delObjects and have to use ids=\n @@ -121,7 +109,7 @@ elif uids is not None:\n except ConflictError:\n raise\n except Exception, message:\n - qs = \'?portal_status_message=%s\' % message\n + pass\n else:\n object_ids = [x.getId() for x in object_to_remove_list]\n comment = Base_translateString(\'Deleted objects: ${object_ids}\',\n @@ -147,7 +135,6 @@ elif uids is not None:\n raise\n except:\n not_deleted_count += 1\n - pass\n \n # Generate message\n if not_deleted_count == 1:\n @@ -158,20 +145,20 @@ elif uids is not None:\n mapping={\'count\': not_deleted_count})\n qs = \'?portal_status_message=%s\' % message\n \n + # make sure nothing is checked after\n + portal.portal_selections.setSelectionCheckedUidsFor(selection_name, ())\n else:\n message = Base_translateString("Please select one or more items first.")\n - qs = \'?portal_status_message=%s\' % message\n \n -# make sure nothing is checked after\n -portal.portal_selections.setSelectionCheckedUidsFor(selection_name, [])\n -return REQUEST.RESPONSE.redirect("%s%s" % (ret_url, qs))\n +return REQUEST.RESPONSE.redirect("%s/%s?portal_status_message=%s"\n + % (context.absolute_url_path(), form_id, message))\n ]]></string> </value> </item> <item> <key> <string>_params</string> </key> - <value> <string>form_id=\'\',selection_index=None,object_uid=None,selection_name=None,field_id=None,uids=None,cancel_url=\'\',listbox_uid=[],md5_object_uid_list=\'\'</string> </value> + <value> <string>form_id=\'\',selection_index=None,object_uid=None,selection_name=None,field_id=None,cancel_url=\'\',md5_object_uid_list=\'\'</string> </value> </item> <item> <key> <string>id</string> </key> diff --git a/product/ERP5/tests/testERP5Core.py b/product/ERP5/tests/testERP5Core.py index 07cbdd35e3..00893c1e15 100644 --- a/product/ERP5/tests/testERP5Core.py +++ b/product/ERP5/tests/testERP5Core.py @@ -405,20 +405,23 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional): response = self.publish(self.portal_id, self.auth) self.assertEquals(HTTP_OK, response.getStatus()) + def _Folder_delete(self, *object_list): + selection_name = 'test_selection' + portal_selections = self.portal.portal_selections + portal_selections.setSelectionCheckedUidsFor(selection_name, + [x.getUid() for x in object_list]) + md5_string = portal_selections.getSelectionChecksum(selection_name) + return object_list[0].getParentValue().Folder_delete( + selection_name=selection_name, md5_object_uid_list=md5_string) + def test_Folder_delete(self): module = self.portal.newContent(portal_type='Folder', id='test_folder') document_1 = module.newContent(portal_type='Folder', id='1') document_2 = module.newContent(portal_type='Folder', id='2') - uid_list = [document_1.getUid(), document_2.getUid()] - self.portal.portal_selections.setSelectionParamsFor( - 'test_selection', dict(uids=uid_list)) self.tic() - md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest() - redirect = module.Folder_delete(selection_name='test_selection', - uids=uid_list, - md5_object_uid_list=md5_string) + redirect = self._Folder_delete(document_1, document_2) self.assert_('Deleted.' in redirect, redirect) - self.assertEquals(len(module.objectValues()), 0) + self.assertEqual(module.objectCount(), 0) def test_Folder_delete_related_object(self): # deletion is refused if there are related objects @@ -435,12 +438,7 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional): self.assertEqual(2, organisation.getRelationCountForDeletion()) self.assertEqual(0, person.getRelationCountForDeletion()) def delete(assert_deleted, obj): - uid_list = [obj.getUid()] - self.portal.portal_selections.setSelectionParamsFor( - 'test_selection', dict(uids=uid_list)) - md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest() - redirect = obj.getParentValue().Folder_delete(uids=uid_list, - selection_name='test_selection', md5_object_uid_list=md5_string) + redirect = self._Folder_delete(obj) self.assertTrue(('Sorry, 1 item is in use.', 'Deleted.')[assert_deleted] in redirect, redirect) self.tic() @@ -463,21 +461,15 @@ class TestERP5Core(ERP5TypeTestCase, ZopeTestCase.Functional): context=document_1, base_category_list=('source',), category_list=document_2.getRelativeUrl()) - uid_list = [document_2.getUid(), ] - self.portal.portal_selections.setSelectionParamsFor( - 'test_selection', dict(uids=uid_list)) self.tic() self.assertEquals([document_1], self.portal.portal_categories.getRelatedValueList(document_2)) - md5_string = md5(str(sorted(map(str, uid_list)))).hexdigest() document_1.manage_permission('View', [], acquire=0) document_1.manage_permission('Access contents information', [], acquire=0) - redirect = module.Folder_delete(selection_name='test_selection', - uids=uid_list, - md5_object_uid_list=md5_string) + redirect = self._Folder_delete(document_2) self.assert_('Sorry, 1 item is in use.' in redirect, redirect) - self.assertEquals(len(module.objectValues()), 2) + self.assertEqual(module.objectCount(), 2) def test_getPropertyForUid(self): error_list = [] diff --git a/product/ERP5Form/ListBox.py b/product/ERP5Form/ListBox.py index bc84c6c5fc..e5db7e9eea 100644 --- a/product/ERP5Form/ListBox.py +++ b/product/ERP5Form/ListBox.py @@ -50,7 +50,6 @@ from Products.ERP5Type.Globals import InitializeClass, get_request from Products.PythonScripts.Utility import allow_class from Products.PageTemplates.PageTemplateFile import PageTemplateFile from warnings import warn -from hashlib import md5 import cgi DEFAULT_LISTBOX_DISPLAY_STYLE = 'table' @@ -2560,20 +2559,14 @@ class ListBoxHTMLRenderer(ListBoxRenderer): """Generate a MD5 checksum against checked uids. This is used to confirm that selected values do not change between a display of a dialog and an execution. - FIXME: this should only use getCheckedUidList, but Folder_deleteObjectList does not use - the feature that checked uids are used when no list method is specified. + FIXME: SelectionTool.getSelectionChecksum's uid_list parameter is + deprecated, but Folder_deleteObjectList does not use the feature that + checked uids are used when no list method is specified. """ - checked_uid_list = self.request.get('uids') - if checked_uid_list is None: - checked_uid_list = self.getCheckedUidList() - if checked_uid_list is not None: - checked_uid_list = [str(uid) for uid in checked_uid_list] - checked_uid_list.sort() - md5_string = md5(str(checked_uid_list)).hexdigest() - else: - md5_string = None - - return md5_string + return self.getSelectionTool().getSelectionChecksum( + self.getSelectionName(), + uid_list=self.request.get('uids'), + ) def getPhysicalRoot(self): """Return the physical root (an Application object). This method is required for diff --git a/product/ERP5Form/Tool/SelectionTool.py b/product/ERP5Form/Tool/SelectionTool.py index 6da356594b..0deb9aee22 100644 --- a/product/ERP5Form/Tool/SelectionTool.py +++ b/product/ERP5Form/Tool/SelectionTool.py @@ -1170,13 +1170,26 @@ class SelectionTool( BaseTool, SimpleItem ): """ We want to be sure that the selection did not change """ + return md5_string != self._getUIDListChecksum(object_uid_list) + + security.declareProtected(ERP5Permissions.View, 'getSelectionChecksum') + def getSelectionChecksum(self, selection_name, uid_list=None): + """Generate an MD5 checksum against checked uids. This is used to confirm + that selected values do not change between a display of a dialog and an + execution. + uid_list (deprecated) + For backward compatibility with code not updating selected uids. + """ + if uid_list is None: + uid_list = self.getSelectionCheckedUidsFor(selection_name) + return self._getUIDListChecksum(uid_list) + + def _getUIDListChecksum(self, uid_list): + if uid_list is None: + return None # XXX To avoid the difference of the string representations of int and long, # convert each element to a string. - object_uid_list = [str(x) for x in object_uid_list] - object_uid_list.sort() - new_md5_string = md5(str(object_uid_list)).hexdigest() - return md5_string != new_md5_string - + return md5(str(sorted(map(str, uid_list)))).hexdigest() # Related document searching def viewSearchRelatedDocumentDialog(self, index, form_id, -- 2.30.9