From f81d516ae4fe0a5178a0c6c24dccb7fc6f68303a Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 23 Jun 2016 15:09:23 +0100 Subject: [PATCH] Robust uniqueness checks. (#4217) * Robust uniqueness checks * Add master to test matrix (allow_failures) --- .travis.yml | 9 ++++++--- rest_framework/validators.py | 35 +++++++++++++++++++++++++++-------- tests/test_validators.py | 16 ++++++++++++++++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 58460398b..c9d9a1648 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,13 +19,16 @@ env: - TOX_ENV=py27-django110 - TOX_ENV=py35-django110 - TOX_ENV=py34-django110 + - TOX_ENV=py27-djangomaster + - TOX_ENV=py34-djangomaster + - TOX_ENV=py35-djangomaster matrix: fast_finish: true allow_failures: - - TOX_ENV=py27-djangomaster - - TOX_ENV=py34-djangomaster - - TOX_ENV=py35-djangomaster + - env: TOX_ENV=py27-djangomaster + - env: TOX_ENV=py34-djangomaster + - env: TOX_ENV=py35-djangomaster install: # Virtualenv < 14 is required to keep the Python 3.2 builds running. diff --git a/rest_framework/validators.py b/rest_framework/validators.py index 83ad6f7d8..ef23b9bd7 100644 --- a/rest_framework/validators.py +++ b/rest_framework/validators.py @@ -8,6 +8,7 @@ object creation, and makes it possible to switch between using the implicit """ from __future__ import unicode_literals +from django.db import DataError from django.utils.translation import ugettext_lazy as _ from rest_framework.compat import unicode_to_repr @@ -15,6 +16,24 @@ from rest_framework.exceptions import ValidationError from rest_framework.utils.representation import smart_repr +# Robust filter and exist implementations. Ensures that queryset.exists() for +# an invalid value returns `False`, rather than raising an error. +# Refs https://github.com/tomchristie/django-rest-framework/issues/3381 + +def qs_exists(queryset): + try: + return queryset.exists() + except (TypeError, ValueError, DataError): + return False + + +def qs_filter(queryset, **kwargs): + try: + return queryset.filter(**kwargs) + except (TypeError, ValueError, DataError): + return queryset.none() + + class UniqueValidator(object): """ Validator that corresponds to `unique=True` on a model field. @@ -44,7 +63,7 @@ class UniqueValidator(object): Filter the queryset to all instances matching the given attribute. """ filter_kwargs = {self.field_name: value} - return queryset.filter(**filter_kwargs) + return qs_filter(queryset, **filter_kwargs) def exclude_current_instance(self, queryset): """ @@ -59,7 +78,7 @@ class UniqueValidator(object): queryset = self.queryset queryset = self.filter_queryset(value, queryset) queryset = self.exclude_current_instance(queryset) - if queryset.exists(): + if qs_exists(queryset): raise ValidationError(self.message) def __repr__(self): @@ -124,7 +143,7 @@ class UniqueTogetherValidator(object): field_name: attrs[field_name] for field_name in self.fields } - return queryset.filter(**filter_kwargs) + return qs_filter(queryset, **filter_kwargs) def exclude_current_instance(self, attrs, queryset): """ @@ -145,7 +164,7 @@ class UniqueTogetherValidator(object): checked_values = [ value for field, value in attrs.items() if field in self.fields ] - if None not in checked_values and queryset.exists(): + if None not in checked_values and qs_exists(queryset): field_names = ', '.join(self.fields) raise ValidationError(self.message.format(field_names=field_names)) @@ -209,7 +228,7 @@ class BaseUniqueForValidator(object): queryset = self.queryset queryset = self.filter_queryset(attrs, queryset) queryset = self.exclude_current_instance(attrs, queryset) - if queryset.exists(): + if qs_exists(queryset): message = self.message.format(date_field=self.date_field) raise ValidationError({self.field: message}) @@ -234,7 +253,7 @@ class UniqueForDateValidator(BaseUniqueForValidator): filter_kwargs['%s__day' % self.date_field_name] = date.day filter_kwargs['%s__month' % self.date_field_name] = date.month filter_kwargs['%s__year' % self.date_field_name] = date.year - return queryset.filter(**filter_kwargs) + return qs_filter(queryset, **filter_kwargs) class UniqueForMonthValidator(BaseUniqueForValidator): @@ -247,7 +266,7 @@ class UniqueForMonthValidator(BaseUniqueForValidator): filter_kwargs = {} filter_kwargs[self.field_name] = value filter_kwargs['%s__month' % self.date_field_name] = date.month - return queryset.filter(**filter_kwargs) + return qs_filter(queryset, **filter_kwargs) class UniqueForYearValidator(BaseUniqueForValidator): @@ -260,4 +279,4 @@ class UniqueForYearValidator(BaseUniqueForValidator): filter_kwargs = {} filter_kwargs[self.field_name] = value filter_kwargs['%s__year' % self.date_field_name] = date.year - return queryset.filter(**filter_kwargs) + return qs_filter(queryset, **filter_kwargs) diff --git a/tests/test_validators.py b/tests/test_validators.py index 5858ad374..32d41f2ba 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -48,6 +48,18 @@ class AnotherUniquenessSerializer(serializers.ModelSerializer): fields = '__all__' +class IntegerFieldModel(models.Model): + integer = models.IntegerField() + + +class UniquenessIntegerSerializer(serializers.Serializer): + # Note that this field *deliberately* does not correspond with the model field. + # This allows us to ensure that `ValueError`, `TypeError` or `DataError` etc + # raised by a uniqueness check does not trigger a deceptive "this field is not unique" + # validation failure. + integer = serializers.CharField(validators=[UniqueValidator(queryset=IntegerFieldModel.objects.all())]) + + class TestUniquenessValidation(TestCase): def setUp(self): self.instance = UniquenessModel.objects.create(username='existing') @@ -100,6 +112,10 @@ class TestUniquenessValidation(TestCase): rs = RelatedModelSerializer(data=data) self.assertTrue(rs.is_valid()) + def test_value_error_treated_as_not_unique(self): + serializer = UniquenessIntegerSerializer(data={'integer': 'abc'}) + assert serializer.is_valid() + # Tests for `UniqueTogetherValidator` # -----------------------------------