Merge pull request #3 from tomchristie/master

Merge upstream

1 files changed, 39 insertions, 14 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index 0d0a4d9a..af8aeb48 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -127,6 +127,14 @@ class BaseSerializer(Field):
             (self.__class__.__module__, self.__class__.__name__)
         )
 
+        assert hasattr(self, '_errors'), (
+            'You must call `.is_valid()` before calling `.save()`.'
+        )
+
+        assert not self.errors, (
+            'You cannot call `.save()` on a serializer with invalid data.'
+        )
+
         validated_data = dict(
             list(self.validated_data.items()) +
             list(kwargs.items())
@@ -600,20 +608,20 @@ class ModelSerializer(Serializer):
     })
     _related_class = PrimaryKeyRelatedField
 
-    def create(self, validated_attrs):
+    def create(self, validated_data):
         """
         We have a bit of extra checking around this in order to provide
         descriptive messages when something goes wrong, but this method is
         essentially just:
 
-            return ExampleModel.objects.create(**validated_attrs)
+            return ExampleModel.objects.create(**validated_data)
 
         If there are many to many fields present on the instance then they
         cannot be set until the model is instantiated, in which case the
         implementation is like so:
 
-            example_relationship = validated_attrs.pop('example_relationship')
-            instance = ExampleModel.objects.create(**validated_attrs)
+            example_relationship = validated_data.pop('example_relationship')
+            instance = ExampleModel.objects.create(**validated_data)
             instance.example_relationship = example_relationship
             return instance
 
@@ -625,8 +633,8 @@ class ModelSerializer(Serializer):
         # If we don't do this explicitly they'd likely get a confusing
         # error at the point of calling `Model.objects.create()`.
         assert not any(
-            isinstance(field, BaseSerializer) and not field.read_only
-            for field in self.fields.values()
+            isinstance(field, BaseSerializer) and (key in validated_attrs)
+            for key, field in self.fields.items()
         ), (
             'The `.create()` method does not suport nested writable fields '
             'by default. Write an explicit `.create()` method for serializer '
@@ -636,16 +644,33 @@ class ModelSerializer(Serializer):
 
         ModelClass = self.Meta.model
 
-        # Remove many-to-many relationships from validated_attrs.
+        # Remove many-to-many relationships from validated_data.
         # They are not valid arguments to the default `.create()` method,
         # as they require that the instance has already been saved.
         info = model_meta.get_field_info(ModelClass)
         many_to_many = {}
         for field_name, relation_info in info.relations.items():
-            if relation_info.to_many and (field_name in validated_attrs):
-                many_to_many[field_name] = validated_attrs.pop(field_name)
+            if relation_info.to_many and (field_name in validated_data):
+                many_to_many[field_name] = validated_data.pop(field_name)
 
-        instance = ModelClass.objects.create(**validated_attrs)
+        try:
+            instance = ModelClass.objects.create(**validated_data)
+        except TypeError as exc:
+            msg = (
+                'Got a `TypeError` when calling `%s.objects.create()`. '
+                'This may be because you have a writable field on the '
+                'serializer class that is not a valid argument to '
+                '`%s.objects.create()`. You may need to make the field '
+                'read-only, or override the %s.create() method to handle '
+                'this correctly.\nOriginal exception text was: %s.' %
+                (
+                    ModelClass.__name__,
+                    ModelClass.__name__,
+                    self.__class__.__name__,
+                    exc
+                )
+            )
+            raise TypeError(msg)
 
         # Save many-to-many relationships after the instance is created.
         if many_to_many:
@@ -654,10 +679,10 @@ class ModelSerializer(Serializer):
 
         return instance
 
-    def update(self, instance, validated_attrs):
+    def update(self, instance, validated_data):
         assert not any(
-            isinstance(field, BaseSerializer) and not field.read_only
-            for field in self.fields.values()
+            isinstance(field, BaseSerializer) and (key in validated_attrs)
+            for key, field in self.fields.items()
         ), (
             'The `.update()` method does not suport nested writable fields '
             'by default. Write an explicit `.update()` method for serializer '
@@ -665,7 +690,7 @@ class ModelSerializer(Serializer):
             (self.__class__.__module__, self.__class__.__name__)
         )
 
-        for attr, value in validated_attrs.items():
+        for attr, value in validated_data.items():
             setattr(instance, attr, value)
         instance.save()
         return instance