Skip to content

Fix nullable source='*' fields #6659

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

Merged
merged 2 commits into from
May 20, 2019
Merged
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
5 changes: 5 additions & 0 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ def validate_empty_values(self, data):
if data is None:
if not self.allow_null:
self.fail('null')
# Nullable `source='*'` fields should not be skipped when its named
# field is given a null value. This is because `source='*'` means
# the field is passed the entire object, which is not null.
elif self.source == '*':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fix seems to be very simple on!! good job

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't the intention to return empty dict from validate_empty_values in this case? Otherwise I see failure in to_internal_value with error 'Invalid data. Expected a dictionary, but got NoneType.'?

return (False, None)
return (True, None)

return (False, data)
Expand Down
44 changes: 41 additions & 3 deletions tests/test_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ def test_validate_list(self):

class TestStarredSource:
"""
Tests for `source='*'` argument, which is used for nested representations.
Tests for `source='*'` argument, which is often used for complex field or
nested representations.

For example:

Expand All @@ -337,11 +338,28 @@ class NestedSerializer2(serializers.Serializer):
c = serializers.IntegerField()
d = serializers.IntegerField()

class TestSerializer(serializers.Serializer):
class NestedBaseSerializer(serializers.Serializer):
nested1 = NestedSerializer1(source='*')
nested2 = NestedSerializer2(source='*')

self.Serializer = TestSerializer
# nullable nested serializer testing
class NullableNestedSerializer(serializers.Serializer):
nested = NestedSerializer1(source='*', allow_null=True)

# nullable custom field testing
class CustomField(serializers.Field):
def to_representation(self, instance):
return getattr(instance, 'foo', None)

def to_internal_value(self, data):
return {'foo': data}

class NullableFieldSerializer(serializers.Serializer):
field = CustomField(source='*', allow_null=True)

self.Serializer = NestedBaseSerializer
self.NullableNestedSerializer = NullableNestedSerializer
self.NullableFieldSerializer = NullableFieldSerializer

def test_nested_validate(self):
"""
Expand All @@ -356,6 +374,12 @@ def test_nested_validate(self):
'd': 4
}

def test_nested_null_validate(self):
serializer = self.NullableNestedSerializer(data={'nested': None})

# validation should fail (but not error) since nested fields are required
assert not serializer.is_valid()

def test_nested_serialize(self):
"""
An object can be serialized into a nested representation.
Expand All @@ -364,6 +388,20 @@ def test_nested_serialize(self):
serializer = self.Serializer(instance)
assert serializer.data == self.data

def test_field_validate(self):
serializer = self.NullableFieldSerializer(data={'field': 'bar'})

# validation should pass since no internal validation
assert serializer.is_valid()
assert serializer.validated_data == {'foo': 'bar'}

def test_field_null_validate(self):
serializer = self.NullableFieldSerializer(data={'field': None})

# validation should pass since no internal validation
assert serializer.is_valid()
assert serializer.validated_data == {'foo': None}


class TestIncorrectlyConfigured:
def test_incorrect_field_name(self):
Expand Down