Robust uniqueness checks. (#4217)

* Robust uniqueness checks
* Add master to test matrix (allow_failures)
This commit is contained in:
Tom Christie 2016-06-23 15:09:23 +01:00 committed by GitHub
parent a20a75636c
commit f81d516ae4
3 changed files with 49 additions and 11 deletions

View File

@ -19,13 +19,16 @@ env:
- TOX_ENV=py27-django110 - TOX_ENV=py27-django110
- TOX_ENV=py35-django110 - TOX_ENV=py35-django110
- TOX_ENV=py34-django110 - TOX_ENV=py34-django110
- TOX_ENV=py27-djangomaster
- TOX_ENV=py34-djangomaster
- TOX_ENV=py35-djangomaster
matrix: matrix:
fast_finish: true fast_finish: true
allow_failures: allow_failures:
- TOX_ENV=py27-djangomaster - env: TOX_ENV=py27-djangomaster
- TOX_ENV=py34-djangomaster - env: TOX_ENV=py34-djangomaster
- TOX_ENV=py35-djangomaster - env: TOX_ENV=py35-djangomaster
install: install:
# Virtualenv < 14 is required to keep the Python 3.2 builds running. # Virtualenv < 14 is required to keep the Python 3.2 builds running.

View File

@ -8,6 +8,7 @@ object creation, and makes it possible to switch between using the implicit
""" """
from __future__ import unicode_literals from __future__ import unicode_literals
from django.db import DataError
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from rest_framework.compat import unicode_to_repr 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 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): class UniqueValidator(object):
""" """
Validator that corresponds to `unique=True` on a model field. 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 the queryset to all instances matching the given attribute.
""" """
filter_kwargs = {self.field_name: value} filter_kwargs = {self.field_name: value}
return queryset.filter(**filter_kwargs) return qs_filter(queryset, **filter_kwargs)
def exclude_current_instance(self, queryset): def exclude_current_instance(self, queryset):
""" """
@ -59,7 +78,7 @@ class UniqueValidator(object):
queryset = self.queryset queryset = self.queryset
queryset = self.filter_queryset(value, queryset) queryset = self.filter_queryset(value, queryset)
queryset = self.exclude_current_instance(queryset) queryset = self.exclude_current_instance(queryset)
if queryset.exists(): if qs_exists(queryset):
raise ValidationError(self.message) raise ValidationError(self.message)
def __repr__(self): def __repr__(self):
@ -124,7 +143,7 @@ class UniqueTogetherValidator(object):
field_name: attrs[field_name] field_name: attrs[field_name]
for field_name in self.fields for field_name in self.fields
} }
return queryset.filter(**filter_kwargs) return qs_filter(queryset, **filter_kwargs)
def exclude_current_instance(self, attrs, queryset): def exclude_current_instance(self, attrs, queryset):
""" """
@ -145,7 +164,7 @@ class UniqueTogetherValidator(object):
checked_values = [ checked_values = [
value for field, value in attrs.items() if field in self.fields 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) field_names = ', '.join(self.fields)
raise ValidationError(self.message.format(field_names=field_names)) raise ValidationError(self.message.format(field_names=field_names))
@ -209,7 +228,7 @@ class BaseUniqueForValidator(object):
queryset = self.queryset queryset = self.queryset
queryset = self.filter_queryset(attrs, queryset) queryset = self.filter_queryset(attrs, queryset)
queryset = self.exclude_current_instance(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) message = self.message.format(date_field=self.date_field)
raise ValidationError({self.field: message}) 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__day' % self.date_field_name] = date.day
filter_kwargs['%s__month' % self.date_field_name] = date.month filter_kwargs['%s__month' % self.date_field_name] = date.month
filter_kwargs['%s__year' % self.date_field_name] = date.year filter_kwargs['%s__year' % self.date_field_name] = date.year
return queryset.filter(**filter_kwargs) return qs_filter(queryset, **filter_kwargs)
class UniqueForMonthValidator(BaseUniqueForValidator): class UniqueForMonthValidator(BaseUniqueForValidator):
@ -247,7 +266,7 @@ class UniqueForMonthValidator(BaseUniqueForValidator):
filter_kwargs = {} filter_kwargs = {}
filter_kwargs[self.field_name] = value filter_kwargs[self.field_name] = value
filter_kwargs['%s__month' % self.date_field_name] = date.month filter_kwargs['%s__month' % self.date_field_name] = date.month
return queryset.filter(**filter_kwargs) return qs_filter(queryset, **filter_kwargs)
class UniqueForYearValidator(BaseUniqueForValidator): class UniqueForYearValidator(BaseUniqueForValidator):
@ -260,4 +279,4 @@ class UniqueForYearValidator(BaseUniqueForValidator):
filter_kwargs = {} filter_kwargs = {}
filter_kwargs[self.field_name] = value filter_kwargs[self.field_name] = value
filter_kwargs['%s__year' % self.date_field_name] = date.year filter_kwargs['%s__year' % self.date_field_name] = date.year
return queryset.filter(**filter_kwargs) return qs_filter(queryset, **filter_kwargs)

View File

@ -48,6 +48,18 @@ class AnotherUniquenessSerializer(serializers.ModelSerializer):
fields = '__all__' 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): class TestUniquenessValidation(TestCase):
def setUp(self): def setUp(self):
self.instance = UniquenessModel.objects.create(username='existing') self.instance = UniquenessModel.objects.create(username='existing')
@ -100,6 +112,10 @@ class TestUniquenessValidation(TestCase):
rs = RelatedModelSerializer(data=data) rs = RelatedModelSerializer(data=data)
self.assertTrue(rs.is_valid()) 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` # Tests for `UniqueTogetherValidator`
# ----------------------------------- # -----------------------------------