From a06d6a67a4a616c1cf5c048fda08fa797e855709 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Wed, 21 Aug 2013 17:29:26 +1200 Subject: [PATCH 1/3] allow overriding of fields in subclassed serializers. --- rest_framework/serializers.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 31cfa34474..e5b5bf3188 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -110,6 +110,16 @@ def _get_declared_fields(bases, attrs): if hasattr(base, 'base_fields'): fields = list(base.base_fields.items()) + fields + # Allow overriding of fields in subclasses, by + # removing the superclass fields from this list. + # (otherwise SortedDict will squash the subclass' field and + # use the superclass field instead.) + seen = {} + for i, (name, field) in reversed(list(enumerate(fields))): + if name in seen: + fields.pop(seen[name]) + seen[name] = i + return SortedDict(fields) From 0f86629fb4792702e8683a1e9f8209e0183ab5e7 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Thu, 22 Aug 2013 10:59:47 +1200 Subject: [PATCH 2/3] add tests. i broke something --- rest_framework/tests/test_serializer.py | 69 +++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/rest_framework/tests/test_serializer.py b/rest_framework/tests/test_serializer.py index c24976603e..85afd8176d 100644 --- a/rest_framework/tests/test_serializer.py +++ b/rest_framework/tests/test_serializer.py @@ -1643,3 +1643,72 @@ def test_serializer_supports_slug_many_relationships(self): serializer = SimpleSlugSourceModelSerializer(data={'text': 'foo', 'targets': [1, 2]}) self.assertTrue(serializer.is_valid()) self.assertEqual(serializer.data, {'text': 'foo', 'targets': [1, 2]}) + + +### Regression test for #1053 + +class OverriddenFieldsBase1(serializers.Serializer): + a_field = serializers.CharField() + + +class OverriddenFieldsBase2(serializers.Serializer): + a_field = serializers.IntegerField() + + +class OverriddenFieldsWithSingleBase(OverriddenFieldsBase1): + a_field = serializers.FloatField() + + +class OverriddenFieldsMultipleBases1(OverriddenFieldsBase1, OverriddenFieldsBase2): + # first base takes precedence; a_field should be a CharField. + pass + + +class OverriddenFieldsMultipleBases2(OverriddenFieldsBase2, OverriddenFieldsBase1): + # first base takes precedence; a_field should be a IntegerField. + pass + + +class OverriddenFieldsMultipleBasesOverridden(OverriddenFieldsBase1, OverriddenFieldsBase2): + a_field = serializers.FloatField() + + +class SerializerSupportsOverriddenFields(TestCase): + def test_base_fields_unchanged(self): + self.assertIsInstance( + OverriddenFieldsBase1.base_fields['a_field'], + serializers.CharField, + ) + s = OverriddenFieldsBase1() + self.assertIsInstance(s.fields['a_field'], serializers.CharField) + + def test_overridden_fields_single_base(self): + self.assertIsInstance( + OverriddenFieldsWithSingleBase.base_fields['a_field'], + serializers.FloatField, + ) + s = OverriddenFieldsWithSingleBase() + self.assertIsInstance(s.fields['a_field'], serializers.FloatField) + + def test_overridden_fields_multiple_bases(self): + self.assertIsInstance( + OverriddenFieldsMultipleBases1.base_fields['a_field'], + serializers.CharField, + ) + s = OverriddenFieldsMultipleBases1() + self.assertIsInstance(s.fields['a_field'], serializers.CharField) + + self.assertIsInstance( + OverriddenFieldsMultipleBases2.base_fields['a_field'], + serializers.IntegerField, + ) + s = OverriddenFieldsMultipleBases2() + self.assertIsInstance(s.fields['a_field'], serializers.IntegerField) + + def test_overridden_fields_multiple_bases_overridden(self): + self.assertIsInstance( + OverriddenFieldsMultipleBasesOverridden.base_fields['a_field'], + serializers.FloatField, + ) + s = OverriddenFieldsMultipleBasesOverridden() + self.assertIsInstance(s.fields['a_field'], serializers.FloatField) From 7dec01d17423a63176a4ed889a6852328c0fff40 Mon Sep 17 00:00:00 2001 From: Craig de Stigter Date: Thu, 22 Aug 2013 11:36:43 +1200 Subject: [PATCH 3/3] fix a bug preventing field overriding in single base serializer subclassing --- rest_framework/serializers.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index e5b5bf3188..03880804a5 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -106,21 +106,29 @@ def _get_declared_fields(bases, attrs): # 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. + # Note: The 'seen' dict here ensures that the fields from the 'first' base + # take precedence over fields from later bases. + # (otherwise SortedDict will squash the first-base field and + # use the field from a later base instead.) + seen = set() + base_fields_map = {} + base_fields_order = [] for base in bases[::-1]: if hasattr(base, 'base_fields'): - fields = list(base.base_fields.items()) + fields - - # Allow overriding of fields in subclasses, by - # removing the superclass fields from this list. - # (otherwise SortedDict will squash the subclass' field and - # use the superclass field instead.) - seen = {} - for i, (name, field) in reversed(list(enumerate(fields))): - if name in seen: - fields.pop(seen[name]) - seen[name] = i - - return SortedDict(fields) + for name, field in base.base_fields.items(): + base_fields_map[name] = field + base_fields_order = list(base.base_fields.keys()) + base_fields_order + + seen = set() + base_fields = [] + for name in base_fields_order: + if name not in seen: + base_fields.append((name, base_fields_map[name])) + seen.add(name) + + # if there are fields in both base_fields and fields, SortedDict + # uses the *last* one defined. So fields needs to go last. + return SortedDict(base_fields + fields) class SerializerMetaclass(type):