Commit e5587493 authored by Nicolas Wavrant's avatar Nicolas Wavrant

erp5_trade: move the composition cache in BusinessProcess.py

Since BusinessProcess.py was moved to portal_components, the cache in mixin/composition.py,
which was kept at the level of the zope process, was causing issue when portal_components
was reset : the classes kept in __class_cache would not get resetted, and later calls
to asComposedDocument would reuse them. In consequence, these classes would kept pointers
to objects in the old class BusinessProcess, causing bugs as these objects would have
been cleaned by the reset.

The first intuition was to remove this cache, unfortunately the reason of this introduction
(fixing a memory leak) is still valid nowadays, so I've decided to move this cache within
BusinessProcess, as it has the advantage of emptying it whenever portal_components is resetted.

For more information of the bug mentionned above, and the trials about removing the cache can
be found in the discussion of this MR :
nexedi/erp5!1032
parent 22a6aaad
......@@ -120,6 +120,12 @@ class BusinessProcess(Path, XMLObject):
zope.interface.implements(IBusinessProcess,
interfaces.IArrowBase)
# Cache used by composition mixin (as a composed document always inherit
# from BusinessProcess. The cache avoids memory leak, and has been moved
# here to be automatically flushed when BusinessProcess class is reseted
# by portal_components
asComposedDocument__class_cache = {}
# ITradeModelPathProcess implementation
security.declareProtected(Permissions.AccessContentsInformation, 'getTradeModelPathValueList')
def getTradeModelPathValueList(self, trade_phase=None, context=None, **kw):
......
......@@ -119,18 +119,16 @@ class asComposedDocument(object):
This class should be seen as a function, and it is named accordingly.
It is out of CompositionMixin class to avoid excessive indentation.
"""
# Cache created classes to make other caches (like Base.aq_portal_type)
# useful and avoid memory leaks.
__class_cache = {}
def __new__(cls, orig_self, portal_type_list=None):
from erp5.component.document.BusinessProcess import BusinessProcess
class_cache = BusinessProcess.asComposedDocument__class_cache
self = orig_self.asContext(_portal_type_list=portal_type_list) # XXX-JPS orig_self -> original_self - please follow conventions
base_class = self.__class__
try:
self.__class__ = cls.__class_cache[base_class]
self.__class__ = class_cache[base_class]
except KeyError:
from erp5.component.document.BusinessProcess import BusinessProcess
cls.__class_cache[base_class] = self.__class__ = \
class_cache[base_class] = self.__class__ = \
type(base_class.__name__, (cls, base_class, BusinessProcess), {})
# here we could inherit many "useful" classes dynamically - héhé
# that would be a "real" abstract composition system
......
......@@ -3232,6 +3232,60 @@ class TestZodbMixinComponent(TestZodbInterfaceComponent):
person_type.setTypeMixinList(person_original_mixin_type_list)
self.commit()
class TestZodbDocumentComponentReload(TestZodbDocumentComponent):
def getBusinessTemplateList(self):
return (
'erp5_core',
'erp5_base',
'erp5_pdm',
'erp5_simulation',
'erp5_trade',
)
def _setBusinessProcessComponentTextContent(self, value):
component = self.portal.portal_components['document.erp5.BusinessProcess']
component.setTextContent(value)
self.tic()
def testAsComposedDocumentCacheIsCorrectlyFlushed(self):
component = self.portal.portal_components['document.erp5.BusinessProcess']
component_original_text_content = component.getTextContent()
# Use try/finally to restore the content of
# document.erp5.BusinessProcess as using addCleanup function here raises
# with :
# ConnectionStateError: Shouldn't load state for
# Products.DCWorkflow.Scripts.Scripts 0x099ad38a68606074
# when the connection is closed
try:
self._setBusinessProcessComponentTextContent(
component_original_text_content + """
def getVersion(self):
return 1
"""
)
movement = self.portal.newContent(portal_type='Movement')
composed_movement = movement.asComposedDocument()
self.assertEqual(composed_movement.getVersion(), 1)
self._setBusinessProcessComponentTextContent(
component_original_text_content + """
def getVersion(self):
return 2
"""
)
composed_movement = movement.asComposedDocument()
self.assertEqual(composed_movement.getVersion(), 2)
finally:
self._setBusinessProcessComponentTextContent(
component_original_text_content
)
def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestPortalTypeClass))
......@@ -3241,4 +3295,5 @@ def test_suite():
suite.addTest(unittest.makeSuite(TestZodbTestComponent))
suite.addTest(unittest.makeSuite(TestZodbInterfaceComponent))
suite.addTest(unittest.makeSuite(TestZodbMixinComponent))
suite.addTest(unittest.makeSuite(TestZodbDocumentComponentReload))
return suite
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment