ModelSerializer not generating validators for constraints #7173

mpratscher · 2020-01-31T21:42:21Z

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

Replacing unique_together on a model with UniqueConstraint constraint (as per Django docs) results in .is_valid() returning True (and an IntegrityError exception being thrown when a call to .save() follows (or 500 Internal Server Error when using API)) rather than .is_valid() returning False (or 400 Bad Request when using API) when uniqueness is violated.

# models.py
class Foo(models.Model):
    f1 = models.CharField(max_length=32)
    f2 = models.IntegerField()

    class Meta:
        # unique_together = ('f1', 'f2')
        constraints = [
            models.UniqueConstraint(name='unique_foo', fields=[
                'f1', 'f2'
            ])
        ]

# serializers.py
class FooSerializer(serializers.ModelSerializer):
    class Meta:
        model = Foo
        fields = '__all__'

Expected behavior

Expected same behavior as when using unique_together:

>>># The following is the behavior when using unique_together
>>> data = {'f1': 'bar', 'f2': 1}
>>> s = FooSerializer(data=data)
>>> if s.is_valid():
...     s.save()
...
<Foo: Foo object (1)>
>>> del s
>>> s = FooSerializer(data=data)
>>> if s.is_valid():
...     s.save()
...
>>>

When using, e.g., the browsable API, this results in a 400 Bad Request.

Actual behavior

>>># The following is the behavior when using UniqueConstraint
>>> data = {'f1': 'bar', 'f2': 1}
>>> s = FooSerializer(data=data)
>>> if s.is_valid():
...     s.save()
...
<Foo: Foo object (1)>
>>> del s
>>> s = FooSerializer(data=data)
>>> if s.is_valid():
...     s.save()
...
Traceback (most recent call last):
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/sqlite3/base.py", line 383, in execute
    return Database.Cursor.execute(self, query, params)
sqlite3.IntegrityError: UNIQUE constraint failed: app_foo.f1, app_foo.f2

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<console>", line 2, in <module>
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/rest_framework/serializers.py", line 212, in save
    self.instance = self.create(validated_data)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/rest_framework/serializers.py", line 948, in create
    instance = ModelClass._default_manager.create(**validated_data)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/query.py", line 422, in create
    obj.save(force_insert=True, using=self.db)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/base.py", line 741, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/base.py", line 779, in save_base
    force_update, using, update_fields,
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/base.py", line 870, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/base.py", line 908, in _do_insert
    using=using, raw=raw)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/query.py", line 1186, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/models/sql/compiler.py", line 1368,
in execute_sql
    cursor.execute(sql, params)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 99, in execute
    return super().execute(sql, params)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/noone/miniconda3/envs/unique_constraint_sample/lib/python3.7/site-packages/django/db/backends/sqlite3/base.py", line 383, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.IntegrityError: UNIQUE constraint failed: app_foo.f1, app_foo.f2
>>>

When using, e.g., the browsable API, this results in a 500 Internal Server Error.

The text was updated successfully, but these errors were encountered:

makegodhere · 2020-02-11T10:15:52Z

Maybe here:

django-rest-framework/rest_framework/serializers.py

Line 1403 in 4137ef4

for unique_together_list in parent_class._meta.unique_together:

Need something like this:

for constraint in self._meta.constraints:
    if constraint is UniqueConstraint:
        print(constraint.fields)

dmwyatt · 2020-06-17T21:53:27Z

Now that the docs recommend using constraints instead of unique_together this should probably get addressed.

@rpkilby Are you working on this? I see you assigned yourself...

This really should've been handled automatically by DRF, and in the future, it will be. But for now, we need to have constraints both on the serializer (to get status code 400), and on the model (to prevent direct database constraint violations). See encode/django-rest-framework#7173

rpkilby · 2020-08-06T00:21:01Z

@dmwyatt I'm not currently working on this - haven't really had the time for larger open source contribution for a while.

re: Assignment, it's not a strict ownership claim, more a signal to other maintainers that I'm interested. It also allows me to more easily find this issue later when I do have time to contribute.

That is all to say, don't let my self-assigning the ticket stop you from opening your own PR.

robd003 · 2020-11-16T06:32:10Z

