Ensure model field validation is performed for ModelSerializers with a custom restore_object method. Fixes #623.

2 files changed, 51 insertions, 6 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index 6ecc7b45..0fed2c29 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -513,6 +513,22 @@ class ModelSerializer(Serializer):
                 exclusions.remove(field_name)
         return exclusions
 
+    def full_clean(self, instance):
+        """
+        Perform Django's full_clean, and populate the `errors` dictionary
+        if any validation errors occur.
+
+        Note that we don't perform this inside the `.restore_object()` method,
+        so that subclasses can override `.restore_object()`, and still get
+        the full_clean validation checking.
+        """
+        try:
+            instance.full_clean(exclude=self.get_validation_exclusions())
+        except ValidationError, err:
+            self._errors = err.message_dict
+            return None
+        return instance
+
     def restore_object(self, attrs, instance=None):
         """
         Restore the model instance.
@@ -544,14 +560,16 @@ class ModelSerializer(Serializer):
         else:
             instance = self.opts.model(**attrs)
 
-        try:
-            instance.full_clean(exclude=self.get_validation_exclusions())
-        except ValidationError, err:
-            self._errors = err.message_dict
-            return None
-
         return instance
 
+    def from_native(self, data, files):
+        """
+        Override the default method to also include model field validation.
+        """
+        instance = super(ModelSerializer, self).from_native(data, files)
+        if instance:
+            return self.full_clean(instance)
+
     def save(self):
         """
         Save the deserialized object and return it.
diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py
index b4428ca3..48b4f1ab 100644
--- a/rest_framework/tests/serializer.py
+++ b/rest_framework/tests/serializer.py
@@ -54,6 +54,19 @@ class ActionItemSerializer(serializers.ModelSerializer):
         model = ActionItem
 
 
+class ActionItemSerializerCustomRestore(serializers.ModelSerializer):
+
+    class Meta:
+        model = ActionItem
+
+    def restore_object(self, data, instance=None):
+        if instance is None:
+            return ActionItem(**data)
+        for key, val in data.items():
+            setattr(instance, key, val)
+        return instance
+
+
 class PersonSerializer(serializers.ModelSerializer):
     info = serializers.Field(source='info')
 
@@ -273,6 +286,20 @@ class ValidationTests(TestCase):
         self.assertEquals(serializer.is_valid(), False)
         self.assertEquals(serializer.errors, {'title': [u'Ensure this value has at most 200 characters (it has 201).']})
 
+    def test_modelserializer_max_length_exceeded_with_custom_restore(self):
+        """
+        When overriding ModelSerializer.restore_object, validation tests should still apply.
+        Regression test for #623.
+
+        https://github.com/tomchristie/django-rest-framework/pull/623
+        """
+        data = {
+            'title': 'x' * 201,
+        }
+        serializer = ActionItemSerializerCustomRestore(data=data)
+        self.assertEquals(serializer.is_valid(), False)
+        self.assertEquals(serializer.errors, {'title': [u'Ensure this value has at most 200 characters (it has 201).']})
+
     def test_default_modelfield_max_length_exceeded(self):
         data = {
             'title': 'Testing "info" field...',