From ca5b99486d15e7392754178ab0948de2a60763a3 Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Thu, 22 Nov 2012 22:36:37 +0100 Subject: [PATCH 1/7] Added _post_clean() behaviour by adding a .perform_model_validation() method. Fixed some tests that were failing due to extra strict validation. --- rest_framework/serializers.py | 57 +++++++++++++++++++++++++++--- rest_framework/tests/models.py | 4 +-- rest_framework/tests/serializer.py | 2 +- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 53dcec163..6d5b4cb53 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -428,10 +428,6 @@ class ModelSerializer(Serializer): kwargs['choices'] = model_field.flatchoices return ChoiceField(**kwargs) - max_length = getattr(model_field, 'max_length', None) - if max_length: - kwargs['max_length'] = max_length - field_mapping = { models.FloatField: FloatField, models.IntegerField: IntegerField, @@ -455,6 +451,59 @@ class ModelSerializer(Serializer): except KeyError: return ModelField(model_field=model_field, **kwargs) + def from_native(self, data, files): + restored_object = super(ModelSerializer, self).from_native(data, files) + + if restored_object is None: + return + + self.perform_model_validation(restored_object) + return restored_object + + def perform_model_validation(self, restored_object): + + # if hasattr(restored_object, '__iter__'): # Iterables are not model instances + # return restored_object + #self._errors[field_name] = list(err.messages) + +# opts = self._meta + # Update the model instance with self.cleaned_data. +# instance = construct_instance(self, self.instance, opts.fields, opts.exclude) + +# exclude = self._get_validation_exclusions() + + # Foreign Keys being used to represent inline relationships + # are excluded from basic field value validation. This is for two + # reasons: firstly, the value may not be supplied (#12507; the + # case of providing new values to the admin); secondly the + # object being referred to may not yet fully exist (#12749). + # However, these fields *must* be included in uniqueness checks, + # so this can't be part of _get_validation_exclusions(). +# for f_name, field in self.fields.items(): +# if isinstance(field, InlineForeignKeyField): +# exclude.append(f_name) + + # Clean the model instance's fields. + try: + restored_object.clean_fields() # exclude=exclude) + except ValidationError as e: + for field_name, error_messages in e.message_dict.items(): + self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) + + # Call the model instance's clean method. + try: + restored_object.clean() + except ValidationError as e: + for field_name, error_messages in e.message_dict.items(): + self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) + + # Validate uniqueness if needed. + # exclude = self._get_validation_exclusions() +# try: +# restored_object.validate_unique() # exclude=exclude) +# except ValidationError as e: +# model_errors.append(e.message_dict) + def restore_object(self, attrs, instance=None): """ Restore the model instance. diff --git a/rest_framework/tests/models.py b/rest_framework/tests/models.py index c35861c6c..9a59e8411 100644 --- a/rest_framework/tests/models.py +++ b/rest_framework/tests/models.py @@ -61,7 +61,7 @@ class BasicModel(RESTFrameworkModel): class SlugBasedModel(RESTFrameworkModel): text = models.CharField(max_length=100) - slug = models.SlugField(max_length=32) + slug = models.SlugField(max_length=32, blank=True) class DefaultValueModel(RESTFrameworkModel): @@ -159,7 +159,7 @@ class Person(RESTFrameworkModel): # Model for issue #324 class BlankFieldModel(RESTFrameworkModel): - title = models.CharField(max_length=100, blank=True) + title = models.CharField(max_length=100, blank=True, null=True) # Model for issue #380 diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 61a05da18..5751e8940 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -169,7 +169,7 @@ class ValidationTests(TestCase): 'content': 'x' * 1001, 'created': datetime.datetime(2012, 1, 1) } - self.actionitem = ActionItem('Some to do item', + self.actionitem = ActionItem(title='Some to do item', ) def test_create(self): From bd8c742df2cc72896fa975196fdf56961e89cd94 Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Thu, 22 Nov 2012 23:39:16 +0100 Subject: [PATCH 2/7] Cleanup. --- rest_framework/serializers.py | 40 ++--------------------------------- 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 6d5b4cb53..bb15faa8a 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -461,49 +461,13 @@ class ModelSerializer(Serializer): return restored_object def perform_model_validation(self, restored_object): - - # if hasattr(restored_object, '__iter__'): # Iterables are not model instances - # return restored_object - #self._errors[field_name] = list(err.messages) - -# opts = self._meta - # Update the model instance with self.cleaned_data. -# instance = construct_instance(self, self.instance, opts.fields, opts.exclude) - -# exclude = self._get_validation_exclusions() - - # Foreign Keys being used to represent inline relationships - # are excluded from basic field value validation. This is for two - # reasons: firstly, the value may not be supplied (#12507; the - # case of providing new values to the admin); secondly the - # object being referred to may not yet fully exist (#12749). - # However, these fields *must* be included in uniqueness checks, - # so this can't be part of _get_validation_exclusions(). -# for f_name, field in self.fields.items(): -# if isinstance(field, InlineForeignKeyField): -# exclude.append(f_name) - - # Clean the model instance's fields. try: - restored_object.clean_fields() # exclude=exclude) + # Call Django's full_clean() which in turn calls: Model.clean_fields(), Model.clean(), Model.validat_unique() + restored_object.full_clean(exclude=list(self.opts.exclude)) except ValidationError as e: for field_name, error_messages in e.message_dict.items(): self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) - # Call the model instance's clean method. - try: - restored_object.clean() - except ValidationError as e: - for field_name, error_messages in e.message_dict.items(): - self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) - - # Validate uniqueness if needed. - # exclude = self._get_validation_exclusions() -# try: -# restored_object.validate_unique() # exclude=exclude) -# except ValidationError as e: -# model_errors.append(e.message_dict) - def restore_object(self, attrs, instance=None): """ Restore the model instance. From 3f47f6cea9d178a57855e3b90208601b7e28a80f Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Thu, 22 Nov 2012 23:50:42 +0100 Subject: [PATCH 3/7] Added a validate_unique test. --- rest_framework/tests/serializer.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 5751e8940..0baf0e89d 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -1,7 +1,7 @@ import datetime from django.test import TestCase from rest_framework import serializers -from rest_framework.tests.models import (ActionItem, Anchor, BasicModel, +from rest_framework.tests.models import (Album, ActionItem, Anchor, BasicModel, BlankFieldModel, BlogPost, Book, CallableDefaultValueModel, DefaultValueModel, ManyToManyModel, Person, ReadOnlyManyToManyModel) @@ -48,7 +48,7 @@ class BookSerializer(serializers.ModelSerializer): class ActionItemSerializer(serializers.ModelSerializer): - + class Meta: model = ActionItem @@ -62,6 +62,12 @@ class PersonSerializer(serializers.ModelSerializer): read_only_fields = ('age',) +class AlbumsSerializer(serializers.ModelSerializer): + + class Meta: + model = Album + + class BasicTests(TestCase): def setUp(self): self.comment = Comment( @@ -276,6 +282,16 @@ class ValidationTests(TestCase): self.assertEquals(serializer.is_valid(), False) self.assertEquals(serializer.errors, {'info': [u'Ensure this value has at most 12 characters (it has 13).']}) + def test_validate_unique(self): + """ + Just check if serializers.ModelSerializer.perform_model_validation() handles unique checks via .full_clean() + """ + serializer = AlbumsSerializer(data={'title': 'a'}) + serializer.is_valid() + serializer.save() + second_serializer = AlbumsSerializer(data={'title': 'a'}) + self.assertFalse(second_serializer.is_valid()) + class RegexValidationTest(TestCase): def test_create_failed(self): From e7666014a85d65e204b40e1f54911e654f974932 Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Mon, 26 Nov 2012 23:39:49 +0100 Subject: [PATCH 4/7] Added an assertion to the tests that checks the '.errors' value for the unique-test --- rest_framework/tests/serializer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 0baf0e89d..bdf72a91b 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -291,6 +291,7 @@ class ValidationTests(TestCase): serializer.save() second_serializer = AlbumsSerializer(data={'title': 'a'}) self.assertFalse(second_serializer.is_valid()) + self.assertEqual(second_serializer.errors, {'title': [u'Album with this Title already exists.']}) class RegexValidationTest(TestCase): From f104f7434052bedf6dd970806ff54b73489b339b Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Tue, 27 Nov 2012 23:21:12 +0100 Subject: [PATCH 5/7] Moved model validation from .perform_validation() to .validate() --- rest_framework/serializers.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index bb15faa8a..5046c7b1e 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -221,10 +221,17 @@ class BaseSerializer(Field): except ValidationError as err: self._errors[field_name] = self._errors.get(field_name, []) + list(err.messages) - try: - attrs = self.validate(attrs) - except ValidationError as err: - self._errors['non_field_errors'] = err.messages + # We don't run .validate() because field-validation failed and thus `attrs` may not be complete. + # which in turn can cause inconsistent validation errors. + if not self._errors: + try: + attrs = self.validate(attrs) + except ValidationError as err: + if hasattr(err, 'message_dict'): + for field_name, error_messages in err.message_dict.items(): + self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) + elif hasattr(err, 'messages'): + self._errors['non_field_errors'] = err.messages return attrs @@ -451,22 +458,15 @@ class ModelSerializer(Serializer): except KeyError: return ModelField(model_field=model_field, **kwargs) - def from_native(self, data, files): - restored_object = super(ModelSerializer, self).from_native(data, files) - - if restored_object is None: - return - + def validate(self, attrs): + copied_attrs = copy.deepcopy(attrs) + restored_object = self.restore_object(copied_attrs, instance=getattr(self, 'object', None)) self.perform_model_validation(restored_object) - return restored_object + return attrs def perform_model_validation(self, restored_object): - try: - # Call Django's full_clean() which in turn calls: Model.clean_fields(), Model.clean(), Model.validat_unique() - restored_object.full_clean(exclude=list(self.opts.exclude)) - except ValidationError as e: - for field_name, error_messages in e.message_dict.items(): - self._errors[field_name] = self._errors.get(field_name, []) + list(error_messages) + # Call Django's full_clean() which in turn calls: Model.clean_fields(), Model.clean(), Model.validat_unique() + restored_object.full_clean(exclude=list(self.opts.exclude)) def restore_object(self, attrs, instance=None): """ From 899f96ae9186e68009dba5d54246232d34457354 Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Tue, 27 Nov 2012 23:49:27 +0100 Subject: [PATCH 6/7] Added a get_excluded_fieldnames() method. Model validation now excludes fields not listed in Meta fields (if set). --- rest_framework/serializers.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 5046c7b1e..775a8a1e2 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -119,6 +119,17 @@ class BaseSerializer(Field): """ return {} + def get_excluded_fieldnames(self): + """ + Returns the fieldnames that should not be validated. + """ + excluded_fields = list(self.opts.exclude) + for field in self.fields.keys() + self.default_fields.keys(): + if self.opts.fields: + if field not in self.opts.fields + self.opts.exclude: + excluded_fields.append(field) + return excluded_fields + def get_fields(self): """ Returns the complete set of fields for the object as a dict. @@ -466,7 +477,7 @@ class ModelSerializer(Serializer): def perform_model_validation(self, restored_object): # Call Django's full_clean() which in turn calls: Model.clean_fields(), Model.clean(), Model.validat_unique() - restored_object.full_clean(exclude=list(self.opts.exclude)) + restored_object.full_clean(exclude=list(self.get_excluded_fieldnames())) def restore_object(self, attrs, instance=None): """ From 919aff329ee1bd214831095e4d96af71795ed572 Mon Sep 17 00:00:00 2001 From: Marko Tibold Date: Fri, 7 Dec 2012 00:08:27 +0100 Subject: [PATCH 7/7] Fix AttributeError caused by accessing a non-existing default_keys attribute. --- rest_framework/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 984f3ac57..43bfda83b 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -123,7 +123,7 @@ class BaseSerializer(Field): Returns the fieldnames that should not be validated. """ excluded_fields = list(self.opts.exclude) - for field in self.fields.keys() + self.default_fields.keys(): + for field in self.fields.keys() + self.get_default_fields().keys(): if self.opts.fields: if field not in self.opts.fields + self.opts.exclude: excluded_fields.append(field)