Plugin framework updates (#3535)

* refactor entrypoint into helpers

* Add lookup by metadata
This is geared towards plugins packaged in pkgs that differ in name from their top module

* Make module lookup predictable in changing pkg-envs

* remove no coverage from plugin packages

* ignore coverage for production loadin

* refactor plugin collection - move assigment out

* do not cover fs errors

* test custom dir loading

* test module meta fetcher

* add a bit more safety

* do not cover sanity checkers

* add folder loading test

* ignore again for cleaner diffs for now

* ignore safety catch

* rename test

* Add test for package installs

* fix docstring name

* depreciate test for now

* Fix for out of BASE_DIR paths

* ignore catch

* remove unneeded complexity

* add testing for outside folders

* more docstrings and simpler methods

* make call simpler

* refactor import

* Add registry with all plugins

* use full registry and make simpler request

* switch path properties to methods

* Add typing to plugin

* Add a checker fnc for is_sample

* Add sample check to admin page

* Make file check a cls

* more cls methods

* Add setting for signature cheks
Fixes #3520

* make property statements simpler

* use same key in all dicts

* Use module name instead of NAME
Fixes #3534

* fix naming

* fix name

* add version checking
Fixes #3478

* fix formatting and typing

* also save reference to full array

* do not cover as we turn on all plugins

* add test for check_version

* Add version e2e test

* make test save

* refactor out assignment

* safe a db reference first

* docstring

* condense code a bit

* rename

* append logging

* rename

* also safe db reference to new object

* docstrings, refactors, typing

* fix key lookup

InvenTree/common/models.py

@@ -1268,6 +1268,13 @@ class InvenTreeSetting(BaseInvenTreeSetting):
             'requires_restart': True,
         },
 
+        'PLUGIN_CHECK_SIGNATURES': {
+            'name': _('Check plugin signatures'),
+            'description': _('Check and show signatures for plugins'),
+            'default': False,
+            'validator': bool,
+        },
+
         # Settings for plugin mixin features
         'ENABLE_PLUGINS_URL': {
             'name': _('Enable URL integration'),

InvenTree/plugin/admin.py

@@ -52,7 +52,7 @@ class PluginConfigAdmin(admin.ModelAdmin):
     """Custom admin with restricted id fields."""
 
     readonly_fields = ["key", "name", ]
-    list_display = ['name', 'key', '__str__', 'active', ]
+    list_display = ['name', 'key', '__str__', 'active', 'is_sample']
     list_filter = ['active']
     actions = [plugin_activate, plugin_deactivate, ]
     inlines = [PluginSettingInline, ]

InvenTree/plugin/models.py

@@ -3,6 +3,7 @@
 import warnings
 
 from django.conf import settings
+from django.contrib import admin
 from django.contrib.auth.models import User
 from django.db import models
 from django.utils.translation import gettext_lazy as _
@@ -123,11 +124,11 @@ class PluginConfig(models.Model):
         self.__org_active = self.active
 
         # append settings from registry
-        self.plugin = registry.plugins.get(self.key, None)
+        plugin = registry.plugins_full.get(self.key, None)
 
         def get_plugin_meta(name):
-            if self.plugin:
+            if plugin:
-                return str(getattr(self.plugin, name, None))
+                return str(getattr(plugin, name, None))
             return None
 
         self.meta = {
@@ -136,6 +137,9 @@ class PluginConfig(models.Model):
                                                   'package_path', 'settings_url', ]
         }
 
+        # Save plugin
+        self.plugin: InvenTreePlugin = plugin
+
     def save(self, force_insert=False, force_update=False, *args, **kwargs):
         """Extend save method to reload plugins if the 'active' status changes."""
         reload = kwargs.pop('no_reload', False)  # check if no_reload flag is set
@@ -151,6 +155,20 @@ class PluginConfig(models.Model):
 
         return ret
 
+    @admin.display(boolean=True, description=_('Sample plugin'))
+    def is_sample(self) -> bool:
+        """Is this plugin a sample app?"""
+        # Loaded and active plugin
+        if isinstance(self.plugin, InvenTreePlugin):
+            return self.plugin.check_is_sample()
+
+        # If no plugin_class is available it can not be a sample
+        if not self.plugin:
+            return False
+
+        # Not loaded plugin
+        return self.plugin.check_is_sample()  # pragma: no cover
+
 
 class PluginSetting(common.models.BaseInvenTreeSetting):
     """This model represents settings for individual plugins."""

InvenTree/plugin/plugin.py

@@ -2,11 +2,10 @@
 
 import inspect
 import logging
-import os
-import pathlib
 import warnings
 from datetime import datetime
 from importlib.metadata import PackageNotFoundError, metadata
+from pathlib import Path
 
 from django.conf import settings
 from django.db.utils import OperationalError, ProgrammingError
@@ -171,7 +170,24 @@ class MixinBase:
         return mixins
 
 
-class InvenTreePlugin(MixinBase, MetaBase):
+class VersionMixin:
+    """Mixin to enable version checking."""
+
+    MIN_VERSION = None
+    MAX_VERSION = None
+
+    def check_version(self, latest=None) -> bool:
+        """Check if plugin functions for the current InvenTree version."""
+        from InvenTree import version
+
+        latest = latest if latest else version.inventreeVersionTuple()
+        min_v = version.inventreeVersionTuple(self.MIN_VERSION)
+        max_v = version.inventreeVersionTuple(self.MAX_VERSION)
+
+        return bool(min_v <= latest <= max_v)
+
+
+class InvenTreePlugin(VersionMixin, MixinBase, MetaBase):
     """The InvenTreePlugin class is used to integrate with 3rd party software.
 
     DO NOT USE THIS DIRECTLY, USE plugin.InvenTreePlugin
@@ -191,11 +207,18 @@ class InvenTreePlugin(MixinBase, MetaBase):
         """
         super().__init__()
         self.add_mixin('base')
-        self.def_path = inspect.getfile(self.__class__)
-        self.path = os.path.dirname(self.def_path)
 
         self.define_package()
 
+    @classmethod
+    def file(cls) -> Path:
+        """File that contains plugin definition."""
+        return Path(inspect.getfile(cls))
+
+    def path(self) -> Path:
+        """Path to plugins base folder."""
+        return self.file().parent
+
     def _get_value(self, meta_name: str, package_name: str) -> str:
         """Extract values from class meta or package info.
 
@@ -243,43 +266,54 @@ class InvenTreePlugin(MixinBase, MetaBase):
     @property
     def version(self):
         """Version of plugin."""
-        version = self._get_value('VERSION', 'version')
+        return self._get_value('VERSION', 'version')
-        return version
 
     @property
     def website(self):
         """Website of plugin - if set else None."""
-        website = self._get_value('WEBSITE', 'website')
+        return self._get_value('WEBSITE', 'website')
-        return website
 
     @property
     def license(self):
         """License of plugin."""
-        lic = self._get_value('LICENSE', 'license')
+        return self._get_value('LICENSE', 'license')
-        return lic
     # endregion
 
+    @classmethod
+    def check_is_package(cls):
+        """Is the plugin delivered as a package."""
+        return getattr(cls, 'is_package', False)
+
     @property
     def _is_package(self):
         """Is the plugin delivered as a package."""
         return getattr(self, 'is_package', False)
 
-    @property
+    @classmethod
-    def is_sample(self):
+    def check_is_sample(cls) -> bool:
         """Is this plugin part of the samples?"""
-        path = str(self.package_path)
+        return str(cls.check_package_path()).startswith('plugin/samples/')
-        return path.startswith('plugin/samples/')
+
+    @property
+    def is_sample(self) -> bool:
+        """Is this plugin part of the samples?"""
+        return self.check_is_sample()
+
+    @classmethod
+    def check_package_path(cls):
+        """Path to the plugin."""
+        if cls.check_is_package():
+            return cls.__module__  # pragma: no cover
+
+        try:
+            return cls.file().relative_to(settings.BASE_DIR)
+        except ValueError:
+            return cls.file()
 
     @property
     def package_path(self):
         """Path to the plugin."""
-        if self._is_package:
+        return self.check_package_path()
-            return self.__module__  # pragma: no cover
-
-        try:
-            return pathlib.Path(self.def_path).relative_to(settings.BASE_DIR)
-        except ValueError:
-            return pathlib.Path(self.def_path)
 
     @property
     def settings_url(self):
@@ -289,7 +323,7 @@ class InvenTreePlugin(MixinBase, MetaBase):
     # region package info
     def _get_package_commit(self):
         """Get last git commit for the plugin."""
-        return get_git_log(self.def_path)
+        return get_git_log(str(self.file()))
 
     @classmethod
     def _get_package_metadata(cls):

InvenTree/plugin/registry.py

@@ -11,7 +11,7 @@ import os
 import subprocess
 from importlib import reload
 from pathlib import Path
-from typing import OrderedDict
+from typing import Dict, List, OrderedDict
 
 from django.apps import apps
 from django.conf import settings
@@ -41,19 +41,20 @@ class PluginsRegistry:
         Set up all needed references for internal and external states.
         """
         # plugin registry
-        self.plugins = {}
+        self.plugins: Dict[str, InvenTreePlugin] = {}           # List of active instances
-        self.plugins_inactive = {}
+        self.plugins_inactive: Dict[str, InvenTreePlugin] = {}  # List of inactive instances
+        self.plugins_full: Dict[str, InvenTreePlugin] = {}      # List of all plugin instances
 
-        self.plugin_modules = []         # Holds all discovered plugins
+        self.plugin_modules: List(InvenTreePlugin) = []         # Holds all discovered plugins
 
-        self.errors = {}                 # Holds discovering errors
+        self.errors = {}                                        # Holds discovering errors
 
         # flags
-        self.is_loading = False
+        self.is_loading = False                                 # Are plugins beeing loaded right now
-        self.apps_loading = True        # Marks if apps were reloaded yet
+        self.apps_loading = True                                # Marks if apps were reloaded yet
-        self.git_is_modern = True       # Is a modern version of git available
+        self.git_is_modern = True                               # Is a modern version of git available
 
-        self.installed_apps = []         # Holds all added plugin_paths
+        self.installed_apps = []                                # Holds all added plugin_paths
 
         # mixins
         self.mixins_settings = {}
@@ -339,74 +340,77 @@ class PluginsRegistry:
     # endregion
 
     # region general internal loading /activating / deactivating / deloading
-    def _init_plugins(self, disabled=None):
+    def _init_plugins(self, disabled: str = None):
         """Initialise all found plugins.
 
-        :param disabled: loading path of disabled app, defaults to None
+        Args:
-        :type disabled: str, optional
+            disabled (str, optional): Loading path of disabled app. Defaults to None.
-        :raises error: IntegrationPluginError
+
+        Raises:
+            error: IntegrationPluginError
         """
         from plugin.models import PluginConfig
 
+        def safe_reference(plugin, key: str, active: bool = True):
+            """Safe reference to plugin dicts."""
+            if active:
+                self.plugins[key] = plugin
+            else:
+                # Deactivate plugin in db
+                if not settings.PLUGIN_TESTING:  # pragma: no cover
+                    plugin.db.active = False
+                    plugin.db.save(no_reload=True)
+                self.plugins_inactive[key] = plugin.db
+            self.plugins_full[key] = plugin
+
         logger.info('Starting plugin initialisation')
 
         # Initialize plugins
-        for plugin in self.plugin_modules:
+        for plg in self.plugin_modules:
-            # Check if package
-            was_packaged = getattr(plugin, 'is_package', False)
-
-            # Check if activated
             # These checks only use attributes - never use plugin supplied functions -> that would lead to arbitrary code execution!!
-            plug_name = plugin.NAME
+            plg_name = plg.NAME
-            plug_key = plugin.SLUG if getattr(plugin, 'SLUG', None) else plug_name
+            plg_key = slugify(plg.SLUG if getattr(plg, 'SLUG', None) else plg_name)  # keys are slugs!
-            plug_key = slugify(plug_key)  # keys are slugs!
+
             try:
-                plugin_db_setting, _ = PluginConfig.objects.get_or_create(key=plug_key, name=plug_name)
+                plg_db, _ = PluginConfig.objects.get_or_create(key=plg_key, name=plg_name)
             except (OperationalError, ProgrammingError) as error:
                 # Exception if the database has not been migrated yet - check if test are running - raise if not
                 if not settings.PLUGIN_TESTING:
                     raise error  # pragma: no cover
-                plugin_db_setting = None
+                plg_db = None
             except (IntegrityError) as error:  # pragma: no cover
-                logger.error(f"Error initializing plugin: {error}")
+                logger.error(f"Error initializing plugin `{plg_name}`: {error}")
+
+            # Append reference to plugin
+            plg.db = plg_db
 
             # Always activate if testing
