Opened 8 years ago

Closed 8 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: dev
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 by Tim Graham, 8 years ago

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 by Claude Paroz, 8 years ago

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 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

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

comment:4 by DavidFozo, 8 years ago

Owner: changed from nobody to DavidFozo
Status: newassigned

comment:5 by Karol Sztajerwald, 8 years ago

Owner: changed from DavidFozo to Karol Sztajerwald

comment:6 by Karol Sztajerwald, 8 years ago

Has patch: set

comment:7 by Claude Paroz, 8 years ago

Needs tests: set

comment:8 by Claude Paroz, 8 years ago

Needs tests: unset

Tests added in that PR.

comment:9 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by GitHub <noreply@…>, 8 years ago

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