Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Dont change return type of Serializer.errors
We should keep `Serializer.errors`'s return type unchanged in
order to maintain backward compatibility.

The error codes will only be propagated to the `exception_handler`
or accessible through the `Serializer._errors` private attribute.
  • Loading branch information
johnraz committed Dec 10, 2015
commit 18347601489f4f868adc82f5cccc9929186513d6
15 changes: 0 additions & 15 deletions rest_framework/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,21 +493,6 @@ def get_rendered_html_form(self, data, view, method, request):
if hasattr(serializer, 'initial_data'):
serializer.is_valid()

# Convert ValidationError to unicode string
# This is mainly a hack to monkey patch the errors and make the form renderer happy...
errors = OrderedDict()
for field_name, values in serializer.errors.items():
if isinstance(values, list):
errors[field_name] = values
continue

errors[field_name] = []
for value in values.detail:
for message in value.detail:
errors[field_name].append(message)

serializer._errors = errors

form_renderer = self.form_renderer_class()
return form_renderer.render(
serializer.data,
Expand Down
40 changes: 38 additions & 2 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,13 @@ def is_valid(self, raise_exception=False):
self._errors = {}

if self._errors and raise_exception:
raise ValidationError(self.errors)
return_errors = None
if isinstance(self._errors, list):
return_errors = ReturnList(self._errors, serializer=self)
elif isinstance(self._errors, dict):
return_errors = ReturnDict(self._errors, serializer=self)

raise ValidationError(return_errors)

return not bool(self._errors)

Expand All @@ -244,12 +250,42 @@ def data(self):
self._data = self.get_initial()
return self._data

def _transform_to_legacy_errors(self, errors_to_transform):
# Do not mutate `errors_to_transform` here.
errors = ReturnDict(serializer=self)
for field_name, values in errors_to_transform.items():
if isinstance(values, list):
errors[field_name] = values
continue

if isinstance(values.detail, list):
errors[field_name] = []
for value in values.detail:
if isinstance(value, ValidationError):
errors[field_name].extend(value.detail)
elif isinstance(value, list):
errors[field_name].extend(value)
else:
errors[field_name].append(value)

elif isinstance(values.detail, dict):
errors[field_name] = {}
for sub_field_name, value in values.detail.items():
errors[field_name][sub_field_name] = []
for validation_error in value:
errors[field_name][sub_field_name].extend(validation_error.detail)
return errors

@property
def errors(self):
if not hasattr(self, '_errors'):
msg = 'You must call `.is_valid()` before accessing `.errors`.'
raise AssertionError(msg)
return self._errors

if isinstance(self._errors, list):
return map(self._transform_to_legacy_errors, self._errors)
else:
return self._transform_to_legacy_errors(self._errors)

@property
def validated_data(self):
Expand Down
15 changes: 2 additions & 13 deletions rest_framework/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

from rest_framework import exceptions, status
from rest_framework.compat import set_rollback
from rest_framework.exceptions import ValidationError, _force_text_recursive
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.settings import api_settings
Expand Down Expand Up @@ -70,18 +69,8 @@ def exception_handler(exc, context):
if getattr(exc, 'wait', None):
headers['Retry-After'] = '%d' % exc.wait

if isinstance(exc.detail, list):
data = _force_text_recursive([item.detail if isinstance(item, ValidationError) else item
for item in exc.detai])
elif isinstance(exc.detail, dict):
for field_name, e in exc.detail.items():
if hasattr(e, 'detail') and isinstance(e.detail[0], ValidationError):
exc.detail[field_name] = e.detail[0].detail
elif isinstance(e, ValidationError):
exc.detail[field_name] = e.detail
else:
exc.detail[field_name] = e
data = exc.detail
if isinstance(exc.detail, (list, dict)):
data = exc.detail.serializer.errors
else:
data = {'detail': exc.detail}

Expand Down
7 changes: 5 additions & 2 deletions tests/test_bound_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ class ExampleSerializer(serializers.Serializer):
serializer = ExampleSerializer(data={'text': 'x' * 1000, 'amount': 123})
serializer.is_valid()

# TODO Should we add the _errors with ValidationError to the bound_field.errors to get acces to the error code?
assert serializer._errors['text'].detail[0].detail == ['Ensure this field has no more than 100 characters.']
assert serializer._errors['text'].detail[0].code == 'max_length'

assert serializer['text'].value == 'x' * 1000
assert serializer['text'].errors.detail[0].detail == ['Ensure this field has no more than 100 characters.']
assert serializer['text'].errors.detail[0].code == 'max_length'
assert serializer['text'].errors == ['Ensure this field has no more than 100 characters.']
assert serializer['text'].name == 'text'
assert serializer['amount'].value is 123
assert serializer['amount'].errors is None
Expand Down
2 changes: 1 addition & 1 deletion tests/test_model_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ class Meta:

s = TestSerializer(data={'address': 'not an ip address'})
self.assertFalse(s.is_valid())
self.assertEquals(1, len(s.errors['address'].detail),
self.assertEquals(1, len(s.errors['address']),
'Unexpected number of validation errors: '
'{0}'.format(s.errors))

Expand Down
10 changes: 6 additions & 4 deletions tests/test_relations_hyperlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,9 @@ def test_foreign_key_update_incorrect_type(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data, context={'request': request})
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail, ['Incorrect type. Expected URL string, received int.'])
self.assertEqual(serializer.errors['target'].code, 'incorrect_type')
self.assertEqual(serializer.errors, {'target': ['Incorrect type. Expected URL string, received int.']})
self.assertEqual(serializer._errors['target'].detail, ['Incorrect type. Expected URL string, received int.'])
self.assertEqual(serializer._errors['target'].code, 'incorrect_type')

def test_reverse_foreign_key_update(self):
data = {'url': 'http://testserver/foreignkeytarget/2/', 'name': 'target-2', 'sources': ['http://testserver/foreignkeysource/1/', 'http://testserver/foreignkeysource/3/']}
Expand Down Expand Up @@ -316,8 +317,9 @@ def test_foreign_key_update_with_invalid_null(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data, context={'request': request})
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer.errors['target'].code, 'null')
self.assertEqual(serializer.errors, {'target': ['This field may not be null.']})
self.assertEqual(serializer._errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer._errors['target'].code, 'null')


