Rework plugin infrastructure to nix side-effects

This reworks the plugin infrastructure so as to remove module-loading
side-effects which were making things a pain in the ass to test.

With the new system, there's no auto-registering meta class. Instead
plugins do whatever they want and then specify a hooks dict that maps
hook names to callables for the things they're tying into. The most
common one (and the only one we've implemented so far) is "setup".

This also simplifies the sampleplugin a little by moving the code
to __init__.py.

docs/source/pluginwriter/quickstart.rst

@@ -50,7 +50,8 @@ The inner ``sampleplugin`` directory is the Python package that holds
 your plugin's code.
 
 The ``__init__.py`` denotes that this is a Python package. It also
-holds the plugin code.
+holds the plugin code and the ``hooks`` dict that specifies which
+hooks the sampleplugin uses.
 
 
 Step 2: README
@@ -107,43 +108,39 @@ The code for ``__init__.py`` looks like this:
 
 .. code-block:: python
    :linenos:
-   :emphasize-lines: 8,19
+   :emphasize-lines: 12,23
 
     import logging
     from mediagoblin.tools.pluginapi import Plugin, get_config
 
 
+    # This creates a logger that you can use to log information to
+    # the console or a log file.
     _log = logging.getLogger(__name__)
 
 
-    class SamplePlugin(Plugin):
+    # This is the function that gets called when the setup
-        """
+    # hook fires.
-        This is a sample plugin class. It automatically registers itself
+    def setup_plugin():
-        with MediaGoblin when this module is imported.
+        _log.info("I've been started!")
-
+        config = get_config('sampleplugin')
-        The setup_plugin method prints configuration for this plugin if
+        if config:
-        there is any.
+            _log.info('%r' % config)
-        """
+        else:
-        def __init__(self):
+            _log.info('There is no configuration set.')
-            pass
-
-        def setup_plugin(self):
-            _log.info("I've been started!")
-            config = get_config('sampleplugin')
-            if config:
-                _log.info('%r' % config)
-            else:
-                _log.info('There is no configuration set.')
 
 
-Line 8 defines a class called ``SamplePlugin`` that subclasses
+    # This is a dict that specifies which hooks this plugin uses.
-``Plugin`` from ``mediagoblin.tools.pluginapi``. When the class is
+    # This one only uses one hook: setup.
-defined, it gets registered with MediaGoblin and MediaGoblin will then
+    hooks = {
-call ``setup_plugin`` on it.
+        'setup': setup_plugin
+        }
 
-Line 19 defines ``setup_plugin``. This gets called when MediaGoblin
+
-starts up after it's registered all the plugins. This is where you can
+Line 12 defines the ``setup_plugin`` function.
-do any initialization for your plugin.
+
+Line 23 defines ``hooks``. When MediaGoblin loads this file, it sees
+``hooks`` and registers all the callables with their respective hooks.
 
 
 Step 6: Installation and configuration

mediagoblin/app.py

@@ -32,7 +32,7 @@ from mediagoblin.init.plugins import setup_plugins
 from mediagoblin.init import (get_jinja_loader, get_staticdirector,
     setup_global_and_app_config, setup_workbench, setup_database,
     setup_storage, setup_beaker_cache)
-from mediagoblin.tools.pluginapi import PluginCache
+from mediagoblin.tools.pluginapi import PluginManager
 
 
 _log = logging.getLogger(__name__)
@@ -82,14 +82,14 @@ class MediaGoblinApp(object):
         self.template_loader = get_jinja_loader(
             app_config.get('local_templates'),
             self.current_theme,
-            PluginCache().get_template_paths()
+            PluginManager().get_template_paths()
             )
 
         # Set up storage systems
         self.public_store, self.queue_store = setup_storage()
 
         # set up routing
-        self.routing = routing.get_mapper(PluginCache().get_routes())
+        self.routing = routing.get_mapper(PluginManager().get_routes())
 
         # set up staticdirector tool
         self.staticdirector = get_staticdirector(app_config)

mediagoblin/init/plugins/__init__.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
+import sys
 
 from mediagoblin import mg_globals
 from mediagoblin.tools import pluginapi
@@ -36,24 +37,21 @@ def setup_plugins():
         _log.info("No plugins to load")
         return
 
-    pcache = pluginapi.PluginCache()
+    pman = pluginapi.PluginManager()
 
     # Go through and import all the modules that are subsections of
-    # the [plugins] section.
+    # the [plugins] section and read in the hooks.
     for plugin_module, config in plugin_section.items():
         _log.info("Importing plugin module: %s" % plugin_module)
+        pman.register_plugin(plugin_module)
         # If this throws errors, that's ok--it'll halt mediagoblin
         # startup.
         __import__(plugin_module)
+        plugin = sys.modules[plugin_module]
+        if hasattr(plugin, 'hooks'):
+            pman.register_hooks(plugin.hooks)
 
-    # Note: One side-effect of importing things is that anything that
+    # Execute anything registered to the setup hook.
-    # subclassed pluginapi.Plugin is registered.
+    setup_list = pman.get_hook_callables('setup')
-
+    for fun in setup_list:
-    # Go through all the plugin classes, instantiate them, and call
+        fun()
-    # setup_plugin so they can figure things out.
-    for plugin_class in pcache.plugin_classes:
-        name = plugin_class.__module__ + "." + plugin_class.__name__
-        _log.info("Loading plugin: %s" % name)
-        plugin_obj = plugin_class()
-        plugin_obj.setup_plugin()
-        pcache.register_plugin_object(plugin_obj)

mediagoblin/plugins/flatpagesfile/__init__.py

@@ -53,27 +53,27 @@ def flatpage_handler_builder(template):
     return _flatpage_handler_builder
 
 
-class FlatpagesFilePlugin(pluginapi.Plugin):
+def setup_plugin():
-    """
+    config = pluginapi.get_config('mediagoblin.plugins.flatpagesfile')
-    This is the flatpages plugin class. See the README for how to use
-    flatpages.
-    """
-    def setup_plugin(self):
-        self.config = pluginapi.get_config('mediagoblin.plugins.flatpagesfile')
 
-        _log.info('Setting up flatpagesfile....')
+    _log.info('Setting up flatpagesfile....')
 
-        # Register the template path.
+    # Register the template path.
-        pluginapi.register_template_path(os.path.join(PLUGIN_DIR, 'templates'))
+    pluginapi.register_template_path(os.path.join(PLUGIN_DIR, 'templates'))
 
-        pages = self.config.items()
+    pages = config.items()
 
-        routes = []
+    routes = []
-        for name, (url, template) in pages:
+    for name, (url, template) in pages:
-            name = 'flatpagesfile.%s' % name.strip()
+        name = 'flatpagesfile.%s' % name.strip()
-            controller = flatpage_handler_builder(template)
+        controller = flatpage_handler_builder(template)
-            routes.append(
+        routes.append(
-                Route(name, url, controller=controller))
+            Route(name, url, controller=controller))
 
-        pluginapi.register_routes(routes)
+    pluginapi.register_routes(routes)
-        _log.info('Done setting up flatpagesfile!')
+    _log.info('Done setting up flatpagesfile!')
+
+
+hooks = {
+    'setup': setup_plugin
+    }

mediagoblin/plugins/sampleplugin/__init__.py

@@ -15,6 +15,28 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-# This imports the module that has the Plugin subclass in it which
+import logging
-# causes that module to get imported and that class to get registered.
+
-import mediagoblin.plugins.sampleplugin.main
+from mediagoblin.tools.pluginapi import get_config
+
+
+_log = logging.getLogger(__name__)
+
+
+_setup_plugin_called = 0
+
+def setup_plugin():
+    global _setup_plugin_called
+
+    _log.info('Sample plugin set up!')
+    config = get_config('mediagoblin.plugins.sampleplugin')
+    if config:
+        _log.info('%r' % config)
+    else:
+        _log.info('There is no configuration set.')
+    _setup_plugin_called += 1
+
+
+hooks = {
+    'setup': setup_plugin
+    }

mediagoblin/plugins/sampleplugin/main.py

@@ -1,42 +0,0 @@
-# GNU MediaGoblin -- federated, autonomous media hosting
-# Copyright (C) 2011, 2012 MediaGoblin contributors.  See AUTHORS.
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Affero General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Affero General Public License for more details.
-#
-# You should have received a copy of the GNU Affero General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import logging
-from mediagoblin.tools.pluginapi import Plugin, get_config
-
-
-_log = logging.getLogger(__name__)
-
-
-class SamplePlugin(Plugin):
-    """
-    This is a sample plugin class. It automatically registers itself
-    with mediagoblin when this module is imported.
-
-    The setup_plugin method prints configuration for this plugin if
-    it exists.
-    """
-    def __init__(self):
-        self._setup_plugin_called = 0
-
-    def setup_plugin(self):
-        _log.info('Sample plugin set up!')
-        config = get_config('mediagoblin.plugins.sampleplugin')
-        if config:
-            _log.info('%r' % config)
-        else:
-            _log.info('There is no configuration set.')
-        self._setup_plugin_called += 1

mediagoblin/tests/test_pluginapi.py

@@ -37,8 +37,8 @@ def with_cleanup(*modules_to_delete):
                     pass
             # The plugin cache gets populated as a side-effect of
             # importing, so it's best to clear it before and after a test.
-            pcache = pluginapi.PluginCache()
+            pman = pluginapi.PluginManager()
-            pcache.clear()
+            pman.clear()
             try:
                 return fun(*args, **kwargs)
             finally:
@@ -51,7 +51,7 @@ def with_cleanup(*modules_to_delete):
                         del sys.modules[module]
                     except KeyError:
                         pass
-                pcache.clear()
+                pman.clear()
 
         _with_cleanup_inner.__name__ = fun.__name__
         return _with_cleanup_inner
@@ -93,16 +93,14 @@ def test_no_plugins():
     mg_globals.app_config = cfg['mediagoblin']
     mg_globals.global_config = cfg
 
-    pcache = pluginapi.PluginCache()
+    pman = pluginapi.PluginManager()
     setup_plugins()
 
     # Make sure we didn't load anything.
-    eq_(len(pcache.plugin_classes), 0)
+    eq_(len(pman.plugins), 0)
-    eq_(len(pcache.plugin_objects), 0)
 
 
-@with_cleanup('mediagoblin.plugins.sampleplugin',
+@with_cleanup('mediagoblin.plugins.sampleplugin')
-              'mediagoblin.plugins.sampleplugin.main')
 def test_one_plugin():
     """Run setup_plugins with a single working plugin"""
     cfg = build_config([
@@ -115,22 +113,21 @@ def test_one_plugin():
     mg_globals.app_config = cfg['mediagoblin']
     mg_globals.global_config = cfg
 
-    pcache = pluginapi.PluginCache()
+    pman = pluginapi.PluginManager()
     setup_plugins()
 
-    # Make sure we only found one plugin class
+    # Make sure we only found one plugin
-    eq_(len(pcache.plugin_classes), 1)
+    eq_(len(pman.plugins), 1)
-    # Make sure the class is the one we think it is.
+    # Make sure the plugin is the one we think it is.
-    eq_(pcache.plugin_classes[0].__name__, 'SamplePlugin')
+    eq_(pman.plugins[0], 'mediagoblin.plugins.sampleplugin')
-
+    # Make sure there was one hook registered
-    # Make sure there was one plugin created
+    eq_(len(pman.hooks), 1)
-    eq_(len(pcache.plugin_objects), 1)
+    # Make sure _setup_plugin_called was called once
-    # Make sure we called setup_plugin on SamplePlugin
+    import mediagoblin.plugins.sampleplugin
-    eq_(pcache.plugin_objects[0]._setup_plugin_called, 1)
+    eq_(mediagoblin.plugins.sampleplugin._setup_plugin_called, 1)
 
 
-@with_cleanup('mediagoblin.plugins.sampleplugin',
+@with_cleanup('mediagoblin.plugins.sampleplugin')
-              'mediagoblin.plugins.sampleplugin.main')
 def test_same_plugin_twice():
     """Run setup_plugins with a single working plugin twice"""
     cfg = build_config([
@@ -144,15 +141,15 @@ def test_same_plugin_twice():
     mg_globals.app_config = cfg['mediagoblin']
     mg_globals.global_config = cfg
 
-    pcache = pluginapi.PluginCache()
+    pman = pluginapi.PluginManager()
     setup_plugins()
 
-    # Make sure we only found one plugin class
+    # Make sure we only found one plugin
-    eq_(len(pcache.plugin_classes), 1)
+    eq_(len(pman.plugins), 1)
-    # Make sure the class is the one we think it is.
+    # Make sure the plugin is the one we think it is.
-    eq_(pcache.plugin_classes[0].__name__, 'SamplePlugin')
+    eq_(pman.plugins[0], 'mediagoblin.plugins.sampleplugin')
-
+    # Make sure there was one hook registered
-    # Make sure there was one plugin created
+    eq_(len(pman.hooks), 1)
-    eq_(len(pcache.plugin_objects), 1)
+    # Make sure _setup_plugin_called was called once
-    # Make sure we called setup_plugin on SamplePlugin
+    import mediagoblin.plugins.sampleplugin
-    eq_(pcache.plugin_objects[0]._setup_plugin_called, 1)
+    eq_(mediagoblin.plugins.sampleplugin._setup_plugin_called, 1)

mediagoblin/tools/pluginapi.py

@@ -15,8 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 """
-This module implements the plugin api bits and provides the plugin
+This module implements the plugin api bits.
-base.
 
 Two things about things in this module:
 
@@ -30,8 +29,8 @@ How do plugins work?
 ====================
 
 Plugins are structured like any Python project. You create a Python package.
-In that package, you define a high-level ``__init__.py`` that either defines
+In that package, you define a high-level ``__init__.py`` module that has a 
-or imports modules that define classes that inherit from the ``Plugin`` class.
+``hooks`` dict that maps hooks to callables that implement those hooks.
 
 Additionally, you want a LICENSE file that specifies the license and a
 ``setup.py`` that specifies the metadata for packaging your plugin. A rough
@@ -42,23 +41,19 @@ file structure could look like this::
      |- README           # holds plugin project information
      |- LICENSE          # holds license information
      |- myplugin/        # plugin package directory
-        |- __init__.py   # imports myplugin.main
+        |- __init__.py   # has hooks dict and code
-        |- main.py       # code for plugin
 
 
 Lifecycle
 =========
 
 1. All the modules listed as subsections of the ``plugins`` section in
-   the config file are imported. This causes any ``Plugin`` subclasses in
+   the config file are imported. MediaGoblin registers any hooks in
-   those modules to be defined and when the classes are defined they get
+   the ``hooks`` dict of those modules.
-   automatically registered with the ``PluginCache``.
 
-2. After all plugin modules are imported, registered plugin classes are
+2. After all plugin modules are imported, the ``setup`` hook is called
-   instantiated and ``setup_plugin`` is called for each plugin object.
+   allowing plugins to do any set up they need to do.
 
-   Plugins can do any setup they need to do in their ``setup_plugin``
-   method.
 """
 
 import logging
@@ -69,14 +64,19 @@ from mediagoblin import mg_globals
 _log = logging.getLogger(__name__)
 
 
-class PluginCache(object):
+class PluginManager(object):
-    """Cache of plugin things"""
+    """Manager for plugin things
+
+    .. Note::
+
+       This is a Borg class--there is one and only one of this class.
+    """
     __state = {
         # list of plugin classes
-        "plugin_classes": [],
+        "plugins": [],
 
-        # list of plugin objects
+        # map of hook names -> list of callables for that hook
-        "plugin_objects": [],
+        "hooks": {},
 
         # list of registered template paths
         "template_paths": set(),
@@ -87,19 +87,31 @@ class PluginCache(object):
 
     def clear(self):
         """This is only useful for testing."""
-        del self.plugin_classes[:]
+        # Why lists don't have a clear is not clear.
-        del self.plugin_objects[:]
+        del self.plugins[:]
+        del self.routes[:]
+        self.hooks.clear()
+        self.template_paths.clear()
 
     def __init__(self):
         self.__dict__ = self.__state
 
-    def register_plugin_class(self, plugin_class):
+    def register_plugin(self, plugin):
         """Registers a plugin class"""
-        self.plugin_classes.append(plugin_class)
+        self.plugins.append(plugin)
 
-    def register_plugin_object(self, plugin_obj):
+    def register_hooks(self, hook_mapping):
-        """Registers a plugin object"""
+        """Takes a hook_mapping and registers all the hooks"""
-        self.plugin_objects.append(plugin_obj)
+        for hook, callables in hook_mapping.items():
+            if isinstance(callables, (list, tuple)):
+                self.hooks.setdefault(hook, []).extend(list(callables))
+            else:
+                # In this case, it's actually a single callable---not a
+                # list of callables.
+                self.hooks.setdefault(hook, []).append(callables)
+
+    def get_hook_callables(self, hook_name):
+        return self.hooks.get(hook_name, [])
 
     def register_template_path(self, path):
         """Registers a template path"""
@@ -117,38 +129,6 @@ class PluginCache(object):
         return tuple(self.routes)
 
 
-class MetaPluginClass(type):
-    """Metaclass for PluginBase derivatives"""
-    def __new__(cls, name, bases, attrs):
-        new_class = super(MetaPluginClass, cls).__new__(cls, name, bases, attrs)
-        parents = [b for b in bases if isinstance(b, MetaPluginClass)]
-        if not parents:
-            return new_class
-        PluginCache().register_plugin_class(new_class)
-        return new_class
-
-
-class Plugin(object):
-    """Extend this class for plugins.
-
-    Example::
-
-        from mediagoblin.tools.pluginapi import Plugin
-
-        class MyPlugin(Plugin):
-            ...
-
-            def setup_plugin(self):
-                ....
-
-    """
-
-    __metaclass__ = MetaPluginClass
-
-    def setup_plugin(self):
-        pass
-
-
 def register_routes(routes):
     """Registers one or more routes
 
@@ -182,9 +162,9 @@ def register_routes(routes):
     """
     if isinstance(routes, (tuple, list)):
         for route in routes:
-            PluginCache().register_route(route)
+            PluginManager().register_route(route)
     else:
-        PluginCache().register_route(routes)
+        PluginManager().register_route(routes)
 
 
 def register_template_path(path):
@@ -205,7 +185,7 @@ def register_template_path(path):
        that will have no effect on template loading.
 
     """
-    PluginCache().register_template_path(path)
+    PluginManager().register_template_path(path)
 
 
 def get_config(key):