encode/django-rest-framework
1
1
Fork 0
mirror of https://github.com/encode/django-rest-framework.git synced 2024-09-20 19:09:03 +03:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Ensure CursorPagination respects nulls in the ordering field (#8912)" (#9381)

Browse Source 
This reverts commit b1cec517ff.
This commit is contained in:
Max Muoto 2024-04-27 06:07:05 -05:00 committed by GitHub
parent f96c065607
commit 7f18ec1b53
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 8 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
rest_framework/pagination.py
View File

@ -11,7 +11,6 @@ from urllib import parse
from django.core.paginator import InvalidPage
from django.core.paginator import Paginator as DjangoPaginator
from django.db.models import Q
from django.template import loader
from django.utils.encoding import force_str
from django.utils.translation import gettext_lazy as _
@ -631,7 +630,7 @@ class CursorPagination(BasePagination):
queryset = queryset.order_by(*self.ordering)
# If we have a cursor with a fixed position then filter by that.
if str(current_position) != 'None':
if current_position is not None:
order = self.ordering[0]
is_reversed = order.startswith('-')
order_attr = order.lstrip('-')
@ -642,12 +641,7 @@ class CursorPagination(BasePagination):
else:
kwargs = {order_attr + '__gt': current_position}
filter_query = Q(**kwargs)
# If some records contain a null for the ordering field, don't lose them.
# When reverse ordering, nulls will come last and need to be included.
if (reverse and not is_reversed) or is_reversed:
filter_query |= Q(**{order_attr + '__isnull': True})
queryset = queryset.filter(filter_query)
queryset = queryset.filter(**kwargs)
# If we have an offset cursor then offset the entire page by that amount.
# We also always fetch an extra item in order to determine if there is a
@ -720,7 +714,7 @@ class CursorPagination(BasePagination):
# The item in this position and the item following it
# have different positions. We can use this position as
# our marker.
has_item_with_unique_position = position is not None
has_item_with_unique_position = True
break
# The item in this position has the same position as the item
@ -773,7 +767,7 @@ class CursorPagination(BasePagination):
# The item in this position and the item following it
# have different positions. We can use this position as
# our marker.
has_item_with_unique_position = position is not None
has_item_with_unique_position = True
break
# The item in this position has the same position as the item
@ -896,7 +890,7 @@ class CursorPagination(BasePagination):
attr = instance[field_name]
else:
attr = getattr(instance, field_name)
return None if attr is None else str(attr)
return str(attr)
def get_paginated_response(self, data):
return Response({

134
tests/test_pagination.py
View File

@ -972,24 +972,17 @@ class TestCursorPagination(CursorPaginationTestsMixin):
def __init__(self, items):
self.items = items
def filter(self, q):
q_args = dict(q.deconstruct()[1])
if not q_args:
# django 3.0.x artifact
q_args = dict(q.deconstruct()[2])
created__gt = q_args.get('created__gt')
created__lt = q_args.get('created__lt')
def filter(self, created__gt=None, created__lt=None):
if created__gt is not None:
return MockQuerySet([
item for item in self.items
if item.created is None or item.created > int(created__gt)
if item.created > int(created__gt)
])
assert created__lt is not None
return MockQuerySet([
item for item in self.items
if item.created is None or item.created < int(created__lt)
if item.created < int(created__lt)
])
def order_by(self, *ordering):
@ -1108,127 +1101,6 @@ class TestCursorPaginationWithValueQueryset(CursorPaginationTestsMixin, TestCase
return (previous, current, next, previous_url, next_url)
class NullableCursorPaginationModel(models.Model):
created = models.IntegerField(null=True)
class TestCursorPaginationWithNulls(TestCase):
"""
Unit tests for `pagination.CursorPagination` with ordering on a nullable field.
"""
def setUp(self):
class ExamplePagination(pagination.CursorPagination):
page_size = 1
ordering = 'created'
self.pagination = ExamplePagination()
data = [
None, None, 3, 4
]
for idx in data:
NullableCursorPaginationModel.objects.create(created=idx)
self.queryset = NullableCursorPaginationModel.objects.all()
get_pages = TestCursorPagination.get_pages
def test_ascending(self):
"""Test paginating one row at a time, current should go 1, 2, 3, 4, 3, 2, 1."""
(previous, current, next, previous_url, next_url) = self.get_pages('/')
assert previous is None
assert current == [None]
assert next == [None]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [None]
assert current == [None]
assert next == [3]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [3] # [None] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L789
assert current == [3]
assert next == [4]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [3]
assert current == [4]
assert next is None
assert next_url is None
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous == [None]
assert current == [3]
assert next == [4]
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous == [None]
assert current == [None]
assert next == [None] # [3] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L731
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous is None
assert current == [None]
assert next == [None]
def test_descending(self):
"""Test paginating one row at a time, current should go 4, 3, 2, 1, 2, 3, 4."""
self.pagination.ordering = ('-created',)
(previous, current, next, previous_url, next_url) = self.get_pages('/')
assert previous is None
assert current == [4]
assert next == [3]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [None] # [4] paging artifact
assert current == [3]
assert next == [None]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [None] # [3] paging artifact
assert current == [None]
assert next == [None]
(previous, current, next, previous_url, next_url) = self.get_pages(next_url)
assert previous == [None]
assert current == [None]
assert next is None
assert next_url is None
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous == [3]
assert current == [None]
assert next == [None]
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous == [None]
assert current == [3]
assert next == [3] # [4] paging artifact documented at https://github.com/ddelange/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L731
# skip back artifact
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
(previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
assert previous is None
assert current == [4]
assert next == [3]
def test_get_displayed_page_numbers():
"""
Test our contextual page display function.