Improve field performance. This changes depends on ERP5Type.Interactor.

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@16501 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ERP5Form/Form.py

@@ -35,6 +35,7 @@ from Products.PageTemplates.ZopePageTemplate import ZopePageTemplate
 from Products.CMFCore.utils import _checkPermission, getToolByName
 from Products.CMFCore.exceptions import AccessControl_Unauthorized
 from Products.ERP5Type import PropertySheet, Permissions
+from Products.ERP5Type.Cache import CachingMethod
 
 from urllib import quote
 from Globals import InitializeClass, PersistentMapping, DTMLFile, get_request
@@ -46,28 +47,55 @@ from Products.ERP5Type.Utils import UpperCase
 from Products.ERP5Type.PsycoWrapper import psyco
 import sys
 
+_field_value_cache = {}
+def purgeFieldValueCache():
+  _field_value_cache.clear()
+
 # Patch the fiels methods to provide improved namespace handling
 
 from Products.Formulator.Field import Field
+from Products.Formulator.TALESField import TALESMethod
 
 from zLOG import LOG, PROBLEM
 
-def get_value(self, id, **kw):
-    """Get value for id."""
-    # FIXME: backwards compat hack to make sure tales dict exists
-    if not hasattr(self, 'tales'):
-        self.tales = {}
+class StaticValue:
+  """
+    Encapsulated a static value in a class
+    (quite heavy, would be faster to store the
+    value as is)
+  """
+  def __init__(self, value):
+    self.value = value
 
-    tales_expr = self.tales.get(id, "")
-    if tales_expr:
+  def __call__(self, field, id, **kw):
+    return self.returnValue(field, id, self.value)
+
+  def returnValue(self, field, id, value):
+    # if normal value is a callable itself, wrap it
+    if callable(value):
+      value = value.__of__(field)
+      #value=value() # Mising call ??? XXX Make sure compatible with listbox methods
+
+    if id == 'default':
+      # We make sure we convert values to empty strings
+      # for most fields (so that we do not get a 'value'
+      # message on screen)
+      # This can be overriden by using TALES in the field
+      if value is None: value = ''
+
+    return value
+
+class TALESValue(StaticValue):
+  def __init__(self, tales_expr):
+    self.tales_expr = tales_expr
+
+  def __call__(self, field, id, **kw):
     REQUEST = get_request()
     if REQUEST is not None:
       # Proxyfield stores the "real" field in the request. Look if the
       # corresponding field exists in request, and use it as field in the
       # TALES context 
-          field = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self)
-        else:
-          field = self
+      field = REQUEST.get('field__proxyfield_%s_%s' % (field.id, id), field)
 
     kw['field'] = field
 
@@ -101,7 +129,7 @@ def get_value(self, id, **kw):
       if getattr(REQUEST, 'cell', None) is not None:
         kw['cell'] = getattr(REQUEST, 'cell')
     try:
-            value = tales_expr.__of__(self)(**kw)
+      value = self.tales_expr.__of__(field)(**kw)
     except (ConflictError, RuntimeError):
       raise
     except:
@@ -109,46 +137,42 @@ def get_value(self, id, **kw):
       # something reasonable rather than generate plenty of errors
       LOG('ERP5Form', PROBLEM,
           'Field.get_value ( %s/%s [%s]), exception on tales_expr: ' %
-                ( form.getId(), self.getId(), id), error=sys.exc_info())
-            value = self.get_orig_value(id)
-    else:
-        # FIXME: backwards compat hack to make sure overrides dict exists
-        if not hasattr(self, 'overrides'):
-            self.overrides = {}
+          ( form.getId(), field.getId(), id), error=sys.exc_info())
+      value = field.get_orig_value(id)
 
-        override = self.overrides.get(id, "")
-        if override:
-            # call wrapped method to get answer
-            value = override.__of__(self)()
-        else:
-            # Get a normal value.
-            value = self.get_orig_value(id)
+    return self.returnValue(field, id, value)
 
-            # For the 'default' value, we try to get a property value
-            # stored in the context, only if the field is prefixed with my_.
-            REQUEST = get_request()
-            if REQUEST is not None:
-              field_id = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id),
-                                      self).id
-            else:
-              field_id = self.id
+class OverrideValue(StaticValue):
+  def __init__(self, override):
+    self.override = override
 
