Merge pull request #7678 from netbox-community/6615-custom-field-filters

Closes #6615: Enable filter lookups for custom fields

netbox/dcim/forms/__init__.py

@@ -1,4 +1,3 @@
-from .fields import *
 from .models import *
 from .filtersets import *
 from .object_create import *

netbox/dcim/forms/fields.py

@@ -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

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):
     """

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()

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.

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)
-
-    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)
+        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_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)

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
-
-            # 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
+            additional_filters.update(cls.get_additional_lookups(existing_filter_name, existing_filter))
 
-            # 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
+        filters.update(additional_filters)
 
-            # 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):

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):

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):