Cleanup API of TransactionalVariable and read-only cache

- New readOnlyTransactionCache context manager replacing
  enableReadOnlyTransactionCache and disableReadOnlyTransactionCache
- Drop compatibility code that tolerate a dummy context to be passed to
  getReadOnlyTransactionCache and getTransactionalVariable.

bt5/erp5_email_reader/DocumentTemplateItem/EmailReader.py

@@ -32,7 +32,7 @@ from Products.CMFCore.utils import getToolByName
 from Products.ERP5Type import Permissions, PropertySheet
 from Products.ERP5.Document.ExternalSource import ExternalSource
 from Products.ERP5Type.XMLObject import XMLObject
-from Products.ERP5Type.Cache import getReadOnlyTransactionCache, enableReadOnlyTransactionCache
+from Products.ERP5Type.Cache import getReadOnlyTransactionCache
 
 # IMAP imports
 import imaplib
@@ -442,18 +442,13 @@ class EmailReader(ExternalSource):
       Returns the list of folders of the current server
       XXX Add read only transaction cache
     """
-    enabled = (getReadOnlyTransactionCache(self) is not None)
-    if not enabled:
-      enableReadOnlyTransactionCache(self)
-    cache = getReadOnlyTransactionCache(self)
+    cache = getReadOnlyTransactionCache()
     if cache is not None:
       key = ('getMessageFolderList', self)
       try:
         return cache[key]
       except KeyError:
         pass
-    else:
-      raise "No Conversion Cache" # XXX - Implement this better
 
     server = self._getMailServer()
     if server is None: return ()
@@ -473,18 +468,13 @@ class EmailReader(ExternalSource):
       break things. An interactor is required to clear
       the variable
     """
-    enabled = (getReadOnlyTransactionCache(self) is not None)
-    if not enabled:
-      enableReadOnlyTransactionCache(self)
-    cache = getReadOnlyTransactionCache(self)
+    cache = getReadOnlyTransactionCache()
     if cache is not None:
       key = ('_getMailServer', self)
       try:
         return cache[key]
       except KeyError:
         pass
-    else:
-      raise "No Conversion Cache" # XXX - Implement this better
 
     # No server defined
     if not self.getURLServer(): return None

bt5/erp5_email_reader/bt/revision

-28
\ No newline at end of file
+29
\ No newline at end of file

product/ERP5/Document/BusinessProcess.py

