Kaydet (Commit) c4980e28 authored tarafından Emad Mokhtar's avatar Emad Mokhtar Kaydeden (comit) Tim Graham

Fixed #26290 -- Warned that paginating an unordered QuerySet may result in inconsistent results.

üst 724dd204
import collections import collections
import warnings
from math import ceil from math import ceil
from django.utils import six from django.utils import six
from django.utils.functional import cached_property from django.utils.functional import cached_property
class UnorderedObjectListWarning(RuntimeWarning):
pass
class InvalidPage(Exception): class InvalidPage(Exception):
pass pass
...@@ -22,6 +27,7 @@ class Paginator(object): ...@@ -22,6 +27,7 @@ class Paginator(object):
def __init__(self, object_list, per_page, orphans=0, def __init__(self, object_list, per_page, orphans=0,
allow_empty_first_page=True): allow_empty_first_page=True):
self.object_list = object_list self.object_list = object_list
self._check_object_list_is_ordered()
self.per_page = int(per_page) self.per_page = int(per_page)
self.orphans = int(orphans) self.orphans = int(orphans)
self.allow_empty_first_page = allow_empty_first_page self.allow_empty_first_page = allow_empty_first_page
...@@ -94,6 +100,17 @@ class Paginator(object): ...@@ -94,6 +100,17 @@ class Paginator(object):
""" """
return six.moves.range(1, self.num_pages + 1) return six.moves.range(1, self.num_pages + 1)
def _check_object_list_is_ordered(self):
"""
Warn if self.object_list is unordered (typically a QuerySet).
"""
if hasattr(self.object_list, 'ordered') and not self.object_list.ordered:
warnings.warn(
'Pagination may yield inconsistent results with an unordered '
'object_list: {!r}'.format(self.object_list),
UnorderedObjectListWarning
)
QuerySetPaginator = Paginator # For backwards-compatibility. QuerySetPaginator = Paginator # For backwards-compatibility.
......
...@@ -481,7 +481,7 @@ class FieldOverridePostAdmin(PostAdmin): ...@@ -481,7 +481,7 @@ class FieldOverridePostAdmin(PostAdmin):
class CustomChangeList(ChangeList): class CustomChangeList(ChangeList):
def get_queryset(self, request): def get_queryset(self, request):
return self.root_queryset.filter(pk=9999) # Does not exist return self.root_queryset.order_by('pk').filter(pk=9999) # Doesn't exist
class GadgetAdmin(admin.ModelAdmin): class GadgetAdmin(admin.ModelAdmin):
......
...@@ -5,6 +5,7 @@ from datetime import datetime ...@@ -5,6 +5,7 @@ from datetime import datetime
from django.core.paginator import ( from django.core.paginator import (
EmptyPage, InvalidPage, PageNotAnInteger, Paginator, EmptyPage, InvalidPage, PageNotAnInteger, Paginator,
UnorderedObjectListWarning,
) )
from django.test import TestCase from django.test import TestCase
from django.utils import six from django.utils import six
...@@ -256,7 +257,7 @@ class ModelPaginationTests(TestCase): ...@@ -256,7 +257,7 @@ class ModelPaginationTests(TestCase):
a.save() a.save()
def test_first_page(self): def test_first_page(self):
paginator = Paginator(Article.objects.all(), 5) paginator = Paginator(Article.objects.order_by('id'), 5)
p = paginator.page(1) p = paginator.page(1)
self.assertEqual("<Page 1 of 2>", six.text_type(p)) self.assertEqual("<Page 1 of 2>", six.text_type(p))
self.assertQuerysetEqual(p.object_list, [ self.assertQuerysetEqual(p.object_list, [
...@@ -265,9 +266,7 @@ class ModelPaginationTests(TestCase): ...@@ -265,9 +266,7 @@ class ModelPaginationTests(TestCase):
"<Article: Article 3>", "<Article: Article 3>",
"<Article: Article 4>", "<Article: Article 4>",
"<Article: Article 5>" "<Article: Article 5>"
], ])
ordered=False
)
self.assertTrue(p.has_next()) self.assertTrue(p.has_next())
self.assertFalse(p.has_previous()) self.assertFalse(p.has_previous())
self.assertTrue(p.has_other_pages()) self.assertTrue(p.has_other_pages())
...@@ -278,7 +277,7 @@ class ModelPaginationTests(TestCase): ...@@ -278,7 +277,7 @@ class ModelPaginationTests(TestCase):
self.assertEqual(5, p.end_index()) self.assertEqual(5, p.end_index())
def test_last_page(self): def test_last_page(self):
paginator = Paginator(Article.objects.all(), 5) paginator = Paginator(Article.objects.order_by('id'), 5)
p = paginator.page(2) p = paginator.page(2)
self.assertEqual("<Page 2 of 2>", six.text_type(p)) self.assertEqual("<Page 2 of 2>", six.text_type(p))
self.assertQuerysetEqual(p.object_list, [ self.assertQuerysetEqual(p.object_list, [
...@@ -286,9 +285,7 @@ class ModelPaginationTests(TestCase): ...@@ -286,9 +285,7 @@ class ModelPaginationTests(TestCase):
"<Article: Article 7>", "<Article: Article 7>",
"<Article: Article 8>", "<Article: Article 8>",
"<Article: Article 9>" "<Article: Article 9>"
], ])
ordered=False
)
self.assertFalse(p.has_next()) self.assertFalse(p.has_next())
self.assertTrue(p.has_previous()) self.assertTrue(p.has_previous())
self.assertTrue(p.has_other_pages()) self.assertTrue(p.has_other_pages())
...@@ -303,7 +300,7 @@ class ModelPaginationTests(TestCase): ...@@ -303,7 +300,7 @@ class ModelPaginationTests(TestCase):
Tests proper behavior of a paginator page __getitem__ (queryset Tests proper behavior of a paginator page __getitem__ (queryset
evaluation, slicing, exception raised). evaluation, slicing, exception raised).
""" """
paginator = Paginator(Article.objects.all(), 5) paginator = Paginator(Article.objects.order_by('id'), 5)
p = paginator.page(1) p = paginator.page(1)
# Make sure object_list queryset is not evaluated by an invalid __getitem__ call. # Make sure object_list queryset is not evaluated by an invalid __getitem__ call.
...@@ -323,3 +320,14 @@ class ModelPaginationTests(TestCase): ...@@ -323,3 +320,14 @@ class ModelPaginationTests(TestCase):
) )
# After __getitem__ is called, object_list is a list # After __getitem__ is called, object_list is a list
self.assertIsInstance(p.object_list, list) self.assertIsInstance(p.object_list, list)
def test_paginating_unordered_queryset_raises_warning(self):
msg = (
"Pagination may yield inconsistent results with an unordered "
"object_list: <QuerySet [<Article: Article 1>, "
"<Article: Article 2>, <Article: Article 3>, <Article: Article 4>, "
"<Article: Article 5>, <Article: Article 6>, <Article: Article 7>, "
"<Article: Article 8>, <Article: Article 9>]>"
)
with self.assertRaisesMessage(UnorderedObjectListWarning, msg):
Paginator(Article.objects.all(), 5)
...@@ -194,7 +194,7 @@ class HTTPSitemapTests(SitemapTestsBase): ...@@ -194,7 +194,7 @@ class HTTPSitemapTests(SitemapTestsBase):
Check to make sure that the raw item is included with each Check to make sure that the raw item is included with each
Sitemap.get_url() url result. Sitemap.get_url() url result.
""" """
test_sitemap = GenericSitemap({'queryset': TestModel.objects.all()}) test_sitemap = GenericSitemap({'queryset': TestModel.objects.order_by('pk').all()})
def is_testmodel(url): def is_testmodel(url):
return isinstance(url['item'], TestModel) return isinstance(url['item'], TestModel)
......
...@@ -27,7 +27,7 @@ class SimpleI18nSitemap(Sitemap): ...@@ -27,7 +27,7 @@ class SimpleI18nSitemap(Sitemap):
i18n = True i18n = True
def items(self): def items(self):
return I18nTestModel.objects.all() return I18nTestModel.objects.order_by('pk').all()
class EmptySitemap(Sitemap): class EmptySitemap(Sitemap):
...@@ -115,7 +115,7 @@ sitemaps_lastmod_descending = OrderedDict([ ...@@ -115,7 +115,7 @@ sitemaps_lastmod_descending = OrderedDict([
]) ])
generic_sitemaps = { generic_sitemaps = {
'generic': GenericSitemap({'queryset': TestModel.objects.all()}), 'generic': GenericSitemap({'queryset': TestModel.objects.order_by('pk').all()}),
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment