Skip to content

Commit

Permalink
Allow missing fields option for inherited serializers. Closes #2388.
Browse files Browse the repository at this point in the history
  • Loading branch information
tomchristie committed Jan 21, 2015
1 parent fdeef89 commit da6ef3d
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 25 deletions.
2 changes: 1 addition & 1 deletion rest_framework/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def unicode_repr(instance):
# Get the repr of an instance, but ensure it is a unicode string
# on both python 3 (already the case) and 2 (not the case).
if six.PY2:
repr(instance).decode('utf-8')
return repr(instance).decode('utf-8')
return repr(instance)


Expand Down
32 changes: 19 additions & 13 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ def _get_declared_fields(cls, 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.
for base in bases[::-1]:
for base in reversed(bases):
if hasattr(base, '_declared_fields'):
fields = list(base._declared_fields.items()) + fields

Expand Down Expand Up @@ -880,8 +880,8 @@ def get_fields(self):
# Retrieve metadata about fields & relationships on the model class.
info = model_meta.get_field_info(model)

# Use the default set of field names if none is supplied explicitly.
if fields is None:
# Use the default set of field names if none is supplied explicitly.
fields = self._get_default_field_names(declared_fields, info)
exclude = getattr(self.Meta, 'exclude', None)
if exclude is not None:
Expand All @@ -891,6 +891,23 @@ def get_fields(self):
field_name
)
fields.remove(field_name)
else:
# Check that any fields declared on the class are
# also explicitly included in `Meta.fields`.

# Note that we ignore any fields that were declared on a parent
# class, in order to support only including a subset of fields
# when subclassing serializers.
declared_field_names = set(declared_fields.keys())
for cls in self.__class__.__bases__:
declared_field_names -= set(getattr(cls, '_declared_fields', []))

missing_fields = declared_field_names - set(fields)
assert not missing_fields, (
'Field `%s` has been declared on serializer `%s`, but '
'is missing from `Meta.fields`.' %
(list(missing_fields)[0], self.__class__.__name__)
)

# Determine the set of model fields, and the fields that they map to.
# We actually only need this to deal with the slightly awkward case
Expand Down Expand Up @@ -1024,17 +1041,6 @@ def get_fields(self):
(field_name, model.__class__.__name__)
)

# Check that any fields declared on the class are
# also explicitly included in `Meta.fields`.
missing_fields = set(declared_fields.keys()) - set(fields)
if missing_fields:
missing_field = list(missing_fields)[0]
raise ImproperlyConfigured(
'Field `%s` has been declared on serializer `%s`, but '
'is missing from `Meta.fields`.' %
(missing_field, self.__class__.__name__)
)

# Populate any kwargs defined in `Meta.extra_kwargs`
extras = extra_kwargs.get(field_name, {})
if extras.get('read_only', False):
Expand Down
3 changes: 3 additions & 0 deletions rest_framework/utils/serializer_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,6 @@ def __iter__(self):

def __len__(self):
return len(self.fields)

def __repr__(self):
return dict.__repr__(self.fields)
52 changes: 41 additions & 11 deletions tests/test_model_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
These tests deal with ensuring that we correctly map the model fields onto
an appropriate set of serializer fields for each case.
"""
from __future__ import unicode_literals
from django.core.exceptions import ImproperlyConfigured
from django.core.validators import MaxValueValidator, MinValueValidator, MinLengthValidator
from django.db import models
from django.test import TestCase
from django.utils import six
from rest_framework import serializers
from rest_framework.compat import unicode_repr


def dedent(blocktext):
Expand Down Expand Up @@ -124,7 +127,7 @@ class Meta:
url_field = URLField(max_length=100)
custom_field = ModelField(model_field=<tests.test_model_serializer.CustomField: custom_field>)
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_field_options(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -142,7 +145,14 @@ class Meta:
descriptive_field = IntegerField(help_text='Some help text', label='A label')
choices_field = ChoiceField(choices=[('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')])
""")
self.assertEqual(repr(TestSerializer()), expected)
if six.PY2:
# This particular case is too awkward to resolve fully across
# both py2 and py3.
expected = expected.replace(
"('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')",
"(u'red', u'Red'), (u'blue', u'Blue'), (u'green', u'Green')"
)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_method_field(self):
"""
Expand Down Expand Up @@ -221,14 +231,34 @@ class Meta:
model = RegularFieldsModel
fields = ('auto_field',)

with self.assertRaises(ImproperlyConfigured) as excinfo:
with self.assertRaises(AssertionError) as excinfo:
TestSerializer().fields
expected = (
'Field `missing` has been declared on serializer '
'`TestSerializer`, but is missing from `Meta.fields`.'
)
assert str(excinfo.exception) == expected

def test_missing_superclass_field(self):
"""
Fields that have been declared on a parent of the serializer class may
be excluded from the `Meta.fields` option.
"""
class TestSerializer(serializers.ModelSerializer):
missing = serializers.ReadOnlyField()

class Meta:
model = RegularFieldsModel

class ChildSerializer(TestSerializer):
missing = serializers.ReadOnlyField()

class Meta:
model = RegularFieldsModel
fields = ('auto_field',)

ChildSerializer().fields


# Tests for relational field mappings.
# ------------------------------------
Expand Down Expand Up @@ -276,7 +306,7 @@ class Meta:
many_to_many = PrimaryKeyRelatedField(many=True, queryset=ManyToManyTargetModel.objects.all())
through = PrimaryKeyRelatedField(many=True, read_only=True)
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_nested_relations(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -300,7 +330,7 @@ class Meta:
id = IntegerField(label='ID', read_only=True)
name = CharField(max_length=100)
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_hyperlinked_relations(self):
class TestSerializer(serializers.HyperlinkedModelSerializer):
Expand All @@ -315,7 +345,7 @@ class Meta:
many_to_many = HyperlinkedRelatedField(many=True, queryset=ManyToManyTargetModel.objects.all(), view_name='manytomanytargetmodel-detail')
through = HyperlinkedRelatedField(many=True, read_only=True, view_name='throughtargetmodel-detail')
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_nested_hyperlinked_relations(self):
class TestSerializer(serializers.HyperlinkedModelSerializer):
Expand All @@ -339,7 +369,7 @@ class Meta:
url = HyperlinkedIdentityField(view_name='throughtargetmodel-detail')
name = CharField(max_length=100)
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_pk_reverse_foreign_key(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -353,7 +383,7 @@ class Meta:
name = CharField(max_length=100)
reverse_foreign_key = PrimaryKeyRelatedField(many=True, queryset=RelationalModel.objects.all())
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_pk_reverse_one_to_one(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -367,7 +397,7 @@ class Meta:
name = CharField(max_length=100)
reverse_one_to_one = PrimaryKeyRelatedField(queryset=RelationalModel.objects.all())
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_pk_reverse_many_to_many(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -381,7 +411,7 @@ class Meta:
name = CharField(max_length=100)
reverse_many_to_many = PrimaryKeyRelatedField(many=True, queryset=RelationalModel.objects.all())
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)

def test_pk_reverse_through(self):
class TestSerializer(serializers.ModelSerializer):
Expand All @@ -395,7 +425,7 @@ class Meta:
name = CharField(max_length=100)
reverse_through = PrimaryKeyRelatedField(many=True, read_only=True)
""")
self.assertEqual(repr(TestSerializer()), expected)
self.assertEqual(unicode_repr(TestSerializer()), expected)


class TestIntegration(TestCase):
Expand Down

0 comments on commit da6ef3d

Please sign in to comment.