Clean up search backends

netbox/extras/management/commands/reindex.py

@@ -19,7 +19,7 @@ class Command(BaseCommand):
                 self.stdout.write(f'Reindexing {app_label}.{name}...', ending="\n")
                 model = idx.model
                 for instance in model.objects.all():
-                    search_backend.cache(model, instance)
+                    search_backend.caching_handler(model, instance)
 
         cache_size = CachedValue.objects.count()
         self.stdout.write(f'Done. Generated {cache_size} cached values', ending="\n")

netbox/extras/migrations/0079_search.py

@@ -25,7 +25,7 @@ class Migration(migrations.Migration):
                 ('object_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to='contenttypes.contenttype')),
             ],
             options={
-                'ordering': ('weight', 'pk'),
+                'ordering': ('weight', 'object_type', 'object_id'),
             },
         ),
     ]

netbox/extras/models/search.py

@@ -34,7 +34,7 @@ class CachedValue(models.Model):
     )
 
     class Meta:
-        ordering = ('weight', 'pk')
+        ordering = ('weight', 'object_type', 'object_id')
 
     def __str__(self):
         return f'{self.object_type} {self.object_id}: {self.field}={self.value}'

netbox/netbox/search/__init__.py

@@ -1,5 +1,18 @@
+from collections import namedtuple
+
+from django.db import models
+
 from extras.registry import registry
 
+ObjectFieldValue = namedtuple('ObjectFieldValue', ('name', 'type', 'weight', 'value'))
+
+
+class FieldTypes:
+    BOOLEAN = 'bool'
+    FLOAT = 'float'
+    INTEGER = 'int'
+    STRING = 'str'
+
 
 class SearchIndex:
     """
@@ -20,15 +33,48 @@ class SearchIndex:
             return cls.category
         return cls.model._meta.app_config.verbose_name
 
+    @staticmethod
+    def get_field_type(instance, field_name):
+        field_cls = instance._meta.get_field(field_name).__class__
+        if issubclass(field_cls, models.BooleanField):
+            return FieldTypes.BOOLEAN
+        if issubclass(field_cls, (models.FloatField, models.DecimalField)):
+            return FieldTypes.FLOAT
+        if issubclass(field_cls, models.IntegerField):
+            return FieldTypes.INTEGER
+        return FieldTypes.STRING
+
+    @staticmethod
+    def get_field_value(instance, field_name):
+        return str(getattr(instance, field_name))
+
     @classmethod
     def to_cache(cls, instance):
-        return [
+        values = []
-            (field, str(getattr(instance, field)), weight)
+        for name, weight in cls.fields:
-            for field, weight in cls.fields
+            type_ = cls.get_field_type(instance, name)
-        ]
+            value = cls.get_field_value(instance, name)
+            values.append(
+                ObjectFieldValue(name, type_, weight, value)
+            )
+
+        return values
+
+
+class SearchResult:
+    """
+    Represents a single result returned by a search backend's search() method.
+    """
+    def __init__(self, obj, field=None, value=None):
+        self.object = obj
+        self.field = field
+        self.value = value
 
 
 def register_search():
+    """
+    Decorator for registering a SearchIndex with a particular model.
+    """
     def _wrapper(cls):
         model = cls.model
         app_label = model._meta.app_label

netbox/netbox/search/backends.py

@@ -4,11 +4,12 @@ from importlib import import_module
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ImproperlyConfigured
-from django.db.models.signals import post_save
+from django.db.models.signals import post_delete, post_save
 
 from extras.models import CachedValue
 from extras.registry import registry
 from netbox.constants import SEARCH_MAX_RESULTS
+from . import SearchResult
 
 # The cache for the initialized backend.
 _backends_cache = {}
@@ -32,8 +33,9 @@ class SearchBackend:
 
     def __init__(self):
 
-        # Connect cache handler to the model post-save signal
+        # Connect handlers to the appropriate model signals
-        post_save.connect(self.cache)
+        post_save.connect(self.caching_handler)
+        post_delete.connect(self.removal_handler)
 
     def get_registry(self):
         r = {}
@@ -64,12 +66,43 @@ class SearchBackend:
         return self._search_choice_options
 
     def search(self, request, value, **kwargs):
-        """Execute a search query for the given value."""
+        """
+        Search cached object representations for the given value.
+        """
         raise NotImplementedError
 
-    @staticmethod
+    @classmethod
-    def cache(sender, instance, **kwargs):
+    def caching_handler(cls, sender, instance, **kwargs):
-        """Create or update the cached copy of an instance."""
+        """
+        Receiver for the post_save signal, responsible for caching object creation/changes.
+        """
+        try:
+            indexer = get_indexer(instance)
+        except KeyError:
+            # No indexer has been registered for this model
+            return
+        data = indexer.to_cache(instance)
+        cls.cache(instance, data)
+
+    @classmethod
+    def removal_handler(cls, sender, instance, **kwargs):
+        """
+        Receiver for the post_delete signal, responsible for caching object deletion.
+        """
+        cls.remove(instance)
+
+    @classmethod
+    def cache(cls, instance, data):
+        """
+        Create or update the cached representation of an instance.
+        """
+        raise NotImplementedError
+
+    @classmethod
+    def remove(cls, instance):
+        """
+        Delete any cached representation of an instance.
+        """
         raise NotImplementedError
 
 
@@ -84,7 +117,9 @@ class FilterSetSearchBackend(SearchBackend):
         search_registry = self.get_registry()
         for obj_type in search_registry.keys():
 
-            queryset = search_registry[obj_type].queryset
+            queryset = getattr(search_registry[obj_type], 'queryset', None)
+            if not queryset:
+                continue
 
             # Restrict the queryset for the current user
             if hasattr(queryset, 'restrict'):
@@ -98,14 +133,18 @@ class FilterSetSearchBackend(SearchBackend):
             queryset = filterset({'q': value}, queryset=queryset).qs[:SEARCH_MAX_RESULTS]
 
             results.extend([
-                {'object': obj}
+                SearchResult(obj) for obj in queryset
-                for obj in queryset
             ])
 
         return results
 
-    @staticmethod
+    @classmethod
-    def cache(sender, instance, **kwargs):
+    def cache(cls, instance, data):
+        # This backend does not utilize a cache
+        pass
+
+    @classmethod
+    def remove(cls, instance):
         # This backend does not utilize a cache
         pass
 
@@ -113,32 +152,30 @@ class FilterSetSearchBackend(SearchBackend):
 class CachedValueSearchBackend(SearchBackend):
 
     def search(self, request, value, **kwargs):
-        return CachedValue.objects.filter(value__icontains=value)
+        return CachedValue.objects.filter(value__icontains=value).prefetch_related('object')
 
-    @staticmethod
+    @classmethod
-    def cache(sender, instance, **kwargs):
+    def cache(cls, instance, data):
-        try:
+        for field in data:
-            indexer = get_indexer(instance)
+            if not field.value:
-        except KeyError:
-            return
-
-        data = indexer.to_cache(instance)
-
-        for field, value, weight in data:
-            if not value:
                 continue
             ct = ContentType.objects.get_for_model(instance)
             CachedValue.objects.update_or_create(
                 defaults={
-                    'value': value,
+                    'value': field.value,
-                    'weight': weight,
+                    'weight': field.weight,
                 },
                 object_type=ct,
                 object_id=instance.pk,
-                field=field,
+                field=field.name,
-                type='text'  # TODO
+                type=field.type
             )
 
+    @classmethod
+    def remove(cls, instance):
+        ct = ContentType.objects.get_for_model(instance)
+        CachedValue.objects.filter(object_type=ct, object_id=instance.pk).delete()
+
 
 def get_backend():
     """Initializes and returns the configured search backend."""