Inconsistent path behaviour with APIRouter prefix

[captaincoordinates]

The pgstac application behaves inconsistently when building response href paths if an APIRouter prefix is provided.

This repo demonstrates the problem behaviour.

The StacApi constructor supports a FastAPI APIRouter object argument. APIRouter supports a prefix string argument. For example:

api = StacApi(
    app=FastAPI(),
    settings=settings,
    extensions=extensions,
    client=CoreCrudClient(post_request_model=post_request_model),
    search_get_request_model=create_get_request_model(extensions),
    search_post_request_model=post_request_model,
    response_class=ORJSONResponse,
    middlewares=[],
    router=APIRouter(prefix="/router_prefix"),
)

The OpenAPI docs correctly include /router_prefix in all paths, but most stac-fastapi responses do not include the prefix in href paths. However, a /search response does include /router_prefix in the next and prev href paths if multiple search pages are available. See the referenced repo for steps to observe this behaviour.

The links provided by STAC API implementations are extremely valuable and place STAC APIs at Level 3 on the Richardson REST Maturity Model, however this only provides value if the paths can be trusted. I think the pgstac application, and presumably also the sqlalchemy application, would benefit from a holistic review of path handling as there are several issues identifying different problems in path logic:

• (this one)
• root_path problem
• forwarding header support: I also created this one and have an open PR to address it across both applications

[geospatial-jeff]

would benefit from a holistic review of path handling as there are several issues identifying different problems in path logic:.

I agree with this. I believe forwarding header support is in a good place now with #415 being merged and root_path is supported by both fastapi/uvicorn/gunicorn (https://fastapi.tiangolo.com/advanced/behind-a-proxy/#providing-the-root_path).

#429 has a working solution for the APIRouter prefixing issue by propagating the router prefix used by the core endpoints to the rest of the application but I don't think it addresses the main issue here which is that API routers are not used or exposed in a consistent manner throughout the project. For example:

• Health check router IS NOT exposed (https://github.com/stac-utils/stac-fastapi/blob/master/stac_fastapi/api/stac_fastapi/api/app.py#L339).
• Transactions extension router IS exposed (https://github.com/stac-utils/stac-fastapi/blob/master/stac_fastapi/extensions/stac_fastapi/extensions/core/transaction.py#L46).
• Bulk transactions extension router IS NOT exposed (https://github.com/stac-utils/stac-fastapi/blob/master/stac_fastapi/extensions/stac_fastapi/extensions/third_party/bulk_transactions.py#L119).

Organizing routers around conformance classes is the most logical approach; all the core endpoints will use one router while all the transactions endpoints will use another etc. But this needs to be implemented consistently throughout the project for it to mean anything.

[drnextgis]

I don't get your comment @geospatial-jeff.

• Health check route takes into account prefix:

  stac-fastapi/stac_fastapi/extensions/stac_fastapi/extensions/third_party/bulk_transactions.py

  Line 119 in 2a04d38

  router = APIRouter(prefix=app.state.router_prefix)

• Bulk transactions extension route takes into account prefix:

  stac-fastapi/stac_fastapi/extensions/stac_fastapi/extensions/third_party/bulk_transactions.py

  Line 119 in 2a04d38

  router = APIRouter(prefix=app.state.router_prefix)

UPD: Ahh I see you mean that for those places the router cannot be overridden by a user.

[geospatial-jeff]

UPD: Ahh I see you mean that for those places the router cannot be overridden by a user.

Yup exactly! If the user can pass their own APIRouter wherever it is appropriate prefixing the entire app (or just a part of it) becomes very simple.