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

@@ -0,0 +1,2 @@
+# Files generated during unit testing
+_testfolder/

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:

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

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."""

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)

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])