Fix CursorPagination when objects get deleted between calls (#6504) (#6593)

* Added regression tests (#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>

* Fix CursorPagination when objects get deleted between calls (#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>

rest_framework/pagination.py

@@ -637,7 +637,7 @@ class CursorPagination(BasePagination):
         if not self.has_next:
             return None
 
-        if self.cursor and self.cursor.reverse and self.cursor.offset != 0:
+        if self.page and self.cursor and self.cursor.reverse and self.cursor.offset != 0:
             # If we're reversing direction and we have an offset cursor
             # then we cannot use the first position we find as a marker.
             compare = self._get_position_from_instance(self.page[-1], self.ordering)
@@ -645,12 +645,14 @@ class CursorPagination(BasePagination):
             compare = self.next_position
         offset = 0
 
+        has_item_with_unique_position = False
         for item in reversed(self.page):
             position = self._get_position_from_instance(item, self.ordering)
             if position != compare:
                 # 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 = True
                 break
 
             # The item in this position has the same position as the item
@@ -659,7 +661,7 @@ class CursorPagination(BasePagination):
             compare = position
             offset += 1
 
-        else:
+        if self.page and not has_item_with_unique_position:
             # There were no unique positions in the page.
             if not self.has_previous:
                 # We are on the first page.
@@ -678,6 +680,9 @@ class CursorPagination(BasePagination):
                 offset = self.cursor.offset + self.page_size
                 position = self.previous_position
 
+        if not self.page:
+            position = self.next_position
+
         cursor = Cursor(offset=offset, reverse=False, position=position)
         return self.encode_cursor(cursor)
 
@@ -685,7 +690,7 @@ class CursorPagination(BasePagination):
         if not self.has_previous:
             return None
 
-        if self.cursor and not self.cursor.reverse and self.cursor.offset != 0:
+        if self.page and self.cursor and not self.cursor.reverse and self.cursor.offset != 0:
             # If we're reversing direction and we have an offset cursor
             # then we cannot use the first position we find as a marker.
             compare = self._get_position_from_instance(self.page[0], self.ordering)
@@ -693,12 +698,14 @@ class CursorPagination(BasePagination):
             compare = self.previous_position
         offset = 0
 
+        has_item_with_unique_position = False
         for item in self.page:
             position = self._get_position_from_instance(item, self.ordering)
             if position != compare:
                 # 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 = True
                 break
 
             # The item in this position has the same position as the item
@@ -707,7 +714,7 @@ class CursorPagination(BasePagination):
             compare = position
             offset += 1
 
-        else:
+        if self.page and not has_item_with_unique_position:
             # There were no unique positions in the page.
             if not self.has_next:
                 # We are on the final page.
@@ -726,6 +733,9 @@ class CursorPagination(BasePagination):
                 offset = 0
                 position = self.next_position
 
+        if not self.page:
+            position = self.previous_position
+
         cursor = Cursor(offset=offset, reverse=True, position=position)
         return self.encode_cursor(cursor)
 

tests/test_pagination.py

@@ -630,6 +630,52 @@ class CursorPaginationTestsMixin:
 
         assert isinstance(self.pagination.to_html(), str)
 
+    def test_cursor_pagination_current_page_empty_forward(self):
+        # Regression test for #6504
+        self.pagination.base_url = "/"
+
+        # We have a cursor on the element at position 100, but this element doesn't exist
+        # anymore.
+        cursor = pagination.Cursor(reverse=False, offset=0, position=100)
+        url = self.pagination.encode_cursor(cursor)
+        self.pagination.base_url = "/"
+
+        # Loading the page with this cursor doesn't crash
+        (previous, current, next, previous_url, next_url) = self.get_pages(url)
+
+        # The previous url doesn't crash either
+        (previous, current, next, previous_url, next_url) = self.get_pages(previous_url)
+
+        # And point to things that are not completely off.
+        assert previous == [7, 7, 7, 8, 9]
+        assert current == [9, 9, 9, 9, 9]
+        assert next == []
+        assert previous_url is not None
+        assert next_url is not None
+
+    def test_cursor_pagination_current_page_empty_reverse(self):
+        # Regression test for #6504
+        self.pagination.base_url = "/"
+
+        # We have a cursor on the element at position 100, but this element doesn't exist
+        # anymore.
+        cursor = pagination.Cursor(reverse=True, offset=0, position=100)
+        url = self.pagination.encode_cursor(cursor)
+        self.pagination.base_url = "/"
+
+        # Loading the page with this cursor doesn't crash
+        (previous, current, next, previous_url, next_url) = self.get_pages(url)
+
+        # The previous url doesn't crash either
+        (previous, current, next, previous_url, next_url) = self.get_pages(next_url)
+
+        # And point to things that are not completely off.
+        assert previous == [7, 7, 7, 7, 8]
+        assert current == []
+        assert next is None
+        assert previous_url is not None
+        assert next_url is None
+
     def test_cursor_pagination_with_page_size(self):
         (previous, current, next, previous_url, next_url) = self.get_pages('/?page_size=20')