Merge pull request #451 from markotibold/#431

Call model's .full_clean() method, eg. to validate uniqueness

3 files changed, 54 insertions, 13 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index 67eafdf0..5edd46f5 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -127,6 +127,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.get_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.
@@ -226,10 +237,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
 
@@ -441,10 +459,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,
@@ -468,6 +482,16 @@ class ModelSerializer(Serializer):
         except KeyError:
             return ModelField(model_field=model_field, **kwargs)
 
+    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 attrs
+
+    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.get_excluded_fieldnames()))
+
     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 76435df8..428bf130 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):
@@ -160,7 +160,7 @@ class Photo(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 6aa211f4..455fa270 100644
--- a/rest_framework/tests/serializer.py
+++ b/rest_framework/tests/serializer.py
@@ -1,7 +1,7 @@
 import datetime, pickle
 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(
@@ -169,7 +175,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):
@@ -276,6 +282,17 @@ 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())
+        self.assertEqual(second_serializer.errors,  {'title': [u'Album with this Title already exists.']})
+
 
 class RegexValidationTest(TestCase):
     def test_create_failed(self):