Fixed a bug that BoundMethod instance in DateTimeField is cached.

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

product/ERP5Form/Form.py

@@ -60,13 +60,29 @@ from Products.Formulator.TALESField import TALESMethod
 from zLOG import LOG, PROBLEM
 
 
+def isCacheable(value):
+  value = aq_base(value)
+  if type(value) is BoundMethod:
+    return False
+
+  jar = getattr(value, '_p_jar', None)
+  if jar is not None:
+    return False
+
+  dic = getattr(value, '__dict__', None)
+  if dic is not None:
+    for i in dic.values():
+      jar = getattr(i, '_p_jar', None)
+      if jar is not None:
+        return False
+  return True
+
+
 def copyMethod(value):
     if type(aq_base(value)) is Method:
       value = Method(value.method_name)
     elif type(aq_base(value)) is TALESMethod:
       value = TALESMethod(value._text)
-    elif type(aq_base(value)) is BoundMethod:
-      value = BoundMethod(value.object, value.method_name)
     return value
 
 
@@ -77,8 +93,6 @@ class StaticValue:
     value as is)
   """
   def __init__(self, value):
-    if isinstance(aq_base(value), (Method, TALESMethod)):
-      value = copyMethod(value)
     self.value = value
 
   def __call__(self, field, id, **kw):
@@ -162,8 +176,6 @@ class TALESValue(StaticValue):
 
 class OverrideValue(StaticValue):
   def __init__(self, override):
-    if isinstance(aq_base(override), (Method, TALESMethod)):
-      override = copyMethod(override)
     self.override = override
 
   def __call__(self, field, id, **kw):
@@ -172,8 +184,6 @@ class OverrideValue(StaticValue):
 class DefaultValue(StaticValue):
   def __init__(self, field_id, value):
     self.key = field_id[3:]
-    if isinstance(aq_base(value), (Method, TALESMethod)):
-      value = copyMethod(value)
     self.value = value
 
   def __call__(self, field, id, **kw):
@@ -209,38 +219,42 @@ class EditableValue(StaticValue):
 
 def getFieldValue(self, field, id, **kw):
   """
-    Return a callable expression
+    Return a callable expression and cacheable boolean flag
   """
   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)
+    tales_expr = copyMethod(tales_expr)
+    return TALESValue(tales_expr), isCacheable(tales_expr)
 
   override = self.overrides.get(id, "")
   if override:
-    return OverrideValue(override)
+    override = copyMethod(override)
+    return OverrideValue(override), isCacheable(override)
 
   # Get a normal value.
   value = self.get_orig_value(id)
+  value = copyMethod(value)
+  cacheable = isCacheable(value)
 
   field_id = field.id
 
   if id == 'default' and field_id.startswith('my_'):
-    return DefaultValue(field_id, value)
+    return DefaultValue(field_id, value), cacheable
 
   # For the 'editable' value, we try to get a default value
   if id == 'editable':
-    return EditableValue(value)
+    return EditableValue(value), cacheable
 
   # Return default value in callable mode
   if callable(value):
-    return StaticValue(value)
+    return StaticValue(value), cacheable
 
   # Return default value in non callable mode
-  return StaticValue(value)(field, id, **kw)
+  return_value = StaticValue(value)(field, id, **kw)
+  return return_value, isCacheable(return_value)
 
 def get_value(self, id, **kw):
   REQUEST = get_request()
@@ -257,20 +271,19 @@ def get_value(self, id, **kw):
   if self._p_oid is None:
     return self._original_get_value(id, **kw)
 
-  if 1:
-    value = getFieldValue(self, field, id, **kw)
-  else:
-    cache_id = ('Form.get_value',
-                self._p_oid,
-                field._p_oid,
-                id)
+  cache_id = ('Form.get_value',
+              self._p_oid,
+              field._p_oid,
+              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)
+  try:
+    value = _field_value_cache[cache_id]
+  except KeyError:
+    # either returns non callable value (ex. "Title")
+    # or a FieldValue instance of appropriate class
+    value, cacheable = getFieldValue(self, field, id, **kw)
+    if cacheable:
+      _field_value_cache[cache_id] = value
 
   if callable(value):
     return value(field, id, **kw)
@@ -788,7 +801,7 @@ class ERP5Form(ZMIForm, ZopePageTemplate):
             type_b = type(b)
             if type_a is not type_b:
                 return False
-            elif type_a is Method or type_a is BoundMethod:
+            elif type_a is Method:
                 return a.method_name==b.method_name
             elif type_a is TALESMethod:
                 return a._text==b._text

product/ERP5Form/ProxyField.py

@@ -52,6 +52,7 @@ from Globals import DTMLFile
 
 from Products.Formulator.TALESField import TALESMethod
 from Products.ERP5Form.Form import StaticValue, TALESValue, OverrideValue, DefaultValue, EditableValue
+from Products.ERP5Form.Form import copyMethod, isCacheable
 
 _USE_ORIGINAL_GET_VALUE_MARKER = []
 
@@ -512,14 +513,15 @@ class ProxyField(ZMIField):
 
   def getFieldValue(self, field, id, **kw):
     """
