Fix pagination links for Orders pagination wrt the use of overriding url_for (#124)

## What I'm changing

Currently, the pagination links for Orders and Order Statuses don't have
`next` link relation hrefs with the value returned from url_for, so
they're always relative to the server's URL (default) rather than what
the implementation has overridden url_for to do.

## How I did it

- the pagination link function calls the url_for function now, so the
url is correct

## Checklist

- [X] Tests pass: `uv run pytest`
- [X] Checks pass: `uv run pre-commit run --all-files`
- [X] CHANGELOG is updated (if necessary)

---------

Co-authored-by: Phil Varner <[email protected]>

Author: philvarner

stapi-fastapi/CHANGELOG.md

@@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
   that will be returned from pagination. When this returns `Some(int)`, the value is used for the `numberMatched` field in
   FeatureCollection returned from the /orders endpoint.  If this feature is not desired, providing a function that returns
   `Nothing` will exclude the `numberMatched` field in the response.
+- ProductRouter and RootRouter now have a method `url_for` that makes the link generation code slightly cleaner and
+  allows for overridding in child classes, to support proxy rewrite of the links.
 
 ### Removed
 

stapi-fastapi/src/stapi_fastapi/routers/base.py

@@ -0,0 +1,13 @@
+from typing import Any
+
+from fastapi import (
+    APIRouter,
+    Request,
+)
+from fastapi.datastructures import URL
+
+
+class StapiFastapiBaseRouter(APIRouter):
+    @staticmethod
+    def url_for(request: Request, name: str, /, **path_params: Any) -> URL:
+        return request.url_for(name, **path_params)

stapi-fastapi/src/stapi_fastapi/routers/product_router.py

@@ -5,7 +5,6 @@
 from typing import TYPE_CHECKING, Any
 
 from fastapi import (
-    APIRouter,
     Depends,
     Header,
     HTTPException,
@@ -38,6 +37,7 @@
 from stapi_fastapi.errors import NotFoundError, QueryablesError
 from stapi_fastapi.models.product import Product
 from stapi_fastapi.responses import GeoJSONResponse
+from stapi_fastapi.routers.base import StapiFastapiBaseRouter
 from stapi_fastapi.routers.route_names import (
     CONFORMANCE,
     CREATE_ORDER,
@@ -47,6 +47,7 @@
     GET_QUERYABLES,
     SEARCH_OPPORTUNITIES,
 )
+from stapi_fastapi.routers.utils import json_link
 
 if TYPE_CHECKING:
     from stapi_fastapi.routers import RootRouter
@@ -84,7 +85,7 @@ def build_conformances(product: Product, root_router: RootRouter) -> list[str]:
     return list(conformances)
 
 
-class ProductRouter(APIRouter):
+class ProductRouter(StapiFastapiBaseRouter):
     # FIXME ruff is complaining that the init is too complex
     def __init__(  # noqa
         self,
@@ -199,31 +200,16 @@ async def _create_order(
                 tags=["Products"],
             )
 
-    @staticmethod
-    def url_for(request: Request, name: str, /, **path_params: Any) -> str:
-        return str(request.url_for(name, **path_params))
-
     def get_product(self, request: Request) -> ProductPydantic:
         links = [
-            Link(
-                href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_PRODUCT}"),
-                rel="self",
-                type=TYPE_JSON,
-            ),
-            Link(
-                href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{CONFORMANCE}"),
-                rel="conformance",
-                type=TYPE_JSON,
+            json_link("self", self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_PRODUCT}")),
+            json_link("conformance", self.url_for(request, f"{self.root_router.name}:{self.product.id}:{CONFORMANCE}")),
+            json_link(
+                "queryables", self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_QUERYABLES}")
             ),
-            Link(
-                href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_QUERYABLES}"),
-                rel="queryables",
-                type=TYPE_JSON,
-            ),
-            Link(
-                href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_ORDER_PARAMETERS}"),
-                rel="order-parameters",
-                type=TYPE_JSON,
+            json_link(
+                "order-parameters",
+                self.url_for(request, f"{self.root_router.name}:{self.product.id}:{GET_ORDER_PARAMETERS}"),
             ),
             Link(
                 href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{CREATE_ORDER}"),
@@ -237,10 +223,9 @@ def get_product(self, request: Request) -> ProductPydantic:
             self.product.supports_async_opportunity_search and self.root_router.supports_async_opportunity_search
         ):
             links.append(
-                Link(
-                    href=self.url_for(request, f"{self.root_router.name}:{self.product.id}:{SEARCH_OPPORTUNITIES}"),
-                    rel="opportunities",
-                    type=TYPE_JSON,
+                json_link(
+                    "opportunities",
+                    self.url_for(request, f"{self.root_router.name}:{self.product.id}:{SEARCH_OPPORTUNITIES}"),
                 ),
             )
 
@@ -411,7 +396,7 @@ def pagination_link(self, request: Request, opp_req: OpportunityPayload, paginat
         body = opp_req.body()
         body["next"] = pagination_token
         return Link(
-            href=str(request.url),
+            href=request.url,
             rel="next",
             type=TYPE_JSON,
             method="POST",
@@ -431,14 +416,13 @@ async def get_opportunity_collection(
         ):
             case Success(Some(opportunity_collection)):
                 opportunity_collection.links.append(
-                    Link(
-                        href=self.url_for(
+                    json_link(
+                        "self",
+                        self.url_for(
                             request,
                             f"{self.root_router.name}:{self.product.id}:{GET_OPPORTUNITY_COLLECTION}",
                             opportunity_collection_id=opportunity_collection_id,
                         ),
-                        rel="self",
-                        type=TYPE_JSON,
                     ),
                 )
                 return opportunity_collection  # type: ignore

stapi-fastapi/src/stapi_fastapi/routers/root_router.py

@@ -2,7 +2,8 @@
 import traceback
 from typing import Any
 
-from fastapi import APIRouter, HTTPException, Request, status
+from fastapi import HTTPException, Request, status
+from fastapi.datastructures import URL
 from returns.maybe import Maybe, Some
 from returns.result import Failure, Success
 from stapi_pydantic import (
@@ -28,10 +29,11 @@
     GetOrderStatuses,
 )
 from stapi_fastapi.conformance import API as API_CONFORMANCE
-from stapi_fastapi.constants import TYPE_GEOJSON, TYPE_JSON
+from stapi_fastapi.constants import TYPE_GEOJSON
 from stapi_fastapi.errors import NotFoundError
 from stapi_fastapi.models.product import Product
 from stapi_fastapi.responses import GeoJSONResponse
+from stapi_fastapi.routers.base import StapiFastapiBaseRouter
 from stapi_fastapi.routers.product_router import ProductRouter
 from stapi_fastapi.routers.route_names import (
     CONFORMANCE,
@@ -44,11 +46,12 @@
     LIST_PRODUCTS,
     ROOT,
 )
+from stapi_fastapi.routers.utils import json_link
 
 logger = logging.getLogger(__name__)
 
 
-class RootRouter(APIRouter):
+class RootRouter(StapiFastapiBaseRouter):
     def __init__(
         self,
         get_orders: GetOrders,
@@ -170,50 +173,35 @@ def __init__(
 
         self.conformances = list(_conformances)
 
-    @staticmethod
-    def url_for(request: Request, name: str, /, **path_params: Any) -> str:
-        return str(request.url_for(name, **path_params))
-
     def get_root(self, request: Request) -> RootResponse:
         links = [
-            Link(
-                href=self.url_for(request, f"{self.name}:{ROOT}"),
-                rel="self",
-                type=TYPE_JSON,
+            json_link(
+                "self",
+                self.url_for(request, f"{self.name}:{ROOT}"),
             ),
-            Link(
-                href=self.url_for(request, self.openapi_endpoint_name),
-                rel="service-description",
-                type=TYPE_JSON,
+            json_link(
+                "service-description",
+                self.url_for(request, self.openapi_endpoint_name),
             ),
             Link(
-                href=self.url_for(request, self.docs_endpoint_name),
                 rel="service-docs",
+                href=self.url_for(request, self.docs_endpoint_name),
                 type="text/html",
             ),
+            json_link("conformance", href=self.url_for(request, f"{self.name}:{CONFORMANCE}")),
+            json_link("products", self.url_for(request, f"{self.name}:{LIST_PRODUCTS}")),
             Link(
-                href=self.url_for(request, f"{self.name}:{CONFORMANCE}"),
-                rel="conformance",
-                type=TYPE_JSON,
-            ),
-            Link(
-                href=self.url_for(request, f"{self.name}:{LIST_PRODUCTS}"),
-                rel="products",
-                type=TYPE_JSON,
-            ),
-            Link(
-                href=self.url_for(request, f"{self.name}:{LIST_ORDERS}"),
                 rel="orders",
+                href=self.url_for(request, f"{self.name}:{LIST_ORDERS}"),
                 type=TYPE_GEOJSON,
             ),
         ]
 
         if self.supports_async_opportunity_search:
             links.append(
-                Link(
-                    href=self.url_for(request, f"{self.name}:{LIST_OPPORTUNITY_SEARCH_RECORDS}"),
-                    rel="opportunity-search-records",
-                    type=TYPE_JSON,
+                json_link(
+                    "opportunity-search-records",
+                    self.url_for(request, f"{self.name}:{LIST_OPPORTUNITY_SEARCH_RECORDS}"),
                 ),
             )
 
@@ -238,14 +226,13 @@ def get_products(self, request: Request, next: str | None = None, limit: int = 1
         end = start + limit
         ids = self.product_ids[start:end]
         links = [
-            Link(
-                href=self.url_for(request, f"{self.name}:{LIST_PRODUCTS}"),
-                rel="self",
-                type=TYPE_JSON,
+            json_link(
+                "self",
+                self.url_for(request, f"{self.name}:{LIST_PRODUCTS}"),
             ),
         ]
         if end > 0 and end < len(self.product_ids):
-            links.append(self.pagination_link(request, self.product_ids[end], limit))
+            links.append(self.pagination_link(request, f"{self.name}:{LIST_PRODUCTS}", self.product_ids[end], limit))
         return ProductsCollection(
             products=[self.product_routers[product_id].get_product(request) for product_id in ids],
             links=links,
@@ -261,8 +248,8 @@ async def get_orders(  # noqa: C901
                 for order in orders:
                     order.links.extend(self.order_links(order, request))
                 match maybe_pagination_token:
-                    case Some(x):
-                        links.append(self.pagination_link(request, x, limit))
+                    case Some(next_):
+                        links.append(self.pagination_link(request, f"{self.name}:{LIST_ORDERS}", next_, limit))
                     case Maybe.empty:
                         pass
                 match maybe_orders_count:
@@ -325,8 +312,12 @@ async def get_order_statuses(
             case Success(Some((statuses, maybe_pagination_token))):
                 links.append(self.order_statuses_link(request, order_id))
                 match maybe_pagination_token:
-                    case Some(x):
-                        links.append(self.pagination_link(request, x, limit))
+                    case Some(next_):
+                        links.append(
+                            self.pagination_link(
+                                request, f"{self.name}:{LIST_ORDER_STATUSES}", next_, limit, order_id=order_id
+                            )
+                        )
                     case Maybe.empty:
                         pass
             case Success(Maybe.empty):
@@ -353,10 +344,10 @@ def add_product(self, product: Product, *args: Any, **kwargs: Any) -> None:
         self.product_routers[product.id] = product_router
         self.product_ids = [*self.product_routers.keys()]
 
-    def generate_order_href(self, request: Request, order_id: str) -> str:
+    def generate_order_href(self, request: Request, order_id: str) -> URL:
         return self.url_for(request, f"{self.name}:{GET_ORDER}", order_id=order_id)
 
-    def generate_order_statuses_href(self, request: Request, order_id: str) -> str:
+    def generate_order_statuses_href(self, request: Request, order_id: str) -> URL:
         return self.url_for(request, f"{self.name}:{LIST_ORDER_STATUSES}", order_id=order_id)
 
     def order_links(self, order: Order[OrderStatus], request: Request) -> list[Link]:
@@ -366,33 +357,19 @@ def order_links(self, order: Order[OrderStatus], request: Request) -> list[Link]
                 rel="self",
                 type=TYPE_GEOJSON,
             ),
-            Link(
-                href=self.generate_order_statuses_href(request, order.id),
-                rel="monitor",
-                type=TYPE_JSON,
+            json_link(
+                "monitor",
+                self.generate_order_statuses_href(request, order.id),
             ),
         ]
 
     def order_statuses_link(self, request: Request, order_id: str) -> Link:
-        return Link(
-            href=self.url_for(
-                request,
-                f"{self.name}:{LIST_ORDER_STATUSES}",
-                order_id=order_id,
-            ),
-            rel="self",
-            type=TYPE_JSON,
-        )
-
-    def pagination_link(self, request: Request, pagination_token: str, limit: int) -> Link:
-        href = str(request.url.include_query_params(next=pagination_token, limit=limit)).replace(
-            str(request.url_for(f"{self.name}:{ROOT}")), self.url_for(request, f"{self.name}:{ROOT}"), 1
-        )
+        return json_link("self", self.url_for(request, f"{self.name}:{LIST_ORDER_STATUSES}", order_id=order_id))
 
-        return Link(
-            href=href,
-            rel="next",
-            type=TYPE_JSON,
+    def pagination_link(self, request: Request, name: str, pagination_token: str, limit: int, **kwargs: Any) -> Link:
+        return json_link(
+            "next",
+            self.url_for(request, name, **kwargs).include_query_params(next=pagination_token, limit=limit),
         )
 
     async def get_opportunity_search_records(
@@ -404,8 +381,12 @@ async def get_opportunity_search_records(
                 for record in records:
                     record.links.append(self.opportunity_search_record_self_link(record, request))
                 match maybe_pagination_token:
-                    case Some(x):
-                        links.append(self.pagination_link(request, x, limit))
+                    case Some(next_):
+                        links.append(
+                            self.pagination_link(
+                                request, f"{self.name}:{LIST_OPPORTUNITY_SEARCH_RECORDS}", next_, limit
+                            )
+                        )
                     case Maybe.empty:
                         pass
             case Failure(ValueError()):
@@ -470,7 +451,7 @@ async def get_opportunity_search_record_statuses(
             case _:
                 raise AssertionError("Expected code to be unreachable")
 
-    def generate_opportunity_search_record_href(self, request: Request, search_record_id: str) -> str:
+    def generate_opportunity_search_record_href(self, request: Request, search_record_id: str) -> URL:
         return self.url_for(
             request,
             f"{self.name}:{GET_OPPORTUNITY_SEARCH_RECORD}",
@@ -480,11 +461,7 @@ def generate_opportunity_search_record_href(self, request: Request, search_recor
     def opportunity_search_record_self_link(
         self, opportunity_search_record: OpportunitySearchRecord, request: Request
     ) -> Link:
-        return Link(
-            href=self.generate_opportunity_search_record_href(request, opportunity_search_record.id),
-            rel="self",
-            type=TYPE_JSON,
-        )
+        return json_link("self", self.generate_opportunity_search_record_href(request, opportunity_search_record.id))
 
     @property
     def _get_order_statuses(self) -> GetOrderStatuses:  # type: ignore

stapi-fastapi/src/stapi_fastapi/routers/utils.py

@@ -0,0 +1,8 @@
+from fastapi.datastructures import URL
+from stapi_pydantic import Link
+
+from stapi_fastapi.constants import TYPE_JSON
+
+
+def json_link(rel: str, href: URL) -> Link:
+    return Link(href=href, rel=rel, type=TYPE_JSON)

stapi-pydantic/CHANGELOG.md

@@ -12,10 +12,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 - pydantic >= 2.12 is now required.
 
-### Added
+### Changed
 
-- ProductRouter and RootRouter now have a method `url_for` that makes the link generation code slightly cleaner and
-  allows for overridding in child classes, to support proxy rewrite of the links.
+- pydantic >= 2.12 is now required.
 
 ## [0.0.4] - 2025-07-17