From 3d85473edf847ba64aa499b336ca21f6b3d3c6b8 Mon Sep 17 00:00:00 2001 From: Aider Ibragimov Date: Wed, 18 Feb 2015 21:00:12 +0300 Subject: [PATCH 1/3] Fix UniqueTogetherValidator for NULL values --- rest_framework/validators.py | 4 +++- tests/test_validators.py | 20 +++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/rest_framework/validators.py b/rest_framework/validators.py index e3719b8d5..c030abdba 100644 --- a/rest_framework/validators.py +++ b/rest_framework/validators.py @@ -138,7 +138,9 @@ class UniqueTogetherValidator: queryset = self.queryset queryset = self.filter_queryset(attrs, queryset) queryset = self.exclude_current_instance(attrs, queryset) - if queryset.exists(): + + # Ignore validation if any field is None + if None not in attrs.values() and queryset.exists(): field_names = ', '.join(self.fields) raise ValidationError(self.message.format(field_names=field_names)) diff --git a/tests/test_validators.py b/tests/test_validators.py index 072cec360..185febf83 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -76,8 +76,8 @@ class TestUniquenessValidation(TestCase): # ----------------------------------- class UniquenessTogetherModel(models.Model): - race_name = models.CharField(max_length=100) - position = models.IntegerField() + race_name = models.CharField(max_length=100, null=True) + position = models.IntegerField(null=True) class Meta: unique_together = ('race_name', 'position') @@ -108,8 +108,8 @@ class TestUniquenessTogetherValidation(TestCase): expected = dedent(""" UniquenessTogetherSerializer(): id = IntegerField(label='ID', read_only=True) - race_name = CharField(max_length=100, required=True) - position = IntegerField(required=True) + race_name = CharField(allow_null=True, max_length=100, required=True) + position = IntegerField(allow_null=True, required=True) class Meta: validators = [] """) @@ -178,10 +178,20 @@ class TestUniquenessTogetherValidation(TestCase): expected = dedent(""" ExcludedFieldSerializer(): id = IntegerField(label='ID', read_only=True) - race_name = CharField(max_length=100) + race_name = CharField(allow_null=True, max_length=100, required=False) """) assert repr(serializer) == expected + def test_ignore_validation_for_null_fields(self): + UniquenessTogetherModel.objects.create( + race_name=None, + position=None + ) + data = {'race_name': None, 'position': None} + serializer = UniquenessTogetherSerializer(data=data) + + assert serializer.is_valid() + # Tests for `UniqueForDateValidator` # ---------------------------------- From fe8d95f93e11d801d07c8852b12abb4f6b21e1e6 Mon Sep 17 00:00:00 2001 From: Aider Ibragimov Date: Thu, 19 Feb 2015 18:03:44 +0300 Subject: [PATCH 2/3] Skip validation of NULL field only if it part of unique_together --- rest_framework/validators.py | 5 +++- tests/test_validators.py | 54 ++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/rest_framework/validators.py b/rest_framework/validators.py index c030abdba..ab3616149 100644 --- a/rest_framework/validators.py +++ b/rest_framework/validators.py @@ -140,7 +140,10 @@ class UniqueTogetherValidator: queryset = self.exclude_current_instance(attrs, queryset) # Ignore validation if any field is None - if None not in attrs.values() and queryset.exists(): + checked_values = [ + value for field, value in attrs.items() if field in self.fields + ] + if None not in checked_values and queryset.exists(): field_names = ', '.join(self.fields) raise ValidationError(self.message.format(field_names=field_names)) diff --git a/tests/test_validators.py b/tests/test_validators.py index 185febf83..c4c60b7fe 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -83,11 +83,37 @@ class UniquenessTogetherModel(models.Model): unique_together = ('race_name', 'position') +class NullUniquenessTogetherModel(models.Model): + """ + Used to ensure that null values are not included when checking + unique_together constraints. + + Ignoring items which have a null in any of the validated fields is the same + behavior that database backends will use when they have the + unique_together constraint added. + + Example case: a null position could indicate a non-finisher in the race, + there could be many non-finishers in a race, but all non-NULL + values *should* be unique against the given `race_name`. + """ + date_of_birth = models.DateField(null=True) # Not part of the uniqueness constraint + race_name = models.CharField(max_length=100) + position = models.IntegerField(null=True) + + class Meta: + unique_together = ('race_name', 'position') + + class UniquenessTogetherSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel +class NullUniquenessTogetherSerializer(serializers.ModelSerializer): + class Meta: + model = NullUniquenessTogetherModel + + class TestUniquenessTogetherValidation(TestCase): def setUp(self): self.instance = UniquenessTogetherModel.objects.create( @@ -183,15 +209,33 @@ class TestUniquenessTogetherValidation(TestCase): assert repr(serializer) == expected def test_ignore_validation_for_null_fields(self): - UniquenessTogetherModel.objects.create( - race_name=None, + # None values that are on fields which are part of the uniqueness + # constraint cause the instance to ignore uniqueness validation. + NullUniquenessTogetherModel.objects.create( + date_of_birth=datetime.date(2000, 1, 1), + race_name='Paris Marathon', position=None ) - data = {'race_name': None, 'position': None} - serializer = UniquenessTogetherSerializer(data=data) - + data = { + 'date': datetime.date(2000, 1, 1), + 'race_name': 'Paris Marathon', + 'position': None + } + serializer = NullUniquenessTogetherSerializer(data=data) assert serializer.is_valid() + 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. + NullUniquenessTogetherModel.objects.create( + date_of_birth=datetime.date(2000, 1, 1), + race_name='Paris Marathon', + position=1 + ) + data = {'date': None, 'race_name': 'Paris Marathon', 'position': 1} + serializer = NullUniquenessTogetherSerializer(data=data) + assert not serializer.is_valid() + # Tests for `UniqueForDateValidator` # ---------------------------------- From aa7ed316d842c06d7eb6907d4481d72c747991d7 Mon Sep 17 00:00:00 2001 From: Aider Ibragimov Date: Thu, 19 Feb 2015 18:09:04 +0300 Subject: [PATCH 3/3] Return UniquenessTogetherModel to previous state --- tests/test_validators.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_validators.py b/tests/test_validators.py index c4c60b7fe..127ec6f8b 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -76,8 +76,8 @@ class TestUniquenessValidation(TestCase): # ----------------------------------- class UniquenessTogetherModel(models.Model): - race_name = models.CharField(max_length=100, null=True) - position = models.IntegerField(null=True) + race_name = models.CharField(max_length=100) + position = models.IntegerField() class Meta: unique_together = ('race_name', 'position') @@ -134,8 +134,8 @@ class TestUniquenessTogetherValidation(TestCase): expected = dedent(""" UniquenessTogetherSerializer(): id = IntegerField(label='ID', read_only=True) - race_name = CharField(allow_null=True, max_length=100, required=True) - position = IntegerField(allow_null=True, required=True) + race_name = CharField(max_length=100, required=True) + position = IntegerField(required=True) class Meta: validators = [] """) @@ -204,7 +204,7 @@ class TestUniquenessTogetherValidation(TestCase): expected = dedent(""" ExcludedFieldSerializer(): id = IntegerField(label='ID', read_only=True) - race_name = CharField(allow_null=True, max_length=100, required=False) + race_name = CharField(max_length=100) """) assert repr(serializer) == expected