-      Return a callable expression
+      Return a callable expression and cacheable boolean flag
     """
     try:
       tales_expr = self.get_tales_expression(id)
     except ValueError:
-      return None
+      return None, False
     if tales_expr:
-      return TALESValue(tales_expr)
+      tales_expr = copyMethod(tales_expr)
+      return TALESValue(tales_expr), isCacheable(tales_expr)
 
     # FIXME: backwards compat hack to make sure overrides dict exists
     if not hasattr(self, 'overrides'):
@@ -527,34 +529,39 @@ class ProxyField(ZMIField):
 
     override = self.overrides.get(id, "")
     if override:
-      return OverrideValue(override)
+      override = copyMethod(override)
+      return OverrideValue(override), isCacheable(override)
 
     # Get a normal value.
     try:
       template_field = self.getRecursiveTemplateField()
       # Old ListBox instance might have default attribute. so we need to check it.
       if checkOriginalGetValue(template_field, id):
-        return _USE_ORIGINAL_GET_VALUE_MARKER
+        return _USE_ORIGINAL_GET_VALUE_MARKER, True
       value = self.get_recursive_orig_value(id)
     except KeyError:
       # For ListBox and other exceptional fields.
-      return self._get_value(id, **kw)
+      return self._get_value(id, **kw), False
 
     field_id = field.id
 
+    value = copyMethod(value)
+    cacheable = isCacheable(value)
+
     if id == 'default' and field_id.startswith('my_'):
-      return DefaultValue(field_id, value)
+      return DefaultValue(field_id, value), cacheable
 
     # For the 'editable' value, we try to get a default value
     if id == 'editable':
-      return EditableValue(value)
+      return EditableValue(value), cacheable
 
     # Return default value in callable mode
     if callable(value):
-      return StaticValue(value)
+      return StaticValue(value), cacheable
 
     # Return default value in non callable mode
-    return StaticValue(value)(field, id, **kw)
+    return_value = StaticValue(value)(field, id, **kw)
+    return return_value, isCacheable(return_value)
 
   security.declareProtected('Access contents information', 'get_value')
   def get_value(self, id, **kw):
@@ -584,7 +591,9 @@ class ProxyField(ZMIField):
     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)
+      value, cacheable = self.getFieldValue(field, id, **kw)
+      if cacheable:
+        _field_value_cache[cache_id] = value
 
     if value is _USE_ORIGINAL_GET_VALUE_MARKER:
       return self.getTemplateField().get_value(id, **kw)

product/ERP5Form/tests/testFields.py

@@ -52,7 +52,8 @@ ZopeTestCase.installProduct('ERP5Form')
 
 from Products.Formulator.StandardFields import FloatField
 from Products.Formulator.StandardFields import StringField
-from Products.Formulator.MethodField import Method
+from Products.Formulator.StandardFields import DateTimeField
+from Products.Formulator.MethodField import Method, BoundMethod
 from Products.Formulator.TALESField import TALESMethod
 
 from Products.ERP5Type.Core.Folder import Folder
@@ -280,10 +281,16 @@ class TestFieldValueCache(unittest.TestCase):
     form.proxy_field_tales._p_oid = makeDummyOid()
     form.proxy_field_tales.tales['form_id'] = TALESMethod('string:form')
     form.proxy_field_tales.tales['field_id'] = TALESMethod('string:field')
+    # datetime field (input style is list)
+    form.datetime_field = DateTimeField('datetime_field')
+    form.datetime_field._p_oid = makeDummyOid()
+    form.datetime_field._edit(dict(input_style='list'))
+    for i in form.datetime_field.sub_form.fields.values():
+      i._p_oid = makeDummyOid()
 
   def test_method_field(self):
     field = self.root.form.field
-    value = getFieldValue(field, field, 'external_validator')
+    value, cacheable = getFieldValue(field, field, 'external_validator')
     self.assertEqual(False, value.value is field.values['external_validator'])
     self.assertEqual(True, type(value.value) is Method)
 
@@ -312,6 +319,41 @@ class TestFieldValueCache(unittest.TestCase):
     self.root.form.proxy_field_tales.get_value('title')
     self.assertEqual(True, cache_size == len(ProxyField._field_value_cache))
 
+  def test_datetime_field(self):
+    purgeFieldValueCache()
+    
+    # make sure that boundmethod must not be cached.
+    year_field = self.root.form.datetime_field.sub_form.get_field('year', include_disabled=1)
+    self.assertEqual(True, type(year_field.overrides['items']) is BoundMethod)
+
+    cache_size = len(Form._field_value_cache)
+    year_field.get_value('items')
+
+    # See Formulator/StandardFields.py(line:174)
+    # there are two get_value, start_datetime and end_datetime
+    cache_size += 2
+
+    # make sure that boundmethod is not cached(cache size does not change)
+    self.assertEqual(True, ('Form.get_value',
+                            self.root.form.datetime_field._p_oid,
+                            self.root.form.datetime_field._p_oid,
+                            'start_datetime'
+                            ) in Form._field_value_cache)
+    self.assertEqual(True, ('Form.get_value',
+                            self.root.form.datetime_field._p_oid,
+                            self.root.form.datetime_field._p_oid,
+                            'end_datetime'
+                            ) in Form._field_value_cache)
+    self.assertEqual(False, ('Form.get_value',
+                            year_field._p_oid,
+                            year_field._p_oid,
+                            'items'
+                            ) in Form._field_value_cache)
+    self.assertEqual(cache_size, len(Form._field_value_cache))
+
+    year_field.get_value('size')
+    year_field.get_value('default')
+    self.assertEqual(cache_size+2, len(Form._field_value_cache))
 
 def makeDummyOid():
   import time, random