ERP5Type: micro-optimize edit/getProperty

Optimize edit by calculating the set of restricted accessors only once
at class generation instead of calculating it each call.
Optimize edit by using the same technique to decide if getattr or
guarded_getattr has to be used to get the property.

TempBase had to be special-cased to keep compatibility with some
existing usages of calling TempBase.edit(modification_date=...) where
somewhere in CMF classes a protected setModificationDate exists. Instead
of rewriting existing cases to use something lighter (why do we need
TempBase anyway ?), we do minimal compatibility fix for TempBase only.

Reuse the INFINITE_SET from matrix after moving it to a shared place,
since this is also useful here.

product/ERP5Type/Base.py

@@ -67,6 +67,7 @@ from Products.ERP5Type.patches.CMFCoreSkinnable import SKINDATA, skinResolve
 from Products.ERP5Type.Utils import UpperCase
 from Products.ERP5Type.Utils import convertToUpperCase, convertToMixedCase
 from Products.ERP5Type.Utils import createExpressionContext, simple_decorator
+from Products.ERP5Type.Utils import INFINITE_SET
 from Products.ERP5Type.Accessor.Accessor import Accessor
 from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
 from Products.ERP5Type.Accessor.TypeDefinition import list_types
@@ -755,6 +756,10 @@ class Base( CopyContainer,
   aq_method_generating = []
   aq_portal_type = {}
   aq_related_generated = 0
+  # These sets are generated when dynamically making a portal type class to
+  # short-cut guarded_getattr in edit/getProperty. For classes that are not
+  # dynamically generated from portal type, we always check security.
+  _restricted_getter_set = _restricted_setter_set = INFINITE_SET
   # Only generateIdList may look at this property. Anything else is unsafe.
   _id_generator_state = None
 
@@ -1222,7 +1227,15 @@ class Base( CopyContainer,
   def _getProperty(self, key, d=_MARKER, restricted=False, **kw):
     __traceback_info__ = (key,)
     accessor_name = 'get' + UpperCase(key)
-    _getattr = guarded_getattr if restricted else getattr
+
+    # for restricted properties (ie. properties protected with another permission
+    # that AccessContentsInformation, which was already checked when calling this
+    # getProperty), we use guarded_getattr, other we use a simple getattr that
+    # is slightly faster.
+    _getattr = guarded_getattr \
+        if restricted and accessor_name in self.__class__._restricted_getter_set \
+        else getattr
+
     aq_self = aq_base(self)
     if getattr(aq_self, accessor_name, None) is not None:
       try:
@@ -1470,14 +1483,8 @@ class Base( CopyContainer,
     unordered_key_list = [k for k in key_list if k not in edit_order]
     ordered_key_list = [k for k in edit_order if k in key_list]
     if restricted:
-      # retrieve list of accessors which doesn't use default permissions
-      restricted_method_set = {method
-        for ancestor in self.__class__.mro()
-        for permissions in getattr(ancestor, '__ac_permissions__', ())
-        if permissions[0] not in ('Access contents information',
-                                  'Modify portal content')
-        for method in permissions[1]
-        if method.startswith('set')}
+      # accessors which doesn't use default permissions
+      restricted_method_set = self.__class__._restricted_setter_set
     else:
       restricted_method_set = ()
 
@@ -1512,7 +1519,7 @@ class Base( CopyContainer,
           accessor_name = 'set' + UpperCase(key)
           if accessor_name in restricted_method_set:
             # will raise Unauthorized when not allowed
-            guarded_getattr(self, accessor_name)
+            guarded_getattr(self, accessor_name, None)
         modified_property_dict[key] = old_value
         if key != 'id':
           modified_object_list = _setProperty(key, kw[key])
@@ -3591,9 +3598,10 @@ def removeIContentishInterface(cls):
 removeIContentishInterface(Base)
 
 class TempBase(Base):
-  """
-    If we need Base services (categories, edit, etc) in temporary objects
-    we shoud used TempBase
+  """A  version of Base that does not persist in ZODB.
+
+  This class only has the Base methods, so most of the times it is
+  preferable to use a temporary portal type instead.
   """
   isIndexable = ConstantGetter('isIndexable', value=False)
   isTempDocument = ConstantGetter('isTempDocument', value=True)
@@ -3642,3 +3650,27 @@ class TempBase(Base):
 # allow_class(TempBase) in ERP5Type/Document/__init__.py will trample our
 # ClassSecurityInfo with one that doesn't declare our public methods
 InitializeClass(TempBase)
+
+# TempBase is not a dynamic class, so it does not get _restricted_getter_set
+# and _restricted_setter_set initialized ( this is only about
+# Products.ERP5Type.Document.newTempBase , other temp objects are initialized ).
+# Because TempBase.edit is public, there are existing cases in ERP5 code base
+# where TempBase.edit is called on documents for which current user does not have
+# modify portal content permission, so if we use the default behavior from Base,
+# which is to check security for everything, some usages starts to raise Unauthorized.
+# Also, these calls would becomes slower and the typical use case is to build a list
+# of temp objects for a listbox, so we'd like to keep this fast.
+TempBase._restricted_setter_set = {
+    method for ancestor in TempBase.mro()
+    for permissions in getattr(ancestor, '__ac_permissions__', ())
+    if permissions[0] not in (
+        Permissions.AccessContentsInformation,
+        Permissions.ModifyPortalContent)
+    for method in permissions[1] if method.startswith('set')
+}
+TempBase._restricted_getter_set = {
+    method for ancestor in TempBase.mro()
+    for permissions in getattr(ancestor, '__ac_permissions__', ())
+    if permissions[0] not in (Permissions.AccessContentsInformation, )
+    for method in permissions[1] if method.startswith('get')
+}

product/ERP5Type/Utils.py

@@ -303,6 +303,10 @@ def cartesianProduct(list_of_list):
       append([v] + p)
   return result
 
+# Emulate an infinite-set, ie. returning containing all objects.
+# At this point we don't implement full API compatibility with set.
+INFINITE_SET = type('INFINITE_SET', (object,), {'__contains__': lambda *args: True})()
+
 # Some list operations
 def keepIn(value_list, filter_list):
   # XXX this is [x for x in value_list if x in filter_list]

product/ERP5Type/dynamic/portal_type_class.py

@@ -323,6 +323,21 @@ def generatePortalTypeClass(site, portal_type_name):
   if portal_type_name in core_portal_type_class_dict:
     core_portal_type_class_dict[portal_type_name]['generating'] = False
 
+  attribute_dict['_restricted_setter_set'] = {method
+        for ancestor in base_class_list
+        for permissions in getattr(ancestor, '__ac_permissions__', ())
+        if permissions[0] not in ('Access contents information',
+                                  'Modify portal content')
+        for method in permissions[1]
+        if method.startswith('set')}
+
+  attribute_dict['_restricted_getter_set'] = {method
+        for ancestor in base_class_list
+        for permissions in getattr(ancestor, '__ac_permissions__', ())
+        if permissions[0] not in ('Access contents information', )
+        for method in permissions[1]
+        if method.startswith('get')}
+
   #LOG("ERP5Type.dynamic", INFO,
   #    "Portal type %s loaded with bases %s" \
   #        % (portal_type_name, repr(base_class_list)))

product/ERP5Type/mixin/matrix.py

@@ -30,12 +30,11 @@ from Products.ERP5Type.Globals import InitializeClass, PersistentMapping
 from Acquisition import aq_base
 from AccessControl import ClassSecurityInfo
 from Products.ERP5Type import Permissions
-from Products.ERP5Type.Utils import cartesianProduct
+from Products.ERP5Type.Utils import cartesianProduct, INFINITE_SET
 from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
 
 from zLOG import LOG
 
-INFINITE_SET = type('', (object,), {'__contains__': lambda *args: True})()
 
 class Matrix(object):
   """A mix-in class which provides a matrix like access to objects.