From 20d1fdba697a590307ef65626bfb574a0c92bc96 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 16 Feb 2016 09:29:48 +0100 Subject: [PATCH 1/3] Fix None UUID ForeignKey serialization --- docs/topics/release-notes.md | 1 + rest_framework/fields.py | 2 ++ tests/models.py | 15 +++++++++++++++ tests/test_relations_pk.py | 21 ++++++++++++++++++++- 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index b6794a817..9c8d9e978 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -45,6 +45,7 @@ You can determine your currently installed version using `pip freeze`: **Unreleased** * Dropped support for EOL Django 1.7 ([#3933][gh3933]) +* Fixed null foreign keys targeting UUIDField primary keys. ([#3936][gh3936]) ### 3.3.2 diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 6d5962c8e..c700b85e8 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -778,6 +778,8 @@ class UUIDField(Field): return data def to_representation(self, value): + if value is None: + return None if self.uuid_format == 'hex_verbose': return str(value) else: diff --git a/tests/models.py b/tests/models.py index 8ec274d8b..5d5d40968 100644 --- a/tests/models.py +++ b/tests/models.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals +import uuid + from django.db import models from django.utils.translation import ugettext_lazy as _ @@ -46,6 +48,11 @@ class ForeignKeyTarget(RESTFrameworkModel): name = models.CharField(max_length=100) +class UUIDForeignKeyTarget(RESTFrameworkModel): + uuid = models.UUIDField(primary_key=True, default=uuid.uuid4) + name = models.CharField(max_length=100) + + class ForeignKeySource(RESTFrameworkModel): name = models.CharField(max_length=100) target = models.ForeignKey(ForeignKeyTarget, related_name='sources', @@ -62,6 +69,14 @@ class NullableForeignKeySource(RESTFrameworkModel): on_delete=models.CASCADE) +class NullableUUIDForeignKeySource(RESTFrameworkModel): + name = models.CharField(max_length=100) + target = models.ForeignKey(ForeignKeyTarget, null=True, blank=True, + related_name='nullable_sources', + verbose_name='Optional target object', + on_delete=models.CASCADE) + + # OneToOne class OneToOneTarget(RESTFrameworkModel): name = models.CharField(max_length=100) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 169f7d9c5..658357b2f 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -6,7 +6,8 @@ from django.utils import six from rest_framework import serializers from tests.models import ( ForeignKeySource, ForeignKeyTarget, ManyToManySource, ManyToManyTarget, - NullableForeignKeySource, NullableOneToOneSource, OneToOneTarget + NullableForeignKeySource, NullableOneToOneSource, + NullableUUIDForeignKeySource, OneToOneTarget, UUIDForeignKeyTarget ) @@ -43,6 +44,18 @@ class NullableForeignKeySourceSerializer(serializers.ModelSerializer): fields = ('id', 'name', 'target') +# Nullable UUIDForeignKey +class NullableUUIDForeignKeySourceSerializer(serializers.ModelSerializer): + target = serializers.PrimaryKeyRelatedField( + pk_field=serializers.UUIDField(), + queryset=UUIDForeignKeyTarget.objects.all(), + allow_empty=True) + + class Meta: + model = NullableUUIDForeignKeySource + fields = ('id', 'name', 'target') + + # Nullable OneToOne class NullableOneToOneTargetSerializer(serializers.ModelSerializer): class Meta: @@ -432,6 +445,12 @@ class PKNullableForeignKeyTests(TestCase): ] self.assertEqual(serializer.data, expected) + def test_null_uuid_foreign_key_serializes_as_none(self): + source = NullableUUIDForeignKeySource(name='Source') + serializer = NullableUUIDForeignKeySourceSerializer(source) + data = serializer.data + self.assertEqual(data["target"], None) + class PKNullableOneToOneTests(TestCase): def setUp(self): From cbb8d8d2546f1673dcd26c4a7f0836a4c10312f3 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 10 Feb 2016 10:07:11 +0100 Subject: [PATCH 2/3] Test deserialising data including `None` fk --- tests/test_relations_pk.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_relations_pk.py b/tests/test_relations_pk.py index 658357b2f..ba75bd94f 100644 --- a/tests/test_relations_pk.py +++ b/tests/test_relations_pk.py @@ -49,7 +49,7 @@ class NullableUUIDForeignKeySourceSerializer(serializers.ModelSerializer): target = serializers.PrimaryKeyRelatedField( pk_field=serializers.UUIDField(), queryset=UUIDForeignKeyTarget.objects.all(), - allow_empty=True) + allow_null=True) class Meta: model = NullableUUIDForeignKeySource @@ -451,6 +451,11 @@ class PKNullableForeignKeyTests(TestCase): data = serializer.data self.assertEqual(data["target"], None) + def test_nullable_uuid_foreign_key_is_valid_when_none(self): + data = {"name": "Source", "target": None} + serializer = NullableUUIDForeignKeySourceSerializer(data=data) + self.assertTrue(serializer.is_valid(), serializer.errors) + class PKNullableOneToOneTests(TestCase): def setUp(self): From 2ef74cfa61d6f52a2a5c32151a052488d5d7e9e2 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Sun, 13 Mar 2016 20:39:19 +0100 Subject: [PATCH 3/3] Bring check for null fk to `BaseSerializer.to_representation` --- rest_framework/fields.py | 2 -- rest_framework/serializers.py | 10 +++++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index c700b85e8..6d5962c8e 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -778,8 +778,6 @@ class UUIDField(Field): return data def to_representation(self, value): - if value is None: - return None if self.uuid_format == 'hex_verbose': return str(value) else: diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index b95bb7fa6..625d32644 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -464,9 +464,13 @@ class Serializer(BaseSerializer): except SkipField: continue - if attribute is None: - # We skip `to_representation` for `None` values so that - # fields do not have to explicitly deal with that case. + # We skip `to_representation` for `None` values so that fields do + # not have to explicitly deal with that case. + # + # For related fields with `use_pk_only_optimization` we need to + # resolve the pk value. + check_for_none = attribute.pk if isinstance(attribute, PKOnlyObject) else attribute + if check_for_none is None: ret[field.field_name] = None else: ret[field.field_name] = field.to_representation(attribute)