Small optimizations

About getSingleCategoryAcquiredMembershipList, getPhysicalPath is so slow that
that cache was useless.

product/CMFCategory/CategoryTool.py

@@ -256,8 +256,7 @@ class CategoryTool( UniqueObject, Folder, Base ):
         relative_url = \
         self._removeDuplicateBaseCategoryIdInCategoryPath(base_category,
                                                                  relative_url)
-        node = self.unrestrictedTraverse(relative_url)
-        value = node
+        value = self.unrestrictedTraverse(relative_url)
       except (TypeError, KeyError, NotFound):
         value = None
 
@@ -843,10 +842,13 @@ class CategoryTool( UniqueObject, Folder, Base ):
                                       'getSingleCategoryAcquiredMembershipList' )
     def getSingleCategoryAcquiredMembershipList(self, context, base_category, base=0,
                                          spec=(), filter=None, _acquired_object_set=None, **kw ):
+      # XXX: This cache is rarely useful, and the overhead quite important.
+      #      It would certainly become counter-productive if any significative
+      #      improvement was done to the cached methods.
       cache = getReadOnlyTransactionCache()
       if cache is not None:
-        key = ('getSingleCategoryAcquiredMembershipList', context.getPhysicalPath(), base_category, base, spec,
-               filter, str(kw))
+        key = ('getSingleCategoryAcquiredMembershipList', context,
+               base_category, base, spec, filter, repr(kw))
         try:
           return cache[key]
         except KeyError:
@@ -1758,19 +1760,14 @@ class CategoryTool( UniqueObject, Folder, Base ):
         __traceback_info__ = relative_url
 
         validate = getSecurityManager().validate
-        def restrictedGetOb(container, key, default):
+        def restrictedGetOb(container):
           obj = container._getOb(key, None)
-          if obj is not None:
-            try:
-              if not validate(container, container, key, obj):
-                raise Unauthorized('unauthorized access to element %s' % key)
-            except Unauthorized:
-              # if user can't access object try to return default passed
-              if default is not _marker:
-                return default
-              else:
-                raise
-          return obj
+          if obj is None or validate(container, container, key, obj):
+            return obj
+          # if user can't access object try to return default passed
+          if default is _marker:
+            raise Unauthorized('unauthorized access to element %s' % key)
+          return default
 
         # XXX Currently, resolveCategory accepts that a category might
         # not start with a Base Category, but with a Module. This is
@@ -1781,28 +1778,28 @@ class CategoryTool( UniqueObject, Folder, Base ):
         if stack:
           portal = aq_inner(self.getPortalObject())
           key = stack.pop()
-          obj = restrictedGetOb(self, key, default)
+          obj = restrictedGetOb(self)
           if obj is None:
-            obj = restrictedGetOb(portal, key, default)
+            obj = restrictedGetOb(portal)
             if obj is not None:
               obj = obj.__of__(self)
           else:
             while stack:
               container = obj
               key = stack.pop()
-              obj = restrictedGetOb(container, key, default)
+              obj = restrictedGetOb(container)
               if obj is not None:
                 break
-              obj = restrictedGetOb(self, key, default)
+              obj = restrictedGetOb(self)
               if obj is None:
-                obj = restrictedGetOb(portal, key, default)
+                obj = restrictedGetOb(portal)
                 if obj is not None:
                   obj = obj.__of__(container)
                 break
 
           while obj is not None and stack:
             key = stack.pop()
-            obj = restrictedGetOb(obj, key, default)
+            obj = restrictedGetOb(obj)
 
         if obj is None:
           LOG('CMFCategory', WARNING,

product/ERP5Type/Accessor/List.py

@@ -223,43 +223,44 @@ class ListGetter(Base.Getter):
     func_code.co_argcount = 1
     func_defaults = ()
 
+    _list_type_list = tuple, list, set
+
     def __init__(self, id, key, property_type, default=None, storage_id=None):
       self._id = id
       self.__name__ = id
       self._key = key
       self._property_type = property_type
       self._null = type_definition[property_type]['null']
+      assert None in self._null and not any(
+          isinstance(x, self._list_type_list) for x in self._null), self._null
       self._default = default
       if storage_id is None:
         storage_id = "%s%s" % (ATTRIBUTE_PREFIX, key)
       self._storage_id = storage_id
       self._is_tales_type = (property_type == 'tales')
 
-    def __call__(self, instance, *args, **kw):
-      if len(args) > 0:
-        default = args[0]
-      else:
-        default = self._default
-      list_value = getattr(aq_base(instance), self._storage_id, default)
-      # We should not use here self._null but None instead XXX
-      if list_value not in self._null:
+    def __call__(aq_base=aq_base, list_type_list=_list_type_list):
+      def __call__(self, instance, *args, **kw):
+        list_value = getattr(aq_base(instance), self._storage_id, None)
+        # We should not use here self._null but None instead XXX
+        if list_value in self._null:
+          list_value = args[0] if args else self._default
+          if list_value in self._null:
+            return list_value
         if self._is_tales_type:
           if kw.get('evaluate', 1):
-            if type(list_value) != type(''):
+            if type(list_value) is not str:
               LOG('ListGetter', 0, 'instance = %r, self._storage_id = %r, list_value = %r' % (instance, self._storage_id, list_value,)) # If we reach this point, something strange is going on
             list_value = evaluateTales(instance=instance, value=list_value)
           else:
             return list_value
         # Even if it is already a list, return a copy as callers
         # expect to be able to modify it on place
-        if isinstance(list_value, (list, tuple, set)):
+        if isinstance(list_value, list_type_list):
           return list(list_value) # Make sure we return a list rather than a tuple
         return list_value
-      if default is None:
-        return None # nothing was defined as default so None is the appropriate value
-      if isinstance(default, (list, tuple, set)):
-        return list(default) # Make sure we return a list rather than a tuple
-      return default
+      return __call__
+    __call__ = __call__()
 
     psyco.bind(__call__)
 

product/ERP5Type/Base.py

@@ -89,7 +89,6 @@ from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod, super_user
 
 from zope.interface import classImplementsOnly, implementedBy
 
-from string import join
 import sys, re
 
 from cStringIO import StringIO
@@ -1718,7 +1717,7 @@ class Base( CopyContainer,
     objectlist = [self.getPhysicalRoot()]
     for element in pathlist[1:] :
       objectlist.append(objectlist[-1][element])
-    return '/' + join([object.getTitle() for object in objectlist[1:]], '/')
+    return '/' + '/'.join(object.getTitle() for object in objectlist[1:])
 
   security.declareProtected(Permissions.AccessContentsInformation, 'getCompactLogicalPath')
   def getCompactLogicalPath(self, REQUEST=None) :
@@ -1730,14 +1729,14 @@ class Base( CopyContainer,
     objectlist = [self.getPhysicalRoot()]
     for element in pathlist[1:] :
       objectlist.append(objectlist[-1][element])
-    return '/' + join([object.getCompactTitle() for object in objectlist[1:]], '/')
+    return '/' + '/'.join(object.getCompactTitle() for object in objectlist[1:])
 
   security.declareProtected(Permissions.AccessContentsInformation, 'getUrl')
   def getUrl(self, REQUEST=None):
     """
       Returns the absolute path of an object
     """
-    return join(self.getPhysicalPath(),'/')
+    return '/'.join(self.getPhysicalPath())
 
   # Old name - for compatibility
   security.declareProtected(Permissions.AccessContentsInformation, 'getPath')