From 009e237cf0c25ce13e73d58feb54ef4f86e47e4e Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 17 May 2012 13:29:52 +0200 Subject: [PATCH] Fixed #17535 -- Optimized list generic views. When allow_empty is False, prevented the view from loading the entire queryset in memory when pagination is enabled. --- django/views/generic/dates.py | 2 +- django/views/generic/list.py | 18 ++++++++++++++---- tests/regressiontests/generic_views/list.py | 10 ++++++++++ tests/regressiontests/generic_views/urls.py | 2 ++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/django/views/generic/dates.py b/django/views/generic/dates.py index 6964624516..00eadb71d5 100644 --- a/django/views/generic/dates.py +++ b/django/views/generic/dates.py @@ -273,7 +273,7 @@ class BaseDateListView(MultipleObjectMixin, DateMixin, View): if not allow_empty: # When pagination is enabled, it's better to do a cheap query # than to load the unpaginated queryset in memory. - is_empty = not bool(qs) if paginate_by is None else not qs.exists() + is_empty = len(qs) == 0 if paginate_by is None else not qs.exists() if is_empty: raise Http404(_(u"No %(verbose_name_plural)s available") % { 'verbose_name_plural': force_unicode(qs.model._meta.verbose_name_plural) diff --git a/django/views/generic/list.py b/django/views/generic/list.py index d4664c34ef..50127066a1 100644 --- a/django/views/generic/list.py +++ b/django/views/generic/list.py @@ -16,7 +16,7 @@ class MultipleObjectMixin(ContextMixin): def get_queryset(self): """ - Get the list of items for this view. This must be an interable, and may + Get the list of items for this view. This must be an iterable, and may be a queryset (in which qs-specific behavior will be enabled). """ if self.queryset is not None: @@ -113,9 +113,19 @@ class BaseListView(MultipleObjectMixin, View): def get(self, request, *args, **kwargs): self.object_list = self.get_queryset() allow_empty = self.get_allow_empty() - if not allow_empty and len(self.object_list) == 0: - raise Http404(_(u"Empty list and '%(class_name)s.allow_empty' is False.") - % {'class_name': self.__class__.__name__}) + + if not allow_empty: + # When pagination is enabled and object_list is a queryset, + # it's better to do a cheap query than to load the unpaginated + # queryset in memory. + if (self.get_paginate_by(self.object_list) is not None + and hasattr(self.object_list, 'exists')): + is_empty = not self.object_list.exists() + else: + is_empty = len(self.object_list) == 0 + if is_empty: + raise Http404(_(u"Empty list and '%(class_name)s.allow_empty' is False.") + % {'class_name': self.__class__.__name__}) context = self.get_context_data(object_list=self.object_list) return self.render_to_response(context) diff --git a/tests/regressiontests/generic_views/list.py b/tests/regressiontests/generic_views/list.py index 9ad00edc3b..b925758524 100644 --- a/tests/regressiontests/generic_views/list.py +++ b/tests/regressiontests/generic_views/list.py @@ -159,6 +159,16 @@ class ListViewTests(TestCase): def test_missing_items(self): self.assertRaises(ImproperlyConfigured, self.client.get, '/list/authors/invalid/') + def test_paginated_list_view_does_not_load_entire_table(self): + # Regression test for #17535 + self._make_authors(3) + # 1 query for authors + with self.assertNumQueries(1): + self.client.get('/list/authors/notempty/') + # same as above + 1 query to test if authors exist + 1 query for pagination + with self.assertNumQueries(3): + self.client.get('/list/authors/notempty/paginated/') + def _make_authors(self, n): Author.objects.all().delete() for i in range(n): diff --git a/tests/regressiontests/generic_views/urls.py b/tests/regressiontests/generic_views/urls.py index 9c47ab8d82..20432f9b6a 100644 --- a/tests/regressiontests/generic_views/urls.py +++ b/tests/regressiontests/generic_views/urls.py @@ -128,6 +128,8 @@ urlpatterns = patterns('', views.AuthorList.as_view(paginate_by=30)), (r'^list/authors/notempty/$', views.AuthorList.as_view(allow_empty=False)), + (r'^list/authors/notempty/paginated/$', + views.AuthorList.as_view(allow_empty=False, paginate_by=2)), (r'^list/authors/template_name/$', views.AuthorList.as_view(template_name='generic_views/list.html')), (r'^list/authors/template_name_suffix/$',