Fix: return next=None on last page of search results

documentcloud/documents/search.py

@@ -596,7 +596,10 @@ def _paginate_page(query_params, user):
             f"The selected `page` value of {page} is over the limit of {max_page}"
         )
     start = (page - 1) * rows
-    return {"rows": rows, "start": start}, {"page": page, "rows": rows}
+    # Request one extra row to detect whether a next page exists,
+    # but never exceed max_page_size
+    solr_rows = rows + 1 if rows < max_page_size else rows
+    return {"rows": solr_rows, "start": start}, {"page": page, "rows": rows}
 
 
 def _paginate_cursor(query_params, user):
@@ -615,19 +618,27 @@ def _paginate_cursor(query_params, user):
         CursorPagination.page_size,
         max_value=max_page_size,
     )
-    return {"rows": rows, "cursorMark": cursor}, {}
+    return {"rows": rows, "cursorMark": cursor}, {"rows": rows}
 
 
 def _format_response(results, query_params, user, escaped, page_data):
     """Emulate the Django Rest Framework response format"""
     base_url = f"{settings.DOCCLOUD_API_URL}/api/documents/search/"
     query_params = query_params.copy()
 
+    per_page = page_data["rows"]
+
     if "page" in page_data:
         page = page_data["page"]
-        per_page = page_data["rows"]
-        max_page = math.ceil(results.hits / per_page)
-        if page < max_page:
+        # When we could request per_page + 1, use the extra row to detect
+        # whether more results exist. Otherwise fall back to hit counts.
+        if len(results.docs) > per_page:
+            has_more = True
+            results.docs = results.docs[:per_page]
+        else:
+            has_more = page < math.ceil(results.hits / per_page)
+
+        if has_more:
             query_params["page"] = page + 1
             next_url = f"{base_url}?{query_params.urlencode()}"
         else:
@@ -639,6 +650,8 @@ def _format_response(results, query_params, user, escaped, page_data):
         else:
             previous_url = None
     else:
+        # For cursor pagination, Solr's nextCursorMark equality check
+        # reliably detects the last page without needing the N+1 trick.
         if query_params.get("cursor", "*") == results.nextCursorMark:
             next_url = None
         else:

documentcloud/documents/tests/test_search.py

@@ -167,6 +167,28 @@ def test_search_page(self):
         response = self.search(str(url.query))
         self.assert_documents(response["results"], slice_=slice(10, 20))
 
+    def test_search_last_page_next_is_none(self):
+        """Test that the last page of results has next=None"""
+
+        # With 11 documents and per_page=10, page 1 should have next
+        response = self.search("")
+        assert response["next"] is not None
+        assert response["count"] == 11
+
+        # Page 2 (the last page) should have next=None
+        url = furl(response["next"])
+        response = self.search(str(url.query))
+        assert response["next"] is None
+        assert len(response["results"]) == 1
+
+    def test_search_exact_page_boundary_next_is_none(self):
+        """Test that next=None when results exactly fill the page"""
+
+        # With 11 documents and per_page=11, all results fit on one page
+        response = self.search("per_page=11")
+        assert response["next"] is None
+        assert len(response["results"]) == 11
+
     def test_search_per_page(self):
         """Test fetching a different number of results per page"""