From 0c319aab1a1cc59aecad692b5e3d901d1cf496d6 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <rpm@fthiessen.de>
Date: Thu, 23 Dec 2021 02:45:51 +0100
Subject: [PATCH] feat(plugins): Identify plugins by id, migrations must be
 provided at defined location, add utils for plugin functions

---
 flaschengeist/__init__.py                    |  2 -
 flaschengeist/app.py                         | 89 +++++++-------------
 flaschengeist/config.py                      |  6 +-
 flaschengeist/database.py                    |  9 +-
 flaschengeist/plugins/__init__.py            | 50 ++++++++---
 flaschengeist/plugins/auth/__init__.py       |  4 +-
 flaschengeist/plugins/auth_ldap/__init__.py  |  2 +
 flaschengeist/plugins/auth_plain/__init__.py |  2 +
 flaschengeist/plugins/balance/__init__.py    |  5 +-
 flaschengeist/plugins/message_mail.py        |  2 +
 flaschengeist/plugins/pricelist/__init__.py  |  4 +-
 flaschengeist/plugins/roles/__init__.py      |  4 +-
 flaschengeist/plugins/users/__init__.py      |  4 +-
 flaschengeist/utils/plugin.py                | 46 ++++++++++
 14 files changed, 141 insertions(+), 88 deletions(-)
 create mode 100644 flaschengeist/utils/plugin.py

diff --git a/flaschengeist/__init__.py b/flaschengeist/__init__.py
index 8a5e19e..5f1a6a2 100644
--- a/flaschengeist/__init__.py
+++ b/flaschengeist/__init__.py
@@ -1,11 +1,9 @@
 """Flaschengeist"""
 import logging
 import pkg_resources
-from pathlib import Path
 from werkzeug.local import LocalProxy
 
 __version__ = pkg_resources.get_distribution("flaschengeist").version
-_module_path = Path(__file__).parent
 __pdoc__ = {}
 
 logger: logging.Logger = LocalProxy(lambda: logging.getLogger(__name__))
diff --git a/flaschengeist/app.py b/flaschengeist/app.py
index 0786745..75d4181 100644
--- a/flaschengeist/app.py
+++ b/flaschengeist/app.py
@@ -1,18 +1,18 @@
 import enum
 
-import pkg_resources
-from flask import Flask, current_app
+from flask import Flask
 from flask_cors import CORS
 from datetime import datetime, date
 from flask.json import JSONEncoder, jsonify
 from sqlalchemy.exc import OperationalError
 from werkzeug.exceptions import HTTPException
 
+from flaschengeist.utils.plugin import get_plugins
+
 from . import logger
-from .plugins import AuthPlugin
-from flaschengeist.config import config, configure_app
-from flaschengeist.controller import roleController
-from flaschengeist.utils.hook import Hook
+from .plugins import Plugin
+from .config import config, configure_app
+from .utils.hook import Hook
 
 
 class CustomJSONEncoder(JSONEncoder):
@@ -37,64 +37,35 @@ class CustomJSONEncoder(JSONEncoder):
 
 
 @Hook("plugins.loaded")
-def __load_plugins(app):
-    logger.debug("Search for plugins")
+def load_plugins(app: Flask):
+    def load_plugin(cls: type[Plugin]):
+        logger.debug(f"Load plugin {cls.id}")
+        # Initialize plugin with config section
+        plugin = cls(config.get(plugin_class.id, config.get(plugin_class.id.split(".")[-1], {})))
+        # Register blueprint if provided
+        if plugin.blueprint is not None:
+            app.register_blueprint(plugin.blueprint)
+        # Save plugin application context
+        app.config.setdefault("FG_PLUGINS", {})[plugin.id] = plugin
+        return plugin
 
