Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 190afdf

Browse files
slivercsliverc
committedAug 24, 2020
Add handling of nested errors
1 parent 3eaf07c commit 190afdf

File tree

6 files changed

+401
-26
lines changed

6 files changed

+401
-26
lines changed
 

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ any parts of the framework not mentioned in the documentation should generally b
1919
* Avoid `AttributeError` for PUT and PATCH methods when using `APIView`
2020
* Clear many-to-many relationships instead of deleting related objects during PATCH on `RelationshipView`
2121
* Allow POST, PATCH, DELETE for actions in `ReadOnlyModelViewSet`. It was problematic since 2.8.0.
22+
* Properly format nested errors
2223

2324
### Changed
2425

‎example/tests/__snapshots__/test_errors.ambr

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# name: test_deprecation_warning
2+
DeprecationWarning('Rendering nested serializer as relationship is deprecated. Use `ResourceRelatedField` instead if DummyNestedSerializer in serializer example.tests.test_errors.test_deprecation_warning.<locals>.DummySerializer should remain a relationship. Otherwise set JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE to True to render nested serializer as nested json attribute')
3+
---
4+
# name: test_first_level_attribute_error
5+
<class 'list'> [
6+
<class 'dict'> {
7+
'code': 'required',
8+
'detail': ErrorDetail(string='This field is required.', code='required'),
9+
'source': <class 'dict'> {
10+
'pointer': '/data/attributes/headline',
11+
},
12+
'status': '400',
13+
},
14+
]
15+
---
16+
# name: test_first_level_custom_attribute_error
17+
<class 'list'> [
18+
<class 'dict'> {
19+
'detail': ErrorDetail(string='Too short', code='invalid'),
20+
'source': <class 'dict'> {
21+
'pointer': '/data/attributes/body-text',
22+
},
23+
'title': ErrorDetail(string='Too Short title', code='invalid'),
24+
},
25+
]
26+
---
27+
# name: test_many_third_level_dict_errors
28+
<class 'list'> [
29+
<class 'dict'> {
30+
'code': 'required',
31+
'detail': ErrorDetail(string='This field is required.', code='required'),
32+
'source': <class 'dict'> {
33+
'pointer': '/data/attributes/comments/0/attachment/data',
34+
},
35+
'status': '400',
36+
},
37+
<class 'dict'> {
38+
'code': 'required',
39+
'detail': ErrorDetail(string='This field is required.', code='required'),
40+
'source': <class 'dict'> {
41+
'pointer': '/data/attributes/comments/0/body',
42+
},
43+
'status': '400',
44+
},
45+
]
46+
---
47+
# name: test_second_level_array_error
48+
<class 'list'> [
49+
<class 'dict'> {
50+
'code': 'required',
51+
'detail': ErrorDetail(string='This field is required.', code='required'),
52+
'source': <class 'dict'> {
53+
'pointer': '/data/attributes/comments/0/body',
54+
},
55+
'status': '400',
56+
},
57+
]
58+
---
59+
# name: test_second_level_dict_error
60+
<class 'list'> [
61+
<class 'dict'> {
62+
'code': 'required',
63+
'detail': ErrorDetail(string='This field is required.', code='required'),
64+
'source': <class 'dict'> {
65+
'pointer': '/data/attributes/comment/body',
66+
},
67+
'status': '400',
68+
},
69+
]
70+
---
71+
# name: test_third_level_array_error
72+
<class 'list'> [
73+
<class 'dict'> {
74+
'code': 'required',
75+
'detail': ErrorDetail(string='This field is required.', code='required'),
76+
'source': <class 'dict'> {
77+
'pointer': '/data/attributes/comments/0/attachments/0/data',
78+
},
79+
'status': '400',
80+
},
81+
]
82+
---
83+
# name: test_third_level_custom_array_error
84+
<class 'list'> [
85+
<class 'dict'> {
86+
'code': 'invalid',
87+
'detail': ErrorDetail(string='Too short data', code='invalid'),
88+
'source': <class 'dict'> {
89+
'pointer': '/data/attributes/comments/0/attachments/0/data',
90+
},
91+
'status': '400',
92+
},
93+
]
94+
---
95+
# name: test_third_level_dict_error
96+
<class 'list'> [
97+
<class 'dict'> {
98+
'code': 'required',
99+
'detail': ErrorDetail(string='This field is required.', code='required'),
100+
'source': <class 'dict'> {
101+
'pointer': '/data/attributes/comments/0/attachment/data',
102+
},
103+
'status': '400',
104+
},
105+
]
106+
---

