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
Next Next commit
Introduce error code for validation errors.
This patch is meant to fix #3111, regarding comments made to #3137
and #3169.

The `ValidationError` will now contain a `code` attribute.

Before this patch, `ValidationError.detail` only contained a
`dict` with values equal to a  `list` of string error messages or
directly a `list` containing string error messages.

Now, the string error messages are replaced with `ValidationError`.

This means that, depending on the case, you will not only get a
string back but a all object containing both the error message and
the error code, respectively `ValidationError.detail` and
`ValidationError.code`.

It is important to note that the `code` attribute is not relevant
when the `ValidationError` represents a combination of errors and
hence is `None` in such cases.

The main benefit of this change is that the error message and
error code are now accessible the custom exception handler and can
be used to format the error response.

An custom exception handler example is available in the
`TestValidationErrorWithCode` test.
  • Loading branch information
johnraz committed Dec 8, 2015
commit 8c29efef48378a9484289fa1ae3b076b65dc6a7f
15 changes: 12 additions & 3 deletions rest_framework/authtoken/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,22 @@ def validate(self, attrs):
if user:
if not user.is_active:
msg = _('User account is disabled.')
raise serializers.ValidationError(msg)
raise serializers.ValidationError(
msg,
code='authorization'
)
else:
msg = _('Unable to log in with provided credentials.')
raise serializers.ValidationError(msg)
raise serializers.ValidationError(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess these would be marginally cleaner reformatted as three lines...

msg = ...
code = ...
raise serializers.ValidationError(msg, code)

msg,
code='authorization'
)
else:
msg = _('Must include "username" and "password".')
raise serializers.ValidationError(msg)
raise serializers.ValidationError(
msg,
code='authorization'
)

attrs['user'] = user
return attrs
17 changes: 15 additions & 2 deletions rest_framework/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ def __str__(self):
return self.detail


def build_error_from_django_validation_error(exc_info):
code = getattr(exc_info, 'code', None) or 'invalid'
return [
ValidationError(msg, code=code)
for msg in exc_info.messages
]


# The recommended style for using `ValidationError` is to keep it namespaced
# under `serializers`, in order to minimize potential confusion with Django's
# built in `ValidationError`. For example:
Expand All @@ -68,12 +76,17 @@ def __str__(self):
class ValidationError(APIException):
status_code = status.HTTP_400_BAD_REQUEST

def __init__(self, detail):
def __init__(self, detail, code=None):
# For validation errors the 'detail' key is always required.
# The details should always be coerced to a list if not already.
if not isinstance(detail, dict) and not isinstance(detail, list):
detail = [detail]
self.detail = _force_text_recursive(detail)
elif isinstance(detail, dict) or (detail and isinstance(detail[0], ValidationError)):
assert code is None, (
'The `code` argument must not be set for compound errors.')

self.detail = detail
self.code = code

def __str__(self):
return six.text_type(self.detail)
Expand Down
12 changes: 8 additions & 4 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
MinValueValidator, duration_string, parse_duration, unicode_repr,
unicode_to_repr
)
from rest_framework.exceptions import ValidationError
from rest_framework.exceptions import (
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well keep that formatting as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed blame my IDE XD

ValidationError, build_error_from_django_validation_error
)
from rest_framework.settings import api_settings
from rest_framework.utils import html, humanize_datetime, representation

Expand Down Expand Up @@ -501,9 +503,11 @@ def run_validators(self, value):
# attempting to accumulate a list of errors.
if isinstance(exc.detail, dict):
raise
errors.extend(exc.detail)
errors.append(ValidationError(exc.detail, code=exc.code))
except DjangoValidationError as exc:
errors.extend(exc.messages)
errors.extend(
build_error_from_django_validation_error(exc)
)
if errors:
raise ValidationError(errors)

Expand Down Expand Up @@ -541,7 +545,7 @@ def fail(self, key, **kwargs):
msg = MISSING_ERROR_MESSAGE.format(class_name=class_name, key=key)
raise AssertionError(msg)
message_string = msg.format(**kwargs)
raise ValidationError(message_string)
raise ValidationError(message_string, code=key)

@cached_property
def root(self):
Expand Down
15 changes: 15 additions & 0 deletions rest_framework/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,21 @@ 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
1 change: 0 additions & 1 deletion rest_framework/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ def __init__(self, data=None, status=None,
'`.error`. representation.'
)
raise AssertionError(msg)

self.data = data
self.template_name = template_name
self.exception = exception
Expand Down
21 changes: 15 additions & 6 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from django.utils.functional import cached_property
from django.utils.translation import ugettext_lazy as _

from rest_framework import exceptions
from rest_framework.compat import DurationField as ModelDurationField
from rest_framework.compat import JSONField as ModelJSONField
from rest_framework.compat import postgres_fields, unicode_to_repr
Expand Down Expand Up @@ -300,7 +301,8 @@ def get_validation_error_detail(exc):
# exception class as well for simpler compat.
# Eg. Calling Model.clean() explicitly inside Serializer.validate()
return {
api_settings.NON_FIELD_ERRORS_KEY: list(exc.messages)
api_settings.NON_FIELD_ERRORS_KEY:
exceptions.build_error_from_django_validation_error(exc)
}
elif isinstance(exc.detail, dict):
# If errors may be a dict we use the standard {key: list of values}.
Expand Down Expand Up @@ -422,8 +424,9 @@ def to_internal_value(self, data):
message = self.error_messages['invalid'].format(
datatype=type(data).__name__
)
error = ValidationError(message, code='invalid')
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [message]
api_settings.NON_FIELD_ERRORS_KEY: [error]
})