-    app.config["FG_PLUGINS"] = {}
-    for entry_point in pkg_resources.iter_entry_points("flaschengeist.plugins"):
-        logger.debug(f"Found plugin: >{entry_point.name}<")
-
-        if entry_point.name == config["FLASCHENGEIST"]["auth"] or (
-            entry_point.name in config and config[entry_point.name].get("enabled", False)
-        ):
-            logger.debug(f"Load plugin {entry_point.name}")
-            try:
-                plugin = entry_point.load()
-                if not hasattr(plugin, "name"):
-                    setattr(plugin, "name", entry_point.name)
-                plugin = plugin(config.get(entry_point.name, {}))
-                if hasattr(plugin, "blueprint") and plugin.blueprint is not None:
-                    app.register_blueprint(plugin.blueprint)
-            except:
-                logger.error(
-                    f"Plugin {entry_point.name} was enabled, but could not be loaded due to an error.",
-                    exc_info=True,
-                )
-                continue
-            if isinstance(plugin, AuthPlugin):
-                if entry_point.name != config["FLASCHENGEIST"]["auth"]:
-                    logger.debug(f"Unload not configured AuthPlugin {entry_point.name}")
-                    del plugin
-                    continue
-                else:
-                    logger.info(f"Using authentication plugin: {entry_point.name}")
-                    app.config["FG_AUTH_BACKEND"] = plugin
-            else:
-                logger.info(f"Using plugin: {entry_point.name}")
-            app.config["FG_PLUGINS"][entry_point.name] = plugin
+    for plugin_class in get_plugins():
+        names = [plugin_class.id, plugin_class.id.split(".")[-1]]
+        if config["FLASCHENGEIST"]["auth"] in names:
+            # Load authentification plugin
+            app.config["FG_AUTH_BACKEND"] = load_plugin(plugin_class)
+            logger.info(f"Using authentication plugin: {plugin_class.id}")
+        elif any([i in config and config[i].get("enabled", False) for i in names]):
+            # Load all other enabled plugins
+            load_plugin(plugin_class)
+            logger.info(f"Using plugin: {plugin_class.id}")
         else:
-            logger.debug(f"Skip disabled plugin {entry_point.name}")
+            logger.debug(f"Skip disabled plugin {plugin_class.id}")
     if "FG_AUTH_BACKEND" not in app.config:
-        logger.error("No authentication plugin configured or authentication plugin not found")
+        logger.fatal("No authentication plugin configured or authentication plugin not found")
         raise RuntimeError("No authentication plugin configured or authentication plugin not found")
 
 
-@Hook("plugins.installed")
-def install_all():
-    from flaschengeist.database import db
-
-    db.create_all()
-    db.session.commit()
-    for name, plugin in current_app.config["FG_PLUGINS"].items():
-        if not plugin:
-            logger.debug(f"Skip disabled plugin: {name}")
-            continue
-        logger.info(f"Install plugin {name}")
-        plugin.install()
-        if plugin.permissions:
-            roleController.create_permissions(plugin.permissions)
-
-
 def create_app(test_config=None, cli=False):
     app = Flask(__name__)
     app.json_encoder = CustomJSONEncoder
@@ -106,7 +77,7 @@ def create_app(test_config=None, cli=False):
         configure_app(app, test_config, cli)
         db.init_app(app)
         migrate.init_app(app, db, compare_type=True)
-        __load_plugins(app)
+        load_plugins(app)
 
     @app.route("/", methods=["GET"])
     def __get_state():
diff --git a/flaschengeist/config.py b/flaschengeist/config.py
index 5b7031c..33f6f9d 100644
--- a/flaschengeist/config.py
+++ b/flaschengeist/config.py
@@ -5,7 +5,7 @@ import collections.abc
 
 from pathlib import Path
 from werkzeug.middleware.proxy_fix import ProxyFix
-from flaschengeist import _module_path, logger
+from flaschengeist import logger
 
 
 # Default config:
@@ -23,7 +23,7 @@ def update_dict(d, u):
 
 def read_configuration(test_config):
     global config
-    paths = [_module_path]
+    paths = [Path(__file__).parent]
 
     if not test_config:
         paths.append(Path.home() / ".config")
@@ -44,7 +44,7 @@ def read_configuration(test_config):
 def configure_logger(cli=False):
     global config
     # Read default config
-    logger_config = toml.load(_module_path / "logging.toml")
+    logger_config = toml.load(Path(__file__).parent / "logging.toml")
 
     if "LOGGING" in config:
         # Override with user config
diff --git a/flaschengeist/database.py b/flaschengeist/database.py
index 560f7ed..ccafd8d 100644
--- a/flaschengeist/database.py
+++ b/flaschengeist/database.py
@@ -24,9 +24,16 @@ migrate = Migrate()
 def configure_alembic(config):
     """Alembic configuration hook
 
+    Inject all migrations paths into the ``version_locations`` config option.
+    This includes even disabled plugins, as simply disabling a plugin without
+    uninstall can break the alembic version management.
     """
+    import inspect, pathlib
+    from flaschengeist.utils.plugin import get_plugins
+
     # Load migration paths from plugins
-    migrations = [str(p.migrations_path) for p in current_app.config["FG_PLUGINS"].values() if p and p.migrations_path]
+    migrations = [(pathlib.Path(inspect.getfile(p)).parent / "migrations") for p in get_plugins()]
+    migrations = [str(m.resolve()) for m in migrations if m.exists()]
     if len(migrations) > 0:
         # Get configured paths
         paths = config.get_main_option("version_locations")
diff --git a/flaschengeist/plugins/__init__.py b/flaschengeist/plugins/__init__.py
index d993879..ac505de 100644
--- a/flaschengeist/plugins/__init__.py
+++ b/flaschengeist/plugins/__init__.py
@@ -25,15 +25,7 @@ For more information, please refer to
 """
 
 import sqlalchemy
-import pkg_resources
-from werkzeug.datastructures import FileStorage
 from werkzeug.exceptions import MethodNotAllowed, NotFound
-from flaschengeist.controller import imageController
-
-from flaschengeist.database import db
-from flaschengeist.models.notification import Notification
-from flaschengeist.models.user import _Avatar, User
-from flaschengeist.models.setting import _PluginSetting
 from flaschengeist.utils.hook import HookBefore, HookAfter
 
 __all__ = [
@@ -112,9 +104,16 @@ class Plugin:
         - *version*: Version of your plugin, can also be guessed by Flaschengeist
     """
 
-    blueprint = None  # You have to override
+    id: str = None
+    """Override with the unique ID of the plugin
+    
+    Hint: Use a fully qualified name like "dev.flaschengeist.plugin"
+    """
+
+    blueprint = None
     """Override with a `flask.blueprint` if the plugin uses custom routes"""
-    permissions = []  # You have to override
+
+    permissions: list[str] = []  # You have to override
     """Override to add custom permissions used by the plugin
     
     A good style is to name the permissions with a prefix related to the plugin name,
@@ -138,7 +137,6 @@ class Plugin:
         Args:
             config: Dict configuration containing the plugin section
         """
-        self.version = pkg_resources.get_distribution(self.__module__.split(".")[0]).version
 
     def install(self):
         """Installation routine
@@ -147,6 +145,17 @@ class Plugin:
         """
         pass
 
+    def uninstall(self):
+        """Uninstall routine
+
+        If the plugin has custom database tables, make sure to remove them.
+        This can be either done by downgrading the plugin *head* to the *base*.
+        Or use custom migrations for the uninstall and *stamp* some version.
+
+        Is always called with Flask application context.
+        """
+        pass
+
     def get_setting(self, name: str, **kwargs):
         """Get plugin setting from database
 
@@ -158,6 +167,8 @@ class Plugin:
         Raises:
             `KeyError` if no such setting exists in the database
         """