‎example/tests/test_errors.py

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
import pytest
2+
from django.test import override_settings
3+
from django.urls import path, reverse
4+
from rest_framework import views
5+
6+
from rest_framework_json_api import serializers
7+
8+
from example.models import Blog
9+
10+
11+
# serializers
12+
class CommentAttachmentSerializer(serializers.Serializer):
13+
data = serializers.CharField(allow_null=False, required=True)
14+
15+
def validate_data(self, value):
16+
if value and len(value) < 10:
17+
raise serializers.ValidationError('Too short data')
18+
19+
20+
class CommentSerializer(serializers.Serializer):
21+
attachments = CommentAttachmentSerializer(many=True, required=False)
22+
attachment = CommentAttachmentSerializer(required=False)
23+
one_more_attachment = CommentAttachmentSerializer(required=False)
24+
body = serializers.CharField(allow_null=False, required=True)
25+
26+
27+
class EntrySerializer(serializers.Serializer):
28+
blog = serializers.IntegerField()
29+
comments = CommentSerializer(many=True, required=False)
30+
comment = CommentSerializer(required=False)
31+
headline = serializers.CharField(allow_null=True, required=True)
32+
body_text = serializers.CharField()
33+
34+
def validate(self, attrs):
35+
body_text = attrs['body_text']
36+
if len(body_text) < 5:
37+
raise serializers.ValidationError({'body_text': {
38+
'title': 'Too Short title', 'detail': 'Too short'}
39+
})
40+
41+
42+
# view
43+
class DummyTestView(views.APIView):
44+
serializer_class = EntrySerializer
45+
resource_name = 'entries'
46+
47+
def post(self, request, *args, **kwargs):
48+
serializer = self.serializer_class(data=request.data)
49+
serializer.is_valid(raise_exception=True)
50+
51+
52+
urlpatterns = [
53+
path('entries-nested', DummyTestView.as_view(),
54+
name='entries-nested-list')
55+
]
56+
57+
58+
@pytest.fixture(scope='function')
59+
def some_blog(db):
60+
return Blog.objects.create(name='Some Blog', tagline="It's a blog")
61+
62+
63+
def perform_error_test(client, data):
64+
with override_settings(
65+
JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE=True,
66+
ROOT_URLCONF=__name__
67+
):
68+
url = reverse('entries-nested-list')
69+
response = client.post(url, data=data)
70+
71+
errors = response.data
72+
return errors
73+
74+
75+
def test_first_level_attribute_error(client, some_blog, snapshot):
76+
data = {
77+
'data': {
78+
'type': 'entries',
79+
'attributes': {
80+
'blog': some_blog.pk,
81+
'bodyText': 'body_text',
82+
}
83+
}
84+
}
85+
assert snapshot == perform_error_test(client, data)
86+
87+
88+
def test_first_level_custom_attribute_error(client, some_blog, snapshot):
89+
data = {
90+
'data': {
91+
'type': 'entries',
92+
'attributes': {
93+
'blog': some_blog.pk,
94+
'body-text': 'body',
95+
'headline': 'headline'
96+
}
97+
}
98+
}
99+
with override_settings(JSON_API_FORMAT_FIELD_NAMES='dasherize'):
100+
assert snapshot == perform_error_test(client, data)
101+
102+
103+
def test_second_level_array_error(client, some_blog, snapshot):
104+
data = {
105+
'data': {
106+
'type': 'entries',
107+
'attributes': {
108+
'blog': some_blog.pk,
109+
'bodyText': 'body_text',
110+
'headline': 'headline',
111+
'comments': [
112+
{
113+
}
114+
]
115+
}
116+
}
117+
}
118+
119+
assert snapshot == perform_error_test(client, data)
120+
121+
122+
def test_second_level_dict_error(client, some_blog, snapshot):
123+
data = {
124+
'data': {
125+
'type': 'entries',
126+
'attributes': {
127+
'blog': some_blog.pk,
128+
'bodyText': 'body_text',
129+
'headline': 'headline',
130+
'comment': {}
131+
}
132+
}
133+
}
134+
135+
assert snapshot == perform_error_test(client, data)
136+
137+
138+
def test_third_level_array_error(client, some_blog, snapshot):
139+
data = {
140+
'data': {
141+
'type': 'entries',
142+
'attributes': {
143+
'blog': some_blog.pk,
144+
'bodyText': 'body_text',
145+
'headline': 'headline',
146+
'comments': [
147+
{
148+
'body': 'test comment',
149+
'attachments': [
150+
{
151+
}
152+
]
153+
}
154+
]
155+
}
156+
}
157+
}
158+
159+
assert snapshot == perform_error_test(client, data)
160+
161+
162+
def test_third_level_custom_array_error(client, some_blog, snapshot):
163+
data = {
164+
'data': {
165+
'type': 'entries',
166+
'attributes': {
167+
'blog': some_blog.pk,
168+
'bodyText': 'body_text',
169+
'headline': 'headline',
170+
'comments': [
171+
{
172+
'body': 'test comment',
173+
'attachments': [
174+
{
175+
'data': 'text'
176+
}
177+
]
178+
}
179+
]
180+
}
181+
}
182+
}
183+
184+
assert snapshot == perform_error_test(client, data)
185+
186+
187+
def test_third_level_dict_error(client, some_blog, snapshot):
188+
data = {
189+
'data': {
190+
'type': 'entries',
191+
'attributes': {
192+
'blog': some_blog.pk,
193+
'bodyText': 'body_text',
194+
'headline': 'headline',
195+
'comments': [
196+
{
197+
'body': 'test comment',
198+
'attachment': {}
199+
}
200+
]
201+
}
202+
}
203+
}
204+
205+
assert snapshot == perform_error_test(client, data)
206+
207+
208+
def test_many_third_level_dict_errors(client, some_blog, snapshot):
209+
data = {
210+
'data': {
211+
'type': 'entries',
212+
'attributes': {
213+
'blog': some_blog.pk,
214+
'bodyText': 'body_text',
215+
'headline': 'headline',
216+
'comments': [
217+
{
218+
'attachment': {}
219+
}
220+
]
221+
}
222+
}
223+
}
224+
225+
assert snapshot == perform_error_test(client, data)
226+
227+
228+
def test_deprecation_warning(recwarn, settings, snapshot):
229+
settings.JSON_API_SERIALIZE_NESTED_SERIALIZERS_AS_ATTRIBUTE = False
230+
231+
class DummyNestedSerializer(serializers.Serializer):
232+
field = serializers.CharField()
233+
234+
class DummySerializer(serializers.Serializer):
235+
nested = DummyNestedSerializer(many=True)
236+
237+
assert len(recwarn) == 1
238+
warning = recwarn.pop(DeprecationWarning)
239+
assert snapshot == warning.message

