From ac849c156664c0867072ae7ae5ee13b22571d15f Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 4 Jan 2022 21:36:27 +1100 Subject: [PATCH] Fixes for unit tests --- InvenTree/plugin/admin.py | 23 +++-- .../plugin/builtin/integration/mixins.py | 14 +-- InvenTree/plugin/integration.py | 19 +++- InvenTree/plugin/plugin.py | 11 ++- InvenTree/plugin/registry.py | 90 ++++++++++++------- InvenTree/plugin/serializers.py | 9 +- InvenTree/plugin/test_api.py | 4 +- InvenTree/plugin/test_integration.py | 2 +- InvenTree/plugin/test_plugin.py | 5 +- InvenTree/plugin/urls.py | 8 +- 10 files changed, 116 insertions(+), 69 deletions(-) diff --git a/InvenTree/plugin/admin.py b/InvenTree/plugin/admin.py index e27c499479..b20aef8057 100644 --- a/InvenTree/plugin/admin.py +++ b/InvenTree/plugin/admin.py @@ -8,30 +8,38 @@ import plugin.registry as registry def plugin_update(queryset, new_status: bool): - """general function for bulk changing plugins""" + """ + General function for bulk changing plugins + """ + apps_changed = False - # run through all plugins in the queryset as the save method needs to be overridden + # Run through all plugins in the queryset as the save method needs to be overridden for plugin in queryset: if plugin.active is not new_status: plugin.active = new_status plugin.save(no_reload=True) apps_changed = True - # reload plugins if they changed + # Reload plugins if they changed if apps_changed: registry.plugin_registry.reload_plugins() @admin.action(description='Activate plugin(s)') def plugin_activate(modeladmin, request, queryset): - """activate a set of plugins""" + """ + Activate a set of plugins + """ plugin_update(queryset, True) @admin.action(description='Deactivate plugin(s)') def plugin_deactivate(modeladmin, request, queryset): - """deactivate a set of plugins""" + """ + Deactivate a set of plugins + """ + plugin_update(queryset, False) @@ -51,7 +59,10 @@ class PluginSettingInline(admin.TabularInline): class PluginConfigAdmin(admin.ModelAdmin): - """Custom admin with restricted id fields""" + """ + Custom admin with restricted id fields + """ + readonly_fields = ["key", "name", ] list_display = ['name', 'key', '__str__', 'active', ] list_filter = ['active'] diff --git a/InvenTree/plugin/builtin/integration/mixins.py b/InvenTree/plugin/builtin/integration/mixins.py index 30f3cd8551..c5d2411e4d 100644 --- a/InvenTree/plugin/builtin/integration/mixins.py +++ b/InvenTree/plugin/builtin/integration/mixins.py @@ -34,17 +34,9 @@ class SettingsMixin: Return the 'value' of the setting associated with this plugin """ - # Find the plugin configuration associated with this plugin + return PluginSetting.get_setting(key, plugin=self) - plugin = self.plugin_config() - - if plugin: - return PluginSetting.get_setting(key, plugin=plugin, settings=self.settings) - else: - # Plugin cannot be found, return default value - return PluginSetting.get_setting_default(key, settings=self.settings) - - def set_setting(self, key, value, user): + def set_setting(self, key, value, user=None): """ Set plugin setting value by key """ @@ -58,7 +50,7 @@ class SettingsMixin: # Cannot find associated plugin model, return return - PluginSetting.set_setting(key, value, user, plugin=plugin, settings=self.settings) + PluginSetting.set_setting(key, value, user, plugin=plugin) class UrlsMixin: diff --git a/InvenTree/plugin/integration.py b/InvenTree/plugin/integration.py index 2c593291d3..73223593a5 100644 --- a/InvenTree/plugin/integration.py +++ b/InvenTree/plugin/integration.py @@ -19,19 +19,27 @@ logger = logging.getLogger("inventree") class MixinBase: - """general base for mixins""" + """ + General base for mixins + """ def __init__(self) -> None: self._mixinreg = {} self._mixins = {} def add_mixin(self, key: str, fnc_enabled=True, cls=None): - """add a mixin to the plugins registry""" + """ + Add a mixin to the plugins registry + """ + self._mixins[key] = fnc_enabled self.setup_mixin(key, cls=cls) def setup_mixin(self, key, cls=None): - """define mixin details for the current mixin -> provides meta details for all active mixins""" + """ + Define mixin details for the current mixin -> provides meta details for all active mixins + """ + # get human name human_name = getattr(cls.MixinMeta, 'MIXIN_NAME', key) if cls and hasattr(cls, 'MixinMeta') else key @@ -43,7 +51,10 @@ class MixinBase: @property def registered_mixins(self, with_base: bool = False): - """get all registered mixins for the plugin""" + """ + Get all registered mixins for the plugin + """ + mixins = getattr(self, '_mixinreg', None) if mixins: # filter out base diff --git a/InvenTree/plugin/plugin.py b/InvenTree/plugin/plugin.py index 0cf8082b7f..35643b36c3 100644 --- a/InvenTree/plugin/plugin.py +++ b/InvenTree/plugin/plugin.py @@ -18,9 +18,9 @@ class InvenTreePlugin(): # Override the plugin name for each concrete plugin instance PLUGIN_NAME = '' - PLUGIN_SLUG = '' + PLUGIN_SLUG = None - PLUGIN_TITLE = '' + PLUGIN_TITLE = None def plugin_name(self): """ @@ -35,11 +35,14 @@ class InvenTreePlugin(): if slug is None: slug = self.plugin_name() - return slugify(slug) + return slugify(slug.lower()) def plugin_title(self): - return self.PLUGIN_TITLE + if self.PLUGIN_TITLE: + return self.PLUGIN_TITLE + else: + return self.plugin_name() def plugin_config(self, raise_error=False): """ diff --git a/InvenTree/plugin/registry.py b/InvenTree/plugin/registry.py index 30dbad5f95..fe28acfadb 100644 --- a/InvenTree/plugin/registry.py +++ b/InvenTree/plugin/registry.py @@ -1,7 +1,10 @@ """ -registry for plugins -holds the class and the object that contains all code to maintain plugin states +Registry for loading and managing multiple plugins at run-time + +- Holds the class and the object that contains all code to maintain plugin states +- Manages setup and teardown of plugin class instances """ + import importlib import pathlib import logging @@ -34,6 +37,10 @@ logger = logging.getLogger('inventree') class PluginsRegistry: + """ + The PluginsRegistry class + """ + def __init__(self) -> None: # plugin registry self.plugins = {} @@ -54,11 +61,15 @@ class PluginsRegistry: # region public plugin functions def load_plugins(self): - """load and activate all IntegrationPlugins""" + """ + Load and activate all IntegrationPlugins + """ + from plugin.helpers import log_plugin_error logger.info('Start loading plugins') - # set maintanace mode + + # Set maintanace mode _maintenance = bool(get_maintenance_mode()) if not _maintenance: set_maintenance_mode(True) @@ -68,7 +79,7 @@ class PluginsRegistry: retry_counter = settings.PLUGIN_RETRY while not registered_sucessfull: try: - # we are using the db so for migrations etc we need to try this block + # We are using the db so for migrations etc we need to try this block self._init_plugins(blocked_plugin) self._activate_plugins() registered_sucessfull = True @@ -81,13 +92,14 @@ class PluginsRegistry: log_plugin_error({error.path: error.message}, 'load') blocked_plugin = error.path # we will not try to load this app again - # init apps without any integration plugins + # Initialize apps without any integration plugins self._clean_registry() self._clean_installed_apps() self._activate_plugins(force_reload=True) - # we do not want to end in an endless loop + # We do not want to end in an endless loop retry_counter -= 1 + if retry_counter <= 0: if settings.PLUGIN_TESTING: print('[PLUGIN] Max retries, breaking loading') @@ -98,15 +110,20 @@ class PluginsRegistry: # now the loading will re-start up with init - # remove maintenance + # Remove maintenance mode if not _maintenance: set_maintenance_mode(False) + logger.info('Finished loading plugins') def unload_plugins(self): - """unload and deactivate all IntegrationPlugins""" + """ + Unload and deactivate all IntegrationPlugins + """ + logger.info('Start unloading plugins') - # set maintanace mode + + # Set maintanace mode _maintenance = bool(get_maintenance_mode()) if not _maintenance: set_maintenance_mode(True) @@ -123,21 +140,27 @@ class PluginsRegistry: logger.info('Finished unloading plugins') def reload_plugins(self): - """safely reload IntegrationPlugins""" - # do not reload whe currently loading + """ + Safely reload IntegrationPlugins + """ + + # Do not reload whe currently loading if self.is_loading: return logger.info('Start reloading plugins') + with maintenance_mode_on(): self.unload_plugins() self.load_plugins() - logger.info('Finished reloading plugins') - # endregion - # region general plugin managment mechanisms + logger.info('Finished reloading plugins') + def collect_plugins(self): - """collect integration plugins from all possible ways of loading""" + """ + Collect integration plugins from all possible ways of loading + """ + self.plugin_modules = [] # clear # Collect plugins from paths @@ -146,7 +169,7 @@ class PluginsRegistry: if modules: [self.plugin_modules.append(item) for item in modules] - # check if not running in testing mode and apps should be loaded from hooks + # Check if not running in testing mode and apps should be loaded from hooks if (not settings.PLUGIN_TESTING) or (settings.PLUGIN_TESTING and settings.PLUGIN_TESTING_SETUP): # Collect plugins from setup entry points for entry in metadata.entry_points().get('inventree_plugins', []): @@ -159,22 +182,25 @@ class PluginsRegistry: logger.info(", ".join([a.__module__ for a in self.plugin_modules])) def _init_plugins(self, disabled=None): - """initialise all found plugins + """ + Initialise all found plugins :param disabled: loading path of disabled app, defaults to None :type disabled: str, optional :raises error: IntegrationPluginError """ + from plugin.models import PluginConfig logger.info('Starting plugin initialisation') + # Initialize integration plugins for plugin in self.plugin_modules: - # check if package + # 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!! + # Check if activated + # These checks only use attributes - never use plugin supplied functions -> that would lead to arbitrary code execution!! plug_name = plugin.PLUGIN_NAME plug_key = plugin.PLUGIN_SLUG if getattr(plugin, 'PLUGIN_SLUG', None) else plug_name plug_key = slugify(plug_key) # keys are slugs! @@ -186,23 +212,23 @@ class PluginsRegistry: raise error plugin_db_setting = None - # always activate if testing + # Always activate if testing if settings.PLUGIN_TESTING or (plugin_db_setting and plugin_db_setting.active): - # check if the plugin was blocked -> threw an error + # Check if the plugin was blocked -> threw an error if disabled: # option1: package, option2: file-based if (plugin.__name__ == disabled) or (plugin.__module__ == disabled): - # errors are bad so disable the plugin in the database + # Errors are bad so disable the plugin in the database if not settings.PLUGIN_TESTING: plugin_db_setting.active = False # TODO save the error to the plugin plugin_db_setting.save(no_reload=True) - # add to inactive plugins so it shows up in the ui + # 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 - # init package + # Initialize package # now we can be sure that an admin has activated the plugin # TODO check more stuff -> as of Nov 2021 there are not many checks in place # but we could enhance those to check signatures, run the plugin against a whitelist etc. @@ -235,7 +261,7 @@ class PluginsRegistry: plugins = self.plugins.items() logger.info(f'Found {len(plugins)} active plugins') - self.activate_integration_globalsettings(plugins) + self.activate_integration_settings(plugins) self.activate_integration_app(plugins, force_reload=force_reload) def _deactivate_plugins(self): @@ -243,10 +269,9 @@ class PluginsRegistry: Run integration deactivation functions for all plugins """ self.deactivate_integration_app() - self.deactivate_integration_globalsettings() - # endregion + self.deactivate_integration_settings() - def activate_integration_globalsettings(self, plugins): + def activate_integration_settings(self, plugins): from common.models import InvenTreeSetting if settings.PLUGIN_TESTING or InvenTreeSetting.get_setting('ENABLE_PLUGINS_GLOBALSETTING'): @@ -256,7 +281,7 @@ class PluginsRegistry: plugin_setting = plugin.settings self.mixins_settings[slug] = plugin_setting - def deactivate_integration_globalsettings(self): + def deactivate_integration_settings(self): # collect all settings plugin_settings = {} @@ -267,7 +292,6 @@ class PluginsRegistry: # clear cache self.mixins_Fsettings = {} - # region integration_app def activate_integration_app(self, plugins, force_reload=False): """activate AppMixin plugins - add custom apps and reload @@ -441,8 +465,6 @@ class PluginsRegistry: return True, [] except Exception as error: get_plugin_error(error, do_raise=True) - # endregion - # endregion plugin_registry = PluginsRegistry() diff --git a/InvenTree/plugin/serializers.py b/InvenTree/plugin/serializers.py index da171a604d..cc999a5a4f 100644 --- a/InvenTree/plugin/serializers.py +++ b/InvenTree/plugin/serializers.py @@ -1,5 +1,5 @@ """ -JSON serializers for Stock app +JSON serializers for plugin app """ # -*- coding: utf-8 -*- @@ -20,7 +20,8 @@ from plugin.models import PluginConfig, PluginSetting class PluginConfigSerializer(serializers.ModelSerializer): - """ Serializer for a PluginConfig: + """ + Serializer for a PluginConfig: """ meta = serializers.DictField(read_only=True) @@ -73,7 +74,7 @@ class PluginConfigInstallSerializer(serializers.Serializer): if not data.get('confirm'): raise ValidationError({'confirm': _('Installation not confirmed')}) if (not data.get('url')) and (not data.get('packagename')): - msg = _('Either packagenmae of url must be provided') + msg = _('Either packagename of URL must be provided') raise ValidationError({'url': msg, 'packagename': msg}) return data @@ -115,7 +116,7 @@ class PluginConfigInstallSerializer(serializers.Serializer): ret['result'] = str(error.output, 'utf-8') ret['error'] = True - # register plugins + # Register plugins # TODO return ret diff --git a/InvenTree/plugin/test_api.py b/InvenTree/plugin/test_api.py index fe1e9802bf..fdc97e407b 100644 --- a/InvenTree/plugin/test_api.py +++ b/InvenTree/plugin/test_api.py @@ -8,7 +8,7 @@ from InvenTree.api_tester import InvenTreeAPITestCase class PluginDetailAPITest(InvenTreeAPITestCase): """ - Tests the plugin AP I endpoints + Tests the plugin API endpoints """ roles = [ @@ -19,7 +19,7 @@ class PluginDetailAPITest(InvenTreeAPITestCase): ] def setUp(self): - self.MSG_NO_PKG = 'Either packagenmae of url must be provided' + self.MSG_NO_PKG = 'Either packagename of URL must be provided' self.PKG_NAME = 'minimal' super().setUp() diff --git a/InvenTree/plugin/test_integration.py b/InvenTree/plugin/test_integration.py index f88994384a..3d88fed4dd 100644 --- a/InvenTree/plugin/test_integration.py +++ b/InvenTree/plugin/test_integration.py @@ -42,7 +42,7 @@ class SettingsMixinTest(BaseMixinDefinition, TestCase): def test_function(self): # settings variable - self.assertEqual(self.mixin.globalsettings, self.TEST_SETTINGS) + self.assertEqual(self.mixin.settings, self.TEST_SETTINGS) # calling settings # not existing diff --git a/InvenTree/plugin/test_plugin.py b/InvenTree/plugin/test_plugin.py index 5724bdb0a9..2013ad43c8 100644 --- a/InvenTree/plugin/test_plugin.py +++ b/InvenTree/plugin/test_plugin.py @@ -1,4 +1,6 @@ -""" Unit tests for plugins """ +""" +Unit tests for plugins +""" from django.test import TestCase @@ -6,7 +8,6 @@ import plugin.plugin import plugin.integration from plugin.samples.integration.sample import SampleIntegrationPlugin from plugin.samples.integration.another_sample import WrongIntegrationPlugin, NoIntegrationPlugin -# from plugin.plugins import load_action_plugins, load_barcode_plugins import plugin.templatetags.plugin_extras as plugin_tags from plugin import plugin_registry diff --git a/InvenTree/plugin/urls.py b/InvenTree/plugin/urls.py index 46cdfd8a84..1457aaf6f1 100644 --- a/InvenTree/plugin/urls.py +++ b/InvenTree/plugin/urls.py @@ -1,6 +1,7 @@ """ URL lookup for plugin app """ + from django.conf.urls import url, include from plugin import plugin_registry @@ -10,9 +11,14 @@ PLUGIN_BASE = 'plugin' # Constant for links def get_plugin_urls(): - """returns a urlpattern that can be integrated into the global urls""" + """ + Returns a urlpattern that can be integrated into the global urls + """ + urls = [] + for plugin in plugin_registry.plugins.values(): if plugin.mixin_enabled('urls'): urls.append(plugin.urlpatterns) + return url(f'^{PLUGIN_BASE}/', include((urls, 'plugin')))