encode / django-rest-framework

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

Unique constraint prevents nested serializers from updating #2996

Closed xordoquy closed 8 years ago

xordoquy commented 9 years ago

If you have a nested serializer which model does have a unique constraint on a field you're not able to reference an existing instance.

class Task(serializers.ModelSerializer):
    class Meta:
        model = models.Task
        fields = ('id', 'name')

class Category(serializers.ModelSerializer):
    tasks = Task(many=True)
    class Meta:
        model = models.Category
        fields = ('id', 'name', 'tasks')

Let's say I created a category & task which I'm updating with the very same data they already have:

c = Category(data={"id": 1, "name": "Django", "tasks": [{"id": 1, "name": "Demo", "owner": "admin"}]})
c.is_valid()  # Returns False
c.errors  # Returns {'tasks': [{'name': ['This field must be unique.']}]}

The point is, name remains unique because it matches its own id

tomchristie commented 9 years ago

It may be that we simply have to document this as a limitation in validators. There's not really going to be any nice automatic ways to handle this.

xordoquy commented 9 years ago

I'm a bit uncomfortable with this. Let's way our nested resource is either created or fetched from the DB ala get_or_create. If we remove the validation, it may run in later issues. If we keep it, it'll be impossible to get an item that already exists without changing it.

tomchristie commented 9 years ago

We don't support (automatic) writable nested serialization, so it's not totally unreasonable to also say 'validators' are not appropraite for this case, here's what you'd need to do. However if there's an alternative that'd def be worth considering.

arw180 commented 9 years ago

This sounds very similar to the issue I ran into today: http://stackoverflow.com/questions/32123148/writable-nested-serializer-with-existing-objects-using-django-rest-framework-3-2

xordoquy commented 9 years ago

@arw180 indeed. Thanks for pointing to this issue, totally forgot about it. Adding a reference to #3312 since it's the same.

irongomme commented 8 years ago

Arrgh, i've search for a day why was it not works!!!

This is a really confusing issue, because it is a common thing to play with nested models, and i don't know how to get around of this problem ...

I've got this :

class Contact(models.Model):
    account = models.OneToOneField(Account, null=True)
    first_name = models.CharField(max_length=40, blank=True)
    last_name = models.CharField(max_length=40, blank=True)
    phone = PhoneNumberField(blank=True, null=True)
    phone_alt = PhoneNumberField(blank=True, null=True)
    birthday = models.DateField(blank=True, null=True)
    comment = models.TextField(blank=True, null=True)
    created = models.DateTimeField(auto_now_add=True)
    modified = models.DateTimeField(auto_now=True)
    is_musician = models.BooleanField(default=False)
    is_volunteer = models.BooleanField(default=False)
    is_active = models.BooleanField(default=True)
class Account(AbstractBaseUser):
    email = models.EmailField(unique=True)
    tagline = models.CharField(max_length=140, blank=True)
    is_admin = models.BooleanField(default=False)
    is_staff = models.BooleanField(default=False)
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

    objects = AccountManager()

    USERNAME_FIELD = 'email'
class ContactSerializer(serializers.ModelSerializer):
    account = AccountContactSerializer()

    class Meta:
        model = Contact
        read_only_fields = ('created', 'modified',)

def create(self, validated_data):
        account_data = validated_data.pop('account')
        account = Account.objects.create_user(**account_data)
        contact = Contact.objects.create(account=account, **validated_data)

        return contact
class AccountContactSerializer(serializers.ModelSerializer):
    password = serializers.CharField(write_only=True, required=False)

    class Meta:
        model = Account
        fields = ('id', 'email', 'password',)

    def create(self, validated_data):
        return Account.objects.create(**validated_data)

And i must save a contact with an account, they are inseparable. How can i do this without this annoying unique validator message ?

xordoquy commented 8 years ago

The discussion group is the best place to take this discussion and other usage questions. Thanks!

ankitml commented 8 years ago

So what is the way out? This to me seems like a major limitation.

  1. Writing custom non-model serializer. - Almost all my endpoints would have custom serializers then.
  2. Ignoring 'id' from 'to_internal_value' method.* - Might run into trouble later on because of this.
  3. Any way to use model serializer but change the validation rule for nested serializr? some sort of mixin.. This would be an ideal way, not sure how to do it.
    • check issue no 2403.
xordoquy commented 8 years ago

The workaround this is to remove the unique constraint from the serializer and move it to the business logic validation part since the nested serializer can not say - as of today - whether he's updating or creating a new object.

BurkovBA commented 8 years ago

Guys, I've run into this problem, too.

