Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation of child runs twice. Is this deliberate as it seems a bit inefficient to run the same code twice. #164

Open
tomgwasira opened this issue Apr 15, 2022 · 6 comments

Comments

@tomgwasira
Copy link

tomgwasira commented Apr 15, 2022

Firstly, huge thanks for the code.

I have a small question. Given two serializers: a parent and a child, inserting a validate method in the child serializer and printing something from there reveals that the function is called twice. Is this deliberate as it seems a bit inefficient to run the same code twice.

@zachrank
Copy link

zachrank commented Apr 30, 2022

Something else that I am observing is that, when performing update operations, the first run of validate on a child does not have self.instance set but the second run does. I am checking self.instance. I check the value of self.instance to determine some conditional validation behavior during updates with the assumption that an object being updated will have self.instance set.

@tomgwasira
Copy link
Author

Something else that I am observing is that, when performing update operations, the first run of validate on a child does not have self.instance set but the second run does. I am checking self.instance. I check the value of self.instance to determine some conditional validation behavior during updates with the assumption that an object being updated will have self.instance set.

Oh yeah. I was also actually trying to use self.instance when I realised that it runs twice. I'm, for now, ignoring the validation when there is no self.instance; I don't know what the consequences of that are going to be.

@zachrank
Copy link

zachrank commented May 5, 2022

I have conditional validation that changes depending upon whether the child was being updated or created and I am reliant on self.instance to know what validation to perform. Having the duplicate validation runs occur but with inconsistent value for self.instance was causing me a headache. My quick and dirty resolution is to eagerly check for the object's existence if self.instance is not defined. This isn't ideal since it will lead to unnecessary queries being run but it gets the job done.

Example:

def validate(self, data):
    pk = data.get('id') or data.get('pk')
    self_exists = bool(self.instance) or self.Meta.model.objects.filter(pk=pk).exists()

The only alternative that I can see is what you're doing @tomgwasira: ignore/skip validation if pk or id has a value but self.instance is not set yet.

@Dooweed
Copy link

Dooweed commented Apr 12, 2023

The same situation applies when I try to use self.parent or self.root in child's validation. The first validation (seems to be run by DRF) is correct, while in the second validation (seems to be run by drf-writable-nested) does not have neither self.parent nor self.root.

It could be very useful if you split code in update_or_create_reverse_relations of BaseNestedModelSerializer into several function calls so that users are able to override only needed methods instead of copypasting whole 66-line long method to change only a few lines.

@matmoxam
Copy link

Would love to see a fix for this. I see a PR exists to address it currently #174 but not sure if it is the ideal solution.

@matmoxam
Copy link

Took the time to really dig into how it works and realized that the double validation occurs because when dealing with List Serializers; the child serializer that is attached to the list serializer, which is basically the same serializer minus the many=True flag, is instantiated each time for each object in the list and then validation is also called each time.

So the sequence of events is the ListSerializer will validate all the objects in the list and then the process is repeated for each object with a new instance of the serializer again. This is done because by default you cannot reuse a serializer once you call save among other blockers that are placed by DRF. It's not the most efficient approach and will cause issues when trying to pass custom params to the serializer constructor and then access them later on in the process.

Its a difficult task to solve but more work is needed on this library for sure. I really appreciate the work done on it so far and it will work for simple use cases but anything too complex and you will start having more issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants