Added plugin decorator and class system (with interface functions)

shepherd/__init__.py

@@ -1,5 +1,5 @@
-from .agent.plugin import plugin  # noqa
 from .agent.plugin import PluginInterface # noqa
+from .agent.plugin import plugin_class  # noqa
 from .agent.plugin import plugin_function # noqa
 from .agent.plugin import plugin_hook # noqa
 from .agent.plugin import plugin_attachment # noqa

shepherd/agent/plugin.py

@@ -5,6 +5,7 @@ import logging
 import sys
 import pkgutil
 from collections import namedtuple
+from functools import partial
 from types import MappingProxyType
 import pkg_resources
 from configspec import ConfigSpecification
@@ -24,14 +25,56 @@ class PluginLoadError(Exception):
     pass
 
 
-def plugin(cls):
+ClassMarker = namedtuple("ClassMarker", [])
+
+
+def is_instance_check(classtype):
+    def instance_checker(obj):
+        return isinstance(obj, classtype)
+    return instance_checker
+
+
+def plugin_class(cls):
+    """
+    Class decorator, used to indicate that a class is to be used as the Plugin Class
+     for this plugin. Note that only one plugin class is allowed per plugin. Only works when placed
+     in the root of the plugin module or package (same as the interface)
+     Use on the class definition:
+
+         @plugin_class
+         class MyPluginClass:
+
+    This is equivalent to registering the class directly with the plugin interface later:
+
+        interface.register_plugin_class(MyPluginClass)
+    """
+    if not inspect.isclass(cls):
+        raise PluginLoadError(F"@plugin_class can only be used to decorate a class")
+    cls._shepherd_load_marker = ClassMarker()
     return cls
 
 
-def plugin_function():
+FunctionMarker = namedtuple("FunctionMarker", ["name"])
-    def wrapped(method):
+
-        return method
+
-    return wrapped
+def plugin_function(func=None, *, name=None):
+    """
+    Method decorator to register a method as a plugin interface function. Either used directly:
+        @plugin_function
+        def my_method(self):
+
+    or with optional keyword arguments:
+        @plugin_function(name="someOtherName")
+        def my_badly_named_method(self):
+
+    Can only be used within the registered Plugin Class (either with @plugin_class or
+    interface.register_plugin_class() )
+    """
+    if func is None:
+        return partial(plugin_function, name=name)
+
+    func._shepherd_load_marker = FunctionMarker(name)
+    return func
 
 
 def plugin_hook(method):
@@ -45,66 +88,55 @@ def plugin_attachment(hookname):
 
 
 class InterfaceFunction():
-    def __init__(self, func):
+    def __init__(self, func, name=None, unbound=False):
-        if not callable(func):
+        """
-            raise TypeError("Argument to InterfaceFunction must be callable")
+        Wrapper around a callable to define a plugin interface function. If unbound true, will use
+         a temp object to analyse the signature, and defer binding it as a method until _bind() is
+         called.
+        """
         self.func = func
+        self._unbound = unbound
+        sigfunc = func
 
-    def __call__(self, *args, **kwargs):
+        if unbound:
-        return self.func(*args, **kwargs)
+            sigfunc = func.__get__(object())
 
+        if not callable(sigfunc):
+            raise TypeError("InterfaceFunction can only be created around a callable or method.")
 
-class PluginInterface():
+        if name:
-    @staticmethod
+            self.name = name
-    def _load_interface(module, plugin_name) -> "PluginInterface":
+        else:
+            self.name = sigfunc.__name__
+
+        params = inspect.signature(sigfunc)
+        log.debug(F"Loaded interface function {self.name} with parameters: {params}")
+
+    def _bind(self, obj):
         """
-        Finds PluginInterface instance in a plugin and returns it.
+        Bind the wrapped method to an object
         """
-        def is_plugininterface(member):
+        self.func = self.func.__get__(obj)
-            return isinstance(member, PluginInterface)
+        self._unbound = False
 
-        interface_list = inspect.getmembers(module, is_plugininterface)
+    def __call__(self, *args, **kwargs):
-        if not interface_list:
+        if self._unbound:
-            raise PluginLoadError("Imported shepherd plugins must contain an instance"
+            raise Exception(
-                                  " of PluginInterface")
+                "Cannot call unbound InterfaceFunction (plugin has not yet initialised)")
+        return self.func(*args, **kwargs)
 