-            if settings.PLUGIN_TESTING or (plugin_db_setting and plugin_db_setting.active):
+            if settings.PLUGIN_TESTING or (plg_db and plg_db.active):
-                # Check if the plugin was blocked -> threw an error
+                # Check if the plugin was blocked -> threw an error; option1: package, option2: file-based
-                if disabled:
+                if disabled and ((plg.__name__ == disabled) or (plg.__module__ == disabled)):
-                    # option1: package, option2: file-based
+                    safe_reference(plugin=plg, key=plg_key, active=False)
-                    if (plugin.__name__ == disabled) or (plugin.__module__ == disabled):
+                    continue  # continue -> the plugin is not loaded
-                        # Errors are bad so disable the plugin in the database
-                        if not settings.PLUGIN_TESTING:  # pragma: no cover
-                            plugin_db_setting.active = False
-                            plugin_db_setting.save(no_reload=True)
-
-                        # Add to inactive plugins so it shows up in the ui
-                        self.plugins_inactive[plug_key] = plugin_db_setting
-                        continue  # continue -> the plugin is not loaded
-
-                # Initialize package
-                # now we can be sure that an admin has activated the plugin
-                logger.info(f'Loading plugin {plug_name}')
 
+                # Initialize package - we can be sure that an admin has activated the plugin
+                logger.info(f'Loading plugin `{plg_name}`')
                 try:
-                    plugin = plugin()
+                    plg_i: InvenTreePlugin = plg()
+                    logger.info(f'Loaded plugin `{plg_name}`')
                 except Exception as error:
-                    # log error and raise it -> disable plugin
+                    handle_error(error, log_name='init')  # log error and raise it -> disable plugin
-                    handle_error(error, log_name='init')
 
-                logger.debug(f'Loaded plugin {plug_name}')
+                # Safe extra attributes
+                plg_i.is_package = getattr(plg_i, 'is_package', False)
+                plg_i.pk = plg_db.pk if plg_db else None
+                plg_i.db = plg_db
 
-                plugin.is_package = was_packaged
+                # Run version check for plugin
-
+                if (plg_i.MIN_VERSION or plg_i.MAX_VERSION) and not plg_i.check_version():
-                if plugin_db_setting:
+                    safe_reference(plugin=plg_i, key=plg_key, active=False)
-                    plugin.pk = plugin_db_setting.pk
+                else:
-
+                    safe_reference(plugin=plg_i, key=plg_key)
-                # safe reference
+            else:  # pragma: no cover
-                self.plugins[plugin.slug] = plugin
+                safe_reference(plugin=plg, key=plg_key, active=False)
-            else:
-                # save for later reference
-                self.plugins_inactive[plug_key] = plugin_db_setting  # pragma: no cover
 
     def _activate_plugins(self, force_reload=False, full_reload: bool = False):
         """Run activation functions for all plugins.
@@ -583,10 +587,10 @@ class PluginsRegistry:
         """
         try:
             # for local path plugins
-            plugin_path = '.'.join(Path(plugin.path).relative_to(settings.BASE_DIR).parts)
+            plugin_path = '.'.join(plugin.path().relative_to(settings.BASE_DIR).parts)
         except ValueError:  # pragma: no cover
-            # plugin is shipped as package
+            # plugin is shipped as package - extract plugin module name
-            plugin_path = plugin.NAME
+            plugin_path = plugin.__module__.split('.')[0]
         return plugin_path
 
     def deactivate_plugin_app(self):
@@ -640,24 +644,25 @@ class PluginsRegistry:
         self.installed_apps = []
 
     def _clean_registry(self):
-        # remove all plugins from registry
+        """Remove all plugins from registry."""
-        self.plugins = {}
+        self.plugins: Dict[str, InvenTreePlugin] = {}
-        self.plugins_inactive = {}
+        self.plugins_inactive: Dict[str, InvenTreePlugin] = {}
+        self.plugins_full: Dict[str, InvenTreePlugin] = {}
 
     def _update_urls(self):
-        from InvenTree.urls import frontendpatterns as urlpatterns
+        from InvenTree.urls import frontendpatterns as urlpattern
         from InvenTree.urls import urlpatterns as global_pattern
         from plugin.urls import get_plugin_urls
 
-        for index, a in enumerate(urlpatterns):
+        for index, url in enumerate(urlpattern):
-            if hasattr(a, 'app_name'):
+            if hasattr(url, 'app_name'):
-                if a.app_name == 'admin':
+                if url.app_name == 'admin':
-                    urlpatterns[index] = re_path(r'^admin/', admin.site.urls, name='inventree-admin')
+                    urlpattern[index] = re_path(r'^admin/', admin.site.urls, name='inventree-admin')
-                elif a.app_name == 'plugin':
+                elif url.app_name == 'plugin':
-                    urlpatterns[index] = get_plugin_urls()
+                    urlpattern[index] = get_plugin_urls()
 
