From 6251bb5292341583f14ec5e3069754882557b07a Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 4 Mar 2020 20:52:32 +0100 Subject: [PATCH] Fix filterset multiple inheritance bug (#1131) * Fix filterset multiple inheritance bug * Ensure filter order under multiple inheritance --- django_filters/filterset.py | 26 ++++++++++++++--------- tests/test_filterset.py | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index 13460e580..0418adf2e 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -95,16 +95,22 @@ def get_declared_filters(cls, bases, attrs): filters.sort(key=lambda x: x[1].creation_counter) - # merge declared filters from base classes - for base in reversed(bases): - if hasattr(base, 'declared_filters'): - filters = [ - (name, f) for name, f - in base.declared_filters.items() - if name not in attrs - ] + filters - - return OrderedDict(filters) + # Ensures a base class field doesn't override cls attrs, and maintains + # field precedence when inheriting multiple parents. e.g. if there is a + # class C(A, B), and A and B both define 'field', use 'field' from A. + known = set(attrs) + + def visit(name): + known.add(name) + return name + + base_filters = [ + (visit(name), f) + for base in bases if hasattr(base, 'declared_filters') + for name, f in base.declared_filters.items() if name not in known + ] + + return OrderedDict(base_filters + filters) FILTER_FOR_DBFIELD_DEFAULTS = { diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 8fa7396f1..6930ce492 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -634,6 +634,48 @@ class Grandchild(Child): self.assertEqual(len(Child.base_filters), 1) self.assertEqual(len(Grandchild.base_filters), 1) + def test_declared_filter_multiple_inheritance(self): + class A(FilterSet): + f = CharFilter() + + class B(FilterSet): + f = NumberFilter() + + class F(A, B): + pass + + filters = {name: type(f) for name, f in F.declared_filters.items()} + self.assertEqual(filters, {'f': CharFilter}) + + def test_declared_filter_multiple_inheritance_field_ordering(self): + class Base(FilterSet): + f1 = CharFilter() + f2 = CharFilter() + + class A(Base): + f3 = NumberFilter() + + class B(FilterSet): + f3 = CharFilter() + f4 = CharFilter() + + class F(A, B): + f2 = NumberFilter() + f5 = CharFilter() + + fields = {name: type(f) for name, f in F.declared_filters.items()} + + # `NumberFilter`s should be the 'winners' in filter name conflicts + # - `F.f2` should override `Base.F2` + # - `A.f3` should override `B.f3` + assert fields == { + 'f1': CharFilter, + 'f2': NumberFilter, + 'f3': NumberFilter, + 'f4': CharFilter, + 'f5': CharFilter, + } + class FilterSetInstantiationTests(TestCase):