Merge pull request #749 from tomchristie/clean-basic-nested-serialization

Clean basic nested serialization

2 files changed, 351 insertions, 39 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index 0b0c2c9c..6aca2f57 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -20,6 +20,25 @@ from rest_framework.relations import *
 from rest_framework.fields import *
 
 
+class NestedValidationError(ValidationError):
+    """
+    The default ValidationError behavior is to stringify each item in the list
+    if the messages are a list of error messages.
+
+    In the case of nested serializers, where the parent has many children,
+    then the child's `serializer.errors` will be a list of dicts.  In the case
+    of a single child, the `serializer.errors` will be a dict.
+
+    We need to override the default behavior to get properly nested error dicts.
+    """
+
+    def __init__(self, message):
+        if isinstance(message, dict):
+            self.messages = [message]
+        else:
+            self.messages = message
+
+
 class DictWithMetadata(dict):
     """
     A dict-like object, that can have additional properties attached.
@@ -98,7 +117,7 @@ class SerializerOptions(object):
         self.exclude = getattr(meta, 'exclude', ())
 
 
-class BaseSerializer(Field):
+class BaseSerializer(WritableField):
     """
     This is the Serializer implementation.
     We need to implement it as `BaseSerializer` due to metaclass magicks.
@@ -110,9 +129,9 @@ class BaseSerializer(Field):
     _dict_class = SortedDictWithMetadata
 
     def __init__(self, instance=None, data=None, files=None,
-                 context=None, partial=False, many=None, source=None,
-                 allow_delete=False):
-        super(BaseSerializer, self).__init__(source=source)
+                 context=None, partial=False, many=None,
+                 allow_delete=False, **kwargs):
+        super(BaseSerializer, self).__init__(**kwargs)
         self.opts = self._options_class(self.Meta)
         self.parent = None
         self.root = None
@@ -305,40 +324,77 @@ class BaseSerializer(Field):
 
     def field_to_native(self, obj, field_name):
         """
-        Override default so that we can apply ModelSerializer as a nested
-        field to relationships.
+        Override default so that the serializer can be used as a nested field
+        across relationships.
         """
         if self.source == '*':
             return self.to_native(obj)
 
         try:
-            if self.source:
-                for component in self.source.split('.'):
-                    obj = getattr(obj, component)
-                    if is_simple_callable(obj):
-                        obj = obj()
-            else:
-                obj = getattr(obj, field_name)
-                if is_simple_callable(obj):
-                    obj = obj()
+            source = self.source or field_name
+            value = obj
+
+            for component in source.split('.'):
+                value = get_component(value, component)
+                if value is None:
+                    break
         except ObjectDoesNotExist:
             return None
 
-        # If the object has an "all" method, assume it's a relationship
-        if is_simple_callable(getattr(obj, 'all', None)):
-            return [self.to_native(item) for item in obj.all()]
+        if is_simple_callable(getattr(value, 'all', None)):
+            return [self.to_native(item) for item in value.all()]
 
-        if obj is None:
+        if value is None:
             return None
 
         if self.many is not None:
             many = self.many
         else:
-            many = hasattr(obj, '__iter__') and not isinstance(obj, (Page, dict, six.text_type))
+            many = hasattr(value, '__iter__') and not isinstance(value, (Page, dict, six.text_type))
 
         if many:
