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
Deal with ValidationError instantiation
  • Loading branch information
johnraz committed Dec 22, 2015
commit 2344d2270a851acea6a3089b9d70b92df3fd7ee2
76 changes: 39 additions & 37 deletions rest_framework/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from __future__ import unicode_literals

import math
from collections import namedtuple

from django.utils import six
from django.utils.encoding import force_text
Expand Down Expand Up @@ -61,7 +62,7 @@ def __str__(self):
def build_error_from_django_validation_error(exc_info):
code = getattr(exc_info, 'code', None) or 'invalid'
return [
(msg, code)
ErrorDetails(msg, code)
for msg in exc_info.messages
]

Expand All @@ -72,60 +73,61 @@ def build_error_from_django_validation_error(exc_info):
# from rest_framework import serializers
# raise serializers.ValidationError('Value was invalid')

ErrorDetails = namedtuple('ErrorDetails', ['message', 'code'])


class ValidationError(APIException):
status_code = status.HTTP_400_BAD_REQUEST
code = None

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 code:
self.full_details = [(detail, code)]
self.full_details = ErrorDetails(detail, code)
else:
self.full_details = detail
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't really want this stuff in. At least at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean ? Above or below ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The population of .full_details. Should just be storing the message and code at this point.
(Let me know if I'm being braindead tho - I only get a brief amount of time on each review)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure to follow.

It seems to me that this is what I'm doing right now:

The population of .full_details. Should just be storing the message and code at this point.

It does store message and code for single items, a list when a list is provided and a dict when a dict is provided
Basically it does store what is passed to the __init__ method.

We then need to transform whatever is given as the detail parameter to the former self.detail format which is what this code does (quite painfully).


if isinstance(self.full_details, tuple):
self.detail, self.code = self.full_details
self.detail = [self.detail]
if not isinstance(self.full_details, dict) \
and not isinstance(self.full_details, list):
self.full_details = [self.full_details]
self.full_details = _force_text_recursive(self.full_details)

elif isinstance(self.full_details, list):
self.detail = detail
if isinstance(self.full_details, list):
if isinstance(self.full_details, ReturnList):
self.detail = ReturnList(serializer=self.full_details.serializer)
self.detail = ReturnList(
serializer=self.full_details.serializer)
else:
self.detail = []
for error in self.full_details:
if isinstance(error, tuple):
message, code = error
self.detail.append(message)
elif isinstance(error, dict):
self.detail = self.full_details
break

for full_detail in self.full_details:
if isinstance(full_detail, ErrorDetails):
self.detail.append(full_detail.message)
elif isinstance(full_detail, dict):
if not full_detail:
self.detail.append(full_detail)
for key, value in full_detail.items():
if isinstance(value, list):
self.detail.append(
{key: [item.message]
if isinstance(item, ErrorDetails)
else [item] for item in value})
elif isinstance(full_detail, list):
self.detail.extend(full_detail)
else:
self.detail.append(full_detail)
elif isinstance(self.full_details, dict):
if isinstance(self.full_details, ReturnDict):
self.detail = ReturnDict(serializer=self.full_details.serializer)
self.detail = ReturnDict(
serializer=self.full_details.serializer)
else:
self.detail = {}

for field_name, errors in self.full_details.items():
self.detail[field_name] = []
if isinstance(errors, tuple):
message, code = errors
self.detail[field_name].append(message)
elif isinstance(errors, list):
for error in errors:
if isinstance(error, tuple):
message, code = error
else:
message = error
if message:
self.detail[field_name].append(message)
else:
self.detail = [self.full_details]

self.detail = _force_text_recursive(self.detail)
for field_name, full_detail in self.full_details.items():
if isinstance(full_detail, list):
self.detail[field_name] = [
item.message if isinstance(item, ErrorDetails) else item
for item in full_detail
]
else:
self.detail[field_name] = full_detail

def __str__(self):
return six.text_type(self.detail)
Expand Down
2 changes: 1 addition & 1 deletion rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ def run_validators(self, value):
# attempting to accumulate a list of errors.
if isinstance(exc.detail, dict):
raise
errors.append((exc.detail, exc.code))
errors.append(exc.full_details)
except DjangoValidationError as exc:
errors.extend(build_error_from_django_validation_error(exc))
if errors:
Expand Down
61 changes: 17 additions & 44 deletions rest_framework/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
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
from rest_framework.exceptions import build_error_from_django_validation_error
from rest_framework.exceptions import (
ErrorDetails, build_error_from_django_validation_error
)
from rest_framework.utils import model_meta
from rest_framework.utils.field_mapping import (
ClassLookupDict, get_field_kwargs, get_nested_relation_kwargs,
Expand Down Expand Up @@ -214,12 +216,12 @@ def is_valid(self, raise_exception=False):
self._validated_data = self.run_validation(self.initial_data)
except ValidationError as exc:
self._validated_data = {}
self._errors = exc.full_details
self._errors = exc.detail
else:
self._errors = {}

if self._errors and raise_exception:
raise ValidationError(self._errors)
raise ValidationError(self.errors)

return not bool(self._errors)

Expand Down Expand Up @@ -249,36 +251,7 @@ def errors(self):
if not hasattr(self, '_errors'):
msg = 'You must call `.is_valid()` before accessing `.errors`.'
raise AssertionError(msg)

if isinstance(self._errors, dict):
errors = ReturnDict(serializer=self)
for key, value in self._errors.items():
if isinstance(value, dict):
errors[key] = {}
for key_, value_ in value.items():
message, code = value_[0]
errors[key][key_] = [message]
elif isinstance(value, list):
if isinstance(value[0], tuple):
message, code = value[0]
else:
message = value[0]
if isinstance(message, list):
errors[key] = message
else:
errors[key] = [message]
elif isinstance(value, tuple):
message, code = value
errors[key] = [message]
else:
errors[key] = [value]
elif isinstance(self._errors, list):
errors = ReturnList(self._errors, serializer=self)
else:
# This shouldn't ever happen.
errors = self._errors

return errors
return self._errors

@property
def validated_data(self):
Expand Down Expand Up @@ -333,21 +306,21 @@ def get_validation_error_detail(exc):
return {
api_settings.NON_FIELD_ERRORS_KEY: error
}
elif isinstance(exc.full_details, dict):
elif isinstance(exc.detail, dict):
# If errors may be a dict we use the standard {key: list of values}.
# Here we ensure that all the values are *lists* of errors.
return {
key: value if isinstance(value, list) else [value]
for key, value in exc.full_details.items()
for key, value in exc.detail.items()
}
elif isinstance(exc.full_details, list):
elif isinstance(exc.detail, list):
# Errors raised as a list are non-field errors.
return {
api_settings.NON_FIELD_ERRORS_KEY: exc.full_details
api_settings.NON_FIELD_ERRORS_KEY: exc.detail
}
# Errors raised as a string are non-field errors.
return {
api_settings.NON_FIELD_ERRORS_KEY: [exc.full_details]
api_settings.NON_FIELD_ERRORS_KEY: [exc.detail]
}


Expand Down Expand Up @@ -455,11 +428,11 @@ def to_internal_value(self, data):
)
code = 'invalid'
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [(message, code)]
api_settings.NON_FIELD_ERRORS_KEY: [ErrorDetails(message, code)]
})

