Skip to content
53 changes: 36 additions & 17 deletions rest_framework/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"""
from django.core.exceptions import FieldError
from django.db import DataError
from django.db.models import Exists
from django.db.models import Exists, Model
from django.utils.translation import gettext_lazy as _

from rest_framework.exceptions import ValidationError
Expand Down Expand Up @@ -69,7 +69,7 @@ def exclude_current_instance(self, queryset, instance):
If an instance is being updated, then do not include
that instance itself as a uniqueness conflict.
"""
if instance is not None:
if instance is not None and isinstance(instance, Model):
return queryset.exclude(pk=instance.pk)
return queryset

Expand Down Expand Up @@ -137,22 +137,20 @@ def enforce_required_fields(self, attrs, serializer):
if missing_items:
raise ValidationError(missing_items, code='required')

def filter_queryset(self, attrs, queryset, serializer):
def filter_queryset(self, attrs, queryset, instance):
"""
Filter the queryset to all instances matching the given attributes.
"""
# field names => field sources
sources = [
serializer.fields[field_name].source
for field_name in self.fields
]
# field names => field sources; resolved by __call__ when available,
Comment on lines +140 to +144
# otherwise fall back to self.fields for direct callers.
sources = getattr(self, '_sources', None) or list(self.fields)

# If this is an update, then any unprovided field should
# have it's value set based on the existing instance attribute.
if serializer.instance is not None:
if instance is not None:
for source in sources:
if source not in attrs:
attrs[source] = getattr(serializer.instance, source)
attrs[source] = getattr(instance, source)

# Determine the filter keyword arguments and filter the queryset.
filter_kwargs = {
Expand All @@ -166,35 +164,56 @@ def exclude_current_instance(self, attrs, queryset, instance):
If an instance is being updated, then do not include
that instance itself as a uniqueness conflict.
"""
if instance is not None:
if instance is not None and isinstance(instance, Model):
return queryset.exclude(pk=instance.pk)
return queryset

def __call__(self, attrs, serializer):
# When many=True is used, the parent ListSerializer's queryset is
# propagated as the child's instance. Treat that as a create so that
# per-object uniqueness checks don't try to call .pk / field attrs on
# the queryset.
instance = serializer.instance if isinstance(serializer.instance, Model) else None
self._sources = [
serializer.fields[field_name].source
for field_name in self.fields
]
Comment on lines +176 to +180

self.enforce_required_fields(attrs, serializer)

# When many=True is used, instance is normalized to None above. If the
# serializer is also partial, some unique-together fields may be absent
# from attrs; a uniqueness check is not meaningful in that case.
if instance is None and serializer.partial:
if any(
serializer.fields[field_name].source not in attrs
for field_name in self.fields
):
return

queryset = self.queryset
queryset = self.filter_queryset(attrs, queryset, serializer)
queryset = self.exclude_current_instance(attrs, queryset, serializer.instance)
queryset = self.filter_queryset(attrs, queryset, instance)
queryset = self.exclude_current_instance(attrs, queryset, instance)

checked_names = [
serializer.fields[field_name].source for field_name in self.fields
]
# Ignore validation if any field is None
if serializer.instance is None:
if instance is None:
checked_values = [attrs[field_name] for field_name in checked_names]
else:
# Ignore validation if all field values are unchanged
checked_values = [
attrs[field_name]
for field_name in checked_names
if attrs[field_name] != getattr(serializer.instance, field_name)
if attrs[field_name] != getattr(instance, field_name)
]

condition_sources = (serializer.fields[field_name].source for field_name in self.condition_fields)
condition_kwargs = {
source: attrs[source]
if source in attrs
else getattr(serializer.instance, source)
else getattr(instance, source)
for source in condition_sources
}
if checked_values and None not in checked_values and qs_exists_with_condition(queryset, self.condition, condition_kwargs):
Expand Down Expand Up @@ -273,7 +292,7 @@ def exclude_current_instance(self, attrs, queryset, instance):
If an instance is being updated, then do not include
that instance itself as a uniqueness conflict.
"""
if instance is not None:
if instance is not None and isinstance(instance, Model):
return queryset.exclude(pk=instance.pk)
return queryset

Expand Down
45 changes: 43 additions & 2 deletions tests/test_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,9 @@ def filter(self, **kwargs):

data = {'race_name': 'bar'}
queryset = MockQueryset()
serializer = UniquenessTogetherSerializer(instance=self.instance)
validator = UniqueTogetherValidator(queryset, fields=('race_name',
'position'))
validator.filter_queryset(attrs=data, queryset=queryset, serializer=serializer)
validator.filter_queryset(attrs=data, queryset=queryset, instance=self.instance)
assert queryset.called_with == {'race_name': 'bar', 'position': 1}

def test_uniq_together_validation_uses_model_fields_method_field(self):
Expand Down Expand Up @@ -832,10 +831,52 @@ def test_unique_constraint_default_message_code(self):
assert serializer.errors == {"non_field_errors": [expected_message]}
assert serializer.errors["non_field_errors"][0].code == UniqueTogetherValidator.code

def test_unique_constraint_with_many_true_does_not_crash(self):
"""
UniqueConstraint validators must not crash with AttributeError when
a queryset is passed as the instance argument together with many=True.

When run_child_validation is not overridden the child serializer has
no per-object instance, so uniqueness checks operate as if all items
are new creates. The important thing is that the code path no longer
raises AttributeError by trying to call .pk on the queryset.

Refs: https://github.com/encode/django-rest-framework/issues/9484
"""
instances = UniqueConstraintModel.objects.all()
# Use global_id values that don't exist yet so uniqueness checks pass.
data = [
{
'race_name': 'new_race',
'position': 10,
'global_id': 100,
'fancy_conditions': 100,
},
{
'race_name': 'other_race',
'position': 20,
'global_id': 200,
'fancy_conditions': 200,
},
]
serializer = UniqueConstraintSerializer(instances, data=data, many=True)
assert serializer.is_valid(), serializer.errors

Comment thread
auvipy marked this conversation as resolved.
# Also cover partial updates: field-level required checks are skipped,
# but uniqueness validators should still fail cleanly (no KeyError / AttributeError).
# Use global_id values that already exist in the DB so the UniqueValidator fires.
partial_data = [
{'global_id': 1},
{'global_id': 2},
]
serializer = UniqueConstraintSerializer(instances, data=partial_data, many=True, partial=True)
assert not serializer.is_valid()
assert isinstance(serializer.errors, list)

# Tests for `UniqueForDateValidator`
# ----------------------------------


class UniqueForDateModel(models.Model):
slug = models.CharField(max_length=100, unique_for_date='published')
published = models.DateField()
Expand Down
Loading