Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #3534 -- newforms ModelChoiceField and ModelMultipleChoiceField…

… no longer cache choices. Instead, they calculate choices via a fresh database query each time the widget is rendered and clean() is called

git-svn-id: http://code.djangoproject.com/svn/django/trunk@4552 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit ee96c7eb2dea86e5bdaf93f7afa91b6f0128dd72 1 parent 5bec651
@adrianholovaty adrianholovaty authored
Showing with 153 additions and 28 deletions.
  1. +69 −19 django/newforms/models.py
  2. +84 −9 tests/modeltests/model_forms/models.py
View
88 django/newforms/models.py
@@ -3,9 +3,11 @@
and database field objects.
"""
+from django.utils.translation import gettext
+from util import ValidationError
from forms import BaseForm, DeclarativeFieldsMetaclass, SortedDictFromList
-from fields import ChoiceField, MultipleChoiceField
-from widgets import Select, SelectMultiple
+from fields import Field, ChoiceField
+from widgets import Select, SelectMultiple, MultipleHiddenInput
__all__ = ('save_instance', 'form_for_model', 'form_for_instance', 'form_for_fields',
'ModelChoiceField', 'ModelMultipleChoiceField')
@@ -104,33 +106,81 @@ def form_for_fields(field_list):
fields = SortedDictFromList([(f.name, f.formfield()) for f in field_list if f.editable])
return type('FormForFields', (BaseForm,), {'base_fields': fields})
+class QuerySetIterator(object):
+ def __init__(self, queryset, empty_label, cache_choices):
+ self.queryset, self.empty_label, self.cache_choices = queryset, empty_label, cache_choices
+
+ def __iter__(self):
+ if self.empty_label is not None:
+ yield (u"", self.empty_label)
+ for obj in self.queryset:
+ yield (obj._get_pk_val(), str(obj))
+ # Clear the QuerySet cache if required.
+ if not self.cache_choices:
+ self.queryset._result_cache = None
+
class ModelChoiceField(ChoiceField):
"A ChoiceField whose choices are a model QuerySet."
- def __init__(self, queryset, empty_label=u"---------", **kwargs):
- self.model = queryset.model
- choices = [(obj._get_pk_val(), str(obj)) for obj in queryset]
- if empty_label is not None:
- choices = [(u"", empty_label)] + choices
- ChoiceField.__init__(self, choices=choices, **kwargs)
+ # This class is a subclass of ChoiceField for purity, but it doesn't
+ # actually use any of ChoiceField's implementation.
+ def __init__(self, queryset, empty_label=u"---------", cache_choices=False,
+ required=True, widget=Select, label=None, initial=None, help_text=None):
+ self.queryset = queryset
+ self.empty_label = empty_label
+ self.cache_choices = cache_choices
+ # Call Field instead of ChoiceField __init__() because we don't need
+ # ChoiceField.__init__().
+ Field.__init__(self, required, widget, label, initial, help_text)
+ self.widget.choices = self.choices
+
+ def _get_choices(self):
+ # If self._choices is set, then somebody must have manually set
+ # the property self.choices. In this case, just return self._choices.
+ if hasattr(self, '_choices'):
+ return self._choices
+ # Otherwise, execute the QuerySet in self.queryset to determine the
+ # choices dynamically.
+ return QuerySetIterator(self.queryset, self.empty_label, self.cache_choices)
+
+ def _set_choices(self, value):
+ # This method is copied from ChoiceField._set_choices(). It's necessary
+ # because property() doesn't allow a subclass to overwrite only
+ # _get_choices without implementing _set_choices.
+ self._choices = self.widget.choices = list(value)
+
+ choices = property(_get_choices, _set_choices)
def clean(self, value):
- value = ChoiceField.clean(self, value)
- if not value:
+ Field.clean(self, value)
+ if value in ('', None):
return None
try:
- value = self.model._default_manager.get(pk=value)
- except self.model.DoesNotExist:
+ value = self.queryset.model._default_manager.get(pk=value)
+ except self.queryset.model.DoesNotExist:
raise ValidationError(gettext(u'Select a valid choice. That choice is not one of the available choices.'))
return value
-class ModelMultipleChoiceField(MultipleChoiceField):
+class ModelMultipleChoiceField(ModelChoiceField):
"A MultipleChoiceField whose choices are a model QuerySet."
- def __init__(self, queryset, **kwargs):
- self.model = queryset.model
- MultipleChoiceField.__init__(self, choices=[(obj._get_pk_val(), str(obj)) for obj in queryset], **kwargs)
+ hidden_widget = MultipleHiddenInput
+ def __init__(self, queryset, cache_choices=False, required=True,
+ widget=SelectMultiple, label=None, initial=None, help_text=None):
+ super(ModelMultipleChoiceField, self).__init__(queryset, None, cache_choices,
+ required, widget, label, initial, help_text)
def clean(self, value):
- value = MultipleChoiceField.clean(self, value)
- if not value:
+ if self.required and not value:
+ raise ValidationError(gettext(u'This field is required.'))
+ elif not self.required and not value:
return []
- return self.model._default_manager.filter(pk__in=value)
+ if not isinstance(value, (list, tuple)):
+ raise ValidationError(gettext(u'Enter a list of values.'))
+ final_values = []
+ for val in value:
+ try:
+ obj = self.queryset.model._default_manager.get(pk=val)
+ except self.queryset.model.DoesNotExist:
+ raise ValidationError(gettext(u'Select a valid choice. %s is not one of the available choices.') % val)
+ else:
+ final_values.append(obj)
+ return final_values
View
93 tests/modeltests/model_forms/models.py
@@ -289,6 +289,46 @@ def __str__(self):
>>> Category.objects.get(id=3)
<Category: Third>
+Here, we demonstrate that choices for a ForeignKey ChoiceField are determined
+at runtime, based on the data in the database when the form is displayed, not
+the data in the database when the form is instantiated.
+>>> ArticleForm = form_for_model(Article)
+>>> f = ArticleForm(auto_id=False)
+>>> print f.as_ul()
+<li>Headline: <input type="text" name="headline" maxlength="50" /></li>
+<li>Pub date: <input type="text" name="pub_date" /></li>
+<li>Writer: <select name="writer">
+<option value="" selected="selected">---------</option>
+<option value="1">Mike Royko</option>
+<option value="2">Bob Woodward</option>
+</select></li>
+<li>Article: <textarea name="article"></textarea></li>
+<li>Categories: <select multiple="multiple" name="categories">
+<option value="1">Entertainment</option>
+<option value="2">It&#39;s a test</option>
+<option value="3">Third</option>
+</select> Hold down "Control", or "Command" on a Mac, to select more than one.</li>
+>>> Category.objects.create(name='Fourth', url='4th')
+<Category: Fourth>
+>>> Writer.objects.create(name='Carl Bernstein')
+<Writer: Carl Bernstein>
+>>> print f.as_ul()
+<li>Headline: <input type="text" name="headline" maxlength="50" /></li>
+<li>Pub date: <input type="text" name="pub_date" /></li>
+<li>Writer: <select name="writer">
+<option value="" selected="selected">---------</option>
+<option value="1">Mike Royko</option>
+<option value="2">Bob Woodward</option>
+<option value="3">Carl Bernstein</option>
+</select></li>
+<li>Article: <textarea name="article"></textarea></li>
+<li>Categories: <select multiple="multiple" name="categories">
+<option value="1">Entertainment</option>
+<option value="2">It&#39;s a test</option>
+<option value="3">Third</option>
+<option value="4">Fourth</option>
+</select> Hold down "Control", or "Command" on a Mac, to select more than one.</li>
+
# ModelChoiceField ############################################################
>>> from django.newforms import ModelChoiceField, ModelMultipleChoiceField
@@ -311,13 +351,30 @@ def __str__(self):
>>> f.clean(2)
<Category: It's a test>
+# Add a Category object *after* the ModelChoiceField has already been
+# instantiated. This proves clean() checks the database during clean() rather
+# than caching it at time of instantiation.
+>>> Category.objects.create(name='Fifth', url='5th')
+<Category: Fifth>
+>>> f.clean(5)
+<Category: Fifth>
+
+# Delete a Category object *after* the ModelChoiceField has already been
+# instantiated. This proves clean() checks the database during clean() rather
+# than caching it at time of instantiation.
+>>> Category.objects.get(url='5th').delete()
+>>> f.clean(5)
+Traceback (most recent call last):
+...
+ValidationError: [u'Select a valid choice. That choice is not one of the available choices.']
+
>>> f = ModelChoiceField(Category.objects.filter(pk=1), required=False)
>>> print f.clean('')
None
>>> f.clean('')
>>> f.clean('1')
<Category: Entertainment>
->>> f.clean('2')
+>>> f.clean('100')
Traceback (most recent call last):
...
ValidationError: [u'Select a valid choice. That choice is not one of the available choices.']
@@ -345,29 +402,47 @@ def __str__(self):
[<Category: Entertainment>, <Category: It's a test>]
>>> f.clean((1, '2'))
[<Category: Entertainment>, <Category: It's a test>]
->>> f.clean(['nonexistent'])
+>>> f.clean(['100'])
Traceback (most recent call last):
...
-ValidationError: [u'Select a valid choice. nonexistent is not one of the available choices.']
+ValidationError: [u'Select a valid choice. 100 is not one of the available choices.']
>>> f.clean('hello')
Traceback (most recent call last):
...
ValidationError: [u'Enter a list of values.']
+
+# Add a Category object *after* the ModelChoiceField has already been
+# instantiated. This proves clean() checks the database during clean() rather
+# than caching it at time of instantiation.
+>>> Category.objects.create(id=6, name='Sixth', url='6th')
+<Category: Sixth>
+>>> f.clean([6])
+[<Category: Sixth>]
+
+# Delete a Category object *after* the ModelChoiceField has already been
+# instantiated. This proves clean() checks the database during clean() rather
+# than caching it at time of instantiation.
+>>> Category.objects.get(url='6th').delete()
+>>> f.clean([6])
+Traceback (most recent call last):
+...
+ValidationError: [u'Select a valid choice. 6 is not one of the available choices.']
+
>>> f = ModelMultipleChoiceField(Category.objects.all(), required=False)
>>> f.clean([])
[]
>>> f.clean(())
[]
->>> f.clean(['4'])
+>>> f.clean(['10'])
Traceback (most recent call last):
...
-ValidationError: [u'Select a valid choice. 4 is not one of the available choices.']
->>> f.clean(['3', '4'])
+ValidationError: [u'Select a valid choice. 10 is not one of the available choices.']
+>>> f.clean(['3', '10'])
Traceback (most recent call last):
...
-ValidationError: [u'Select a valid choice. 4 is not one of the available choices.']
->>> f.clean(['1', '5'])
+ValidationError: [u'Select a valid choice. 10 is not one of the available choices.']
+>>> f.clean(['1', '10'])
Traceback (most recent call last):
...
-ValidationError: [u'Select a valid choice. 5 is not one of the available choices.']
+ValidationError: [u'Select a valid choice. 10 is not one of the available choices.']
"""}
Please sign in to comment.
Something went wrong with that request. Please try again.