Opened 10 years ago

Closed 7 years ago

#21332 closed Cleanup/optimization (fixed)

BaseInlineFormSet.add_fields adds multiple InlineForeignKeyFields

Reported by: Raumkraut Owned by: nobody
Component: Forms Version: 1.6-beta-1
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

When a formset object has its forms iterated over, the _construct_form(...) method calls self.add_fields(...) against each form in the set.

However, for a BaseInlineFormSet, the add_fields implementation creates an InlineForeignKeyField, and appends it to form._meta.fields. It does this for each form, and since form._meta is a class attribute, it ends up with multiple duplicate InlineForeignKeyField instances.

This doesn't appear to be normally evidenced in any usual output, since all the generated FK fields have the same name, so overwrite each other in name-keyed dictionaries of the fields.

Tested on the latest stable/1.6.x branch

Change History (8)

comment:1 in reply to:  description Changed 10 years ago by Raumkraut

Also note that, unless I'm mistaken, the InlineForeignKeyField instances will continue to be appended to _meta.fields for as long as the server is running, and formsets are being iterated over. Which would make this something of a memory leak.

comment:2 Changed 10 years ago by Tim Graham

Do you have any ideas to address this? Could you provide a sample project that demonstrates the problem?

comment:3 Changed 10 years ago by Raumkraut

Okay, so I was wrong about what was getting appended to _meta.fields: It's not InlineForeignKeyField instances which get added, it's only the name of the foreign-key field.

I've cooked up a basic project to demonstrate the issue:
https://github.com/Raumkraut/test_formsets
It should be enough to clone, syncdb and runserver. The root page demonstrates the issue in question.

Since it is just the name of the field which gets appended repeatedly, unless I've missed something, it seems a fix could be fairly simple:

diff --git a/django/forms/models.py b/django/forms/models.py
index 71da433..20a3d54 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -893,7 +893,7 @@ class BaseInlineFormSet(BaseModelFormSet):
 
         # Add the generated field to form._meta.fields if it's defined to make
         # sure validation isn't skipped on that field.
-        if form._meta.fields:
+        if form._meta.fields and self.fk.name not in form._meta.fields:
             if isinstance(form._meta.fields, tuple):
                 form._meta.fields = list(form._meta.fields)
             form._meta.fields.append(self.fk.name)

comment:4 Changed 10 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

I can reproduce, on 1.5.x, 1.6.x and master. The project doesn't seem to be doing anything special.

comment:5 Changed 10 years ago by Carl Meyer

Type: UncategorizedCleanup/optimization

comment:6 Changed 7 years ago by Tim Graham

Has patch: set

#26537 is a duplicate with a PR.

comment:7 Changed 7 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:8 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In a5c8a6ce:

Fixed #21332, #26538 -- Fixed inconsistent and duplicate form fields on inline formsets.

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