فهرست منبع

Fixes #23027: Pin config context cache invalidation to the saving connection

The config-context cache invalidation added in #21025 issued every query
unpinned, so Django resolved each one through DATABASE_ROUTERS instead of
using the alias the triggering signal supplies as `using`. On a deployment
with routers configured, the affected PKs could be read from — and
`_config_context_data` NULLed in — a database other than the one holding the
change which triggered the invalidation, leaving the intended objects with a
stale pre-rendered context while needlessly invalidating unrelated objects
elsewhere. The write also fell outside the transaction the save runs in, so
it was not rolled back with a failed save.

Thread `using` through the invalidation path, following the pattern the scope
propagation handlers adopted in #22922 and #22963:

- extras/cache.py: all three entry points accept `using` and pin the
  chunked_update() NULL-out, the ltree path lookups, the TaggedItem lookups,
  and the Device/VM PK selects to it. transaction.on_commit() is tied to the
  same connection.
- extras/models/configs.py: get_affected_objects() and
  _get_affected_object_filters() accept `using`, pinning the eagerly
  evaluated M2M scope reads and returning querysets bound to the alias.
- extras/signals.py: every invalidation receiver forwards the alias supplied
  by post_save/m2m_changed/pre_delete.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Jeremy Stretch 11 ساعت پیش
والد
کامیت
e7c0061dd4
4فایلهای تغییر یافته به همراه336 افزوده شده و 49 حذف شده
  1. 38 13
      netbox/extras/cache.py
  2. 13 9
      netbox/extras/models/configs.py
  3. 28 23
      netbox/extras/signals.py
  4. 257 4
      netbox/extras/tests/test_configcontext_cache.py

+ 38 - 13
netbox/extras/cache.py

@@ -6,7 +6,7 @@ funnels through this module, so the synchronous NULL-out and background job enqu
 expressed in exactly one place.
 expressed in exactly one place.
 """
 """
 from django.apps import apps
 from django.apps import apps
-from django.db import transaction
+from django.db import router, transaction
 from django.db.models import F, Q
 from django.db.models import F, Q
 
 
 from dcim.models import Device
 from dcim.models import Device
@@ -16,7 +16,7 @@ from utilities.querysets import chunked_update
 from virtualization.models import VirtualMachine
 from virtualization.models import VirtualMachine
 
 
 
 
