Only load a Component through import and update tests accordingly.

Before, a Component may have also be loaded through __getattr__ as defined in
DynamicModule but it makes the code unnecessarily more complicated just for
External Method. From now on, use __import__ rather than getattr as it will
work exactly the same way as in Component source code containing imports.

product/ERP5Type/dynamic/component_class.py

@@ -27,47 +27,43 @@
 ##############################################################################
 
 import sys
+import threading
 
-from Products.ERP5Type.dynamic.dynamic_module import DynamicModule
 from Products.ERP5.ERP5Site import getSite
 from types import ModuleType
 from zLOG import LOG, INFO
 
-class ComponentDynamicPackage(DynamicModule):
+class ComponentDynamicPackage(ModuleType):
   """
   A top-level component is a package as it contains modules, this is required
   to be able to add import hooks (as described in PEP 302) when a in the
   source code of a Component, another Component is imported.
 
-  A Component can be loaded in two different ways:
-
-   1/ When erp5.component.extension.XXX is accessed (for example for External
-      Method as per ExternalMethod patch), thus ending up calling __getattr__
-      (DynamicModule) which then load the Component through __call__();
-
-   2/ Upon import, for example in a Document Component with ``import
-      erp5.component.XXX.YYY'', through the Importer Protocol (PEP 302), by
-      adding an instance of this class to sys.meta_path and through
-      find_module() and load_module methods. After that, this is the same as
-      1/.
-
-      This is required because Component classes do not have any physical
-      location on the filesystem, however extra care must be taken for
-      performances because load_module() will be called each time an import is
-      done, therefore the loader should be added to sys.meta_path as late as
-      possible to keep startup time to the minimum.
+  A Component is loaded when being imported, for example in a Document
+  Component with ``import erp5.component.XXX.YYY'', through the Importer
+  Protocol (PEP 302), by adding an instance of this class to sys.meta_path and
+  through find_module() and load_module() methods. The latter method takes
+  care of loading the code into a new module.
+
+  This is required because Component classes do not have any physical location
+  on the filesystem, however extra care must be taken for performances because
+  load_module() will be called each time an import is done, therefore the
+  loader should be added to sys.meta_path as late as possible to keep startup
+  time to the minimum.
   """
   # Necessary otherwise imports will fail because an object is considered a
   # package only if __path__ is defined
   __path__ = []
 
   def __init__(self, namespace, portal_type):
-    super(ComponentDynamicPackage, self).__init__(namespace, self)
+    super(ComponentDynamicPackage, self).__init__(namespace)
 
     self._namespace = namespace
     self._namespace_prefix = namespace + '.'
     self._portal_type = portal_type
 
+    self._lock = threading.RLock()
+
     # Add this module to sys.path for future imports
     sys.modules[namespace] = self
 
@@ -126,21 +122,11 @@ class ComponentDynamicPackage(DynamicModule):
     if module is not None:
       return module
 
-    # Load the module by trying to access it
-    name = fullname.replace(self._namespace_prefix, '')
-    try:
-      module = getattr(self, name)
-    except AttributeError, e:
-      return None
-
-    module.__loader__ = self
-    return module
-
-  def __call__(self, component_name):
     site = getSite()
 
     # XXX-arnau: erp5.component.extension.VERSION.REFERENCE perhaps but there
     # should be a a way to specify priorities such as portal_skins maybe?
+    component_name = fullname.replace(self._namespace_prefix, '')
     component_id = '%s.%s' % (self._namespace, component_name)
     try:
       # XXX-arnau: Performances (~ 200x slower than direct access to ZODB) and
@@ -155,12 +141,12 @@ class ComponentDynamicPackage(DynamicModule):
       # component = getattr(site.portal_components, component_id)
     except IndexError:
       LOG("ERP5Type.dynamic", INFO,
-          "Could not find %s, perhaps it has not been migrated yet?" % \
-            component_id)
+          "Could not find %s or it has not been validated or it has not been "
+          "migrated yet?" % component_id)
 
-      raise AttributeError("Component %s not found or not validated" % \
-                             component_id)
-    else:
+      return None
+
+    with self._lock:
       new_module = ModuleType(component_id, component.getDescription())
 
       # The module *must* be in sys.modules before executing the code in case
@@ -177,4 +163,8 @@ class ComponentDynamicPackage(DynamicModule):
         raise
 
       new_module.__path__ = []
+      new_module.__loader__ = self
+      new_module.__name__ = component_id
+
+      setattr(self, component_name, new_module)
       return new_module

product/ERP5Type/dynamic/portal_type_class.py

@@ -197,9 +197,14 @@ def generatePortalTypeClass(site, portal_type_name):
     type_class_namespace = document_class_registry.get(type_class, '')
     if not (type_class_namespace.startswith('Products.ERP5Type') or
             portal_type_name in core_portal_type_class_dict):
-      import erp5.component.document
-      module = getattr(erp5.component.document, type_class, None)
-      klass = module and getattr(module, type_class, None) or None
+      try:
+        klass = getattr(__import__('erp5.component.document.' + type_class,
+                                   fromlist=['erp5.component.document'],
+                                   level=0),
+                        type_class)
+
+      except (ImportError, AttributeError):
+        pass
 
     if klass is None:
       type_class_path = document_class_registry.get(type_class, None)

product/ERP5Type/patches/ExternalMethod.py

@@ -21,11 +21,13 @@ ExternalMethod.getFunction = getFunction
 
 ExternalMethod__call__ = ExternalMethod.__call__
 def __call__(self, *args, **kw):
-  import erp5.component.extension
   try:
