Do not recursively reindex security unnecessarily

In ERP5, objects that do not acquire local roles (according to their portal type configuration) do not need to be reindexed with their containers.
So do not reindex recursively if portal_types of contained objects don't need it.

Security reindexing can be customized with a type-based method.

Fix test that wrongly relied on unrestricted access for updating object roles.

product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Base_reindexObjectSecurity.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>Script_magic</string> </key>
+            <value> <int>3</int> </value>
+        </item>
+        <item>
+            <key> <string>_bind_names</string> </key>
+            <value>
+              <object>
+                <klass>
+                  <global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
+                </klass>
+                <tuple/>
+                <state>
+                  <dictionary>
+                    <item>
+                        <key> <string>_asgns</string> </key>
+                        <value>
+                          <dictionary>
+                            <item>
+                                <key> <string>name_container</string> </key>
+                                <value> <string>container</string> </value>
+                            </item>
+                            <item>
+                                <key> <string>name_context</string> </key>
+                                <value> <string>context</string> </value>
+                            </item>
+                            <item>
+                                <key> <string>name_m_self</string> </key>
+                                <value> <string>script</string> </value>
+                            </item>
+                            <item>
+                                <key> <string>name_subpath</string> </key>
+                                <value> <string>traverse_subpath</string> </value>
+                            </item>
+                          </dictionary>
+                        </value>
+                    </item>
+                  </dictionary>
+                </state>
+              </object>
+            </value>
+        </item>
+        <item>
+            <key> <string>_body</string> </key>
+            <value> <string># Do not re-index security recursively if contained objects don\'t acquire roles\n
+#\n
+# After all, recursively re-indexing a module in a production system\n
+# with lots of content could mean hours of non-usable overloaded system.\n
+type_tool = context.getPortalObject().portal_types \n
+for portal_type_name in context.getTypeInfo().getTypeAllowedContentTypeList():\n
+  portal_type = type_tool[portal_type_name]\n
+  if portal_type.getTypeAcquireLocalRole():\n
+    reindex = context.recursiveReindexObject\n
+    break\n
+else:\n
+  reindex = context.reindexObject\n
+\n
+return reindex(*args, **kw)\n
+</string> </value>
+        </item>
+        <item>
+            <key> <string>_params</string> </key>
+            <value> <string>*args, **kw</string> </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>Base_reindexObjectSecurity</string> </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+</ZopeData>

product/ERP5/bootstrap/erp5_core/bt/revision

-41040
+41041
\ No newline at end of file

product/ERP5/tests/testCRM.py

@@ -1609,6 +1609,7 @@ class TestCRMMailSend(BaseTestCRM):
       self.assertEquals(event.getSourceSection(), user.getSubordination())
     finally:
       # clean up created roles on portal_types
+      self.login() # admin
       for portal_type in portal_type_list:
         portal_type_object = getattr(self.portal.portal_types, portal_type)
         portal_type_object._delObject('manager_role')

product/ERP5Security/tests/testERP5Security.py

@@ -32,6 +32,8 @@
 
 import unittest
 
+import transaction
+
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
 from Products.ERP5Type.tests.utils import createZODBPythonScript
 from AccessControl.SecurityManagement import newSecurityManager
@@ -902,6 +904,39 @@ class TestLocalRoleManagement(ERP5TypeTestCase):
         (((cloning_owner_id), ('Owner',)),)
     )
 
+  def _checkMessageMethodIdList(self, expected_method_id_list):
+    actual_method_id_list = sorted([
+        message.method_id
+        for message in self.portal.portal_activities.getMessageList()
+    ])
+    self.assertEqual(expected_method_id_list, actual_method_id_list)
+
+  def test_reindexObjectSecurity_on_modules(self):
+    person_module = self.portal.person_module
+    portal_activities = self.portal.portal_activities
+    check = self._checkMessageMethodIdList
+
+    check([])
+    # We need at least one person for this test.
+    self.assertTrue(len(person_module.keys()))
+    # When we update security of a module...
+    person_module.reindexObjectSecurity()
+    transaction.commit()
+    # we don't want all underlying objects to be recursively
+    # reindexed. After all, its contents do not acquire local roles.
+    check(['immediateReindexObject'])
+    self.tic()
+    check([])
+    # But non-module objects, with subobjects that acquire local
+    # roles, should reindex their security recursively:
+    person, = [rec.getObject()
+               for rec in person_module.searchFolder(reference=self.username)]
+    self.assertTrue(len(person.objectIds()))
+    person.reindexObjectSecurity()
+    transaction.commit()
+    check(['recursiveImmediateReindexObject'])
+    self.tic()
+
 def test_suite():
   suite = unittest.TestSuite()
   suite.addTest(unittest.makeSuite(TestUserManagement))

product/ERP5Type/Core/Folder.py

@@ -1253,10 +1253,11 @@ class Folder(CopyContainer, CMFBTreeFolder, CMFHBTreeFolder, Base, FolderMixIn):
   def reindexObjectSecurity(self, *args, **kw):
     """
         Reindex security-related indexes on the object
-        (and its descendants).
     """
-    # In ERP5, simply reindex all objects.
-    self.recursiveReindexObject(*args, **kw)
+    # In ERP5, simply reindex all objects, recursively by default.
+    reindex = self._getTypeBasedMethod('reindexObjectSecurity',
+                                       'recursiveReindexObject')
+    reindex(*args, **kw)
 
   security.declarePublic( 'recursiveReindexObject' )
   def recursiveReindexObject(self, activate_kw=None, **kw):