Any updates on this?

k01ek · 2021-05-12T06:36:38Z

Did anyone find a workaround?

charettes · 2021-06-01T22:36:30Z

Since this seems like a desired feature I thought this django-developers post might be of interested to you https://groups.google.com/g/django-developers/c/iiOnxAn3vy4.

If Constraint.validate landed in Django it would be trivial to implement the DRF serializer abstraction on top of it.

mohmyo · 2021-07-15T01:27:45Z

Got the same problem right now and did this as an easy workaround until it is fixed:

Using the same example as in the issue:

# models.py
class Foo(models.Model):
    f1 = models.CharField(max_length=32)
    f2 = models.IntegerField()

    class Meta:
        # unique_together = ('f1', 'f2')
        constraints = [
            models.UniqueConstraint(name='unique_foo', fields=[
                'f1', 'f2'
            ])
        ]

# serializers.py
class FooSerializer(serializers.ModelSerializer):

    def validate(self, data):
        #this works great with POST, PUT but with PATCH you have to add extra logic since some fields are not presented and you should get it from `self.instance`
        if Foo.objects.filter(f1=data['f1'], f2=data['f2']).exists():   
            raise serializers.ValidationError(
                {'error': 'f1 and f2 should be unique together'})
        return data

    class Meta:
        model = Foo
        fields = '__all__'

Still looking for any updates on this!

UPDATE:
Use this method if you are just looking for making a collection of fields unique nothing more or less, otherwise you don't have any other option than working with validate() as shown but you will have to add extra logic for handling PATCH requests since some fields are not presented and you should get it from self.instance

seychellesproperty · 2021-08-06T10:49:24Z

Got the same problem right now and did this as an easy workaround until it is fixed:

Using the same example as in the issue:

# models.py
class Foo(models.Model):
    f1 = models.CharField(max_length=32)
    f2 = models.IntegerField()

    class Meta:
        # unique_together = ('f1', 'f2')
        constraints = [
            models.UniqueConstraint(name='unique_foo', fields=[
                'f1', 'f2'
            ])
        ]

# serializers.py
class FooSerializer(serializers.ModelSerializer):

    def validate(self, attrs):
        if Foo.objects.filter(f1=attrs['f1'], f2=attrs['f2']).exists():
            raise serializers.ValidationError(
                {'error': 'f1 and f2 should be unique together'})
        return attrs

    class Meta:
        model = Foo
        fields = '__all__'

Still looking for any updates on this!

You are running a query everytime the serializer is called, doesn't seem optimal performance-wise.

I'd rather override the specific actions in the viewset (if you haven't done that already), and catch the IntegrityError in there.

mohmyo · 2021-08-18T16:14:30Z

You are running a query everytime the serializer is called, doesn't seem optimal performance-wise.

I think it's fine, validate() will only run whenever a de-serialization happen and we need to keep up with uniqueness constraint for new inserts or updates only too, so we check for it when we only need it, nothing more or less.

I'd rather override the specific actions in the viewset (if you haven't done that already), and catch the IntegrityError in there.

Good idea at first glance, but I'm afraid that it won't work for our case.
Unique constraint is required on object creation if it was not null so you can't try to save a unique field separately as you normally do and catch an IntegrityError later, also an IntegrityError can be raised for any other reasons, like other fields or the field itself are/is missing or not a valid type, ... etc.

Hamza-Lachi · 2022-04-08T16:57:38Z

any update on this issue?

daniel-adesegun · 2022-04-27T14:33:27Z

The issue lives on, just catching up with it!

charettes · 2022-05-10T14:42:33Z

Support for Constraint.validate constraint landed today (django/django@6671058) and should be available in Django 4.1.

I guess a ConstraintValidator meant to be provided to Serializer.Meta.validators could be built to accept a reference to a Constraint and simply call .validate on it.

AnasKg · 2022-05-27T05:15:59Z

While we are waiting for the update, you can try this method. What do you think?

from rest_framework.validators import UniqueTogetherValidator
from rest_framework.exceptions import ValidationError


class CustomUniqueConstraintValidator(UniqueTogetherValidator):

    def __init__(self, queryset, fields, instance, condition=None, message=None):
        super().__init__(queryset, fields, message)
        if condition is not None:
            self.queryset = queryset.filter(condition)
        self.instance = instance


