diff --git a/django/core/paginator.py b/django/core/paginator.py index 29b8969b90f..e64d9804c5a 100644 --- a/django/core/paginator.py +++ b/django/core/paginator.py @@ -1,10 +1,15 @@ import collections +import warnings from math import ceil from django.utils import six from django.utils.functional import cached_property +class UnorderedObjectListWarning(RuntimeWarning): + pass + + class InvalidPage(Exception): pass @@ -22,6 +27,7 @@ class Paginator(object): def __init__(self, object_list, per_page, orphans=0, allow_empty_first_page=True): self.object_list = object_list + self._check_object_list_is_ordered() self.per_page = int(per_page) self.orphans = int(orphans) self.allow_empty_first_page = allow_empty_first_page @@ -94,6 +100,17 @@ class Paginator(object): """ 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. diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 2146fa66f29..a987e146dc8 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -481,7 +481,7 @@ class FieldOverridePostAdmin(PostAdmin): class CustomChangeList(ChangeList): 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): diff --git a/tests/pagination/tests.py b/tests/pagination/tests.py index 8990c7fb5cb..cc063c9eb80 100644 --- a/tests/pagination/tests.py +++ b/tests/pagination/tests.py @@ -5,6 +5,7 @@ from datetime import datetime from django.core.paginator import ( EmptyPage, InvalidPage, PageNotAnInteger, Paginator, + UnorderedObjectListWarning, ) from django.test import TestCase from django.utils import six @@ -256,7 +257,7 @@ class ModelPaginationTests(TestCase): a.save() def test_first_page(self): - paginator = Paginator(Article.objects.all(), 5) + paginator = Paginator(Article.objects.order_by('id'), 5) p = paginator.page(1) self.assertEqual("", six.text_type(p)) self.assertQuerysetEqual(p.object_list, [ @@ -265,9 +266,7 @@ class ModelPaginationTests(TestCase): "", "", "" - ], - ordered=False - ) + ]) self.assertTrue(p.has_next()) self.assertFalse(p.has_previous()) self.assertTrue(p.has_other_pages()) @@ -278,7 +277,7 @@ class ModelPaginationTests(TestCase): self.assertEqual(5, p.end_index()) def test_last_page(self): - paginator = Paginator(Article.objects.all(), 5) + paginator = Paginator(Article.objects.order_by('id'), 5) p = paginator.page(2) self.assertEqual("", six.text_type(p)) self.assertQuerysetEqual(p.object_list, [ @@ -286,9 +285,7 @@ class ModelPaginationTests(TestCase): "", "", "" - ], - ordered=False - ) + ]) self.assertFalse(p.has_next()) self.assertTrue(p.has_previous()) self.assertTrue(p.has_other_pages()) @@ -303,7 +300,7 @@ class ModelPaginationTests(TestCase): Tests proper behavior of a paginator page __getitem__ (queryset evaluation, slicing, exception raised). """ - paginator = Paginator(Article.objects.all(), 5) + paginator = Paginator(Article.objects.order_by('id'), 5) p = paginator.page(1) # Make sure object_list queryset is not evaluated by an invalid __getitem__ call. @@ -323,3 +320,14 @@ class ModelPaginationTests(TestCase): ) # After __getitem__ is called, object_list is a 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: , " + ", , , " + ", , , " + ", ]>" + ) + with self.assertRaisesMessage(UnorderedObjectListWarning, msg): + Paginator(Article.objects.all(), 5) diff --git a/tests/sitemaps_tests/test_http.py b/tests/sitemaps_tests/test_http.py index aac4d69a20d..33599e9ed99 100644 --- a/tests/sitemaps_tests/test_http.py +++ b/tests/sitemaps_tests/test_http.py @@ -194,7 +194,7 @@ class HTTPSitemapTests(SitemapTestsBase): Check to make sure that the raw item is included with each 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): return isinstance(url['item'], TestModel) diff --git a/tests/sitemaps_tests/urls/http.py b/tests/sitemaps_tests/urls/http.py index 72e14d5e7fc..e2fc991a29e 100644 --- a/tests/sitemaps_tests/urls/http.py +++ b/tests/sitemaps_tests/urls/http.py @@ -27,7 +27,7 @@ class SimpleI18nSitemap(Sitemap): i18n = True def items(self): - return I18nTestModel.objects.all() + return I18nTestModel.objects.order_by('pk').all() class EmptySitemap(Sitemap): @@ -115,7 +115,7 @@ sitemaps_lastmod_descending = OrderedDict([ ]) generic_sitemaps = { - 'generic': GenericSitemap({'queryset': TestModel.objects.all()}), + 'generic': GenericSitemap({'queryset': TestModel.objects.order_by('pk').all()}), }