-            if id == 'default' and field_id.startswith('my_'):
+  def __call__(self, field, id, **kw):
+    return self.returnValue(field, id, self.override.__of__(field)())
+
+class DefaultValue(StaticValue):
+  def __init__(self, field_id, value):
+    self.key = field_id[3:]
+    self.value = value
+
+  def __call__(self, field, id, **kw):
     try:
-                form = self.aq_parent
+      form = field.aq_parent
       ob = getattr(form, 'aq_parent', None)
-                key = field_id[3:]
+      value = self.value
       if value not in (None, ''):
         # If a default value is defined on the field, it has precedence
-                  value = ob.getProperty(key, d=value)
+        value = ob.getProperty(self.key, d=value)
       else:
         # else we should give a chance to the accessor to provide
         # a default value (including None)
-                  value = ob.getProperty(key)
+        value = ob.getProperty(self.key)
     except (KeyError, AttributeError):
       value = None
-            # For the 'editable' value, we try to get a default value
-            elif id == 'editable':
+    return self.returnValue(field, id, value)
+
+class EditableValue(StaticValue):
+
+  def __call__(self, field, id, **kw):
     # By default, pages are editable and
     # fields are editable if they are set to editable mode
     # However, if the REQUEST defines editable_mode to 0
@@ -158,20 +182,73 @@ def get_value(self, id, **kw):
     # which defines the current layout
     if kw.has_key('REQUEST'):
       if not getattr(kw['REQUEST'], 'editable_mode', 1):
-                    value = 0
+        self.value = 0
+    return self.value
 
-    # if normal value is a callable itself, wrap it
+def getFieldValue(self, field, id, **kw):
+  """
+    Return a callable expression
+  """
+  # FIXME: backwards compat hack to make sure tales dict exists
+  if not hasattr(self, 'tales'):
+    self.tales = {}
+
+  tales_expr = self.tales.get(id, "")
+  if tales_expr:
+    # TALESMethod is persistent object, so that we cannot cache original one.
+    # Becase if connection which original talesmethod uses is closed,
+    # RuntimeError must occurs in __setstate__.
+    clone = TALESMethod(tales_expr._text)
+    return TALESValue(clone)
+
+  # FIXME: backwards compat hack to make sure overrides dict exists
+  if not hasattr(self, 'overrides'):
+      self.overrides = {}
+
+  override = self.overrides.get(id, "")
+  if override:
+    return OverrideValue(override)
+
+  # Get a normal value.
+  value = self.get_orig_value(id)
+
+  field_id = field.id
+
+  if id == 'default' and field_id.startswith('my_'):
+    return DefaultValue(field_id, value)
+
+  # For the 'editable' value, we try to get a default value
+  if id == 'editable':
+    return EditableValue(value)
+
+  # Return default value in non callable mode
   if callable(value):
-        value = value.__of__(self)
-        #value=value() # Mising call ??? XXX Make sure compatible with listbox methods
+    return StaticValue(value)
 
-    if id == 'default':
-        # We make sure we convert values to empty strings
-        # for most fields (so that we do not get a 'value'
-        # message on screen)
-        # This can be overriden by using TALES in the field
-        if value is None: value = ''
+  # Return default value in non callable mode
+  return StaticValue(value)(field, id, **kw)
+
+def get_value(self, id, **kw):
+  REQUEST = get_request()
+  if REQUEST is not None:
+    field = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self)
+  else:
+    field = self
+
+  cache_id = ('Form.get_value',
+              self._p_oid or repr(self),
+              field._p_oid or repr(field),
+              id)
+
+  try:
+    value = _field_value_cache[cache_id]
+  except KeyError:
+    # either returns non callable value (ex. "Title")
+    # or a FieldValue instance of appropriate class
+    value = _field_value_cache[cache_id] = getFieldValue(self, field, id, **kw)
 
+  if callable(value):
+    return value(field, id, **kw)
   return value
 
 psyco.bind(get_value)
@@ -584,9 +661,7 @@ class ERP5Form(ZMIForm, ZopePageTemplate):
             portal = portal_url.getPortalObject()
             portal_skins = getToolByName(self, 'portal_skins')
 