-            return [self.to_native(item) for item in obj]
-        return self.to_native(obj)
+            return [self.to_native(item) for item in value]
+        return self.to_native(value)
+
+    def field_from_native(self, data, files, field_name, into):
+        """
+        Override default so that the serializer can be used as a writable
+        nested field across relationships.
+        """
+        if self.read_only:
+            return
+
+        try:
+            value = data[field_name]
+        except KeyError:
+            if self.default is not None and not self.partial:
+                # Note: partial updates shouldn't set defaults
+                value = copy.deepcopy(self.default)
+            else:
+                if self.required:
+                    raise ValidationError(self.error_messages['required'])
+                return
+
+        # Set the serializer object if it exists
+        obj = getattr(self.parent.object, field_name) if self.parent.object else None
+
+        if value in (None, ''):
+            into[(self.source or field_name)] = None
+        else:
+            kwargs = {
+                'instance': obj,
+                'data': value,
+                'context': self.context,
+                'partial': self.partial,
+                'many': self.many
+            }
+            serializer = self.__class__(**kwargs)
+
+            if serializer.is_valid():
+                into[self.source or field_name] = serializer.object
+            else:
+                # Propagate errors up to our parent
+                raise NestedValidationError(serializer.errors)
 
     def get_identity(self, data):
         """
@@ -637,33 +693,43 @@ class ModelSerializer(Serializer):
         """
         Restore the model instance.
         """
-        self.m2m_data = {}
-        self.related_data = {}
+        m2m_data = {}
+        related_data = {}
+        meta = self.opts.model._meta
 
-        # Reverse fk relations
-        for (obj, model) in self.opts.model._meta.get_all_related_objects_with_model():
+        # Reverse fk or one-to-one relations
+        for (obj, model) in meta.get_all_related_objects_with_model():
             field_name = obj.field.related_query_name()
             if field_name in attrs:
-                self.related_data[field_name] = attrs.pop(field_name)
+                related_data[field_name] = attrs.pop(field_name)
 
         # Reverse m2m relations
-        for (obj, model) in self.opts.model._meta.get_all_related_m2m_objects_with_model():
+        for (obj, model) in meta.get_all_related_m2m_objects_with_model():
             field_name = obj.field.related_query_name()
             if field_name in attrs:
-                self.m2m_data[field_name] = attrs.pop(field_name)
+                m2m_data[field_name] = attrs.pop(field_name)
 
         # Forward m2m relations
-        for field in self.opts.model._meta.many_to_many:
+        for field in meta.many_to_many:
             if field.name in attrs:
-                self.m2m_data[field.name] = attrs.pop(field.name)
+                m2m_data[field.name] = attrs.pop(field.name)
 
+        # Update an existing instance...
         if instance is not None:
             for key, val in attrs.items():
                 setattr(instance, key, val)
 
+        # ...or create a new instance
         else:
             instance = self.opts.model(**attrs)
 
+        # Any relations that cannot be set until we've
+        # saved the model get hidden away on these
+        # private attributes, so we can deal with them
+        # at the point of save.
+        instance._related_data = related_data
+        instance._m2m_data = m2m_data
+
         return instance
 
     def from_native(self, data, files):
@@ -680,15 +746,15 @@ class ModelSerializer(Serializer):
         """
         obj.save(**kwargs)
 
-        if getattr(self, 'm2m_data', None):
-            for accessor_name, object_list in self.m2m_data.items():
-                setattr(self.object, accessor_name, object_list)
-            self.m2m_data = {}
+        if getattr(obj, '_m2m_data', None):
+            for accessor_name, object_list in obj._m2m_data.items():
+                setattr(obj, accessor_name, object_list)
+            del(obj._m2m_data)
 
-        if getattr(self, 'related_data', None):
-            for accessor_name, object_list in self.related_data.items():
-                setattr(self.object, accessor_name, object_list)
-            self.related_data = {}
+        if getattr(obj, '_related_data', None):
+            for accessor_name, related in obj._related_data.items():
+                setattr(obj, accessor_name, related)
+            del(obj._related_data)
 
 
 class HyperlinkedModelSerializerOptions(ModelSerializerOptions):