-    f = getattr(getattr(erp5.component.extension, self._module),
+    f = getattr(__import__('erp5.component.extension.' + self._module,
+                           fromlist=['erp5.component.extension'],
+                           level=0),
                 self._function)
-  except AttributeError:
+
+  except (ImportError, AttributeError):
     return ExternalMethod__call__(self, *args, **kw)
   else:
     _v_f = getattr(self, '_v_f', None)

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -1240,6 +1240,31 @@ class _TestZodbComponent(ERP5TypeTestCase):
   def _getComponentModuleName(self):
     pass
 
+  def _getComponentFullModuleName(self, module_name):
+    return "%s.%s" % (self._getComponentModuleName(), module_name)    
+
+  def failIfModuleImportable(self, module_name):
+    full_module_name = self._getComponentFullModuleName(module_name)
+
+    try:
+      __import__(full_module_name, fromlist=[self._getComponentModuleName()],
+                 level=0)
+    except ImportError:
+      pass
+    else:
+      raise AssertionError("Component '%s' should have been generated" % \
+                             full_module_name)
+
+  def assertModuleImportable(self, module_name):
+    full_module_name = self._getComponentFullModuleName(module_name)    
+
+    try:
+      __import__(full_module_name, fromlist=[self._getComponentModuleName()],
+                 level=0)
+    except ImportError:
+      raise AssertionError("Component '%s' should not have been generated" % \
+                             full_module_name)
+
   def testValidateInvalidate(self):
     """
     The new Component should only be in erp5.component.XXX when validated,
@@ -1253,19 +1278,16 @@ class _TestZodbComponent(ERP5TypeTestCase):
     transaction.commit()
     self.tic()
 
-    self.assertHasAttribute(self._module,
-                            'TestValidateInvalidateComponent')
+    self.assertModuleImportable('TestValidateInvalidateComponent')
     test_component.invalidate()
     transaction.commit()
     self.tic()
-    self.failIfHasAttribute(self._module,
-                            'TestValidateInvalidateComponent')
+    self.failIfModuleImportable('TestValidateInvalidateComponent')
 
     test_component.validate()
     transaction.commit()
     self.tic()
-    self.assertHasAttribute(self._module,
-                            'TestValidateInvalidateComponent')
+    self.assertModuleImportable('TestValidateInvalidateComponent')
 
   def testSourceCodeWithSyntaxError(self):
     valid_code = 'def foobar(*args, **kwargs):\n  return 42'
@@ -1281,7 +1303,7 @@ class _TestZodbComponent(ERP5TypeTestCase):
     self.assertEquals(component.getValidationState(), 'validated')
     self.assertEquals(component.getTextContent(), valid_code)
     self.assertEquals(component.getTextContent(validated_only=True), valid_code)
-    self.assertHasAttribute(self._module, 'TestComponentWithSyntaxError')
+    self.assertModuleImportable('TestComponentWithSyntaxError')
 
     invalid_code = 'def foobar(*args, **kwargs)\n  return 42'
     ComponentTool.reset = assertResetNotCalled
@@ -1297,7 +1319,7 @@ class _TestZodbComponent(ERP5TypeTestCase):
     self.assertEquals(component.getTextContent(), invalid_code)
     self.assertEquals(component.getTextContent(validated_only=True), valid_code)
     self._component_tool.reset()
-    self.assertHasAttribute(self._module, 'TestComponentWithSyntaxError')
+    self.assertModuleImportable('TestComponentWithSyntaxError')
 
     ComponentTool.reset = assertResetCalled
     try:
@@ -1314,7 +1336,7 @@ class _TestZodbComponent(ERP5TypeTestCase):
     self.assertEquals(component._getErrorMessage(), '')
     self.assertEquals(component.getTextContent(), valid_code)
     self.assertEquals(component.getTextContent(validated_only=True), valid_code)    
-    self.assertHasAttribute(self._module, 'TestComponentWithSyntaxError')
+    self.assertModuleImportable('TestComponentWithSyntaxError')
 
 from Products.ERP5Type.Core.ExtensionComponent import ExtensionComponent
 
@@ -1338,8 +1360,7 @@ class TestZodbExtensionComponent(_TestZodbComponent):
     transaction.commit()
     self.tic()
 
-    self.assertHasAttribute(self._module,
-                            'TestExternalMethodComponent')
+    self.assertModuleImportable('TestExternalMethodComponent')
 
     # Add an External Method using the Extension Component defined above and
     # check that it returns 42
@@ -1396,7 +1417,7 @@ class TestZodbDocumentComponent(_TestZodbComponent):
   def testAssignToPortalTypeClass(self):
     from Products.ERP5.Document.Person import Person as PersonDocument
 
-    self.failIfHasAttribute(self._module, 'TestPortalType')
+    self.failIfModuleImportable('TestPortalType')
 
     # Create a new Document Component inheriting from Person Document which
     # defines only one additional method (meaningful to make sure that the
@@ -1418,7 +1439,7 @@ class TestPortalType(Person):
 
     # As TestPortalType Document Component has been validated, it should now
     # be available
-    self.assertHasAttribute(self._module, 'TestPortalType')
+    self.assertModuleImportable('TestPortalType')
 
     person_type = self._portal.portal_types.Person
     person_type_class = person_type.getTypeClass()
@@ -1448,7 +1469,7 @@ class TestPortalType(Person):
       
       # The Portal Type class should not be in ghost state by now as we tried
       # to access test42() defined in TestPortalType Document Component
-      self.assertHasAttribute(self._module, 'TestPortalType')
+      self.assertModuleImportable('TestPortalType')
       self.assertTrue(self._module.TestPortalType.TestPortalType in person.__class__.mro())
       self.assertTrue(PersonDocument in person.__class__.mro())