Invalid self.instance when validating the serializer using many=True

[sshishov]

Currently it is almost impossible to use the same serializer with and without many=True and the problem that sometimes serializer depending on the instance state or value during serialization and validation (usually it is PATCH method and partial=True

This is the simplest example to understand:

from django.db import models
from rest_framework import serializers


class MyClass(models.Model):
    name = models.CharField(max_length=100)
    status = models.CharField(max_length=100, blank=True)

    @property
    def is_valid(self):
        return self.name == 'valid'


class MyClassSerializer(serializers.ModelSerializer):
    class Meta:
        model = MyClass
        fields = ('id', 'name', 'status')

    def validate_status(self, value):
        if value and not self.instance.is_valid:
            raise serializers.ValidationError('Status cannot be set for invalid instance')
        return value


objs = MyClass.objects.bulk_create(
    objs=[
        MyClass(name='valid'),
        MyClass(name='invalid'),
        MyClass(name='other'),
    ]
)

serializer = MyClassSerializer(
    data=[{'status': 'set', 'id': instance.id} for instance in objs],
    instance=objs,
    many=True,
    partial=True,
)
serializer.is_valid()
print(serializer.errors)

Please note that it is just an example and real serializers and models can be much more complex. The case is working when we pass the single instance to the serializer but not working when we are trying to validate bulk operation, like update in bulk (let's say)

The following exception raised:

AttributeError: 'list' object has no attribute 'is_valid'

When expected behavior would be to get this one (pretty-printed):

[
    {},
    {'status': [ErrorDetail(string='Status cannot be set for invalid instance', code='invalid')]},
    {'status': [ErrorDetail(string='Status cannot be set for invalid instance', code='invalid')]},
]

P.S. I was able to manually patch the serializer to obtain the instance from the data provided, but I do not think that it should be done manually for every serializer. I assume that when we are validating the instance using PARTIAL, then the appropriate instance which is currently on validation, should be available inside the serializer. In this case we will be able to use the same serializer as single and multi entity-validation.

P.P.S. Also I think that if user require some logic for the list of objects, it should be placed inside the overridden list_serializer_class, no?

[saadullahaleem]

I understand what you're trying to do here but I'm not sure how the values in the data list and the objs that you pass as the instance would correspond to each other. Would you assume that they are matched via indexes? Would you want to introduce a new keyword argument (e.g instances) just to make it clearer?

Also, from what I understand, many only works when you want serialize multiple objects into a list.

Interesting idea for a new feature, nonetheless. I know that Django does support bulk updating so this should be possible to do. However, I'm not sure if the team behind DRF would want to add this to the core project at this point.

[auvipy]

if anything is supported in django ORM core then that should be supported in DRF too.

[saadullahaleem]

My only concern is the mapping between the data and the objs in this example. Maybe instead of passing the objs it would be better to do something like this:

serializer = MyClassSerializer(
    data=[{'status': 'set', 'id': instance.id} for instance in objs],
    key='id',
    many=True,
    partial=True,
)

This way the serializer can internally make a query to to fetch all objects based on the provided key (the argument doesn't have to have this name) and make a single query to fetch them. It could then do a bulk update as defined in the Django Docs

[saadullahaleem]

So I did some more digging into it. The problem is caused by the following method:

    def validate_status(self, value):
        if value and not self.instance.is_valid:
            raise serializers.ValidationError('Status cannot be set for invalid instance')
        return value

Specifically, it's caused by the self.instance is set to the value passed as data. Also, when you set many to True, the serializer that is declared is actually a ListSerializer instance. I'm not sure how we could make the self.instance variable point to the right dict object within the data list. From the looks of it, this would require significant reworking within the validation of the serializer classes.

[sshishov]

@saadullahaleem it would be great to support it in a long term, but it is not a deal breaker currently (especially if we see the big redesign here)

[sshishov]

Guys, Just to make sure... It can be a breaking change for people who were using `many=True` and validation of instances... Or am I missing something?

…

On Mon, May 29, 2023, 18:03 Asif Saif Uddin ***@***.***> wrote: Closed #8926 <#8926> as completed via #8979 <#8979>. — Reply to this email directly, view it on GitHub <#8926 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAUWBFF3QUFQ4CK6EAITCLTXISUC3ANCNFSM6AAAAAAWRFIQTY> . You are receiving this because you authored the thread.Message ID: ***@***.*** com>

[auvipy]

we will include this in a major release. so we can explicitly declare that

[sshishov]

Will we be able to update stubs to support this? It will be a huge step forward...

Thank you @auvipy 👍

[auvipy]

we might need a better implementation then the already merged PR

[auvipy]

Have to reopen this as the initial fix was not enough to cover all related cases.