Issue #332: improve next page handling

- eliminate support for old pagination format
- fix "no next page" use case

openeo_driver/_version.py

@@ -1 +1 @@
-__version__ = "0.120.1a1"
+__version__ = "0.120.2a1"

openeo_driver/jobregistry.py

@@ -628,30 +628,31 @@ def _search_paginated(
         # Response structure:
         #   {
         #       "jobs": [list of job docs],
-        #       "pagination': {"previous": "size=5&page=1", "next": "size=5&page=3"}
+        #       "pagination": {"previous": {"size": 5, "page": 6}, "next": {"size": 5, "page": 8}}}
         #    }
         next_page_number = self._parse_response_pagination(
-            pagination=response.get("pagination", {}).get("next"),
+            pagination=response.get("pagination"),
             expected_size=page_size,
         )
-        self.logger.debug(f"Parsed {next_page_number=} from {response.get('pagination')=}")
         return self.PaginatedSearchResult(
             jobs=response.get("jobs", []),
             next_page=next_page_number,
         )
 
-    @staticmethod
-    def _parse_response_pagination(pagination: dict, expected_size: int) -> Union[int, None]:
+    def _parse_response_pagination(self, pagination: Union[dict, None], expected_size: int) -> Union[int, None]:
         """Extract page number from pagination construct"""
-        if isinstance(pagination, str):
-            # TODO #332 get rid of this legacy format translation code path
-            params = urllib.parse.parse_qs(pagination)
-            if "size" in params and "page" in params:
-                pagination = {"size": int(params["size"][0]), "page": int(params["page"][0])}
-        if "size" in pagination and "page" in pagination:
-            if int(pagination["size"]) != expected_size:
-                raise ValueError(f"Page size mismatch {expected_size=} vs {pagination=}")
-            return int(pagination["page"])
+        next_page_params = (pagination or {}).get("next")
+        if (
+            isinstance(next_page_params, dict)
+            and "size" in next_page_params
+            and next_page_params["size"] == expected_size
+            and "page" in next_page_params
+        ):
+            next_page_number = next_page_params["page"]
+        else:
+            next_page_number = None
+        self.logger.debug(f"_search_paginated: parsed {next_page_number=} from {pagination=}")
+        return next_page_number
 
     def list_user_jobs(
         self,
@@ -675,15 +676,17 @@ def list_user_jobs(
             # Do paginated search
             # TODO #332 make this the one and only code path
             page_number = (request_parameters or {}).get(self.PAGINATION_URL_PARAM)
-            if page_number:
+            if page_number is not None:
                 page_number = int(page_number)
-            else:
-                page_number = None
+
             data = self._search_paginated(query=query, fields=fields, page_size=limit, page_number=page_number)
-            return JobListing(
-                jobs=[ejr_job_info_to_metadata(j, full=False) for j in data.jobs],
-                next_parameters={"limit": limit, self.PAGINATION_URL_PARAM: data.next_page},
-            )
+
+            jobs = [ejr_job_info_to_metadata(j, full=False) for j in data.jobs]
+            if data.next_page:
+                next_parameters = {"limit": limit, self.PAGINATION_URL_PARAM: data.next_page}
+            else:
+                next_parameters = None
+            return JobListing(jobs=jobs, next_parameters=next_parameters)
         else:
             # Deprecated non-paginated search
             # TODO #332 eliminate this code path

tests/test_jobregistry.py

@@ -9,6 +9,7 @@
 import requests_mock
 import time_machine
 from openeo.rest.auth.testing import OidcMock
+from openeo.util import dict_no_none
 
 from openeo_driver.backend import JobListing
 from openeo_driver.constants import JOB_STATUS
@@ -528,14 +529,14 @@ def post_jobs_search(request, context):
 
 
     def _build_url(self, params: dict):
-        return "https://oeo.test/jobs?" + urllib.parse.urlencode(query=params)
+        return "https://oeo.test/jobs?" + urllib.parse.urlencode(query=dict_no_none(params))
 
     @pytest.mark.parametrize(
         [
             "limit",
             "request_parameters",
             "expected_query_string",
-            "legacy_pagination",
+            "pagination_response",
             "expected_next_url",
             "expected_jobs",
         ],
@@ -544,39 +545,47 @@ def _build_url(self, params: dict):
                 3,
                 None,
                 {"size": ["3"]},
-                True,
-                "https://oeo.test/jobs?limit=3&page=1",
-                ["j-1", "j-2", "j-3"],
-            ),
-            (
-                3,
-                None,
-                {"size": ["3"]},
-                False,
+                "auto",
                 "https://oeo.test/jobs?limit=3&page=1",
                 ["j-1", "j-2", "j-3"],
             ),
             (
                 3,
                 {ElasticJobRegistry.PAGINATION_URL_PARAM: "1"},
                 {"size": ["3"], "page": ["1"]},
-                True,
+                "auto",
                 "https://oeo.test/jobs?limit=3&page=2",
                 ["j-4", "j-5", "j-6"],
             ),
             (
                 3,
-                {ElasticJobRegistry.PAGINATION_URL_PARAM: "1"},
-                {"size": ["3"], "page": ["1"]},
-                False,
-                "https://oeo.test/jobs?limit=3&page=2",
-                ["j-4", "j-5", "j-6"],
+                {ElasticJobRegistry.PAGINATION_URL_PARAM: "10"},
+                {"size": ["3"], "page": ["10"]},
+                None,
+                None,
+                ["j-31", "j-32", "j-33"],
+            ),
+            (
+                3,
+                {ElasticJobRegistry.PAGINATION_URL_PARAM: "10"},
+                {"size": ["3"], "page": ["10"]},
+                {},
+                None,
+                ["j-31", "j-32", "j-33"],
+            ),
+            (
+                3,
+                {ElasticJobRegistry.PAGINATION_URL_PARAM: "10"},
+                {"size": ["3"], "page": ["10"]},
+                {"invalid": "stuff"},
+                None,
+                ["j-31", "j-32", "j-33"],
             ),
             (
                 5,
                 {ElasticJobRegistry.PAGINATION_URL_PARAM: "111"},
                 {"size": ["5"], "page": ["111"]},
-                False,
+                "auto",
                 "https://oeo.test/jobs?limit=5&page=112",
                 ["j-556", "j-557", "j-558", "j-559", "j-560"],
             ),
@@ -590,7 +599,7 @@ def test_list_user_jobs_paginated(
         limit,
         request_parameters,
         expected_query_string,
-        legacy_pagination,
+        pagination_response,
         expected_next_url,
         expected_jobs,
     ):
@@ -611,29 +620,30 @@ def post_jobs_search_paginated(request, context):
                 },
                 "_source": ListSubSet(["job_id", "user_id", "status"]),
             }
-            if legacy_pagination:
-                pagination = {"next": f"size={limit}&page={next_page}"}
-            else:
-                pagination = {"next": {"size": limit, "page": next_page}}
+            nonlocal pagination_response
+            if pagination_response == "auto":
+                pagination_response = {"next": {"size": limit, "page": next_page}}
             return {
                 "jobs": [
                     {"job_id": f"j-{j}", "status": "created", "created": f"2024-12-06T12:00:00Z"}
                     for j in range(1 + this_page * limit, 1 + next_page * limit)
                 ],
-                "pagination": pagination,
+                "pagination": pagination_response,
             }
 
         requests_mock.post(f"{self.EJR_API_URL}/jobs/search/paginated", json=post_jobs_search_paginated)
 
         listing = ejr.list_user_jobs(user_id="john", limit=limit, request_parameters=request_parameters)
         assert isinstance(listing, JobListing)
+        if expected_next_url:
+            expected_links = [{"href": expected_next_url, "rel": "next"}]
+        else:
+            expected_links = []
         assert listing.to_response_dict(build_url=self._build_url) == {
             "jobs": [
                 {"id": j, "status": "created", "created": "2024-12-06T12:00:00Z", "progress": 0} for j in expected_jobs
             ],
-            "links": [
-                {"href": expected_next_url, "rel": "next"},
-            ],
+            "links": expected_links,
         }
 
     @pytest.mark.parametrize(