class UniqueConstraintMixin(object):

    def validate(self, attrs):
        constraints = self.Meta.model._meta.constraints
        errors = []
        for constraint in constraints:
            validator = CustomUniqueConstraintValidator(
                queryset=self.Meta.model._default_manager,
                fields=constraint.fields,
                instance=self.instance,
                condition=constraint.condition
            )
            try:
                validator(attrs)
            except ValidationError as exc:
                errors.extend(exc.detail)

        if errors:
            raise ValidationError(errors)

        return attrs


class FooSerializer(UniqueConstraintMixin, serializers.ModelSerializer):
    class Meta:
        model = Foo
        fields = '__all__'

stale · 2022-08-13T03:28:14Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

itsjef · 2022-09-28T10:49:14Z

bump. This is a real issue.

ckarli · 2022-11-09T12:27:28Z

While we are waiting for the update, you can try this method. What do you think?

from rest_framework.validators import UniqueTogetherValidator
from rest_framework.exceptions import ValidationError


class CustomUniqueConstraintValidator(UniqueTogetherValidator):

    def __init__(self, queryset, fields, instance, condition=None, message=None):
        super().__init__(queryset, fields, message)
        if condition is not None:
            self.queryset = queryset.filter(condition)
        self.instance = instance


class UniqueConstraintMixin(object):

    def validate(self, attrs):
        constraints = self.Meta.model._meta.constraints
        errors = []
        for constraint in constraints:
            validator = CustomUniqueConstraintValidator(
                queryset=self.Meta.model._default_manager,
                fields=constraint.fields,
                instance=self.instance,
                condition=constraint.condition
            )
            try:
                validator(attrs)
            except ValidationError as exc:
                errors.extend(exc.detail)

        if errors:
            raise ValidationError(errors)

        return attrs


class FooSerializer(UniqueConstraintMixin, serializers.ModelSerializer):
    class Meta:
        model = Foo
        fields = '__all__'

This gives a KeyError if one of the constraints is excluded on the serializer.

stale · 2023-01-16T05:09:01Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

robd003 · 2023-01-16T05:45:20Z

This is a real issue, please keep it open.

auvipy · 2023-03-03T07:08:14Z

primary support has been added on main branch. in future we should also look for additional new API's & django validation based simpler implementation when we are 4.1/4.2+ only

stale · 2023-05-21T13:10:28Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

robd003 · 2023-05-21T16:06:02Z

Please keep this open until a solution emerges

auvipy · 2023-06-18T07:53:20Z

you can check this for primary support #7438

AGarrow · 2023-06-22T17:51:23Z

you can check this for primary support #7438

@auvipy has this been included in a release? I'm pulling 3.14.0 but don't see it there

auvipy · 2023-06-24T06:09:29Z

no it will be part of 3.15 release, you can try from main branch

FraCata00 · 2023-09-15T15:33:41Z

It can be used the function on UniqueTogetherValidator, enforce_required_fields to avoid the keyerror:

def enforce_required_fields(self, attrs, serializer):
        """
        The `UniqueTogetherValidator` always forces an implied 'required'
        state on the fields it applies to.
        """
        if serializer.instance is not None:
            return

        missing_items = {
            field_name: self.missing_message
            for field_name in self.fields
            if serializer.fields[field_name].source not in attrs
        }
        if missing_items:
            raise ValidationError(missing_items, code='required')

tu-pm · 2023-10-19T06:59:39Z

Thanks @auvipy @kalekseev for your hard works on this feature, I can't wait for release 3.15 to try it out.

Another thought: Can we extend this further so that DRF Validators are compatible with CheckConstraint as well?

auvipy · 2023-10-19T07:03:55Z

Isnt that already working?

rpkilby changed the title ~~UniqueConstraint replacing unique_together does not validate correctly~~ ModelSerializer not generating validator for UniqueConstraint Jan 31, 2020

rpkilby changed the title ~~ModelSerializer not generating validator for UniqueConstraint~~ ModelSerializer not generating validators for constraints Jan 31, 2020

rpkilby self-assigned this Feb 1, 2020

