Complete comments in source code of ZODB Components.

product/ERP5/Document/BusinessTemplate.py

@@ -3437,8 +3437,8 @@ class ModuleTemplateItem(BaseTemplateItem):
     # Do not remove any module for safety.
     pass
 
-# XXX-arnau: when everything has been migrated to Components, everything in
-#            this class should be moved to DocumentTemplateItem
+# XXX-arnau: when everything has been migrated to Components, this class
+# should be renamed to DocumentTemplateItem
 class FilesystemDocumentTemplateItem(BaseTemplateItem):
   local_file_reader_name = staticmethod(readLocalDocument)
   local_file_writer_name = staticmethod(writeLocalDocument)
@@ -3667,6 +3667,10 @@ class FilesystemToZodbTemplateItem(FilesystemDocumentTemplateItem,
   preinstall = _filesystemCompatibilityWrapper('preinstall', '_objects')
 
   def _importFile(self, file_name, *args, **kw):
+    """
+    Import file by calling the appropriate base class according to the file
+    name extensions
+    """
     if file_name.endswith('.xml'):
       return ObjectTemplateItem._importFile(self, file_name, *args, **kw)
     else:
@@ -3903,6 +3907,14 @@ class DocumentTemplateItem(FilesystemToZodbTemplateItem):
 
     2/ Run the migration script which will update properly the Document IDs in
        the Business Template.
+
+  Upon import or export, two files will be created:
+
+    - XML file: contains metadata
+    - Python file: contains the source code itself
+
+  This allows to keep Git history and having readable source code instead of
+  being crippled into an XML file
   """
   _tool_id = 'portal_components'
 
@@ -3918,6 +3930,12 @@ class DocumentTemplateItem(FilesystemToZodbTemplateItem):
                         class_id + ".py")
 
   def _importFile(self, file_name, file_obj):
+    """
+    Upon import, only consider XML file for ZODB Components (as the Python
+    source file will be read and set to text_content property on the new
+    object when the XML will be processed) and for backward compatibility,
+    handle non-migrated Document as well
+    """
     if file_name.endswith('.py'):
       # If portal_components/XXX.py, then ignore it as it will be handled when
       # the .xml file will be processed
@@ -3942,6 +3960,10 @@ class DocumentTemplateItem(FilesystemToZodbTemplateItem):
       LOG('Business Template', 0, 'Skipping file "%s"' % file_name)
 
   def export(self, context, bta, **kw):
+    """
+    Export a Document as two files for ZODB Components, one for metadata
+    (.xml) and the other for the Python source code (.py)
+    """
     path = self.__class__.__name__ + '/'
     for key, obj in self._objects.iteritems():
       # Back compatibility with filesystem Documents
@@ -3964,6 +3986,10 @@ class DocumentTemplateItem(FilesystemToZodbTemplateItem):
         bta.addObject(f, key, path=path)
 
   def getTemplateIdList(self):
+    """
+    Getter for Document property on the Business Template, must be overriden
+    in children classes (e.g. ExtensionDocumentTemplateItem for example)
+    """
     return self.getTemplateDocumentIdList()
 
   def build(self, context, **kw):
@@ -4025,6 +4051,11 @@ class ExtensionTemplateItem(DocumentTemplateItem):
     return self.getTemplateExtensionIdList()
 
 class TestTemplateItem(DocumentTemplateItem):
+  """
+  Live Tests are now stored in ZODB rather than on the filesystem. However,
+  some Business Templates may still have filesystem Live Tests which need to
+  be migrated to the ZODB.
+  """
   local_file_reader_name = staticmethod(readLocalTest)
   local_file_writer_name = staticmethod(writeLocalTest)
   # Test needs no import
@@ -5912,6 +5943,11 @@ Business Template is a set of definitions, such as skins, portal types and categ
                                         component_portal_type_dict,
                                         erase_existing=False,
                                         **kw):
+      """
+      Migrate the given components from filesystem to ZODB by calling the
+      appropriate importFromFilesystem according to the destination Portal
+      Type and then update the Business Template property with migrated IDs
+      """
       if not component_portal_type_dict:
         return {}
 

product/ERP5/ERP5Site.py

@@ -447,16 +447,19 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin):
                             'getVersionPriorityList')
   def getVersionPriorityList(self):
     """
-    Return the Component version priorities defined on the site
+    Return the Component version priorities defined on the site in descending
+    order. Whatever happens, a version must always be returned otherwise it
+    may render the site unusable when all Products will have been migrated
     """
-    # Whatever happens, a version must always be returned otherwise it may
-    # render the site unusable when all Products will have been migrated
     return self._version_priority_list or ('erp5 | 0.0',)
 
   security.declareProtected(Permissions.ModifyPortalContent,
                             'setVersionPriorityList' )
   def setVersionPriorityList(self, version_priority_tuple):
     """
+    Set Version Priority List and make sure that erp5 version is always
+    defined whatever the given value is
+
     XXX-arnau: must be written through an interaction workflow when ERP5Site
                will become a real ERP5 object...
     """
@@ -479,6 +482,7 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin):
     except AttributeError:
       pass
 
+    # Make sure that reset is not performed when creating a new site
     if not getattr(self, '_v_bootstrapping', False):
       self.portal_components.resetOnceAtTransactionBoundary()
 

product/ERP5Type/Core/DocumentComponent.py

@@ -35,13 +35,16 @@ import zope.interface
 from Products.ERP5Type.interfaces.component import IComponent
 
 class DocumentComponent(ComponentMixin):
-  # CMF Type Definition
+  """
+  ZODB Component for Documents in bt5 only for now (which used to be installed
+  in INSTANCE_HOME/Document) but this will also be used later on for Documents
+  in Products
+  """
   meta_type = 'ERP5 Document Component'
   portal_type = 'Document Component'
 
   zope.interface.implements(IComponent)
 
-  # Declarative security
   security = ClassSecurityInfo()
   security.declareObjectProtected(Permissions.AccessContentsInformation)
 

product/ERP5Type/Core/ExtensionComponent.py

@@ -35,13 +35,15 @@ import zope.interface
 from Products.ERP5Type.interfaces.component import IComponent
 
 class ExtensionComponent(ComponentMixin):
-  # CMF Type Definition
+  """
+  ZODB Component for Extensions previously defined in the bt5 and installed in
+  INSTANCE_HOME/Extensions
+  """
   meta_type = 'ERP5 Extension Component'
   portal_type = 'Extension Component'
 
   zope.interface.implements(IComponent)
 
-  # Declarative security
   security = ClassSecurityInfo()
   security.declareObjectProtected(Permissions.AccessContentsInformation)
 

product/ERP5Type/Core/TestComponent.py

@@ -35,13 +35,16 @@ import zope.interface
 from Products.ERP5Type.interfaces.component import IComponent
 
 class TestComponent(ComponentMixin):
-  # CMF Type Definition
+  """
+  ZODB Component for Live Tests only (previously defined in the bt5 and
+  installed in INSTANCE_HOME/tests) as other kind of Tests should be
+  deprecated at some point
+  """
   meta_type = 'ERP5 Test Component'
   portal_type = 'Test Component'
 
   zope.interface.implements(IComponent)
 
-  # Declarative security
   security = ClassSecurityInfo()
   security.declareObjectProtected(Permissions.AccessContentsInformation)
 

product/ERP5Type/Permissions.py

@@ -146,4 +146,7 @@ AddERP5Content = AddPortalContent # Since we put come CPS content in ERP5 docume
 # Source Code Management - this is the highest possible permission
 ManageExtensions = "Manage extensions"
 
+# Permission for resetting ZODB Components, this is required to allow a
+# Manager to reset Dynamic Classes but not to modify the source code of
+# Components
 ResetDynamicClasses = "Reset dynamic classes"

product/ERP5Type/Tool/ComponentTool.py

@@ -64,33 +64,45 @@ class ComponentTool(BaseTool):
   security.declareProtected(Permissions.ResetDynamicClasses, 'reset')
   def reset(self, force=False, reset_portal_type=False):
     """
-    XXX-arnau: global reset
+    Reset all ZODB Component packages. A cache cookie is used to check whether
+    the reset is necessary when force is not specified. This allows to make
+    sure that all ZEO clients get reset (checked in __of__ on ERP5Site) when
+    one given ZEO client gets reset when Component(s) are modified or
+    invalidated.
+
+    Also, as resetting ZODB Components Package usually implies to reset Portal
+    Type as Classes (because the former are used as bases), perform the reset
+    by default.
+
+    XXX-arnau: for now, this is a global reset but it might be improved in the
+    future if required...
     """
     portal = self.getPortalObject()
 
-    # XXX-arnau: copy/paste from portal_type_class, but is this really
-    # necessary as even for Portal Type classes, synchronizeDynamicModules
-    # seems to always called with force=True?
     global last_sync
     if force:
-      # hard invalidation to force sync between nodes
+      # Hard invalidation to force sync between nodes
       portal.newCacheCookie('component_packages')
       last_sync = portal.getCacheCookie('component_packages')
     else:
       cookie = portal.getCacheCookie('component_packages')
       if cookie == last_sync:
         return False
+
       last_sync = cookie
 
     LOG("ERP5Type.Tool.ComponentTool", INFO, "Resetting Components")
 
     type_tool = portal.portal_types
 
+    # One Component Package per allowed Portal Types on Component Tool
     allowed_content_type_list = type_tool.getTypeInfo(
       self.getPortalType()).getTypeAllowedContentTypeList()
 
     import erp5.component
 
+    # Make sure that it is not possible to load Components or load Portal Type
+    # class when Components are reset through aq_method_lock
     with Base.aq_method_lock:
       for content_type in allowed_content_type_list:
         package_name = content_type.split(' ')[0].lower()
@@ -112,10 +124,10 @@ class ComponentTool(BaseTool):
                             'resetOnceAtTransactionBoundary')
   def resetOnceAtTransactionBoundary(self):
     """
-    Schedule a single reset at the end of the transaction, only once.  The
-    idea behind this is that a reset is (very) costly and that we want to do
-    it as little often as possible.  Moreover, doing it twice in a transaction
-    is useless (but still twice as costly).
+    Schedule a single reset at the end of the transaction. The idea behind
+    this is that a reset is (very) costly and that we want to do it as little
+    often as possible.  Moreover, doing it twice in a transaction is useless
+    (but still twice as costly).
     """
     tv = getTransactionalVariable()
     key = 'ComponentTool.resetOnceAtTransactionBoundary'

product/ERP5Type/Tool/TypesTool.py

@@ -201,7 +201,8 @@ class TypesTool(TypeProvider):
   security.declareProtected(Permissions.AccessContentsInformation, 'getDocumentTypeList')
   def getDocumentTypeList(self):
     """
-    Return a list of Document types that can be used as Base classes
+    Return a list of Document types (including filesystem and ZODB Component
+    Documents) that can be used as Base classes
     """
     from Products.ERP5Type import document_class_registry
     document_type_set = set(document_class_registry)

product/ERP5Type/dynamic/component_package.py

@@ -86,29 +86,25 @@ class ComponentDynamicPackage(ModuleType):
     """
     Create the component registry, this is very similar to
     Products.ERP5Type.document_class_registry and avoids checking whether a
-    Component exists at each call at the expense to increase startup
-    time. Moreover, it allows to handle reference easily.
-
-    XXX-arnau: handle different versions of a Component, perhaps something
-    like erp5.component.extension.VERSION.REFERENCE perhaps but there should
-    be a a way to specify priorities such as portal_skins maybe?
+    Component exists at each call at the expense of being slower when being
+    re-generated after a reset. Moreover, it allows to handle reference
+    easily.
     """
     if not self.__registry_dict:
       portal = getSite()
 
       try:
         component_tool = portal.portal_components
-      # XXX-arnau: When installing ERP5 site, erp5_core_components has not
-      # been installed yet, thus this will obviously failed...
+      # When installing ERP5 site, erp5_core_components has not been installed
+      # yet, thus this will obviously failed...
       except AttributeError:
         return {}
 
       version_priority_set = set(portal.getVersionPriorityNameList())
 
       # objectValues should not be used for a large number of objects, but
-      # this is only done at startup or upon reset, moreover using the Catalog
-      # is too risky as it lags behind and depends upon objects being
-      # reindexed
+      # this is only done upon reset, moreover using the Catalog is too risky
+      # as it lags behind and depends upon objects being reindexed
       with Base.aq_method_lock:
         for component in component_tool.objectValues(portal_type=self._portal_type):
           # Only consider modified or validated states as state transition will
@@ -138,6 +134,16 @@ class ComponentDynamicPackage(ModuleType):
                    module.__file__[1:-1]).getTextContent(validated_only=True)
 
   def find_module(self, fullname, path=None):
+    """
+    PEP-302 Finder which determines which packages and modules will be handled
+    by this class. It must be done carefully to avoid handling packages and
+    modules the Loader (load_module()) will not be handled later as the latter
+    would raise ImportError...
+
+    As per PEP-302, returns None if this Finder cannot handle the given name,
+    perhaps because the Finder of another Component Package could do it or
+    because this is a filesystem module...
+    """
     # 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
@@ -162,7 +168,7 @@ class ComponentDynamicPackage(ModuleType):
       except KeyError:
         return None
 
-    # Skip components not available, otherwise Products for example could be
+    # Skip unavailable components, otherwise Products for example could be
     # wrongly considered as importable and thus the actual filesystem class
     # ignored
     elif (name not in self._registry_dict and
@@ -172,6 +178,13 @@ class ComponentDynamicPackage(ModuleType):
     return self
 
   def _getVersionPackage(self, version):
+    """
+    Get the version package (NAMESPACE.VERSION_version) for the given version
+    and create it if it does not already exist
+    """
+    # Version are appended with '_version' to distinguish them from top-level
+    # Component modules (Component checkConsistency() forbids Component name
+    # ending with _version)
     version += '_version'
     version_package = getattr(self, version, None)
     if version_package is None:
@@ -185,12 +198,24 @@ class ComponentDynamicPackage(ModuleType):
 
   def __load_module(self, fullname):
     """
-    Load a module with given fullname (see PEP 302) if it's not
-    already in sys.modules. It is assumed that imports are filtered
-    properly in find_module().
+    Load a module with given fullname (see PEP 302) if it's not already in
+    sys.modules. It is assumed that imports are filtered properly in
+    find_module().
+
+    Also, when the top-level Component module is requested
+    (erp5.component.XXX.COMPONENT_NAME), the Component with the highest
+    version priority will be loaded into the Version package
+    (erp5.component.XXX.VERSION_version.COMPONENT_NAME. Therefore, the
+    top-level Component module will just be an alias of the versioned one.
+
+    As per PEP-302, raise an ImportError if the Loader could not load the
+    module for any reason...
     """
     site = getSite()
     name = fullname[len(self._namespace_prefix):]
+
+    # if only Version package (erp5.component.XXX.VERSION_version) is
+    # requested to be loaded, then create it if necessary
     if name.endswith('_version'):
       version = name[:-self.__version_suffix_len]
       return (version in site.getVersionPriorityNameList() and
@@ -198,6 +223,8 @@ class ComponentDynamicPackage(ModuleType):
 
     module_fullname_alias = None
     version_package_name = name[:-self.__version_suffix_len]
+
+    # If a specific version of the Component has been requested
     if '.' in name:
       try:
         version, name = name.split('.')
@@ -212,6 +239,7 @@ class ComponentDynamicPackage(ModuleType):
         raise ImportError("%s: version %s of Component %s could not be found" % \
                             (fullname, version, name))
 
+    # Otherwise, find the Component with the highest version priority
     else:
       try:
         component_version_dict = self._registry_dict[name]
@@ -219,6 +247,7 @@ class ComponentDynamicPackage(ModuleType):
         raise ImportError("%s: Component %s could not be found" % (fullname,
                                                                    name))
 
+      # Version priority name list is ordered in descending order
       for version in site.getVersionPriorityNameList():
         component = component_version_dict.get(version)
         if component is not None:
@@ -227,6 +256,8 @@ class ComponentDynamicPackage(ModuleType):
         raise ImportError("%s: no version of Component %s in Site priority" % \
                             (fullname, name))
 
+      # Check whether this module has already been loaded before for a
+      # specific version, if so, just add it to the upper level
       try:
         module = getattr(getattr(self, version + '_version'), name)
       except AttributeError:
@@ -264,6 +295,8 @@ class ComponentDynamicPackage(ModuleType):
     module.__loader__ = self
     module.__name__ = module_fullname
 
+    # Add the newly created module to the Version package and add it as an
+    # alias to the top-level package as well
     setattr(self._getVersionPackage(version), name, module)
     if module_fullname_alias:
       setattr(self, name, module)
@@ -287,7 +320,7 @@ class ComponentDynamicPackage(ModuleType):
     if sub_package:
       package = sub_package
     else:
-      # Clear the Component registry
+      # Clear the Component registry only once
       self.__registry_dict.clear()
       package = self
 
@@ -306,7 +339,8 @@ class ComponentDynamicPackage(ModuleType):
       # the meantime
       del sys.modules[module_name]
 
-      # Delete linecache data
+      # Delete linecache data to get updated source code (__file__ attribute
+      # (<ComponentID>) is used as linecache key)
       import linecache
       try:
         del linecache.cache[getattr(package, name).__file__]
@@ -314,5 +348,4 @@ class ComponentDynamicPackage(ModuleType):
       except (AttributeError, KeyError):
         pass
 
-      # And finally remove the module
       delattr(package, name)

product/ERP5Type/dynamic/dynamic_module.py

@@ -86,9 +86,14 @@ def initializeDynamicModules():
     erp5.accessor_holder.portal_type
       holds accessors holders of Portal Types
     erp5.component:
-      holds component modules
+      holds ZODB Component packages
+    erp5.component.document:
+      holds Document modules previously found in bt5 in $INSTANCE_HOME/Document
     erp5.component.extension:
-      holds extension classes previously found in bt5 in instancehome/Extensions
+      holds Extension modules previously found in bt5 in
+      $INSTANCE_HOME/Extensions
+    erp5.component.test:
+      holds Live Test modules previously found in bt5 in $INSTANCE_HOME/test
   """
   erp5 = ModuleType("erp5")
   sys.modules["erp5"] = erp5
@@ -121,7 +126,7 @@ def initializeDynamicModules():
   erp5.temp_portal_type = registerDynamicModule('erp5.temp_portal_type',
                                                 loadTempPortalTypeClass)
 
-  # Components
+  # ZODB Components
   erp5.component = ModuleType("erp5.component")
   sys.modules["erp5.component"] = erp5.component
 

product/ERP5Type/mixin/component.py

@@ -116,6 +116,22 @@ class RecordablePropertyMetaClass(ExtensionClass):
 
 class ComponentMixin(PropertyRecordableMixin, Base):
   """
+  Mixin used for all ZODB Components. Most of the code is generic, thus actual
+  ZODB Components should have almost nothing to defined...
+
+  From a security point of view, only Developer Role defined on Component Tool
+  can manage Components (as exec is used and anything potentially damaging
+  could be done on the filesystem), while only Manager or Developer Roles can
+  reset Component Packages (see ERP5Type.Permissions). All the permissions are
+  defined on Component Tool itself and newly created Components just inherits
+  permissions defined on the former.
+
+  The Developer Role is not a typical Role as only users defined in Zope
+  configuration can be added to this Role (which is displayed in the list of
+  available Roles in ZMI). This is achieved by two monkey patches
+  (ERP5Type.patches.{User,PropertiedUser}) and modifications in
+  ERP5Security.ERP5UserFactory.
+
   XXX-arnau: add tests to ERP5 itself to make sure all securities are defined
   properly everywhere (see naming convention test)
   """
@@ -157,7 +173,15 @@ class ComponentMixin(PropertyRecordableMixin, Base):
   security.declareProtected(Permissions.ModifyPortalContent, 'checkConsistency')
   def checkConsistency(self, *args, **kw):
     """
-    XXX-arnau: should probably be in a separate Constraint class?
+    Check the consistency of the Component upon validate or when being
+    modified after being validated.
+
+    Some keywords are forbidden for reference and version. As Version package
+    always ends with '_version', reference is checked more carefully to avoid
+    clashing with existing method names (such as the ones required for PEP
+    302).
+
+    XXX-arnau: separate Constraint class?
     """
     error_list = super(ComponentMixin, self).checkConsistency(*args, **kw)
     object_relative_url = self.getRelativeUrl()
@@ -201,6 +225,7 @@ class ComponentMixin(PropertyRecordableMixin, Base):
     else:
       message = None
       try:
+        # Check for any error in the source code by trying to load it
         self.load({}, text_content=text_content)
       except SyntaxError, e:
         mapping = dict(error_message=str(e),
@@ -226,10 +251,11 @@ class ComponentMixin(PropertyRecordableMixin, Base):
                             'checkConsistencyAndValidate')
   def checkConsistencyAndValidate(self):
     """
-    When a Component is in validated or modified validation state and
-    it is modified, modified state is set then this checks whether the
-    Component can be validated again if checkConsistency returns no
-    error
+    When a Component is in validated or modified validation state and it is
+    modified, modified state is set then this checks whether the Component can
+    be validated again if checkConsistency returns no error. Otherwise, it
+    stays in modified state and previously validated values are used for
+    reference, version and text_content
     """
     error_list = self.checkConsistency()
     if error_list:
@@ -249,7 +275,7 @@ class ComponentMixin(PropertyRecordableMixin, Base):
   def hasErrorMessageList(self):
     """
     Check whether there are error messages, useful to display errors in the UI
-    without calling getErrorMessageList() which translates error messages
+    without calling getErrorMessageList() as it translates error messages
     """
     workflow = self.workflow_history['component_validation_workflow'][-1]
     return bool(workflow['error_message'])
@@ -274,10 +300,8 @@ class ComponentMixin(PropertyRecordableMixin, Base):
     __dict__ is given rather than creating an empty dict and returning it.
 
     Initially, namespace_dict default parameter value was an empty dict to
-    allow checking the source code before validate, but this introduces a bug
-    when namespace_dict was not given because the first call would exec
-    directly into function namespace_dict default parameter, thus the second
-    call would have namespace_dict default value to the previous call.
+    allow checking the source code before validate, but this is completely
+    wrong as the object reference is kept accross each call
     """
     if text_content is None:
       text_content = self.getTextContent(validated_only=validated_only)
@@ -312,7 +336,7 @@ class ComponentMixin(PropertyRecordableMixin, Base):
     Get source for FTP/Webdav. The default implementation of GET for Webdav,
     available in webdav.Resource, calls manage_FTPget
 
-    XXX-arnau: encoding?
+    XXX-arnau: encoding issue?
     """
     return self.getTextContent()
 
@@ -322,8 +346,9 @@ class ComponentMixin(PropertyRecordableMixin, Base):
   def importFromFilesystem(cls, context, reference, version,
                            erase_existing=False):
     """
-    Import a Component from the filesystem into ZODB after checking that the
-    source code is valid
+    Import a Component from the filesystem into ZODB and validate it so it can
+    be loaded straightaway provided validate() does not raise any error of
+    course
     """
     object_id = '%s.%s.%s' % (cls._getDynamicModuleNamespace(), version,
                               reference)
@@ -358,7 +383,7 @@ class ComponentMixin(PropertyRecordableMixin, Base):
     # straightaway as there should be no error
     new_component.validate()
 
-    # XXX-arnau: is it really safe?
+    # Remove now useless Component on filesystem
     os.remove(path)
 
     return new_component

product/ERP5Type/patches/ExternalMethod.py

+##############################################################################
+#
+# Copyright (c) 2002 Zope Foundation and Contributors.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE
+#
+##############################################################################
+
 from Products.ExternalMethod.ExternalMethod import ExternalMethod
 
 from App.Extensions import FuncCode, getObject
 def getFunction(self, reload=False, f=None):
+  """
+  Patch to get ZODB Component Extension function if available, otherwise
+  fallback on filesystem Extension
+  """
   if f is None:
     import erp5.component.extension
     try:
@@ -21,6 +38,11 @@ ExternalMethod.getFunction = getFunction
 
 ExternalMethod__call__ = ExternalMethod.__call__
 def __call__(self, *args, **kw):
+  """
+  Patch to call ZODB Component Extension, by trying first to import ZODB
+  Component Extension if available, otherwise fallback on filesystem
+  Extension
+  """
   try:
     f = getattr(__import__('erp5.component.extension.' + self._module,
                            fromlist=['erp5.component.extension'],

product/ERP5Type/patches/User.py

@@ -94,7 +94,7 @@ def getRolesInContext(self, object):
       continue
     break
 
-  # Patched: Developer role should not never be available as local role
+  # Patched: Developer role should never be available as local role
   local.pop('Developer', None)
   roles=list(roles) + local.keys()
   return roles

product/ERP5Type/tests/testDynamicClassGeneration.py

@@ -1209,6 +1209,15 @@ ComponentTool._original_reset = ComponentTool.reset
 ComponentTool._reset_performed = False
 
 def assertResetNotCalled(*args, **kwargs):
+  """
+  Assert that reset has *not* been called, ignoring reset() which has not
+  actually been done because force has not been specified (for example when
+  reset is being called from __of__). When a Component is validated or
+  modified after being validated, reset will always be forced anyway...
+
+  This function is supposed to replace ComponentTool.reset() which is restored
+  afterwards
+  """
   reset_performed = ComponentTool._original_reset(*args, **kwargs)
   if reset_performed:
     raise AssertionError("reset should not have been performed")
@@ -1216,6 +1225,15 @@ def assertResetNotCalled(*args, **kwargs):
   return reset_performed
 
 def assertResetCalled(*args, **kwargs):
+  """
+  Assert that reset has been called, ignoring reset() which has not actually
+  been done because force has not been specified (for example when reset is
+  being called from __of__). When a Component is validated or modified after
+  being validated, reset will always be forced anyway...
+
+  This function is supposed to replace ComponentTool.reset() which is restored
+  afterwards
+  """
   reset_performed = ComponentTool._original_reset(*args, **kwargs)
   if reset_performed:
     ComponentTool._reset_performed = True
@@ -1229,6 +1247,10 @@ from Products.ERP5Type.tests.SecurityTestCase import SecurityTestCase
 from App.config import getConfiguration
 
 class _TestZodbComponent(SecurityTestCase):
+  """
+  Abstract class which defined convenient methods used by any Component Test
+  and tests ran for all Component Test classes
+  """
   __metaclass__ = abc.ABCMeta
 
   def getBusinessTemplateList(self):
@@ -1236,6 +1258,10 @@ class _TestZodbComponent(SecurityTestCase):
             'erp5_core_component')
 
   def login(self, user_name='ERP5TypeTestCase', quiet=0):
+    """
+    Make sure that the test user has Developer Role, otherwise the user cannot
+    do anything on Components...
+    """
     product_config = getattr(getConfiguration(), 'product_config', None)
     if product_config is None:
       class DummyDeveloperConfig(object):
@@ -1258,16 +1284,26 @@ class _TestZodbComponent(SecurityTestCase):
 
   @abc.abstractmethod
   def _newComponent(self, reference, text_content, version='erp5'):
+    """
+    Abstract method to create a new Component
+    """
     pass
 
   @abc.abstractmethod
   def _getComponentModuleName(self):
+    """
+    Abstract method defining ZODB Component top-level package name
+    """
     pass
 
   def _getComponentFullModuleName(self, module_name):
     return self._getComponentModuleName() + '.' + module_name
 
   def failIfModuleImportable(self, module_name):
+    """
+    Check that the given module name is *not* importable (ZODB Components
+    relies solely on import hooks)
+    """
     full_module_name = self._getComponentFullModuleName(module_name)
 
     try:
@@ -1280,6 +1316,10 @@ class _TestZodbComponent(SecurityTestCase):
                   full_module_name)
 
   def assertModuleImportable(self, module_name):
+    """
+    Check that the given module name is importable (ZODB Components relies
+    solely on import hooks)
+    """
     full_module_name = self._getComponentFullModuleName(module_name)
 
     try:
@@ -1292,7 +1332,7 @@ class _TestZodbComponent(SecurityTestCase):
   def testValidateInvalidate(self):
     """
     The new Component should only be in erp5.component.XXX when validated,
-    otherwise an AttributeError should be raised
+    otherwise it should not be importable at all
     """
     test_component = self._newComponent(
       'TestValidateInvalidateComponent',
@@ -1316,7 +1356,11 @@ class _TestZodbComponent(SecurityTestCase):
   def testReferenceWithReservedKeywords(self):
     """
     Check whether checkConsistency has been properly implemented for checking
-    Component Reference field, e.g. no reserved keywords can be used
+    Component Reference, e.g. no reserved keywords can be used.
+
+    Also, check resets which should be performed when the Component is
+    validated but not when an error was encountered (implemented in
+    dynamic_class_generation_interaction_workflow)
     """
     valid_reference = 'TestReferenceWithReservedKeywords'
     ComponentTool.reset = assertResetCalled
@@ -1339,14 +1383,21 @@ class _TestZodbComponent(SecurityTestCase):
     self.assertEquals(component.getReference(validated_only=True), valid_reference)
     self.assertModuleImportable(valid_reference)
 
+    # Check that checkConsistency returns the proper error message for the
+    # following reserved keywords
     invalid_reference_dict = {
       None: ComponentMixin._message_reference_not_set,
+      # '_version' could clash with Version package name
       'ReferenceReservedKeywords_version': ComponentMixin._message_invalid_reference,
+      # Besides of clashing with protected attributes/methods, it does not
+      # make sense to have reference starting with '_'
       '_ReferenceReservedKeywords': ComponentMixin._message_invalid_reference,
+      # PEP-302 required functions defined on top-level Component Package
       'find_module': ComponentMixin._message_invalid_reference,
       'load_module': ComponentMixin._message_invalid_reference}
 
     for invalid_reference, error_message in invalid_reference_dict.iteritems():
+      # Reset should not be performed
       ComponentTool.reset = assertResetNotCalled
       try:
         component.setReference(invalid_reference)
@@ -1355,6 +1406,7 @@ class _TestZodbComponent(SecurityTestCase):
       finally:
         ComponentTool.reset = ComponentTool._original_reset
 
+      # Should be in modified state as an error has been encountered
       self.assertEquals(component.getValidationState(), 'modified')
       error_list = component.getErrorMessageList()
       self.assertNotEquals(error_list, [])
@@ -1365,6 +1417,8 @@ class _TestZodbComponent(SecurityTestCase):
       self._component_tool.reset(force=True, reset_portal_type=True)
       self.assertModuleImportable(valid_reference)
 
+    # Set a valid reference and check that the Component is in validated state
+    # and no error was raised
     ComponentTool.reset = assertResetCalled
     try:
       component.setReference(valid_reference)
@@ -1385,7 +1439,11 @@ class _TestZodbComponent(SecurityTestCase):
   def testVersionWithReservedKeywords(self):
     """
     Check whether checkConsistency has been properly implemented for checking
-    Component version field, e.g. no reserved keywords can be used
+    Component version field, e.g. no reserved keywords can be used.
+    
+    Also, check resets which should be performed when the Component is
+    validated but not when an error was encountered (implemented in
+    dynamic_class_generation_interaction_workflow)
     """
     reference = 'TestVersionWithReservedKeywords'
     valid_version = 'erp5'
@@ -1410,11 +1468,16 @@ class _TestZodbComponent(SecurityTestCase):
     self.assertEquals(component.getVersion(validated_only=True), valid_version)
     self.assertModuleImportable(reference)
 
+    # Check that checkConsistency returns the proper error message for the
+    # following reserved keywords
     invalid_version_dict = {
       '': ComponentMixin._message_version_not_set,
+      # Besides of clashing with protected attributes/methods, it does not
+      # make sense to have reference starting with '_'
       '_TestVersionWithReservedKeywords': ComponentMixin._message_invalid_version}
 
     for invalid_version, error_message in invalid_version_dict.iteritems():
+      # Reset should not be performed
       ComponentTool.reset = assertResetNotCalled
       try:
         component.setVersion(invalid_version)
@@ -1423,6 +1486,7 @@ class _TestZodbComponent(SecurityTestCase):
       finally:
         ComponentTool.reset = ComponentTool._original_reset
 
+      # Should be in modified state as an error has been encountered
       self.assertEquals(component.getValidationState(), 'modified')
       error_list = component.getErrorMessageList()
       self.assertNotEquals(error_list, [])
@@ -1433,6 +1497,8 @@ class _TestZodbComponent(SecurityTestCase):
       self._component_tool.reset(force=True, reset_portal_type=True)
       self.assertModuleImportable(reference)
 
+    # Set a valid version and check that the Component is in validated state
+    # and no error was raised
     ComponentTool.reset = assertResetCalled
     try:
       component.setVersion(valid_version)
@@ -1453,7 +1519,11 @@ class _TestZodbComponent(SecurityTestCase):
   def testInvalidSourceCode(self):
     """
     Check whether checkConsistency has been properly implemented for checking
-    Component source code field
+    Component source code field.
+    
+    Also, check resets which should be performed when the Component is
+    validated but not when an error was encountered (implemented in
+    dynamic_class_generation_interaction_workflow)
     """
     valid_code = 'def foobar(*args, **kwargs):\n  return 42'
     ComponentTool.reset = assertResetCalled
@@ -1474,14 +1544,17 @@ class _TestZodbComponent(SecurityTestCase):
     self.assertEquals(component.getTextContent(validated_only=True), valid_code)
     self.assertModuleImportable('TestComponentWithSyntaxError')
 
-    # Make sure that foobar NameError is at the end to make sure that after
-    # defining foobar function, the symbol is not available anymore
+    # Check that checkConsistency returns the proper error message for the
+    # following Python errors
     invalid_code_dict = (
       (None, ComponentMixin._message_text_content_not_set),
       ('def foobar(*args, **kwargs)\n  return 42', 'Syntax error in source code:'),
+      # Make sure that foobar NameError is at the end to make sure that after
+      # defining foobar function, it is not available at all
       ('foobar', 'Source code:'))
 
     for invalid_code, error_message in invalid_code_dict:
+      # Reset should not be performed
       ComponentTool.reset = assertResetNotCalled
       try:
         component.setTextContent(invalid_code)
@@ -1490,6 +1563,7 @@ class _TestZodbComponent(SecurityTestCase):
       finally:
         ComponentTool.reset = ComponentTool._original_reset
 
+      # Should be in modified state as an error has been encountered
       self.assertEquals(component.getValidationState(), 'modified')
       error_list = component.getErrorMessageList()
       self.assertNotEqual(error_list, [])
@@ -1500,6 +1574,8 @@ class _TestZodbComponent(SecurityTestCase):
       self._component_tool.reset(force=True, reset_portal_type=True)
       self.assertModuleImportable('TestComponentWithSyntaxError')
 
+    # Set a valid source code and check that the Component is in validated
+    # state and no error was raised
     ComponentTool.reset = assertResetCalled
     try:
       component.setTextContent(valid_code)
@@ -1518,6 +1594,11 @@ class _TestZodbComponent(SecurityTestCase):
     self.assertModuleImportable('TestComponentWithSyntaxError')
 
   def testImportVersionedComponentOnly(self):
+    """
+    Most of the time, erp5.component.XXX.COMPONENT_NAME is imported but
+    sometimes it may be useful to import a specific version of a Component,
+    available as erp5.component.XXX.VERSION_version.COMPONENT_NAME.
+    """
     component = self._newComponent(
       'TestImportedVersionedComponentOnly',
       """def foo(*args, **kwargs):
@@ -1530,6 +1611,8 @@ class _TestZodbComponent(SecurityTestCase):
 
     top_module_name = self._getComponentModuleName()
 
+    # Create a new Component which uses a specific version of the previously
+    # created Component
     component_import = self._newComponent(
       'TestImportVersionedComponentOnly',
       """from %s.erp5_version.TestImportedVersionedComponentOnly import foo
@@ -1548,6 +1631,7 @@ def bar(*args, **kwargs):
     top_module = __import__(top_module_name, level=0,
                             fromlist=[top_module_name])
 
+    # Function defined in versioned Component must be available and callable
     self.assertHasAttribute(
       top_module.erp5_version.TestImportedVersionedComponentOnly, 'foo')
 
@@ -1555,9 +1639,16 @@ def bar(*args, **kwargs):
       top_module.erp5_version.TestImportedVersionedComponentOnly.foo(),
       'TestImportedVersionedComponentOnly')
 
+    # The alias module on the top-level package must not have been created as
+    # only the versioned Component has been used
     self.failIfHasAttribute(top_module, 'TestImportedVersionedComponentOnly')
 
   def testVersionPriority(self):
+    """
+    Check whether Version priorities properly works by adding and removing
+    version priorities on ERP5Site and checking whether the proper Component
+    is loaded
+    """
     component_erp5_version = self._newComponent(
       'TestVersionPriority',
       """def function_foo(*args, **kwargs):
@@ -1581,6 +1672,8 @@ def bar(*args, **kwargs):
 
     self.assertModuleImportable('TestVersionPriority')
     self.assertModuleImportable('erp5_version.TestVersionPriority')
+    # Component for 'foo_version' must not be importable as 'foo' has not been
+    # added to ERP5Site version priorities
     self.failIfModuleImportable('foo_version.TestVersionPriority')
 
     top_module_name = self._getComponentModuleName()
@@ -1596,6 +1689,8 @@ def bar(*args, **kwargs):
     ComponentTool.reset = assertResetCalled
     priority_tuple = site.getVersionPriorityList()
     try:
+      # Add 'foo' version with a higher priority as 'erp5' version and check
+      # whether 'foo' version of the Component is used and not erp5 version
       site.setVersionPriorityList(('foo | 99.0',) + priority_tuple)
       transaction.commit()
       self.tic()
@@ -1618,6 +1713,8 @@ def bar(*args, **kwargs):
 
   def testDeveloperRoleSecurity(self):
     """
+    Only Developer Role must be able to manage Components
+
     XXX-arnau: test with different users and workflows
     """
     component = self._newComponent('TestDeveloperRoleSecurity',
@@ -1662,6 +1759,10 @@ def bar(*args, **kwargs):
 from Products.ERP5Type.Core.ExtensionComponent import ExtensionComponent
 
 class TestZodbExtensionComponent(_TestZodbComponent):
+  """
+  Tests specific to ZODB Extension Component (previously defined in bt5 and
+  installed on the filesystem in $INSTANCE_HOME/Extensions)
+  """
   def _newComponent(self, reference, text_content, version='erp5'):
     return self._component_tool.newContent(
       id='%s.%s.%s' % (self._getComponentModuleName(),
@@ -1676,6 +1777,10 @@ class TestZodbExtensionComponent(_TestZodbComponent):
     return ExtensionComponent._getDynamicModuleNamespace()
 
   def testExternalMethod(self):
+    """
+    Check that ExternalMethod monkey-patch to use ZODB Components works well
+    by creating a new External Method and then a Python Script to call it
+    """
     test_component = self._newComponent(
       'TestExternalMethodComponent',
       'def foobar(*args, **kwargs):\n  return 42')
@@ -1711,12 +1816,13 @@ class TestZodbExtensionComponent(_TestZodbComponent):
 
     self.assertEqual(self.getPortal().TestPythonScript(), 42)
 
-    # Invalidate the Extension Component
+    # Invalidate the Extension Component and check that it's not callable
+    # anymore
     test_component.invalidate()
     transaction.commit()
     self.tic()
 
-    # XXX-arnau: perhaps the error message should be more meaningful
+    # XXX-arnau: perhaps the error message should be more meaningful?
     try:
       external_method()
     except RuntimeError, e:
@@ -1728,6 +1834,11 @@ class TestZodbExtensionComponent(_TestZodbComponent):
 from Products.ERP5Type.Core.DocumentComponent import DocumentComponent
 
 class TestZodbDocumentComponent(_TestZodbComponent):
+  """
+  Tests specific to ZODB Document Component. This is only for Document
+  previously defined in bt5 and installed on the filesystem in
+  $INSTANCE_HOME/Document. Later on, Product Documents will also be migrated
+  """
   def _newComponent(self, reference, text_content, version='erp5'):
     return self._component_tool.newContent(
       id='%s.%s.%s' % (self._getComponentModuleName(),
@@ -1741,6 +1852,13 @@ class TestZodbDocumentComponent(_TestZodbComponent):
     return DocumentComponent._getDynamicModuleNamespace()
 
   def testAssignToPortalTypeClass(self):
+    """
+    Create a new Document Component inheriting from Person Document and try to
+    assign it to Person Portal Type, then create a new Person and check
+    whether it has been successfully added to its Portal Type class bases and
+    that the newly-defined function on ZODB Component can be called as well as
+    methods from Person Document
+    """
     from Products.ERP5.Document.Person import Person as PersonDocument
 
     self.failIfModuleImportable('TestPortalType')
@@ -1777,14 +1895,14 @@ class TestPortalType(Person):
     self.assertTrue(PersonDocument in person.__class__.mro())
 
     # There is no reason that TestPortalType Document Component has been
-    # assigned to a Person, otherwise there is something really bad going on
+    # assigned to a Person
     self.failIfHasAttribute(person, 'test42')
     self.assertFalse(self._module.TestPortalType in person.__class__.mro())
 
     # Reset Portal Type classes to ghost to make sure that everything is reset
     self._component_tool.reset(force=True, reset_portal_type=True)
 
-    # TestPortalType must be in available type class list
+    # TestPortalType must be available in type class list
     self.assertTrue('TestPortalType' in person_type.getDocumentTypeList())
     try:
       person_type.setTypeClass('TestPortalType')
@@ -1804,6 +1922,10 @@ class TestPortalType(Person):
       transaction.commit()
 
   def testDocumentWithImport(self):
+    """
+    Create two new Components and check whether one can import the other one
+    after the latter has been validated
+    """
     self.failIfModuleImportable('TestDocumentWithImport')
     self.failIfModuleImportable('TestDocumentImported')
 
@@ -1851,6 +1973,10 @@ class TestDocumentWithImport(TestDocumentImported):
 from Products.ERP5Type.Core.TestComponent import TestComponent
 
 class TestZodbTestComponent(_TestZodbComponent):
+  """
+  Tests specific to ZODB Test Component (known as Live Tests, and previously
+  defined in bt5 and installed in $INSTANCE_HOME/test)
+  """
   def _newComponent(self, reference, text_content, version='erp5'):
     return self._component_tool.newContent(
       id='%s.%s.%s' % (self._getComponentModuleName(),
@@ -1864,6 +1990,11 @@ class TestZodbTestComponent(_TestZodbComponent):
     return TestComponent._getDynamicModuleNamespace()
 
   def testRunLiveTest(self):
+    """
+    Create a new ZODB Test Component and try to run it as a live tests and
+    check the expected output
+    """
+    # First try with a test which run successfully
     source_code = '''
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
 
@@ -1914,6 +2045,7 @@ class Test(ERP5TypeTestCase):
                         "Expected 'Ran 1 test.*OK' in '%s'" % output)
 
 
+    # Secondly, add a test which will always fail
     source_code += '''
   def test_02_sampleTestWithFailure(self):
     self.assertEqual(0, 1)