Skip to content
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

Add validate method back to ListSerializer #2225

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
36 changes: 36 additions & 0 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,42 @@ def get_value(self, dictionary):
return html.parse_html_list(dictionary, prefix=self.field_name)
return dictionary.get(self.field_name, empty)

def run_validation(self, data=empty):
data = super(ListSerializer, self).run_validation(data)
Copy link
Member

Choose a reason for hiding this comment

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

I would probably recommend we drop the call to super here, in the same way we do for Serializer. It gets pretty confusing otherwise. This'd also let us move the if not isinstance(data, list) check out of to_internal_value which would be nicer and would mirror the Serializer classes if not isinstance(data, dict) check that occurs.


try:
data = self.validate(data)
assert data is not None, '.validate() should return the validated data'
except ValidationError as exc:
if isinstance(exc.detail, dict):
# .validate() errors may be a dict, in which case, use
# standard {key: list of values} style.
raise ValidationError(dict([
(key, value if isinstance(value, list) else [value])
for key, value in exc.detail.items()
]))
elif isinstance(exc.detail, list):
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: exc.detail
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we use NON_FIELD_ERRORS_KEY for a list validation failure, or would a new key be better?

})
else:
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [exc.detail]
})
except DjangoValidationError as exc:
# Normally you should raise `serializers.ValidationError`
# inside your codebase, but we handle Django's validation
# exception class as well for simpler compat.
# Eg. Calling Model.clean() explicitly inside Serializer.validate()
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: list(exc.messages)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a large copy/paste from Serializer.run_validation. I would prefer to refactor this to be more DRY, but I'm not sure how best to do this.

Copy link
Member

Choose a reason for hiding this comment

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

If we dropped the ValidationError and DjangoValidationError handling code into a function that takes an exception and returns the details for ValidationError to be raised, we could have that just in once place, which would be better.

Copy link
Member

Choose a reason for hiding this comment

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


return data

def validate(self, attrs):
return attrs

def to_internal_value(self, data):
"""
List of dicts of native values <- List of dicts of primitive datatypes.
Expand Down
16 changes: 16 additions & 0 deletions tests/test_serializer_lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,19 @@ def test_validate_html_input(self):
serializer = self.Serializer(data=input_data)
assert serializer.is_valid()
assert serializer.validated_data == expected_output


class TestListSerializerClass:
"""Tests for a custom list_serializer_class."""
def test_list_serializer_class_validate(self):
class CustomListSerializer(serializers.ListSerializer):
def validate(self, attrs):
raise serializers.ValidationError('Non field error')

class TestSerializer(serializers.Serializer):
class Meta:
list_serializer_class = CustomListSerializer

serializer = TestSerializer(data=[], many=True)
assert not serializer.is_valid()
assert serializer.errors == {'non_field_errors': ['Non field error']}