Skip to content

Closes #4108: Extraneous queryset evaluation by FilterChoiceFields #4128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/version-2.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion netbox/circuits/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 1 addition & 13 deletions netbox/dcim/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions netbox/extras/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
8 changes: 0 additions & 8 deletions netbox/ipam/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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",
Expand Down
3 changes: 0 additions & 3 deletions netbox/tenancy/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
58 changes: 19 additions & 39 deletions netbox/utilities/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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 *
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -581,46 +578,29 @@ def get_choices():
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):
Expand Down
9 changes: 0 additions & 9 deletions netbox/utilities/templates/widgets/select_api.html

This file was deleted.

7 changes: 0 additions & 7 deletions netbox/virtualization/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand All @@ -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/",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down