From 6d84cb7d3beb9bbe0b5338a3cfe20e08a4bdd237 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Thu, 10 Oct 2019 23:07:38 -0700 Subject: [PATCH 1/3] Expand declared filtering tests - Test declared filter ordering - Test multiple inheritance --- tests/test_serializer.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index 0d4b50c1dd..af786158b5 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -682,3 +682,39 @@ class Grandchild(Child): assert len(Parent().get_fields()) == 2 assert len(Child().get_fields()) == 2 assert len(Grandchild().get_fields()) == 2 + + def test_multiple_inheritance(self): + class A(serializers.Serializer): + field = serializers.CharField() + + class B(serializers.Serializer): + field = serializers.IntegerField() + + class TestSerializer(A, B): + pass + + fields = { + name: type(f) for name, f + in TestSerializer()._declared_fields.items() + } + assert fields == { + 'field': serializers.CharField + } + + def test_field_ordering(self): + class Base(serializers.Serializer): + f1 = serializers.CharField() + f2 = serializers.CharField() + + class A(Base): + f3 = serializers.CharField() + + class B(serializers.Serializer): + f4 = serializers.CharField() + + class TestSerializer(A, B): + f2 = serializers.CharField() + f5 = serializers.CharField() + + field_names = list(TestSerializer()._declared_fields) + assert field_names == ['f1', 'f2', 'f3', 'f4', 'f5'] From 2d72436bfc26a2b24525d764e47939cdf31015f8 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 11 Oct 2019 01:05:10 -0700 Subject: [PATCH 2/3] Fix serializer multiple inheritance bug --- rest_framework/serializers.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 01f34298b4..0ff1e3973c 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -299,18 +299,22 @@ def _get_declared_fields(cls, bases, attrs): if isinstance(obj, Field)] fields.sort(key=lambda x: x[1]._creation_counter) - # If this class is subclassing another Serializer, add that Serializer's - # fields. Note that we loop over the bases in *reverse*. This is necessary - # in order to maintain the correct order of fields. - for base in reversed(bases): - if hasattr(base, '_declared_fields'): - fields = [ - (field_name, obj) for field_name, obj - in base._declared_fields.items() - if field_name not in attrs - ] + fields - - return OrderedDict(fields) + # 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_fields = [ + (visit(name), f) + for base in bases if hasattr(base, '_declared_fields') + for name, f in base._declared_fields.items() if name not in known + ] + + return OrderedDict(base_fields + fields) def __new__(cls, name, bases, attrs): attrs['_declared_fields'] = cls._get_declared_fields(bases, attrs) From 51915583e2f706605b6c96e0b09c3daf3ebec13c Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 11 Oct 2019 14:41:26 -0700 Subject: [PATCH 3/3] Improve field order test to check for field types --- tests/test_serializer.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index af786158b5..ea1139def5 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -698,7 +698,7 @@ class TestSerializer(A, B): in TestSerializer()._declared_fields.items() } assert fields == { - 'field': serializers.CharField + 'field': serializers.CharField, } def test_field_ordering(self): @@ -707,14 +707,28 @@ class Base(serializers.Serializer): f2 = serializers.CharField() class A(Base): - f3 = serializers.CharField() + f3 = serializers.IntegerField() class B(serializers.Serializer): + f3 = serializers.CharField() f4 = serializers.CharField() class TestSerializer(A, B): - f2 = serializers.CharField() + f2 = serializers.IntegerField() f5 = serializers.CharField() - field_names = list(TestSerializer()._declared_fields) - assert field_names == ['f1', 'f2', 'f3', 'f4', 'f5'] + fields = { + name: type(f) for name, f + in TestSerializer()._declared_fields.items() + } + + # `IntegerField`s should be the 'winners' in field name conflicts + # - `TestSerializer.f2` should override `Base.F2` + # - `A.f3` should override `B.f3` + assert fields == { + 'f1': serializers.CharField, + 'f2': serializers.IntegerField, + 'f3': serializers.IntegerField, + 'f4': serializers.CharField, + 'f5': serializers.CharField, + }