Opened 3 years ago
Closed 3 years ago
#34148 closed Bug (fixed)
Removing a field from form.fields previously added to _bound_fields_cache has no effect
| Reported by: | Jan Pieter Waagmeester | Owned by: | Francesco Panico |
|---|---|---|---|
| Component: | Forms | Version: | 4.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Chris Jerdonek | 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
The fix for issue "BaseForm.__getitem__() does unneeded work in the happy path" (#32901) added to main in https://github.com/django/django/commit/edde2a069929c93e37835dc3f7c9a229040058e2, A shortcut was added to retrieve bound fields from form._bound_fields_cache.
If a form removes a field after the cache entry for that field was created, the field can be still retrieved from the cache, even though the field is not in form.fields anymore:
import django
from django import forms
from django.test import SimpleTestCase
class Form(forms.Form):
action = forms.CharField()
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# The form was iterated (in my case, separating basic and advanced fields)
[field for field in self if getattr(field, 'advanced', False)]
# After that, we remove a field (i.e. because it is not relevant for admins)
del self.fields["action"]
class FormTestCase(SimpleTestCase):
def test_form(self):
"""
The field should be removed from the instance,
but it can be still retrieved from self._bound_fields_cache.
"""
print("django version", django.VERSION)
form = Form()
with self.assertRaises(KeyError):
form["action"]
This succeeds with django==3.2:
pip install django==3.2.16 ... ./manage.py test tests System check identified no issues (0 silenced). django version (3, 2, 16, 'final', 0) . ---------------------------------------------------------------------- Ran 1 test in 0.000s OK
But fails with django==4.0.8 (and django==4.1.3):
pip install django==4.0.8
...
./manage test tests
Found 1 test(s).
System check identified no issues (0 silenced).
django version (4, 0, 8, 'final', 0)
F
======================================================================
FAIL: test_form (test_forms.FormTestCase)
The field should be removed from the instance,
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/jieter/workspace/django-test/tests/test_forms.py", line 27, in test_form
form["action"]
AssertionError: KeyError not raised
----------------------------------------------------------------------
Ran 1 test in 0.000s
FAILED (failures=1)
Change History (5)
comment:1 by , 3 years ago
| Cc: | added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 3 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Thanks for the report.
Looks like a bug yes. I think likely we just need to revert edde2a069929c93e37835dc3f7c9a229040058e2. 🤔