Fixed #10156: `ModelMultipleChoiceField.clean` now does a single query 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

django/forms/models.py

@@ -794,14 +794,14 @@ class ModelMultipleChoiceField(ModelChoiceField):
             return []
         if not isinstance(value, (list, tuple)):
             raise ValidationError(self.error_messages['list'])
-        final_values = []
+        for pk in value:
-        for val in value:
             try:
-                obj = self.queryset.get(pk=val)
+                self.queryset.filter(pk=pk)
-            except self.queryset.model.DoesNotExist:
-                raise ValidationError(self.error_messages['invalid_choice'] % val)
             except ValueError:
-                raise ValidationError(self.error_messages['invalid_pk_value'] % val)
+                raise ValidationError(self.error_messages['invalid_pk_value'] % pk)
-            else:
+        qs = self.queryset.filter(pk__in=value)
-                final_values.append(obj)
+        pks = set([force_unicode(o.pk) for o in qs])
-        return final_values
+        for val in value:
+            if force_unicode(val) not in pks:
+                raise ValidationError(self.error_messages['invalid_choice'] % val)
+        return qs

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']
-"""}
-

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'])