[Django] #29158: ModelChoiceField crashes when checking choices' length
Django
-------------------------------------+-------------------------------------
Reporter: François | Owner: François Freitag
Freitag |
Type: | Status: assigned
Uncategorized |
Component: Forms | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
ModelChoiceField must accept Manager as a `.queryset` for backward
compatibility reasons (see #25683). However, `ModelChoiceIterator` does
not play nice with Managers:
{{{
class TestModelChoiceField(TestCase):
def test_queryset_manager_has_length(self):
f = ModelChoiceField(ChoiceOptionModel.objects)
len(f.choices)
}}}
Errors with:
{{{
======================================================================
ERROR: test_queryset_manager_has_length
(forms_tests.test_tmp.TestModelChoiceField)
----------------------------------------------------------------------
Traceback (most recent call last):
File "django/tests/forms_tests/test_tmp.py", line 10, in
test_queryset_manager_has_length
len(f.choices)
File "django/django/forms/models.py", line 1141, in __len__
return len(self.queryset) + (1 if self.field.empty_label is not None
else 0)
TypeError: object of type 'Manager' has no len()
}}}
----
The reason why this `TypeError` was not reported earlier is probably
because Python swallows `TypeError` silently for `__len__` when `list` is
called, because generators have no `len`:
{{{
Python 3.6.4 (default, Jan 5 2018, 02:35:40)
[GCC 7.2.1 20171224] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def gene():
... yield 1
...
>>> len(gene())
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: object of type 'generator' has no len()
>>> list(gene())
[1]
--
Ticket URL: <https://code.djangoproject.com/ticket/29158>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Freitag
Type: Uncategorized | Status: assigned
Component: Forms | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/9725 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/29158#comment:1>
Django
manager
-------------------------------------+-------------------------------------
Reporter: François Freitag | Owner: François
| Freitag
Component: Forms | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Ready for checkin
* type: Uncategorized => Bug
Comment:
(pending a few cosmetic updates)
--
Ticket URL: <https://code.djangoproject.com/ticket/29158#comment:2>
Django
manager
-------------------------------------+-------------------------------------
Reporter: François Freitag | Owner: François
| Freitag
Component: Forms | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"40f0aa988571431508a508f4fac5ba94c6443f5c" 40f0aa98]:
{{{
#!CommitTicketReference repository=""
revision="40f0aa988571431508a508f4fac5ba94c6443f5c"
Fixed #29158 -- Fixed len(choices) crash if ModelChoiceField's queryset is
a manager.
Removing all() in __iter__() prevents a duplicate query when choices are
cast to a list and there's a prefetch_related().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29158#comment:3>
Django
manager
-------------------------------------+-------------------------------------
Reporter: François Freitag | Owner: François
| Freitag
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Gavin Wahl):
The fix has introduced a problem where the queryset is cloned while
setting it. This causes redudant queries when there are copies of the same
for, ie a formset.
--
Ticket URL: <https://code.djangoproject.com/ticket/29158#comment:4>
Django
manager
-------------------------------------+-------------------------------------
Reporter: François Freitag | Owner: François
| Freitag
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Gavin Wahl (added)
Comment:
Hi Gavin — can I ask you to follow-up in a new ticket with a minimal
reproduce of the issue you're seeing here please? Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/29158#comment:5>