Closes #22485: Break the search signal wiring import cycle (#22744)

Closes #22485

netbox/core/apps.py

@@ -32,6 +32,7 @@ class CoreConfig(AppConfig):
         from core.checks import check_duplicate_indexes, check_postgresql_version, check_redis_version  # noqa: F401
         from netbox import context_managers  # noqa: F401
         from netbox.models.features import register_models
+        from netbox.search import signals as search_signals  # noqa: F401
 
         from . import data_backends, events, search  # noqa: F401
 

netbox/netbox/search/backends.py

@@ -8,7 +8,6 @@ from django.db import DatabaseError, ProgrammingError, transaction
 from django.db.models import F, Q, Window, prefetch_related_objects
 from django.db.models.fields.related import ForeignKey
 from django.db.models.functions import window
-from django.db.models.signals import post_delete, post_save
 from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 from netaddr.core import AddrFormatError
@@ -21,7 +20,6 @@ from utilities.querysets import RestrictedPrefetch
 from utilities.string import title
 
 from . import FieldTypes, LookupTypes, get_indexer
-from .deferred import OP_CACHE, OP_REMOVE, mark_for_deferred_indexing
 
 DEFAULT_LOOKUP_TYPE = LookupTypes.PARTIAL
 MAX_RESULTS = 1000
@@ -64,11 +62,11 @@ class SearchBackend:
         """
         raise NotImplementedError
 
-    # caching_handler() and removal_handler() are the default, synchronous signal receivers connected
+    # caching_handler() and removal_handler() are the default, synchronous signal receivers; they are
-    # to post_save/post_delete at module load. They are internal plumbing for signal dispatch, not a
+    # connected to post_save/post_delete from netbox.search.signals (wired from CoreConfig.ready()).
-    # documented extension point: the public backend contract is cache()/remove()/clear(). A backend
+    # They are internal plumbing for signal dispatch, not a documented extension point: the public
-    # that needs to do something other than index inline (e.g. defer the work) overrides these in its
+    # backend contract is cache()/remove()/clear(). A backend that needs to do something other than
-    # subclass; see CachedValueSearchBackend.
+    # index inline (e.g. defer the work) overrides these in its subclass; see CachedValueSearchBackend.
     def caching_handler(self, sender, instance, created, **kwargs):
         """
         Receiver for the post_save signal, responsible for caching object creation/changes.
@@ -127,10 +125,18 @@ class CachedValueSearchBackend(SearchBackend):
     # the originating routing context is gone, so the alias must be captured here and replayed on the
     # deferred write to keep cache entries in the originating schema (e.g. a branch schema under
     # netbox-branching). Deferral is internal to this backend; the public contract is unchanged.
+    #
+    # mark_for_deferred_indexing() etc. are imported inside each method rather than at module level:
+    # this module's own top would import deferred.py *before* search_backend is defined further down
+    # this same file, and deferred.py (plus jobs.py) need that singleton at their own module level.
+    # A module-level import here would close that loop into a backends -> deferred -> backends
+    # cycle. See #22485.
     def caching_handler(self, sender, instance, created, using=None, **kwargs):
         """
         Receiver for the post_save signal, responsible for caching object creation/changes.
         """
+        from .deferred import OP_CACHE, mark_for_deferred_indexing
+
         # Skip non-cacheable objects without scheduling any deferred work.
         try:
             indexer = get_indexer(instance)
@@ -150,6 +156,8 @@ class CachedValueSearchBackend(SearchBackend):
         """
         Receiver for the post_delete signal, responsible for caching object deletion.
         """
+        from .deferred import OP_REMOVE, mark_for_deferred_indexing
+
         # Skip non-cacheable objects without scheduling any deferred work.
         try:
             indexer = get_indexer(instance)
@@ -440,7 +448,3 @@ def get_backend():
 
 
 search_backend = get_backend()
-
-# Connect handlers to the appropriate model signals
-post_save.connect(search_backend.caching_handler)
-post_delete.connect(search_backend.removal_handler)

