ERP5Catalog: Reduce reliance on user lookup.

Use Owner role to help filling user_set: owners must be users, never
groups.
Only look for users among group_ids when they are candidate for indexation
into a catalog_role column. This should very significantly reduce the
number of user lookups, even bringing it to zero on instance with default
catalog setting of catalog_role_set = set(["viewable_owner"]) .
Also, only look for users if there are candidates for lookup.
Also, rename local variable to better describe its content.
Also, use set.__iadd__ instead of set.update (shorter).

product/ERP5Catalog/CatalogTool.py

@@ -118,19 +118,33 @@ class IndexableObjectWrapper(object):
             if role[:1] == '-':
               skip_role(role[1:])
             elif role not in skip_role_set:
+              if role == 'Owner':
+                # Owner role may only be granted to users, not to groups so we
+                # can immediately know this security group id is a user.
+                self.__user_set.add(group_id)
               new_role(role)
           if new_role_list:
             local_role_dict[group_id] = new_role_list
         self.__local_role_cache = local_role_dict
       return local_role_dict
 
-    def _getSecurityGroupIdList(self):
+    def _getSecurityGroupIdGenerator(self):
       """
       Return the list of security group identifiers this document is
-      interested in. They may be actual user identifiers or not.
+      interested to know whether they are users or groups: this only matters
+      for security group ids which are granted at least one role mapping to a
+      role column.
+      They may be user identifiers or group identifiers.
       Supposed to be accessed by CatalogTool.
       """
-      return self.__getLocalRoleDict().keys()
+      no_indexable_role = self.__catalog_role_set.isdisjoint
+      return (
+        group_id
+        for group_id, role_list in self.__getLocalRoleDict().iteritems()
+        if group_id not in self.__user_set and
+          # group_id is returned only if any of its roles is indexable
+          not no_indexable_role(role_list)
+      )
 
     def _getSecurityParameterList(self):
       result = self.__security_parameter_cache
@@ -197,6 +211,8 @@ class IndexableObjectWrapper(object):
         catalog_role_set = self.__catalog_role_set
         user_set = self.__user_set
         for group_id, role_list in self.__getLocalRoleDict().iteritems():
+          # Warning: only valid when group_id is candidate for indexation in a
+          # catalog_role column !
           group_id_is_user = group_id in user_set
           prefix = 'user:' + group_id
           for role in role_list:
@@ -849,13 +865,13 @@ class CatalogTool (UniqueObject, ZCatalog, CMFCoreCatalogTool, ActiveObject):
       catalog_security_uid_groups_columns_dict = catalog_value.getSQLCatalogSecurityUidGroupsColumnsDict()
       default_security_uid_column = catalog_security_uid_groups_columns_dict['']
       getPredicatePropertyDict = catalog_value.getPredicatePropertyDict
-      security_group_set = set()
+      group_and_user_id_set = set()
       wrapper_list = []
       for object_value in object_value_list:
         document_object = aq_inner(object_value)
         w = IndexableObjectWrapper(document_object, user_set, catalog_role_set)
         w.predicate_property_dict = getPredicatePropertyDict(object_value) or {}
-        security_group_set.update(w._getSecurityGroupIdList())
+        group_and_user_id_set += w._getSecurityGroupIdGenerator()
 
         # Find the parent definition for security
         is_acquired = 0
@@ -871,20 +887,22 @@ class CatalogTool (UniqueObject, ZCatalog, CMFCoreCatalogTool, ActiveObject):
             break
         if is_acquired:
           document_w = IndexableObjectWrapper(document_object, user_set, catalog_role_set)
-          security_group_set.update(document_w._getSecurityGroupIdList())
+          group_and_user_id_set += document_w._getSecurityGroupIdGenerator()
         else:
           document_w = w
         wrapper_list.append((document_object, w, document_w))
 
-      # Note: we mutate the set, so all related wrappers get (purposedly)
-      # affected by this, which must happen before _getSecurityParameterList
-      # is called (which happens when calling getSecurityUidDict below).
-      user_set.update(
-        x['id'] for x in portal.acl_users.searchUsers(
-          id=list(security_group_set),
-          exact_match=True,
+      group_and_user_id_set -= user_set
+      if group_and_user_id_set:
+        # Note: we mutate the set, so all related wrappers get (purposedly)
+        # affected by this, which must happen before _getSecurityParameterList
+        # is called (which happens when calling getSecurityUidDict below).
+        user_set += (
+            x['id'] for x in portal.acl_users.searchUsers(
+            id=list(group_and_user_id_set),
+            exact_match=True,
+          )
         )
-      )
 
       getSecurityUidDict = catalog_value.getSecurityUidDict
       getSubjectSetUid = catalog_value.getSubjectSetUid

product/ERP5Catalog/tests/testERP5Catalog.py

@@ -30,7 +30,6 @@
 from random import randint
 import sys
 import unittest
-from unittest import expectedFailure
 from AccessControl import getSecurityManager
 from AccessControl.SecurityManagement import newSecurityManager
 from DateTime import DateTime
@@ -2837,9 +2836,6 @@ VALUES
       sql_catalog.sql_search_tables = current_sql_search_tables
       self.commit()
 
-  # Low priority bug, which needs a lot of time to be fixed
-  # Marked as expectedFailure
-  @expectedFailure
   def test_PersonDocumentWithMonovaluedLocalRole(self):
     """Test when user is added, which has local roles on own Person Document