From 38a86adcdb78c1944c26a5ab8deddff619b33bcf Mon Sep 17 00:00:00 2001
From: J08nY
Date: Wed, 31 May 2017 02:09:09 +0200
Subject: Add pluggable components.

 - Adds the notion of a 'plugin'.
   - A plugin has a package path and a flag specifying whether
     it's enabled or not.

 - Adds a find_pluggable_components function similar to the
   find_components one. This one dynamically searches not only
   the mailman package but all of plugins.
   - e.g. find_pluggable_components('rules', IRule) finds
     all IRule components in mailman.rules but also in
     example_plugin.rules for plugin names example_plugin.

 - Uses the find_pluggable_components function in place of
   find_components when searching for Rules, Handlers, Chains,
   EmailCommands, and Styles.
---
 src/mailman/bin/mailman.py | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

(limited to 'src/mailman/bin/mailman.py')

diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py
index e012237f3..ece223180 100644
--- a/src/mailman/bin/mailman.py
+++ b/src/mailman/bin/mailman.py
@@ -16,7 +16,6 @@
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
 """The 'mailman' command dispatcher."""
-
 import click
 
 from contextlib import ExitStack
@@ -25,7 +24,7 @@ from mailman.core.i18n import _
 from mailman.core.initialize import initialize
 from mailman.database.transaction import transaction
 from mailman.interfaces.command import ICLISubCommand
-from mailman.utilities.modules import add_components
+from mailman.utilities.plugins import add_pluggable_components
 from mailman.version import MAILMAN_VERSION_FULL
 from public import public
 
@@ -35,17 +34,22 @@ class Subcommands(click.MultiCommand):
     def __init__(self, *args, **kws):
         super().__init__(*args, **kws)
         self._commands = {}
-        # Look at all modules in the mailman.bin package and if they are
-        # prepared to add a subcommand, let them do so.  I'm still undecided as
-        # to whether this should be pluggable or not.  If so, then we'll
-        # probably have to partially parse the arguments now, then initialize
-        # the system, then find the plugins.  Punt on this for now.
-        add_components('mailman.commands', ICLISubCommand, self._commands)
+        self._loaded = False
+
+    def _load(self):
+        # Load commands lazily as pluggable commands need a parsed config to
+        # find plugins.
+        if not self._loaded:
+            add_pluggable_components('commands', ICLISubCommand,
+                                     self._commands)
+            self._loaded = True
 
     def list_commands(self, ctx):
+        self._load()
         return sorted(self._commands)               # pragma: nocover
 
     def get_command(self, ctx, name):
+        self._load()
         try:
             return self._commands[name].command
         except KeyError as error:
@@ -83,10 +87,12 @@ class Subcommands(click.MultiCommand):
                 formatter.write_dl(opts)
 
 
-@click.group(
-    cls=Subcommands,
-    context_settings=dict(help_option_names=['-h', '--help']))
-@click.pass_context
+def initialize_config(ctx, param, value):
+    if ctx.resilient_parsing:
+        return
+    initialize(value)
+
+
 @click.option(
     '-C', '--config', 'config_file',
     envvar='MAILMAN_CONFIG_FILE',
@@ -94,7 +100,12 @@ class Subcommands(click.MultiCommand):
     help=_("""\
     Configuration file to use.  If not given, the environment variable
     MAILMAN_CONFIG_FILE is consulted and used if set.  If neither are given, a
-    default configuration file is loaded."""))
+    default configuration file is loaded."""),
+    is_eager=True, callback=initialize_config)
+@click.group(
+    cls=Subcommands,
+    context_settings=dict(help_option_names=['-h', '--help']))
+@click.pass_context
 @click.version_option(MAILMAN_VERSION_FULL, message='%(version)s')
 @public
 def main(ctx, config_file):
@@ -104,6 +115,4 @@ def main(ctx, config_file):
     Copyright 1998-2017 by the Free Software Foundation, Inc.
     http://www.list.org
     """
-    # Initialize the system.  Honor the -C flag if given.
-    initialize(config_file)
     # click handles dispatching to the subcommand via the Subcommands class.
-- 
cgit v1.2.3-70-g09d2


From 8addebbf9802e911c06f6a27b7ffff1e0f1d2e57 Mon Sep 17 00:00:00 2001
From: J08nY
Date: Tue, 25 Jul 2017 21:44:54 +0200
Subject: Fix coverage, deprecate, but run non-plugin (post|pre)_hooks.

---
 src/mailman/app/docs/plugins.rst          | 50 +++++++++++++++++++++++++++----
 src/mailman/bin/mailman.py                |  6 ++--
 src/mailman/bin/master.py                 |  2 +-
 src/mailman/bin/tests/test_mailman.py     | 18 +++++++++--
 src/mailman/config/schema.cfg             |  9 +++++-
 src/mailman/core/initialize.py            | 28 +++++++++--------
 src/mailman/rest/docs/systemconf.rst      |  2 ++
 src/mailman/rest/tests/test_systemconf.py |  2 ++
 src/mailman/utilities/plugins.py          |  2 +-
 9 files changed, 94 insertions(+), 25 deletions(-)

(limited to 'src/mailman/bin/mailman.py')

diff --git a/src/mailman/app/docs/plugins.rst b/src/mailman/app/docs/plugins.rst
index a66e4413d..4430179ae 100644
--- a/src/mailman/app/docs/plugins.rst
+++ b/src/mailman/app/docs/plugins.rst
@@ -117,7 +117,6 @@ initialization process.
     ...     def rest_object(self):
     ...         pass
     ... """, file=fp)
-    >>> fp.close()
 
 Running the hooks
 -----------------
@@ -143,12 +142,12 @@ script that will produce no output to force the hooks to run.
 
     >>> import subprocess
     >>> from mailman.testing.layers import ConfigLayer
-    >>> def call():
+    >>> def call(cfg_path, python_path):
     ...     exe = os.path.join(os.path.dirname(sys.executable), 'mailman')
     ...     env = os.environ.copy()
     ...     env.update(
-    ...         MAILMAN_CONFIG_FILE=config_path,
-    ...         PYTHONPATH=config_directory,
+    ...         MAILMAN_CONFIG_FILE=cfg_path,
+    ...         PYTHONPATH=python_path,
     ...         )
     ...     test_cfg = os.environ.get('MAILMAN_EXTRA_TESTING_CFG')
     ...     if test_cfg is not None:
@@ -161,10 +160,51 @@ script that will produce no output to force the hooks to run.
     ...     stdout, stderr = proc.communicate()
     ...     assert proc.returncode == 0, stderr
     ...     print(stdout)
+    ...     print(stderr)
 
-    >>> call()
+    >>> call(config_path, config_directory)
     pre-hook: 1
     post-hook: 2
     <BLANKLINE>
+    <BLANKLINE>
 
     >>> os.remove(config_path)
+
+Deprecated hooks
+----------------
+
+The old-style `pre_hook` and `post_hook` callables are deprecated and are no
+longer called upon startup.
+::
+
+    >>> deprecated_hook_path = os.path.join(config_directory, 'deprecated_hooks.py')
+    >>> with open(deprecated_hook_path, 'w') as fp:
+    ...     print("""\
+    ... def do_something():
+    ...     print("does something")
+    ...
+    ... def do_something_else():
+    ...     print("does something else")
+    ...
+    ... """, file=fp)
+
+    >>> deprecated_config_path = os.path.join(config_directory, 'deprecated.cfg')
+    >>> with open(deprecated_config_path, 'w') as fp:
+    ...     print("""\
+    ... [meta]
+    ... extends: test.cfg
+    ...
+    ... [mailman]
+    ... pre_hook: deprecated_hooks.do_something
+    ... post_hook: deprecated_hooks.do_something_else
+    ... """, file=fp)
+
+    >>> call(deprecated_config_path, config_directory)
+    does something
+    does something else
+    <BLANKLINE>
+    ... UserWarning: The pre_hook configuration value has been replaced by the plugins infrastructure. ...
+    ... UserWarning: The post_hook configuration value has been replaced by the plugins infrastructure. ...
+    <BLANKLINE>
+
+    >>> os.remove(deprecated_config_path)
diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py
index ece223180..f8006218d 100644
--- a/src/mailman/bin/mailman.py
+++ b/src/mailman/bin/mailman.py
@@ -44,9 +44,9 @@ class Subcommands(click.MultiCommand):
                                      self._commands)
             self._loaded = True
 
-    def list_commands(self, ctx):
+    def list_commands(self, ctx):  # pragma: nocover
         self._load()
-        return sorted(self._commands)               # pragma: nocover
+        return sorted(self._commands)
 
     def get_command(self, ctx, name):
         self._load()
@@ -88,7 +88,7 @@ class Subcommands(click.MultiCommand):
 
 
 def initialize_config(ctx, param, value):
-    if ctx.resilient_parsing:
+    if ctx.resilient_parsing:  # pragma: nocover
         return
     initialize(value)
 
diff --git a/src/mailman/bin/master.py b/src/mailman/bin/master.py
index a23fb8e0b..ccaa16398 100644
--- a/src/mailman/bin/master.py
+++ b/src/mailman/bin/master.py
@@ -405,7 +405,7 @@ class Loop:
             except InterruptedError:                # pragma: nocover
                 # If the system call got interrupted, just restart it.
                 continue
-            if pid not in self._kids:
+            if pid not in self._kids:  # pragma: nocover
                 # Not a runner subprocess, maybe a plugin started one
                 # ignore it
                 continue
diff --git a/src/mailman/bin/tests/test_mailman.py b/src/mailman/bin/tests/test_mailman.py
index 54ee54bce..196437291 100644
--- a/src/mailman/bin/tests/test_mailman.py
+++ b/src/mailman/bin/tests/test_mailman.py
@@ -27,6 +27,7 @@ from mailman.config import config
 from mailman.database.transaction import transaction
 from mailman.testing.layers import ConfigLayer
 from mailman.utilities.datetime import now
+from pkg_resources import resource_filename
 from unittest.mock import patch
 
 
@@ -36,7 +37,19 @@ class TestMailmanCommand(unittest.TestCase):
     def setUp(self):
         self._command = CliRunner()
 
-    def test_mailman_command_without_subcommand_prints_help(self):
+    def test_mailman_command_config(self):
+        config_path = resource_filename('mailman.testing', 'testing.cfg')
+        with patch('mailman.bin.mailman.initialize') as init:
+            self._command.invoke(main, ('-C', config_path, 'info'))
+        init.assert_called_once_with(config_path)
+
+    def test_mailman_command_no_config(self):
+        with patch('mailman.bin.mailman.initialize') as init:
+            self._command.invoke(main, ('info',))
+        init.assert_called_once_with(None)
+
+    @patch('mailman.bin.mailman.initialize')
+    def test_mailman_command_without_subcommand_prints_help(self, mock):
         # Issue #137: Running `mailman` without a subcommand raises an
         # AttributeError.
         result = self._command.invoke(main)
@@ -46,7 +59,8 @@ class TestMailmanCommand(unittest.TestCase):
         # command line.
         self.assertEqual(lines[0], 'Usage: main [OPTIONS] COMMAND [ARGS]...')
 
-    def test_mailman_command_with_bad_subcommand_prints_help(self):
+    @patch('mailman.bin.mailman.initialize')
+    def test_mailman_command_with_bad_subcommand_prints_help(self, mock):
         # Issue #137: Running `mailman` without a subcommand raises an
         # AttributeError.
         result = self._command.invoke(main, ('not-a-subcommand',))
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index a55c37ff4..742bceae1 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -74,10 +74,17 @@ html_to_plain_text_command: /usr/bin/lynx -dump $filename
 # unpredictable.
 listname_chars: [-_.0-9a-z]
 
+# Deprecated, callable run before DB initialization.
+pre_hook:
+
+# Deprecated, callable run after DB initialization.
+post_hook:
+
+
 [plugin.master]
 # Plugin package
 # It's sub packages will be searched for components.
-#   - commands for IEmailCommand
+#   - commands for IEmailCommand and ICliSubCommand
 #   - chains for IChain
 #   - rules for IRule
 #   - pipelines for IPipeline
diff --git a/src/mailman/core/initialize.py b/src/mailman/core/initialize.py
index a1314b51e..830bd9b84 100644
--- a/src/mailman/core/initialize.py
+++ b/src/mailman/core/initialize.py
@@ -31,7 +31,9 @@ import traceback
 import mailman.config.config
 import mailman.core.logging
 
+from mailman.core.i18n import _
 from mailman.interfaces.database import IDatabaseFactory
+from mailman.utilities.modules import call_name
 from pkg_resources import resource_string as resource_bytes
 from public import public
 from zope.component import getUtility
@@ -152,27 +154,28 @@ def initialize_2(debug=False, propagate_logs=None, testing=False):
     initialize_plugins()
     # Check for deprecated features in config.
     config = mailman.config.config
-    if 'pre_hook' in config.mailman:   # pragma: no cover
+    if config.mailman.pre_hook:  # pragma: nocover
         warnings.warn(
-            'The pre_hook configuration value has been replaced by the '
-            'plugins infrastructure.', DeprecationWarning)
+            _('The pre_hook configuration value has been replaced by the '
+              'plugins infrastructure.'), UserWarning)
+        call_name(config.mailman.pre_hook)
     # Run the plugin pre_hooks, if one fails, disable the offending plugin.
     for name in list(config.plugins.keys()):
         plugin = config.plugins[name]
         try:
             plugin.pre_hook()
-        except BaseException:   # pragma: no cover
+        except:   # pragma: nocover
             traceback.print_exc()
-            warnings.warn('Plugin {} failed to run its pre_hook,'
-                          'it will be disabled and its components'
-                          'wont be loaded.'.format(name), RuntimeWarning)
+            warnings.warn(_('Plugin $name failed to run its pre_hook,'
+                            'it will be disabled and its components'
+                            'wont be loaded.'), RuntimeWarning)
             # It failed, push disabling overlay to config. This will stop
             # components from being loaded.
             config.push(name + '_error', """\
 [plugin.{}]
 enable: no
             """.format(name))
-            # And forget about it. This will stop running its pot_hook.
+            # And forget about it. This will stop running its post_hook.
             del config.plugins[name]
     # Instantiate the database class, ensure that it's of the right type, and
     # initialize it.  Then stash the object on our configuration object.
@@ -199,14 +202,15 @@ def initialize_3():
     """
     # Run the plugin post_hooks
     config = mailman.config.config
