Просмотр исходного кода

Closes #23572: Evaluate only the filter parameters present in a request (#23577)

Jeremy Stretch 7 часов назад
Родитель
Сommit
33c3b9ed9e

+ 1 - 1
.claude/skills/add-model-field/SKILL.md

@@ -406,5 +406,5 @@ Add the new field to the model's documentation page. Include:
 - Real example (adding a JSONField): `git show 5f802bb18` — adds `choice_colors` to CustomFieldChoiceSet across model, forms, filterset, serializer, GraphQL, and tests
 - Real example (adding a JSONField): `git show 5f802bb18` — adds `choice_colors` to CustomFieldChoiceSet across model, forms, filterset, serializer, GraphQL, and tests
 - Panel attrs reference: `netbox/netbox/ui/attrs.py`
 - Panel attrs reference: `netbox/netbox/ui/attrs.py`
 - Panel classes: `netbox/<app>/ui/panels.py`
 - Panel classes: `netbox/<app>/ui/panels.py`
-- Base filterset classes: `netbox/netbox/filtersets.py`
+- Base filterset classes: `netbox/netbox/filtersets/__init__.py`
 - Contributing guide: `docs/development/extending-models.md`
 - Contributing guide: `docs/development/extending-models.md`

+ 1 - 1
.claude/skills/remove-model-field/SKILL.md

@@ -212,6 +212,6 @@ python manage.py migrate
 
 
 - Panel attrs reference: `netbox/netbox/ui/attrs.py`
 - Panel attrs reference: `netbox/netbox/ui/attrs.py`
 - Panel classes: `netbox/<app>/ui/panels.py`
 - Panel classes: `netbox/<app>/ui/panels.py`
-- Base filterset classes: `netbox/netbox/filtersets.py`
+- Base filterset classes: `netbox/netbox/filtersets/__init__.py`
 - `add-model-field` skill: `.claude/skills/add-model-field/SKILL.md` (reverse of this skill)
 - `add-model-field` skill: `.claude/skills/add-model-field/SKILL.md` (reverse of this skill)
 - Contributing guide: `docs/development/extending-models.md`
 - Contributing guide: `docs/development/extending-models.md`

+ 13 - 0
docs/plugins/development/filtersets.md

@@ -90,6 +90,19 @@ class MyFilterSet(NetBoxModelFilterSet):
 
 
 The `search` method is also used by the `q` filter in `NetBoxModelFilterSet` which in turn is used by the Search field in the filters tab.
 The `search` method is also used by the `q` filter in `NetBoxModelFilterSet` which in turn is used by the Search field in the filters tab.
 
 
+### Legacy Evaluation
+
+!!! info "This feature was introduced in NetBox v4.8."
+
+NetBox evaluates only the filters whose parameters are present in a request. A parameter supplied with an empty value (e.g. `?status=`) is still validated and passed to its filter. Required and disabled filters are always evaluated, as are filters with a custom widget (or subwidget) which overrides `value_from_datadict()`. A filter set which depends on every filter being evaluated for each request, as in earlier releases, can set `legacy_evaluation` to restore that behavior. This is intended as a temporary measure, and will be revisited in NetBox v5.0.
+
+```python
+class MyFilterSet(NetBoxModelFilterSet):
+    legacy_evaluation = True
+```
+
+Legacy evaluation also applies automatically to a filter set which uses a form prefix, customizes its form (via `Meta.form`, `get_form_class()`, or the `form` property), or inherits from a mixin listed after the NetBox base class (e.g. `class MyFilterSet(NetBoxModelFilterSet, MyMixin)`) which overrides `__init__()`, `filter_queryset()`, `errors`, or `is_valid()`. To evaluate only the supplied filters, list such a mixin before the NetBox base class.
+
 ## Filter Classes
 ## Filter Classes
 
 
 ### TagFilter
 ### TagFilter

+ 1 - 1
netbox/extras/tests/query_counts.json

@@ -3,7 +3,7 @@
   "bookmark:list_objects_with_permission": 16,
   "bookmark:list_objects_with_permission": 16,
   "configcontext:api_list_objects": 22,
   "configcontext:api_list_objects": 22,
   "configcontext:list_objects_with_permission": 15,
   "configcontext:list_objects_with_permission": 15,
-  "configcontextprofile:api_list_objects": 12,
+  "configcontextprofile:api_list_objects": 11,
   "configcontextprofile:list_objects_with_permission": 15,
   "configcontextprofile:list_objects_with_permission": 15,
   "configtemplate:api_list_objects": 10,
   "configtemplate:api_list_objects": 10,
   "configtemplate:list_objects_with_permission": 16,
   "configtemplate:list_objects_with_permission": 16,

+ 4 - 3
netbox/ipam/filtersets.py

@@ -11,6 +11,7 @@ from circuits.models import Provider
 from dcim.base_filtersets import ScopedFilterSet
 from dcim.base_filtersets import ScopedFilterSet
 from dcim.models import Device, Interface, Region, Site, SiteGroup
 from dcim.models import Device, Interface, Region, Site, SiteGroup
 from netbox.filtersets import (
 from netbox.filtersets import (
+    BaseFilterSet,
     ChangeLoggedModelFilterSet,
     ChangeLoggedModelFilterSet,
     NetBoxModelFilterSet,
     NetBoxModelFilterSet,
     OrganizationalModelFilterSet,
     OrganizationalModelFilterSet,
@@ -1242,7 +1243,7 @@ SERVICE_PORT_FILTERS = {
 }
 }
 
 
 
 
-class ServicePortMappingFilterMixin(django_filters.FilterSet):
+class ServicePortMappingFilterMixin(BaseFilterSet):
     """
     """
     Shared ``port_mappings``, ``protocol`` and ``port`` filtering for Service and ServiceTemplate, all
     Shared ``port_mappings``, ``protocol`` and ``port`` filtering for Service and ServiceTemplate, all
     operating on the ``port_mappings`` array. ``protocol`` and every active ``port`` lookup are
     operating on the ``port_mappings`` array. ``protocol`` and every active ``port`` lookup are
@@ -1312,7 +1313,7 @@ class ServicePortMappingFilterMixin(django_filters.FilterSet):
         """
         """
         queryset = super().filter_queryset(queryset)
         queryset = super().filter_queryset(queryset)
 
 
-        cleaned_data = self.form.cleaned_data
+        cleaned_data = self.cleaned_data
         protocols = cleaned_data.get('protocol') or []
         protocols = cleaned_data.get('protocol') or []
         port_tests = [
         port_tests = [
             (lookup, values)
             (lookup, values)
@@ -1361,7 +1362,7 @@ class ServicePortMappingFilterMixin(django_filters.FilterSet):
         # ?protocol=tcp&port__n=80 excludes only services exposing tcp/80 (not those exposing udp/80).
         # ?protocol=tcp&port__n=80 excludes only services exposing tcp/80 (not those exposing udp/80).
         if not value:
         if not value:
             return queryset
             return queryset
-        protocols = self.form.cleaned_data.get('protocol') or []
+        protocols = self.cleaned_data.get('protocol') or []
         return queryset.exclude(port_mapping_q(protocols, [('exact', value)]))
         return queryset.exclude(port_mapping_q(protocols, [('exact', value)]))
 
 
 
 

+ 4 - 9
netbox/ipam/views.py

@@ -573,11 +573,8 @@ class ChildAvailabilityMixin:
     def _get_detection_filterset(self, request):
     def _get_detection_filterset(self, request):
         """
         """
         Return the bound FilterSet used to evaluate the request, or None if the view declares no
         Return the bound FilterSet used to evaluate the request, or None if the view declares no
-        filterset. ObjectChildrenView.get() has already built one and validated its form while
-        resolving the child queryset, so reuse it rather than paying for a second FilterSet:
-        get_filters() regenerates every dynamic lookup variant and NetBoxModelFilterSet.__init__
-        queries the custom fields for the model. Build one only for direct calls where get() has
-        not run.
+        filterset. Reuse the one ObjectChildrenView.get() built while resolving the child queryset,
+        if any.
         """
         """
         if self.filterset_instance is not None:
         if self.filterset_instance is not None:
             return self.filterset_instance
             return self.filterset_instance
@@ -600,7 +597,7 @@ class ChildAvailabilityMixin:
 
 
         self._active_child_filters = False
         self._active_child_filters = False
 
 
-        # An empty request cannot activate a filter, so skip validating a form for nothing.
+        # An empty request cannot activate a filter.
         if not request.GET:
         if not request.GET:
             return False
             return False
 
 
@@ -610,9 +607,7 @@ class ChildAvailabilityMixin:
 
 
         # A non-empty cleaned value means the request activated a declared filter. Emptiness
         # A non-empty cleaned value means the request activated a declared filter. Emptiness
         # follows each filter's own semantics, so absent multi-value fields stay inactive.
         # follows each filter's own semantics, so absent multi-value fields stay inactive.
-        # This is a no-op for a reused FilterSet: .qs validated the form to build the queryset.
-        filterset.form.is_valid()
-        for name, value in filterset.form.cleaned_data.items():
+        for name, value in filterset.cleaned_data.items():
             if isinstance(filterset.filters[name], django_filters.MultipleChoiceFilter):
             if isinstance(filterset.filters[name], django_filters.MultipleChoiceFilter):
                 if value:  # mirrors MultipleChoiceFilter.filter()
                 if value:  # mirrors MultipleChoiceFilter.filter()
                     self._active_child_filters = True
                     self._active_child_filters = True

+ 152 - 8
netbox/netbox/filtersets.py → netbox/netbox/filtersets/__init__.py

@@ -2,10 +2,15 @@ import json
 from copy import deepcopy
 from copy import deepcopy
 
 
 import django_filters
 import django_filters
+from django import forms
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.db import models
 from django.db.models import Q
 from django.db.models import Q
+from django.forms.utils import ErrorDict, ErrorList
+from django.http import QueryDict
+from django.utils.datastructures import MultiValueDict
 from django.utils.translation import gettext as _
 from django.utils.translation import gettext as _
 from django_filters.exceptions import FieldLookupError
 from django_filters.exceptions import FieldLookupError
 from django_filters.utils import get_model_field, resolve_field
 from django_filters.utils import get_model_field, resolve_field
@@ -15,6 +20,7 @@ from core.models import ObjectChange
 from extras.choices import CustomFieldFilterLogicChoices
 from extras.choices import CustomFieldFilterLogicChoices
 from extras.filters import TagFilter, TagIDFilter
 from extras.filters import TagFilter, TagIDFilter
 from extras.models import CustomField, SavedFilter
 from extras.models import CustomField, SavedFilter
+from netbox.filtersets.utils import FilterDict, FilterIndex
 from users.filterset_mixins import OwnerFilterMixin
 from users.filterset_mixins import OwnerFilterMixin
 from utilities import filters
 from utilities import filters
 from utilities.constants import (
 from utilities.constants import (
@@ -112,12 +118,10 @@ class BaseFilterSet(django_filters.FilterSet):
         },
         },
     })
     })
 
 
-    def __init__(self, data=None, *args, **kwargs):
-        # bit of a hack for #9231 - extras.lookup.Empty is registered in apps.ready
-        # however FilterSet Factory is setup before this which creates the
-        # initial filters.  This recreates the filters so Empty is picked up correctly.
-        self.base_filters = self.__class__.get_filters()
+    # Set to True to evaluate every filter on each request, as before NetBox v4.8 (to be revisited in v5.0)
+    legacy_evaluation = False
 
 
+    def __init__(self, data=None, queryset=None, *, request=None, prefix=None):
         # Apply any referenced SavedFilters
         # Apply any referenced SavedFilters
         if data and ('filter' in data or 'filter_id' in data):
         if data and ('filter' in data or 'filter_id' in data):
             data = data.copy()  # Get a mutable copy
             data = data.copy()  # Get a mutable copy