-            default_field_library_path = portal.getProperty(
-                                  'erp5_default_field_library_path',
-                                  'erp5_core.Base_viewFieldLibrary')
+            default_field_library_path = portal.getProperty('erp5_default_field_library_path', None)
             if (not default_field_library_path or
                 len(default_field_library_path.split('.'))!=2):
                 return
@@ -597,24 +672,21 @@ class ERP5Form(ZMIForm, ZopePageTemplate):
             default_field_library = getattr(skinfolder, form_id, None)
             if default_field_library is None:
                 return
+
+            if not default_field_library_path in form_order:
                 for i in default_field_library.objectValues():
                     field_meta_type, proxy_flag = get_field_meta_type_and_proxy_flag(i)
                     if meta_type==field_meta_type:
                         if proxy_flag:
-                        field_meta_type = '%s(Proxy)' % field_meta_type
+                            field_meta_type = '%s(Proxy' % field_meta_type
                         matched_item = {'form_id':form_id,
                                         'field_type':field_meta_type,
                                         'field_object':i,
                                         'proxy_flag':proxy_flag,
                                         'matched_rate':0
                                         }
-                    if not default_field_library_path in form_order:
                         matched_append(default_field_library_path,
                                        matched_item)
-                    if not default_field_library_path in \
-                                          perfect_matched_form_order:
-                        perfect_matched_append(default_field_library_path,
-                                               matched_item)
 
         id_ = field.getId()
         meta_type = field.meta_type
@@ -654,7 +726,6 @@ class ERP5Form(ZMIForm, ZopePageTemplate):
 
         if perfect_matched:
             perfect_matched_form_order.sort()
-            add_default_field_library()
             return perfect_matched_form_order, perfect_matched
 
         form_order.sort()
@@ -782,6 +853,37 @@ class ERP5Form(ZMIForm, ZopePageTemplate):
     psyco.bind(__call__)
     psyco.bind(_exec)
 
+    # Overload of the Form method
+    #   Use the include_disabled parameter since
+    #   we should consider all fields to render the group tab
+    #   moreoever, listbox rendering fails whenever enabled
+    #   is based on the cell parameter.
+    security.declareProtected('View', 'get_largest_group_length')
+    def get_largest_group_length(self):
+        """Get the largest group length available; necessary for
+        'order' screen user interface.
+        XXX - Copyright issue
+        """
+        max = 0
+        for group in self.get_groups(include_empty=1):
+            fields = self.get_fields_in_group(group, include_disabled=1)
+            if len(fields) > max:
+                max = len(fields)
+        return max
+
+    security.declareProtected('View', 'get_groups')
+    def get_groups(self, include_empty=0):
+        """Get a list of all groups, in display order.
+
+        If include_empty is false, suppress groups that do not have
+        enabled fields.
+        XXX - Copyright issue
+        """
+        if include_empty:
+            return self.group_list
+        return [group for group in self.group_list
+                if self.get_fields_in_group(group, include_disabled=1)]
+
 
 # utility function
 def get_field_meta_type_and_proxy_flag(field):
@@ -789,8 +891,7 @@ def get_field_meta_type_and_proxy_flag(field):
         try:
             return field.getRecursiveTemplateField().meta_type, True
         except AttributeError:
-            raise AttributeError, 'The proxy target of %s field does not '\
-                  'exists. Please check the field setting.' % field.getId()
+            raise AttributeError, 'The proxy target of %s field does not exists. Please check the field setting.' % field.getId()
     else:
         return field.meta_type, False
 
@@ -811,3 +912,7 @@ psyco.bind(Field.get_value)
 
 #from Products.CMFCore.ActionsTool import ActionsTool
 #psyco.bind(ActionsTool.listFilteredActionsFor)
+
+# install interactor
+from Products.ERP5Type.Interactor import fielf_value_interactor
+fielf_value_interactor.install()

product/ERP5Form/ProxyField.py

@@ -31,7 +31,9 @@ from Products.Formulator import Widget, Validator
 from Products.Formulator.Field import ZMIField
 from Products.Formulator.DummyField import fields
 from Products.Formulator.Errors import ValidationError
+from Products.Formulator import MethodField
 from Products.ERP5Type.Utils import convertToUpperCase
+from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
 from Products.CMFCore.utils import getToolByName
 
 from Products.PageTemplates.PageTemplateFile import PageTemplateFile
