Skip to content

Fix model fields not being omitted on output/serialization #5473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
from collections import OrderedDict

from django.conf import settings
from django.contrib.contenttypes import fields as ct_fields
from django.core.exceptions import ValidationError as DjangoValidationError
from django.core.exceptions import ObjectDoesNotExist
from django.core.exceptions import FieldDoesNotExist, ObjectDoesNotExist
from django.core.validators import (
EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator,
MinValueValidator, RegexValidator, URLValidator, ip_address_validators
)
from django.db import models
from django.forms import FilePathField as DjangoFilePathField
from django.forms import ImageField as DjangoImageField
from django.utils import six, timezone
Expand Down Expand Up @@ -85,7 +87,7 @@ def is_simple_callable(obj):
return len_args <= len_defaults


def get_attribute(instance, attrs):
def get_attribute(instance, attrs, exc_on_model_default=False):
"""
Similar to Python's built in `getattr(instance, attr)`,
but takes a list of nested attributes, instead of a single attribute.
Expand All @@ -96,6 +98,33 @@ def get_attribute(instance, attrs):
try:
if isinstance(instance, collections.Mapping):
instance = instance[attr]
elif exc_on_model_default and isinstance(instance, models.Model):
# Lookup the model field default
try:
field = instance._meta.get_field(attr)
except FieldDoesNotExist:
field = None
else:
if isinstance(field, ct_fields.GenericForeignKey):
# For generic relations, use the foreign key as the
# default
field = instance._meta.get_field(field.fk_field)
elif not hasattr(field, 'get_default') and hasattr(
field, 'target_field'):
# Some relationship fields don't have their own
# `get_default()`
field = field.target_field
value = getattr(instance, attr)
if field is not None:
default = field.get_default()
if default is not empty and value == default:
# Support skipping model fields. They always return
# at least the field default so there's no
# AttributeError unless we force it.
raise AttributeError(
'{0!r} object has no attribute {1!r}'.format(
instance, attr))
instance = value
else:
instance = getattr(instance, attr)
except ObjectDoesNotExist:
Expand Down Expand Up @@ -438,7 +467,9 @@ def get_attribute(self, instance):
that should be used for this field.
"""
try:
return get_attribute(instance, self.source_attrs)
return get_attribute(
instance, self.source_attrs, exc_on_model_default=(
self.default is not empty or not self.required))
except (KeyError, AttributeError) as exc:
if self.default is not empty:
return self.get_default()
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def get_attribute(self, instance):
pass

# Standard case, return the object instance.
return get_attribute(instance, self.source_attrs)
return super(RelatedField, self).get_attribute(instance)

def get_choices(self, cutoff=None):
queryset = self.get_queryset()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_multitable_inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def test_multitable_inherited_model_fields_as_expected(self):
"""
child = ChildModel(name1='parent name', name2='child name')
serializer = DerivedModelSerializer(child)
assert set(serializer.data.keys()) == set(['name1', 'name2', 'id'])
assert set(serializer.data.keys()) == set(['name1', 'name2'])

def test_onetoone_primary_key_model_fields_as_expected(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/test_one_to_one_with_inheritance.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ def test_multitable_inherited_model_fields_as_expected(self):
child = ChildModel(name1='parent name', name2='child name')
serializer = DerivedModelSerializer(child)
self.assertEqual(set(serializer.data.keys()),
set(['name1', 'name2', 'id', 'childassociatedmodel']))
set(['name1', 'name2', 'childassociatedmodel']))
4 changes: 2 additions & 2 deletions tests/test_relations_pk.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def test_many_to_many_unsaved(self):

serializer = ManyToManySourceSerializer(source)

expected = {'id': None, 'name': 'source-unsaved', 'targets': []}
expected = {'name': 'source-unsaved', 'targets': []}
# no query if source hasn't been created yet
with self.assertNumQueries(0):
assert serializer.data == expected
Expand Down Expand Up @@ -330,7 +330,7 @@ def test_foreign_key_update_with_invalid_null(self):

def test_foreign_key_with_unsaved(self):
source = ForeignKeySource(name='source-unsaved')
expected = {'id': None, 'name': 'source-unsaved', 'target': None}
expected = {'name': 'source-unsaved', 'target': None}

serializer = ForeignKeySourceSerializer(source)

Expand Down
13 changes: 9 additions & 4 deletions tests/test_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,17 @@ def test_not_required_output_for_object(self):
"""
'required=False' should allow an object attribute to be missing in output.
"""
class ExampleSerializer(serializers.Serializer):
omitted = serializers.CharField(required=False)
included = serializers.CharField()
class MyModel(models.Model):
omitted = models.CharField(max_length=10, blank=True)
included = models.CharField(max_length=10)

class ExampleSerializer(serializers.ModelSerializer):
class Meta:
model = MyModel
exclude = ('id', )

def create(self, validated_data):
return MockObject(**validated_data)
return self.Meta.model(**validated_data)

serializer = ExampleSerializer(data={'included': 'abc'})
serializer.is_valid()
Expand Down