♻️(plugins) rewrite plugin system as django app

This allow more flexibility around the installed plugins, this will
allow to add models in plugins if needed.

CHANGELOG.md

@@ -15,6 +15,10 @@ and this project adheres to
 - ✨(oidc) add simple introspection backend #832
 - 🧑‍💻(tasks) run management commands #814
 
+### Changed
+
+- ♻️(plugins) rewrite plugin system as django app #844
+
 ### Fixed
 
 - 🐛(oauth2) force JWT signed for /userinfo #804

env.d/development/common.e2e.dist

@@ -4,3 +4,5 @@ BURST_THROTTLE_RATES="200/minute"
 
 OAUTH2_PROVIDER_OIDC_ENABLED = True
 OAUTH2_PROVIDER_VALIDATOR_CLASS: "mailbox_oauth2.validators.ProConnectValidator"
+
+INSTALLED_PLUGINS=plugins.la_suite.apps.LaSuitePluginConfig

env.d/development/france.dist

@@ -1,2 +1,2 @@
-ORGANIZATION_PLUGINS=plugins.organizations.NameFromSiretOrganizationPlugin,plugins.organizations.CommuneCreation
-DNS_PROVISIONING_TARGET_ZONE=test.collectivite.fr
+INSTALLED_PLUGINS=plugins.la_suite.apps.LaSuitePluginConfig
+DNS_PROVISIONING_TARGET_ZONE=test.collectivite.fr

src/backend/core/admin.py

@@ -10,10 +10,7 @@ from treebeard.forms import movenodeform_factory
 from mailbox_manager.admin import MailDomainAccessInline
 
 from . import models
-from .plugins.loader import (
-    get_organization_plugins,
-    organization_plugins_run_after_create,
-)
+from .plugins.registry import registry as plugin_hooks_registry
 
 
 class TeamAccessInline(admin.TabularInline):
@@ -229,7 +226,7 @@ class OrganizationServiceProviderInline(admin.TabularInline):
 def run_post_creation_plugins(modeladmin, request, queryset):  # pylint: disable=unused-argument
     """Run the post creation plugins for the selected organizations."""
     for organization in queryset:
-        organization_plugins_run_after_create(organization)
+        plugin_hooks_registry.execute_hook("organization_created", organization)
 
     messages.success(
         request,
@@ -254,7 +251,7 @@ class OrganizationAdmin(admin.ModelAdmin):
     def get_actions(self, request):
         """Adapt actions list to the context."""
         actions = super().get_actions(request)
-        if not get_organization_plugins():
+        if not plugin_hooks_registry.get_callbacks("organization_created"):
             actions.pop("run_post_creation_plugins", None)
         return actions
 

src/backend/core/models.py

@@ -31,10 +31,7 @@ from timezone_field import TimeZoneField
 from treebeard.mp_tree import MP_Node, MP_NodeManager
 
 from core.enums import WebhookStatusChoices
-from core.plugins.loader import (
-    organization_plugins_run_after_create,
-    organization_plugins_run_after_grant_access,
-)
+from core.plugins.registry import registry as plugin_hooks_registry
 from core.utils.webhooks import scim_synchronizer
 from core.validators import get_field_validators_from_setting
 
@@ -325,7 +322,7 @@ class OrganizationManager(models.Manager):
         This method is overridden to call the Organization plugins.
         """
         instance = super().create(**kwargs)
-        organization_plugins_run_after_create(instance)
+        plugin_hooks_registry.execute_hook("organization_created", instance)
         return instance
 
 
@@ -341,7 +338,7 @@ class OrganizationAccessManager(models.Manager):
         This method is overridden to call the Organization plugins.
         """
         instance = super().create(**kwargs)
-        organization_plugins_run_after_grant_access(instance)
+        plugin_hooks_registry.execute_hook("organization_access_granted", instance)
         return instance
 
 

src/backend/core/plugins/base.py

@@ -1,19 +1,16 @@
-"""Base plugin class for organization plugins."""
+"""Base Django Application Configuration for plugins."""
+
+from django.apps import AppConfig
 
 
-class BaseOrganizationPlugin:
-    """
-    Base class for organization plugins.
+class BasePluginAppConfig(AppConfig):
+    """Configuration for the La Suite plugin application."""
 
-    Plugins must implement all methods of this class even if it is only to "pass".
-    """
+    def ready(self):
+        """
+        Initialize the hooks registry when the application is ready.
+        This is called by Django when the application is loaded.
+        """
+        from .registry import registry  # pylint: disable=import-outside-toplevel
 
-    def run_after_create(self, organization) -> None:
-        """Method called after creating an organization."""
-        raise NotImplementedError("Plugins must implement the run_after_create method")
-
-    def run_after_grant_access(self, organization_access) -> None:
-        """Method called after creating an organization."""
-        raise NotImplementedError(
-            "Plugins must implement the run_after_grant_access method"
-        )
+        registry.register_app(self.name)

src/backend/core/plugins/loader.py

@@ -1,44 +0,0 @@
-"""Helper functions to load and run organization plugins."""
-
-from functools import lru_cache
-from typing import List
-
-from django.conf import settings
-from django.utils.module_loading import import_string
-
-from core.plugins.base import BaseOrganizationPlugin
-
-
-@lru_cache(maxsize=None)
-def get_organization_plugins() -> List[BaseOrganizationPlugin]:
-    """
-    Return a list of all organization plugins.
-    While the plugins initialization does not depend on the request, we can cache the result.
-    """
-    return [
-        import_string(plugin_path)() for plugin_path in settings.ORGANIZATION_PLUGINS
-    ]
-
-
-def organization_plugins_run_after_create(organization):
-    """
-    Run the after create method for all organization plugins.
-
-    Each plugin will be called in the order they are listed in the settings.
-    Each plugin is responsible to save changes if needed, this is not optimized
-    but this could be easily improved later if needed.
-    """
-    for plugin_instance in get_organization_plugins():
-        plugin_instance.run_after_create(organization)
-
-
-def organization_plugins_run_after_grant_access(organization_access):
-    """
-    Run the after grant access method for all organization plugins.
-
-    Each plugin will be called in the order they are listed in the settings.
-    Each plugin is responsible to save changes if needed, this is not optimized
-    but this could be easily improved later if needed.
-    """
-    for plugin_instance in get_organization_plugins():
-        plugin_instance.run_after_grant_access(organization_access)

src/backend/core/plugins/registry.py

@@ -0,0 +1,125 @@
+"""Registry for hooks."""
+
+import logging
+from typing import Callable, Dict, List, Set
+
+logger = logging.getLogger(__name__)
+
+
+class HooksRegistry:
+    """Registry for hooks."""
+
+    _available_hooks = {
+        "organization_created",
+        "organization_access_granted",
+    }
+
+    def __init__(self):
+        """Initialize the registry."""
+        self._hooks: Dict[str, List[Callable]] = {
+            hook_name: [] for hook_name in self._available_hooks
+        }
+        self._registered_apps: Set[str] = set()
+
+    def register_hook(self, hook_name: str, callback: Callable) -> None:
+        """
+        Register a hook callback.
+
+        Args:
+            hook_name: The name of the hook.
+            callback: The callback function to register.
+        """
+        try:
+            self._hooks[hook_name].append(callback)
+        except KeyError as exc:
+            logger.exception(
+                "Failed to register hook '%s' is not a valid hook: %s", hook_name, exc
+            )
+        logger.info("Registered hook %s: %s", hook_name, callback)
+
+    def register_app(self, app_name: str) -> None:
+        """
+        Register an app as having hooks.
+
+        Args:
+            app_name: The name of the app.
+        """
+        if app_name in self._registered_apps:
+            return
+
+        self._registered_apps.add(app_name)
+        try:
+            # Try to import the hooks module from the app
+            __import__(f"{app_name}.hooks")
+            logger.info("Registered hooks from app: %s", app_name)
+        except ImportError:
+            # It's okay if the app doesn't have a hooks module
+            logger.info("App %s has no hooks module", app_name)
+
+    def get_callbacks(self, hook_name: str) -> List[Callable]:
+        """
+        Get all callbacks for a hook.
+
+        Args:
+            hook_name: The name of the hook.
+
+        Returns:
+            A list of callback functions.
+        """
+        try:
+            return self._hooks[hook_name]
+        except KeyError as exc:
+            logger.exception(
+                "Failed to get callbacks for hook '%s' is not a valid hook: %s",
+                hook_name,
+                exc,
+            )
+            return []
+
+    def execute_hook(self, hook_name: str, *args, **kwargs):
+        """
+        Execute all callbacks for a hook.
+
+        Args:
+            hook_name: The name of the hook.
+            *args: Positional arguments to pass to the callbacks.
+            **kwargs: Keyword arguments to pass to the callbacks.
+
+        Returns:
+            A list of results from the callbacks.
+        """
+        results = []
+        for callback in self.get_callbacks(hook_name):
+            try:
+                result = callback(*args, **kwargs)
+                results.append(result)
+            except Exception as e:  # pylint: disable=broad-except
+                logger.exception("Error executing hook %s: %s", hook_name, e)
+        return results
+
+    def reset(self):
+        """Function to reset the registry, to be used in test only."""
+        self._hooks = {hook_name: [] for hook_name in self._available_hooks}
+        self._registered_apps = set()
+
+
+# Create a singleton instance of the registry
+registry = HooksRegistry()
+
+
+def register_hook(hook_name: str):
+    """
+    Decorator to register a function as a hook callback.
+
+    Args:
+        hook_name: The name of the hook.
+
+    Returns:
+        A decorator function.
+    """
+
+    def decorator(func):
+        registry.register_hook(hook_name, func)
+        return func
+
+    return decorator

src/backend/people/settings.py

@@ -217,6 +217,11 @@ class Base(Configuration):
     ]
 
     # Django's applications from the highest priority to the lowest