@@ -44,10 +46,17 @@ from Products.PythonScripts.standard import url_quote_plus
 from AccessControl import ClassSecurityInfo
 from MethodObject import Method
 
-from zLOG import LOG, WARNING, DEBUG
+from zLOG import LOG, WARNING, DEBUG, PROBLEM
 from Acquisition import aq_base, aq_inner, aq_acquire, aq_chain
 from Globals import DTMLFile
 
+from Products.Formulator.TALESField import TALESMethod
+from Products.ERP5Form.ListBox import ListBox
+from Products.ERP5Form.Form import StaticValue, TALESValue, OverrideValue, DefaultValue, EditableValue
+
+_field_value_cache = {}
+def purgeFieldValueCache():
+  _field_value_cache.clear()
 
 class WidgetDelegatedMethod(Method):
   """Method delegated to the proxied field's widget.
@@ -324,6 +333,11 @@ class ProxyField(ZMIField):
     """
     Return template field of the proxy field.
     """
+    try:
+      return self._getTemplateFieldCache()
+    except KeyError:
+      pass
+
     form = self.aq_parent
     object = form.aq_parent
     try:
@@ -335,6 +349,7 @@ class ProxyField(ZMIField):
           'Could not get a field from a proxy field %s in %s' % \
               (self.id, object.id))
       proxy_field = None
+    self._setTemplateFieldCache(proxy_field)
     return proxy_field
 
   def getRecursiveTemplateField(self):
@@ -342,10 +357,12 @@ class ProxyField(ZMIField):
     Return template field of the proxy field.
     This result must not be a ProxyField.
     """
