UniqueTogetherValidator is incompatible with field source · Issue #7003 · encode/django-rest-framework (original) (raw)

The UniqueTogetherValidator is incompatible with serializer fields where the serializer field name is different than the underlying model field name. i.e., fields that have a source specified.

This manifests itself in two ways:

For writeable fields, the validator raises a required validation error.
For read-only fields that have a default value, this passes through to the ORM and ultimately raises a django.core.exceptions.FieldError. Example setup

from django.db import models from rest_framework import serializers from rest_framework.validators import UniqueTogetherValidator

from django.test import TestCase

class ExampleModel(models.Model): field1 = models.CharField(max_length=60) field2 = models.CharField(max_length=60)

class Meta:
    unique_together = ['field1', 'field2']

class WriteableExample(serializers.ModelSerializer): alias = serializers.CharField(source='field1')

class Meta:
    model = ExampleModel
    fields = ['alias', 'field2']
    validators = [
        UniqueTogetherValidator(
            queryset=ExampleModel.objects.all(),
            fields=['alias', 'field2']
        )
    ]

class ReadOnlyExample(serializers.ModelSerializer): alias = serializers.CharField(source='field1', default='test', read_only=True)

class Meta:
    model = ExampleModel
    fields = ['alias', 'field2']
    validators = [
        UniqueTogetherValidator(
            queryset=ExampleModel.objects.all(),
            fields=['alias', 'field2']
        )
    ]

class ExampleTests(TestCase):

def test_writeable(self):
    serializer = WriteableExample(data={'alias': 'test', 'field2': 'test'})
    serializer.is_valid(raise_exception=True)

def test_read_only(self):
    serializer = ReadOnlyExample(data={'field2': 'test'})
    serializer.is_valid(raise_exception=True)

Test output

==================================================== FAILURES =====================================================
___________________________________________ ExampleTests.test_read_only ___________________________________________
tests/test_unique_together_example.py:52: in test_read_only
    serializer.is_valid(raise_exception=True)
rest_framework/serializers.py:234: in is_valid
    self._validated_data = self.run_validation(self.initial_data)
rest_framework/serializers.py:431: in run_validation
    self.run_validators(value)
rest_framework/serializers.py:464: in run_validators
    super().run_validators(to_validate)
rest_framework/fields.py:557: in run_validators
    validator(value)
rest_framework/validators.py:157: in __call__
    queryset = self.filter_queryset(attrs, queryset)
rest_framework/validators.py:143: in filter_queryset
    return qs_filter(queryset, **filter_kwargs)
rest_framework/validators.py:28: in qs_filter
    return queryset.filter(**kwargs)
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/query.py:892: in filter
    return self._filter_or_exclude(False, *args, **kwargs)
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/query.py:910: in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/sql/query.py:1290: in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/sql/query.py:1318: in _add_q
    split_subq=split_subq, simple_col=simple_col,
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/sql/query.py:1190: in build_filter
    lookups, parts, reffed_expression = self.solve_lookup_type(arg)
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/sql/query.py:1049: in solve_lookup_type
    _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta())
.tox/venvs/py37-django22/lib/python3.7/site-packages/django/db/models/sql/query.py:1420: in names_to_path
    "Choices are: %s" % (name, ", ".join(available)))
E   django.core.exceptions.FieldError: Cannot resolve keyword 'alias' into field. Choices are: field1, field2, id

___________________________________________ ExampleTests.test_writeable ___________________________________________
tests/test_unique_together_example.py:48: in test_writeable
    serializer.is_valid(raise_exception=True)
rest_framework/serializers.py:242: in is_valid
    raise ValidationError(self.errors)
E   rest_framework.exceptions.ValidationError: {'alias': [ErrorDetail(string='This field is required.', code='required')]}

Original Description

Checklist

[+] I have verified that that issue exists against the master branch of Django REST framework.
[+] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
[+] This is not a usage question. (Those should be directed to the discussion group instead.)
[+] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
[+] I have reduced the issue to the simplest possible case.
I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

according to the changes in drf 3.8.2 the read_only+default fields should be explicilty saved in perform_create of viewset or create, save or update method of serializer.

this perform_create is called after validating the data by serializer.is_valid() inside create method of CreateModelMixin

    def create(self, request, *args, **kwargs):
        serializer = self.get_serializer(data=request.data)
        serializer.is_valid(raise_exception=True)
        self.perform_create(serializer)
        headers = self.get_success_headers(serializer.data)
        return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

now in my codebase i had two cases:

case1 : when the name of field of serializer is same as model like:

model:

class A(models.Model):
      a = models.ForeignKey(B, on_delete=models.CASCADE)
      c = models.CharField(max_length=32)
      ....

serializer:

 class SomeSerializer(serializers.ModelSerializer):
      a = HyperlinkedRelatedField(view_name='c-detail', read_only=True, default=<some instance of Model B>)

      class Meta:
      model=A
      fields=( 'a', 'c', ......)
      validators = [
            UniqueTogetherValidator(
                queryset=A.objects.all(),
                fields=('a', 'c')
            )
        ]

In this case serializer.is_valid() is not failing if i POST something to this serializer and my overwritten perform_create is being called so problem here

because as we can see these fields are included in _read_only_defaults after #5922 and here https://github.com/encode/django-rest-framework/blob/master/rest_framework/serializers.py#L451 the field.field_name is added in the return value that will be validated with uniquetogether so no issues come.

case 2: when the name of field of serializer is 'not' same as model's field and a source is added in the serializer field like:
model:

class A(models.Model):
      a = models.ForeignKey(B, on_delete=models.CASCADE)
      c = models.CharField(max_length=32)
      ....

serializer:

 class SomeSerializer(serializers.ModelSerializer):
      b = HyperlinkedRelatedField(source='a', view_name='c-detail', read_only=True, default=<some instance of Model B>)

      class Meta:
      model=A
      fields=( 'b', 'c', ......)
      validators = [
            UniqueTogetherValidator(
                queryset=A.objects.all(),
                fields=('a', 'c')
            )
        ]

in this case, if i POST something to this serializer, a ValidationError exception is thrown
saying
{'a': [ErrorDetail(string='This field is required.', code='required')]}

because since field.field_name is included in the return ordered dictionary as key so here in the unique together will fail as it expects ('a', 'c') but gets ('b', 'c').

Expected behavior

in second case also it should work fine

Actual behavior

throwing error {'a': [ErrorDetail(string='This field is required.', code='required')]}

Possible source of error found

Since all the fields (including read_only) were earlier present in _wriable_fields, this is the place where it used to get the field name from source - https://github.com/encode/django-rest-framework/blob/master/rest_framework/serializers.py#L496

And since it started using the method _read_only_defaults for these read_only+default fields, it populates using field name rather than source name - https://github.com/encode/django-rest-framework/blob/master/rest_framework/serializers.py#L451

which maybe a bug