encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
27.85k stars 6.76k forks source link

3.15 regression: Serializer validation failed for unique together constraint #9358

Open anndoc opened 3 months ago

anndoc commented 3 months ago

The new 3.15.0 release introduced a bug with validation unique constraint.

Code to reproduce an error:

from django.db import models
from rest_framework import serializers

class Pet(models.Model):
    name = models.CharField(max_length=100)
    animal_type = models.CharField(max_length=100)
    can_fly = models.BooleanField(null=True)

    class Meta:
        constraints = (
            UniqueConstraint(
                fields=["name", "animal_type"],
                name="unique_pet",
                condition=Q(can_fly__isnull=True),
            ),
        )

class PetSerializer(serializers.ModelSerializer):
    class Meta:
        model = Pet
        fields = ('name', 'animal_type', 'can_fly')

Pet.objects.create(animal_type='dog', name='Fluffy', can_fly=None)
serializer = PetSerializer(data={
    'can_fly': False,
    'animal_type': 'dog',
    'name': 'Fluffy'
})
serializer.is_valid(raise_exception=True)

The last line raises the error:

Error
Traceback (most recent call last):
    rest_framework.exceptions.ValidationError: {
        'non_field_errors': [ErrorDetail(string='The fields name, animal_type must make a unique set.', code='unique')]
    }

Validation ignores that can_fly field is present in the serializer.initial_data and just runs UniqueTogetherValidator for animal_type and name fields.

AGarrow commented 2 months ago

bump on this 😄 , causing some issues for us in prod after upgrading 😅

terjekv commented 1 month ago

This looks similar to what we're seeing here: https://github.com/unioslo/mreg/pull/537, except that we're seeing a RelatedObjectDoesNotExist exception:

ERROR    django.request:log.py:241 Internal Server Error: /api/v1/hosts/
Traceback (most recent call last):
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 65, in _view_wrapper
    return view_func(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/mreg/api/v1/views.py", line 354, in post
    if ipserializer.is_valid():
       ^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 223, in is_valid
    self._validated_data = self.run_validation(self.initial_data)
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 444, in run_validation
    self.run_validators(value)
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/serializers.py", line 477, in run_validators
    super().run_validators(to_validate)
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/fields.py", line 553, in run_validators
    validator(value, self)
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/validators.py", line 169, in __call__
    checked_values = [
                     ^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/rest_framework/validators.py", line 172, in <listcomp>
    if field in self.fields and value != getattr(serializer.instance, field)
                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../projects/uio/mreg/env/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 264, in __get__
    raise self.RelatedObjectDoesNotExist(
mreg.models.host.Ipaddress.host.RelatedObjectDoesNotExist: Ipaddress has no host.

The issue comes from this bit of code: https://github.com/unioslo/mreg/blob/fa6ca20a41bd486cc8053a116f412b1d526a72ef/mreg/api/v1/views.py#L345-L351 combined with the unique_together constraint in Ipaddress: https://github.com/unioslo/mreg/blob/fa6ca20a41bd486cc8053a116f412b1d526a72ef/mreg/models/host.py#L25-L36.

This (very old) code works fine if we use 3.14, but running 3.15.* gives the above.

dfn-certling commented 3 weeks ago

We see the same issue as @terjekv . This is due to #9154 . Before you could create a bare model of the serializer's instance class in memory and provide it to the serializer, resulting in a model stored to the DB. Now the UniqueTogetherValidator tries to check for changes in the unique values in the provided instance that has never been stored to the DB and does not have the corresponding fields initialized.