diff --git a/netbox/dcim/forms/__init__.py b/netbox/dcim/forms/__init__.py index 322abff9a..22f0b1204 100644 --- a/netbox/dcim/forms/__init__.py +++ b/netbox/dcim/forms/__init__.py @@ -1,4 +1,3 @@ -from .fields import * from .models import * from .filtersets import * from .object_create import * diff --git a/netbox/dcim/forms/fields.py b/netbox/dcim/forms/fields.py deleted file mode 100644 index 25a20667b..000000000 --- a/netbox/dcim/forms/fields.py +++ /dev/null @@ -1,25 +0,0 @@ -from django import forms -from netaddr import EUI -from netaddr.core import AddrFormatError - -__all__ = ( - 'MACAddressField', -) - - -class MACAddressField(forms.Field): - widget = forms.CharField - default_error_messages = { - 'invalid': 'MAC address must be in EUI-48 format', - } - - def to_python(self, value): - value = super().to_python(value) - - # Validate MAC address format - try: - value = EUI(value.strip()) - except AddrFormatError: - raise forms.ValidationError(self.error_messages['invalid'], code='invalid') - - return value diff --git a/netbox/extras/filters.py b/netbox/extras/filters.py index b37aaf40e..de739aa59 100644 --- a/netbox/extras/filters.py +++ b/netbox/extras/filters.py @@ -1,47 +1,11 @@ import django_filters -from django.forms import DateField, IntegerField, NullBooleanField from .models import Tag -from .choices import * __all__ = ( - 'CustomFieldFilter', 'TagFilter', ) -EXACT_FILTER_TYPES = ( - CustomFieldTypeChoices.TYPE_BOOLEAN, - CustomFieldTypeChoices.TYPE_DATE, - CustomFieldTypeChoices.TYPE_INTEGER, - CustomFieldTypeChoices.TYPE_SELECT, - CustomFieldTypeChoices.TYPE_MULTISELECT, -) - - -class CustomFieldFilter(django_filters.Filter): - """ - Filter objects by the presence of a CustomFieldValue. The filter's name is used as the CustomField name. - """ - def __init__(self, custom_field, *args, **kwargs): - self.custom_field = custom_field - - if custom_field.type == CustomFieldTypeChoices.TYPE_INTEGER: - self.field_class = IntegerField - elif custom_field.type == CustomFieldTypeChoices.TYPE_BOOLEAN: - self.field_class = NullBooleanField - elif custom_field.type == CustomFieldTypeChoices.TYPE_DATE: - self.field_class = DateField - - super().__init__(*args, **kwargs) - - self.field_name = f'custom_field_data__{self.field_name}' - - if custom_field.type == CustomFieldTypeChoices.TYPE_MULTISELECT: - self.lookup_expr = 'has_key' - elif custom_field.type not in EXACT_FILTER_TYPES: - if custom_field.filter_logic == CustomFieldFilterLogicChoices.FILTER_LOOSE: - self.lookup_expr = 'icontains' - class TagFilter(django_filters.ModelMultipleChoiceFilter): """ diff --git a/netbox/extras/filtersets.py b/netbox/extras/filtersets.py index af8d904f4..0d44eab57 100644 --- a/netbox/extras/filtersets.py +++ b/netbox/extras/filtersets.py @@ -26,13 +26,6 @@ __all__ = ( 'WebhookFilterSet', ) -EXACT_FILTER_TYPES = ( - CustomFieldTypeChoices.TYPE_BOOLEAN, - CustomFieldTypeChoices.TYPE_DATE, - CustomFieldTypeChoices.TYPE_INTEGER, - CustomFieldTypeChoices.TYPE_SELECT, -) - class WebhookFilterSet(BaseFilterSet): content_types = ContentTypeFilter() diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index bc6458039..84ba13263 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -1,6 +1,7 @@ import re from datetime import datetime, date +import django_filters from django import forms from django.contrib.contenttypes.models import ContentType from django.contrib.postgres.fields import ArrayField @@ -12,6 +13,7 @@ from django.utils.safestring import mark_safe from extras.choices import * from extras.utils import FeatureQuery, extras_features from netbox.models import ChangeLoggedModel +from utilities import filters from utilities.forms import ( CSVChoiceField, DatePicker, LaxURLField, StaticSelectMultiple, StaticSelect, add_blank_choice, ) @@ -308,6 +310,58 @@ class CustomField(ChangeLoggedModel): return field + def to_filter(self, lookup_expr=None): + """ + Return a django_filters Filter instance suitable for this field type. + + :param lookup_expr: Custom lookup expression (optional) + """ + kwargs = { + 'field_name': f'custom_field_data__{self.name}' + } + if lookup_expr is not None: + kwargs['lookup_expr'] = lookup_expr + + # Text/URL + if self.type in ( + CustomFieldTypeChoices.TYPE_TEXT, + CustomFieldTypeChoices.TYPE_LONGTEXT, + CustomFieldTypeChoices.TYPE_URL, + ): + filter_class = filters.MultiValueCharFilter + if self.filter_logic == CustomFieldFilterLogicChoices.FILTER_LOOSE: + kwargs['lookup_expr'] = 'icontains' + + # Integer + elif self.type == CustomFieldTypeChoices.TYPE_INTEGER: + filter_class = filters.MultiValueNumberFilter + + # Boolean + elif self.type == CustomFieldTypeChoices.TYPE_BOOLEAN: + filter_class = django_filters.BooleanFilter + + # Date + elif self.type == CustomFieldTypeChoices.TYPE_DATE: + filter_class = filters.MultiValueDateFilter + + # Select + elif self.type == CustomFieldTypeChoices.TYPE_SELECT: + filter_class = filters.MultiValueCharFilter + + # Multiselect + elif self.type == CustomFieldTypeChoices.TYPE_MULTISELECT: + filter_class = filters.MultiValueCharFilter + kwargs['lookup_expr'] = 'has_key' + + # Unsupported custom field type + else: + return None + + filter_instance = filter_class(**kwargs) + filter_instance.custom_field = self + + return filter_instance + def validate(self, value): """ Validate a value according to the field's type validation rules. diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index 0b51a4de3..5a9c4257f 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -719,7 +719,7 @@ class CustomFieldModelTest(TestCase): site.clean() -class CustomFieldFilterTest(TestCase): +class CustomFieldModelFilterTest(TestCase): queryset = Site.objects.all() filterset = SiteFilterSet @@ -772,7 +772,7 @@ class CustomFieldFilterTest(TestCase): cf.content_types.set([obj_type]) # Multiselect filtering - cf = CustomField(name='cf9', type=CustomFieldTypeChoices.TYPE_MULTISELECT, choices=['A', 'AA', 'B', 'C']) + cf = CustomField(name='cf9', type=CustomFieldTypeChoices.TYPE_MULTISELECT, choices=['A', 'B', 'C', 'X']) cf.save() cf.content_types.set([obj_type]) @@ -783,49 +783,88 @@ class CustomFieldFilterTest(TestCase): 'cf3': 'foo', 'cf4': 'foo', 'cf5': '2016-06-26', - 'cf6': 'http://foo.example.com/', - 'cf7': 'http://foo.example.com/', + 'cf6': 'http://a.example.com', + 'cf7': 'http://a.example.com', 'cf8': 'Foo', - 'cf9': ['A', 'B'], + 'cf9': ['A', 'X'], }), Site(name='Site 2', slug='site-2', custom_field_data={ 'cf1': 200, - 'cf2': False, + 'cf2': True, 'cf3': 'foobar', 'cf4': 'foobar', 'cf5': '2016-06-27', - 'cf6': 'http://bar.example.com/', - 'cf7': 'http://bar.example.com/', + 'cf6': 'http://b.example.com', + 'cf7': 'http://b.example.com', 'cf8': 'Bar', - 'cf9': ['AA', 'B'], + 'cf9': ['B', 'X'], + }), + Site(name='Site 3', slug='site-3', custom_field_data={ + 'cf1': 300, + 'cf2': False, + 'cf3': 'bar', + 'cf4': 'bar', + 'cf5': '2016-06-28', + 'cf6': 'http://c.example.com', + 'cf7': 'http://c.example.com', + 'cf8': 'Baz', + 'cf9': ['C', 'X'], }), - Site(name='Site 3', slug='site-3'), ]) def test_filter_integer(self): - self.assertEqual(self.filterset({'cf_cf1': 100}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf1': [100, 200]}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf1__n': [200]}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf1__gt': [200]}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf1__gte': [200]}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf1__lt': [200]}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf1__lte': [200]}, self.queryset).qs.count(), 2) def test_filter_boolean(self): - self.assertEqual(self.filterset({'cf_cf2': True}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf2': True}, self.queryset).qs.count(), 2) self.assertEqual(self.filterset({'cf_cf2': False}, self.queryset).qs.count(), 1) - def test_filter_text(self): - self.assertEqual(self.filterset({'cf_cf3': 'foo'}, self.queryset).qs.count(), 1) - self.assertEqual(self.filterset({'cf_cf4': 'foo'}, self.queryset).qs.count(), 2) + def test_filter_text_strict(self): + self.assertEqual(self.filterset({'cf_cf3': ['foo']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf3__n': ['foo']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf3__ic': ['foo']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf3__nic': ['foo']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf3__isw': ['foo']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf3__nisw': ['foo']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf3__iew': ['bar']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf3__niew': ['bar']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf3__ie': ['FOO']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf3__nie': ['FOO']}, self.queryset).qs.count(), 2) + + def test_filter_text_loose(self): + self.assertEqual(self.filterset({'cf_cf4': ['foo']}, self.queryset).qs.count(), 2) def test_filter_date(self): - self.assertEqual(self.filterset({'cf_cf5': '2016-06-26'}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf5': ['2016-06-26', '2016-06-27']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf5__n': ['2016-06-27']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf5__gt': ['2016-06-27']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf5__gte': ['2016-06-27']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf5__lt': ['2016-06-27']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf5__lte': ['2016-06-27']}, self.queryset).qs.count(), 2) - def test_filter_url(self): - self.assertEqual(self.filterset({'cf_cf6': 'http://foo.example.com/'}, self.queryset).qs.count(), 1) - self.assertEqual(self.filterset({'cf_cf7': 'example.com'}, self.queryset).qs.count(), 2) + def test_filter_url_strict(self): + self.assertEqual(self.filterset({'cf_cf6': ['http://a.example.com', 'http://b.example.com']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf6__n': ['http://b.example.com']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf6__ic': ['b']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf6__nic': ['b']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf6__isw': ['http://']}, self.queryset).qs.count(), 3) + self.assertEqual(self.filterset({'cf_cf6__nisw': ['http://']}, self.queryset).qs.count(), 0) + self.assertEqual(self.filterset({'cf_cf6__iew': ['.com']}, self.queryset).qs.count(), 3) + self.assertEqual(self.filterset({'cf_cf6__niew': ['.com']}, self.queryset).qs.count(), 0) + self.assertEqual(self.filterset({'cf_cf6__ie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 1) + self.assertEqual(self.filterset({'cf_cf6__nie': ['HTTP://A.EXAMPLE.COM']}, self.queryset).qs.count(), 2) + + def test_filter_url_loose(self): + self.assertEqual(self.filterset({'cf_cf7': ['example.com']}, self.queryset).qs.count(), 3) def test_filter_select(self): - self.assertEqual(self.filterset({'cf_cf8': 'Foo'}, self.queryset).qs.count(), 1) - self.assertEqual(self.filterset({'cf_cf8': 'Bar'}, self.queryset).qs.count(), 1) - self.assertEqual(self.filterset({'cf_cf8': 'Baz'}, self.queryset).qs.count(), 0) + self.assertEqual(self.filterset({'cf_cf8': ['Foo', 'Bar']}, self.queryset).qs.count(), 2) def test_filter_multiselect(self): - self.assertEqual(self.filterset({'cf_cf9': 'A'}, self.queryset).qs.count(), 1) - self.assertEqual(self.filterset({'cf_cf9': 'B'}, self.queryset).qs.count(), 2) - self.assertEqual(self.filterset({'cf_cf9': 'C'}, self.queryset).qs.count(), 0) + self.assertEqual(self.filterset({'cf_cf9': ['A', 'B']}, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset({'cf_cf9': ['X']}, self.queryset).qs.count(), 3) diff --git a/netbox/netbox/filtersets.py b/netbox/netbox/filtersets.py index 791c21d19..f42ab064b 100644 --- a/netbox/netbox/filtersets.py +++ b/netbox/netbox/filtersets.py @@ -2,19 +2,19 @@ import django_filters from copy import deepcopy from django.contrib.contenttypes.models import ContentType from django.db import models +from django_filters.exceptions import FieldLookupError from django_filters.utils import get_model_field, resolve_field -from dcim.forms import MACAddressField from extras.choices import CustomFieldFilterLogicChoices -from extras.filters import CustomFieldFilter, TagFilter +from extras.filters import TagFilter from extras.models import CustomField from utilities.constants import ( FILTER_CHAR_BASED_LOOKUP_MAP, FILTER_NEGATION_LOOKUP_MAP, FILTER_TREENODE_NEGATION_LOOKUP_MAP, FILTER_NUMERIC_BASED_LOOKUP_MAP ) +from utilities.forms import MACAddressField from utilities import filters - __all__ = ( 'BaseFilterSet', 'ChangeLoggedModelFilterSet', @@ -84,6 +84,7 @@ class BaseFilterSet(django_filters.FilterSet): def _get_filter_lookup_dict(existing_filter): # Choose the lookup expression map based on the filter type if isinstance(existing_filter, ( + django_filters.NumberFilter, filters.MultiValueDateFilter, filters.MultiValueDateTimeFilter, filters.MultiValueNumberFilter, @@ -115,6 +116,63 @@ class BaseFilterSet(django_filters.FilterSet): return None + @classmethod + def get_additional_lookups(cls, existing_filter_name, existing_filter): + new_filters = {} + + # Skip nonstandard lookup expressions + if existing_filter.method is not None or existing_filter.lookup_expr not in ['exact', 'in']: + return {} + + # Choose the lookup expression map based on the filter type + lookup_map = cls._get_filter_lookup_dict(existing_filter) + if lookup_map is None: + # Do not augment this filter type with more lookup expressions + return {} + + # Get properties of the existing filter for later use + field_name = existing_filter.field_name + field = get_model_field(cls._meta.model, field_name) + + # Create new filters for each lookup expression in the map + for lookup_name, lookup_expr in lookup_map.items(): + new_filter_name = f'{existing_filter_name}__{lookup_name}' + + try: + if existing_filter_name in cls.declared_filters: + # The filter field has been explicitly defined on the filterset class so we must manually + # create the new filter with the same type because there is no guarantee the defined type + # is the same as the default type for the field + resolve_field(field, lookup_expr) # Will raise FieldLookupError if the lookup is invalid + new_filter = type(existing_filter)( + field_name=field_name, + lookup_expr=lookup_expr, + label=existing_filter.label, + exclude=existing_filter.exclude, + distinct=existing_filter.distinct, + **existing_filter.extra + ) + elif hasattr(existing_filter, 'custom_field'): + # Filter is for a custom field + custom_field = existing_filter.custom_field + new_filter = custom_field.to_filter(lookup_expr=lookup_expr) + else: + # The filter field is listed in Meta.fields so we can safely rely on default behaviour + # Will raise FieldLookupError if the lookup is invalid + new_filter = cls.filter_for_field(field, field_name, lookup_expr) + except FieldLookupError: + # The filter could not be created because the lookup expression is not supported on the field + continue + + if lookup_name.startswith('n'): + # This is a negation filter which requires a queryset.exclude() clause + # Of course setting the negation of the existing filter's exclude attribute handles both cases + new_filter.exclude = not existing_filter.exclude + + new_filters[new_filter_name] = new_filter + + return new_filters + @classmethod def get_filters(cls): """ @@ -125,59 +183,12 @@ class BaseFilterSet(django_filters.FilterSet): """ filters = super().get_filters() - new_filters = {} + additional_filters = {} for existing_filter_name, existing_filter in filters.items(): - # Loop over existing filters to extract metadata by which to create new filters + additional_filters.update(cls.get_additional_lookups(existing_filter_name, existing_filter)) - # If the filter makes use of a custom filter method or lookup expression skip it - # as we cannot sanely handle these cases in a generic mannor - if existing_filter.method is not None or existing_filter.lookup_expr not in ['exact', 'in']: - continue + filters.update(additional_filters) - # Choose the lookup expression map based on the filter type - lookup_map = cls._get_filter_lookup_dict(existing_filter) - if lookup_map is None: - # Do not augment this filter type with more lookup expressions - continue - - # Get properties of the existing filter for later use - field_name = existing_filter.field_name - field = get_model_field(cls._meta.model, field_name) - - # Create new filters for each lookup expression in the map - for lookup_name, lookup_expr in lookup_map.items(): - new_filter_name = '{}__{}'.format(existing_filter_name, lookup_name) - - try: - if existing_filter_name in cls.declared_filters: - # The filter field has been explicity defined on the filterset class so we must manually - # create the new filter with the same type because there is no guarantee the defined type - # is the same as the default type for the field - resolve_field(field, lookup_expr) # Will raise FieldLookupError if the lookup is invalid - new_filter = type(existing_filter)( - field_name=field_name, - lookup_expr=lookup_expr, - label=existing_filter.label, - exclude=existing_filter.exclude, - distinct=existing_filter.distinct, - **existing_filter.extra - ) - else: - # The filter field is listed in Meta.fields so we can safely rely on default behaviour - # Will raise FieldLookupError if the lookup is invalid - new_filter = cls.filter_for_field(field, field_name, lookup_expr) - except django_filters.exceptions.FieldLookupError: - # The filter could not be created because the lookup expression is not supported on the field - continue - - if lookup_name.startswith('n'): - # This is a negation filter which requires a queryset.exclude() clause - # Of course setting the negation of the existing filter's exclude attribute handles both cases - new_filter.exclude = not existing_filter.exclude - - new_filters[new_filter_name] = new_filter - - filters.update(new_filters) return filters @@ -213,8 +224,19 @@ class PrimaryModelFilterSet(ChangeLoggedModelFilterSet): ).exclude( filter_logic=CustomFieldFilterLogicChoices.FILTER_DISABLED ) - for cf in custom_fields: - self.filters['cf_{}'.format(cf.name)] = CustomFieldFilter(field_name=cf.name, custom_field=cf) + + custom_field_filters = {} + for custom_field in custom_fields: + filter_name = f'cf_{custom_field.name}' + filter_instance = custom_field.to_filter() + if filter_instance: + custom_field_filters[filter_name] = filter_instance + + # Add relevant additional lookups + additional_lookups = self.get_additional_lookups(filter_name, filter_instance) + custom_field_filters.update(additional_lookups) + + self.filters.update(custom_field_filters) class OrganizationalModelFilterSet(PrimaryModelFilterSet): diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 8dac65aac..fe4bae3b4 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -3,7 +3,7 @@ from django import forms from django.conf import settings from django_filters.constants import EMPTY_VALUES -from dcim.forms import MACAddressField +from utilities.forms import MACAddressField def multivalue_field_factory(field_class): diff --git a/netbox/utilities/forms/fields.py b/netbox/utilities/forms/fields.py index 2561c2e22..332da9ed9 100644 --- a/netbox/utilities/forms/fields.py +++ b/netbox/utilities/forms/fields.py @@ -2,6 +2,7 @@ import csv import json import re from io import StringIO +from netaddr import AddrFormatError, EUI import django_filters from django import forms @@ -38,6 +39,7 @@ __all__ = ( 'ExpandableNameField', 'JSONField', 'LaxURLField', + 'MACAddressField', 'SlugField', 'TagFilterField', ) @@ -129,6 +131,28 @@ class JSONField(_JSONField): return json.dumps(value, sort_keys=True, indent=4) +class MACAddressField(forms.Field): + widget = forms.CharField + default_error_messages = { + 'invalid': 'MAC address must be in EUI-48 format', + } + + def to_python(self, value): + value = super().to_python(value) + + # Validate MAC address format + try: + value = EUI(value.strip()) + except AddrFormatError: + raise forms.ValidationError(self.error_messages['invalid'], code='invalid') + + return value + + +# +# Content type fields +# + class ContentTypeChoiceMixin: def __init__(self, queryset, *args, **kwargs):