From 03f86a5adb930eac55dea1e903fb958c002d5bc4 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 20 May 2012 13:18:42 +0200 Subject: [PATCH] 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. --- django/views/generic/dates.py | 25 +++++++++---------- tests/regressiontests/generic_views/dates.py | 14 +++++++++-- .../templates/generic_views/book_archive.html | 2 +- .../generic_views/book_archive_year.html | 2 +- tests/regressiontests/generic_views/urls.py | 5 ++++ 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\d{4})/reverse/$', + views.BookYearArchive.as_view(queryset=models.Book.objects.order_by('pubdate'))), (r'^dates/booksignings/(?P\d{4})/$', views.BookSigningYearArchive.as_view()),