Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #20849 -- ModelForms do not work well with prefetch_related.

model_to_dict() (used when rendering forms) queries the database
to get the list of primary keys for ManyToMany fields. This is
unnecessary if the field queryset has been prefetched, all the
keys are already in memory and can be obtained with a simple
iteration.
  • Loading branch information...
commit 539e3693d4712b249a95cfad8cfdeecdad1777a6 1 parent 4202d9c
@jimpot jimpot authored akaariai committed
Showing with 40 additions and 1 deletion.
  1. +5 −1 django/forms/models.py
  2. +35 −0 tests/model_forms/tests.py
View
6 django/forms/models.py
@@ -134,7 +134,11 @@ def model_to_dict(instance, fields=None, exclude=None):
data[f.name] = []
else:
# MultipleChoiceWidget needs a list of pks, not object instances.
- data[f.name] = list(f.value_from_object(instance).values_list('pk', flat=True))
+ qs = f.value_from_object(instance)
+ if qs._result_cache is not None:
+ data[f.name] = [item.pk for item in qs]
+ else:
+ data[f.name] = list(qs.values_list('pk', flat=True))
else:
data[f.name] = f.value_from_object(instance)
return data
View
35 tests/model_forms/tests.py
@@ -824,6 +824,41 @@ def test_model_to_dict_many_to_many(self):
# Ensure many-to-many relation appears as a list
self.assertIsInstance(d['categories'], list)
+ def test_reuse_prefetched(self):
+ # model_to_dict should not hit the database if it can reuse
+ # the data populated by prefetch_related.
+ categories = [
+ Category(name='TestName1', slug='TestName1', url='url1'),
+ Category(name='TestName2', slug='TestName2', url='url2'),
+ Category(name='TestName3', slug='TestName3', url='url3')
+ ]
+ for c in categories:
+ c.save()
+ writer = Writer(name='Test writer')
+ writer.save()
+
+ art = Article(
+ headline='Test article',
+ slug='test-article',
+ pub_date=datetime.date(1988, 1, 4),
+ writer=writer,
+ article='Hello.'
+ )
+ art.save()
+ for c in categories:
+ art.categories.add(c)
+
+ art = Article.objects.prefetch_related('categories').get(pk=art.pk)
+
+ with self.assertNumQueries(0):
+ d = model_to_dict(art)
+
+ #Ensure all many-to-many categories appear in model_to_dict
+ for c in categories:
+ self.assertIn(c.pk, d['categories'])
+ #Ensure many-to-many relation appears as a list
+ self.assertIsInstance(d['categories'], list)
+
class OldFormForXTests(TestCase):
def test_base_form(self):
self.assertEqual(Category.objects.count(), 0)
Please sign in to comment.
Something went wrong with that request. Please try again.