2 files changed, 200 insertions, 14 deletions

diff --git a/src/mailman/utilities/modules.py b/src/mailman/utilities/modules.py
index 7aa2ac009..83a4384b7 100644
--- a/src/mailman/utilities/modules.py
+++ b/src/mailman/utilities/modules.py
@@ -25,6 +25,19 @@ from public import public
 
 
 @public
+def abstract_component(cls):
+    """Decorator preventing `find_components()` from instantiating the class.
+
+    Normally, `find_components()` instantiates any component class that
+    it finds matching the given interface.  Some component classes must not be
+    instantiated though, because they act as base classes.  Put this decorator
+    on the class definition to prevent instantiation.
+    """
+    cls.__abstract_component__ = True
+    return cls
+
+
+@public
 def find_name(dotted_name):
     """Import and return the named object in package space.
 
@@ -55,24 +68,36 @@ def call_name(dotted_name, *args, **kws):
     return named_callable(*args, **kws)
 
 
-@public
 def scan_module(module, interface):
-    """Return all the items in a module that conform to an interface.
+    """Return all the object in a module that conform to an interface.
+
+    Scan every item named in the module's `__all__`.  If that item conforms to
+    the given interface, *and* the item is not declared as an
+    `@abstract_component`, then instantiate the item and return the resulting
+    instance.
 
-    :param module: A module object.  The module's `__all__` will be scanned.
+    :param module: A module object.
     :type module: module
     :param interface: The interface that returned objects must conform to.
     :type interface: `Interface`
-    :return: The sequence of matching components.
-    :rtype: objects implementing `interface`
+    :return: The sequence of instantiated matching components.
+    :rtype: instantiated objects implementing `interface`
     """
     missing = object()
     for name in module.__all__:
         component = getattr(module, name, missing)
         assert component is not missing, (
             '%s has bad __all__: %s' % (module, name))   # pragma: no cover
-        if interface.implementedBy(component):
-            yield component
+        if (interface.implementedBy(component)
+                # We cannot use getattr() here because that will return True
+                # for all subclasses.  __abstract_component__ should *not* be
+                # inherited, meaning subclasses must declare themselves to be
+                # abstract if they also don't want to be instantiated.  Only
+                # by looking at the component's __dict__ can we know for sure
+                # where the marker has been placed.  The value of
+                # __abstract_component__ doesn't matter, only its presence.
+                and '__abstract_component__' not in component.__dict__):
+            yield component()
 
 
 @public
@@ -80,14 +105,15 @@ def find_components(package, interface):
     """Find components which conform to a given interface.
 
     Search all the modules in a given package, returning an iterator over all
-    objects found that conform to the given interface.
+    objects found that conform to the given interface, unless that object is
+    decorated with `@abstract_component`.
 
     :param package: The package path to search.
     :type package: string
     :param interface: The interface that returned objects must conform to.
     :type interface: `Interface`
-    :return: The sequence of matching components.
-    :rtype: objects implementing `interface`
+    :return: The sequence of instantiated matching components.
+    :rtype: instantiated objects implementing `interface`
     """
     for filename in resource_listdir(package, ''):
         basename, extension = os.path.splitext(filename)
@@ -102,6 +128,35 @@ def find_components(package, interface):
 
 
 @public
+def add_components(package, interface, mapping):
+    """Add components to a given mapping.
+
+    Similarly to `find_components()` this inspects all modules in a given
+    package looking for objects that conform to a given interface.  All such
+    found objects (unless decorated with `@abstract_component`) are added to
+    the given mapping, keyed by the object's `.name` attribute, which is
+    required.  It is a fatal error if that key already exists in the mapping.
+
+    :param package: The package path to search.
+    :type package: string
+    :param interface: The interface that returned objects must conform to.
+        Objects found must have a `.name` attribute containing a unique
+        string.
+    :type interface: `Interface`
+    :param mapping: The mapping to add the found components to.
+    :type mapping: A dict-like mapping.  This only needs to support
+        containment tests (e.g. `in` and `not in`) and `__setitem__()`.
+    :raises RuntimeError: when a duplicate key is found.
+    """
+    for component in find_components(package, interface):
+        if component.name in mapping:
+            raise RuntimeError(
+                'Duplicate key "{}" found in {}; previously {}'.format(
+                    component.name, component, mapping[component.name]))
+        mapping[component.name] = component
+
+
+@public
 def expand_path(url):
     """Expand a python: path, returning the absolute file system path."""
     # Is the context coming from a file system or Python path?
diff --git a/src/mailman/utilities/tests/test_modules.py b/src/mailman/utilities/tests/test_modules.py
index 068dee2cb..20735486c 100644
--- a/src/mailman/utilities/tests/test_modules.py
+++ b/src/mailman/utilities/tests/test_modules.py
@@ -23,7 +23,7 @@ import unittest
 
 from contextlib import ExitStack, contextmanager
 from mailman.interfaces.styles import IStyle
-from mailman.utilities.modules import find_components
+from mailman.utilities.modules import add_components, find_components
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
@@ -50,8 +50,8 @@ class TestModuleImports(unittest.TestCase):
     def test_find_modules_with_dotfiles(self):
         # Emacs creates lock files when a single file is opened by more than
         # one user. These files look like .#<filename>.py because of which
