Save reverse OneToOne relations, possible fix for issue #5996

rest_framework/serializers.py

@@ -929,11 +929,16 @@ def create(self, validated_data):
         # 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.
+        # Collect the reverse fields too so we can save them after the
+        # instance is created
         info = model_meta.get_field_info(ModelClass)
         many_to_many = {}
+        reverse = {}
         for field_name, relation_info in info.relations.items():
             if relation_info.to_many and (field_name in validated_data):
                 many_to_many[field_name] = validated_data.pop(field_name)
+            elif relation_info.reverse and (field_name in validated_data):
+                reverse[field_name] = validated_data[field_name]
 
         try:
             instance = ModelClass._default_manager.create(**validated_data)
@@ -963,6 +968,12 @@ def create(self, validated_data):
                 field = getattr(instance, field_name)
                 field.set(value)
 
+        # Save the reverse relations
+        if reverse:
+            for field_name, value in reverse.items():
+                setattr(instance, field_name, value)
+                value.save()
+
         return instance
 
     def update(self, instance, validated_data):
@@ -979,6 +990,10 @@ def update(self, instance, validated_data):
                 m2m_fields.append((attr, value))
             else:
                 setattr(instance, attr, value)
+                # 'value' is the corresponding instance from the
+                # reverse relation, let's save it
+                if attr in info.relations and info.relations[attr].reverse:
+                    value.save()
 
         instance.save()
 

tests/models.py

@@ -85,7 +85,7 @@ class NullableForeignKeySource(RESTFrameworkModel):
 class NullableUUIDForeignKeySource(RESTFrameworkModel):
     name = models.CharField(max_length=100)
     target = models.ForeignKey(ForeignKeyTarget, null=True, blank=True,
-                               related_name='nullable_sources',
+                               related_name='nullable_uuid_sources',
                                verbose_name='Optional target object',
                                on_delete=models.CASCADE)
 

tests/test_relations_slug.py

@@ -2,7 +2,8 @@
 
 from rest_framework import serializers
 from tests.models import (
-    ForeignKeySource, ForeignKeyTarget, NullableForeignKeySource
+    ForeignKeySource, ForeignKeyTarget, NullableForeignKeySource,
+    NullableOneToOneSource, OneToOneTarget
 )
 
 
@@ -18,6 +19,18 @@ class Meta:
         fields = '__all__'
 
 
+class NullableForeignKeyTargetSerializer(serializers.ModelSerializer):
+    nullable_sources = serializers.SlugRelatedField(
+        slug_field='name',
+        queryset=NullableForeignKeySource.objects.all(),
+        many=True
+    )
+
+    class Meta:
+        model = ForeignKeyTarget
+        fields = '__all__'
+
+
 class ForeignKeySourceSerializer(serializers.ModelSerializer):
     target = serializers.SlugRelatedField(
         slug_field='name',
@@ -41,6 +54,30 @@ class Meta:
         fields = '__all__'
 
 
+class NullableOneToOneSourceSerializer(serializers.ModelSerializer):
+    target = serializers.SlugRelatedField(
+        slug_field='name',
+        queryset=OneToOneTarget.objects.all(),
+        allow_null=True
+    )
+
+    class Meta:
+        model = NullableOneToOneSource
+        fields = '__all__'
+
+
+class OneToOneTargetSerializer(serializers.ModelSerializer):
+    nullable_source = serializers.SlugRelatedField(
+        slug_field='name',
+        queryset=NullableOneToOneSource.objects.all(),
+        allow_null=True
+    )
+
+    class Meta:
+        model = OneToOneTarget
+        fields = '__all__'
+
+
 # TODO: M2M Tests, FKTests (Non-nullable), One2One
 class SlugForeignKeyTests(TestCase):
     def setUp(self):
@@ -285,3 +322,106 @@ def test_foreign_key_update_with_valid_emptystring(self):
             {'id': 3, 'name': 'source-3', 'target': None}
         ]
         assert serializer.data == expected