-        if len(interface_list) > 1:
-            log.warning(F"Plugin module {module.__name__} has more"
-                        F" than one PluginInterface instance.")
 
-        _, interface = interface_list[0]
+class PluginInterface():
-        interface._plugin_name = plugin_name
-        return interface
 
     def __init__(self):
         self._confspec = None
         self._loaded = False
         self._functions = {}
+        self._plugin_class = None
         self.config = None
         self.plugins = None
         self._plugin_name = "<not yet loaded>"
 
-    def _load_confspec(self, module):
-        """
-        If not already registered, looks for a ConfigSpecification instance in a plugin,
-        using a blank one by default.
-        """
-        if self._confspec is not None:
-            return
-
-        def is_confspec(member):
-            return isinstance(member, ConfigSpecification)
-
-        confspec_list = inspect.getmembers(module, is_confspec)
-        if not confspec_list:
-            self._confspec = ConfigSpecification()
-
-        if len(confspec_list) > 1:
-            log.warning(F"Plugin {self._plugin_name} has more"
-                        F" than one root ConfigSpecification instance.")
-
-        self.register_confspec(confspec_list[0][1])
-
     def _load_pluginclass(self, module):
         pass
 
@@ -115,20 +147,36 @@ class PluginInterface():
 
     def register_confspec(self, confspec):
         self._load_guard()
+        if self._confspec is not None:
+            raise PluginLoadError("Plugin can only register one ConfigSpecification")
         if not isinstance(confspec, ConfigSpecification):
             raise PluginLoadError("confspec must be an instance of ConfigSpecification")
         self._confspec = confspec
 
+    def register_class(self, cls):
+        self._load_guard()
+        if self._plugin_class is not None:
+            raise PluginLoadError("Plugin can only register one plugin class")
+        if not inspect.isclass(cls):
+            raise PluginLoadError("plugin_class must be a class")
+        self._plugin_class = cls
+
     def register_function(self, func, name=None):
+        """
+        Register a function or method as an interface function for the plugin. If name is not
+         provided, the name of the callable will be used.
+        """
         self._load_guard()
 
-        if not name:
+        if isinstance(func, InterfaceFunction):
-            name = func.__name__
+            ifunc = func
+        else:
+            ifunc = InterfaceFunction(func, name)
 
-        if name in self._functions:
+        if ifunc.name in self._functions:
-            raise PluginLoadError(F"Interface function with name '{name}' already exists")
+            raise PluginLoadError(F"Interface function with name '{ifunc.name}' already exists")
 
-        self._functions[name] = InterfaceFunction(func)
+        self._functions[ifunc.name] = ifunc
 
     @property
     def confspec(self):
@@ -175,6 +223,13 @@ def load_plugin(plugin_name, plugin_dir=None):
             names match the plugin name.
         3. Any installed packages supplying the ''shepherd.plugin'' entrypoint.
 
+    Once a module is found, loading it involves scanning the root of the module for a
+     PluginInterface instance. If a confspec isn't registered, a ConfigSpecification instance
+     at the module root will also be implicitly registered to the interface.
+
+    Lastly, any plugin decorators are scanned for and registered (including a plugin class if
+     present).
+
     Args:
         plugin_name: Name used to try and locate the plugin
         plugin_dir: Optional directory path to be used for custom plugins
@@ -188,32 +243,76 @@ def load_plugin(plugin_name, plugin_dir=None):
     # Each of the 3 plugin sources have different import mechanisms. Discovery is broken out to
     # allow them to be listed. Using a try/except block wouldn't be able to tell the difference
     # between a plugin not being found or //it's// imports not loading correctly.
-    mod = None
+    module = None
     if plugin_name in discover_base_plugins():
-        mod = importlib.import_module(base_plugins.__name__+'.'+plugin_name)
+        module = importlib.import_module(base_plugins.__name__+'.'+plugin_name)
         log.info(F"Loading base plugin {plugin_name}")
 
     elif plugin_name in discover_custom_plugins(plugin_dir):
         saved_syspath = sys.path
         try:
             sys.path = [str(plugin_dir)]
