From dc1c57d595c3917e3fed9076894d5fa88ec083c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Mon, 21 Jan 2013 12:45:30 +0100 Subject: [PATCH 1/5] Add failed testcase for fieldvalidation --- rest_framework/tests/serializer.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index bd96ba23e..0ba4e7650 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -241,6 +241,14 @@ class ValidationTests(TestCase): self.assertFalse(serializer.is_valid()) self.assertEquals(serializer.errors, {'content': [u'Test not in value']}) + incomplete_data = { + 'email': 'tom@example.com', + 'created': datetime.datetime(2012, 1, 1) + } + serializer = CommentSerializerWithFieldValidator(data=incomplete_data) + self.assertFalse(serializer.is_valid()) + self.assertEquals(serializer.errors, {'content': [u'This field is required.']}) + def test_bad_type_data_is_false(self): """ Data of the wrong type is not valid. From 2250ab6418d3cf99719ea7c5e3b3a861afa850bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Mon, 21 Jan 2013 12:50:39 +0100 Subject: [PATCH 2/5] Add possible solution for field validation error --- rest_framework/serializers.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 27458f968..0c60d17f5 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -227,13 +227,14 @@ class BaseSerializer(Field): Run `validate_()` and `validate()` methods on the serializer """ for field_name, field in self.fields.items(): - try: - validate_method = getattr(self, 'validate_%s' % field_name, None) - if validate_method: - source = field.source or field_name - attrs = validate_method(attrs, source) - except ValidationError as err: - self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) + if field_name not in self._errors: + try: + validate_method = getattr(self, 'validate_%s' % field_name, None) + if validate_method: + source = field.source or field_name + attrs = validate_method(attrs, source) + except ValidationError as err: + self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) # If there are already errors, we don't run .validate() because # field-validation failed and thus `attrs` may not be complete. From f0071dbccd592ba3157738ced66809869f68b1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Wed, 23 Jan 2013 07:52:56 +0100 Subject: [PATCH 3/5] Add separate test for failed custom validation --- rest_framework/tests/serializer.py | 96 ++++++++++++++++++------------ 1 file changed, 59 insertions(+), 37 deletions(-) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 0ba4e7650..b4428ca31 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -162,7 +162,6 @@ class BasicTests(TestCase): """ Attempting to update fields set as read_only should have no effect. """ - serializer = PersonSerializer(self.person, data={'name': 'dwight', 'age': 99}) self.assertEquals(serializer.is_valid(), True) instance = serializer.save() @@ -183,8 +182,7 @@ class ValidationTests(TestCase): 'content': 'x' * 1001, 'created': datetime.datetime(2012, 1, 1) } - self.actionitem = ActionItem(title='Some to do item', - ) + self.actionitem = ActionItem(title='Some to do item',) def test_create(self): serializer = CommentSerializer(data=self.data) @@ -216,39 +214,6 @@ class ValidationTests(TestCase): self.assertEquals(serializer.is_valid(), True) self.assertEquals(serializer.errors, {}) - def test_field_validation(self): - - class CommentSerializerWithFieldValidator(CommentSerializer): - - def validate_content(self, attrs, source): - value = attrs[source] - if "test" not in value: - raise serializers.ValidationError("Test not in value") - return attrs - - data = { - 'email': 'tom@example.com', - 'content': 'A test comment', - 'created': datetime.datetime(2012, 1, 1) - } - - serializer = CommentSerializerWithFieldValidator(data=data) - self.assertTrue(serializer.is_valid()) - - data['content'] = 'This should not validate' - - serializer = CommentSerializerWithFieldValidator(data=data) - self.assertFalse(serializer.is_valid()) - self.assertEquals(serializer.errors, {'content': [u'Test not in value']}) - - incomplete_data = { - 'email': 'tom@example.com', - 'created': datetime.datetime(2012, 1, 1) - } - serializer = CommentSerializerWithFieldValidator(data=incomplete_data) - self.assertFalse(serializer.is_valid()) - self.assertEquals(serializer.errors, {'content': [u'This field is required.']}) - def test_bad_type_data_is_false(self): """ Data of the wrong type is not valid. @@ -318,12 +283,69 @@ class ValidationTests(TestCase): self.assertEquals(serializer.errors, {'info': [u'Ensure this value has at most 12 characters (it has 13).']}) +class CustomValidationTests(TestCase): + class CommentSerializerWithFieldValidator(CommentSerializer): + + def validate_email(self, attrs, source): + value = attrs[source] + + return attrs + + def validate_content(self, attrs, source): + value = attrs[source] + if "test" not in value: + raise serializers.ValidationError("Test not in value") + return attrs + + def test_field_validation(self): + data = { + 'email': 'tom@example.com', + 'content': 'A test comment', + 'created': datetime.datetime(2012, 1, 1) + } + + serializer = self.CommentSerializerWithFieldValidator(data=data) + self.assertTrue(serializer.is_valid()) + + data['content'] = 'This should not validate' + + serializer = self.CommentSerializerWithFieldValidator(data=data) + self.assertFalse(serializer.is_valid()) + self.assertEquals(serializer.errors, {'content': [u'Test not in value']}) + + def test_missing_data(self): + """ + Make sure that validate_content isn't called if the field is missing + """ + incomplete_data = { + 'email': 'tom@example.com', + 'created': datetime.datetime(2012, 1, 1) + } + serializer = self.CommentSerializerWithFieldValidator(data=incomplete_data) + self.assertFalse(serializer.is_valid()) + self.assertEquals(serializer.errors, {'content': [u'This field is required.']}) + + def test_wrong_data(self): + """ + Make sure that validate_content isn't called if the field input is wrong + """ + wrong_data = { + 'email': 'not an email', + 'content': 'A test comment', + 'created': datetime.datetime(2012, 1, 1) + } + serializer = self.CommentSerializerWithFieldValidator(data=wrong_data) + self.assertFalse(serializer.is_valid()) + self.assertEquals(serializer.errors, {'email': [u'Enter a valid e-mail address.']}) + + class PositiveIntegerAsChoiceTests(TestCase): def test_positive_integer_in_json_is_correctly_parsed(self): - data = {'some_integer':1} + data = {'some_integer': 1} serializer = PositiveIntegerAsChoiceSerializer(data=data) self.assertEquals(serializer.is_valid(), True) + class ModelValidationTests(TestCase): def test_validate_unique(self): """ From 69e62457ef80b40398a6b137f10d5a0e05c3f07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Wed, 23 Jan 2013 07:53:54 +0100 Subject: [PATCH 4/5] Improve validate_ fix --- rest_framework/serializers.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 0c60d17f5..1450b9c76 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -227,14 +227,15 @@ class BaseSerializer(Field): Run `validate_()` and `validate()` methods on the serializer """ for field_name, field in self.fields.items(): - if field_name not in self._errors: - try: - validate_method = getattr(self, 'validate_%s' % field_name, None) - if validate_method: - source = field.source or field_name - attrs = validate_method(attrs, source) - except ValidationError as err: - self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) + if field_name in self._errors: + continue + try: + validate_method = getattr(self, 'validate_%s' % field_name, None) + if validate_method: + source = field.source or field_name + attrs = validate_method(attrs, source) + except ValidationError as err: + self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) # If there are already errors, we don't run .validate() because # field-validation failed and thus `attrs` may not be complete. From 30046cae8c64790d7ae0d9ca4d2faee1cd2968aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20Gro=C3=9F?= Date: Wed, 23 Jan 2013 07:55:00 +0100 Subject: [PATCH 5/5] Add validate_ bugfix to release notes --- docs/topics/release-notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index 58471a790..54c249bb3 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -22,6 +22,7 @@ Major version numbers (x.0.0) are reserved for project milestones. No major poi * `format_suffix_patterns()` now supports `include` style URL patterns. * Bugfix: Return proper validation errors when incorrect types supplied for relational fields. * Bugfix: Support nullable FKs with `SlugRelatedField`. +* Bugfix: Don't call custom validation methods if the field has an error. ### 2.1.16