Merge pull request #2215 from tomchristie/tomchristie-better-serializer-errors

Better serializer errors for nested writes.

1 files changed, 61 insertions, 21 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index e1851ddd..c022cad3 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -561,6 +561,64 @@ class ListSerializer(BaseSerializer):
 # ModelSerializer & HyperlinkedModelSerializer
 # --------------------------------------------
 
+def raise_errors_on_nested_writes(method_name, serializer):
+    """
+    Give explicit errors when users attempt to pass writable nested data.
+
+    If we don't do this explicitly they'd get a less helpful error when
+    calling `.save()` on the serializer.
+
+    We don't *automatically* support these sorts of nested writes brecause
+    there are too many ambiguities to define a default behavior.
+
+    Eg. Suppose we have a `UserSerializer` with a nested profile. How should
+    we handle the case of an update, where the `profile` realtionship does
+    not exist? Any of the following might be valid:
+
+    * Raise an application error.
+    * Silently ignore the nested part of the update.
+    * Automatically create a profile instance.
+    """
+
+    # Ensure we don't have a writable nested field. For example:
+    #
+    # class UserSerializer(ModelSerializer):
+    #     ...
+    #     profile = ProfileSerializer()
+    assert not any(
+        isinstance(field, BaseSerializer) and (key in validated_attrs)
+        for key, field in serializer.fields.items()
+    ), (
+        'The `.{method_name}()` method does not support writable nested'
+        'fields by default.\nWrite an explicit `.{method_name}()` method for '
+        'serializer `{module}.{class_name}`, or set `read_only=True` on '
+        'nested serializer fields.'.format(
+            method_name=method_name,
+            module=serializer.__class__.__module__,
+            class_name=serializer.__class__.__name__
+        )
+    )
+
+    # Ensure we don't have a writable dotted-source field. For example:
+    #
+    # class UserSerializer(ModelSerializer):
+    #     ...
+    #     address = serializer.CharField('profile.address')
+    assert not any(
+        '.' in field.source and (key in validated_attrs)
+        for key, field in serializer.fields.items()
+    ), (
+        'The `.{method_name}()` method does not support writable dotted-source '
+        'fields by default.\nWrite an explicit `.{method_name}()` method for '
+        'serializer `{module}.{class_name}`, or set `read_only=True` on '
+        'dotted-source serializer fields.'.format(
+            method_name=method_name,
+            module=serializer.__class__.__module__,
+            class_name=serializer.__class__.__name__
+        )
+    )
+
+
 class ModelSerializer(Serializer):
     """
     A `ModelSerializer` is just a regular `Serializer`, except that:
@@ -624,18 +682,7 @@ class ModelSerializer(Serializer):
         If you want to support writable nested relationships you'll need
         to write an explicit `.create()` method.
         """
-        # Check that the user isn't trying to handle a writable nested field.
-        # 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 (key in validated_attrs)
-            for key, field in self.fields.items()
-        ), (
-            'The `.create()` method does not support nested writable fields '
-            'by default. Write an explicit `.create()` method for serializer '
-            '`%s.%s`, or set `read_only=True` on nested serializer fields.' %
-            (self.__class__.__module__, self.__class__.__name__)
-        )
+        raise_errors_on_nested_writes('create', self)
 
         ModelClass = self.Meta.model
 
@@ -675,19 +722,12 @@ class ModelSerializer(Serializer):
         return instance
 
     def update(self, instance, validated_data):
-        assert not any(
-            isinstance(field, BaseSerializer) and (key in validated_attrs)
-            for key, field in self.fields.items()
-        ), (
-            'The `.update()` method does not support nested writable fields '
-            'by default. Write an explicit `.update()` method for serializer '
-            '`%s.%s`, or set `read_only=True` on nested serializer fields.' %
-            (self.__class__.__module__, self.__class__.__name__)
-        )
+        raise_errors_on_nested_writes('update', self)
 
         for attr, value in validated_data.items():
             setattr(instance, attr, value)
         instance.save()
+
         return instance
 
     def get_validators(self):