ret = OrderedDict()
Expand All @@ -438,9 +441,11 @@ def to_internal_value(self, data):
if validate_method is not None:
validated_value = validate_method(validated_value)
except ValidationError as exc:
errors[field.field_name] = exc.detail
errors[field.field_name] = exc
except DjangoValidationError as exc:
errors[field.field_name] = list(exc.messages)
errors[field.field_name] = (
exceptions.build_error_from_django_validation_error(exc)
)
except SkipField:
pass
else:
Expand Down Expand Up @@ -575,14 +580,18 @@ def to_internal_value(self, data):
message = self.error_messages['not_a_list'].format(
input_type=type(data).__name__
)
error = ValidationError(
message,
code='not_a_list'
)
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [message]
api_settings.NON_FIELD_ERRORS_KEY: [error]
})

if not self.allow_empty and len(data) == 0:
message = self.error_messages['empty']
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [message]
api_settings.NON_FIELD_ERRORS_KEY: [ValidationError(message, code='empty_not_allowed')]
})

ret = []
Expand Down
16 changes: 11 additions & 5 deletions rest_framework/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def __call__(self, value):
queryset = self.filter_queryset(value, queryset)
queryset = self.exclude_current_instance(queryset)
if queryset.exists():
raise ValidationError(self.message)
raise ValidationError(self.message, code='unique')

def __repr__(self):
return unicode_to_repr('<%s(queryset=%s)>' % (
Expand Down Expand Up @@ -101,7 +101,9 @@ def enforce_required_fields(self, attrs):
return

missing = {
field_name: self.missing_message
field_name: ValidationError(
self.missing_message,
code='required')
for field_name in self.fields
if field_name not in attrs
}
Expand Down Expand Up @@ -147,7 +149,8 @@ def __call__(self, attrs):
]
if None not in checked_values and queryset.exists():
field_names = ', '.join(self.fields)
raise ValidationError(self.message.format(field_names=field_names))
raise ValidationError(self.message.format(field_names=field_names),
code='unique')

def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, fields=%s)>' % (
Expand Down Expand Up @@ -185,7 +188,9 @@ def enforce_required_fields(self, attrs):
'required' state on the fields they are applied to.
"""
missing = {
field_name: self.missing_message
field_name: ValidationError(
self.missing_message,
code='required')
for field_name in [self.field, self.date_field]
if field_name not in attrs
}
Expand All @@ -211,7 +216,8 @@ def __call__(self, attrs):
queryset = self.exclude_current_instance(attrs, queryset)
if queryset.exists():
message = self.message.format(date_field=self.date_field)
raise ValidationError({self.field: message})
error = ValidationError(message, code='unique')
raise ValidationError({self.field: error})

def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, field=%s, date_field=%s)>' % (
Expand Down
13 changes: 12 additions & 1 deletion rest_framework/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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 @@ -69,7 +70,17 @@ def exception_handler(exc, context):
if getattr(exc, 'wait', None):
headers['Retry-After'] = '%d' % exc.wait

if isinstance(exc.detail, (list, dict)):
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
else:
data = {'detail': exc.detail}
Expand Down
3 changes: 2 additions & 1 deletion tests/test_bound_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class ExampleSerializer(serializers.Serializer):
serializer.is_valid()

assert serializer['text'].value == 'x' * 1000
assert serializer['text'].errors == ['Ensure this field has no more than 100 characters.']
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'].name == 'text'
assert serializer['amount'].value is 123
assert serializer['amount'].errors is None
Expand Down
14 changes: 12 additions & 2 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import rest_framework
from rest_framework import serializers
from rest_framework.exceptions import ValidationError


# Tests for field keyword arguments and core functionality.
Expand Down Expand Up @@ -426,7 +427,13 @@ def test_invalid_inputs(self):
for input_value, expected_failure in get_items(self.invalid_inputs):
with pytest.raises(serializers.ValidationError) as exc_info:
self.field.run_validation(input_value)
assert exc_info.value.detail == expected_failure

if isinstance(exc_info.value.detail[0], ValidationError):
failure = exc_info.value.detail[0].detail
else:
failure = exc_info.value.detail

assert failure == expected_failure

def test_outputs(self):
for output_value, expected_output in get_items(self.outputs):
Expand Down Expand Up @@ -1393,7 +1400,10 @@ class TestFieldFieldWithName(FieldValues):
# call into it's regular validation, or require PIL for testing.
class FailImageValidation(object):
def to_python(self, value):
raise serializers.ValidationError(self.error_messages['invalid_image'])
raise serializers.ValidationError(
self.error_messages['invalid_image'],
code='invalid_image'
)


class PassImageValidation(object):
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']),
self.assertEquals(1, len(s.errors['address'].detail),
'Unexpected number of validation errors: '
'{0}'.format(s.errors))

Expand Down
6 changes: 4 additions & 2 deletions tests/test_relations_hyperlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ 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': ['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 @@ -315,7 +316,8 @@ 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': ['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
7 changes: 5 additions & 2 deletions tests/test_relations_pk.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,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': ['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')

def test_reverse_foreign_key_update(self):
data = {'id': 2, 'name': 'target-2', 'sources': [1, 3]}
Expand Down Expand Up @@ -306,7 +308,8 @@ 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': ['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
6 changes: 4 additions & 2 deletions tests/test_relations_slug.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ 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': ['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 @@ -176,7 +177,8 @@ 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': ['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
Loading