+        from flaschengeist.models.setting import _PluginSetting
+
         try:
             setting = (
                 _PluginSetting.query.filter(_PluginSetting.plugin == self.name)
@@ -178,6 +189,9 @@ class Plugin:
             name: String identifying the setting
             value: Value to be stored
         """
+        from flaschengeist.models.setting import _PluginSetting
+        from flaschengeist.database import db
+
         setting = (
             _PluginSetting.query.filter(_PluginSetting.plugin == self.name)
             .filter(_PluginSetting.name == name)
@@ -204,6 +218,9 @@ class Plugin:
 
         Hint: use the data for frontend actions.
         """
+        from flaschengeist.models.notification import Notification
+        from flaschengeist.database import db
+
         if not user.deleted:
             n = Notification(text=text, data=data, plugin=self.id, user_=user)
             db.session.add(n)
@@ -220,6 +237,11 @@ class Plugin:
 
 
 class AuthPlugin(Plugin):
+    """Base class for all authentification plugins
+
+    See also `Plugin`
+    """
+
     def login(self, user, pw):
         """Login routine, MUST BE IMPLEMENTED!
 
@@ -282,7 +304,7 @@ class AuthPlugin(Plugin):
         """
         raise MethodNotAllowed
 
-    def get_avatar(self, user: User) -> _Avatar:
+    def get_avatar(self, user):
         """Retrieve avatar for given user (if supported by auth backend)
 
         Default behavior is to use native Image objects,
@@ -308,9 +330,11 @@ class AuthPlugin(Plugin):
             MethodNotAllowed: If not supported by Backend
             Any valid HTTP exception
         """
+        from flaschengeist.controller import imageController
+
         user.avatar_ = imageController.upload_image(file)
 
-    def delete_avatar(self, user: User):
+    def delete_avatar(self, user):
         """Delete the avatar for given user (if supported by auth backend)
 
         Default behavior is to use the imageController and native Image objects.
diff --git a/flaschengeist/plugins/auth/__init__.py b/flaschengeist/plugins/auth/__init__.py
index 9c08463..66d77be 100644
--- a/flaschengeist/plugins/auth/__init__.py
+++ b/flaschengeist/plugins/auth/__init__.py
@@ -13,8 +13,8 @@ from flaschengeist.controller import sessionController, userController
 
 
 class AuthRoutePlugin(Plugin):
-    name = "auth"
-    blueprint = Blueprint(name, __name__)
+    id = "dev.flaschengeist.auth"
+    blueprint = Blueprint("auth", __name__)
 
 
 @AuthRoutePlugin.blueprint.route("/auth", methods=["POST"])
diff --git a/flaschengeist/plugins/auth_ldap/__init__.py b/flaschengeist/plugins/auth_ldap/__init__.py
index 697e972..20fb46f 100644
--- a/flaschengeist/plugins/auth_ldap/__init__.py
+++ b/flaschengeist/plugins/auth_ldap/__init__.py
@@ -17,6 +17,8 @@ from flaschengeist.plugins import AuthPlugin, before_role_updated
 
 
 class AuthLDAP(AuthPlugin):
+    id = "auth_ldap"
+
     def __init__(self, config):
         super().__init__()
         app.config.update(
diff --git a/flaschengeist/plugins/auth_plain/__init__.py b/flaschengeist/plugins/auth_plain/__init__.py
index 10d72d3..e73b98c 100644
--- a/flaschengeist/plugins/auth_plain/__init__.py
+++ b/flaschengeist/plugins/auth_plain/__init__.py
@@ -14,6 +14,8 @@ from flaschengeist import logger
 
 
 class AuthPlain(AuthPlugin):
+    id = "auth_plain"
+
     def install(self):
         plugins_installed(self.post_install)
 
diff --git a/flaschengeist/plugins/balance/__init__.py b/flaschengeist/plugins/balance/__init__.py
index 73841f7..ea08857 100644
--- a/flaschengeist/plugins/balance/__init__.py
+++ b/flaschengeist/plugins/balance/__init__.py
@@ -57,9 +57,10 @@ def service_debit():
 
 
 class BalancePlugin(Plugin):
-    name = "balance"
+    """Balance Plugin"""
+
     id = "dev.flaschengeist.balance"
-    blueprint = Blueprint(name, __name__)
+    blueprint = Blueprint("balance", __name__)
     permissions = permissions.permissions
     plugin: "BalancePlugin" = LocalProxy(lambda: current_app.config["FG_PLUGINS"][BalancePlugin.name])
     models = models
diff --git a/flaschengeist/plugins/message_mail.py b/flaschengeist/plugins/message_mail.py
index 79b6a64..d0cebdb 100644
--- a/flaschengeist/plugins/message_mail.py
+++ b/flaschengeist/plugins/message_mail.py
@@ -12,6 +12,8 @@ from . import Plugin
 
 
 class MailMessagePlugin(Plugin):
+    id = "dev.flaschengeist.mail_plugin"
+
     def __init__(self, config):
         super().__init__()
         self.server = config["SERVER"]
diff --git a/flaschengeist/plugins/pricelist/__init__.py b/flaschengeist/plugins/pricelist/__init__.py
index dac2f82..70fe1f7 100644
--- a/flaschengeist/plugins/pricelist/__init__.py
+++ b/flaschengeist/plugins/pricelist/__init__.py
@@ -17,9 +17,9 @@ from . import pricelist_controller, permissions
 
 
 class PriceListPlugin(Plugin):
-    name = "pricelist"
+    id = "dev.flaschengeist.pricelist"
     permissions = permissions.permissions
-    blueprint = Blueprint(name, __name__, url_prefix="/pricelist")
+    blueprint = Blueprint("pricelist", __name__, url_prefix="/pricelist")
     plugin = LocalProxy(lambda: current_app.config["FG_PLUGINS"][PriceListPlugin.name])
     models = models
 
diff --git a/flaschengeist/plugins/roles/__init__.py b/flaschengeist/plugins/roles/__init__.py
index 954ba21..3a48ba1 100644
--- a/flaschengeist/plugins/roles/__init__.py
+++ b/flaschengeist/plugins/roles/__init__.py
@@ -16,8 +16,8 @@ from . import permissions
 
 
 class RolesPlugin(Plugin):
-    name = "roles"
-    blueprint = Blueprint(name, __name__)
+    id = "dev.flaschengeist.roles"
+    blueprint = Blueprint("roles", __name__)
     permissions = permissions.permissions
 
 
diff --git a/flaschengeist/plugins/users/__init__.py b/flaschengeist/plugins/users/__init__.py
index b485f97..e03da8a 100644
--- a/flaschengeist/plugins/users/__init__.py
+++ b/flaschengeist/plugins/users/__init__.py
@@ -18,8 +18,8 @@ from flaschengeist.utils.datetime import from_iso_format
 
 
 class UsersPlugin(Plugin):
-    name = "users"
-    blueprint = Blueprint(name, __name__)
+    id = "dev.flaschengeist.users"
+    blueprint = Blueprint("users", __name__)
     permissions = permissions.permissions
 
 
diff --git a/flaschengeist/utils/plugin.py b/flaschengeist/utils/plugin.py
new file mode 100644
index 0000000..b37e582
--- /dev/null
+++ b/flaschengeist/utils/plugin.py
@@ -0,0 +1,46 @@
+"""Plugin utils
+
+Utilities for handling Flaschengeist plugins
+"""
+
+import pkg_resources
+from flaschengeist import logger
+from flaschengeist.plugins import Plugin
+
+
+def get_plugins() -> list[type[Plugin]]:
+    """Get all installed plugins for Flaschengeist
+
+    Returns:
+        list of classes implementing `flaschengeist.plugins.Plugin`
+    """
+    logger.debug("Search for plugins")
+
+    plugins = []
+    for entry_point in pkg_resources.iter_entry_points("flaschengeist.plugins"):
+        try:
+            logger.debug(f"Found plugin: >{entry_point.name}<")
+            plugin_class = entry_point.load()
+            if issubclass(plugin_class, Plugin):
+                plugins.append(plugin_class)
+        except TypeError:
+            logger.error(f"Invalid entry point for plugin {entry_point.name} found.")
+    return plugins
+
+
+def plugin_version(plugin: type[Plugin]) -> str:
+    """Get version of plugin
+
+    Returns the version of a plugin, if plugin does not set the
+    version property, the version of the package providing the
+    plugin is taken.
+
+    Args:
+        plugin: Plugin or Plugin class
+
+    Returns:
+        Version as string
+    """
+    if plugin.version:
+        return plugin.version
+    return pkg_resources.get_distribution(plugin.__module__.split(".", 1)[0]).version