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.
This commit is contained in:
parent
b0c1e5c081
commit
03f86a5adb
|
@ -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):
|
||||
"""
|
||||
|
|
|
@ -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/')
|
||||
|
|
|
@ -1 +1 @@
|
|||
Archive of books from {{ date_list }}.
|
||||
Archive of books from {{ date_list }}. {{ object_list|length }} books found.
|
||||
|
|
|
@ -1 +1 @@
|
|||
Archive of books from {{ year }}.
|
||||
Archive of books from {{ year }}. {{ object_list|length }} books found.
|
||||
|
|
|
@ -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()),
|
||||
|
||||
|
|
Loading…
Reference in New Issue