@@ -132,7 +136,6 @@ class BaseFilterSet(django_filters.FilterSet):
 
 
             # Only apply SavedFilters the requesting user is permitted to see (#22790). Fall back to
             # Only apply SavedFilters the requesting user is permitted to see (#22790). Fall back to
             # anonymous visibility (shared filters only) when no request is available.
             # anonymous visibility (shared filters only) when no request is available.
-            request = kwargs.get('request')
             user = request.user if request else AnonymousUser()
             user = request.user if request else AnonymousUser()
             saved_filters = SavedFilter.objects.restrict_to_shared(user).filter(
             saved_filters = SavedFilter.objects.restrict_to_shared(user).filter(
                 Q(slug__in=data.pop('filter', [])) |
                 Q(slug__in=data.pop('filter', [])) |
@@ -149,7 +152,138 @@ class BaseFilterSet(django_filters.FilterSet):
                     else:
                     else:
                         data.setlist(key, value)
                         data.setlist(key, value)
 
 
-        super().__init__(data, *args, **kwargs)
+        if (
+            self.legacy_evaluation or
+            prefix is not None or
+            self._has_custom_form() or
+            self._has_overrides_after_base()
+        ):
+            # bit of a hack for #9231 - extras.lookup.Empty is registered in apps.ready
+            # however FilterSet Factory is setup before this which creates the
+            # initial filters.  This recreates the filters so Empty is picked up correctly.
+            self.base_filters = self.__class__.get_filters()
+            super().__init__(data, queryset, request=request, prefix=prefix)
+            return
+
+        # Mirror FilterSet.__init__() without deep-copying every filter; FilterDict copies each one on first access
+        if queryset is None:
+            queryset = self._meta.model._default_manager.all()
+        self.is_bound = data is not None
+        self.data = data or QueryDict()
+        self.queryset = queryset
+        self.request = request
+        self.form_prefix = prefix
+        self.filters = FilterDict(self, self._get_filter_index())
+
+    @classmethod
+    def _get_filter_index(cls):
+        """
+        Return the class's filters, generated on first use (after Empty has been registered; see #9231). A class
+        which overrides get_filters() regenerates them on every call.
+        """
+        if cls.get_filters.__func__ is not BaseFilterSet.get_filters.__func__:
+            return FilterIndex(cls.get_filters())
+        if '_filter_index' not in cls.__dict__:
+            cls._filter_index = FilterIndex(cls.get_filters())
+        return cls._filter_index
+
+    @classmethod
+    def _has_custom_form(cls):
+        return (
+            cls._meta.form is not forms.Form or
+            cls.get_form_class is not django_filters.FilterSet.get_form_class or
+            cls.form is not BaseFilterSet.form
+        )
+
+    @classmethod
+    def _has_overrides_after_base(cls):
+        """
+        Return True if a class which follows BaseFilterSet in the MRO overrides a method or property which lazy
+        evaluation bypasses.
+        """
+        mro = cls.__mro__
+        for klass in mro[mro.index(BaseFilterSet) + 1:]:
+            if klass is object or klass.__module__.startswith('django_filters'):
+                continue
+            if any(name in vars(klass) for name in ('__init__', 'errors', 'filter_queryset', 'form', 'is_valid')):
+                return True
+        return False
+
+    @property
+    def _lazy(self):
+        # False under legacy evaluation, or if the filters have been replaced
+        return isinstance(self.filters, FilterDict)
+
+    @property
+    def form(self):
+        form = super().form
+        if self._lazy and self.is_bound:
+            # Evaluation no longer validates the form, so validate it here for code which reads its cleaned_data. Reuse
+            # the initial values of disabled filters, since a callable may return a different value on each call.
+            self._clean()
+            form.initial.update(self._initial)
+            form.is_valid()
+        return form
+
+    @property
+    def errors(self):
+        if not self._lazy:
+            return super().errors
+        self._clean()
+        return self._errors
+
+    def is_valid(self):
+        if not self._lazy:
+            return super().is_valid()
+        return self.is_bound and not self.errors
+
+    @property
+    def cleaned_data(self):
+        """
+        The cleaned values of the filters which the request supplies, or which are cleaned even when absent (e.g.
+        required filters). For internal use only.
+        """
+        if not self._lazy:
+            self.form.is_valid()
+            return getattr(self.form, 'cleaned_data', {})
+        self._clean()
+        return self._cleaned_data
+
+    def filter_queryset(self, queryset):
+        if not self._lazy:
+            return super().filter_queryset(queryset)
+        for name, value in self.cleaned_data.items():
+            queryset = self.filters[name].filter(queryset, value)
+            assert isinstance(queryset, models.QuerySet), (
+                f"Expected '{type(self).__name__}.{name}' to return a QuerySet, but got a "
+                f"{type(queryset).__name__} instead."
+            )
+        return queryset
+
+    def _clean(self):
+        """
+        Clean the value of each filter which the request supplies, or which is cleaned even when absent (e.g. a
+        required filter), in declaration order.
+        """
+        if hasattr(self, '_cleaned_data'):
+            return
+        self._cleaned_data = {}
+        self._errors = ErrorDict()
+        self._initial = {}
+        if not self.is_bound:
+            return
+        names = self.filters.get_evaluated(self.data.keys())
+        files = MultiValueDict()
+        for name in sorted(names, key=self.filters.position):
+            field = self.filters[name].field
+            if field.disabled:
+                value = self._initial[name] = forms.Form().get_initial_for_field(field, name)
+            else:
+                value = field.widget.value_from_datadict(self.data, files, name)
+            try:
+                self._cleaned_data[name] = field.clean(value)
+            except ValidationError as e:
+                self._errors[name] = ErrorList(e.error_list)
 
 
     @staticmethod
     @staticmethod
     def _get_filter_lookup_dict(existing_filter):
     def _get_filter_lookup_dict(existing_filter):
@@ -351,6 +485,16 @@ class NetBoxModelFilterSet(ChangeLoggedModelFilterSet):
     def __init__(self, *args, **kwargs):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         super().__init__(*args, **kwargs)
 
 
+        if isinstance(self.filters, FilterDict):
+            # Build custom field filters only once one is needed
+            self.filters.add_lazy('cf_', self._get_custom_field_filters)
+        else:
+            self.filters.update(self._get_custom_field_filters())
+
+    def _get_custom_field_filters(self):
+        """
+        Return filters for the model's custom fields, including their additional lookups.
+        """
         custom_field_filters = {}
         custom_field_filters = {}
         for custom_field in CustomField.objects.get_for_model(self._meta.model):
         for custom_field in CustomField.objects.get_for_model(self._meta.model):
             if custom_field.filter_logic == CustomFieldFilterLogicChoices.FILTER_DISABLED:
             if custom_field.filter_logic == CustomFieldFilterLogicChoices.FILTER_DISABLED:
@@ -364,7 +508,7 @@ class NetBoxModelFilterSet(ChangeLoggedModelFilterSet):
                 additional_lookups = self.get_additional_lookups(filter_name, filter_instance)
                 additional_lookups = self.get_additional_lookups(filter_name, filter_instance)
                 custom_field_filters.update(additional_lookups)
                 custom_field_filters.update(additional_lookups)
 
 
-        self.filters.update(custom_field_filters)
+        return custom_field_filters
 
 
     def search(self, queryset, name, value):
     def search(self, queryset, name, value):
         """
         """

+ 256 - 0
netbox/netbox/filtersets/utils.py

@@ -0,0 +1,256 @@
+from collections.abc import MutableMapping
+from copy import deepcopy
+
+from django import forms
+from django.forms.widgets import ChoiceWidget
+from django_filters.widgets import BaseCSVWidget, BooleanWidget, LinkWidget, QueryArrayWidget, SuffixedMultiWidget
+
+__all__ = (
+    'FilterDict',
+    'FilterIndex',
+    'LazyFilters',
+    'get_data_keys',
+)
+
+# Widget classes whose value_from_datadict() reads only the key matching the field's name
+SINGLE_KEY_WIDGETS = (
+    forms.CheckboxInput,
+    forms.MultipleHiddenInput,
+    forms.NullBooleanSelect,
+    forms.SelectMultiple,
+    forms.Widget,
+    ChoiceWidget,
+    BaseCSVWidget,
+    BooleanWidget,
+    LinkWidget,
+)
+
+
+def get_field_attr(filter_, attr):
+    """
+    Return an attribute (e.g. "required") of the given filter's form field, without building the field if it hasn't
+    been already.
+    """
+    # Filter.field caches the field as _field once built, after which changes to the filter's extra don't apply
+    if (field := vars(filter_).get('_field')) is not None:
+        return getattr(field, attr)
+    return filter_.extra.get(attr)
+
+
+def get_data_keys(name, filter_):
+    """
+    Return the keys of request data from which the given filter reads its value, or None if they can't be determined.
+    """
+    widget = get_field_attr(filter_, 'widget') or getattr(filter_.field_class, 'widget', None)
+    if isinstance(widget, type) and issubclass(widget, forms.MultiWidget):
+        # Only an instance holds its subwidgets
+        widget = deepcopy(filter_).field.widget
+    return get_widget_keys(name, widget)
+
+
+def get_widget_keys(name, widget):
+    """
+    Return the keys of request data from which the given widget reads its value, or None if they can't be determined.
+    """
+    widget_class = widget if isinstance(widget, type) else type(widget)
+    # The keys read depend on which class implements value_from_datadict()
+    reader = next((klass for klass in widget_class.__mro__ if 'value_from_datadict' in vars(klass)), None)
+    if reader is QueryArrayWidget:
+        return [name, f'{name}[]']
+    if reader is SuffixedMultiWidget:
+        subnames = [widget.suffixed(name, suffix) for suffix in widget.suffixes]
+    elif reader is forms.MultiWidget:
+        subnames = [f'{name}{suffix}' for suffix in widget.widgets_names]
+    elif reader in SINGLE_KEY_WIDGETS:
+        return [name]
+    else:
+        return None
+    keys = []
+    for subname, subwidget in zip(subnames, widget.widgets):
+        if (subkeys := get_widget_keys(subname, subwidget)) is None:
+            return None
+        keys.extend(subkeys)
+    return keys
+
+
+def is_unconditional(filter_, data_keys):
+    """
+    Return True if the given filter is cleaned even when absent from the request data: if it's required or disabled,
+    or if the keys it reads (data_keys) can't be determined.
+    """
+    return data_keys is None or bool(get_field_attr(filter_, 'required') or get_field_attr(filter_, 'disabled'))
+
+
+class FilterIndex:
+    """
+    The filters of a FilterSet class, indexed for resolving request data.
+
+    Attributes:
+        filters: The class's filters, keyed by name in declaration order; shared by all instances of the FilterSet
+        positions: The position of each filter in declaration order, keyed by name
+        data_keys: Maps each request data key read by a filter of a different name (e.g. "created_after") to the
+            names of the filters which read it
+        unconditional: The names of filters which are cleaned even when absent from the request data: those which
+            are required or disabled, or whose data keys can't be determined
+    """
+    def __init__(self, filters):
+        self.filters = filters
+        self.positions = {name: i for i, name in enumerate(filters)}
+        self.data_keys = {}
+        self.unconditional = []
+        for name, filter_ in filters.items():
+            keys = get_data_keys(name, filter_)
+            if is_unconditional(filter_, keys):
+                self.unconditional.append(name)
+            for key in keys or ():
+                if key != name:
+                    self.data_keys.setdefault(key, []).append(name)
+
+
+class LazyFilters:
+    """
+    Filters built by calling a loader once a filter whose name begins with the given prefix is requested.
+
+    Attributes:
+        prefix: The prefix of the names of the filters the loader returns (e.g. "cf_")
+        loader: A callable which returns the filters, keyed by name
+        names: The names of the filters which the loader added, or None until it has been called
+    """
+    def __init__(self, prefix, loader):
+        self.prefix = prefix
+        self.loader = loader
+        self.names = None
+
+
+class FilterDict(MutableMapping):
+    """
+    The filters of a FilterSet instance. Each of the class's filters is copied on first access, so that a request
+    copies only the filters it uses.
+
+    Attributes:
+        filterset: The FilterSet instance to which the filters belong
+        index: The FilterIndex of the FilterSet's class
+        filters: The filters copied from the index, added, or loaded for this instance, keyed by name. Any of these may
+            have been changed since, so the request data each reads is determined only once the request is evaluated.
+        removed: The names of class filters removed from this instance
+        extra: The names of filters added to this instance, and its LazyFilters, in the order added
+    """
+    def __init__(self, filterset, index):
+        self.filterset = filterset
+        self.index = index
+        self.filters = {}
+        self.removed = set()
+        self.extra = []
+
+    def __getitem__(self, name):
+        if name not in self.filters:
+            if self._is_class_filter(name):
+                filter_ = deepcopy(self.index.filters[name])
+                filter_.model = self.filterset.queryset.model
+                filter_.parent = self.filterset
+                self.filters[name] = filter_
+            elif not self._load(name):
+                raise KeyError(name)
+        return self.filters[name]
+
+    def __setitem__(self, name, filter_):
+        if name not in self:
+            self.extra.append(name)
+        self.filters[name] = filter_
+
+    def __delitem__(self, name):
+        if name not in self:
+            raise KeyError(name)
+        self.filters.pop(name, None)
+        if self._is_class_filter(name):
+            self.removed.add(name)
+        elif name in self.extra:
+            self.extra.remove(name)
+        else:
+            for entry in self.extra:
+                if isinstance(entry, LazyFilters) and name in (entry.names or []):
+                    entry.names.remove(name)
+
+    def __iter__(self):
+        self._load()
+        for name in self.index.filters:
+            if name not in self.removed:
+                yield name
+        yield from self._extra_names()
+
+    def __len__(self):
+        self._load()
+        return len(self.index.filters) - len(self.removed) + len(self._extra_names())
+
+    def __contains__(self, name):
+        return name in self.filters or self._is_class_filter(name) or self._load(name)
+
+    def copy(self):
+        """
+        Return the filters as a dict.
+        """
+        return dict(self)
+
+    def add_lazy(self, prefix, loader):
+        """
+        Register a callable which returns filters named with the given prefix, to be called once one is needed.
+        """
+        self.extra.append(LazyFilters(prefix, loader))
+
+    def get_evaluated(self, keys):
+        """
+        Return the names of the filters to clean for request data with the given keys: those which read any of the
+        keys, and those which are cleaned even when absent from the request data.
+        """
+        keys = set(keys)
+        # Look up each key first, which loads any lazy filters it names
+        names = {key for key in keys if key in self}
+        for key in keys:
+            names.update(name for name in self.index.data_keys.get(key, ()) if self._is_indexed(name))
+        names.update(name for name in self.index.unconditional if self._is_indexed(name))
+        # The index doesn't reflect changes to the filters held by this instance, so examine each of them
+        for name, filter_ in self.filters.items():
+            data_keys = get_data_keys(name, filter_)
+            if is_unconditional(filter_, data_keys) or not keys.isdisjoint(data_keys):
+                names.add(name)
+        return names
+
+    def position(self, name):
+        """
+        Return the position of the named filter in declaration order.
+        """
+        if self._is_class_filter(name):
+            return self.index.positions[name]
+        return len(self.index.positions) + self._extra_names().index(name)
+
+    def _is_class_filter(self, name):
+        return name in self.index.filters and name not in self.removed
+
+    def _is_indexed(self, name):
+        # A class filter which this instance hasn't copied, replaced, or removed
+        return name not in self.filters and self._is_class_filter(name)
+
+    def _extra_names(self):
+        names = []
+        for entry in self.extra:
+            if isinstance(entry, LazyFilters):
+                names.extend(entry.names or [])
+            else:
+                names.append(entry)
+        return names
+
+    def _load(self, name=None):
+        """
+        Call any pending loaders for the given filter name (or all of them). Return True if the name now exists.
+        """
+        for entry in self.extra:
+            if not isinstance(entry, LazyFilters) or entry.names is not None:
+                continue
+            if name is not None and not name.startswith(entry.prefix):
+                continue
+            entry.names = []
+            for filter_name, filter_ in entry.loader().items():
+                if filter_name not in self.filters and not self._is_class_filter(filter_name):
+                    entry.names.append(filter_name)
+                self.filters[filter_name] = filter_
+        return name is not None and name in self.filters

+ 457 - 0
netbox/netbox/tests/test_filtersets.py

@@ -0,0 +1,457 @@
+from copy import deepcopy
+from importlib import import_module
+
+from django import forms
+from django.conf import settings
+from django.core.exceptions import EmptyResultSet
+from django.http import QueryDict
+from django.test import TestCase
+from django.utils.datastructures import MultiValueDict
+from django_filters import CharFilter, DateTimeFromToRangeFilter, FilterSet, MultipleChoiceFilter, RangeFilter
+from django_filters.widgets import QueryArrayWidget, RangeWidget
+
+from core.filtersets import ObjectChangeFilterSet
+from core.models import ObjectChange, ObjectType
+from dcim.choices import SiteStatusChoices
+from dcim.filtersets import SiteFilterSet
+from dcim.models import Region, Site
+from extras.choices import CustomFieldFilterLogicChoices, CustomFieldTypeChoices
+from extras.models import CustomField
+from netbox.filtersets.utils import FilterDict, get_data_keys
+from netbox.registry import registry
+from utilities.filters import ContentTypeFilter, MultiValueCharFilter, MultiValueContentTypeFilter
+
+CORE_APPS = ('circuits', 'core', 'dcim', 'extras', 'ipam', 'tenancy', 'users', 'virtualization', 'vpn', 'wireless')
+
+
+def get_filterset_classes():
+    """
+    Return every registered FilterSet class, after importing the core FilterSets so that they're registered.
+    """
+    for app_label in CORE_APPS:
+        import_module(f'{app_label}.filtersets')
+    return list(registry['filtersets'].values())
+
+
+def get_sample_value(field):
+    """
+    Return a representative value for the given form field.
+    """
+    if isinstance(field, (forms.BooleanField, forms.NullBooleanField)):
+        return 'true'
+    if isinstance(field, (forms.ModelChoiceField, forms.ModelMultipleChoiceField)):
+        return settings.FILTERS_NULL_CHOICE_VALUE
+    if isinstance(field, forms.ChoiceField):
+        for value, label in field.choices:
+            # Descend into grouped choices
+            if isinstance(label, (list, tuple)):
+                value = label[0][0]
+            if value not in ('', None, settings.FILTERS_NULL_CHOICE_VALUE):
+                return str(value)
+    if isinstance(field, forms.DateTimeField):
+        return '2026-01-01 00:00:00'
+    if isinstance(field, forms.DateField):
+        return '2026-01-01'
+    if isinstance(field, forms.TimeField):
+        return '00:00:00'
+    if isinstance(field, forms.DurationField):
+        return '00:01:00'
+    if isinstance(field, forms.UUIDField):
+        return '00000000-0000-0000-0000-000000000001'
+    if isinstance(field, (forms.IntegerField, forms.DecimalField, forms.FloatField)):
+        return '1'
+    return 'x'
+
+
+def get_sample_data(filterset_class):
+    """
+    Return request data which supplies a value for every filter on the given FilterSet class.
+    """
+    data = QueryDict(mutable=True)
+    for name, filter_ in filterset_class.get_filters().items():
+        field = deepcopy(filter_).field
+        if isinstance(filter_, (ContentTypeFilter, MultiValueContentTypeFilter)):
+            data[name] = 'dcim.site'
+        elif isinstance(field, forms.MultiValueField):
+            for key, subfield in zip(get_data_keys(name, filter_), field.fields):
+                data[key] = get_sample_value(subfield)
+        elif filter_.method and isinstance(field, (forms.ModelChoiceField, forms.ModelMultipleChoiceField)):
+            # Filter methods expect model instances, which this test doesn't create
+            continue
+        else:
+            data[name] = get_sample_value(field)
+    return data
+
+
+def get_empty_data(filterset_class):
+    """
+    Return request data which supplies an empty value for every filter on the given FilterSet class.
+    """
+    data = QueryDict(mutable=True)
+    for name, filter_ in filterset_class.get_filters().items():
+        for key in get_data_keys(name, filter_):
+            data[key] = ''
+    return data
+
+
+def get_subset(data, keys):
+    """
+    Return a copy of the given request data with only the given keys.
+    """
+    subset = QueryDict(mutable=True)
+    for key in keys:
+        subset.setlist(key, data.getlist(key))
+    return subset
+
+
+def get_sql(filterset):
+    """
+    Return the SQL of the given FilterSet's queryset, or None if the queryset is empty.
+    """
+    try:
+        return str(filterset.qs.query)
+    except EmptyResultSet:
+        return None
+
+
+def get_emptying_keys(filterset_class, data):
+    """
+    Return the keys of the given request data which each empty the FilterSet's queryset when supplied alone.
+    """
+    queryset = filterset_class._meta.model.objects.all()
+    if get_sql(filterset_class(data, queryset)) is not None:
+        return []
+    return [key for key in data if get_sql(filterset_class(get_subset(data, [key]), queryset)) is None]
+
+
+def evaluate(filterset):
+    """
+    Return the errors and SQL resulting from evaluation of the given FilterSet.
+    """
+    return list(filterset.errors), filterset.errors.get_json_data(), get_sql(filterset)
+
+
+class RecordingQueryDict(QueryDict):
+    """
+    An empty QueryDict which records the keys read from it, and whether its keys were enumerated.
+    """
+    def __init__(self):
+        super().__init__()
+        self.keys_read = set()
+        self.enumerated = False
+
+    def __contains__(self, key):
+        self.keys_read.add(key)
+        return super().__contains__(key)
+
+    def __getitem__(self, key):
+        self.keys_read.add(key)
+        return super().__getitem__(key)
+
+    def _getlist(self, key, *args, **kwargs):
+        self.keys_read.add(key)
+        return super()._getlist(key, *args, **kwargs)
+
+    def __iter__(self):
+        self.enumerated = True
+        return super().__iter__()
+
+    def keys(self):
+        self.enumerated = True
+        return super().keys()
+
+    def lists(self):
+        self.enumerated = True
+        return super().lists()
+
+
+class FilterSetEvaluationTestCase(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.region = Region.objects.create(name='Region 1', slug='region-1')
+        Site.objects.bulk_create((
+            Site(name='Site 1', slug='site-1', status=SiteStatusChoices.STATUS_ACTIVE),
+            Site(name='Site 2', slug='site-2', status=SiteStatusChoices.STATUS_PLANNED),
+            Site(name='Site 3', slug='site-3', status=SiteStatusChoices.STATUS_ACTIVE),
+        ))
+
+    def test_only_supplied_filters_are_copied(self):
+        filterset = SiteFilterSet({'status': ['active'], 'limit': ['10']}, Site.objects.all())
+        self.assertEqual(filterset.qs.count(), 2)
+        self.assertEqual(list(filterset.filters.filters), ['status'])
+        self.assertNotIn('_form', vars(filterset))
+
+    def test_unbound(self):
+        filterset = SiteFilterSet(queryset=Site.objects.all())
+        self.assertFalse(filterset.is_valid())
+        self.assertEqual(filterset.qs.count(), 3)
+
+    def test_invalid_value(self):
+        filterset = SiteFilterSet({'region_id': ['999999'], 'status': ['active']}, Site.objects.all())
+        self.assertFalse(filterset.is_valid())
+        self.assertEqual(list(filterset.errors), ['region_id'])
+        # Valid filters are still applied
+        self.assertEqual(filterset.qs.count(), 2)
+
+    def test_legacy_evaluation(self):
+        class LegacySiteFilterSet(SiteFilterSet):
+            legacy_evaluation = True
+
+        filterset = LegacySiteFilterSet({'status': ['active']}, Site.objects.all())
+        self.assertEqual(filterset.qs.count(), 2)
+        self.assertNotIsInstance(filterset.filters, FilterDict)
+        self.assertIn('_form', vars(filterset))
+
+    def test_form(self):
+        filterset = SiteFilterSet({'status': ['active']}, Site.objects.all())
+        self.assertEqual(list(filterset.form.fields), list(filterset.filters))
+        self.assertEqual(filterset.form.cleaned_data['status'], ['active'])
+
+    def test_filter_changes_are_per_instance(self):
+        data = {'region_id': [self.region.pk]}
+        filterset = SiteFilterSet(data, Site.objects.all())
+        filterset.filters['region_id'].queryset = Region.objects.none()
+        self.assertFalse(filterset.is_valid())
+        self.assertTrue(SiteFilterSet(data, Site.objects.all()).is_valid())
+
+    def test_added_filter(self):
+        class ExtendedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['label'] = MultiValueCharFilter(field_name='name')
+
+        self.assertEqual(ExtendedSiteFilterSet({'label': ['Site 1']}, Site.objects.all()).qs.count(), 1)
+
+    def test_replaced_filter(self):
+        class ExtendedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['created'] = DateTimeFromToRangeFilter(field_name='created')
+
+        # The replacement's data keys supersede those of the class filter
+        filterset = ExtendedSiteFilterSet({'created_after': '2100-01-01 00:00:00'}, Site.objects.all())
+        self.assertEqual(list(filterset.cleaned_data), ['created'])
+        self.assertEqual(filterset.qs.count(), 0)
+
+    def test_required_added_filter(self):
+        class ExtendedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['label'] = CharFilter(field_name='name', required=True)
+
+        filterset = ExtendedSiteFilterSet({'status': ['active']}, Site.objects.all())
+        self.assertFalse(filterset.is_valid())
+        self.assertEqual(list(filterset.errors), ['label'])
+
+    def test_changed_filters(self):
+        class ChangedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['q'].extra['required'] = True
+                self.filters['created_by_request'].field.required = True
+                self.filters['status'].extra['widget'] = QueryArrayWidget
+
+        # Changes to the instance's filters, or to their fields, supersede the class's filters
+        filterset = ChangedSiteFilterSet(QueryDict('status[]=planned'), Site.objects.all())
+        self.assertEqual(set(filterset.errors), {'q', 'created_by_request'})
+        self.assertEqual(filterset.qs.count(), 1)
+
+    def test_removed_filters(self):
+        class ReducedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['created'] = DateTimeFromToRangeFilter(field_name='created')
+                del self.filters['created']
+                del self.filters['status']
+
+        data = {'status': ['planned'], 'created_after': '2100-01-01 00:00:00'}
+        filterset = ReducedSiteFilterSet(data, Site.objects.all())
+        self.assertNotIn('status', filterset.filters)
+        self.assertNotIn('created', filterset.filters)
+        self.assertEqual(filterset.cleaned_data, {})
+        self.assertEqual(filterset.qs.count(), 3)
+        # Other instances are unaffected
+        self.assertEqual(SiteFilterSet(data, Site.objects.all()).qs.count(), 1)
+
+    def test_copy(self):
+        filterset = SiteFilterSet({'status': ['active']}, Site.objects.all())
+        filters = filterset.filters.copy()
+        self.assertIs(type(filters), dict)
+        self.assertEqual(list(filters), list(filterset.filters))
+        del filters['status']
+        self.assertIn('status', filterset.filters)
+
+    def test_overrides_after_base(self):
+        class StatusMixin(FilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.initialized = True
+
+            def filter_queryset(self, queryset):
+                return super().filter_queryset(queryset).filter(status=SiteStatusChoices.STATUS_ACTIVE)
+
+        # A mixin which follows BaseFilterSet in the MRO forces legacy evaluation, so that its overrides are called
+        class MixedSiteFilterSet(SiteFilterSet, StatusMixin):
+            pass
+
+        filterset = MixedSiteFilterSet({'name': ['Site 1', 'Site 2']}, Site.objects.all())
+        self.assertNotIsInstance(filterset.filters, FilterDict)
+        self.assertTrue(filterset.initialized)
+        self.assertEqual(filterset.qs.count(), 1)
+
+    def test_form_override(self):
+        class RequiredSearchMixin(FilterSet):
+            @property
+            def form(self):
+                form = super().form
+                form.fields['q'].required = True
+                return form
+
+        # Overriding the form forces legacy evaluation, which validates via the form, whether the override precedes
+        # or follows BaseFilterSet in the MRO
+        for bases in ((RequiredSearchMixin, SiteFilterSet), (SiteFilterSet, RequiredSearchMixin)):
+            with self.subTest(bases=[base.__name__ for base in bases]):
+                filterset_class = type('CustomSiteFilterSet', bases, {})
+                filterset = filterset_class({'status': ['active']}, Site.objects.all())
+                self.assertNotIsInstance(filterset.filters, FilterDict)
+                self.assertEqual(list(filterset.errors), ['q'])
+
+    def test_query_array_widget(self):
+        class ExtendedSiteFilterSet(SiteFilterSet):
+            status_array = MultipleChoiceFilter(field_name='status', choices=SiteStatusChoices, widget=QueryArrayWidget)
+
+        filterset = ExtendedSiteFilterSet(QueryDict('status_array[]=planned'), Site.objects.all())
+        self.assertEqual(filterset.qs.count(), 1)
+
+    def test_disabled_filters(self):
+        class DeclaredSiteFilterSet(SiteFilterSet):
+            label = CharFilter(field_name='name', lookup_expr='istartswith', disabled=True, initial='Site 2')
+
+        class AddedSiteFilterSet(SiteFilterSet):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
+                self.filters['label'] = CharFilter(field_name='name', disabled=True, initial='Site 2')
+
+        # A disabled filter takes its initial value, whether or not the request supplies one
+        for filterset_class in (DeclaredSiteFilterSet, AddedSiteFilterSet):
+            for data in ({'label': 'Site 1'}, {}):
+                with self.subTest(filterset=filterset_class.__name__, data=data):
+                    filterset = filterset_class(data, Site.objects.all())
+                    self.assertEqual(list(filterset.qs.values_list('name', flat=True)), ['Site 2'])
+
+    def test_disabled_filter_callable_initial(self):
+        names = iter(['Site 2', 'Site 3'])
+
+        class DisabledSiteFilterSet(SiteFilterSet):
+            label = CharFilter(field_name='name', lookup_expr='istartswith', disabled=True, initial=lambda: next(names))
+
+        # The form reuses the value with which the filter was evaluated
+        filterset = DisabledSiteFilterSet({}, Site.objects.all())
+        self.assertEqual(list(filterset.qs.values_list('name', flat=True)), ['Site 2'])
+        self.assertEqual(filterset.form.cleaned_data['label'], 'Site 2')
+        self.assertEqual(filterset.form['label'].value(), 'Site 2')
+
+    def test_custom_widget(self):
+        class AliasWidget(forms.TextInput):
+            def value_from_datadict(self, data, files, name):
+                return data.get(f'{name}_alias')
+
+        class AliasRangeWidget(RangeWidget):
+            def __init__(self, attrs=None):
+                super().__init__(attrs)
+                self.widgets[0] = AliasWidget()
+
+        class ExtendedSiteFilterSet(SiteFilterSet):
+            label = CharFilter(field_name='name', widget=AliasWidget)
+            span = RangeFilter(field_name='pk', widget=AliasRangeWidget)
+
+        # A filter whose widget, or any of its subwidgets, reads unknown keys is cleaned on every request
+        last_pk = Site.objects.order_by('pk').last().pk
+        for name, data in (('label', {'label_alias': 'Site 1'}), ('span', {'span_min_alias': last_pk})):
+            with self.subTest(filter=name):
+                filterset = ExtendedSiteFilterSet(data, Site.objects.all())
+                self.assertIn(name, filterset.cleaned_data)
+                self.assertEqual(filterset.qs.count(), 1)
+
+    def test_get_filters_override(self):
+        calls = []
+
+        class CustomSiteFilterSet(SiteFilterSet):
+            @classmethod
+            def get_filters(cls):
+                calls.append(cls)
+                return super().get_filters()
+
+        # A class which overrides get_filters() regenerates its filters on every instantiation
+        calls.clear()
+        CustomSiteFilterSet({}, Site.objects.all())
+        CustomSiteFilterSet({}, Site.objects.all())
+        self.assertEqual(len(calls), 2)
+
+    def test_custom_field_filters(self):
+        custom_field = CustomField.objects.create(
+            name='cf1',
+            type=CustomFieldTypeChoices.TYPE_TEXT,
+            filter_logic=CustomFieldFilterLogicChoices.FILTER_EXACT
+        )
+        custom_field.object_types.set([ObjectType.objects.get_for_model(Site)])
+        Site.objects.filter(slug='site-1').update(custom_field_data={'cf1': 'foo'})
+
+        self.assertEqual(SiteFilterSet({'cf_cf1': ['foo']}, Site.objects.all()).qs.count(), 1)
+        self.assertEqual(SiteFilterSet({'cf_cf1__n': ['foo']}, Site.objects.all()).qs.count(), 2)
+        self.assertIn('cf_cf1__ic', SiteFilterSet().filters)
+
+    def test_custom_field_filters_not_loaded(self):
+        # Custom fields aren't queried unless a cf_* filter is needed
+        with self.assertNumQueries(0):
+            SiteFilterSet({'status': ['active']}, Site.objects.all()).qs
+
+    def test_widget_suffixed_data_keys(self):
+        filterset = ObjectChangeFilterSet({'time_after': '2026-01-01 00:00:00'}, ObjectChange.objects.all())
+        self.assertEqual(list(filterset.cleaned_data), ['time'])
+        self.assertIn('"time" >=', str(filterset.qs.query))
+
+
+class LegacyEvaluationParityTestCase(TestCase):
+
+    def test_parity(self):
+        """
+        Evaluate every registered FilterSet with all of its filters supplied, with all of them supplied but empty, and
+        with none of them supplied, and check that each result matches legacy evaluation. A supplied value which
+        empties the queryset is evaluated alone, so that it doesn't mask the SQL of the others.
+        """
+        for filterset_class in get_filterset_classes():
+            legacy_class = type(filterset_class.__name__, (filterset_class,), {'legacy_evaluation': True})
+            queryset = filterset_class._meta.model.objects.all()
+            sample_data = get_sample_data(filterset_class)
+            emptying_keys = get_emptying_keys(filterset_class, sample_data)
+            datasets = {
+                'supplied': get_subset(sample_data, [key for key in sample_data if key not in emptying_keys]),
+                **{f'{key} only': get_subset(sample_data, [key]) for key in emptying_keys},
+                'empty': get_empty_data(filterset_class),
+                'absent': QueryDict(),
+            }
+            for dataset, data in datasets.items():
+                with self.subTest(filterset=filterset_class.__name__, data=dataset):
+                    result = evaluate(filterset_class(data, queryset))
+                    self.assertEqual(result, evaluate(legacy_class(data, queryset)))
+                    if dataset == 'supplied':
+                        # The remaining values mustn't empty the queryset (leaving no SQL) in combination
+                        self.assertIsNotNone(result[2])
+
+    def test_data_keys(self):
+        """
+        Check that get_data_keys() includes every key of request data which each filter's widget reads.
+        """
+        for filterset_class in get_filterset_classes():
+            for name, filter_ in filterset_class.get_filters().items():
+                with self.subTest(filterset=filterset_class.__name__, filter=name):
+                    keys = get_data_keys(name, filter_)
+                    # Every core filter's widget should be recognized
+                    self.assertIsNotNone(keys)
+                    # Supply no data, so that widgets also read any keys they fall back to
+                    data = RecordingQueryDict()
+                    deepcopy(filter_).field.widget.value_from_datadict(data, MultiValueDict(), name)
+                    self.assertFalse(data.enumerated)
+                    self.assertEqual(data.keys_read - set(keys), set())