Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #10156: `ModelMultipleChoiceField.clean` now does a single quer…

…y instead of O(N). Thanks, Alex Gaynor. Also, I ported a few more doctests to unittests.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10582 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 41260fb93165a9fa18dba31b4766c8be50b430cb 1 parent 8003119
@jacobian jacobian authored
View
18 django/forms/models.py
@@ -794,14 +794,14 @@ def clean(self, value):
return []
if not isinstance(value, (list, tuple)):
raise ValidationError(self.error_messages['list'])
- final_values = []
- for val in value:
+ for pk in value:
try:
- obj = self.queryset.get(pk=val)
- except self.queryset.model.DoesNotExist:
- raise ValidationError(self.error_messages['invalid_choice'] % val)
+ self.queryset.filter(pk=pk)
except ValueError:
- raise ValidationError(self.error_messages['invalid_pk_value'] % val)
- else:
- final_values.append(obj)
- return final_values
+ raise ValidationError(self.error_messages['invalid_pk_value'] % pk)
+ qs = self.queryset.filter(pk__in=value)
+ pks = set([force_unicode(o.pk) for o in qs])
+ for val in value:
+ if force_unicode(val) not in pks:
+ raise ValidationError(self.error_messages['invalid_choice'] % val)
+ return qs
View
37 tests/regressiontests/model_forms_regress/models.py
@@ -1,47 +1,16 @@
import os
from django.db import models
-from django import forms
+
+class Person(models.Model):
+ name = models.CharField(max_length=100)
class Triple(models.Model):
left = models.IntegerField()
middle = models.IntegerField()
right = models.IntegerField()
- def __unicode__(self):
- return u"%d, %d, %d" % (self.left, self.middle, self.right)
-
class Meta:
unique_together = (('left', 'middle'), (u'middle', u'right'))
class FilePathModel(models.Model):
path = models.FilePathField(path=os.path.dirname(__file__), match=".*\.py$", blank=True)
-
-__test__ = {'API_TESTS': """
-When the same field is involved in multiple unique_together constraints, we
-need to make sure we don't remove the data for it before doing all the
-validation checking (not just failing after the first one).
-
->>> _ = Triple.objects.create(left=1, middle=2, right=3)
->>> class TripleForm(forms.ModelForm):
-... class Meta:
-... model = Triple
-
->>> form = TripleForm({'left': '1', 'middle': '2', 'right': '3'})
->>> form.is_valid()
-False
->>> form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
->>> form.is_valid()
-True
-
-# Regression test for #8842: FilePathField(blank=True)
->>> class FPForm(forms.ModelForm):
-... class Meta:
-... model = FilePathModel
-
->>> form = FPForm()
->>> names = [c[1] for c in form['path'].field.choices]
->>> names.sort()
->>> names
-['---------', '__init__.py', 'models.py']
-"""}
-
View
61 tests/regressiontests/model_forms_regress/tests.py
@@ -0,0 +1,61 @@
+from django import db
+from django import forms
+from django.conf import settings
+from django.test import TestCase
+from models import Person, Triple, FilePathModel
+
+class ModelMultipleChoiceFieldTests(TestCase):
+
+ def setUp(self):
+ self.old_debug = settings.DEBUG
+ settings.DEBUG = True
+
+ def tearDown(self):
+ settings.DEBUG = self.old_debug
+
+ def test_model_multiple_choice_number_of_queries(self):
+ """
+ Test that ModelMultipleChoiceField does O(1) queries instead of
+ O(n) (#10156).
+ """
+ for i in range(30):
+ Person.objects.create(name="Person %s" % i)
+
+ db.reset_queries()
+ f = forms.ModelMultipleChoiceField(queryset=Person.objects.all())
+ selected = f.clean([1, 3, 5, 7, 9])
+ self.assertEquals(len(db.connection.queries), 1)
+
+class TripleForm(forms.ModelForm):
+ class Meta:
+ model = Triple
+
+class UniqueTogetherTests(TestCase):
+ def test_multiple_field_unique_together(self):
+ """
+ When the same field is involved in multiple unique_together
+ constraints, we need to make sure we don't remove the data for it
+ before doing all the validation checking (not just failing after
+ the first one).
+ """
+ Triple.objects.create(left=1, middle=2, right=3)
+
+ form = TripleForm({'left': '1', 'middle': '2', 'right': '3'})
+ self.failIf(form.is_valid())
+
+ form = TripleForm({'left': '1', 'middle': '3', 'right': '1'})
+ self.failUnless(form.is_valid())
+
+class FPForm(forms.ModelForm):
+ class Meta:
+ model = FilePathModel
+
+class FilePathFieldTests(TestCase):
+ def test_file_path_field_blank(self):
+ """
+ Regression test for #8842: FilePathField(blank=True)
+ """
+ form = FPForm()
+ names = [p[1] for p in form['path'].field.choices]
+ names.sort()
+ self.assertEqual(names, ['---------', '__init__.py', 'models.py', 'tests.py'])
Please sign in to comment.
Something went wrong with that request. Please try again.