+    INSTALLED_PLUGINS = values.ListValue(
+        default=[],
+        environ_name="INSTALLED_PLUGINS",
+        environ_prefix=None,
+    )
     INSTALLED_APPS = [
         # People
         "admin.apps.PeopleAdminConfig",  # replaces 'django.contrib.admin'
@@ -224,9 +229,10 @@ class Base(Configuration):
         "demo",
         "mailbox_manager.apps.MailboxManagerConfig",
         "mailbox_oauth2",
+        *INSTALLED_PLUGINS,
+        # Third party apps
         "drf_spectacular",
         "drf_spectacular_sidecar",  # required for Django collectstatic discovery
-        # Third party apps
         "corsheaders",
         "django_celery_beat",
         "django_celery_results",
@@ -574,11 +580,6 @@ class Base(Configuration):
             environ_prefix=None,
         )
     )
-    ORGANIZATION_PLUGINS = values.ListValue(
-        default=[],
-        environ_name="ORGANIZATION_PLUGINS",
-        environ_prefix=None,
-    )
     ORGANIZATION_METADATA_SCHEMA = values.Value(
         default=None,
         environ_name="ORGANIZATION_METADATA_SCHEMA",

src/backend/plugins/la_suite/__init__.py

@@ -0,0 +1 @@
+"""Plugin module for La Suite numérique."""

src/backend/plugins/la_suite/apps.py

@@ -0,0 +1,10 @@
+"""La Suite plugin application configuration."""
+
+from core.plugins.base import BasePluginAppConfig
+
+
+class LaSuitePluginConfig(BasePluginAppConfig):
+    """Configuration for the La Suite plugin application."""
+
+    name = "plugins.la_suite"
+    verbose_name = "La Suite Plugin"

src/backend/plugins/la_suite/hooks.py

@@ -0,0 +1,32 @@
+"""
+Hooks registration for the la_suite plugin.
+
+This module is automagically loaded by the plugin system.
+Putting hooks registration here allows to test the "utils"
+function without registering the hook unwillingly.
+"""
+
+from core.plugins.registry import register_hook
+
+from plugins.la_suite.hooks_utils.all_organizations import (
+    get_organization_name_and_metadata_from_siret,
+)
+from plugins.la_suite.hooks_utils.communes import CommuneCreation
+
+
+@register_hook("organization_created")
+def get_organization_name_and_metadata_from_siret_hook(organization):
+    """After creating an organization, update the organization name & metadata."""
+    get_organization_name_and_metadata_from_siret(organization)
+
+
+@register_hook("organization_created")
+def commune_organization_created(organization):
+    """After creating an organization, update the organization name."""
+    CommuneCreation().run_after_create(organization)
+
+
+@register_hook("organization_access_granted")
+def commune_organization_access_granted(organization_access):
+    """After granting an organization access, check for needed domain access grant."""
+    CommuneCreation().run_after_grant_access(organization_access)

src/backend/plugins/la_suite/hooks_utils/__init__.py

@@ -0,0 +1 @@
+"""Hook modules for La Suite"""

src/backend/plugins/la_suite/hooks_utils/all_organizations.py

@@ -0,0 +1,79 @@
+"""
+This hook is used to convert the organization registration ID
+to the proper name. For French organization the registration ID
+is the SIRET.
+
+This is a very specific plugin for French organizations and this
+first implementation is very basic. It surely needs to be improved
+later.
+"""
+
+import logging
+
+import requests
+from requests.adapters import HTTPAdapter, Retry
+
+logger = logging.getLogger(__name__)
+
+
+API_URL = "https://recherche-entreprises.api.gouv.fr/search?q={siret}"
+
+
+def _get_organization_name_and_metadata_from_results(data, siret):
+    """Return the organization name and metadata from the results of a SIRET search."""
+    org_metadata = {}
+    for result in data["results"]:
+        for organization in result["matching_etablissements"]:
+            if organization.get("siret") == siret:
+                org_metadata["is_public_service"] = result.get("complements", {}).get(
+                    "est_service_public", False
+                )
+                org_metadata["is_commune"] = (
+                    str(result.get("nature_juridique", "")) == "7210"
+                )
+
+                store_signs = organization.get("liste_enseignes") or []
+                if store_signs:
+                    return store_signs[0].title(), org_metadata
+                if name := result.get("nom_raison_sociale"):
+                    return name.title(), org_metadata
+
+    logger.warning("No organization name found for SIRET %s", siret)
+    return None, org_metadata
+
+
+def get_organization_name_and_metadata_from_siret(organization):
+    """After creating an organization, update the organization name."""
+    if not organization.registration_id_list:
+        # No registration ID to convert...
+        return
+
+    if organization.name not in organization.registration_id_list:
+        # The name has probably already been customized
+        return
+
+    # In the nominal case, there is only one registration ID because
+    # the organization as been created from it.
+    try:
+        # Retry logic as the API may be rate limited
+        s = requests.Session()
+        retries = Retry(total=5, backoff_factor=0.1, status_forcelist=[429])
+        s.mount("https://", HTTPAdapter(max_retries=retries))
+
+        siret = organization.registration_id_list[0]
+        response = s.get(API_URL.format(siret=siret), timeout=10)
+        response.raise_for_status()
+        data = response.json()
+    except requests.RequestException as exc:
+        logger.exception("%s: Unable to fetch organization name from SIRET", exc)
+        return
+
+    name, metadata = _get_organization_name_and_metadata_from_results(data, siret)
+    if not name:  # don't consider metadata either
+        return
+
+    organization.name = name
+    organization.metadata = (organization.metadata or {}) | metadata
+
+    organization.save(update_fields=["name", "metadata", "updated_at"])
+    logger.info("Organization %s name updated to %s", organization, name)

src/backend/plugins/la_suite/hooks_utils/communes.py

@@ -9,92 +9,12 @@ from django.utils.text import slugify
 import requests
 from requests.adapters import HTTPAdapter, Retry
 
-from core.plugins.base import BaseOrganizationPlugin
-
 from mailbox_manager.enums import MailDomainRoleChoices
 from mailbox_manager.models import MailDomain, MailDomainAccess
 
 logger = logging.getLogger(__name__)
 
 
-class NameFromSiretOrganizationPlugin(BaseOrganizationPlugin):
-    """
-    This plugin is used to convert the organization registration ID
-    to the proper name. For French organization the registration ID
-    is the SIRET.
-
-    This is a very specific plugin for French organizations and this
-    first implementation is very basic. It surely needs to be improved
-    later.
-    """
-
-    _api_url = "https://recherche-entreprises.api.gouv.fr/search?q={siret}"
-
-    @staticmethod
-    def get_organization_name_and_metadata_from_results(data, siret):
-        """Return the organization name and metadata from the results of a SIRET search."""
-        org_metadata = {}
-        for result in data["results"]:
-            for organization in result["matching_etablissements"]:
-                if organization.get("siret") == siret:
-                    org_metadata["is_public_service"] = result.get(
-                        "complements", {}
-                    ).get("est_service_public", False)
-                    org_metadata["is_commune"] = (
-                        str(result.get("nature_juridique", "")) == "7210"
-                    )
-
-                    store_signs = organization.get("liste_enseignes") or []
-                    if store_signs:
-                        return store_signs[0].title(), org_metadata
-                    if name := result.get("nom_raison_sociale"):
-                        return name.title(), org_metadata
-
-        logger.warning("No organization name found for SIRET %s", siret)
-        return None, org_metadata
-
-    def run_after_create(self, organization):
-        """After creating an organization, update the organization name."""
-        if not organization.registration_id_list:
-            # No registration ID to convert...
-            return
-
-        if organization.name not in organization.registration_id_list:
-            # The name has probably already been customized
-            return
-
-        # In the nominal case, there is only one registration ID because
-        # the organization as been created from it.
-        try:
-            # Retry logic as the API may be rate limited
-            s = requests.Session()
-            retries = Retry(total=5, backoff_factor=0.1, status_forcelist=[429])
-            s.mount("https://", HTTPAdapter(max_retries=retries))
-
-            siret = organization.registration_id_list[0]
-            response = s.get(self._api_url.format(siret=siret), timeout=10)
-            response.raise_for_status()
-            data = response.json()
-        except requests.RequestException as exc:
-            logger.exception("%s: Unable to fetch organization name from SIRET", exc)
-            return
-
-        name, metadata = self.get_organization_name_and_metadata_from_results(
-            data, siret
-        )
-        if not name:  # don't consider metadata either
-            return
-
-        organization.name = name
-        organization.metadata = (organization.metadata or {}) | metadata
-
-        organization.save(update_fields=["name", "metadata", "updated_at"])
-        logger.info("Organization %s name updated to %s", organization, name)
-
-    def run_after_grant_access(self, organization_access):
-        """After granting an organization access, we don't need to do anything."""
-
-
 class ApiCall:
     """Encapsulates a call to an external API"""
 
@@ -134,7 +54,7 @@ class ApiCall:
         )
 
 
