Merge pull request #4128 from netbox-community/4108-apiselect-performance

Closes #4108: Extraneous queryset evaluation by FilterChoiceFields

docs/release-notes/version-2.7.md

@@ -13,6 +13,7 @@
 * [#4090](https://github.com/netbox-community/netbox/issues/4090) - Render URL custom fields as links under object view
 * [#4091](https://github.com/netbox-community/netbox/issues/4091) - Fix filtering of objects by custom fields using UI search form
 * [#4099](https://github.com/netbox-community/netbox/issues/4099) - Linkify interfaces on global interfaces list
+* [#4108](https://github.com/netbox-community/netbox/issues/4108) - Avoid extraneous database queries when rendering search forms
 
 # v2.7.4 (2020-02-04)
 

netbox/circuits/forms.py

@@ -311,7 +311,7 @@ class CircuitFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm
         required=False,
         widget=StaticSelect2Multiple()
     )
-    region = forms.ModelMultipleChoiceField(
+    region = FilterChoiceField(
         queryset=Region.objects.all(),
         to_field_name='slug',
         required=False,

netbox/dcim/forms.py

@@ -369,10 +369,9 @@ class SiteFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
         required=False,
         widget=StaticSelect2Multiple()
     )
-    region = forms.ModelMultipleChoiceField(
+    region = FilterChoiceField(
         queryset=Region.objects.all(),
         to_field_name='slug',
-        required=False,
         widget=APISelectMultiple(
             api_url="/api/dcim/regions/",
             value_field="slug",
@@ -734,7 +733,6 @@ class RackFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
             'site'
         ),
         label='Rack group',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/rack-groups/",
             null_option=True
@@ -748,7 +746,6 @@ class RackFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
     role = FilterChoiceField(
         queryset=RackRole.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/rack-roles/",
             value_field="slug",
@@ -874,7 +871,6 @@ class RackReservationFilterForm(BootstrapMixin, TenancyFilterForm):
     group_id = FilterChoiceField(
         queryset=RackGroup.objects.prefetch_related('site'),
         label='Rack group',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/rack-groups/",
             null_option=True,
@@ -2182,7 +2178,6 @@ class DeviceFilterForm(BootstrapMixin, LocalConfigContextFilterForm, TenancyFilt
     rack_id = FilterChoiceField(
         queryset=Rack.objects.all(),
         label='Rack',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/racks/",
             null_option=True,
@@ -2219,7 +2214,6 @@ class DeviceFilterForm(BootstrapMixin, LocalConfigContextFilterForm, TenancyFilt
     platform = FilterChoiceField(
         queryset=Platform.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/platforms/",
             value_field="slug",
@@ -3913,7 +3907,6 @@ class CableFilterForm(BootstrapMixin, forms.Form):
     rack_id = FilterChoiceField(
         queryset=Rack.objects.all(),
         label='Rack',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/racks/",
             null_option=True,
@@ -4471,7 +4464,6 @@ class VirtualChassisFilterForm(BootstrapMixin, CustomFieldFilterForm):
     tenant_group = FilterChoiceField(
         queryset=TenantGroup.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/tenancy/tenant-groups/",
             value_field="slug",
@@ -4484,7 +4476,6 @@ class VirtualChassisFilterForm(BootstrapMixin, CustomFieldFilterForm):
     tenant = FilterChoiceField(
         queryset=Tenant.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/tenancy/tenants/",
             value_field="slug",
@@ -4592,7 +4583,6 @@ class PowerPanelFilterForm(BootstrapMixin, CustomFieldFilterForm):
     rack_group_id = FilterChoiceField(
         queryset=RackGroup.objects.all(),
         label='Rack group (ID)',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/rack-groups/",
             null_option=True,
@@ -4826,7 +4816,6 @@ class PowerFeedFilterForm(BootstrapMixin, CustomFieldFilterForm):
     power_panel_id = FilterChoiceField(
         queryset=PowerPanel.objects.all(),
         label='Power panel',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/power-panels/",
             null_option=True,
@@ -4835,7 +4824,6 @@ class PowerFeedFilterForm(BootstrapMixin, CustomFieldFilterForm):
     rack_id = FilterChoiceField(
         queryset=Rack.objects.all(),
         label='Rack',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/racks/",
             null_option=True,

netbox/extras/forms.py

@@ -387,11 +387,14 @@ class ObjectChangeFilterForm(BootstrapMixin, forms.Form):
     )
     action = forms.ChoiceField(
         choices=add_blank_choice(ObjectChangeActionChoices),
-        required=False
+        required=False,
+        widget=StaticSelect2()
     )
+    # TODO: Convert to FilterChoiceField once we have an API endpoint for users
     user = forms.ModelChoiceField(
         queryset=User.objects.order_by('username'),
-        required=False
+        required=False,
+        widget=StaticSelect2()
     )
     changed_object_type = forms.ModelChoiceField(
         queryset=ContentType.objects.order_by('model'),

netbox/ipam/forms.py

@@ -528,7 +528,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm)
     vrf_id = FilterChoiceField(
         queryset=VRF.objects.all(),
         label='VRF',
-        null_label='-- Global --',
         widget=APISelectMultiple(
             api_url="/api/ipam/vrfs/",
             null_option=True,
@@ -554,7 +553,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm)
     site = FilterChoiceField(
         queryset=Site.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/dcim/sites/",
             value_field="slug",
@@ -564,7 +562,6 @@ class PrefixFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm)
     role = FilterChoiceField(
         queryset=Role.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/ipam/roles/",
             value_field="slug",
@@ -999,7 +996,6 @@ class IPAddressFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterFo
     vrf_id = FilterChoiceField(
         queryset=VRF.objects.all(),
         label='VRF',
-        null_label='-- Global --',
         widget=APISelectMultiple(
             api_url="/api/ipam/vrfs/",
             null_option=True,
@@ -1080,7 +1076,6 @@ class VLANGroupFilterForm(BootstrapMixin, forms.Form):
     site = FilterChoiceField(
         queryset=Site.objects.all(),
         to_field_name='slug',
-        null_label='-- Global --',
         widget=APISelectMultiple(
             api_url="/api/dcim/sites/",
             value_field="slug",
@@ -1279,7 +1274,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
     site = FilterChoiceField(
         queryset=Site.objects.all(),
         to_field_name='slug',
-        null_label='-- Global --',
         widget=APISelectMultiple(
             api_url="/api/dcim/sites/",
             value_field="slug",
@@ -1289,7 +1283,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
     group_id = FilterChoiceField(
         queryset=VLANGroup.objects.all(),
         label='VLAN group',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/ipam/vlan-groups/",
             null_option=True,
@@ -1303,7 +1296,6 @@ class VLANFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm):
     role = FilterChoiceField(
         queryset=Role.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/ipam/roles/",
             value_field="slug",

netbox/tenancy/forms.py

@@ -108,7 +108,6 @@ class TenantFilterForm(BootstrapMixin, CustomFieldFilterForm):
     group = FilterChoiceField(
         queryset=TenantGroup.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/tenancy/tenant-groups/",
             value_field="slug",
@@ -163,7 +162,6 @@ class TenancyFilterForm(forms.Form):
     tenant_group = FilterChoiceField(
         queryset=TenantGroup.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/tenancy/tenant-groups/",
             value_field="slug",
@@ -176,7 +174,6 @@ class TenancyFilterForm(forms.Form):
     tenant = FilterChoiceField(
         queryset=Tenant.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url="/api/tenancy/tenants/",
             value_field="slug",

netbox/utilities/forms.py

@@ -8,7 +8,7 @@ from django import forms
 from django.conf import settings
 from django.contrib.postgres.forms.jsonb import JSONField as _JSONField, InvalidJSONInput
 from django.db.models import Count
-from mptt.forms import TreeNodeMultipleChoiceField
+from django.forms import BoundField
 
 from .choices import unpack_grouped_choices
 from .constants import *
@@ -211,7 +211,7 @@ class SelectWithPK(StaticSelect2):
     option_template_name = 'widgets/select_option_with_pk.html'
 
 
-class ContentTypeSelect(forms.Select):
+class ContentTypeSelect(StaticSelect2):
     """
     Appends an `api-value` attribute equal to the slugified model name for each ContentType. For example:
         <option value="37" api-value="console-server-port">console server port</option>
@@ -259,9 +259,6 @@ class APISelect(SelectWithDisabled):
         name of the query param and the value if the query param's value.
     :param null_option: If true, include the static null option in the selection list.
     """
-    # Only preload the selected option(s); new options are dynamically displayed and added via the API
-    template_name = 'widgets/select_api.html'
-
     def __init__(
         self,
         api_url,
@@ -581,46 +578,29 @@ class TagFilterField(forms.MultipleChoiceField):
         super().__init__(label='Tags', choices=get_choices, required=False, *args, **kwargs)
 
 
-class FilterChoiceIterator(forms.models.ModelChoiceIterator):
-
-    def __iter__(self):
-        # Filter on "empty" choice using FILTERS_NULL_CHOICE_VALUE (instead of an empty string)
-        if self.field.null_label is not None:
-            yield (settings.FILTERS_NULL_CHOICE_VALUE, self.field.null_label)
-        queryset = self.queryset.all()
-        # Can't use iterator() when queryset uses prefetch_related()
-        if not queryset._prefetch_related_lookups:
-            queryset = queryset.iterator()
-        for obj in queryset:
-            yield self.choice(obj)
-
-
-class FilterChoiceFieldMixin(object):
-    iterator = FilterChoiceIterator
-
-    def __init__(self, null_label=None, count_attr='filter_count', *args, **kwargs):
-        self.null_label = null_label
-        self.count_attr = count_attr
+class FilterChoiceField(forms.ModelMultipleChoiceField):
+    """
+    Override get_bound_field() to avoid pre-populating field choices with a SQL query. The field will be
+    rendered only with choices set via bound data. Choices are populated on-demand via the APISelect widget.
+    """
+    def __init__(self, *args, **kwargs):
+        # Filter fields are not required by default
         if 'required' not in kwargs:
             kwargs['required'] = False
-        if 'widget' not in kwargs:
-            kwargs['widget'] = forms.SelectMultiple(attrs={'size': 6})
         super().__init__(*args, **kwargs)
 
-    def label_from_instance(self, obj):
-        label = super().label_from_instance(obj)
-        obj_count = getattr(obj, self.count_attr, None)
-        if obj_count is not None:
-            return '{} ({})'.format(label, obj_count)
-        return label
-
-
-class FilterChoiceField(FilterChoiceFieldMixin, forms.ModelMultipleChoiceField):
-    pass
+    def get_bound_field(self, form, field_name):
+        bound_field = BoundField(form, self, field_name)
 
+        # Modify the QuerySet of the field before we return it. Limit choices to any data already bound: Options
+        # will be populated on-demand via the APISelect widget.
+        if bound_field.data:
+            kwargs = {'{}__in'.format(self.to_field_name or 'pk'): bound_field.data}
+            self.queryset = self.queryset.filter(**kwargs)
+        else:
+            self.queryset = self.queryset.none()
 
-class FilterTreeNodeMultipleChoiceField(FilterChoiceFieldMixin, TreeNodeMultipleChoiceField):
-    pass
+        return bound_field
 
 
 class LaxURLField(forms.URLField):

netbox/utilities/templates/widgets/select_api.html

@@ -1,9 +0,0 @@
-<select name="{{ widget.name }}"{% include "django/forms/widgets/attrs.html" %}>
-{% for group_name, group_choices, group_index in widget.optgroups %}
-  {% if group_name %}<optgroup label="{{ group_name }}">{% endif %}
-  {% for option in group_choices %}
-    {% if option.attrs.selected or option.value == "null" %}{% include option.template_name with widget=option %}{% endif %}
-  {% endfor %}
-  {% if group_name %}</optgroup>{% endif %}
-{% endfor %}
-</select>

netbox/virtualization/forms.py

@@ -213,7 +213,6 @@ class ClusterFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm
     site = FilterChoiceField(
         queryset=Site.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         required=False,
         widget=APISelectMultiple(
             api_url="/api/dcim/sites/",
@@ -224,7 +223,6 @@ class ClusterFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFilterForm
     group = FilterChoiceField(
         queryset=ClusterGroup.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         required=False,
         widget=APISelectMultiple(
             api_url="/api/virtualization/cluster-groups/",
@@ -562,7 +560,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
     cluster_group = FilterChoiceField(
         queryset=ClusterGroup.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url='/api/virtualization/cluster-groups/',
             value_field="slug",
@@ -572,7 +569,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
     cluster_type = FilterChoiceField(
         queryset=ClusterType.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url='/api/virtualization/cluster-types/',
             value_field="slug",
@@ -601,7 +597,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
     site = FilterChoiceField(
         queryset=Site.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url='/api/dcim/sites/',
             value_field="slug",
@@ -611,7 +606,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
     role = FilterChoiceField(
         queryset=DeviceRole.objects.filter(vm_role=True),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url='/api/dcim/device-roles/',
             value_field="slug",
@@ -629,7 +623,6 @@ class VirtualMachineFilterForm(BootstrapMixin, TenancyFilterForm, CustomFieldFil
     platform = FilterChoiceField(
         queryset=Platform.objects.all(),
         to_field_name='slug',
-        null_label='-- None --',
         widget=APISelectMultiple(
             api_url='/api/dcim/platforms/',
             value_field="slug",