-    if 'post_hook' in config.mailman:   # pragma: no cover
+    if config.mailman.post_hook:  # pragma: nocover
         warnings.warn(
-            'The post_hook configuration value has been replaced by the '
-            'plugins infrastructure.', DeprecationWarning)
+            _('The post_hook configuration value has been replaced by the '
+              'plugins infrastructure.'), UserWarning)
+        call_name(config.mailman.post_hook)
     for plugin in config.plugins.values():
         try:
             plugin.post_hook()
-        except:   # pragma: no cover
+        except:   # pragma: nocover
             # A post_hook may fail, here we just hope for the best that the
             # plugin can work even if it post_hook failed as it's components
             # are already loaded.
diff --git a/src/mailman/rest/docs/systemconf.rst b/src/mailman/rest/docs/systemconf.rst
index f5c9b691a..a65b81ab8 100644
--- a/src/mailman/rest/docs/systemconf.rst
+++ b/src/mailman/rest/docs/systemconf.rst
@@ -24,6 +24,8 @@ You can also get all the values for a particular section, such as the
     listname_chars: [-_.0-9a-z]
     noreply_address: noreply
     pending_request_life: 3d
+    post_hook:
+    pre_hook:
     self_link: http://localhost:9001/3.0/system/configuration/mailman
     sender_headers: from from_ reply-to sender
     site_owner: noreply@example.com