-        # replace frontendpatterns
+        # Replace frontendpatterns
-        global_pattern[0] = re_path('', include(urlpatterns))
+        global_pattern[0] = re_path('', include(urlpattern))
         clear_url_caches()
 
     def _reload_apps(self, force_reload: bool = False, full_reload: bool = False):
@@ -693,7 +698,7 @@ class PluginsRegistry:
     # endregion
 
 
-registry = PluginsRegistry()
+registry: PluginsRegistry = PluginsRegistry()
 
 
 def call_function(plugin_name, function_name, *args, **kwargs):

InvenTree/plugin/samples/integration/version.py

@@ -0,0 +1,9 @@
+"""Sample plugin for versioning."""
+from plugin import InvenTreePlugin
+
+
+class VersionPlugin(InvenTreePlugin):
+    """A small version sample."""
+
+    NAME = "version"
+    MAX_VERSION = '0.1.0'

InvenTree/plugin/template.py

@@ -1,7 +1,5 @@
 """Load templates for loaded plugins."""
 
-from pathlib import Path
-
 from django.template.loaders.filesystem import Loader as FilesystemLoader
 
 from plugin import registry
@@ -26,8 +24,8 @@ class PluginTemplateLoader(FilesystemLoader):
         template_dirs = []
 
         for plugin in registry.plugins.values():
-            new_path = Path(plugin.path) / dirname
+            new_path = plugin.path().joinpath(dirname)
-            if Path(new_path).is_dir():
+            if new_path.is_dir():
                 template_dirs.append(new_path)
 
         return tuple(template_dirs)

InvenTree/plugin/test_plugin.py

@@ -100,6 +100,14 @@ class InvenTreePluginTests(TestCase):
         self.plugin_name = NameInvenTreePlugin()
         self.plugin_sample = SampleIntegrationPlugin()
 
+        class VersionInvenTreePlugin(InvenTreePlugin):
+            NAME = 'Version'
+
+            MIN_VERSION = '0.1.0'
+            MAX_VERSION = '0.1.3'
+
+        self.plugin_version = VersionInvenTreePlugin()
+
     def test_basic_plugin_init(self):
         """Check if a basic plugin intis."""
         self.assertEqual(self.plugin.NAME, '')
@@ -169,6 +177,16 @@ class InvenTreePluginTests(TestCase):
             # check default value is used
             self.assertEqual(self.plugin_old.get_meta_value('ABC', 'ABCD', '123'), '123')
 
+    def test_version(self):
+        """Test Version checks"""
+
+        self.assertFalse(self.plugin_version.check_version([0, 0, 3]))
+        self.assertTrue(self.plugin_version.check_version([0, 1, 0]))
+        self.assertFalse(self.plugin_version.check_version([0, 1, 4]))
+
+        plug = registry.plugins_full.get('version')
+        self.assertEqual(plug.is_active(), False)
+
 
 class RegistryTests(TestCase):
     """Tests for registry loading methods."""

InvenTree/templates/InvenTree/settings/plugin.html

@@ -25,6 +25,8 @@
         {% include "InvenTree/settings/setting.html" with key="ENABLE_PLUGINS_NAVIGATION" icon="fa-sitemap" %}
         {% include "InvenTree/settings/setting.html" with key="ENABLE_PLUGINS_APP" icon="fa-rocket" %}
         {% include "InvenTree/settings/setting.html" with key="PLUGIN_ON_STARTUP" %}
+        <tr><td colspan='5'></td></tr>
+        {% include "InvenTree/settings/setting.html" with key="PLUGIN_CHECK_SIGNATURES" %}
     </tbody>
 </table>
 </div>

InvenTree/templates/InvenTree/settings/plugin_settings.html

@@ -112,6 +112,9 @@
                 <td>{% trans "Commit Message" %}</td><td>{{ plugin.package.message }}{% include "clip.html" %}</td>
             </tr>
             {% endif %}
+
+            {% settings_value "PLUGIN_CHECK_SIGNATURES" as signatures %}
+            {% if signatures %}
             <tr>
                 <td><span class='text-{{plugin.sign_color}} fas fa-check'></span></td>
                 <td>{% trans "Sign Status" %}</td>
@@ -122,6 +125,7 @@
                 <td>{% trans "Sign Key" %}</td>
                 <td class="bg-{{plugin.sign_color}}">{{ plugin.package.key }}{% include "clip.html" %}</td>
             </tr>
+            {% endif %}
         </table>
         </div>
     </div>