-class CommuneCreation(BaseOrganizationPlugin):
+class CommuneCreation:
     """
     This plugin handles setup tasks for French communes.
     """

src/backend/plugins/la_suite/tests/hooks/test_commune_creation.py

@@ -6,7 +6,7 @@ from django.test.utils import override_settings
 import pytest
 import responses
 
-from plugins.organizations import ApiCall, CommuneCreation
+from plugins.la_suite.hooks_utils.communes import ApiCall, CommuneCreation
 
 pytestmark = pytest.mark.django_db
 

src/backend/plugins/la_suite/tests/hooks/test_get_organization_name_and_metadata_from_siret.py

@@ -4,7 +4,11 @@ import pytest
 import responses
 
 from core.models import Organization, get_organization_metadata_schema
-from core.plugins.loader import get_organization_plugins
+from core.plugins.registry import registry
+
+from plugins.la_suite.hooks_utils.all_organizations import (
+    get_organization_name_and_metadata_from_siret,
+)
 
 pytestmark = pytest.mark.django_db
 
@@ -14,21 +18,16 @@ pytestmark = pytest.mark.django_db
 # pylint: disable=unused-argument
 
 
-@pytest.fixture(name="organization_plugins_settings")
-def organization_plugins_settings_fixture(settings):
+@pytest.fixture(name="hook_settings")
+def hook_settings_fixture(settings):
     """
     Fixture to set the organization plugins settings and
     leave the initial state after the test.
     """
