ERP5Type: compatibility for document classes

TODO: update message. old message below (scope and implementation changed since we cover
original Products.*.Document and don't do newTempX )

---

make Products.ERP5Type.Document a dynamic module

Historically all document classes from Products/*/Document/*.py and from
$INSTANCE_HOME/Document/*.py were dynamically loaded in
Products.ERP5Type.Document namespace and objects in ZODB were instances
of Products.ERP5Type.Document.*.* classes. This allowed moving the
python code from one product to another without having to wory about
persistent references in ZODB.
This was done in Products.ERP5Type.Utils.importLocalDocument which was
used for Products/*/Document/*py and $INSTANCE_HOME/Document/*.py but
that was never the case for document components - they are loaded on demand.

When "portal types as classes" have been introduced, we used a new
indirect erp5.portal_type namespace, for similar reasons as the
Products.ERP5Type.Document namespace. The approach to migrate existing
documents was to hook the loading from ZODB ( Base.__setstate__ , monkey
patched from Products/ERP5Type/dynamic/persistent_migration.py ) to
replace the class from the old Products.ERP5Type.Document.* by its
corresponding erp5.portal_type.* class.

This was working fine to migrate documents whose classes were defined in
Products/*/Document/*.py, but now that these classes have been moved to
document components, this does not work anymore, because unlike when
loading a document class from Products/*/Document/X.py we don't create
the corresponding Products.ERP5Type.Document.X module when loading from
X component. As a result, existing documents of class
Products.ERP5Type.Document.X.X which did not get a chance to be migrated
before X was moved to component were now broken. In the observed case,
there was several Address, Telephone or Email created ~10 years ago.

These changes address this issue by making Products.ERP5Type.Document a
dynamic module, ie. changing the previous beaviour of copying all
document classes to Products.ERP5Type.Document by introducing a module
that will dynamically lookup the document classes on demand, first from
erp5.component.document modules, then falling back to the legacy
document classes registry (populated at startup from
Products/*/Document/*.py and $INSTANCE_HOME/Document/*.py)

A side effect of this is that newTempX constructors are now created from
document components as well (but they are still deprecated because their
behavior is really strange: when you call newTempX - X is the name of a
document class, which has a portal_type attribute and newTempX create an
instance of the portal type defined on that class, but the portal type
might be referencing anothing class)

product/ERP5Type/__init__.py

@@ -162,6 +162,9 @@ def initialize( context ):
   Timeout.publisher_timeout = getattr(erp5_conf, 'publisher_timeout', None)
   Timeout.activity_timeout = getattr(erp5_conf, 'activity_timeout', None)
 
+  initialized.append(True)
+
+initialized = []
 from AccessControl.SecurityInfo import allow_module
 from AccessControl.SecurityInfo import ModuleSecurityInfo
 

product/ERP5Type/dynamic/component_package.py

@@ -34,6 +34,7 @@ import sys
 import imp
 import collections
 
+from Products.ERP5Type import initialized as Products_ERP5Type_initialized
 from Products.ERP5.ERP5Site import getSite
 from . import aq_method_lock
 from types import ModuleType
@@ -80,6 +81,10 @@ class ComponentDynamicPackage(ModuleType):
     self._portal_type = portal_type
     self.__version_suffix_len = len('_version')
     self.__fullname_source_code_dict = {}
+    # A mapping of legacy documents (Products.*.Document.{name}) to redirect to the
+    # new component name (erp5.component.document.{name}). We remember this to be
+    # able to clean up theses modules on reset.
+    self.__legacy_document_mapping = {}
 
     # Add this module to sys.path for future imports
     sys.modules[namespace] = self
@@ -109,6 +114,15 @@ class ComponentDynamicPackage(ModuleType):
     perhaps because the Finder of another Component Package could do it or
     because this is a filesystem module...
     """
+
+    # TODO can't we do better than this Products_ERP5Type_initialized ? (register this loader later ?)
+    if fullname.startswith('Products.') and Products_ERP5Type_initialized:
+      # Dynamically handle Products.*.Document namespace for compatibility, when an import
+      # for Products.*.Document.X is requested and there's a document component X, use the component instead.
+      names = fullname.split('.')
+      if not (len(names) == 4 and names[2] == 'Document'):
+        return None
+    else:
       # Ignore imports with a path which are filesystem-only and any
       # absolute imports which does not start with this package prefix,
       # None there means that "normal" sys.path will be used
@@ -127,7 +141,8 @@ class ComponentDynamicPackage(ModuleType):
       # __import__ will first try a relative import, for example
       # erp5.component.XXX.YYY.ZZZ where erp5.component.XXX.YYY is the current
       # Component where an import is done
-      name = fullname[len(self._namespace_prefix):]
+      name = fullname[len(self._namespace_prefix):] if fullname.startswith(self._namespace_prefix) else ''
+
       # name=VERSION_version.REFERENCE
       if '.' in name:
         try:
@@ -173,6 +188,28 @@ class ComponentDynamicPackage(ModuleType):
           if component is not None and component.getValidationState() in ('modified',
                                                                           'validated'):
             break
+        else:
+          # maybe a legacy import in the form Products.*.Document.{name}
+          # If we have a document component which was created to replace this, use the
+          # component instead.
+          names = fullname.split('.')
+          if not (names[0] == 'Products' and len(names) == 4 and names[2] == 'Document'):
+            return None
+          name = names[-1]
+          for version in site.getVersionPriorityNameList():
+            id_ = "%s.%s.%s" % (self._id_prefix, version, name)
+            component = getattr(component_tool, id_, None)
+            if component is not None and component.getValidationState() in ('modified',
+                                                                            'validated'):
+              # Products.ERP5Type.Document is a special case here, because historically
+              # all documents were also dynamically loaded on this module.
+              if names[1] == 'ERP5Type' or component.getSourceReference() == fullname:
+                self.__legacy_document_mapping[fullname] = 'erp5.component.document.%s' % name
+                # TODO maybe it would be more performant to use the versionned module here, since we already know it
+                # but it seems erp5.component.document.X_version.Y and erp5.component.document.Y are not the same module
+                # and modules are loaded twice (a assertIs() test is failing)
+                # self.__legacy_document_mapping[fullname] = 'erp5.component.document.%s_version.%s' % (version, name)
+                break
           else:
             return None
 
@@ -218,6 +255,16 @@ class ComponentDynamicPackage(ModuleType):
     As per PEP-302, raise an ImportError if the Loader could not load the
     module for any reason...
     """
+    if fullname in self.__legacy_document_mapping:
+      module = self.__load_module(self.__legacy_document_mapping[fullname])
+      # TODO: do we need this lock ? is this deadlock-safe ?
+      imp.acquire_lock()
+      try:
+        sys.modules[fullname] = module
+      finally:
+        imp.release_lock()
+      return module
+
     site = getSite()
     name = fullname[len(self._namespace_prefix):]
 
@@ -428,6 +475,10 @@ class ComponentDynamicPackage(ModuleType):
       del sys.modules[module_name]
 
       delattr(package, name)
+    for module_name in list(self.__legacy_document_mapping):
+      sys.modules.pop(module_name, None)
+    self.__legacy_document_mapping.clear()
+
 
 class ToolComponentDynamicPackage(ComponentDynamicPackage):
   def reset(self, *args, **kw):

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -2902,6 +2902,103 @@ class TestGC(XMLObject):
        'gc: collectable <Implements 0x%x>\n' % Implements_id],
       sorted(found_line_list))
 
+  def testProductsERP5TypeDocumentCompatibility(self):
+    """Check that document class also exist in Products.ERP5Type.Document namespace
+    for compatibility.
+
+    We also check that this module is properly reloaded when a document component
+    is modified.
+    """
+    self.failIfModuleImportable('TestProductsERP5TypeDocumentCompatibility')
+
+    test_component = self._newComponent(
+        'TestProductsERP5TypeDocumentCompatibility',
+        """\
+from Products.ERP5Type.Base import Base
+class TestProductsERP5TypeDocumentCompatibility(Base):
+  portal_type = 'Test ProductsERP5TypeDocument Compatibility'
+  generation = 1
+"""
+    )
+    test_component.validate()
+    self.tic()
+
+    self.assertModuleImportable('TestProductsERP5TypeDocumentCompatibility')
+
+    from Products.ERP5Type.Document.TestProductsERP5TypeDocumentCompatibility import TestProductsERP5TypeDocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+    self.assertEqual(TestProductsERP5TypeDocumentCompatibility.generation, 1)
+
+    test_component.setTextContent(
+        """\
+from Products.ERP5Type.Base import Base
+class TestProductsERP5TypeDocumentCompatibility(Base):
+  portal_type = 'Test ProductsERP5TypeDocument Compatibility'
+  generation = 2
+""")
+    self.tic()
+    self.assertModuleImportable('TestProductsERP5TypeDocumentCompatibility')
+    from Products.ERP5Type.Document.TestProductsERP5TypeDocumentCompatibility import TestProductsERP5TypeDocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+    self.assertEqual(TestProductsERP5TypeDocumentCompatibility.generation, 2)
+
+  def testProductsERP5DocumentCompatibility(self):
+    """Check that document class also exist in its original namespace (source_reference)
+
+    Document Component that were moved from file system Products/*/Document needs
+    to be still importable from their initial location, as there might be classes
+    in the database of these instances.
+    """
+    self.failIfModuleImportable('TestProductsERP5DocumentCompatibility')
+
+    test_component = self._newComponent(
+        'TestProductsERP5DocumentCompatibility',
+        """\
+from Products.ERP5Type.Base import Base
+class TestProductsERP5DocumentCompatibility(Base):
+  portal_type = 'Test ProductsERP5Document Compatibility'
+  test_attribute = 'TestProductsERP5DocumentCompatibility'
+"""
+    )
+    test_component.setSourceReference('Products.ERP5.Document.TestProductsERP5DocumentCompatibility')
+    test_component.validate()
+    self.tic()
+
+    self.assertModuleImportable('TestProductsERP5DocumentCompatibility')
+
+    from Products.ERP5.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+    self.assertEqual(TestProductsERP5DocumentCompatibility.test_attribute, 'TestProductsERP5DocumentCompatibility')
+
+    # this also exist in Products.ERP5Type.Document
+    from Products.ERP5Type.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility as TestProductsERP5DocumentCompatibility_from_ProductsERP5Type  # pylint:disable=import-error,no-name-in-module
+    self.assertIs(TestProductsERP5DocumentCompatibility_from_ProductsERP5Type, TestProductsERP5DocumentCompatibility)
+
+    # another component can also import the migrated component from its original name
+    test_component_importing = self._newComponent(
+        'TestComponentImporting',
+        """\
+from Products.ERP5.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility
+class TestComponentImporting(TestProductsERP5DocumentCompatibility):
+  pass
+"""
+    )
+    test_component_importing.validate()
+    self.tic()
+
+    self.assertModuleImportable('TestComponentImporting')
+    from erp5.component.document.TestComponentImporting import TestComponentImporting  # pylint:disable=import-error,no-name-in-module
+
+    from Products.ERP5.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+    self.assertTrue(issubclass(TestComponentImporting, TestProductsERP5DocumentCompatibility))
+
+    test_component.invalidate()
+    self.tic()
+
+    # after invalidating the component, the legacy modules are no longer importable
+    with self.assertRaises(ImportError):
+      from Products.ERP5.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+    with self.assertRaises(ImportError):
+      from Products.ERP5Type.Document.TestProductsERP5DocumentCompatibility import TestProductsERP5DocumentCompatibility  # pylint:disable=import-error,no-name-in-module
+
+
 from Products.ERP5Type.Core.TestComponent import TestComponent
 
 class TestZodbTestComponent(_TestZodbComponent):

product/ERP5Type/tests/testERP5Type.py

@@ -51,7 +51,7 @@ from Products.ERP5Type.Accessor.Constant import PropertyGetter as ConstantGetter
 from AccessControl.SecurityManagement import newSecurityManager
 from AccessControl import getSecurityManager
 from AccessControl import Unauthorized
-from AccessControl.ZopeGuards import guarded_getattr, guarded_hasattr
+from AccessControl.ZopeGuards import guarded_getattr, guarded_hasattr, guarded_import
 from Products.ERP5Type.tests.utils import createZODBPythonScript
 from Products.ERP5Type.tests.utils import removeZODBPythonScript
 from Products.ERP5Type import Permissions
@@ -261,6 +261,15 @@ class TestERP5Type(PropertySheetTestCase, LogInterceptor):
       self.assertEqual(b.isTempObject(), 1)
       self.assertEqual(b.getId(), str(2))
 
+      # Products.ERP5Type.Document.newTempBase is another (not recommended) way
+      # of creating temp objects
+      import Products.ERP5Type.Document
+      o = Products.ERP5Type.Document.newTempBase(self.portal, 'id')
+      self.assertEqual(o.getId(), 'id')
+      self.assertEqual(o.getPortalType(), 'Base Object')
+      self.assertTrue(o.isTempObject())
+      self.assertTrue(guarded_import("Products.ERP5Type.Document", fromlist=["newTempBase"]))
+
       # Test newContent with the temp_object parameter and where a non-temp_object would not be allowed
       o = portal.person_module.newContent(portal_type="Organisation", temp_object=1)
       o.setTitle('bar')
@@ -3320,6 +3329,31 @@ return [
           '<Organisation at /%s/organisation_module/organisation_id>' % self.portal.getId(),
           repr(document))
 
+    def test_products_document_legacy(self):
+      """check document classes defined in Products/*/Document/*.py
+      """
+      # note: this assertion below checks Alarm is really a legacy document class.
+      # if one day Alarm is moved to component, then this test needs to be updated
+      # with another module that lives on the file system.
+      import Products.ERP5.Document.Alarm
+      self.assertIn('product/ERP5/Document/Alarm.py', Products.ERP5.Document.Alarm.__file__)
+
+      # document classes are also dynamically loaded in Products.ERP5Type.Document module
+      from Products.ERP5Type.Document.Alarm import Alarm as Alarm_from_ERP5Type  # pylint:disable=import-error,no-name-in-module
+      self.assertIs(Alarm_from_ERP5Type, Products.ERP5.Document.Alarm.Alarm)
+
+      # a new temp constructor is created
+      from Products.ERP5Type.Document import newTempAlarm  # pylint:disable=import-error,no-name-in-module
+      self.assertIn(Alarm_from_ERP5Type, newTempAlarm(self.portal, '').__class__.mro())
+
+      # temp constructors are deprecated, they issue a warning when called
+      with mock.patch('Products.ERP5Type.Utils.warnings.warn') as warn:
+          newTempAlarm(self.portal, '')
+      warn.assert_called_with(
+          'newTemp*(self, ID) will be removed, use self.newContent(temp_object=True, id=ID, portal_type=...)',
+          DeprecationWarning, 2)
+
+
 class TestAccessControl(ERP5TypeTestCase):
   # Isolate test in a dedicaced class in order not to break other tests
   # when this one fails.