From 66605acaf02d46eb899f495137afb4f9ff127ff0 Mon Sep 17 00:00:00 2001 From: Ian Dash Date: Thu, 7 Mar 2013 17:29:25 +0000 Subject: [PATCH 1/4] Errors during deserializing lists now return a list of tuples with index of bad item in data plus usual errors dict --- docs/api-guide/serializers.md | 2 ++ rest_framework/serializers.py | 17 +++++++---- rest_framework/tests/serializer.py | 45 +++++++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index 6f1f28832..de2cf7d8f 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -93,6 +93,8 @@ To serialize a queryset instead of an object instance, you should pass the `many When deserializing data, you always need to call `is_valid()` before attempting to access the deserialized object. If any validation errors occur, the `.errors` and `.non_field_errors` properties will contain the resulting error messages. +When deserialising a list of items, errors will be returned as a list of tuples. The first item in an error tuple will be the index of the item with the error in the original data; The second item in the tuple will be a dict with the individual errors for that item. + ### Field-level validation You can specify custom field-level validation by adding `.validate_` methods to your `Serializer` subclass. These are analagous to `.clean_` methods on Django forms, but accept slightly different arguments. diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index ba9e9e9cf..80287522f 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -286,8 +286,18 @@ class BaseSerializer(Field): Deserialize primitives -> objects. """ if hasattr(data, '__iter__') and not isinstance(data, (dict, six.text_type)): - # TODO: error data when deserializing lists - return [self.from_native(item, None) for item in data] + object_list = list() + error_list = list() + for count, item in enumerate(data): + obj = self.from_native(item, None) + if self._errors: + error_list.append((count, self._errors)) + object_list.append(obj) + if not error_list: + return object_list + + self._errors = error_list + return None self._errors = {} if data is not None or files is not None: @@ -354,9 +364,6 @@ class BaseSerializer(Field): 'Use the `many=True` flag when instantiating the serializer.', PendingDeprecationWarning, stacklevel=3) - # TODO: error data when deserializing lists - if many: - ret = [self.from_native(item, None) for item in data] ret = self.from_native(data, files) if not self._errors: diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 510650173..339109363 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -268,7 +268,16 @@ class ValidationTests(TestCase): data = ['i am', 'a', 'list'] serializer = CommentSerializer(self.comment, data=data, many=True) self.assertEqual(serializer.is_valid(), False) - self.assertEqual(serializer.errors, {'non_field_errors': ['Invalid data']}) + self.assertTrue(isinstance(serializer.errors, list)) + + self.assertEqual( + serializer.errors, + [ + (0, {'non_field_errors': ['Invalid data']}), + (1, {'non_field_errors': ['Invalid data']}), + (2, {'non_field_errors': ['Invalid data']}) + ] + ) data = 'and i am a string' serializer = CommentSerializer(self.comment, data=data) @@ -1072,3 +1081,37 @@ class NestedSerializerContextTests(TestCase): # This will raise RuntimeError if context doesn't get passed correctly to the nested Serializers AlbumCollectionSerializer(album_collection, context={'context_item': 'album context'}).data + + +class DeserializeListTestCase(TestCase): + + def setUp(self): + self.data = { + 'email': 'nobody@nowhere.com', + 'content': 'This is some test content', + 'created': datetime.datetime(2013, 3, 7), + } + + def test_no_errors(self): + data = [self.data.copy() for x in range(0, 3)] + serializer = CommentSerializer(data=data) + self.assertTrue(serializer.is_valid()) + self.assertTrue(isinstance(serializer.object, list)) + self.assertTrue( + all((isinstance(item, Comment) for item in serializer.object)) + ) + + def test_errors_return_as_list(self): + invalid_item = self.data.copy() + invalid_item['email'] = '' + data = [self.data.copy(), invalid_item, self.data.copy()] + + serializer = CommentSerializer(data=data) + self.assertFalse(serializer.is_valid()) + self.assertTrue(isinstance(serializer.errors, list)) + self.assertEqual(1, len(serializer.errors)) + expected = (1, {'email': ['This field is required.']}) + self.assertEqual( + serializer.errors[0], + expected + ) From 68683b2ea2907f367fdff60de91656504a242a14 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 8 Mar 2013 22:19:09 +0000 Subject: [PATCH 2/4] Tweak implementation, and use FormSet style errors --- rest_framework/serializers.py | 24 +++++++++--------------- rest_framework/tests/serializer.py | 15 +++++---------- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 80287522f..25790dbc4 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -285,20 +285,6 @@ class BaseSerializer(Field): """ Deserialize primitives -> objects. """ - if hasattr(data, '__iter__') and not isinstance(data, (dict, six.text_type)): - object_list = list() - error_list = list() - for count, item in enumerate(data): - obj = self.from_native(item, None) - if self._errors: - error_list.append((count, self._errors)) - object_list.append(obj) - if not error_list: - return object_list - - self._errors = error_list - return None - self._errors = {} if data is not None or files is not None: attrs = self.restore_fields(data, files) @@ -364,7 +350,15 @@ class BaseSerializer(Field): 'Use the `many=True` flag when instantiating the serializer.', PendingDeprecationWarning, stacklevel=3) - ret = self.from_native(data, files) + if many: + ret = [] + errors = [] + for item in data: + ret.append(self.from_native(item, None)) + errors.append(self._errors) + self._errors = any(errors) and errors or [] + else: + ret = self.from_native(data, files) if not self._errors: self.object = ret diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 339109363..394af8278 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -273,9 +273,9 @@ class ValidationTests(TestCase): self.assertEqual( serializer.errors, [ - (0, {'non_field_errors': ['Invalid data']}), - (1, {'non_field_errors': ['Invalid data']}), - (2, {'non_field_errors': ['Invalid data']}) + {'non_field_errors': ['Invalid data']}, + {'non_field_errors': ['Invalid data']}, + {'non_field_errors': ['Invalid data']} ] ) @@ -1108,10 +1108,5 @@ class DeserializeListTestCase(TestCase): serializer = CommentSerializer(data=data) self.assertFalse(serializer.is_valid()) - self.assertTrue(isinstance(serializer.errors, list)) - self.assertEqual(1, len(serializer.errors)) - expected = (1, {'email': ['This field is required.']}) - self.assertEqual( - serializer.errors[0], - expected - ) + expected = [{}, {'email': [u'This field is required.']}, {}] + self.assertEqual(serializer.errors, expected) From 0b6267d8cd45995585f0c02a4f9c96c0691fd32f Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 8 Mar 2013 22:28:59 +0000 Subject: [PATCH 3/4] Added @bitmonkey. Thanks! For work on handling errors when deserializing lists of objects. --- docs/topics/credits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/credits.md b/docs/topics/credits.md index 190ce490e..d6f312ed8 100644 --- a/docs/topics/credits.md +++ b/docs/topics/credits.md @@ -108,6 +108,7 @@ The following people have helped make REST framework great. * Omer Katz - [thedrow] * Wiliam Souza - [waa] * Jonas Braun - [iekadou] +* Ian Dash - [bitmonkey] Many thanks to everyone who's contributed to the project. @@ -250,3 +251,4 @@ You can also contact [@_tomchristie][twitter] directly on twitter. [thedrow]: https://github.com/thedrow [waa]: https://github.com/wiliamsouza [iekadou]: https://github.com/iekadou +[bitmonkey]: https://github.com/bitmonkey From 28ae26466e1b1493feeba19480c6eb148d603330 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 8 Mar 2013 22:43:46 +0000 Subject: [PATCH 4/4] Py3k fixes. --- rest_framework/serializers.py | 8 ++++---- rest_framework/tests/serializer.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 25790dbc4..106e3f17a 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -7,8 +7,7 @@ from django.core.paginator import Page from django.db import models from django.forms import widgets from django.utils.datastructures import SortedDict -from rest_framework.compat import get_concrete_model -from rest_framework.compat import six +from rest_framework.compat import get_concrete_model, six # Note: We do the following so that users of the framework can use this style: # @@ -326,7 +325,7 @@ class BaseSerializer(Field): if self.many is not None: many = self.many else: - many = hasattr(obj, '__iter__') and not isinstance(obj, (Page, dict)) + many = hasattr(obj, '__iter__') and not isinstance(obj, (Page, dict, six.text_type)) if many: return [self.to_native(item) for item in obj] @@ -344,7 +343,7 @@ class BaseSerializer(Field): if self.many is not None: many = self.many else: - many = hasattr(data, '__iter__') and not isinstance(data, (Page, dict)) + many = hasattr(data, '__iter__') and not isinstance(data, (Page, dict, six.text_type)) if many: warnings.warn('Implict list/queryset serialization is due to be deprecated. ' 'Use the `many=True` flag when instantiating the serializer.', @@ -362,6 +361,7 @@ class BaseSerializer(Field): if not self._errors: self.object = ret + return self._errors def is_valid(self): diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 394af8278..beb372c2b 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -1108,5 +1108,5 @@ class DeserializeListTestCase(TestCase): serializer = CommentSerializer(data=data) self.assertFalse(serializer.is_valid()) - expected = [{}, {'email': [u'This field is required.']}, {}] + expected = [{}, {'email': ['This field is required.']}, {}] self.assertEqual(serializer.errors, expected)