From 335a27f0fd7ef4a3e82be17b106c6358a361e57c Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Wed, 11 Sep 2024 22:10:17 +0100 Subject: [PATCH 1/5] Add test to reproduce problem with nullable fields part of a unique constraint Ref #9378 --- tests/test_model_serializer.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index ae1a2b0fa1..312c6952ac 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -1400,3 +1400,32 @@ class Meta: serializer.save() self.assertEqual(instance.char_field, 'value changed by signal') + + +class Tag(models.Model): + name = models.CharField(max_length=100) + + +class UniqueConstraintModel(models.Model): + title = models.CharField(max_length=100) + age = models.IntegerField(null=True) + tag = models.ForeignKey(Tag, on_delete=models.CASCADE, related_name='instances', null=True) + + class Meta: + constraints = [ + # Unique constraint on 2 nullable fields + models.UniqueConstraint(name='unique_constraint', fields=('age', 'tag')) + ] + + +class TestUniqueConstraintWithNullableFields(TestCase): + def test_nullable_unique_constraint_fields_are_not_required(self): + class UniqueConstraintSerializer(serializers.ModelSerializer): + class Meta: + model = UniqueConstraintModel + fields = '__all__' + + serializer = UniqueConstraintSerializer(data={'title': 'Bob'}) + self.assertTrue(serializer.is_valid(), serializer.errors) + result = serializer.save() + self.assertIsInstance(result, UniqueConstraintModel) From 81c74d903df9571b0a966f238db05ea1ba1e56d7 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Wed, 11 Sep 2024 22:32:47 +0100 Subject: [PATCH 2/5] Simplify test case and add similar case for unique_together --- tests/test_model_serializer.py | 38 ++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 312c6952ac..2817205179 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -542,6 +542,15 @@ class Meta: unique_together = ("foreign_key", "one_to_one") +class NullableUniqueTogetherModel(models.Model): + name = models.CharField(max_length=100) + field_1 = models.IntegerField(null=True) + field_2 = models.TextField(null=True) + + class Meta: + unique_together = ("field_1", "field_2") + + class TestRelationalFieldMappings(TestCase): def test_pk_relations(self): class TestSerializer(serializers.ModelSerializer): @@ -731,6 +740,17 @@ class Meta: """) self.assertEqual(repr(TestSerializer()), expected) + def test_nullable_unique_together(self): + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = NullableUniqueTogetherModel + fields = ('name', 'field_1', 'field_2') + + serializer = TestSerializer(data={"name": "Test"}) + self.assertTrue(serializer.is_valid(), serializer.errors) + instance = serializer.save() + assert isinstance(instance, NullableUniqueTogetherModel) + class DisplayValueTargetModel(models.Model): name = models.CharField(max_length=100) @@ -1402,14 +1422,10 @@ class Meta: self.assertEqual(instance.char_field, 'value changed by signal') -class Tag(models.Model): - name = models.CharField(max_length=100) - - -class UniqueConstraintModel(models.Model): +class UniqueConstraintNullableModel(models.Model): title = models.CharField(max_length=100) age = models.IntegerField(null=True) - tag = models.ForeignKey(Tag, on_delete=models.CASCADE, related_name='instances', null=True) + tag = models.CharField(max_length=100, null=True) class Meta: constraints = [ @@ -1420,12 +1436,12 @@ class Meta: class TestUniqueConstraintWithNullableFields(TestCase): def test_nullable_unique_constraint_fields_are_not_required(self): - class UniqueConstraintSerializer(serializers.ModelSerializer): + class UniqueConstraintNullableSerializer(serializers.ModelSerializer): class Meta: - model = UniqueConstraintModel - fields = '__all__' + model = UniqueConstraintNullableModel + fields = ('title', 'age', 'tag') - serializer = UniqueConstraintSerializer(data={'title': 'Bob'}) + serializer = UniqueConstraintNullableSerializer(data={'title': 'Bob'}) self.assertTrue(serializer.is_valid(), serializer.errors) result = serializer.save() - self.assertIsInstance(result, UniqueConstraintModel) + self.assertIsInstance(result, UniqueConstraintNullableModel) From 2bf6c7df6d972ee433a1b33a321fdfee64d19ca0 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Wed, 11 Sep 2024 22:50:55 +0100 Subject: [PATCH 3/5] Add test for unique together in a better place --- tests/test_validators.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_validators.py b/tests/test_validators.py index 4bb8658d5b..f09ecc09e0 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -441,6 +441,14 @@ def test_ignore_validation_for_null_fields(self): serializer = NullUniquenessTogetherSerializer(data=data) assert serializer.is_valid() + def test_ignore_validation_for_missing_nullable_fields(self): + data = { + 'date': datetime.date(2000, 1, 1), + 'race_name': 'Paris Marathon', + } + serializer = NullUniquenessTogetherSerializer(data=data) + assert serializer.is_valid(), serializer.errors + def test_do_not_ignore_validation_for_null_fields(self): # None values that are not on fields part of the uniqueness constraint # do not cause the instance to skip validation. From 25e01d0761abbb78a8b0983e6a1d224767f5a000 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Mon, 14 Oct 2024 19:06:37 +0100 Subject: [PATCH 4/5] Default nullable fields to null in unique constraints checks --- rest_framework/serializers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index b1b7b64774..f37bd3a3d6 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -1490,6 +1490,8 @@ def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs default = timezone.now elif unique_constraint_field.has_default(): default = unique_constraint_field.default + elif unique_constraint_field.null: + default = None else: default = empty From d2da2f5e06c3d551f62526fe9651cb758468a8eb Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Mon, 14 Oct 2024 19:24:56 +0100 Subject: [PATCH 5/5] Remove redundant test and move other to more appropriate place --- tests/test_model_serializer.py | 45 ---------------------------------- tests/test_validators.py | 24 ++++++++++++++++++ 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 2817205179..ae1a2b0fa1 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -542,15 +542,6 @@ class Meta: unique_together = ("foreign_key", "one_to_one") -class NullableUniqueTogetherModel(models.Model): - name = models.CharField(max_length=100) - field_1 = models.IntegerField(null=True) - field_2 = models.TextField(null=True) - - class Meta: - unique_together = ("field_1", "field_2") - - class TestRelationalFieldMappings(TestCase): def test_pk_relations(self): class TestSerializer(serializers.ModelSerializer): @@ -740,17 +731,6 @@ class Meta: """) self.assertEqual(repr(TestSerializer()), expected) - def test_nullable_unique_together(self): - class TestSerializer(serializers.ModelSerializer): - class Meta: - model = NullableUniqueTogetherModel - fields = ('name', 'field_1', 'field_2') - - serializer = TestSerializer(data={"name": "Test"}) - self.assertTrue(serializer.is_valid(), serializer.errors) - instance = serializer.save() - assert isinstance(instance, NullableUniqueTogetherModel) - class DisplayValueTargetModel(models.Model): name = models.CharField(max_length=100) @@ -1420,28 +1400,3 @@ class Meta: serializer.save() self.assertEqual(instance.char_field, 'value changed by signal') - - -class UniqueConstraintNullableModel(models.Model): - title = models.CharField(max_length=100) - age = models.IntegerField(null=True) - tag = models.CharField(max_length=100, null=True) - - class Meta: - constraints = [ - # Unique constraint on 2 nullable fields - models.UniqueConstraint(name='unique_constraint', fields=('age', 'tag')) - ] - - -class TestUniqueConstraintWithNullableFields(TestCase): - def test_nullable_unique_constraint_fields_are_not_required(self): - class UniqueConstraintNullableSerializer(serializers.ModelSerializer): - class Meta: - model = UniqueConstraintNullableModel - fields = ('title', 'age', 'tag') - - serializer = UniqueConstraintNullableSerializer(data={'title': 'Bob'}) - self.assertTrue(serializer.is_valid(), serializer.errors) - result = serializer.save() - self.assertIsInstance(result, UniqueConstraintNullableModel) diff --git a/tests/test_validators.py b/tests/test_validators.py index f09ecc09e0..9c1a0eac31 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -547,12 +547,30 @@ class Meta: ] +class UniqueConstraintNullableModel(models.Model): + title = models.CharField(max_length=100) + age = models.IntegerField(null=True) + tag = models.CharField(max_length=100, null=True) + + class Meta: + constraints = [ + # Unique constraint on 2 nullable fields + models.UniqueConstraint(name='unique_constraint', fields=('age', 'tag')) + ] + + class UniqueConstraintSerializer(serializers.ModelSerializer): class Meta: model = UniqueConstraintModel fields = '__all__' +class UniqueConstraintNullableSerializer(serializers.ModelSerializer): + class Meta: + model = UniqueConstraintNullableModel + fields = ('title', 'age', 'tag') + + class TestUniqueConstraintValidation(TestCase): def setUp(self): self.instance = UniqueConstraintModel.objects.create( @@ -619,6 +637,12 @@ def test_single_field_uniq_validators(self): ids_in_qs = {frozenset(v.queryset.values_list(flat=True)) for v in validators if hasattr(v, "queryset")} assert ids_in_qs == {frozenset([1]), frozenset([3])} + def test_nullable_unique_constraint_fields_are_not_required(self): + serializer = UniqueConstraintNullableSerializer(data={'title': 'Bob'}) + self.assertTrue(serializer.is_valid(), serializer.errors) + result = serializer.save() + self.assertIsInstance(result, UniqueConstraintNullableModel) + # Tests for `UniqueForDateValidator` # ----------------------------------