class HyperlinkedNullableForeignKeyTests(TestCase):
Expand Down
11 changes: 7 additions & 4 deletions tests/test_relations_pk.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,11 @@ def test_foreign_key_update_incorrect_type(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data)
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail,
self.assertEqual(serializer.errors,
{'target': ['Incorrect type. Expected pk value, received %s.' % six.text_type.__name__]})
self.assertEqual(serializer._errors['target'].detail,
['Incorrect type. Expected pk value, received %s.' % six.text_type.__name__])
self.assertEqual(serializer.errors['target'].code, 'incorrect_type')
self.assertEqual(serializer._errors['target'].code, 'incorrect_type')

def test_reverse_foreign_key_update(self):
data = {'id': 2, 'name': 'target-2', 'sources': [1, 3]}
Expand Down Expand Up @@ -308,8 +310,9 @@ def test_foreign_key_update_with_invalid_null(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data)
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer.errors['target'].code, 'null')
self.assertEqual(serializer.errors, {'target': ['This field may not be null.']})
self.assertEqual(serializer._errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer._errors['target'].code, 'null')

def test_foreign_key_with_unsaved(self):
source = ForeignKeySource(name='source-unsaved')
Expand Down
10 changes: 6 additions & 4 deletions tests/test_relations_slug.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ def test_foreign_key_update_incorrect_type(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data)
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail, ['Object with name=123 does not exist.'])
self.assertEqual(serializer.errors['target'].code, 'does_not_exist')
self.assertEqual(serializer.errors, {'target': ['Object with name=123 does not exist.']})
self.assertEqual(serializer._errors['target'].detail, ['Object with name=123 does not exist.'])
self.assertEqual(serializer._errors['target'].code, 'does_not_exist')

def test_reverse_foreign_key_update(self):
data = {'id': 2, 'name': 'target-2', 'sources': ['source-1', 'source-3']}
Expand Down Expand Up @@ -177,8 +178,9 @@ def test_foreign_key_update_with_invalid_null(self):
instance = ForeignKeySource.objects.get(pk=1)
serializer = ForeignKeySourceSerializer(instance, data=data)
self.assertFalse(serializer.is_valid())
self.assertEqual(serializer.errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer.errors['target'].code, 'null')
self.assertEqual(serializer.errors, {'target': ['This field may not be null.']})
self.assertEqual(serializer._errors['target'].detail, ['This field may not be null.'])
self.assertEqual(serializer._errors['target'].code, 'null')


