Don't persist relation changes in ModelSerializer#restore_object()

3 files changed, 35 insertions, 25 deletions

diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py
index ed173d85..24674f2a 100644
--- a/rest_framework/serializers.py
+++ b/rest_framework/serializers.py
@@ -498,28 +498,28 @@ class ModelSerializer(Serializer):
         self.m2m_data = {}
         self.related_data = {}
 
+        # Reverse fk relations
+        for (obj, model) in self.opts.model._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)
+
+        # Reverse m2m relations
+        for (obj, model) in self.opts.model._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)
+
+        # Forward m2m relations
+        for field in self.opts.model._meta.many_to_many:
+            if field.name in attrs:
+                self.m2m_data[field.name] = attrs.pop(field.name)
+
         if instance is not None:
             for key, val in attrs.items():
                 setattr(instance, key, val)
 
         else:
-            # Reverse fk relations
-            for (obj, model) in self.opts.model._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)
-
-            # Reverse m2m relations
-            for (obj, model) in self.opts.model._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)
-
-            # Forward m2m relations
-            for field in self.opts.model._meta.many_to_many:
-                if field.name in attrs:
-                    self.m2m_data[field.name] = attrs.pop(field.name)
-
             instance = self.opts.model(**attrs)
 
         try:
diff --git a/rest_framework/tests/relations_hyperlink.py b/rest_framework/tests/relations_hyperlink.py
index 24039410..4179ec68 100644
--- a/rest_framework/tests/relations_hyperlink.py
+++ b/rest_framework/tests/relations_hyperlink.py
@@ -114,8 +114,8 @@ class HyperlinkedManyToManyTests(TestCase):
         instance = ManyToManySource.objects.get(pk=1)
         serializer = ManyToManySourceSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure source 1 is updated, and everything else is as expected
         queryset = ManyToManySource.objects.all()
@@ -132,8 +132,8 @@ class HyperlinkedManyToManyTests(TestCase):
         instance = ManyToManyTarget.objects.get(pk=1)
         serializer = ManyToManyTargetSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure target 1 is updated, and everything else is as expected
         queryset = ManyToManyTarget.objects.all()
@@ -239,8 +239,8 @@ class HyperlinkedForeignKeyTests(TestCase):
         instance = ForeignKeyTarget.objects.get(pk=2)
         serializer = ForeignKeyTargetSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure target 2 is update, and everything else is as expected
         queryset = ForeignKeyTarget.objects.all()
diff --git a/rest_framework/tests/relations_pk.py b/rest_framework/tests/relations_pk.py
index 01109ef9..28967099 100644
--- a/rest_framework/tests/relations_pk.py
+++ b/rest_framework/tests/relations_pk.py
@@ -99,8 +99,8 @@ class PKManyToManyTests(TestCase):
         instance = ManyToManySource.objects.get(pk=1)
         serializer = ManyToManySourceSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure source 1 is updated, and everything else is as expected
         queryset = ManyToManySource.objects.all()
@@ -117,8 +117,8 @@ class PKManyToManyTests(TestCase):
         instance = ManyToManyTarget.objects.get(pk=1)
         serializer = ManyToManyTargetSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure target 1 is updated, and everything else is as expected
         queryset = ManyToManyTarget.objects.all()
@@ -221,8 +221,18 @@ class PKForeignKeyTests(TestCase):
         instance = ForeignKeyTarget.objects.get(pk=2)
         serializer = ForeignKeyTargetSerializer(instance, data=data)
         self.assertTrue(serializer.is_valid())
-        self.assertEquals(serializer.data, data)
+        # We shouldn't have saved anything to the db yet since save
+        # hasn't been called.
+        queryset = ForeignKeyTarget.objects.all()
+        new_serializer = ForeignKeyTargetSerializer(queryset)
+        expected = [
+            {'id': 1, 'name': u'target-1', 'sources': [1, 2, 3]},
+            {'id': 2, 'name': u'target-2', 'sources': []},
+        ]        
+        self.assertEquals(new_serializer.data, expected)
+
         serializer.save()
+        self.assertEquals(serializer.data, data)
 
         # Ensure target 2 is update, and everything else is as expected
         queryset = ForeignKeyTarget.objects.all()
@@ -241,7 +251,7 @@ class PKForeignKeyTests(TestCase):
         self.assertEquals(serializer.data, data)
         self.assertEqual(obj.name, u'source-4')
 
-        # Ensure source 1 is updated, and everything else is as expected
+        # Ensure source 4 is added, and everything else is as expected
         queryset = ForeignKeySource.objects.all()
         serializer = ForeignKeySourceSerializer(queryset)
         expected = [
@@ -260,7 +270,7 @@ class PKForeignKeyTests(TestCase):
         self.assertEquals(serializer.data, data)
         self.assertEqual(obj.name, u'target-3')
 
-        # Ensure target 4 is added, and everything else is as expected
+        # Ensure target 3 is added, and everything else is as expected
         queryset = ForeignKeyTarget.objects.all()
         serializer = ForeignKeyTargetSerializer(queryset)
         expected = [