@@ -617,7 +617,7 @@ class BusinessProcess(Path, XMLObject):
     """
     remaining_trade_phase_list = []
     trade_state = business_link.getSuccessor()
-    tv = getTransactionalVariable(self)
+    tv = getTransactionalVariable()
     # We might need a key which depends on the explanation
     key = 'BusinessProcess_predecessor_successor_%s' % self.getRelativeUrl()
     predecessor_successor_dict = tv.get(key, None)

product/ERP5/Document/ContributionPredicate.py

@@ -31,7 +31,7 @@ from AccessControl import ClassSecurityInfo
 from Products.ERP5Type.Core.Predicate import Predicate
 from Products.ERP5Type.XMLObject import XMLObject
 from Products.ERP5Type import Permissions, PropertySheet
-from Products.ERP5Type.Cache import getReadOnlyTransactionCache, enableReadOnlyTransactionCache, disableReadOnlyTransactionCache
+from Products.ERP5Type.Cache import readOnlyTransactionCache
 
 
 class ContributionPredicate(Predicate, XMLObject):
@@ -94,13 +94,9 @@ class ContributionPredicate(Predicate, XMLObject):
           tested_base_category_list]
 
     # Test category memberships. Enable the read-only transaction cache
-    # temporarily, if not enabled, because this part is strictly read-only,
-    # and context.isMemberOf is very expensive, when the category list has
-    # many items.
-    enabled = getReadOnlyTransactionCache() is not None
-    try:
-      if not enabled:
-        enableReadOnlyTransactionCache()
+    # because this part is strictly read-only, and context.isMemberOf
+    # is very expensive when the category list has many items.
+    with readOnlyTransactionCache():
       for c in membership_criterion_category_list:
         bc = c.split('/', 1)[0]
         if (bc not in tested_base_category) and \
@@ -115,9 +111,6 @@ class ContributionPredicate(Predicate, XMLObject):
         elif (bc in membership_criterion_base_category_list):
           tested_base_category[bc] = tested_base_category[bc] or \
                                      context.isMemberOf(c)
-    finally:
-      if not enabled:
-        disableReadOnlyTransactionCache()
 
     result = result and (0 not in tested_base_category.values())
     # Test method calls

product/ERP5/mixin/builder.py

@@ -755,18 +755,17 @@ class BuilderMixin(XMLObject, Amount, Predicate):
     return obj
 
   def _isUpdated(self, obj, level):
-    tv = getTransactionalVariable(self)
-    return level in tv['builder_processed_list'].get(obj, [])
+    tv = getTransactionalVariable()
+    return level in tv['builder_processed_list'].get(obj, ())
 
   def _setUpdated(self, obj, level):
-    tv = getTransactionalVariable(self)
-    if tv.get('builder_processed_list', None) is None:
+    tv = getTransactionalVariable()
+    if 'builder_processed_list' not in tv:
       self._resetUpdated()
-    tv['builder_processed_list'][obj] = \
-       tv['builder_processed_list'].get(obj, []) + [level]
+    tv['builder_processed_list'].setdefault(obj, []).append(level)
 
   def _resetUpdated(self):
-    tv = getTransactionalVariable(self)
+    tv = getTransactionalVariable()
     tv['builder_processed_list'] = {}
 
   # for backward compatibilities.

product/ERP5Type/Cache.py

@@ -28,6 +28,7 @@
 ##############################################################################
 
 import string
+from contextlib import contextmanager
 from time import time
 from AccessControl import allow_class, ClassSecurityInfo
 from Acquisition import aq_base
@@ -36,7 +37,7 @@ from CachePlugins.BaseCache import CachedMethodError
 from persistent import Persistent
 from zLOG import LOG, WARNING
 from Products.ERP5Type import Permissions
-from Products.ERP5Type.TransactionalVariable import getTransactionalVariable, _MARKER
+from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
 from Products.ERP5Type.Utils import simple_decorator
 from warnings import warn
 
@@ -291,20 +292,22 @@ allow_class(CachingMethod)
 # TransactionCache is a cache per transaction. The purpose of this cache is
 # to accelerate some heavy read-only operations. Note that this must not be
 # enabled when a transaction may modify ZODB objects.
-def getReadOnlyTransactionCache(context=_MARKER):
+def getReadOnlyTransactionCache():
   """Get the transaction cache.
   """
-  return getTransactionalVariable(context).get('read_only_transaction_cache')
+  return getTransactionalVariable().get('read_only_transaction_cache')
 
-def enableReadOnlyTransactionCache(context=_MARKER):
-  """Enable the transaction cache.
-  """
-  getTransactionalVariable(context)['read_only_transaction_cache'] = {}
-
-def disableReadOnlyTransactionCache(context=_MARKER):
-  """Disable the transaction cache.
-  """
-  getTransactionalVariable(context).pop('read_only_transaction_cache', None)
+@contextmanager
+def readOnlyTransactionCache():
+  tv = getTransactionalVariable()
+  if 'read_only_transaction_cache' in tv:
+    yield
+  else:
+    tv['read_only_transaction_cache'] = {}
+    try:
+      yield
+    finally:
+      del tv['read_only_transaction_cache']
 
 ########################################################
 ## Old global cache functions                         ##

product/ERP5Type/Core/Predicate.py

@@ -40,7 +40,7 @@ from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
 from Products.ERP5Type.Document import newTempBase
 from Products.ERP5Type.XMLObject import XMLObject
 from Products.ERP5Type.Utils import convertToUpperCase
-from Products.ERP5Type.Cache import getReadOnlyTransactionCache, enableReadOnlyTransactionCache, disableReadOnlyTransactionCache
+from Products.ERP5Type.Cache import readOnlyTransactionCache
 from Products.ZSQLCatalog.SQLCatalog import SQLQuery
 from Products.ERP5Type.Globals import PersistentMapping
 from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod
@@ -150,13 +150,9 @@ class Predicate(XMLObject):
           tested_base_category_list]
 
     # Test category memberships. Enable the read-only transaction cache
-    # temporarily, if not enabled, because this part is strictly read-only,
-    # and context.isMemberOf is very expensive, when the category list has
-    # many items.
-    enabled = getReadOnlyTransactionCache() is not None
-    try:
-      if not enabled:
-        enableReadOnlyTransactionCache()
+    # because this part is strictly read-only, and context.isMemberOf
+    # is very expensive when the category list has many items.
+    with readOnlyTransactionCache():
       for c in membership_criterion_category_list:
         bc = c.split('/', 1)[0]
         if (bc not in tested_base_category) and \
@@ -176,10 +172,6 @@ class Predicate(XMLObject):
           tested_base_category[bc] = tested_base_category[bc] or \
                                      context.isMemberOf(c,
                                          strict_membership=strict_membership)
-    finally:
-      if not enabled:
-        disableReadOnlyTransactionCache()
-
 #        LOG('predicate test', 0,
 #            '%s after single membership to %s' % \
 #            (tested_base_category[bc], c))

product/ERP5Type/TransactionalVariable.py

@@ -102,13 +102,8 @@ class TransactionalVariable(dict):
 
 transactional_variable_pool = local()
 
-_MARKER = object()
-def getTransactionalVariable(context=_MARKER):
+def getTransactionalVariable():
   """Return a transactional variable."""
-  if context is not _MARKER:
-    warnings.warn("Passing a parameter to getTransactionalVariable() is"
-                  " deprecated and will not be tolerated in the future",
-                  DeprecationWarning)
   try:
     return transactional_variable_pool.instance
   except AttributeError:

product/ZSQLCatalog/SQLCatalog.py

@@ -39,6 +39,7 @@ import string
 import pprint
 import re
 import warnings
+from contextlib import contextmanager
 from cStringIO import StringIO
 from xml.dom.minidom import parse
 from xml.sax.saxutils import escape, quoteattr
@@ -79,17 +80,17 @@ try:
 except ImportError:
   psyco = None
 
+@contextmanager
+def noReadOnlyTransactionCache():
+  yield
 try:
-  from Products.ERP5Type.Cache import enableReadOnlyTransactionCache, \
-       disableReadOnlyTransactionCache, caching_instance_method
+  from Products.ERP5Type.Cache import \
+    readOnlyTransactionCache, caching_instance_method
 except ImportError:
   LOG('SQLCatalog', WARNING, 'Count not import caching_instance_method, expect slowness.')
-  def doNothing(context):
-    pass
   def caching_instance_method(*args, **kw):
     return lambda method: method
-  enableReadOnlyTransactionCache = doNothing
-  disableReadOnlyTransactionCache = doNothing
+  readOnlyTransactionCache = noReadOnlyTransactionCache
 
 try:
   from Products.ERP5Type.TransactionalVariable import getTransactionalVariable
@@ -1502,10 +1503,8 @@ class Catalog(Folder,
     econtext = getEngine().getContext()
     argument_cache = {}
 
-    try:
-      if not disable_cache:
-        enableReadOnlyTransactionCache()
-
+    with (noReadOnlyTransactionCache if disable_cache else
+          readOnlyTransactionCache)():
       filter_dict = self.filter_dict
       catalogged_object_list_cache = {}
       for method_name in method_id_list:
@@ -1626,9 +1625,6 @@ class Catalog(Folder,
           LOG('SQLCatalog', WARNING, 'could not catalog objects %s with method %s' % (object_list, method_name),
               error=sys.exc_info())
           raise
-    finally:
-      if not disable_cache:
-        disableReadOnlyTransactionCache()
 
   if psyco is not None:
     psyco.bind(_catalogObjectList)