Opened 3 years ago

Closed 2 years ago

#27119 closed Cleanup/optimization (fixed)

full_clean() called too many times during formset validation

Reported by: Claude Paroz Owned by: Karol Sztajerwald
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a formset with 3 forms in it. I had to debug some stuff with it, but then I realized the full_clean method is called 21 times when calling the formset.is_valid(), while I would expect 4 times (once for the formset itself and once for each subform). For example, a validation occurs each time the management_form property is called. While it may not cause real issues in practice, it's poor design and annoying when trying to debug and follow the validation workflow.

Change History (10)

comment:1 Changed 3 years ago by Tim Graham

I think I'm lacking some details to understand this. Is this for plain formsets and/or model formsets? Are the duplicated calls to BaseFormSet.full_clean() and/or some other full_clean()? For example, I added some debugging in Django's test_basic_formset but only saw BaseFormSet.full_clean() called once.

comment:2 Changed 3 years ago by Claude Paroz

I think this affects all sort of formsets. The problem might rather affect the full_clean() methods of included subforms (notably for the management form).

comment:3 Changed 3 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Making BaseFormSet.management_form a @cached_property seems to help (not sure if it fully resolves the issue).

comment:4 Changed 3 years ago by DavidFozo

Owner: changed from nobody to DavidFozo
Status: newassigned

comment:5 Changed 3 years ago by Karol Sztajerwald

Owner: changed from DavidFozo to Karol Sztajerwald

comment:6 Changed 3 years ago by Karol Sztajerwald

Has patch: set

comment:7 Changed 3 years ago by Claude Paroz

Needs tests: set

comment:8 Changed 2 years ago by Claude Paroz

Needs tests: unset

Tests added in that PR.

comment:9 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:10 Changed 2 years ago by GitHub <noreply@…>

Resolution: fixed
Status: assignedclosed

In d49551bc:

Fixed #27119 -- Cached BaseFormSet.management_form property

Thanks Tim Graham for the review.

Note: See TracTickets for help on using tickets.
Back to Top