netbox/netbox/search/deferred.py

@@ -4,6 +4,8 @@ from django.db import DEFAULT_DB_ALIAS, connections, transaction
 from redis.exceptions import RedisError
 
 from netbox.constants import RQ_QUEUE_DEFAULT
+from netbox.search.backends import search_backend
+from netbox.search.jobs import SearchCacheJob
 from utilities.rqworker import any_workers_for_queue
 
 # This module is internal plumbing for the search signal handlers; nothing here
@@ -157,14 +159,6 @@ def _flush(batch, using):
         groups = remove_groups if op == OP_REMOVE else cache_groups
         groups.setdefault(object_type_id, []).append(pk)
 
-    # Imported here, not at module load, to avoid an import cycle: backends.py
-    # imports this module at module level (for the signal handlers), and
-    # netbox.search.jobs imports the search_backend singleton from backends.py,
-    # which is bound at the bottom of that module. A proper fix is tracked in
-    # #22485.
-    from netbox.search.backends import search_backend
-    from netbox.search.jobs import SearchCacheJob
-
     try:
         # Both the worker-availability check and the job enqueue talk to Redis,
         # and a worker can die between the two. Treat any Redis failure across the

netbox/netbox/search/signals.py

@@ -0,0 +1,31 @@
+"""
+Connects the global search cache's post_save/post_delete receivers.
+
+Wired explicitly from CoreConfig.ready() (core/apps.py) rather than as a side effect of this
+module being imported, so connection happens deterministically at startup instead of depending
+on which of this subsystem's several consumers (netbox.forms.search, netbox.views.misc,
+extras.management.commands.reindex, netbox.search.jobs, core.jobs, dcim.signals) happens to
+import netbox.search first.
+"""
+from django.db.models.signals import post_delete, post_save
+from django.dispatch import receiver
+
+from .backends import search_backend
+
+
+@receiver(post_save)
+def caching_handler(sender, instance, created, **kwargs):
+    """
+    Update the search cache when an object is created or modified. Delegates to whichever
+    backend is configured; see SearchBackend.caching_handler().
+    """
+    search_backend.caching_handler(sender, instance, created=created, **kwargs)
+
+
+@receiver(post_delete)
+def removal_handler(sender, instance, **kwargs):
+    """
+    Remove an object's cached representation when it is deleted. Delegates to whichever
+    backend is configured; see SearchBackend.removal_handler().
+    """
+    search_backend.removal_handler(sender, instance, **kwargs)

netbox/netbox/tests/test_search.py

@@ -741,7 +741,8 @@ class CustomBackendContractTestCase(TransactionTestCase):
         backend = _MinimalSearchBackend()
 
         # Connect the custom backend's (inherited, synchronous) handlers, exactly as
-        # backends.py connects the configured backend at import. Same call site, no type-check.
+        # netbox.search.signals connects the configured backend from CoreConfig.ready(). Same
+        # call site, no type-check.
         post_save.connect(backend.caching_handler, sender=Site)
         post_delete.connect(backend.removal_handler, sender=Site)
         self.addCleanup(post_save.disconnect, backend.caching_handler, sender=Site)
@@ -757,9 +758,10 @@ class CustomBackendContractTestCase(TransactionTestCase):
         self.assertEqual(len(backend.removed), 1)
 
     def test_default_backend_defers_via_same_call_path(self):
-        # The default backend (CachedValueSearchBackend) IS connected at import, and reaches the
+        # The default backend (CachedValueSearchBackend) IS connected via netbox.search.signals, and
-        # SAME caching_handler call path -- but its override defers instead of indexing inline.
+        # reaches the SAME caching_handler call path -- but its override defers instead of indexing
-        # This contrasts with the custom backend above: identical dispatch, polymorphic behavior.
+        # inline. This contrasts with the custom backend above: identical dispatch, polymorphic
+        # behavior.
         with transaction.atomic():
             Site.objects.create(name='Default Defers', slug='default-defers')
             scheduled = scheduled_search_flushes()