[feature] Added generic solution for cache invalidation in config models #1095

docs/developer/utils.rst

@@ -407,3 +407,93 @@ updates a ``WHOISInfo`` record.
 This signal is emitted when a WHOIS lookup is not triggered because the
 lookup conditions were not met (for example, an up-to-date WHOIS record
 already exists).
+
+.. _cache_invalidation:
+
+Cache Invalidation
+------------------
+
+OpenWISP Controller caches expensive values such as the configuration
+checksum of devices and VPN servers, and the controller view responses.
+When a *related* object changes (for example a certificate is renewed, a
+device group context is edited, or a template is deleted) the cached value
+can become stale and must be invalidated.
+
+This is handled by a declarative engine built around the
+``CacheDependency`` class. Instead of scattering ``signal.connect()``
+calls across the codebase, each model that owns a cached value declares,
+in one place, which related changes invalidate it.
+
+A model that owns a cache mixes in ``CacheInvalidationMixin`` and
+overrides ``get_cache_dependencies()`` to return a list of
+``CacheDependency`` objects:
+
+.. code-block:: python
+
+    from openwisp_controller.config.base.cache import (
+        CacheDependency,
+        CacheInvalidationMixin,
+    )
+
+
+    class AbstractConfig(CacheInvalidationMixin, ...):
+        @classmethod
+        def get_cache_dependencies(cls):
+            return [
+                # recompute the owning Config checksum when its client
+                # certificate changes
+                CacheDependency(
+                    source="django_x509.Cert",
+                    signal="post_save",
+                    resolve=cls._resolve_cert_dependency,
+                    target="update_status_if_checksum_changed",
+                ),
+            ]
+
+These declarations are wired at startup by
+``register_cache_dependencies()``. Caches that are not owned by a model
+(the controller view caches and the device group cache) are declared
+instead in the ``AppConfig``, in
+``ConfigConfig.connect_cache_dependencies()``.
+
+.. _print_cache_dependencies:
+
+``print_cache_dependencies``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Because dependencies are declared in more than one place, this management
+command prints every cache dependency wired in the running project, so the
+whole invalidation graph can be inspected at a glance:
+
+.. code-block:: bash
+
+    python manage.py print_cache_dependencies
+
+The output is grouped by source and signal, and reports the target action,
+the resolver, any tracked fields, and the dispatch UID of each dependency:
+
+.. code-block:: text
+
+    config.device (post_save)
+      target: update_status_if_checksum_changed
+        resolve: _resolve_device_dependency   track_fields: os, group_id, organization_id   on_create: False   on_commit: True
+        uid: cache_invalidation.config.config.config.device.post_save.update_status_if_checksum_changed._resolve_device_dependency.os+group_id+organization_id
+
+    config.template (pre_delete)
+      target: update_status_if_checksum_changed
+        resolve: _resolve_template_dependency   on_create: False   on_commit: True
+        uid: cache_invalidation.config.config.config.template.pre_delete.update_status_if_checksum_changed._resolve_template_dependency
+
+Pass ``--format json`` for machine-readable output (useful, for example,
+in a CI check that the wiring has not silently drifted):
+
+.. code-block:: bash
+
+    python manage.py print_cache_dependencies --format json
+
+.. note::
+
+    This engine invalidates caches automatically when related objects
+    change. You still need to run ``clear_cache`` manually after editing
+    the :ref:`OPENWISP_CONTROLLER_CONTEXT <context_setting>` setting,
+    because those system-wide variables are not tied to any model change.

openwisp_controller/config/api/views.py

@@ -40,7 +40,6 @@
 Config = load_model("config", "Config")
 VpnClient = load_model("config", "VpnClient")
 Cert = load_model("django_x509", "Cert")
-Organization = load_model("openwisp_users", "Organization")
 
 
 class TemplateListCreateView(ProtectedAPIMixin, ListCreateAPIView):
@@ -274,14 +273,12 @@ def devicegroup_delete_invalidates_cache(cls, organization_id):
         cls._invalidate_from_queryset(qs)
 
     @classmethod
-    def certificate_delete_invalidates_cache(cls, organization_id, common_name):
-        try:
-            assert common_name
-            org_slug = Organization.objects.only("slug").get(id=organization_id).slug
-        except (AssertionError, Organization.DoesNotExist):
+    def certificate_delete_invalidates_cache(cls, common_name, organization_slug=None):
+        if not common_name:
             return
         cls.get_device_group.invalidate(cls, "", common_name)
-        cls.get_device_group.invalidate(cls, org_slug, common_name)
+        if organization_slug:
+            cls.get_device_group.invalidate(cls, organization_slug, common_name)
 
 
 template_list = TemplateListCreateView.as_view()