-    _original_plugins = settings.ORGANIZATION_PLUGINS
-
-    settings.ORGANIZATION_PLUGINS = [
-        "plugins.organizations.NameFromSiretOrganizationPlugin"
-    ]
-
-    # reset get_organization_plugins cache
-    get_organization_plugins.cache_clear()
-    get_organization_plugins()  # call to populate the cache
+    _original_hooks = dict(registry._hooks.items())  # pylint: disable=protected-access
+    registry.register_hook(
+        "organization_created", get_organization_name_and_metadata_from_siret
+    )
 
     settings.ORGANIZATION_METADATA_SCHEMA = "fr/organization_metadata.json"
 
@@ -38,10 +37,8 @@ def organization_plugins_settings_fixture(settings):
 
     yield
 
-    # reset get_organization_plugins cache
-    settings.ORGANIZATION_PLUGINS = _original_plugins
-    get_organization_plugins.cache_clear()
-    get_organization_plugins()  # call to populate the cache
+    # reset the hooks
+    registry._hooks = _original_hooks  # pylint: disable=protected-access
 
     settings.ORGANIZATION_METADATA_SCHEMA = None
 
@@ -61,7 +58,7 @@ def organization_plugins_settings_fixture(settings):
     ],
 )
 def test_organization_plugins_run_after_create(
-    organization_plugins_settings, nature_juridique, is_commune, is_public_service
+    hook_settings, nature_juridique, is_commune, is_public_service
 ):
     """Test the run_after_create method of the organization plugins for nominal case."""
     responses.add(
@@ -107,7 +104,7 @@ def test_organization_plugins_run_after_create(
 
 
 @responses.activate
-def test_organization_plugins_run_after_create_api_fail(organization_plugins_settings):
+def test_organization_plugins_run_after_create_api_fail(hook_settings):
     """Test the plugin when the API call fails."""
     responses.add(
         responses.GET,
@@ -140,9 +137,7 @@ def test_organization_plugins_run_after_create_api_fail(organization_plugins_set
         },
     ],
 )
-def test_organization_plugins_run_after_create_missing_data(
-    organization_plugins_settings, results
-):
+def test_organization_plugins_run_after_create_missing_data(hook_settings, results):
     """Test the plugin when the API call returns missing data."""
     responses.add(
         responses.GET,
@@ -159,7 +154,7 @@ def test_organization_plugins_run_after_create_missing_data(
 
 @responses.activate
 def test_organization_plugins_run_after_create_name_already_set(
-    organization_plugins_settings,
+    hook_settings,
 ):
     """Test the plugin does nothing when the name already differs from the registration ID."""
     organization = Organization.objects.create(
@@ -170,7 +165,7 @@ def test_organization_plugins_run_after_create_name_already_set(
 
 @responses.activate
 def test_organization_plugins_run_after_create_no_list_enseignes(
-    organization_plugins_settings,
+    hook_settings,
 ):
     """Test the run_after_create method of the organization plugins for nominal case."""
     responses.add(

src/backend/plugins/la_suite/tests/hooks/test_hooks_loaded.py

@@ -0,0 +1,42 @@
+"""Test module to check all application hooks are loaded."""
+
+from core.plugins.registry import registry
+
+from plugins.la_suite.apps import LaSuitePluginConfig
+
+
+def test_hooks_loaded():
+    """Test to check all application hooks are loaded."""
+    _original_hooks = dict(registry._hooks.items())  # pylint: disable=protected-access
+    _original_registered_apps = set(registry._registered_apps)  # pylint: disable=protected-access
+
+    registry.reset()
+
+    assert registry.get_callbacks("organization_created") == []
+    assert registry.get_callbacks("organization_access_granted") == []
+
+    # Force the application to run "ready" method
+    LaSuitePluginConfig(
+        app_name="plugins.la_suite", app_module=__import__("plugins.la_suite")
+    ).ready()
+
+    # Check that the hooks are loaded
+    organization_created_hook_names = [
+        callback.__name__ for callback in registry.get_callbacks("organization_created")
+    ]
+    assert organization_created_hook_names == [
+        "get_organization_name_and_metadata_from_siret_hook",
+        "commune_organization_created",
+    ]
+
+    organization_access_granted_hook_names = [
+        callback.__name__
+        for callback in registry.get_callbacks("organization_access_granted")
+    ]
+    assert organization_access_granted_hook_names == [
+        "commune_organization_access_granted"
+    ]
+
+    # cleanup the hooks
+    registry._hooks = _original_hooks  # pylint: disable=protected-access
+    registry._registered_apps = _original_registered_apps  # pylint: disable=protected-access