Fixed #18354 -- Performance issue in CBV.

Prevented repeating a query twice when the model isn't ordered by
-date_field (in Meta), allow_empty is False and pagination isn't
enabled.

5 files changed, 31 insertions, 17 deletions

diff --git a/django/views/generic/dates.py b/django/views/generic/dates.py
index a634d170c2..804e03dd20 100644
--- a/django/views/generic/dates.py
+++ b/django/views/generic/dates.py
@@ -315,7 +315,7 @@ class BaseDateListView(MultipleObjectMixin, DateMixin, View):
         """
         raise NotImplementedError('A DateView must provide an implementation of get_dated_items()')
 
-    def get_dated_queryset(self, **lookup):
+    def get_dated_queryset(self, ordering=None, **lookup):
         """
         Get a queryset properly filtered according to `allow_future` and any
         extra lookup kwargs.
@@ -326,6 +326,9 @@ class BaseDateListView(MultipleObjectMixin, DateMixin, View):
         allow_empty = self.get_allow_empty()
         paginate_by = self.get_paginate_by(qs)
 
+        if ordering is not None:
+            qs = qs.order_by(ordering)
+
         if not allow_future:
             now = timezone.now() if self.uses_datetime_field else timezone_today()
             qs = qs.filter(**{'%s__lte' % date_field: now})
@@ -370,15 +373,13 @@ class BaseArchiveIndexView(BaseDateListView):
         """
         Return (date_list, items, extra_context) for this request.
         """
-        qs = self.get_dated_queryset()
+        qs = self.get_dated_queryset(ordering='-%s' % self.get_date_field())
         date_list = self.get_date_list(qs, 'year')
 
-        if date_list:
-            object_list = qs.order_by('-' + self.get_date_field())
-        else:
-            object_list = qs.none()
+        if not date_list:
+            qs = qs.none()
 
-        return (date_list, object_list, {})
+        return (date_list, qs, {})
 
 
 class ArchiveIndexView(MultipleObjectTemplateResponseMixin, BaseArchiveIndexView):
@@ -410,17 +411,15 @@ class BaseYearArchiveView(YearMixin, BaseDateListView):
             '%s__lt' % date_field: until,
         }
 
-        qs = self.get_dated_queryset(**lookup_kwargs)
+        qs = self.get_dated_queryset(ordering='-%s' % date_field, **lookup_kwargs)
         date_list = self.get_date_list(qs, 'month')
 
-        if self.get_make_object_list():
-            object_list = qs.order_by('-' + date_field)
-        else:
+        if not self.get_make_object_list():
             # We need this to be a queryset since parent classes introspect it
             # to find information about the model.
-            object_list = qs.none()
+            qs = qs.none()
 
-        return (date_list, object_list, {'year': year})
+        return (date_list, qs, {'year': year})
 
     def get_make_object_list(self):
         """
diff --git a/tests/regressiontests/generic_views/dates.py b/tests/regressiontests/generic_views/dates.py
index 59833a780c..8f82dddbcf 100644
--- a/tests/regressiontests/generic_views/dates.py
+++ b/tests/regressiontests/generic_views/dates.py
@@ -86,10 +86,15 @@ class ArchiveIndexViewTests(TestCase):
         # 1 query for years list + 1 query for books
         with self.assertNumQueries(2):
             self.client.get('/dates/books/')
-        # same as above + 1 query to test if books exist
-        with self.assertNumQueries(3):
+        # same as above + 1 query to test if books exist + 1 query to count them
+        with self.assertNumQueries(4):
             self.client.get('/dates/books/paginated/')
 
+    def test_no_duplicate_query(self):
+        # Regression test for #18354
+        with self.assertNumQueries(2):
+            self.client.get('/dates/books/reverse/')
+
     def test_datetime_archive_view(self):
         BookSigning.objects.create(event_date=datetime.datetime(2008, 4, 2, 12, 0))
         res = self.client.get('/dates/booksignings/')
@@ -155,6 +160,11 @@ class YearArchiveViewTests(TestCase):
         res = self.client.get('/dates/books/no_year/')
         self.assertEqual(res.status_code, 404)
 
+    def test_no_duplicate_query(self):
+        # Regression test for #18354
+        with self.assertNumQueries(2):
+            self.client.get('/dates/books/2008/reverse/')
+
     def test_datetime_year_view(self):
         BookSigning.objects.create(event_date=datetime.datetime(2008, 4, 2, 12, 0))
         res = self.client.get('/dates/booksignings/2008/')
diff --git a/tests/regressiontests/generic_views/templates/generic_views/book_archive.html b/tests/regressiontests/generic_views/templates/generic_views/book_archive.html
index 5fc11149bd..03f45a38c2 100644
--- a/tests/regressiontests/generic_views/templates/generic_views/book_archive.html
+++ b/tests/regressiontests/generic_views/templates/generic_views/book_archive.html
@@ -1 +1 @@
-Archive of books from {{ date_list }}.
\ No newline at end of file
+Archive of books from {{ date_list }}. {{ object_list|length }} books found.
diff --git a/tests/regressiontests/generic_views/templates/generic_views/book_archive_year.html b/tests/regressiontests/generic_views/templates/generic_views/book_archive_year.html
index 7705914e97..fde198dcaf 100644
--- a/tests/regressiontests/generic_views/templates/generic_views/book_archive_year.html
+++ b/tests/regressiontests/generic_views/templates/generic_views/book_archive_year.html
@@ -1 +1 @@
-Archive of books from {{ year }}.
\ No newline at end of file
+Archive of books from {{ year }}. {{ object_list|length }} books found.
diff --git a/tests/regressiontests/generic_views/urls.py b/tests/regressiontests/generic_views/urls.py
index 96d77e7ed1..14d4f685a6 100644
--- a/tests/regressiontests/generic_views/urls.py
+++ b/tests/regressiontests/generic_views/urls.py
@@ -4,6 +4,7 @@ from django.conf.urls import patterns, url
 from django.views.decorators.cache import cache_page
 from django.views.generic import TemplateView
 
+from . import models
 from . import views
 
 
@@ -108,6 +109,8 @@ urlpatterns = patterns('',
         views.BookArchive.as_view(queryset=None)),
     (r'^dates/books/paginated/$',
         views.BookArchive.as_view(paginate_by=10)),
+    (r'^dates/books/reverse/$',
+        views.BookArchive.as_view(queryset=models.Book.objects.order_by('pubdate'))),
     (r'^dates/booksignings/$',
         views.BookSigningArchive.as_view()),
 
@@ -160,6 +163,8 @@ urlpatterns = patterns('',
         views.BookYearArchive.as_view(make_object_list=True, paginate_by=30)),
     (r'^dates/books/no_year/$',
         views.BookYearArchive.as_view()),
+    (r'^dates/books/(?P<year>\d{4})/reverse/$',
+        views.BookYearArchive.as_view(queryset=models.Book.objects.order_by('pubdate'))),
     (r'^dates/booksignings/(?P<year>\d{4})/$',
         views.BookSigningYearArchive.as_view()),