class SlugNullableForeignKeyTests(TestCase):
Expand Down
5 changes: 3 additions & 2 deletions tests/test_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ def test_invalid_serializer(self):
serializer = self.Serializer(data={'char': 'abc'})
assert not serializer.is_valid()
assert serializer.validated_data == {}
assert serializer.errors['integer'].detail == ['This field is required.']
assert serializer.errors['integer'].code == 'required'
assert serializer.errors == {'integer': ['This field is required.']}
assert serializer._errors['integer'].detail == ['This field is required.']
assert serializer._errors['integer'].code == 'required'

def test_partial_validation(self):
serializer = self.Serializer(data={'char': 'abc'}, partial=True)
Expand Down
10 changes: 9 additions & 1 deletion tests/test_serializer_bulk_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,15 @@ def test_bulk_create_errors(self):
serializer = self.BookSerializer(data=data, many=True)
self.assertEqual(serializer.is_valid(), False)

for idx, error in enumerate(serializer.errors):
expected_errors = [
{},
{},
{'id': ['A valid integer is required.']}
]

self.assertEqual(serializer.errors, expected_errors)

for idx, error in enumerate(serializer._errors):
if idx < 2:
self.assertEqual(error, {})
else:
Expand Down
15 changes: 11 additions & 4 deletions tests/test_serializer_nested.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,11 @@ def test_null_is_not_allowed_if_allow_null_is_not_set(self):

assert not serializer.is_valid()

assert serializer.errors['not_allow_null'].detail == [serializer.error_messages['null']]
assert serializer.errors['not_allow_null'].code == 'null'
expected_errors = {'not_allow_null': [serializer.error_messages['null']]}
assert serializer.errors == expected_errors

assert serializer._errors['not_allow_null'].detail == [serializer.error_messages['null']]
assert serializer._errors['not_allow_null'].code == 'null'

def test_run_the_field_validation_even_if_the_field_is_null(self):
class TestSerializer(self.Serializer):
Expand Down Expand Up @@ -165,7 +168,11 @@ def test_empty_not_allowed_if_allow_empty_is_set_to_false(self):

assert not serializer.is_valid()

assert serializer.errors['not_allow_empty'].detail['non_field_errors'][0].detail == \
expected_errors = {
'not_allow_empty': {'non_field_errors': [serializers.ListSerializer.default_error_messages['empty']]}}
assert serializer.errors == expected_errors

assert serializer._errors['not_allow_empty'].detail['non_field_errors'][0].detail == \
[serializers.ListSerializer.default_error_messages['empty']]

assert serializer.errors['not_allow_empty'].detail['non_field_errors'][0].code == 'empty_not_allowed'
assert serializer._errors['not_allow_empty'].detail['non_field_errors'][0].code == 'empty_not_allowed'
10 changes: 7 additions & 3 deletions tests/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,13 @@ def test_max_value_validation_serializer_success(self):
def test_max_value_validation_serializer_fails(self):
serializer = ValidationMaxValueValidatorModelSerializer(data={'number_value': 101})
self.assertFalse(serializer.is_valid())
self.assertEqual(['Ensure this value is less than or equal to 100.'], serializer.errors['number_value'].detail[0].detail)
self.assertEqual(None, serializer.errors['number_value'].code)
self.assertEqual('max_value', serializer.errors['number_value'].detail[0].code)

self.assertDictEqual({'number_value': ['Ensure this value is less than or equal to 100.']}, serializer.errors)

self.assertEqual(['Ensure this value is less than or equal to 100.'],
serializer._errors['number_value'].detail[0].detail)
self.assertEqual(None, serializer._errors['number_value'].code)
self.assertEqual('max_value', serializer._errors['number_value'].detail[0].code)

def test_max_value_validation_success(self):
obj = ValidationMaxValueValidatorModel.objects.create(number_value=100)
Expand Down
9 changes: 6 additions & 3 deletions tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ def test_is_not_unique(self):
serializer = UniquenessSerializer(data=data)

assert not serializer.is_valid()
assert serializer.errors['username'].code is None
assert serializer.errors['username'].detail[0].code == 'unique'
assert serializer.errors['username'].detail[0].detail == ['UniquenessModel with this username already exists.']

assert serializer.errors == {'username': ['UniquenessModel with this username already exists.']}

assert serializer._errors['username'].code is None
assert serializer._errors['username'].detail[0].code == 'unique'
assert serializer._errors['username'].detail[0].detail == ['UniquenessModel with this username already exists.']

def test_is_unique(self):
data = {'username': 'other'}
Expand Down