From 61158151082e4505cc6127a1c82103055f0e361f Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 30 May 2017 13:57:45 -0400 Subject: [PATCH 1/2] Special case for when OneToOneField is also primary key. https://github.com/encode/django-rest-framework/issues/5135 --- rest_framework/serializers.py | 5 +++ tests/models.py | 8 +++++ tests/test_relations_pk.py | 57 ++++++++++++++++++++++++++++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index e27610178..a4b51ae9d 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1159,6 +1159,11 @@ class ModelSerializer(Serializer): field_class = field_mapping[model_field] field_kwargs = get_field_kwargs(field_name, model_field) + # Special case to handle when a OneToOneField is also the primary key + if model_field.one_to_one and model_field.primary_key: + field_class = self.serializer_related_field + field_kwargs['queryset'] = model_field.related_model.objects + if 'choices' in field_kwargs: # Fields with choices get coerced into `ChoiceField` # instead of using their regular typed field. diff --git a/tests/models.py b/tests/models.py index 85143566e..6c9dde8fa 100644 --- a/tests/models.py +++ b/tests/models.py @@ -88,3 +88,11 @@ class NullableOneToOneSource(RESTFrameworkModel): target = models.OneToOneField( OneToOneTarget, null=True, blank=True, related_name='nullable_source', on_delete=models.CASCADE) + + +class OneToOnePKSource(RESTFrameworkModel): + """ Test model where the primary key is a OneToOneField with another model. """ + name = models.CharField(max_length=100) + target = models.OneToOneField( + OneToOneTarget, primary_key=True, + related_name='required_source', on_delete=models.CASCADE) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 8ccf0e117..69572581a 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -6,7 +6,7 @@ from django.utils import six from rest_framework import serializers from tests.models import ( ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, - NullableForeignKeySource, NullableOneToOneSource, + NullableForeignKeySource, NullableOneToOneSource, OneToOnePKSource, NullableUUIDForeignKeySource, OneToOneTarget, UUIDForeignKeyTarget ) @@ -63,6 +63,13 @@ class NullableOneToOneTargetSerializer(serializers.ModelSerializer): fields = ('id', 'name', 'nullable_source') +class OneToOnePKSourceSerializer(serializers.ModelSerializer): + + class Meta: + model = OneToOnePKSource + fields = '__all__' + + # TODO: Add test that .data cannot be accessed prior to .is_valid class PKManyToManyTests(TestCase): @@ -486,3 +493,51 @@ class PKNullableOneToOneTests(TestCase): {'id': 2, 'name': 'target-2', 'nullable_source': 1}, ] assert serializer.data == expected + + +class OneToOnePrimaryKeyTests(TestCase): + + def setUp(self): + # Given: Some target models already exist + self.target = target = OneToOneTarget(name='target-1') + target.save() + self.alt_target = alt_target = OneToOneTarget(name='target-2') + alt_target.save() + + def test_one_to_one_when_primary_key(self): + # When: Creating a Source pointing at the id of the second Target + target_pk = self.alt_target.id + source = OneToOnePKSourceSerializer(data={'name': 'source-2', 'target': target_pk}) + # Then: The source is valid with the serializer + if not source.is_valid(): + self.fail("Expected OneToOnePKTargetSerializer to be valid but had errors: {}".format(source.errors)) + # Then: Saving the serializer creates a new object + new_source = source.save() + # Then: The new object has the same pk as the target object + self.assertEqual(new_source.pk, target_pk) + + def test_one_to_one_when_primary_key_no_duplicates(self): + # When: Creating a Source pointing at the id of the second Target + target_pk = self.target.id + data = {'name': 'source-1', 'target': target_pk} + source = OneToOnePKSourceSerializer(data=data) + # Then: The source is valid with the serializer + self.assertTrue(source.is_valid()) + # Then: Saving the serializer creates a new object + new_source = source.save() + # Then: The new object has the same pk as the target object + self.assertEqual(new_source.pk, target_pk) + # When: Trying to create a second object + second_source = OneToOnePKSourceSerializer(data=data) + self.assertFalse(second_source.is_valid()) + expected = {'target': [u'one to one pk source with this target already exists.']} + self.assertDictEqual(second_source.errors, expected) + + def test_one_to_one_when_primary_key_does_not_exist(self): + # Given: a target PK that does not exist + target_pk = self.target.pk + self.alt_target.pk + source = OneToOnePKSourceSerializer(data={'name': 'source-2', 'target': target_pk}) + # Then: The source is not valid with the serializer + self.assertFalse(source.is_valid()) + self.assertIn("Invalid pk", source.errors['target'][0]) + self.assertIn("object does not exist", source.errors['target'][0]) From 88f9dbceecfaa08f68e5e60dc575e10a666de909 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 30 May 2017 14:13:29 -0400 Subject: [PATCH 2/2] Silly linting change import ordering matters --- tests/test_relations_pk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 69572581a..2eebe1b5c 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -6,8 +6,8 @@ from django.utils import six from rest_framework import serializers from tests.models import ( ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, - NullableForeignKeySource, NullableOneToOneSource, OneToOnePKSource, - NullableUUIDForeignKeySource, OneToOneTarget, UUIDForeignKeyTarget + NullableForeignKeySource, NullableOneToOneSource, NullableUUIDForeignKeySource, + OneToOnePKSource, OneToOneTarget, UUIDForeignKeyTarget )