Just wanted to say that (unfortunately) without a generic WritableNestedModelSerializer out-of-the-box, Django REST framework is hardly useful. With hyperlinked APIs only, you often have to create chains of 3-4-5 consecutive POST/PUT ajax requests, which is waste of time. Writing one from scratch is problematic, repetitive and requires good knowledge of DRF internals. Much more importantly, if you want to expose your API to domain area specialists (e.g. bioinformaticians), they won't buy Hyperlinked one. They want a clear API with nested data structures and, to be honest, they are right.

I''ve been studying the details of this for a couple of days now, got stuck on this error and not sure, whether I should continue messing with DRF internals or stop (breaking the deadlines) investing time into this and start learning Loopback.

I'm pretty sure that with advent of update_or_create function in Django 1.7 it's possible to think out a generic way of nesting writable serializers.

dedsm commented 8 years ago

but can't the nested serializer figure out if it's updating an instance by checking parent.instance?

xordoquy commented 8 years ago

@dedsm sure, but that would also break cases where the uniqueness constraint is broken on the "main" serializer and end up in 500 errors with database errors.

getup8 commented 7 years ago

Sorry I can't really tell from this thread; was this issue fixed?

I'm trying to update a ManyToMany Field via nested serializer and it gets stuck on run_validators() because the relation object I'm trying to attach to my main object already exists.

Would be nice to at least put the recommended workaround (with a few specifics) for a pretty common use case like this.

xordoquy commented 7 years ago

The workaround this is to remove the unique constraint from the serializer and move it to the business logic validation part since the nested serializer can not say - as of today - whether he's updating or creating a new object.

ColinBarnwell commented 7 years ago

I recently came across this issue (a user object attached to an account object via an intermediary relationship).

Within the context of the account, I wanted to be able to update both the user details and the relationship details as a single endpoint. The problem being that the email field on the user model is unique and fell foul of the issue described here.

I wasn't happy with breaking some very necessary validation, so I took a more direct approach. At the point of serializer initialisation, if the email supplied in the data is already the same as the email on the user, I simply removed it from the data. No change = no validation required*.

def __init__(self, instance, data=empty, **kwargs):
    if (
        data is not empty and
        'details.email' in data and
        instance.user is not None and
        instance.user.email == data['details.email']
    ):
        del data['details.email']
    super(AccountUserSerializer, self).__init__(instance, data=data, **kwargs)
Ostlander commented 7 years ago

@xordoquy, would that work?

class MySerializer(serializers.ModelSerializer):
    nested = NestedSerializer()
    ...

    def get_fields(self):
        fields = super().get_fields()

        if self.instance is not None:
            nested_serializer = copy.deepcopy(fields.pop('nested'))
            nested_serializer.instance = self.instance.nested
            fields['nested'] = nested_serializer
        return fields
xordoquy commented 7 years ago

The discussion group is the best place to take further this discussion and other usage questions. Thanks!

xtornasol512 commented 7 years ago

I don't kwon if there is a documentation for this now, but i recently fix this problem with a basic zero validators on the field that cause the problem. I took a lot of time discovering this , so I hope some one else help it youruniquefield = serializers.CharField(max_length=255, min_length=4, validators=[]) Before this I always raise a already exist error even if I correct the problem with managers to use ger_or_create.

xordoquy commented 7 years ago

I wrote some time ago an article about that if that helps: https://medium.com/django-rest-framework/dealing-with-unique-constraints-in-nested-serializers-dade33b831d9

m-ali-94 commented 4 years ago

there is this 3rd party library https://github.com/beda-software/drf-writable-nested, it has a UniqueFieldsMixin which can do the magic to handle unique constraints.

studioj commented 3 years ago

for those ending up here => this SO answer helps https://stackoverflow.com/a/32452603/2559785

iamunadike commented 2 years ago

Wow DRF , you guys should wake up It is 2022 and still the bug is not YET fixed all we find are irrelevant workarounds Its a pity and a bad pity at THAT DRF WAKE UP and update your repo

mirodil1 commented 10 months ago

Same problem here. Has it fixed yet ?

auvipy commented 10 months ago

there is this 3rd party library https://github.com/beda-software/drf-writable-nested, it has a UniqueFieldsMixin which can do the magic to handle unique constraints.

you might try this and report back?

basheeromy commented 9 months ago

@auvipy , I have used the package suggested (drf-writable-nested). It is actually so helpful. nothing to worry about updating multiple models through nested serializer.

psgpyc commented 1 week ago

Has this been fixed yet? I tried doing the same thing today and the issue persists. I am using PrimaryKeyRelatedField as a workaround.