-def invalidate_config_context_for_objects(model_label, pks):
+def invalidate_config_context_for_objects(model_label, pks, using=None):
     """
     """
     Synchronously NULL the `_config_context_data` cache on the given objects (bumping the
     Synchronously NULL the `_config_context_data` cache on the given objects (bumping the
     generation counter so an in-flight render can't overwrite the invalidation), then enqueue a
     generation counter so an in-flight render can't overwrite the invalidation), then enqueue a
@@ -25,14 +25,31 @@ def invalidate_config_context_for_objects(model_label, pks):
     Args:
     Args:
         model_label: 'dcim.device' or 'virtualization.virtualmachine'.
         model_label: 'dcim.device' or 'virtualization.virtualmachine'.
         pks: Any iterable of object PKs (queryset, list, set, generator). An empty iterable is a no-op.
         pks: Any iterable of object PKs (queryset, list, set, generator). An empty iterable is a no-op.
+        using: The database alias to pin the invalidation to. Callers pass the alias supplied by the
+            signal which triggered the invalidation, so that the UPDATE lands in the same database
+            (and the same transaction) as the change which necessitated it. None defers to the
+            router, as an unpinned query would.
     """
     """
     pks = list(pks)
     pks = list(pks)
     if not pks:
     if not pks:
         return
         return
 
 
     Model = apps.get_model(model_label)
     Model = apps.get_model(model_label)
+    # Resolve the alias once, so that the UPDATE below and the on_commit() callback which follows
+    # it are bound to the same database. Left as None the two would diverge: an unpinned queryset
+    # consults the router, but transaction.on_commit() does not -- it attaches to 'default' -- so
+    # on a deployment whose router writes elsewhere the callback would be registered against a
+    # connection other than the one being written.
+    #
+    # This also decides which connection's commit the enqueue waits on, so it is not strictly an
+    # improvement on the previous 'default' binding for every caller: one which passes no alias
+    # while holding a transaction opened on 'default' (rather than on the router's write alias)
+    # leaves no atomic block open on the resolved alias, and on_commit() then runs the callback
+    # immediately rather than deferring it. Every caller in NetBox supplies `using`, and the
+    # generic views open their atomic block on router.db_for_write(model), so the two agree there.
+    using = using or router.db_for_write(Model)
     updated = chunked_update(
     updated = chunked_update(
-        Model.objects.filter(pk__in=pks),
+        Model.objects.using(using).filter(pk__in=pks),
         _config_context_data=None,
         _config_context_data=None,
         _config_context_generation=F('_config_context_generation') + 1,
         _config_context_generation=F('_config_context_generation') + 1,
     )
     )
@@ -53,22 +70,25 @@ def invalidate_config_context_for_objects(model_label, pks):
     # remain correct in the interim because get_config_context() renders on demand when the cache is
     # remain correct in the interim because get_config_context() renders on demand when the cache is
     # NULL; the sweep only restores the pre-rendered fast path.)
     # NULL; the sweep only restores the pre-rendered fast path.)
     transaction.on_commit(
     transaction.on_commit(
-        lambda: RenderConfigContextJob.enqueue_once(instance=None)
+        lambda: RenderConfigContextJob.enqueue_once(instance=None),
+        using=using,
     )
     )
 
 
 
 
-def invalidate_config_context_for_configcontext(configcontext):
+def invalidate_config_context_for_configcontext(configcontext, using=None):
     """
     """
-    Invalidate caches for all objects currently in scope for the given ConfigContext.
+    Invalidate caches for all objects currently in scope for the given ConfigContext. `using` is
+    the database alias to pin every query to (see invalidate_config_context_for_objects()).
     """
     """
-    for queryset in configcontext.get_affected_objects():
+    for queryset in configcontext.get_affected_objects(using=using):
         invalidate_config_context_for_objects(
         invalidate_config_context_for_objects(
             queryset.model._meta.label_lower,
             queryset.model._meta.label_lower,
             queryset.values_list('pk', flat=True),
             queryset.values_list('pk', flat=True),
+            using=using,
         )
         )
 
 
 
 
-def invalidate_for_scope_delta(scope_field, scope_pks):
+def invalidate_for_scope_delta(scope_field, scope_pks, using=None):
     """
     """
     Invalidate the cache of every Device/VirtualMachine that is matchable via the given scope
     Invalidate the cache of every Device/VirtualMachine that is matchable via the given scope
     items, regardless of which ConfigContext those items belong to. Used when items are removed
     items, regardless of which ConfigContext those items belong to. Used when items are removed
@@ -77,6 +97,7 @@ def invalidate_for_scope_delta(scope_field, scope_pks):
 
 
     `scope_field` is the ConfigContext M2M attribute name ('sites', 'regions', 'tags', ...).
     `scope_field` is the ConfigContext M2M attribute name ('sites', 'regions', 'tags', ...).
     `scope_pks` is the iterable of PKs of scope items that were removed/cleared.
     `scope_pks` is the iterable of PKs of scope items that were removed/cleared.
+    `using` is the database alias to pin every query to (see invalidate_config_context_for_objects()).
     """
     """
     scope_pks = list(scope_pks or ())
     scope_pks = list(scope_pks or ())
     if not scope_pks:
     if not scope_pks:
@@ -108,7 +129,7 @@ def invalidate_for_scope_delta(scope_field, scope_pks):
         app, model_name, object_path = nested_attrs[scope_field]
         app, model_name, object_path = nested_attrs[scope_field]
         Model = apps.get_model(app, model_name)
         Model = apps.get_model(app, model_name)
         subtree_q = Q()
         subtree_q = Q()
-        for path in Model.objects.filter(pk__in=scope_pks).values_list('path', flat=True):
+        for path in Model.objects.using(using).filter(pk__in=scope_pks).values_list('path', flat=True):
             subtree_q |= Q(**{f'{object_path}__descendant_or_equal': path})
             subtree_q |= Q(**{f'{object_path}__descendant_or_equal': path})
         if not subtree_q:
         if not subtree_q:
             return
             return
@@ -121,12 +142,12 @@ def invalidate_for_scope_delta(scope_field, scope_pks):
         if scope_field != 'device_types':
         if scope_field != 'device_types':
             vm_q = Q(**{attr_path: scope_pks})
             vm_q = Q(**{attr_path: scope_pks})
     elif scope_field == 'tags':
     elif scope_field == 'tags':
-        device_tagged = TaggedItem.objects.filter(
+        device_tagged = TaggedItem.objects.using(using).filter(
             tag_id__in=scope_pks,
             tag_id__in=scope_pks,
             content_type__app_label='dcim',
             content_type__app_label='dcim',
             content_type__model='device',
             content_type__model='device',
         ).values_list('object_id', flat=True)
         ).values_list('object_id', flat=True)
-        vm_tagged = TaggedItem.objects.filter(
+        vm_tagged = TaggedItem.objects.using(using).filter(
             tag_id__in=scope_pks,
             tag_id__in=scope_pks,
             content_type__app_label='virtualization',
             content_type__app_label='virtualization',
             content_type__model='virtualmachine',
             content_type__model='virtualmachine',
@@ -138,9 +159,13 @@ def invalidate_for_scope_delta(scope_field, scope_pks):
 
 
     if device_q is not None:
     if device_q is not None:
         invalidate_config_context_for_objects(
         invalidate_config_context_for_objects(
-            'dcim.device', Device.objects.filter(device_q).values_list('pk', flat=True)
+            'dcim.device',
+            Device.objects.using(using).filter(device_q).values_list('pk', flat=True),
+            using=using,
         )
         )
     if vm_q is not None:
     if vm_q is not None:
         invalidate_config_context_for_objects(
         invalidate_config_context_for_objects(
-            'virtualization.virtualmachine', VirtualMachine.objects.filter(vm_q).values_list('pk', flat=True)
+            'virtualization.virtualmachine',
+            VirtualMachine.objects.using(using).filter(vm_q).values_list('pk', flat=True),
+            using=using,
         )
         )

+ 13 - 9
netbox/extras/models/configs.py

@@ -220,28 +220,32 @@ class ConfigContext(SyncedDataMixin, CloningMixin, CustomLinksMixin, OwnerMixin,
         self.data = self.data_file.get_data()
         self.data = self.data_file.get_data()
     sync_data.alters_data = True
     sync_data.alters_data = True
 
 
-    def get_affected_objects(self):
+    def get_affected_objects(self, using=None):
         """
         """
         Return a (device_qs, vm_qs) tuple of all Devices and VirtualMachines that fall within this
         Return a (device_qs, vm_qs) tuple of all Devices and VirtualMachines that fall within this
         ConfigContext's scope. This is the inverse of ConfigContextQuerySet.get_for_object().
         ConfigContext's scope. This is the inverse of ConfigContextQuerySet.get_for_object().
         Used to determine which pre-rendered context caches must be invalidated when this
         Used to determine which pre-rendered context caches must be invalidated when this
         ConfigContext changes.
         ConfigContext changes.
+
+        `using` pins every query (both the scope lookups and the returned querysets) to the given
+        database alias; None defers to the router, as an unpinned query would.
         """
         """
         from dcim.models import Device
         from dcim.models import Device
         from virtualization.models import VirtualMachine
         from virtualization.models import VirtualMachine
 
 
-        device_q, vm_q = self._get_affected_object_filters()
+        device_q, vm_q = self._get_affected_object_filters(using=using)
         return (
         return (
-            Device.objects.filter(device_q),
-            VirtualMachine.objects.filter(vm_q),
+            Device.objects.using(using).filter(device_q),
+            VirtualMachine.objects.using(using).filter(vm_q),
         )
         )
 
 
-    def _get_affected_object_filters(self):
+    def _get_affected_object_filters(self, using=None):
         """
         """
         Build the Q expressions matching Devices and VirtualMachines in this context's scope.
         Build the Q expressions matching Devices and VirtualMachines in this context's scope.
         Returns (device_q, vm_q). Does NOT consider `is_active` — callers that need that should
         Returns (device_q, vm_q). Does NOT consider `is_active` — callers that need that should
         check it separately. For invalidation purposes, we want the scope set regardless of
         check it separately. For invalidation purposes, we want the scope set regardless of
         whether the context is currently active (toggling is_active also requires invalidation).
         whether the context is currently active (toggling is_active also requires invalidation).
+        `using` pins the scope lookups to the given database alias.
         """
         """
         from extras.models.tags import TaggedItem
         from extras.models.tags import TaggedItem
 
 
@@ -251,7 +255,7 @@ class ConfigContext(SyncedDataMixin, CloningMixin, CustomLinksMixin, OwnerMixin,
             # the forward `<object>__path__ancestor_or_equal` match in ConfigContextQuerySet: there
             # the forward `<object>__path__ancestor_or_equal` match in ConfigContextQuerySet: there
             # a CC's node must be an ancestor of the object's node; here the object's node must fall
             # a CC's node must be an ancestor of the object's node; here the object's node must fall
             # within a CC node's subtree. Returns None if the m2m is empty (no scope restriction).
             # within a CC node's subtree. Returns None if the m2m is empty (no scope restriction).
-            paths = list(m2m.values_list('path', flat=True))
+            paths = list(m2m.using(using).values_list('path', flat=True))
             if not paths:
             if not paths:
                 return None
                 return None
             q = Q()
             q = Q()
@@ -260,7 +264,7 @@ class ConfigContext(SyncedDataMixin, CloningMixin, CustomLinksMixin, OwnerMixin,
             return q
             return q
 
 
         def _direct_pks(m2m):
         def _direct_pks(m2m):
-            pks = list(m2m.values_list('pk', flat=True))
+            pks = list(m2m.using(using).values_list('pk', flat=True))
             return pks or None
             return pks or None
 
 
         # Shared filters (applicable to both Device and VirtualMachine)
         # Shared filters (applicable to both Device and VirtualMachine)
@@ -312,12 +316,12 @@ class ConfigContext(SyncedDataMixin, CloningMixin, CustomLinksMixin, OwnerMixin,
             vm_q &= Q(pk__in=())
             vm_q &= Q(pk__in=())
 
 
         if tag_pks is not None:
         if tag_pks is not None:
-            device_tagged = TaggedItem.objects.filter(
+            device_tagged = TaggedItem.objects.using(using).filter(
                 tag_id__in=tag_pks,
                 tag_id__in=tag_pks,
                 content_type__app_label='dcim',
                 content_type__app_label='dcim',
                 content_type__model='device',
                 content_type__model='device',
             ).values_list('object_id', flat=True)
             ).values_list('object_id', flat=True)
-            vm_tagged = TaggedItem.objects.filter(
+            vm_tagged = TaggedItem.objects.using(using).filter(
                 tag_id__in=tag_pks,
                 tag_id__in=tag_pks,
                 content_type__app_label='virtualization',
                 content_type__app_label='virtualization',
                 content_type__model='virtualmachine',
                 content_type__model='virtualmachine',

+ 28 - 23
netbox/extras/signals.py

@@ -113,26 +113,26 @@ def validate_assigned_tags(sender, instance, action, model, pk_set, **kwargs):
 #
 #
 
 
 @receiver(post_save, sender=ConfigContext)
 @receiver(post_save, sender=ConfigContext)
-def invalidate_on_configcontext_save(sender, instance, **kwargs):
+def invalidate_on_configcontext_save(sender, instance, using=None, **kwargs):
     """
     """
     Whenever a ConfigContext's scalar fields change (e.g. `data`, `weight`, `is_active`),
     Whenever a ConfigContext's scalar fields change (e.g. `data`, `weight`, `is_active`),
     invalidate the caches of all Devices/VMs currently in scope. M2M scope changes are handled
     invalidate the caches of all Devices/VMs currently in scope. M2M scope changes are handled
     separately by invalidate_on_configcontext_m2m_change().
     separately by invalidate_on_configcontext_m2m_change().
     """
     """
-    invalidate_config_context_for_configcontext(instance)
+    invalidate_config_context_for_configcontext(instance, using=using)
 
 
 
 
 @receiver(pre_delete, sender=ConfigContext)
 @receiver(pre_delete, sender=ConfigContext)
-def invalidate_on_configcontext_delete(sender, instance, **kwargs):
+def invalidate_on_configcontext_delete(sender, instance, using=None, **kwargs):
     """
     """
     Before a ConfigContext is deleted, invalidate the caches of all Devices/VMs currently in
     Before a ConfigContext is deleted, invalidate the caches of all Devices/VMs currently in
     scope. The scope is still readable here (pre_delete fires before the row and its M2M rows
     scope. The scope is still readable here (pre_delete fires before the row and its M2M rows
     are removed).
     are removed).
     """
     """
-    invalidate_config_context_for_configcontext(instance)
+    invalidate_config_context_for_configcontext(instance, using=using)
 
 
 
 
-def invalidate_on_configcontext_m2m_change(sender, instance, action, pk_set, scope_field, **kwargs):
+def invalidate_on_configcontext_m2m_change(sender, instance, action, pk_set, scope_field, using=None, **kwargs):
     """
     """
     Whenever a ConfigContext's scope M2M changes, invalidate the caches of all Devices/VMs that
     Whenever a ConfigContext's scope M2M changes, invalidate the caches of all Devices/VMs that
     were or now are in scope.
     were or now are in scope.
@@ -149,11 +149,11 @@ def invalidate_on_configcontext_m2m_change(sender, instance, action, pk_set, sco
         return
         return
 
 
     # Always invalidate based on the current (post-change) scope.
     # Always invalidate based on the current (post-change) scope.
-    invalidate_config_context_for_configcontext(instance)
+    invalidate_config_context_for_configcontext(instance, using=using)
 
 
     # For post_remove, also invalidate devices/VMs that matched via the removed scope items.
     # For post_remove, also invalidate devices/VMs that matched via the removed scope items.
     if action == 'post_remove' and pk_set:
     if action == 'post_remove' and pk_set:
-        invalidate_for_scope_delta(scope_field, pk_set)
+        invalidate_for_scope_delta(scope_field, pk_set, using=using)
 
 
 
 
 def _connect_configcontext_m2m_handlers():
 def _connect_configcontext_m2m_handlers():
@@ -166,13 +166,14 @@ def _connect_configcontext_m2m_handlers():
         field_name = m2m_field.name
         field_name = m2m_field.name
         through = getattr(ConfigContext, field_name).through
         through = getattr(ConfigContext, field_name).through
 
 
-        def _handler(sender, instance, action, pk_set, _field=field_name, **kwargs):
+        def _handler(sender, instance, action, pk_set, using=None, _field=field_name, **kwargs):
             invalidate_on_configcontext_m2m_change(
             invalidate_on_configcontext_m2m_change(
                 sender=sender,
                 sender=sender,
                 instance=instance,
                 instance=instance,
                 action=action,
                 action=action,
                 pk_set=pk_set,
                 pk_set=pk_set,
                 scope_field=_field,
                 scope_field=_field,
+                using=using,
                 **kwargs,
                 **kwargs,
             )
             )
 
 
@@ -204,11 +205,11 @@ def _changed_fields(instance, fields):
 def _make_object_save_handler(model_label):
 def _make_object_save_handler(model_label):
     fields = CC_FIELDS_BY_MODEL[model_label]
     fields = CC_FIELDS_BY_MODEL[model_label]
 
 
-    def _handler(sender, instance, created, **kwargs):
+    def _handler(sender, instance, created, using=None, **kwargs):
         # On creation, enqueue a render so the new object's cache is warmed promptly (there is no
         # On creation, enqueue a render so the new object's cache is warmed promptly (there is no
         # recurring sweep). On update, only invalidate when a scope-relevant field actually changed.
         # recurring sweep). On update, only invalidate when a scope-relevant field actually changed.
         if created or _changed_fields(instance, fields):
         if created or _changed_fields(instance, fields):
-            invalidate_config_context_for_objects(model_label, [instance.pk])
+            invalidate_config_context_for_objects(model_label, [instance.pk], using=using)
 
 
     return _handler
     return _handler
 
 
@@ -225,7 +226,7 @@ _connect_object_save_handlers()
 
 
 
 
 @receiver(m2m_changed, sender=TaggedItem)
 @receiver(m2m_changed, sender=TaggedItem)
-def invalidate_on_device_vm_tag_change(sender, instance, action, **kwargs):
+def invalidate_on_device_vm_tag_change(sender, instance, action, using=None, **kwargs):
     """
     """
     When tags are added or removed on a Device/VM, invalidate that object's cache.
     When tags are added or removed on a Device/VM, invalidate that object's cache.
     """
     """
@@ -235,9 +236,9 @@ def invalidate_on_device_vm_tag_change(sender, instance, action, **kwargs):
     from virtualization.models import VirtualMachine
     from virtualization.models import VirtualMachine
 
 
     if isinstance(instance, Device):
     if isinstance(instance, Device):
-        invalidate_config_context_for_objects('dcim.device', [instance.pk])
+        invalidate_config_context_for_objects('dcim.device', [instance.pk], using=using)
     elif isinstance(instance, VirtualMachine):
     elif isinstance(instance, VirtualMachine):
-        invalidate_config_context_for_objects('virtualization.virtualmachine', [instance.pk])
+        invalidate_config_context_for_objects('virtualization.virtualmachine', [instance.pk], using=using)
 
 
 
 
 # Upstream object changes that affect ConfigContext matching even when the Device/VM itself is
 # Upstream object changes that affect ConfigContext matching even when the Device/VM itself is
@@ -251,7 +252,7 @@ def invalidate_on_device_vm_tag_change(sender, instance, action, **kwargs):
 
 
 
 
 def _make_direct_upstream_handler(fields, device_lookup, vm_lookup):
 def _make_direct_upstream_handler(fields, device_lookup, vm_lookup):
-    def _handler(sender, instance, created, **kwargs):
+    def _handler(sender, instance, created, using=None, **kwargs):
         if created or not _changed_fields(instance, fields):
         if created or not _changed_fields(instance, fields):
             return
             return
         from dcim.models import Device
         from dcim.models import Device
@@ -260,19 +261,21 @@ def _make_direct_upstream_handler(fields, device_lookup, vm_lookup):
         if device_lookup:
         if device_lookup:
             invalidate_config_context_for_objects(
             invalidate_config_context_for_objects(
                 'dcim.device',
                 'dcim.device',
-                Device.objects.filter(**{device_lookup: instance.pk}).values_list('pk', flat=True),
+                Device.objects.using(using).filter(**{device_lookup: instance.pk}).values_list('pk', flat=True),
+                using=using,
             )
             )
         if vm_lookup:
         if vm_lookup:
             invalidate_config_context_for_objects(
             invalidate_config_context_for_objects(
                 'virtualization.virtualmachine',
                 'virtualization.virtualmachine',
-                VirtualMachine.objects.filter(**{vm_lookup: instance.pk}).values_list('pk', flat=True),
+                VirtualMachine.objects.using(using).filter(**{vm_lookup: instance.pk}).values_list('pk', flat=True),
+                using=using,
             )
             )
 
 
     return _handler
     return _handler
 
 
 
 
 def _make_reparent_handler(device_attr, vm_attr):
 def _make_reparent_handler(device_attr, vm_attr):
-    def _handler(sender, instance, created, **kwargs):
+    def _handler(sender, instance, created, using=None, **kwargs):
         if created or not _changed_fields(instance, ('parent_id',)):
         if created or not _changed_fields(instance, ('parent_id',)):
             return
             return
         from dcim.models import Device
         from dcim.models import Device
@@ -284,22 +287,24 @@ def _make_reparent_handler(device_attr, vm_attr):
         # (post-move) subtree. The set of node PKs is invariant under a move; only their paths
         # (post-move) subtree. The set of node PKs is invariant under a move; only their paths
         # shift, so this matches the same Devices/VMs regardless of timing.
         # shift, so this matches the same Devices/VMs regardless of timing.
         model = type(instance)
         model = type(instance)
-        node_path = model.objects.filter(pk=instance.pk).values_list('path', flat=True).first()
+        node_path = model.objects.using(using).filter(pk=instance.pk).values_list('path', flat=True).first()
         if node_path is None:
         if node_path is None:
             return
             return
         subtree_pks = list(
         subtree_pks = list(
-            model.objects.filter(path__descendant_or_equal=node_path).values_list('pk', flat=True)
+            model.objects.using(using).filter(path__descendant_or_equal=node_path).values_list('pk', flat=True)
         )
         )
 
 
         if device_attr:
         if device_attr:
             invalidate_config_context_for_objects(
             invalidate_config_context_for_objects(
                 'dcim.device',
                 'dcim.device',
-                Device.objects.filter(**{device_attr: subtree_pks}).values_list('pk', flat=True),
+                Device.objects.using(using).filter(**{device_attr: subtree_pks}).values_list('pk', flat=True),
+                using=using,
             )
             )
         if vm_attr:
         if vm_attr:
             invalidate_config_context_for_objects(
             invalidate_config_context_for_objects(
                 'virtualization.virtualmachine',
                 'virtualization.virtualmachine',
-                VirtualMachine.objects.filter(**{vm_attr: subtree_pks}).values_list('pk', flat=True),
+                VirtualMachine.objects.using(using).filter(**{vm_attr: subtree_pks}).values_list('pk', flat=True),
+                using=using,
             )
             )
 
 
     return _handler
     return _handler
@@ -362,8 +367,8 @@ _connect_upstream_handlers()
 # whose FK is about to be nulled.
 # whose FK is about to be nulled.
 
 
 def _make_upstream_delete_handler(scope_field):
 def _make_upstream_delete_handler(scope_field):
-    def _handler(sender, instance, **kwargs):
-        invalidate_for_scope_delta(scope_field, [instance.pk])
+    def _handler(sender, instance, using=None, **kwargs):
+        invalidate_for_scope_delta(scope_field, [instance.pk], using=using)
 
 
     return _handler
     return _handler
 
 

+ 257 - 4
netbox/extras/tests/test_configcontext_cache.py

@@ -1,16 +1,22 @@
 from unittest import mock
 from unittest import mock
 
 
-from django.db import connection
-from django.db.models import F
-from django.test import TestCase
+from django.db import connection, router
+from django.db.models import F, Q
+from django.test import TestCase, override_settings
 from django.test.utils import CaptureQueriesContext
 from django.test.utils import CaptureQueriesContext
 
 
 from core.models import Job
 from core.models import Job
 from dcim.models import Device, DeviceRole, DeviceType, Location, Manufacturer, Platform, Region, Site, SiteGroup
 from dcim.models import Device, DeviceRole, DeviceType, Location, Manufacturer, Platform, Region, Site, SiteGroup
-from extras.cache import invalidate_config_context_for_objects
+from extras.cache import (
+    invalidate_config_context_for_configcontext,
+    invalidate_config_context_for_objects,
+    invalidate_for_scope_delta,
+)
 from extras.jobs import RenderConfigContextJob
 from extras.jobs import RenderConfigContextJob
 from extras.models import ConfigContext, Tag
 from extras.models import ConfigContext, Tag
+from extras.signals import _make_direct_upstream_handler, _make_reparent_handler
 from tenancy.models import Tenant, TenantGroup
 from tenancy.models import Tenant, TenantGroup
+from utilities.testing import PinnedConnectionRouter
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
 
 
 
 
@@ -707,3 +713,250 @@ class ConditionalConfigContextAnnotationTest(TestCase):
         self.assertEqual(rows[self.cold.pk], {'a': 1, 'b': 2})
         self.assertEqual(rows[self.cold.pk], {'a': 1, 'b': 2})
         # A single query backs the whole page — no per-object fallback to get_for_object().
         # A single query backs the whole page — no per-object fallback to get_for_object().
         self.assertEqual(len(ctx.captured_queries), 1)
         self.assertEqual(len(ctx.captured_queries), 1)
+
+
+# Resolving a ConfigContext's affected object set reads every dimension of its scope, whether or
+# not that dimension is populated (see ConfigContext._get_affected_object_filters). A router which
+# named only Device and VirtualMachine would therefore leave those scope lookups unchecked, so the
+# ConfigContext-driven tests below name them all.
+CC_SCOPE_MODELS = (
+    Cluster,
+    ClusterGroup,
+    ClusterType,
+    DeviceRole,
+    DeviceType,
+    Location,
+    Platform,
+    Region,
+    Site,
+    SiteGroup,
+    Tenant,
+    TenantGroup,
+)
+
+
+class ConfigContextInvalidationRoutingTest(TestCase):
+    """
+    Every query the invalidation makes must be issued against the connection the triggering object
+    was saved on — the alias the signal supplies as `using` — rather than being resolved anew by
+    DATABASE_ROUTERS. A router which resolved them elsewhere would read the affected PKs from, and
+    NULL the cache in, a database other than the one holding the triggering change.
+
+    PinnedConnectionRouter raises on any unpinned read or write of the models it is given. Each
+    test omits the model being saved or deleted, as Django routes that operation itself.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        manufacturer = Manufacturer.objects.create(name='Mfr', slug='mfr')
+        cls.devicetype = DeviceType.objects.create(manufacturer=manufacturer, model='DT', slug='dt')
+        cls.role = DeviceRole.objects.create(name='Role', slug='role')
+        cls.region = Region.objects.create(name='Region', slug='region')
+        cls.site_a = Site.objects.create(name='Site A', slug='site-a', region=cls.region)
+        cls.site_b = Site.objects.create(name='Site B', slug='site-b')
+        cls.platform = Platform.objects.create(name='Platform', slug='platform')
+        cls.location = Location.objects.create(name='Location', slug='location', site=cls.site_a)
+        cls.device = Device.objects.create(
+            name='Device',
+            device_type=cls.devicetype,
+            role=cls.role,
+            site=cls.site_a,
+            location=cls.location,
+            platform=cls.platform,
+        )
+        cls.cluster_type = ClusterType.objects.create(name='Cluster Type', slug='cluster-type')
+        cls.cluster = Cluster.objects.create(name='Cluster', type=cls.cluster_type, scope=cls.site_a)
+        cls.vm = VirtualMachine.objects.create(
+            name='VM', cluster=cls.cluster, role=cls.role, platform=cls.platform
+        )
+
+    def test_helper_pins_update_to_given_connection(self):
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine)]):
+            invalidate_config_context_for_objects('dcim.device', [self.device.pk], using='default')
+
+    def test_helper_resolves_one_alias_for_update_and_enqueue(self):
+        # Called without an alias, the UPDATE falls to the router but transaction.on_commit()
+        # would fall to 'default', which need not be the same database: the callback would then
+        # be attached to a connection other than the one being written, and could fire before
+        # the UPDATE it waits on. Both must be bound to the alias the router chooses.
+        with mock.patch('extras.cache.transaction.on_commit') as on_commit:
+            invalidate_config_context_for_objects('dcim.device', [self.device.pk])
+
+        self.assertEqual(on_commit.call_args.kwargs['using'], router.db_for_write(Device))
+
+    def test_location_site_change_pins_device_query(self):
+        location = Location.objects.get(pk=self.location.pk)
+        location.site_id = self.site_b.pk
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device)]):
+            location.save()
+
+    def test_cluster_scope_change_pins_vm_query(self):
+        cluster = Cluster.objects.get(pk=self.cluster.pk)
+        cluster.scope_id = self.site_b.pk
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(VirtualMachine)]):
+            cluster.save()
+
+    def test_region_reparent_pins_device_and_vm_queries(self):
+        parent = Region.objects.create(name='Parent', slug='parent')
+        region = Region.objects.get(pk=self.region.pk)
+        region.parent = parent
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine)]):
+            region.save()
+
+    def test_reparent_handler_pins_subtree_queries_to_given_connection(self):
+        # The reparent handler resolves the moved node's post-move path and the PKs of its
+        # subtree before it can name the affected objects, and both reads must follow the
+        # saving connection. The handler is invoked directly so that the reparented model can
+        # be named in the router: saving the Region under it would trip on the unpinned cycle
+        # check in the ltree base save, which this handler does not control.
+        parent = Region.objects.create(name='Parent', slug='parent')
+        region = Region.objects.get(pk=self.region.pk)
+        region.parent = parent
+        region.save()
+
+        handler = _make_reparent_handler('site__region__in', 'site__region__in')
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, Region)]):
+            handler(sender=Region, instance=region, created=False, using='default')
+
+    def test_configcontext_save_pins_device_and_vm_queries(self):
+        # Both a nested (ltree) and a direct scope dimension are populated, so that the path
+        # lookup and the PK lookup which resolve them are each exercised.
+        cc = ConfigContext.objects.create(name='CC', weight=100, data={'a': 1})
+        cc.sites.add(self.site_a)
+        cc.regions.add(self.region)
+        cc.data = {'a': 2}
+        with override_settings(
+            DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, *CC_SCOPE_MODELS)]
+        ):
+            cc.save()
+
+    def test_configcontext_delete_pins_device_and_vm_queries(self):
+        cc = ConfigContext.objects.create(name='CC', weight=100, data={'a': 1})
+        cc.sites.add(self.site_a)
+        cc.regions.add(self.region)
+        with override_settings(
+            DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, *CC_SCOPE_MODELS)]
+        ):
+            cc.delete()
+
+    def test_configcontext_m2m_add_pins_device_and_vm_queries(self):
+        cc = ConfigContext.objects.create(name='CC', weight=100, data={'a': 1})
+        with override_settings(
+            DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, *CC_SCOPE_MODELS)]
+        ):
+            cc.sites.add(self.site_a)
+
+    def test_configcontext_m2m_remove_pins_device_and_vm_queries(self):
+        # post_remove additionally resolves the removed scope items via
+        # invalidate_for_scope_delta(), which must be pinned too.
+        cc = ConfigContext.objects.create(name='CC', weight=100, data={'a': 1})
+        cc.sites.add(self.site_a)
+        with override_settings(
+            DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, *CC_SCOPE_MODELS)]
+        ):
+            cc.sites.remove(self.site_a)
+
+    def test_configcontext_nested_m2m_remove_pins_scope_delta_queries(self):
+        # A nested (ltree) scope dimension resolves the removed items' paths as well. Region is
+        # named so that read is checked rather than merely performed.
+        cc = ConfigContext.objects.create(name='CC', weight=100, data={'a': 1})
+        cc.regions.add(self.region)
+        with override_settings(
+            DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine, *CC_SCOPE_MODELS)]
+        ):
+            cc.regions.remove(self.region)
+
+    def test_upstream_delete_pins_device_and_vm_queries(self):
+        # Platform is a SET_NULL feeder on both Device and VirtualMachine.
+        platform = Platform.objects.create(name='Spare', slug='spare')
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device, VirtualMachine)]):
+            platform.delete()
+
+    def test_device_tag_change_pins_device_query(self):
+        tag = Tag.objects.create(name='Tag', slug='tag')
+        with override_settings(DATABASE_ROUTERS=[PinnedConnectionRouter(Device)]):
+            self.device.tags.add(tag)
+
+
+# An alias which is deliberately absent from DATABASES. Every query issued against it would raise
+# ConnectionDoesNotExist, so the tests below stop at the boundary where the alias is handed off
+# (the queryset's `_db`, or the `using` kwarg) rather than executing anything.
+OTHER_ALIAS = 'other'
+
+
+class ConfigContextInvalidationAliasThreadingTest(TestCase):
+    """
+    ConfigContextInvalidationRoutingTest establishes that no query in the invalidation path is left
+    for DATABASE_ROUTERS to resolve. That is only half of the requirement: a query pinned to the
+    wrong alias consults no router either. These tests close the other half by handing each entry
+    point an alias of their own choosing and asserting that *that* alias is what reaches the
+    queryset and the enqueue — which a hardcoded `.using('default')` would fail.
+
+    The alias names no real connection, so nothing here may execute a query against it. Each test
+    patches the boundary immediately below the code under test, leaving the querysets unevaluated.
+    """
+    def test_helper_pins_update_and_enqueue_to_supplied_alias(self):
+        with (
+            mock.patch('extras.cache.chunked_update', return_value=1) as chunked,
+            mock.patch('extras.cache.transaction.on_commit') as on_commit,
+        ):
+            invalidate_config_context_for_objects('dcim.device', [1], using=OTHER_ALIAS)
+
+        self.assertEqual(chunked.call_args.args[0]._db, OTHER_ALIAS)
+        self.assertEqual(on_commit.call_args.kwargs['using'], OTHER_ALIAS)
+
+    def test_helper_pins_update_to_the_alias_it_resolves(self):
+        # Called without an alias, the UPDATE and the enqueue must still agree: the alias the
+        # helper resolves for on_commit() (asserted by the routing test above) is the one the
+        # UPDATE has to carry, or the callback waits on a connection other than the one written.
+        with (
+            mock.patch('extras.cache.chunked_update', return_value=1) as chunked,
+            mock.patch('extras.cache.transaction.on_commit') as on_commit,
+        ):
+            invalidate_config_context_for_objects('dcim.device', [1])
+
+        self.assertEqual(chunked.call_args.args[0]._db, router.db_for_write(Device))
+        self.assertEqual(chunked.call_args.args[0]._db, on_commit.call_args.kwargs['using'])
+
+    def test_configcontext_helper_forwards_alias(self):
+        cc = ConfigContext(name='CC', weight=100, data={})
+        affected = (Device.objects.using(OTHER_ALIAS), VirtualMachine.objects.using(OTHER_ALIAS))
+        with (
+            mock.patch.object(ConfigContext, 'get_affected_objects', return_value=affected) as get_affected,
+            mock.patch('extras.cache.invalidate_config_context_for_objects') as invalidate,
+        ):
+            invalidate_config_context_for_configcontext(cc, using=OTHER_ALIAS)
+
+        self.assertEqual(get_affected.call_args.kwargs['using'], OTHER_ALIAS)
+        self.assertEqual([c.kwargs['using'] for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])
+        self.assertEqual([c.args[1]._db for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])
+
+    def test_scope_delta_pins_pk_selects_to_supplied_alias(self):
+        # 'sites' is a direct (non-nested, non-tag) scope dimension, so the function resolves it
+        # without a query of its own and the Device/VM PK selects are the only reads to check.
+        with mock.patch('extras.cache.invalidate_config_context_for_objects') as invalidate:
+            invalidate_for_scope_delta('sites', [1], using=OTHER_ALIAS)
+
+        self.assertEqual([c.kwargs['using'] for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])
+        self.assertEqual([c.args[1]._db for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])
+
+    def test_get_affected_objects_binds_querysets_to_supplied_alias(self):
+        cc = ConfigContext(name='CC', weight=100, data={})
+        with mock.patch.object(
+            ConfigContext, '_get_affected_object_filters', return_value=(Q(), Q())
+        ) as get_filters:
+            device_qs, vm_qs = cc.get_affected_objects(using=OTHER_ALIAS)
+
+        self.assertEqual(get_filters.call_args.kwargs['using'], OTHER_ALIAS)
+        self.assertEqual(device_qs._db, OTHER_ALIAS)
+        self.assertEqual(vm_qs._db, OTHER_ALIAS)
+
+    def test_upstream_handler_forwards_alias_to_helper(self):
+        # The receivers take their alias from the signal; the handler is invoked directly so that
+        # an alias other than the one the test connection would supply can be threaded through it.
+        handler = _make_direct_upstream_handler(('name',), 'platform_id', 'platform_id')
+        with mock.patch('extras.signals.invalidate_config_context_for_objects') as invalidate:
+            handler(sender=Platform, instance=Platform(pk=1, name='Platform'), created=False, using=OTHER_ALIAS)
+
+        self.assertEqual([c.kwargs['using'] for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])
+        self.assertEqual([c.args[1]._db for c in invalidate.call_args_list], [OTHER_ALIAS, OTHER_ALIAS])