Implement PluginManager and HelpPlugin (#326)

* Implement PluginManager interface

Co-authored-by: Alex Tzonkov <4975715+attzonko@users.noreply.github.com>

.github/workflows/lint.yml

@@ -35,7 +35,7 @@ jobs:
       - name: Run Flake8
         run: flake8
       - name: Black code style
-        run: black . --check --target-version py38 --exclude '\.mypy_cache/|\.venv/|env/|(.*/)*snapshots/|.pytype/'
+        run: black . --check --target-version py310 --exclude '\.mypy_cache/|\.venv/|env/|(.*/)*snapshots/|.pytype/'
       - name: Docstring formatting
         run: docformatter -c -r . --wrap-summaries 88 --wrap-descriptions 88
       - name: Check import order with isort

.pre-commit-config.yaml

@@ -22,7 +22,7 @@ repos:
           - "--filter-files"
   - repo: https://github.com/psf/black
     # Code style formatting
-    rev: 21.5b1
+    rev: 22.3.0
     hooks:
       - id: black
         exclude: (.*/)*snapshots/
@@ -31,6 +31,7 @@ repos:
     rev: 3.9.0
     hooks:
       - id: flake8
+        exclude: (.*/)*snapshots/
   - repo: https://github.com/mattseymour/pre-commit-pytype
     rev: '2020.10.8'
     hooks:

mmpy_bot/bot.py

@@ -1,11 +1,17 @@
 import asyncio
 import logging
 import sys
-from typing import Optional, Sequence
+from typing import List, Optional, Union
 
 from mmpy_bot.driver import Driver
 from mmpy_bot.event_handler import EventHandler
-from mmpy_bot.plugins import ExamplePlugin, Plugin, WebHookExample
+from mmpy_bot.plugins import (
+    ExamplePlugin,
+    HelpPlugin,
+    Plugin,
+    PluginManager,
+    WebHookExample,
+)
 from mmpy_bot.settings import Settings
 from mmpy_bot.webhook_server import WebHookServer
 
@@ -22,18 +28,18 @@ class Bot:
     def __init__(
         self,
         settings: Optional[Settings] = None,
-        plugins: Optional[Sequence[Plugin]] = None,
+        plugins: Optional[Union[List[Plugin], PluginManager]] = None,
         enable_logging: bool = True,
     ):
-        if plugins is None:
-            plugins = [ExamplePlugin(), WebHookExample()]
+        self._setup_plugin_manager(plugins)
+
         # Use default settings if none were specified.
         self.settings = settings or Settings()
 
+        self.console = None
+
         if enable_logging:
             self._register_logger()
-        else:
-            self.console = None
 
         self.driver = Driver(
             {
@@ -48,9 +54,9 @@ def __init__(
             }
         )
         self.driver.login()
-        self.plugins = self._initialize_plugins(plugins)
+        self.plugin_manager.initialize(self.driver, self.settings)
         self.event_handler = EventHandler(
-            self.driver, settings=self.settings, plugins=self.plugins
+            self.driver, settings=self.settings, plugin_manager=self.plugin_manager
         )
         self.webhook_server = None
 
@@ -59,6 +65,16 @@ def __init__(
 
         self.running = False
 
+    def _setup_plugin_manager(self, plugins):
+        if plugins is None:
+            self.plugin_manager = PluginManager(
+                [HelpPlugin(), ExamplePlugin(), WebHookExample()]
+            )
+        elif isinstance(plugins, PluginManager):
+            self.plugin_manager = plugins
+        else:
+            self.plugin_manager = PluginManager(plugins)
+
     def _register_logger(self):
         logging.basicConfig(
             **{
@@ -80,11 +96,6 @@ def _register_logger(self):
             )
             logging.getLogger("").addHandler(self.console)
 
-    def _initialize_plugins(self, plugins: Sequence[Plugin]):
-        for plugin in plugins:
-            plugin.initialize(self.driver, self.settings)
-        return plugins
-
     def _initialize_webhook_server(self):
         self.webhook_server = WebHookServer(
             url=self.settings.WEBHOOK_HOST_URL, port=self.settings.WEBHOOK_HOST_PORT
@@ -110,8 +121,8 @@ def run(self):
             if self.settings.WEBHOOK_HOST_ENABLED:
                 self.driver.threadpool.start_webhook_server_thread(self.webhook_server)
 
-            for plugin in self.plugins:
-                plugin.on_start()
+            # Trigger "start" methods on every plugin
+            self.plugin_manager.start()
 
             # Start listening for events
             self.event_handler.start()
@@ -129,9 +140,10 @@ def stop(self):
             return
 
         log.info("Stopping bot.")
+
         # Shutdown the running plugins
-        for plugin in self.plugins:
-            plugin.on_stop()
+        self.plugin_manager.stop()
+
         # Stop the threadpool
         self.driver.threadpool.stop()
         self.running = False

mmpy_bot/event_handler.py

@@ -3,11 +3,9 @@
 import logging
 import queue
 import re
-from collections import defaultdict
-from typing import Sequence
 
 from mmpy_bot.driver import Driver
-from mmpy_bot.plugins import Plugin
+from mmpy_bot.plugins import PluginManager
 from mmpy_bot.settings import Settings
 from mmpy_bot.webhook_server import NoResponse
 from mmpy_bot.wrappers import Message, WebHookEvent
@@ -20,27 +18,18 @@ def __init__(
         self,
         driver: Driver,
         settings: Settings,
-        plugins: Sequence[Plugin],
+        plugin_manager: PluginManager,
         ignore_own_messages=True,
     ):
         """The EventHandler class takes care of the connection to mattermost and calling
         the appropriate response function to each event."""
         self.driver = driver
         self.settings = settings
         self.ignore_own_messages = ignore_own_messages
-        self.plugins = plugins
+        self.plugin_manager = plugin_manager
 
         self._name_matcher = re.compile(rf"^@?{self.driver.username}\:?\s?")
 
-        # Collect the listeners from all plugins
-        self.message_listeners = defaultdict(list)
-        self.webhook_listeners = defaultdict(list)
-        for plugin in self.plugins:
-            for matcher, functions in plugin.message_listeners.items():
-                self.message_listeners[matcher].extend(functions)
-            for matcher, functions in plugin.webhook_listeners.items():
-                self.webhook_listeners[matcher].extend(functions)
-
     def start(self):
         # This is blocking, will loop forever
         self.driver.init_websocket(self._handle_event)
@@ -87,7 +76,7 @@ async def _handle_post(self, post):
         # Find all the listeners that match this message, and have their plugins handle
         # the rest.
         tasks = []
-        for matcher, functions in self.message_listeners.items():
+        for matcher, functions in self.plugin_manager.message_listeners.items():
             match = matcher.search(message.text)
             if match:
                 groups = list([group for group in match.groups() if group != ""])
@@ -107,7 +96,7 @@ async def _handle_webhook(self, event: WebHookEvent):
         # Find all the listeners that match this webhook id, and have their plugins
         # handle the rest.
         tasks = []
-        for matcher, functions in self.webhook_listeners.items():
+        for matcher, functions in self.plugin_manager.webhook_listeners.items():
             match = matcher.search(event.webhook_id)
             if match:
                 for function in functions:

mmpy_bot/function.py

@@ -5,21 +5,25 @@
 import logging
 import re
 from abc import ABC, abstractmethod
-from typing import Callable, Optional, Sequence, Union
+from typing import TYPE_CHECKING, Callable, Optional, Sequence, Union
 
 import click
 
-from mmpy_bot.utils import completed_future, spaces
+from mmpy_bot.utils import completed_future
 from mmpy_bot.webhook_server import NoResponse
 from mmpy_bot.wrappers import Message, WebHookEvent
 
+if TYPE_CHECKING:
+    from mmpy_bot.plugins import Plugin
+
+
 log = logging.getLogger("mmpy.function")
 
 
 class Function(ABC):
     def __init__(
         self,
-        function: Union[Function, click.command],
+        function: Union[Function, click.Command],
         matcher: re.Pattern,
         **metadata,
     ):
@@ -32,7 +36,11 @@ def __init__(
             self.siblings.append(function)
             function = function.function
 
-        # FIXME: After this while loop it is possible that function is not a Function, do we really want to assign self.function to something which is not a Function? Check if this is needed for the click.Command case
+        if function is None:
+            raise ValueError(
+                "ERROR: Possible bug, inside the Function class function should not end up being None"
+            )
+
         self.function = function
         self.is_coroutine = asyncio.iscoroutinefunction(function)
         self.is_click_function: bool = False
@@ -46,21 +54,14 @@ def __init__(
             self.function.invoke = None
 
         # To be set in the child class or from the parent plugin
-        self.plugin = None
+        self.plugin: Optional[Plugin] = None
         self.name: Optional[str] = None
-        self.docstring: Optional[str] = None
+        self.docstring = self.function.__doc__ or ""
 
         @abstractmethod
         def __call__(self, *args):
             pass
 
-    def get_help_string(self):
-        string = f"`{self.matcher.pattern}`:\n"
-        # Add a docstring
-        doc = self.docstring or "No description provided."
-        string += f"{spaces(8)}{doc}\n"
-        return string
-
 
 class MessageFunction(Function):
     """Wrapper around a Plugin class method that should respond to certain Mattermost
@@ -95,7 +96,6 @@ def __init__(
 
         # Default for non-click functions
         _function: Union[Callable, click.Command] = self.function
-        self.docstring = self.function.__doc__
 
         if self.is_click_function:
             _function = self.function.callback
@@ -109,9 +109,8 @@ def __init__(
                 info_name=self.matcher.pattern.strip("^").split(" (.*)?")[0],
             ) as ctx:
                 # Get click help string and do some extra formatting
-                self.docstring = self.function.get_help(ctx).replace(
-                    "\n", f"\n{spaces(8)}"
-                )
+                self.docstring += f"\n\n{self.function.get_help(ctx)}"
+
         if _function is not None:
             self.name = _function.__qualname__
 
@@ -169,38 +168,6 @@ def __call__(self, message: Message, *args):
 
         return self.function(self.plugin, message, *args)
 
-    def get_help_string(self):
-        string = super().get_help_string()
-        if any(
-            [
-                self.needs_mention,
-                self.direct_only,
-                self.allowed_users,
-                self.allowed_channels,
-                self.silence_fail_msg,
-            ]
-        ):
-            # Print some information describing the usage settings.
-            string += f"{spaces(4)}Additional information:\n"
-            if self.needs_mention:
-                string += (
-                    f"{spaces(4)}- Needs to either mention @{self.plugin.driver.username}"
-                    " or be a direct message.\n"
-                )
-            if self.direct_only:
-                string += f"{spaces(4)}- Needs to be a direct message.\n"
-
-            if self.allowed_users:
-                string += f"{spaces(4)}- Restricted to certain users.\n"
-
-            if self.allowed_channels:
-                string += f"{spaces(4)}- Restricted to certain channels.\n"
-
-            if self.silence_fail_msg:
-                string += f"{spaces(4)}- If it should reply to a non privileged user / in a non privileged channel.\n"
-
-        return string
-
 
 def listen_to(
     regexp: str,
@@ -238,7 +205,7 @@ def wrapped_func(func):
             reg = f"^{reg.strip('^')} (.*)?"  # noqa
 
         pattern = re.compile(reg, regexp_flag)
-        return MessageFunction(
+        new_func = MessageFunction(
             func,
             matcher=pattern,
             direct_only=direct_only,
@@ -249,6 +216,10 @@ def wrapped_func(func):
             **metadata,
         )
 
+        # Preserve docstring
+        new_func.__doc__ = func.__doc__
+        return new_func
+
     return wrapped_func
 
 
@@ -267,7 +238,6 @@ def __init__(
             )
 
         self.name = self.function.__qualname__
-        self.docstring = self.function.__doc__
 
         argspec = list(inspect.signature(self.function).parameters.keys())
         if not argspec == ["self", "event"]:
@@ -305,10 +275,14 @@ def listen_webhook(
 
     def wrapped_func(func):
         pattern = re.compile(regexp)
-        return WebHookFunction(
+        new_func = WebHookFunction(
             func,
             matcher=pattern,
             **metadata,
         )
 
+        # Preserve docstring
+        new_func.__doc__ = func.__doc__
+        return new_func
+
     return wrapped_func

mmpy_bot/plugins/__init__.py

@@ -1,5 +1,6 @@
-from mmpy_bot.plugins.base import Plugin
+from mmpy_bot.plugins.base import Plugin, PluginManager
 from mmpy_bot.plugins.example import ExamplePlugin
+from mmpy_bot.plugins.help_example import HelpPlugin
 from mmpy_bot.plugins.webhook_example import WebHookExample
 
-__all__ = ["Plugin", "ExamplePlugin", "WebHookExample"]
+__all__ = ["Plugin", "PluginManager", "HelpPlugin", "ExamplePlugin", "WebHookExample"]