diff --git a/rest_framework/tests/serializer_nested.py b/rest_framework/tests/serializer_nested.py
new file mode 100644
index 00000000..6a29c652
--- /dev/null
+++ b/rest_framework/tests/serializer_nested.py
@@ -0,0 +1,246 @@
+"""
+Tests to cover nested serializers.
+
+Doesn't cover model serializers.
+"""
+from __future__ import unicode_literals
+from django.test import TestCase
+from rest_framework import serializers
+
+
+class WritableNestedSerializerBasicTests(TestCase):
+    """
+    Tests for deserializing nested entities.
+    Basic tests that use serializers that simply restore to dicts.
+    """
+
+    def setUp(self):
+        class TrackSerializer(serializers.Serializer):
+            order = serializers.IntegerField()
+            title = serializers.CharField(max_length=100)
+            duration = serializers.IntegerField()
+
+        class AlbumSerializer(serializers.Serializer):
+            album_name = serializers.CharField(max_length=100)
+            artist = serializers.CharField(max_length=100)
+            tracks = TrackSerializer(many=True)
+
+        self.AlbumSerializer = AlbumSerializer
+
+    def test_nested_validation_success(self):
+        """
+        Correct nested serialization should return the input data.
+        """
+
+        data = {
+            'album_name': 'Discovery',
+            'artist': 'Daft Punk',
+            'tracks': [
+                {'order': 1, 'title': 'One More Time', 'duration': 235},
+                {'order': 2, 'title': 'Aerodynamic', 'duration': 184},
+                {'order': 3, 'title': 'Digital Love', 'duration': 239}
+            ]
+        }
+
+        serializer = self.AlbumSerializer(data=data)
+        self.assertEqual(serializer.is_valid(), True)
+        self.assertEqual(serializer.object, data)
+
+    def test_nested_validation_error(self):
+        """
+        Incorrect nested serialization should return appropriate error data.
+        """
+
+        data = {
+            'album_name': 'Discovery',
+            'artist': 'Daft Punk',
+            'tracks': [
+                {'order': 1, 'title': 'One More Time', 'duration': 235},
+                {'order': 2, 'title': 'Aerodynamic', 'duration': 184},
+                {'order': 3, 'title': 'Digital Love', 'duration': 'foobar'}
+            ]
+        }
+        expected_errors = {
+            'tracks': [
+                {},
+                {},
+                {'duration': ['Enter a whole number.']}
+            ]
+        }
+
+        serializer = self.AlbumSerializer(data=data)
+        self.assertEqual(serializer.is_valid(), False)
+        self.assertEqual(serializer.errors, expected_errors)
+
+    def test_many_nested_validation_error(self):
+        """
+        Incorrect nested serialization should return appropriate error data
+        when multiple entities are being deserialized.
+        """
+
+        data = [
+            {
+                'album_name': 'Russian Red',
+                'artist': 'I Love Your Glasses',
+                'tracks': [
+                    {'order': 1, 'title': 'Cigarettes', 'duration': 121},
+                    {'order': 2, 'title': 'No Past Land', 'duration': 198},
+                    {'order': 3, 'title': 'They Don\'t Believe', 'duration': 191}
+                ]
+            },
+            {
+                'album_name': 'Discovery',
+                'artist': 'Daft Punk',
+                'tracks': [
+                    {'order': 1, 'title': 'One More Time', 'duration': 235},
+                    {'order': 2, 'title': 'Aerodynamic', 'duration': 184},
+                    {'order': 3, 'title': 'Digital Love', 'duration': 'foobar'}
+                ]
+            }
+        ]
+        expected_errors = [
+            {},
+            {
+                'tracks': [
+                    {},
+                    {},
+                    {'duration': ['Enter a whole number.']}
+                ]
+            }
+        ]
+
+        serializer = self.AlbumSerializer(data=data)
+        self.assertEqual(serializer.is_valid(), False)
+        self.assertEqual(serializer.errors, expected_errors)
+
+
+class WritableNestedSerializerObjectTests(TestCase):
+    """
+    Tests for deserializing nested entities.
+    These tests use serializers that restore to concrete objects.
+    """
+
+    def setUp(self):
+        # Couple of concrete objects that we're going to deserialize into
+        class Track(object):
+            def __init__(self, order, title, duration):
+                self.order, self.title, self.duration = order, title, duration
+
+            def __eq__(self, other):
+                return (
+                    self.order == other.order and
+                    self.title == other.title and
+                    self.duration == other.duration
+                )
+
+        class Album(object):
+            def __init__(self, album_name, artist, tracks):
+                self.album_name, self.artist, self.tracks = album_name, artist, tracks
+
+            def __eq__(self, other):
+                return (
+                    self.album_name == other.album_name and
+                    self.artist == other.artist and
+                    self.tracks == other.tracks
+                )
+
+        # And their corresponding serializers
+        class TrackSerializer(serializers.Serializer):
+            order = serializers.IntegerField()
+            title = serializers.CharField(max_length=100)
+            duration = serializers.IntegerField()
+
+            def restore_object(self, attrs, instance=None):
+                return Track(attrs['order'], attrs['title'], attrs['duration'])
+
+        class AlbumSerializer(serializers.Serializer):
+            album_name = serializers.CharField(max_length=100)
+            artist = serializers.CharField(max_length=100)
+            tracks = TrackSerializer(many=True)
+
+            def restore_object(self, attrs, instance=None):
+                return Album(attrs['album_name'], attrs['artist'], attrs['tracks'])
+
+        self.Album, self.Track = Album, Track
+        self.AlbumSerializer = AlbumSerializer
+
+    def test_nested_validation_success(self):
+        """
+        Correct nested serialization should return a restored object
+        that corresponds to the input data.
+        """
+
+        data = {
+            'album_name': 'Discovery',
+            'artist': 'Daft Punk',
+            'tracks': [
+                {'order': 1, 'title': 'One More Time', 'duration': 235},
+                {'order': 2, 'title': 'Aerodynamic', 'duration': 184},
+                {'order': 3, 'title': 'Digital Love', 'duration': 239}
+            ]
+        }
+        expected_object = self.Album(
+            album_name='Discovery',
+            artist='Daft Punk',
+            tracks=[
+                self.Track(order=1, title='One More Time', duration=235),
+                self.Track(order=2, title='Aerodynamic', duration=184),
+                self.Track(order=3, title='Digital Love', duration=239),
+            ]
+        )
+
+        serializer = self.AlbumSerializer(data=data)
+        self.assertEqual(serializer.is_valid(), True)
+        self.assertEqual(serializer.object, expected_object)
+
+    def test_many_nested_validation_success(self):
+        """
+        Correct nested serialization should return multiple restored objects
+        that corresponds to the input data when multiple objects are
+        being deserialized.
+        """
+
+        data = [
+            {
+                'album_name': 'Russian Red',
+                'artist': 'I Love Your Glasses',
+                'tracks': [
+                    {'order': 1, 'title': 'Cigarettes', 'duration': 121},
+                    {'order': 2, 'title': 'No Past Land', 'duration': 198},
+                    {'order': 3, 'title': 'They Don\'t Believe', 'duration': 191}
+                ]
+            },
+            {
+                'album_name': 'Discovery',
+                'artist': 'Daft Punk',
+                'tracks': [
+                    {'order': 1, 'title': 'One More Time', 'duration': 235},
+                    {'order': 2, 'title': 'Aerodynamic', 'duration': 184},
+                    {'order': 3, 'title': 'Digital Love', 'duration': 239}
+                ]
+            }
+        ]
+        expected_object = [
+            self.Album(
+                album_name='Russian Red',
+                artist='I Love Your Glasses',
+                tracks=[
+                    self.Track(order=1, title='Cigarettes', duration=121),
+                    self.Track(order=2, title='No Past Land', duration=198),
+                    self.Track(order=3, title='They Don\'t Believe', duration=191),
+                ]
+            ),
+            self.Album(
+                album_name='Discovery',
+                artist='Daft Punk',
+                tracks=[
+                    self.Track(order=1, title='One More Time', duration=235),
+                    self.Track(order=2, title='Aerodynamic', duration=184),
+                    self.Track(order=3, title='Digital Love', duration=239),
+                ]
+            )
+        ]
+
+        serializer = self.AlbumSerializer(data=data)
+        self.assertEqual(serializer.is_valid(), True)
+        self.assertEqual(serializer.object, expected_object)