Skip to content

Commit

Permalink
Fixed #17535 -- Optimized list generic views.
Browse files Browse the repository at this point in the history
When allow_empty is False, prevented the view from loading
the entire queryset in memory when pagination is enabled.
  • Loading branch information
aaugustin committed May 17, 2012
1 parent 006c2b8 commit 009e237
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
2 changes: 1 addition & 1 deletion django/views/generic/dates.py
Expand Up @@ -273,7 +273,7 @@ def get_dated_queryset(self, **lookup):
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)
Expand Down
18 changes: 14 additions & 4 deletions django/views/generic/list.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
10 changes: 10 additions & 0 deletions tests/regressiontests/generic_views/list.py
Expand Up @@ -159,6 +159,16 @@ def test_duplicate_context_object_name(self):
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):
Expand Down
2 changes: 2 additions & 0 deletions tests/regressiontests/generic_views/urls.py
Expand Up @@ -128,6 +128,8 @@
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/$',
Expand Down

0 comments on commit 009e237

Please sign in to comment.