openwisp_controller/config/apps.py

@@ -2,13 +2,7 @@
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.db.models import Case, Count, When
-from django.db.models.signals import (
-    m2m_changed,
-    post_delete,
-    post_save,
-    pre_delete,
-    pre_save,
-)
+from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save
 from django.urls import register_converter
 from django.utils.translation import gettext_lazy as _
 from openwisp_notifications.types import (
@@ -53,12 +47,114 @@ def ready(self, *args, **kwargs):
         self.connect_signals()
         self.register_notification_types()
         self.add_ignore_notification_widget()
-        self.enable_cache_invalidation()
+        self.connect_related_changes_handlers()
+        self.connect_cache_dependencies()
         self.register_dashboard_charts()
         self.register_menu_groups()
         self.notification_cache_update()
         connect_whois_handlers()
 
+    def connect_cache_dependencies(self):
+        """
+        Wires the declarative cache-invalidation dependencies.
+
+        Models that own a cached value declare their related-change
+        dependencies in ``get_cache_dependencies`` (see
+        ``CacheInvalidationMixin``).
+        Caches that are not owned by a model (controller view caches and device
+        group caches) are declared here. Connecting all of them in one place
+        replaces the cache-invalidation ``signal.connect()`` calls that were
+        previously scattered across the codebase.
+        """
+        from .base.cache import CacheDependency, _resolve_pk_snapshot
+        from .controller.views import DeviceChecksumView
+        from .handlers import (
+            devicegroup_delete_handler,
+            invalidate_devicegroup_cache_change_handler,
+            organization_disabled_handler,
+        )
+
+        # Model-owned checksum caches (declared on the models themselves).
+        self.config_model.register_cache_dependencies()
+        self.vpn_model.register_cache_dependencies()
+
+        dependencies = [
+            # DeviceChecksumView caches are invalidated when a device is created,
+            # updated, deleted or when its config is deactivated.
+            CacheDependency(
+                source=self.device_model,
+                signal="post_save",
+                on_create=True,
+                target=DeviceChecksumView.invalidate_get_device_cache,
+            ),
+            # Deferred to commit so a concurrent request cannot repopulate the
+            # cache with a device that is about to be (or was just) deleted.
+            # ``post_delete`` + ``_resolve_pk_snapshot`` because Django clears
+            # ``instance.pk`` on deleted instances before the deferred
+            # on_commit callback runs (see ``_resolve_pk_snapshot``).
+            CacheDependency(
+                source=self.device_model,
+                signal="post_delete",
+                resolve=_resolve_pk_snapshot,
+                target=DeviceChecksumView.invalidate_get_device_cache,
+            ),
+            CacheDependency(
+                signal_obj=config_deactivated,
+                name="config_deactivated",
+                target=(
+                    DeviceChecksumView.invalidate_get_device_cache_on_config_deactivated
+                ),
+            ),
+            # When an organization is disabled, all its devices are deactivated,
+            # so we need to invalidate the controller view caches for all objects.
+            CacheDependency(
+                source=self.org_model,
+                signal="pre_save",
+                on_commit=False,
+                target=organization_disabled_handler,
+            ),
+            # Invalidate the DeviceGroupCommonName cache when a device's group,
+            # a device group, or a certificate changes.
+            CacheDependency(
+                signal_obj=device_group_changed,
+                name="device_group_changed",
+                source=self.device_model,
+                target=invalidate_devicegroup_cache_change_handler,
+            ),
+            CacheDependency(
+                source=self.devicegroup_model,
+                signal="post_save",
+                target=invalidate_devicegroup_cache_change_handler,
+            ),
+            CacheDependency(
+                source=self.cert_model,
+                signal="post_save",
+                target=invalidate_devicegroup_cache_change_handler,
+            ),
+            # Kept synchronous (on_commit=False) so devicegroup_delete_handler
+            # still receives the live instance and can read organization_id
+            # before Django clears instance.pk post-delete. The handler
+            # itself defers the actual task enqueue via transaction.on_commit().
+            CacheDependency(
+                source=self.devicegroup_model,
+                signal="post_delete",
+                on_commit=False,
+                target=devicegroup_delete_handler,
+            ),
+            # Same as above: kept synchronous so the handler can also read
+            # common_name before Django clears instance.pk post-delete.
+            CacheDependency(
+                source=self.cert_model,
+                signal="post_delete",
+                on_commit=False,
+                target=devicegroup_delete_handler,
+            ),
+        ]
+        for dependency in dependencies:
+            dependency.connect(
+                dispatch_uid=dependency.build_dispatch_uid("cache_invalidation.app")
+            )
+
     def __setmodels__(self):
         self.device_model = load_model("config", "Device")
         self.template_model = load_model("config", "Template")
@@ -125,11 +221,6 @@ def connect_signals(self):
             sender=self.vpn_model,
             dispatch_uid="vpn.post_delete",
         )