‎example/tests/test_generic_viewset.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@ def test_custom_validation_exceptions(self):
9595
"""
9696
expected = {
9797
'errors': [
98-
{
99-
'id': 'armageddon101',
100-
'detail': 'Hey! You need a last name!',
101-
'meta': 'something',
102-
},
10398
{
10499
'status': '400',
105100
'source': {
@@ -108,6 +103,12 @@ def test_custom_validation_exceptions(self):
108103
'detail': 'Enter a valid email address.',
109104
'code': 'invalid',
110105
},
106+
{
107+
'id': 'armageddon101',
108+
'detail': 'Hey! You need a last name!',
109+
'meta': 'something',
110+
'source': {'pointer': '/data/attributes/lastName'}
111+
},
111112
]
112113
}
113114
response = self.client.post('/identities', {

‎requirements/requirements-testing.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ pytest==6.0.1
55
pytest-cov==2.10.1
66
pytest-django==3.9.0
77
pytest-factoryboy==2.0.3
8+
syrupy==0.6.1

‎rest_framework_json_api/utils.py

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -312,29 +312,25 @@ def format_drf_errors(response, context, exc):
312312
# handle generic errors. ValidationError('test') in a view for example
313313
if isinstance(response.data, list):
314314
for message in response.data:
315-
errors.append(format_error_object(message, '/data', response))
315+
errors.extend(format_error_object(message, '/data', response))
316316
# handle all errors thrown from serializers
317317
else:
318318
for field, error in response.data.items():
319319
field = format_value(field)
320320
pointer = '/data/attributes/{}'.format(field)
321-
# see if they passed a dictionary to ValidationError manually
322-
if isinstance(error, dict):
323-
errors.append(error)
324-
elif isinstance(exc, Http404) and isinstance(error, str):
321+
if isinstance(exc, Http404) and isinstance(error, str):
325322
# 404 errors don't have a pointer
326-
errors.append(format_error_object(error, None, response))
323+
errors.extend(format_error_object(error, None, response))
327324
elif isinstance(error, str):
328325
classes = inspect.getmembers(exceptions, inspect.isclass)
329326
# DRF sets the `field` to 'detail' for its own exceptions
330327
if isinstance(exc, tuple(x[1] for x in classes)):
331328
pointer = '/data'
332-
errors.append(format_error_object(error, pointer, response))
329+
errors.extend(format_error_object(error, pointer, response))
333330
elif isinstance(error, list):
334-
for message in error:
335-
errors.append(format_error_object(message, pointer, response))
331+
errors.extend(format_error_object(error, pointer, response))
336332
else:
337-
errors.append(format_error_object(error, pointer, response))
333+
errors.extend(format_error_object(error, pointer, response))
338334

339335
context['view'].resource_name = 'errors'
340336
response.data = errors
@@ -343,18 +339,49 @@ def format_drf_errors(response, context, exc):
343339

344340

345341
def format_error_object(message, pointer, response):
346-
error_obj = {
347-
'detail': message,
348-
'status': encoding.force_str(response.status_code),
349-
}
350-
if pointer is not None:
351-
error_obj['source'] = {
352-
'pointer': pointer,
342+
errors = []
343+
if isinstance(message, dict):
344+
345+
# as there is no required field in error object we check that all fields are string
346+
# except links and source which might be a dict
347+
is_custom_error = all([
348+
isinstance(value, str)
349+
for key, value in message.items() if key not in ['links', 'source']
350+
])
351+
352+
if is_custom_error:
353+
if 'source' not in message:
354+
message['source'] = {}
355+
message['source'] = {
356+
'pointer': pointer,
357+
}
358+
errors.append(message)
359+
else:
360+
for k, v in message.items():
361+
errors.extend(format_error_object(v, pointer + '/{}'.format(k), response))
362+
elif isinstance(message, list):
363+
for num, error in enumerate(message):
364+
if isinstance(error, (list, dict)):
365+
new_pointer = pointer + '/{}'.format(num)
366+
else:
367+
new_pointer = pointer
368+
if error:
369+
errors.extend(format_error_object(error, new_pointer, response))
370+
else:
371+
error_obj = {
372+
'detail': message,
373+
'status': encoding.force_str(response.status_code),
353374
}
354-
code = getattr(message, "code", None)
355-
if code is not None:
356-
error_obj['code'] = code
357-
return error_obj
375+
if pointer is not None:
376+
error_obj['source'] = {
377+
'pointer': pointer,
378+
}
379+
code = getattr(message, "code", None)
380+
if code is not None:
381+
error_obj['code'] = code
382+
errors.append(error_obj)
383+
384+
return errors
358385

359386

360387
def format_errors(data):

0 commit comments

Comments
 (0)
Please sign in to comment.