-            mod = importlib.import_module(plugin_name)
+            module = importlib.import_module(plugin_name)
         finally:
             sys.path = saved_syspath
-        modpath = getattr(mod, "__path__", [mod.__file__])[0]
+        modulepath = getattr(module, "__path__", [module.__file__])[0]
-        log.info(F"Loading custom plugin {plugin_name} from {modpath}")
+        log.info(F"Loading custom plugin {plugin_name} from {modulepath}")
 
     elif plugin_name in discover_installed_plugins():
-        mod = pkg_resources.iter_entry_points('shepherd.plugins', plugin_name)[0].load()
+        module = pkg_resources.iter_entry_points('shepherd.plugins', plugin_name)[0].load()
-        log.info(F"Loading installed plugin {plugin_name} from {mod.__name__}")
+        log.info(F"Loading installed plugin {plugin_name} from {module.__name__}")
 
-    if not mod:
+    if not module:
         raise PluginLoadError("Could not find plugin "+plugin_name)
 
-    interface = PluginInterface._load_interface(mod, plugin_name)
+    # Now we have the module, scan it for the two implicit objects we look for - the interface and
-    interface._load_confspec(mod)
+    # the confspec
+
+    interface_list = inspect.getmembers(module, is_instance_check(PluginInterface))
+    if not interface_list:
+        raise PluginLoadError("Imported shepherd plugins must contain an instance"
+                              " of PluginInterface")
+
+    if len(interface_list) > 1:
+        log.warning(F"Plugin module {module.__name__} has more"
+                    F" than one PluginInterface instance.")
+
+    _, interface = interface_list[0]
+    interface._plugin_name = plugin_name
+
+    # Only looks for implicit confspec if one isn't registered. Uses a blank one if none are
+    # supplied.
+
+    if interface._confspec is None:
+
+        confspec_list = inspect.getmembers(module, is_instance_check(ConfigSpecification))
+        if confspec_list:
+            if len(confspec_list) > 1:
+                log.warning(F"Plugin {interface._plugin_name} has more"
+                            F" than one root ConfigSpecification instance.")
+            interface.register_confspec(confspec_list[0][1])
+        else:
+            interface._confspec = ConfigSpecification()
+
+    # Scan module for load markers left by decorators
+
+    for name, attr in module.__dict__.items():
+        if hasattr(attr, "_shepherd_load_marker"):
+            if isinstance(attr._shepherd_load_marker, FunctionMarker):
+                interface.register_function(attr, **attr._shepherd_load_marker._asdict())
+            elif isinstance(attr._shepherd_load_marker, ClassMarker):
+                interface.register_class(attr)
+
+    if interface._plugin_class is not None:
+        # Scan plugin class for marked methods
+        for name, attr in interface._plugin_class.__dict__.items():
+            if hasattr(attr, "_shepherd_load_marker"):
+                if isinstance(attr._shepherd_load_marker, FunctionMarker):
+                    # Instance doesn't exist yet, so need to save unbound methods for binding later
+                    unbound_func = InterfaceFunction(
+                        attr, unbound=True, **attr._shepherd_load_marker._asdict())
+                    interface.register_function(unbound_func)
 
-    # TODO Populate plugin interface
     interface._loaded = True
 
     _loaded_plugins[plugin_name] = interface
@@ -224,19 +323,29 @@ def init_plugins(plugin_configs, core_config):
     """
     Initialise plugins named as keys in plugin_configs. Plugins must already be loaded.
     """
-    # Run plugin init and init hooks
-    plugin_names = plugin_configs.keys()
 
-    for plugin_name in plugin_names:
+    # Pick out plugins to init
+    for plugin_name in plugin_configs.keys():
         plugin_interfaces[plugin_name] = _loaded_plugins[plugin_name]
-        plugin_functions[plugin_name] = _loaded_plugins[plugin_name]._functions
 
     plugin_functions_tuples = {}
