From 8e8b7158b7d59c02123c5f8391f6c8d851e8d478 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 22 Jan 2025 16:34:13 +1100 Subject: [PATCH] Plugin reload fix (#8922) * Add option to disable auto-reload of dev server * Force plugin reload * Add unit testing for plugin reload - Requires modifications to registry.py --- src/backend/InvenTree/.gitignore | 2 + src/backend/InvenTree/plugin/models.py | 8 +- src/backend/InvenTree/plugin/registry.py | 48 +++++++-- src/backend/InvenTree/plugin/test_api.py | 2 +- src/backend/InvenTree/plugin/test_plugin.py | 113 ++++++++++++++++++++ tasks.py | 21 +++- 6 files changed, 179 insertions(+), 15 deletions(-) create mode 100644 src/backend/InvenTree/.gitignore diff --git a/src/backend/InvenTree/.gitignore b/src/backend/InvenTree/.gitignore new file mode 100644 index 0000000000..4af4473ebe --- /dev/null +++ b/src/backend/InvenTree/.gitignore @@ -0,0 +1,2 @@ +# Files generated during unit testing +_testfolder/ diff --git a/src/backend/InvenTree/plugin/models.py b/src/backend/InvenTree/plugin/models.py index 18d4c6c51f..3068dbcfad 100644 --- a/src/backend/InvenTree/plugin/models.py +++ b/src/backend/InvenTree/plugin/models.py @@ -142,7 +142,7 @@ class PluginConfig(InvenTree.models.MetadataMixin, models.Model): 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 + no_reload = kwargs.pop('no_reload', False) # check if no_reload flag is set super().save(force_insert, force_update, *args, **kwargs) @@ -150,10 +150,10 @@ class PluginConfig(InvenTree.models.MetadataMixin, models.Model): # Force active if builtin self.active = True - if not reload and self.active != self.__org_active: + if not no_reload and self.active != self.__org_active: if settings.PLUGIN_TESTING: - warnings.warn('A reload was triggered', stacklevel=2) - registry.reload_plugins() + warnings.warn('A plugin registry reload was triggered', stacklevel=2) + registry.reload_plugins(full_reload=True, force_reload=True, collect=True) @admin.display(boolean=True, description=_('Installed')) def is_installed(self) -> bool: diff --git a/src/backend/InvenTree/plugin/registry.py b/src/backend/InvenTree/plugin/registry.py index 014f41d72d..f6d0b17b23 100644 --- a/src/backend/InvenTree/plugin/registry.py +++ b/src/backend/InvenTree/plugin/registry.py @@ -453,6 +453,8 @@ class PluginsRegistry: Args: plugin: Plugin module + configs: Plugin configuration dictionary + force_reload (bool, optional): Force reload of plugin. Defaults to False. """ from InvenTree import version @@ -485,6 +487,7 @@ class PluginsRegistry: # Check if this is a 'builtin' plugin builtin = plugin.check_is_builtin() + sample = plugin.check_is_sample() package_name = None @@ -510,11 +513,37 @@ class PluginsRegistry: # Initialize package - we can be sure that an admin has activated the plugin logger.debug('Loading plugin `%s`', plg_name) + # If this is a third-party plugin, reload the source module + # This is required to ensure that separate processes are using the same code + if not builtin and not sample: + plugin_name = plugin.__name__ + module_name = plugin.__module__ + + if plugin_module := sys.modules.get(module_name): + logger.debug('Reloading plugin `%s`', plg_name) + # Reload the module + try: + importlib.reload(plugin_module) + plugin = getattr(plugin_module, plugin_name) + except ModuleNotFoundError: + # No module found - try to import it directly + try: + raw_module = _load_source( + module_name, plugin_module.__file__ + ) + plugin = getattr(raw_module, plugin_name) + except Exception: + pass + except Exception: + logger.exception('Failed to reload plugin `%s`', plg_name) + try: t_start = time.time() plg_i: InvenTreePlugin = plugin() dt = time.time() - t_start logger.debug('Loaded plugin `%s` in %.3fs', plg_name, dt) + except ModuleNotFoundError as e: + raise e except Exception as error: handle_error( error, log_name='init' @@ -745,7 +774,7 @@ class PluginsRegistry: if old_hash != self.registry_hash: try: - logger.debug( + logger.info( 'Updating plugin registry hash: %s', str(self.registry_hash) ) set_global_setting( @@ -839,11 +868,16 @@ def _load_source(modname, filename): See https://docs.python.org/3/whatsnew/3.12.html#imp """ - loader = importlib.machinery.SourceFileLoader(modname, filename) - spec = importlib.util.spec_from_file_location(modname, filename, loader=loader) + if modname in sys.modules: + del sys.modules[modname] + + # loader = importlib.machinery.SourceFileLoader(modname, filename) + spec = importlib.util.spec_from_file_location(modname, filename) # , loader=loader) module = importlib.util.module_from_spec(spec) - # The module is always executed and not cached in sys.modules. - # Uncomment the following line to cache the module. - # sys.modules[module.__name__] = module - loader.exec_module(module) + + sys.modules[module.__name__] = module + + if spec.loader: + spec.loader.exec_module(module) + return module diff --git a/src/backend/InvenTree/plugin/test_api.py b/src/backend/InvenTree/plugin/test_api.py index 2b73bea416..5f3a481ce8 100644 --- a/src/backend/InvenTree/plugin/test_api.py +++ b/src/backend/InvenTree/plugin/test_api.py @@ -205,7 +205,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): plg_inactive.active = True plg_inactive.save() - self.assertEqual(cm.warning.args[0], 'A reload was triggered') + self.assertEqual(cm.warning.args[0], 'A plugin registry reload was triggered') def test_check_plugin(self): """Test check_plugin function.""" diff --git a/src/backend/InvenTree/plugin/test_plugin.py b/src/backend/InvenTree/plugin/test_plugin.py index cc034acdee..5f0810073b 100644 --- a/src/backend/InvenTree/plugin/test_plugin.py +++ b/src/backend/InvenTree/plugin/test_plugin.py @@ -4,9 +4,11 @@ import os import shutil import subprocess import tempfile +import textwrap from datetime import datetime from pathlib import Path from unittest import mock +from unittest.mock import patch from django.test import TestCase, override_settings @@ -18,6 +20,9 @@ from plugin.samples.integration.another_sample import ( ) from plugin.samples.integration.sample import SampleIntegrationPlugin +# Directory for testing plugins during CI +PLUGIN_TEST_DIR = '_testfolder/test_plugins' + class PluginTagTests(TestCase): """Tests for the plugin extras.""" @@ -287,3 +292,111 @@ class RegistryTests(TestCase): self.assertEqual( registry.errors.get('init')[0]['broken_sample'], "'This is a dummy error'" ) + + @override_settings(PLUGIN_TESTING=True, PLUGIN_TESTING_SETUP=True) + @patch.dict(os.environ, {'INVENTREE_PLUGIN_TEST_DIR': PLUGIN_TEST_DIR}) + def test_registry_reload(self): + """Test that the registry correctly reloads plugin modules. + + - Create a simple plugin which we can change the version + - Ensure that the "hash" of the plugin registry changes + """ + dummy_file = os.path.join(PLUGIN_TEST_DIR, 'dummy_ci_plugin.py') + + # Ensure the plugin dir exists + os.makedirs(PLUGIN_TEST_DIR, exist_ok=True) + + # Create an __init__.py file + init_file = os.path.join(PLUGIN_TEST_DIR, '__init__.py') + if not os.path.exists(init_file): + with open(os.path.join(init_file), 'w', encoding='utf-8') as f: + f.write('') + + def plugin_content(version): + """Return the content of the plugin file.""" + content = f""" + from plugin import InvenTreePlugin + + PLG_VERSION = "{version}" + + print(">>> LOADING DUMMY PLUGIN v" + PLG_VERSION + " <<<") + + class DummyCIPlugin(InvenTreePlugin): + + NAME = "DummyCIPlugin" + SLUG = "dummyci" + TITLE = "Dummy plugin for CI testing" + + VERSION = PLG_VERSION + + """ + + return textwrap.dedent(content) + + def create_plugin_file( + version: str, enabled: bool = True, reload: bool = True + ) -> str: + """Create a plugin file with the given version. + + Arguments: + version: The version string to use for the plugin file + enabled: Whether the plugin should be enabled or not + + Returns: + str: The plugin registry hash + """ + import time + + content = plugin_content(version) + + with open(dummy_file, 'w', encoding='utf-8') as f: + f.write(content) + + # Wait for the file to be written + time.sleep(2) + + if reload: + # Ensure the plugin is activated + registry.set_plugin_state('dummyci', enabled) + registry.reload_plugins( + full_reload=True, collect=True, force_reload=True + ) + + registry.update_plugin_hash() + + return registry.registry_hash + + # Initial hash, with plugin disabled + hash_disabled = create_plugin_file('0.0.1', enabled=False, reload=False) + + # Perform initial registry reload + registry.reload_plugins(full_reload=True, collect=True, force_reload=True) + + # Start plugin in known state + registry.set_plugin_state('dummyci', False) + + hash_disabled = create_plugin_file('0.0.1', enabled=False) + + # Enable the plugin + hash_enabled = create_plugin_file('0.1.0', enabled=True) + + # Hash must be different! + self.assertNotEqual(hash_disabled, hash_enabled) + + plugin_hash = hash_enabled + + for v in ['0.1.1', '7.1.2', '1.2.1', '4.0.1']: + h = create_plugin_file(v, enabled=True) + self.assertNotEqual(plugin_hash, h) + plugin_hash = h + + # Revert back to original 'version' + h = create_plugin_file('0.1.0', enabled=True) + self.assertEqual(hash_enabled, h) + + # Disable the plugin + h = create_plugin_file('0.0.1', enabled=False) + self.assertEqual(hash_disabled, h) + + # Finally, ensure that the plugin file is removed after testing + os.remove(dummy_file) diff --git a/tasks.py b/tasks.py index 4133005305..d19ea633aa 100644 --- a/tasks.py +++ b/tasks.py @@ -911,13 +911,28 @@ def gunicorn(c, address='0.0.0.0:8000', workers=None): run(c, cmd, pty=True) -@task(pre=[wait], help={'address': 'Server address:port (default=127.0.0.1:8000)'}) -def server(c, address='127.0.0.1:8000'): +@task( + pre=[wait], + help={ + 'address': 'Server address:port (default=127.0.0.1:8000)', + 'no_reload': 'Do not automatically reload the server in response to code changes', + 'no_threading': 'Disable multi-threading for the development server', + }, +) +def server(c, address='127.0.0.1:8000', no_reload=False, no_threading=False): """Launch a (development) server using Django's in-built webserver. Note: This is *not* sufficient for a production installation. """ - manage(c, f'runserver {address}', pty=True) + cmd = f'runserver {address}' + + if no_reload: + cmd += ' --noreload' + + if no_threading: + cmd += ' --nothreading' + + manage(c, cmd, pty=True) @task(pre=[wait])