ret = ReturnDict(serializer=self)
errors = ReturnDict(serializer=self)
ret = OrderedDict()
errors = OrderedDict()
fields = self._writable_fields

for field in fields:
Expand All @@ -470,7 +443,7 @@ 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.full_details
errors[field.field_name] = exc.detail
except DjangoValidationError as exc:
error = build_error_from_django_validation_error(exc)
errors[field.field_name] = error
Expand Down Expand Up @@ -610,14 +583,14 @@ def to_internal_value(self, data):
)
code = 'not_a_list'
raise ValidationError({
api_settings.NON_FIELD_ERRORS_KEY: [(message, code)]
api_settings.NON_FIELD_ERRORS_KEY: [ErrorDetails(message, code)]
})

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

ret = []
Expand Down
10 changes: 5 additions & 5 deletions rest_framework/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from django.utils.translation import ugettext_lazy as _

from rest_framework.compat import unicode_to_repr
from rest_framework.exceptions import ValidationError
from rest_framework.exceptions import ErrorDetails, ValidationError
from rest_framework.utils.representation import smart_repr


Expand Down Expand Up @@ -102,7 +102,7 @@ def enforce_required_fields(self, attrs):

code = 'required'
missing = {
field_name: [(self.missing_message, code)]
field_name: ErrorDetails(self.missing_message, code)
for field_name in self.fields
if field_name not in attrs
}
Expand Down Expand Up @@ -150,7 +150,7 @@ def __call__(self, attrs):
field_names = ', '.join(self.fields)
message = self.message.format(field_names=field_names)
code = 'unique'
raise ValidationError(message, code=code)
raise ValidationError(ErrorDetails(message, code=code))

def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, fields=%s)>' % (
Expand Down Expand Up @@ -189,7 +189,7 @@ def enforce_required_fields(self, attrs):
"""
code = 'required'
missing = {
field_name: [(self.missing_message, code)]
field_name: ErrorDetails(self.missing_message, code)
for field_name in [self.field, self.date_field]
if field_name not in attrs
}
Expand All @@ -216,7 +216,7 @@ def __call__(self, attrs):
if queryset.exists():
message = self.message.format(date_field=self.date_field)
code = 'unique'
raise ValidationError({self.field: [(message, code)]})
raise ValidationError({self.field: ErrorDetails(message, code)})

def __repr__(self):
return unicode_to_repr('<%s(queryset=%s, field=%s, date_field=%s)>' % (
Expand Down
2 changes: 1 addition & 1 deletion tests/test_validation_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def setUp(self):

def exception_handler(exc, request):
return_errors = {}
for field_name, errors in exc.full_details.items():
for field_name, errors in exc.detail.items():
return_errors[field_name] = []
for message, code in errors:
return_errors[field_name].append({
Expand Down