From 37952254d73af40e8d6389c5d04c3e3356d47c6d Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Tue, 8 Oct 2019 22:19:42 -0700 Subject: [PATCH 1/2] Fix filterset multiple inheritance bug --- django_filters/filterset.py | 4 ++-- tests/test_filterset.py | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index a594a1229..51b08db8d 100644 --- a/django_filters/filterset.py +++ b/django_filters/filterset.py @@ -98,11 +98,11 @@ def get_declared_filters(cls, bases, attrs): # merge declared filters from base classes for base in reversed(bases): if hasattr(base, 'declared_filters'): - filters = [ + filters = filters + [ (name, f) for name, f in base.declared_filters.items() if name not in attrs - ] + filters + ] return OrderedDict(filters) diff --git a/tests/test_filterset.py b/tests/test_filterset.py index 23460eca6..085a4a9bc 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -619,6 +619,19 @@ 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}) + class FilterSetInstantiationTests(TestCase): From f7e7af3e180f8fa48abffc1b4946df0bef11c145 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Wed, 4 Mar 2020 11:40:32 -0800 Subject: [PATCH 2/2] Ensure filter order under multiple inheritance --- django_filters/filterset.py | 26 ++++++++++++++++---------- tests/test_filterset.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/django_filters/filterset.py b/django_filters/filterset.py index 51b08db8d..7f5e22013 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 = filters + [ - (name, f) for name, f - in base.declared_filters.items() - if name not in attrs - ] - - 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 085a4a9bc..9fc4adf6c 100644 --- a/tests/test_filterset.py +++ b/tests/test_filterset.py @@ -632,6 +632,35 @@ class F(A, B): 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):