kalekseev mentioned this issue Jul 29, 2020

Primary Support of UniqueConstraint #7438

Merged

encode deleted a comment from auvipy Aug 6, 2020

encode deleted a comment from remigermain Aug 6, 2020

k01ek mentioned this issue May 12, 2021

ASN API Internal Server Error: duplicate key value violates unique constraint "uniqie_number" netbox-community/netbox-bgp#32

Closed

tomchristie added the Priority label Jun 6, 2022

stale bot added the stale label Aug 13, 2022

stale bot removed the stale label Sep 28, 2022

stale bot added the stale label Jan 16, 2023

stale bot removed the stale label Jan 16, 2023

auvipy unassigned rpkilby Jan 25, 2023

stumpylog mentioned this issue Mar 8, 2023

Feature: owner-aware unique model name constraint paperless-ngx/paperless-ngx#2827

Merged

10 tasks

stale bot added the stale label May 21, 2023

stale bot removed the stale label May 21, 2023

stumpylog mentioned this issue Feb 3, 2024

[BUG] Adding a custom field with the same name of an existing field leads to internal server error paperless-ngx/paperless-ngx#5643

Closed

3 tasks

Zacharis278 mentioned this issue Mar 27, 2024

fix: dont run model unique constraint on ExamSerializer edx/edx-exams#265

Merged

AlexanderNeilson mentioned this issue Mar 29, 2024

3.15(.1?) regression: optional fields in serializers are suddenly required (or need explicit None passed) #9345

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ModelSerializer not generating validators for constraints #7173

ModelSerializer not generating validators for constraints #7173

mpratscher commented Jan 31, 2020

makegodhere commented Feb 11, 2020

dmwyatt commented Jun 17, 2020

rpkilby commented Aug 6, 2020

robd003 commented Nov 16, 2020

k01ek commented May 12, 2021

charettes commented Jun 1, 2021

mohmyo commented Jul 15, 2021 •

edited

seychellesproperty commented Aug 6, 2021

mohmyo commented Aug 18, 2021

Hamza-Lachi commented Apr 8, 2022

daniel-adesegun commented Apr 27, 2022

charettes commented May 10, 2022

AnasKg commented May 27, 2022

stale bot commented Aug 13, 2022

itsjef commented Sep 28, 2022

ckarli commented Nov 9, 2022

stale bot commented Jan 16, 2023

robd003 commented Jan 16, 2023

auvipy commented Mar 3, 2023

stale bot commented May 21, 2023

robd003 commented May 21, 2023

auvipy commented Jun 18, 2023

AGarrow commented Jun 22, 2023

auvipy commented Jun 24, 2023

FraCata00 commented Sep 15, 2023

tu-pm commented Oct 19, 2023

auvipy commented Oct 19, 2023

ModelSerializer not generating validators for constraints #7173

ModelSerializer not generating validators for constraints #7173

Comments

mpratscher commented Jan 31, 2020

Checklist

Steps to reproduce

Expected behavior

Actual behavior

makegodhere commented Feb 11, 2020

dmwyatt commented Jun 17, 2020

rpkilby commented Aug 6, 2020

robd003 commented Nov 16, 2020

k01ek commented May 12, 2021

charettes commented Jun 1, 2021

mohmyo commented Jul 15, 2021 • edited

seychellesproperty commented Aug 6, 2021

mohmyo commented Aug 18, 2021

Hamza-Lachi commented Apr 8, 2022

daniel-adesegun commented Apr 27, 2022

charettes commented May 10, 2022

AnasKg commented May 27, 2022

stale bot commented Aug 13, 2022

itsjef commented Sep 28, 2022

ckarli commented Nov 9, 2022

stale bot commented Jan 16, 2023

robd003 commented Jan 16, 2023

auvipy commented Mar 3, 2023

stale bot commented May 21, 2023

robd003 commented May 21, 2023

auvipy commented Jun 18, 2023

AGarrow commented Jun 22, 2023

auvipy commented Jun 24, 2023

FraCata00 commented Sep 15, 2023

tu-pm commented Oct 19, 2023

auvipy commented Oct 19, 2023

mohmyo commented Jul 15, 2021 •

edited