-        post_save.connect(
-            self.config_model.certificate_updated,
-            sender=self.cert_model,
-            dispatch_uid="cert_update_invalidate_checksum_cache",
-        )
         group_templates_changed.connect(
             handlers.devicegroup_templates_change_handler,
             sender=self.devicegroup_model,
@@ -150,11 +241,6 @@ def connect_signals(self):
             sender=self.template_model,
             dispatch_uid="template_pre_save_handler",
         )
-        pre_save.connect(
-            handlers.organization_disabled_handler,
-            sender=self.org_model,
-            dispatch_uid="organization_disabled_pre_save_clear_device_checksum_cache",
-        )
         post_save.connect(
             self.template_model.post_save_handler,
             sender=self.template_model,
@@ -268,76 +354,32 @@ def add_ignore_notification_widget(self):
                 obj_notification_widget,
             )
 
-    def enable_cache_invalidation(self):
+    def connect_related_changes_handlers(self):
         """
-        Triggers the cache invalidation for the
-        device config checksum (view and model method)
+        Connects signal handlers that react to a change in one object by
+        propagating side effects to related objects. These are intentionally
+        kept out of the declarative cache-invalidation engine (see
+        ``connect_cache_dependencies``) because they do more than invalidate a
+        cached value:
+
+        * clearing a device's management IP when its config is deactivated;
+        * re-applying group templates when a device's group changes
+          (``devicegroup_change_handler``);
+        * refreshing the configs of a VPN server's clients when the server
+          changes. ``vpn_server_change_handler`` recomputes each client's
+          checksum and emits ``config_modified`` for it, but only when that
+          checksum actually changed.
         """
-        from .controller.views import DeviceChecksumView, GetVpnView
-        from .handlers import (
-            device_cache_invalidation_handler,
-            devicegroup_change_handler,
-            devicegroup_delete_handler,
-            vpn_server_change_handler,
-        )
+        from .handlers import devicegroup_change_handler, vpn_server_change_handler
 
-        post_save.connect(
-            DeviceChecksumView.invalidate_get_device_cache,
-            sender=self.device_model,
-            dispatch_uid="invalidate_get_device_cache",
-        )
         config_deactivated.connect(
             self.device_model.config_deactivated_clear_management_ip,
             dispatch_uid="config_deactivated_clear_management_ip",
         )
-        config_deactivated.connect(
-            DeviceChecksumView.invalidate_get_device_cache_on_config_deactivated,
-            dispatch_uid="config_deactivated_invalidate_get_device_cache",
-        )
-        # VPN cache invalidation
-        post_save.connect(
-            GetVpnView.invalidate_get_vpn_cache,
-            sender=self.vpn_model,
-            dispatch_uid="invalidate_get_vpn_cache",
-        )
-        pre_delete.connect(
-            GetVpnView.invalidate_get_vpn_cache,
-            sender=self.vpn_model,
-            dispatch_uid="vpn_server_pre_delete_invalidate_get_vpn_cache",
-        )
-        vpn_server_modified.connect(
-            GetVpnView.invalidate_get_vpn_cache,
-            dispatch_uid="vpn_server_modified_invalidate_get_vpn_cache",
-        )
         device_group_changed.connect(
             devicegroup_change_handler,
             sender=self.device_model,
-            dispatch_uid="invalidate_devicegroup_cache_on_device_change",
-        )
-        post_save.connect(
-            devicegroup_change_handler,
-            sender=self.devicegroup_model,
-            dispatch_uid="invalidate_devicegroup_cache_on_devicegroup_change",
-        )
-        post_save.connect(
-            devicegroup_change_handler,
-            sender=self.cert_model,
-            dispatch_uid="invalidate_devicegroup_cache_on_certificate_change",
-        )
-        post_delete.connect(
-            devicegroup_delete_handler,
-            sender=self.devicegroup_model,
-            dispatch_uid="invalidate_devicegroup_cache_on_devicegroup_delete",
-        )
-        post_delete.connect(
-            devicegroup_delete_handler,
-            sender=self.cert_model,
-            dispatch_uid="invalidate_devicegroup_cache_on_certificate_delete",
-        )
-        pre_delete.connect(
-            device_cache_invalidation_handler,
-            sender=self.device_model,
-            dispatch_uid="device.invalidate_cache",
+            dispatch_uid="manage_devicegroup_templates_on_device_change",
         )
         vpn_server_modified.connect(
             vpn_server_change_handler,