-        # find_components tries to import them but fails. All such files should
-        # be ignored by default.
+        # find_components() tries to import them but fails. All such files
+        # should be ignored by default.
         with ExitStack() as resources:
             # Creating a temporary directory and adding it to sys.path.
             temp_package = resources.enter_context(TemporaryDirectory())
@@ -62,9 +62,9 @@ class TestModuleImports(unittest.TestCase):
             module_path = os.path.join(temp_package, 'mypackage')
             os.mkdir(module_path)
             init_file = os.path.join(module_path, '__init__.py')
+            Path(init_file).touch()
             good_file = os.path.join(module_path, 'goodfile.py')
             bad_file = os.path.join(module_path, '.#badfile.py')
-            Path(init_file).touch()
             with open(good_file, 'w', encoding='utf-8') as fp:
                 print("""\
 from public import public
@@ -97,3 +97,134 @@ class BadStyle:
                      for component
                      in find_components('mypackage', IStyle)]
             self.assertEqual(names, ['good-style'])
+
+    def test_find_components_no_instantiate(self):
+        # find_components() now instantiates the class unless it's been
+        # decorated with the @abstract_component decorator.
+        with ExitStack() as resources:
+            # Creating a temporary directory and adding it to sys.path.
+            temp_package = resources.enter_context(TemporaryDirectory())
+            resources.enter_context(hack_syspath(0, temp_package))
+            resources.callback(clean_mypackage)
+            # Create a module inside the above package along with an
+            # __init__.py file so that we can import from it.
+            module_path = os.path.join(temp_package, 'mypackage')
+            os.mkdir(module_path)
+            init_file = os.path.join(module_path, '__init__.py')
+            Path(init_file).touch()
+            component_file = os.path.join(module_path, 'components.py')
+            with open(component_file, 'w', encoding='utf-8') as fp:
+                print("""\
+from mailman.interfaces.styles import IStyle
+from mailman.utilities.modules import abstract_component
+from public import public
+from zope.interface import implementer
+
+@public
+@implementer(IStyle)
+class ConcreteStyle:
+    name = 'concrete-style'
+    def apply(self):
+        pass
+
+@public
+@implementer(IStyle)
+@abstract_component
+class AbstractStyle:
+    name = 'abstract-style'
+    def apply(self):
+        pass
+""", file=fp)
+            names = [component.name
+                     for component
+                     in find_components('mypackage', IStyle)]
+            self.assertEqual(names, ['concrete-style'])
+
+    def test_add_components(self):
+        with ExitStack() as resources:
+            # Creating a temporary directory and adding it to sys.path.
+            temp_package = resources.enter_context(TemporaryDirectory())
+            resources.enter_context(hack_syspath(0, temp_package))
+            resources.callback(clean_mypackage)
+            # Create a module inside the above package along with an
+            # __init__.py file so that we can import from it.
+            module_path = os.path.join(temp_package, 'mypackage')
+            os.mkdir(module_path)
+            init_file = os.path.join(module_path, '__init__.py')
+            Path(init_file).touch()
+            component_file = os.path.join(module_path, 'components.py')
+            with open(component_file, 'w', encoding='utf-8') as fp:
+                print("""\
+from mailman.interfaces.styles import IStyle
+from mailman.utilities.modules import abstract_component
+from public import public
+from zope.interface import implementer
+
+@public
+@implementer(IStyle)
+class StyleA:
+    name = 'styleA'
+    def apply(self):
+        pass
+
+@public
+@implementer(IStyle)
+@abstract_component
+class StyleB:
+    name = 'styleB'
+    def apply(self):
+        pass
+
+@public
+@implementer(IStyle)
+class StyleC:
+    name = 'styleC'
+    def apply(self):
+        pass
+""", file=fp)
+            styles = {}
+            add_components('mypackage', IStyle, styles)
+            self.assertEqual(set(styles), {'styleA', 'styleC'})
+
+    def test_add_components_duplicates(self):
+        # A duplicate name exists, so a RuntimeError is raised.
+        with ExitStack() as resources:
+            # Creating a temporary directory and adding it to sys.path.
+            temp_package = resources.enter_context(TemporaryDirectory())
+            resources.enter_context(hack_syspath(0, temp_package))
+            resources.callback(clean_mypackage)
+            # Create a module inside the above package along with an
+            # __init__.py file so that we can import from it.
+            module_path = os.path.join(temp_package, 'mypackage')
+            os.mkdir(module_path)
+            init_file = os.path.join(module_path, '__init__.py')
+            Path(init_file).touch()
+            component_file = os.path.join(module_path, 'components.py')
+            with open(component_file, 'w', encoding='utf-8') as fp:
+                print("""\
+from mailman.interfaces.styles import IStyle
+from mailman.utilities.modules import abstract_component
+from public import public
+from zope.interface import implementer
+
+@public
+@implementer(IStyle)
+class StyleA1:
+    name = 'styleA'
+    def apply(self):
+        pass
+
+@public
+@implementer(IStyle)
+class StyleA2:
+    name = 'styleA'
+    def apply(self):
+        pass
+""", file=fp)
+            with self.assertRaisesRegex(
+                    RuntimeError,
+                    'Duplicate key "styleA" found in '
+                    '<mypackage.components.StyleA2 object at .*>; '
+                    'previously <mypackage.components.StyleA1 object at '
+                    '.*>'):
+                add_components('mypackage', IStyle, {})