Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add paging links tests #195

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions stac_fastapi/pgstac/models/links.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def link_next(self) -> Optional[Dict[str, Any]]:
"rel": Relations.next,
"type": MimeTypes.geojson,
"method": method,
"href": f"{self.request.url}",
"href": str(self.request.url),
"body": {**self.request.postbody, "token": f"next:{self.next}"},
}

Expand All @@ -167,7 +167,7 @@ def link_prev(self) -> Optional[Dict[str, Any]]:
"rel": Relations.previous,
"type": MimeTypes.geojson,
"method": method,
"href": f"{self.request.url}",
"href": str(self.request.url),
"body": {**self.request.postbody, "token": f"prev:{self.prev}"},
}
return None
Expand Down
94 changes: 94 additions & 0 deletions tests/api/test_links.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import pytest
from fastapi import APIRouter, FastAPI
from starlette.requests import Request
from starlette.testclient import TestClient

from stac_fastapi.pgstac.models import links as app_links


@pytest.mark.parametrize("root_path", ["", "/api/v1"])
@pytest.mark.parametrize("prefix", ["", "/stac"])
def tests_app_links(prefix, root_path):
endpoint_prefix = root_path + prefix
url_prefix = "http://stac.io" + endpoint_prefix
Comment on lines +12 to +13

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #201 , but this should also work:

Suggested change
endpoint_prefix = root_path + prefix
url_prefix = "http://stac.io" + endpoint_prefix
endpoint_prefix = prefix
url_prefix = "http://stac.io" + root_path + endpoint_prefix

we want to model situation when root_path is dropped by the reverse proxy, but needs to be present in the output links as this is the only way to reach the API.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'm not sure to get this to be honest

In all cases, the client should be called {root-path}/{prefix}/endpoint as shown in https://github.com/encode/starlette/blob/a7d0b14c7378aa2e95b0b13583fe0dadace363be/tests/test_routing.py#L573-L575

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I'm not sure to get this to be honest

In all cases, the client should be called {root-path}/{prefix}/endpoint as shown in https://github.com/encode/starlette/blob/a7d0b14c7378aa2e95b0b13583fe0dadace363be/tests/test_routing.py#L573-L575

I don't understand how you come to this conclusion from that one test alone. I'm just trying to replicate the scenario I see in the debugger of a real system. Omitting the root path in the call to the test client still routes to the endpoint in question, but this time with the request object behaving the same way as in the real system behind a reverse proxy that removes routing prefix.


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

@router.get("/search")
@router.post("/search")
async def search(request: Request):
links = app_links.PagingLinks(request, next="yo:2", prev="yo:1")
return {
"url": links.url,
"base_url": links.base_url,
"links": await links.get_links(),
}

@router.get("/collections")
async def collections(request: Request):
pgstac_next = {
"rel": "next",
"body": {"offset": 1},
"href": "./collections",
"type": "application/json",
"merge": True,
"method": "GET",
}
pgstac_prev = {
"rel": "prev",
"body": {"offset": 0},
"href": "./collections",
"type": "application/json",
"merge": True,
"method": "GET",
}
links = app_links.CollectionSearchPagingLinks(
request, next=pgstac_next, prev=pgstac_prev
)
return {
"url": links.url,
"base_url": links.base_url,
"links": await links.get_links(),
}

app.include_router(router)

with TestClient(
app,
base_url="http://stac.io",
root_path=root_path,
) as client:
response = client.get(f"{endpoint_prefix}/search")
assert response.status_code == 200
assert response.json()["url"] == url_prefix + "/search"
assert response.json()["base_url"].rstrip("/") == url_prefix
links = response.json()["links"]
for link in links:
if link["rel"] in ["previous", "next"]:
assert link["method"] == "GET"
assert link["href"].startswith(url_prefix)
assert {"next", "previous", "root", "self"} == {link["rel"] for link in links}

response = client.post(f"{endpoint_prefix}/search", json={})
assert response.status_code == 200
assert response.json()["url"] == url_prefix + "/search"
assert response.json()["base_url"].rstrip("/") == url_prefix
links = response.json()["links"]
for link in links:
if link["rel"] in ["previous", "next"]:
assert link["method"] == "POST"
assert link["href"].startswith(url_prefix)
assert {"next", "previous", "root", "self"} == {link["rel"] for link in links}

response = client.get(f"{endpoint_prefix}/collections")
assert response.status_code == 200
assert response.json()["url"] == url_prefix + "/collections"
assert response.json()["base_url"].rstrip("/") == url_prefix
links = response.json()["links"]
for link in links:
if link["rel"] in ["previous", "next"]:
assert link["method"] == "GET"
assert link["href"].startswith(url_prefix)
assert {"next", "previous", "root", "self"} == {link["rel"] for link in links}
Loading