From 41aa7cb1c1df5a27496240152a2a314caa1dee76 Mon Sep 17 00:00:00 2001 From: Loes Crama Date: Wed, 10 Jul 2024 13:26:50 +0200 Subject: [PATCH 1/4] Add unknown_field_behavior feature --- django_filters/filterset.py | 33 ++++++++++++++++++----- tests/test_filterset.py | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index 6c7019e2..191118ee 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -1,4 +1,5 @@ import copy +import warnings from collections import OrderedDict from django import forms @@ -53,6 +54,8 @@ def __init__(self, options=None): self.form = getattr(options, "form", forms.Form) + self.unknown_field_behavior = getattr(options, "unknown_field_behavior", "raise") + class FilterSetMetaclass(type): def __new__(cls, name, bases, attrs): @@ -338,9 +341,11 @@ def get_filters(cls): continue if field is not None: - filters[filter_name] = cls.filter_for_field( + filter_instance = cls.filter_for_field( field, field_name, lookup_expr ) + if filter_instance is not None: + filters[filter_name] = filter_instance # Allow Meta.fields to contain declared filters *only* when a list/tuple if isinstance(cls._meta.fields, (list, tuple)): @@ -357,6 +362,18 @@ def get_filters(cls): filters.update(cls.declared_filters) return filters + @classmethod + def handle_unrecognized_field(cls, field_name, message): + behavior = cls._meta.unknown_field_behavior + if behavior == "raise": + raise AssertionError(message) + elif behavior == "warn": + warnings.warn(f"Unrecognized field type for '{field_name}'. Field will be ignored.") + elif behavior == "ignore": + pass + else: + raise ValueError(f"Invalid unknown_field_behavior: {behavior}") + @classmethod def filter_for_field(cls, field, field_name, lookup_expr=None): if lookup_expr is None: @@ -371,12 +388,14 @@ def filter_for_field(cls, field, field_name, lookup_expr=None): filter_class, params = cls.filter_for_lookup(field, lookup_type) default.update(params) - assert filter_class is not None, ( - "%s resolved field '%s' with '%s' lookup to an unrecognized field " - "type %s. Try adding an override to 'Meta.filter_overrides'. See: " - "https://django-filter.readthedocs.io/en/main/ref/filterset.html" - "#customise-filter-generation-with-filter-overrides" - ) % (cls.__name__, field_name, lookup_expr, field.__class__.__name__) + if filter_class is None: + cls.handle_unrecognized_field(field_name, ( + "%s resolved field '%s' with '%s' lookup to an unrecognized field " + "type %s. Try adding an override to 'Meta.filter_overrides'. See: " + "https://django-filter.readthedocs.io/en/main/ref/filterset.html" + "#customise-filter-generation-with-filter-overrides" + ) % (cls.__name__, field_name, lookup_expr, field.__class__.__name__)) + return None return filter_class(**default) diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 706eda25..3deede45 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -1,4 +1,5 @@ import unittest +import warnings from unittest import mock from django.db import models @@ -147,6 +148,7 @@ def test_field_that_is_subclassed(self): def test_unknown_field_type_error(self): f = NetworkSetting._meta.get_field("mask") + FilterSet._meta.unknown_field_behavior = "raise" with self.assertRaises(AssertionError) as excinfo: FilterSet.filter_for_field(f, "mask") @@ -157,6 +159,14 @@ def test_unknown_field_type_error(self): excinfo.exception.args[0], ) + def test_return_none(self): + f = NetworkSetting._meta.get_field("mask") + # Set unknown_field_behavior to 'ignore' to avoid raising exceptions + FilterSet._meta.unknown_field_behavior = "ignore" + result = FilterSet.filter_for_field(f, "mask") + + self.assertIsNone(result) + def test_symmetrical_selfref_m2m_field(self): f = Node._meta.get_field("adjacents") result = FilterSet.filter_for_field(f, "adjacents") @@ -202,6 +212,48 @@ def test_filter_overrides(self): pass +class TestHandleUnknownField(TestCase): + def setUp(self): + class NetworkSettingFilterSet(FilterSet): + class Meta: + model = NetworkSetting + fields = ["ip", "mask"] + # Initial field behavior set to 'ignore' to avoid crashing in setUp + unknown_field_behavior = "ignore" + + self.FilterSet = NetworkSettingFilterSet + + def test_raise_unknown_field_behavior(self): + self.FilterSet._meta.unknown_field_behavior = "raise" + + with self.assertRaises(AssertionError): + self.FilterSet.handle_unrecognized_field("mask", "test_message") + + def test_unknown_field_warn_behavior(self): + self.FilterSet._meta.unknown_field_behavior = "warn" + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.FilterSet.handle_unrecognized_field("mask", "test_message") + + self.assertIn( + "Unrecognized field type for 'mask'. " + "Field will be ignored.", + str(w[-1].message), + ) + + def test_unknown_field_ignore_behavior(self): + # No exception or warning should be raised + self.FilterSet._meta.unknown_field_behavior = "ignore" + self.FilterSet.handle_unrecognized_field("mask", "test_message") + + def test_unknown_field_invalid_behavior(self): + self.FilterSet._meta.unknown_field_behavior = "invalid" + + with self.assertRaises(ValueError): + self.FilterSet.handle_unrecognized_field("mask", "test_message") + + class FilterSetFilterForLookupTests(TestCase): def test_filter_for_ISNULL_lookup(self): f = Article._meta.get_field("author") From f5b6fc62cb07b9852866bb96b2fdcedfc84fdac6 Mon Sep 17 00:00:00 2001 From: Loes Crama Date: Thu, 11 Jul 2024 12:47:58 +0200 Subject: [PATCH 2/4] Use Enum for unknown field options, add early validation for invalid behavior --- django_filters/__init__.py | 2 +- django_filters/filterset.py | 22 ++++++++++++++++------ tests/test_filterset.py | 29 +++++++++++++++++++---------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/django_filters/__init__.py b/django_filters/__init__.py index ca1f82d7..a9ad1c21 100644 --- a/django_filters/__init__.py +++ b/django_filters/__init__.py @@ -2,7 +2,7 @@ from importlib import util as importlib_util from .filters import * -from .filterset import FilterSet +from .filterset import FilterSet, UnknownFieldBehavior # We make the `rest_framework` module available without an additional import. # If DRF is not installed, no-op. diff --git a/django_filters/filterset.py b/django_filters/filterset.py index 191118ee..45e7bd38 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -1,6 +1,7 @@ import copy import warnings from collections import OrderedDict +from enum import Enum from django import forms from django.db import models @@ -44,6 +45,12 @@ def remote_queryset(field): return model._default_manager.complex_filter(limit_choices_to) +class UnknownFieldBehavior(Enum): + RAISE = "raise" + WARN = "warn" + IGNORE = "ignore" + + class FilterSetOptions: def __init__(self, options=None): self.model = getattr(options, "model", None) @@ -54,7 +61,12 @@ def __init__(self, options=None): self.form = getattr(options, "form", forms.Form) - self.unknown_field_behavior = getattr(options, "unknown_field_behavior", "raise") + behavior = getattr(options, "unknown_field_behavior", UnknownFieldBehavior.RAISE) + + if not isinstance(behavior, UnknownFieldBehavior): + raise ValueError(f"Invalid unknown_field_behavior: {behavior}") + + self.unknown_field_behavior = behavior class FilterSetMetaclass(type): @@ -365,14 +377,12 @@ def get_filters(cls): @classmethod def handle_unrecognized_field(cls, field_name, message): behavior = cls._meta.unknown_field_behavior - if behavior == "raise": + if behavior == UnknownFieldBehavior.RAISE: raise AssertionError(message) - elif behavior == "warn": + elif behavior == UnknownFieldBehavior.WARN: warnings.warn(f"Unrecognized field type for '{field_name}'. Field will be ignored.") - elif behavior == "ignore": + elif behavior == UnknownFieldBehavior.IGNORE: pass - else: - raise ValueError(f"Invalid unknown_field_behavior: {behavior}") @classmethod def filter_for_field(cls, field, field_name, lookup_expr=None): diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 3deede45..0af72b1c 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -22,6 +22,7 @@ UUIDFilter, ) from django_filters.filterset import ( + UnknownFieldBehavior, FILTER_FOR_DBFIELD_DEFAULTS, FilterSet, filterset_factory, @@ -148,7 +149,7 @@ def test_field_that_is_subclassed(self): def test_unknown_field_type_error(self): f = NetworkSetting._meta.get_field("mask") - FilterSet._meta.unknown_field_behavior = "raise" + FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.RAISE with self.assertRaises(AssertionError) as excinfo: FilterSet.filter_for_field(f, "mask") @@ -211,26 +212,25 @@ def test_modified_default_lookup(self): def test_filter_overrides(self): pass - -class TestHandleUnknownField(TestCase): +class HandleUnknownFieldTests(TestCase): def setUp(self): class NetworkSettingFilterSet(FilterSet): class Meta: model = NetworkSetting fields = ["ip", "mask"] # Initial field behavior set to 'ignore' to avoid crashing in setUp - unknown_field_behavior = "ignore" + unknown_field_behavior = UnknownFieldBehavior.IGNORE self.FilterSet = NetworkSettingFilterSet def test_raise_unknown_field_behavior(self): - self.FilterSet._meta.unknown_field_behavior = "raise" + self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.RAISE with self.assertRaises(AssertionError): self.FilterSet.handle_unrecognized_field("mask", "test_message") def test_unknown_field_warn_behavior(self): - self.FilterSet._meta.unknown_field_behavior = "warn" + self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.WARN with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") @@ -244,14 +244,23 @@ def test_unknown_field_warn_behavior(self): def test_unknown_field_ignore_behavior(self): # No exception or warning should be raised - self.FilterSet._meta.unknown_field_behavior = "ignore" + self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.IGNORE self.FilterSet.handle_unrecognized_field("mask", "test_message") def test_unknown_field_invalid_behavior(self): - self.FilterSet._meta.unknown_field_behavior = "invalid" + # Creation of new custom FilterSet to set initial field behavior + with self.assertRaises(ValueError) as excinfo: - with self.assertRaises(ValueError): - self.FilterSet.handle_unrecognized_field("mask", "test_message") + class InvalidBehaviorFilterSet(FilterSet): + class Meta: + model = NetworkSetting + fields = ["ip", "mask"] + unknown_field_behavior = "invalid" + + self.assertIn( + "Invalid unknown_field_behavior: invalid", + str(excinfo.exception), + ) class FilterSetFilterForLookupTests(TestCase): From 167c5062e10180b23c0c56538786ed7e731f265d Mon Sep 17 00:00:00 2001 From: Loes Crama Date: Thu, 11 Jul 2024 16:23:18 +0200 Subject: [PATCH 3/4] Add check for invalid behavior value, add and clean up various related tests --- django_filters/filterset.py | 2 ++ tests/test_filterset.py | 35 ++++++++++++++++++++++++++--------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index 45e7bd38..23a88b45 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -383,6 +383,8 @@ def handle_unrecognized_field(cls, field_name, message): warnings.warn(f"Unrecognized field type for '{field_name}'. Field will be ignored.") elif behavior == UnknownFieldBehavior.IGNORE: pass + else: + raise ValueError(f"Invalid unknown_field_behavior: {behavior}") @classmethod def filter_for_field(cls, field, field_name, lookup_expr=None): diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 0af72b1c..77fafee5 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -22,9 +22,9 @@ UUIDFilter, ) from django_filters.filterset import ( - UnknownFieldBehavior, FILTER_FOR_DBFIELD_DEFAULTS, FilterSet, + UnknownFieldBehavior, filterset_factory, ) from django_filters.widgets import BooleanWidget @@ -163,7 +163,7 @@ def test_unknown_field_type_error(self): def test_return_none(self): f = NetworkSetting._meta.get_field("mask") # Set unknown_field_behavior to 'ignore' to avoid raising exceptions - FilterSet._meta.unknown_field_behavior = "ignore" + FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.IGNORE result = FilterSet.filter_for_field(f, "mask") self.assertIsNone(result) @@ -212,6 +212,7 @@ def test_modified_default_lookup(self): def test_filter_overrides(self): pass + class HandleUnknownFieldTests(TestCase): def setUp(self): class NetworkSettingFilterSet(FilterSet): @@ -226,9 +227,14 @@ class Meta: def test_raise_unknown_field_behavior(self): self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.RAISE - with self.assertRaises(AssertionError): + with self.assertRaises(AssertionError) as excinfo: self.FilterSet.handle_unrecognized_field("mask", "test_message") + self.assertIn( + "test_message", + excinfo.exception.args[0], + ) + def test_unknown_field_warn_behavior(self): self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.WARN @@ -236,18 +242,18 @@ def test_unknown_field_warn_behavior(self): warnings.simplefilter("always") self.FilterSet.handle_unrecognized_field("mask", "test_message") - self.assertIn( - "Unrecognized field type for 'mask'. " - "Field will be ignored.", - str(w[-1].message), - ) + self.assertIn( + "Unrecognized field type for 'mask'. " + "Field will be ignored.", + str(w[-1].message), + ) def test_unknown_field_ignore_behavior(self): # No exception or warning should be raised self.FilterSet._meta.unknown_field_behavior = UnknownFieldBehavior.IGNORE self.FilterSet.handle_unrecognized_field("mask", "test_message") - def test_unknown_field_invalid_behavior(self): + def test_unknown_field_invalid_initial_behavior(self): # Creation of new custom FilterSet to set initial field behavior with self.assertRaises(ValueError) as excinfo: @@ -262,6 +268,17 @@ class Meta: str(excinfo.exception), ) + def test_unknown_field_invalid_changed_option_behavior(self): + self.FilterSet._meta.unknown_field_behavior = "invalid" + + with self.assertRaises(ValueError) as excinfo: + self.FilterSet.handle_unrecognized_field("mask", "test_message") + + self.assertIn( + "Invalid unknown_field_behavior: invalid", + str(excinfo.exception), + ) + class FilterSetFilterForLookupTests(TestCase): def test_filter_for_ISNULL_lookup(self): From 9092589ef5960847494c5bd276870ab97c78752c Mon Sep 17 00:00:00 2001 From: Loes Crama Date: Fri, 12 Jul 2024 14:47:46 +0200 Subject: [PATCH 4/4] Add unknown_field_behavior option and example to docs --- docs/ref/filterset.txt | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/ref/filterset.txt b/docs/ref/filterset.txt index ecc9e558..6265a4b0 100644 --- a/docs/ref/filterset.txt +++ b/docs/ref/filterset.txt @@ -12,6 +12,7 @@ Meta options - :ref:`exclude ` - :ref:`form
` - :ref:`filter_overrides ` +- :ref:`unknown_field_behavior ` .. _model: @@ -146,6 +147,34 @@ This is a map of model fields to filter classes with options:: }, } + +.. _unknown_field_behavior: + +Handling unknown fields with ``unknown_field_behavior`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``unknown_field_behavior`` option specifies how unknown fields are handled +in a ``FilterSet``. You can set this option using the values of the +``UnknownFieldBehavior`` enum: + +- ``UnknownFieldBehavior.RAISE``: Raise an assertion error (default) +- ``UnknownFieldBehavior.WARN``: Issue a warning and ignore the field +- ``UnknownFieldBehavior.IGNORE``: Silently ignore the field + +Note that both the ``WARN`` and ``IGNORE`` options do not include the unknown +field(s) in the list of filters. + +.. code-block:: python + + from django_filters import UnknownFieldBehavior + + class UserFilter(django_filters.FilterSet): + class Meta: + model = User + fields = ['username', 'last_login'] + unknown_field_behavior = UnknownFieldBehavior.WARN + + Overriding ``FilterSet`` methods --------------------------------