diff --git a/src/mailman/rest/tests/test_systemconf.py b/src/mailman/rest/tests/test_systemconf.py
index fe5d951a8..59f0619a3 100644
--- a/src/mailman/rest/tests/test_systemconf.py
+++ b/src/mailman/rest/tests/test_systemconf.py
@@ -46,6 +46,8 @@ class TestSystemConfiguration(unittest.TestCase):
             listname_chars='[-_.0-9a-z]',
             noreply_address='noreply',
             pending_request_life='3d',
+            post_hook='',
+            pre_hook='',
             self_link='http://localhost:9001/3.0/system/configuration/mailman',
             sender_headers='from from_ reply-to sender',
             site_owner='noreply@example.com',
diff --git a/src/mailman/utilities/plugins.py b/src/mailman/utilities/plugins.py
index 3fb5e49bc..aad909056 100644
--- a/src/mailman/utilities/plugins.py
+++ b/src/mailman/utilities/plugins.py
@@ -70,7 +70,7 @@ def add_pluggable_components(subpackage, interface, mapping):
     """
     for component in find_pluggable_components(subpackage, interface):
         if component.name in mapping:
-            raise RuntimeError(     # pragma: no cover
+            raise RuntimeError(     # pragma: nocover
                 'Duplicate key "{}" found in {}; previously {}'.format(
                     component.name, component, mapping[component.name]))
         mapping[component.name] = component
-- 
cgit v1.2.3-70-g09d2