-    template_field = self.getTemplateField()
-    if template_field.__class__ == ProxyField:
-      return template_field.getRecursiveTemplateField()
-    else:
+    field = self
+    while True:
+      template_field = field.getTemplateField()
+      if template_field.__class__ != ProxyField:
+        break
+      field = template_field
     return template_field
 
   security.declareProtected('Access contents information', 
@@ -439,26 +456,6 @@ class ProxyField(ZMIField):
       # ("form_id and field_id don't define a valid template")
       pass
 
-  security.declareProtected('Access contents information', 'get_value')
-  def get_value(self, id, **kw):
-    """Get value for id.
-
-    Optionally pass keyword arguments that get passed to TALES
-    expression.
-    """
-    result = None
-    if (id in self.widget.property_names) or \
-       (not self.is_delegated(id)):
-      result = ZMIField.get_value(self, id, **kw)
-    else:
-      proxy_field = self.getTemplateField()
-      if proxy_field is not None:
-        REQUEST = get_request()
-        REQUEST.set('field__proxyfield_%s_%s' % (proxy_field.id, id),
-                    REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self))
-        result = proxy_field.get_value(id, **kw)
-    return result
-
   security.declareProtected('Access contents information', 'has_value')
   def has_value(self, id):
     """
@@ -485,3 +482,115 @@ class ProxyField(ZMIField):
     else:
       result = ZMIField._get_user_input_value(self, key, REQUEST)
     return result
+
+
+  #
+  # Performance improvement
+  #
+  def get_tales_expression(self, id):
+    field = self
+    while True:
+      if (id in field.widget.property_names or
+          not field.is_delegated(id)):
+        tales = field.get_tales(id)
+        if tales:
+          return TALESMethod(tales._text)
+        else:
+          return None
+      proxied_field = field.getTemplateField()
+      if proxied_field.__class__ == ProxyField:
+        field = proxied_field
+      elif proxied_field is None:
+        raise ValueError, "Can't find the template field of %s" % self.id
+      else:
+        tales = proxied_field.get_tales(id)
+        if tales:
+          return TALESMethod(tales._text)
+        else:
+          return None
+
+  def getFieldValue(self, field, id, **kw):
+    """
+      Return a callable expression
+    """
+    tales_expr = self.get_tales_expression(id)
+    if tales_expr:
+      return TALESValue(tales_expr)
+
+    # FIXME: backwards compat hack to make sure overrides dict exists
+    if not hasattr(self, 'overrides'):
+        self.overrides = {}
+
+    override = self.overrides.get(id, "")
+    if override:
+      return OverrideValue(override)
+
+    # Get a normal value.
+    try:
+      template_field = self.getRecursiveTemplateField()
+      # Old ListBox instance might have default attribute. so we need to check it.
+      if id=='default' and isinstance(aq_base(template_field), ListBox):
+        return self._get_value(id, **kw)
+      value = self.get_recursive_orig_value(id)
+    except KeyError:
+      # For ListBox
+      return self._get_value(id, **kw)
+
+    field_id = field.id
+
+    if id == 'default' and field_id.startswith('my_'):
+      return DefaultValue(field_id, value)
+
+    # For the 'editable' value, we try to get a default value
+    if id == 'editable':
+      return EditableValue(value)
+
+    # Return default value in non callable mode
+    if callable(value):
+      return StaticValue(value)
+
+    # Return default value in non callable mode
+    return StaticValue(value)(field, id, **kw)
+
+  security.declareProtected('Access contents information', 'get_value')
+  def get_value(self, id, **kw):
+    REQUEST = get_request()
+    if ((id in self.widget.property_names) or
+        (not self.is_delegated(id))):
+      return ZMIField.get_value(self, id, **kw)
+
+    field = self
+    proxy_field = self.getTemplateField()
+    if proxy_field is not None and REQUEST is not None:
+      field = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self)
+      REQUEST.set('field__proxyfield_%s_%s' % (proxy_field.id, id), field)
+
+    cache_id = ('ProxyField.get_value',
+                self._p_oid or repr(self),
+                field._p_oid or repr(field),
+                id)
+
+    try:
+      value = _field_value_cache[cache_id]
+    except KeyError:
+      # either returns non callable value (ex. "Title")
+      # or a FieldValue instance of appropriate class
+      value = _field_value_cache[cache_id] = self.getFieldValue(field, id, **kw)
+
+    if callable(value):
+      return value(field, id, **kw)
+    return value
+
+  def _get_value(self, id, **kw):
+    proxy_field = self.getTemplateField()
+    if proxy_field is not None:
+      return proxy_field.get_value(id, **kw)
+
+  def _getCacheId(self):
+    return '%s%s' % ('ProxyField', self._p_oid or repr(self))
+
+  def _setTemplateFieldCache(self, field):
+    getTransactionalVariable(self)[self._getCacheId()] = field
+
+  def _getTemplateFieldCache(self):
+    return getTransactionalVariable(self)[self._getCacheId()].__of__(self.aq_parent)

product/ERP5Form/tests/testFields.py

@@ -53,7 +53,7 @@ from Products.Formulator.StandardFields import FloatField
 from Products.Formulator.StandardFields import StringField
 
 from Products.ERP5Type.Core.Folder import Folder
-from Products.ERP5Form.Form import ERP5Form
+from Products.ERP5Form.Form import ERP5Form, purgeFieldValueCache
 
 
 class TestFloatField(unittest.TestCase):
@@ -75,6 +75,7 @@ class TestFloatField(unittest.TestCase):
     self.field.values['precision'] = 0
     self.assertEquals('12', self.widget.format_value(self.field, 12.34))
 
+    purgeFieldValueCache() # call this before changing internal field values.
     self.field.values['precision'] = 2
     self.assertEquals('0.01', self.widget.format_value(self.field, 0.011))
     # value is rounded

product/ERP5Form/tests/testProxify.py

@@ -50,6 +50,7 @@ ZopeTestCase.installProduct('ERP5Form')
 from Products.Formulator.TALESField import TALESMethod
 from Products.ERP5Type.Core.Folder import Folder
 from Products.ERP5Form.Form import ERP5Form
+from Products.ERP5Form.ProxyField import purgeFieldValueCache
 
 
 class TestProxify(unittest.TestCase):
@@ -103,6 +104,7 @@ class TestProxify(unittest.TestCase):
     self.assertEqual(field.is_delegated('description'), True)
     self.assertEqual(field.get_value('description'), '')
 
+    purgeFieldValueCache() # must purge cache before changing internal field value.
     template_field = self.base_view.my_string_field
     template_field.values['description'] = 'Description'
     self.assertEqual(field.get_value('description'), 'Description')
@@ -131,6 +133,7 @@ class TestProxify(unittest.TestCase):
 
     self.assertEqual(field.has_value('scrap_variable'), 0)
 
+    purgeFieldValueCache() # must purge cache before changing internal field value.
     template_field = self.address_view.my_region
     template_field.values['title'] = 'Region'
     self.assertEqual(field.get_value('title'), 'Region')