+
+    def test_foreign_key_reverse_relation_correctly_updated(self):
+        # source 3 has no target yet
+        data = {'id': 1, 'name': 'target-1', 'nullable_sources': ['source-3']}
+        instance = ForeignKeyTarget.objects.get(pk=1)
+
+        serializer = NullableForeignKeyTargetSerializer(instance, data=data)
+        assert serializer.is_valid()
+        serializer.save()
+
+        # the serializer says it updated the sources field
+        assert serializer.data == data
+        assert serializer.data['nullable_sources'] == ['source-3']
+
+        # make sure it was really updated in the database
+        instance.refresh_from_db()
+        assert instance.nullable_sources.exists()
+        assert instance.nullable_sources.first().name == 'source-3'
+
+    def test_foreign_key_reverse_relation_correctly_created(self):
+        # source 3 has no target yet
+        data = {'id': 2, 'name': 'target-2', 'nullable_sources': ['source-3']}
+
+        serializer = NullableForeignKeyTargetSerializer(data=data)
+        assert serializer.is_valid()
+        serializer.save()
+
+        # the serializer says it updated the sources field
+        assert serializer.data == data
+        assert serializer.data['nullable_sources'] == ['source-3']
+
+        # make sure it was really updated in the database
+        instance = ForeignKeyTarget.objects.get(pk=2)
+        instance.refresh_from_db()
+        assert instance.nullable_sources.exists()
+        assert instance.nullable_sources.first().name == 'source-3'
+
+
+class SlugNullableOneToOneTests(TestCase):
+    def setUp(self):
+        target = OneToOneTarget(name='target-1')
+        target.save()
+        source = NullableOneToOneSource(name='source-1', target=None)
+        source.save()
+
+    def test_one_to_one_correctly_updated(self):
+        data = {'id': 1, 'name': 'source-1', 'target': 'target-1'}
+        instance = NullableOneToOneSource.objects.get(pk=1)
+
+        serializer = NullableOneToOneSourceSerializer(instance, data=data)
+        assert serializer.is_valid()
+        serializer.save()
+
+        assert serializer.data == data
+
+        # also check if the data has been saved to the database
+        instance.refresh_from_db()
+        assert instance.target.name == "target-1"
+
+        # see if the reverse relation was updated
+        instance = OneToOneTarget.objects.get(pk=1)
+        serializer = OneToOneTargetSerializer(instance)
+        assert serializer.data['nullable_source'] == 'source-1'
+
+    def test_one_to_one_reverse_correctly_updated(self):
+        data = {'id': 1, 'name': 'target-1', 'nullable_source': 'source-1'}
+        instance = OneToOneTarget.objects.get(pk=1)
+
+        serializer = OneToOneTargetSerializer(instance, data=data)
+        assert serializer.is_valid()
+        serializer.save()
+
+        # the serializer says it updated the nullable_source field
+        assert serializer.data == data
+        assert serializer.data['nullable_source'] == 'source-1'
+
+        # and it says the instance is a NullableOneToOneSource
+        assert isinstance(
+            serializer.instance.nullable_source, NullableOneToOneSource)
+
+        # make sure it was really updated in the database
+        instance.refresh_from_db()
+        assert instance.nullable_source
+
+    def test_one_to_one_reverse_correctly_created(self):
+        data = {'id': 2, 'name': 'target-2', 'nullable_source': 'source-1'}
+
+        serializer = OneToOneTargetSerializer(data=data)
+        assert serializer.is_valid()
+        serializer.save()
+
+        # the serializer says it updated the nullable_source field
+        assert serializer.data == data
+        assert serializer.data['nullable_source'] == 'source-1'
+
+        # and it says the instance is a NullableOneToOneSource
+        assert isinstance(
+            serializer.instance.nullable_source, NullableOneToOneSource)
+
+        # make sure it was really updated in the database
+        instance = OneToOneTarget.objects.get(pk=2)
+        instance.refresh_from_db()
+        assert instance.nullable_source