+    plugin_functions_view = MappingProxyType(plugin_functions_tuples)
+
+    # Run plugin init and init hooks
+    for plugin_name, interface in plugin_interfaces.items():
+        interface.plugins = plugin_functions_view
+        interface.config = plugin_configs[plugin_name]
+
+        # If it has one, instantiate the plugin object and bind functions
+        if interface._plugin_class is not None:
+            interface._plugin_obj = interface._plugin_class()
+            for funcname, ifunc in interface._functions.items():
+                if ifunc._unbound:
+                    ifunc._bind(interface._plugin_obj)
+
+        plugin_functions[plugin_name] = interface._functions
+
+    # Fill in the interface functions view that plugins now have access to
     for name, functions in plugin_functions.items():
         plugin_functions_tuples[name] = namedtuple(
             F'{name}_interface_functions', functions.keys())(**functions)
-
-    plugin_functions_view = MappingProxyType(plugin_functions_tuples)
-
-    for name, plugin_interface in plugin_interfaces.items():
-        plugin_interface.plugins = plugin_functions_view

tests/classtestplugin.py

@@ -0,0 +1,47 @@
+from configspec import *
+from shepherd import PluginInterface, plugin_class, plugin_function, plugin_hook, plugin_attachment
+
+interface = PluginInterface()
+
+
+confspec = ConfigSpecification()
+confspec.add_spec("spec1", StringSpec(helptext="helping!"))
+
+
+@plugin_function
+def module_function(a: str):
+    return "module func return"
+
+
+@plugin_class
+class ClassPlugin():
+    def __init__(self):
+        self.config = interface.config
+        self.interface = interface
+        self.plugins = interface.plugins
+        # self.hooks = interface.hooks
+
+    @plugin_function
+    def instance_method(self, a: str):
+        return "instance method return"
+
+    @plugin_function
+    @classmethod
+    def class_method(cls, a: str):
+        return "class method return"
+
+    @plugin_function
+    @staticmethod
+    def static_method(a: str):
+        return "static method return"
+
+    # @plugin_hook
+    # def callback(self):
+    #    pass
+
+    # @plugin_attachment("pluginname.hookname")
+    # def caller(self):
+    #    pass
+
+
+# interface.register_plugin(SystemPlugin)

tests/test_plugins.py

@@ -1,8 +1,17 @@
 # pylint: disable=redefined-outer-name
+import sys
 import pytest
 from shepherd.agent import plugin
 
 
+def clear_plugin_state(plugin_name):
+    # Make sure it's loading a fresh copy
+    sys.modules.pop(plugin_name, None)
+    plugin._loaded_plugins = {}
+    plugin.plugin_interfaces = {}
+    plugin.plugin_functions = {}
+
+
 @pytest.fixture
 def simple_plugin(request):
     interface = plugin.load_plugin("simpletestplugin", request.fspath.dirname)
@@ -22,9 +31,10 @@ def test_simple_interface_function_load(simple_plugin: plugin.PluginInterface):
 
 @pytest.fixture
 def simple_running_plugin(request):
+    clear_plugin_state("simpletestplugin")
     interface = plugin.load_plugin("simpletestplugin", request.fspath.dirname)
     template_config = interface.confspec.get_template()
-    plugin.init_plugins({"simpletestplugin": template_config}, {"core_conf": "core_conf_vals"})
+    plugin.init_plugins({"simpletestplugin": template_config}, {"ckey": "cval"})
     return interface
 
 
@@ -49,3 +59,24 @@ def test_dirty_plugin_load(request):
 
     # Should prefer the confspec actually registered, even if declared after
     assert "spec2" in interface.confspec.spec_dict
+
+
+@pytest.fixture
+def running_class_plugin(request):
+    clear_plugin_state("classtestplugin")
+    interface = plugin.load_plugin("classtestplugin", request.fspath.dirname)
+    template_config = interface.confspec.get_template()
+    plugin.init_plugins({"classtestplugin": template_config}, {"core_conf": "core_conf_vals"})
+    return interface
+
+
+def test_class_plugin_init(running_class_plugin):
+    assert "classtestplugin" in plugin.plugin_interfaces
+
+
+def test_class_interface_functions(running_class_plugin):
+    ifuncs = running_class_plugin.plugins["classtestplugin"]
+    assert ifuncs.module_function("a") == "module func return"
+    assert ifuncs.instance_method("a") == "instance method return"
+    assert ifuncs.class_method("a") == "class method return"
+    assert ifuncs.static_method("a") == "static method return"