From 98d6488ca213b380c80757da8306231aee58878b Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 21:38:46 -0700 Subject: [PATCH 1/8] fix: remove root path prefix when root_path is set on app --- stac_fastapi/pgstac/models/links.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/stac_fastapi/pgstac/models/links.py b/stac_fastapi/pgstac/models/links.py index ae7e3cf..31bf920 100644 --- a/stac_fastapi/pgstac/models/links.py +++ b/stac_fastapi/pgstac/models/links.py @@ -65,9 +65,7 @@ def url(self): # We need to remove the root path prefix from the path before # joining the base_url and path to get the full url to avoid # having root_path twice in the url - if ( - root_path := self.request.scope.get("root_path") - ) and not self.request.app.root_path: + if root_path := self.request.scope.get("root_path"): # self.request.app.root_path is set by FastAPI when running with FastAPI(root_path="...") # If self.request.app.root_path is not set but self.request.scope.get("root_path") is set, # then the root path is set by uvicorn From 92bd06f1c72e7606e85dbcf3e3a0ba62b5d78e0a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 22:45:52 -0700 Subject: [PATCH 2/8] Add tests from #265 --- tests/api/test_links_with_root_path.py | 140 +++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 tests/api/test_links_with_root_path.py diff --git a/tests/api/test_links_with_root_path.py b/tests/api/test_links_with_root_path.py new file mode 100644 index 0000000..0c0d5d6 --- /dev/null +++ b/tests/api/test_links_with_root_path.py @@ -0,0 +1,140 @@ +import pytest +from httpx import ASGITransport, AsyncClient + +from stac_fastapi.pgstac.db import close_db_connection, connect_to_db + +root_path_value = "/stac/v1" + +# Append the root path to the base URL, this is key to reproducing the issue where the root path appears twice in some links +base_url = f"http://api.acme.com{root_path_value}" + + +@pytest.fixture(scope="function") +async def app_with_root_path(database, monkeypatch): + """ + Provides the global stac_fastapi.pgstac.app.app instance, configured with a + specific ROOT_PATH environment variable and connected to the test database. + """ + + monkeypatch.setenv("ROOT_PATH", root_path_value) + monkeypatch.setenv("PGUSER", database.user) + monkeypatch.setenv("PGPASSWORD", database.password) + monkeypatch.setenv("PGHOST", database.host) + monkeypatch.setenv("PGPORT", str(database.port)) + monkeypatch.setenv("PGDATABASE", database.dbname) + monkeypatch.setenv("ENABLE_TRANSACTIONS_EXTENSIONS", "TRUE") + + from stac_fastapi.pgstac.app import app, with_transactions + + # Ensure the app's root_path is configured as expected + assert ( + app.root_path == root_path_value + ), f"app_with_root_path fixture: app.root_path is '{app.root_path}', expected '{root_path_value}'" + + await connect_to_db(app, add_write_connection_pool=with_transactions) + yield app + await close_db_connection(app) + + +@pytest.fixture(scope="function") +async def client_with_root_path(app_with_root_path): + async with AsyncClient( + transport=ASGITransport(app=app_with_root_path), + base_url=base_url, + ) as c: + yield c + + +@pytest.fixture(scope="function") +async def loaded_client(client_with_root_path, load_test_data): + col = load_test_data("test_collection.json") + resp = await client_with_root_path.post( + "/collections", + json=col, + ) + assert resp.status_code == 201 + item = load_test_data("test_item.json") + resp = await client_with_root_path.post( + f"/collections/{col['id']}/items", + json=item, + ) + assert resp.status_code == 201 + item = load_test_data("test_item2.json") + resp = await client_with_root_path.post( + f"/collections/{col['id']}/items", + json=item, + ) + assert resp.status_code == 201 + yield client_with_root_path + + +async def test_search_links_are_valid(loaded_client): + resp = await loaded_client.get("/search?limit=1") + assert resp.status_code == 200 + response_json = resp.json() + assert_links_href(response_json.get("links", []), base_url) + + +async def test_collection_links_are_valid(loaded_client): + resp = await loaded_client.get("/collections?limit=1") + assert resp.status_code == 200 + response_json = resp.json() + assert_links_href(response_json.get("links", []), base_url) + + +async def test_items_collection_links_are_valid(loaded_client): + resp = await loaded_client.get("/collections/test-collection/items?limit=1") + assert resp.status_code == 200 + response_json = resp.json() + assert_links_href(response_json.get("links", []), base_url) + + +def assert_links_href(links, url_prefix): + """ + Ensure all links start with the expected URL prefix and check that + there is no root_path duplicated in the URL. + + Args: + links: List of link dictionaries with 'href' keys + url_prefix: Expected URL prefix (e.g., 'http://test/stac/v1') + """ + from urllib.parse import urlparse + + failed_links = [] + parsed_prefix = urlparse(url_prefix) + root_path = parsed_prefix.path # e.g., '/stac/v1' + + for link in links: + href = link["href"] + rel = link.get("rel", "unknown") + + # Check if link starts with the expected prefix + if not href.startswith(url_prefix): + failed_links.append( + { + "rel": rel, + "href": href, + "error": f"does not start with expected prefix '{url_prefix}'", + } + ) + continue + + # Check for duplicated root path + if root_path and root_path != "/": + remainder = href[len(url_prefix) :] + if remainder.startswith(root_path): + failed_links.append( + { + "rel": rel, + "href": href, + "error": f"contains duplicated root path '{root_path}'", + } + ) + + # If there are failed links, create a detailed error report + if failed_links: + error_report = "Link validation failed:\n" + for failed_link in failed_links: + error_report += f" - rel: '{failed_link['rel']}', href: '{failed_link['href']}' - {failed_link['error']}\n" + + raise AssertionError(error_report) From b345f6f27e43c53b0f08c278fbfcdde6401d2200 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 22:49:51 -0700 Subject: [PATCH 3/8] Revert "fix: remove root path prefix when root_path is set on app" This reverts commit 98d6488ca213b380c80757da8306231aee58878b. --- stac_fastapi/pgstac/models/links.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/stac_fastapi/pgstac/models/links.py b/stac_fastapi/pgstac/models/links.py index 31bf920..ae7e3cf 100644 --- a/stac_fastapi/pgstac/models/links.py +++ b/stac_fastapi/pgstac/models/links.py @@ -65,7 +65,9 @@ def url(self): # We need to remove the root path prefix from the path before # joining the base_url and path to get the full url to avoid # having root_path twice in the url - if root_path := self.request.scope.get("root_path"): + if ( + root_path := self.request.scope.get("root_path") + ) and not self.request.app.root_path: # self.request.app.root_path is set by FastAPI when running with FastAPI(root_path="...") # If self.request.app.root_path is not set but self.request.scope.get("root_path") is set, # then the root path is set by uvicorn From fa5e01054be94780dea1e58b55dc42d118e49972 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 23:01:22 -0700 Subject: [PATCH 4/8] Reload app after setting env vars --- tests/api/test_links_with_root_path.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/api/test_links_with_root_path.py b/tests/api/test_links_with_root_path.py index 0c0d5d6..46e1803 100644 --- a/tests/api/test_links_with_root_path.py +++ b/tests/api/test_links_with_root_path.py @@ -1,3 +1,5 @@ +import importlib + import pytest from httpx import ASGITransport, AsyncClient @@ -24,6 +26,11 @@ async def app_with_root_path(database, monkeypatch): monkeypatch.setenv("PGDATABASE", database.dbname) monkeypatch.setenv("ENABLE_TRANSACTIONS_EXTENSIONS", "TRUE") + # Reload the app module to pick up the new environment variables + import stac_fastapi.pgstac.app + + importlib.reload(stac_fastapi.pgstac.app) + from stac_fastapi.pgstac.app import app, with_transactions # Ensure the app's root_path is configured as expected From 938b598183c8cd7da395a933747bfe307a755ac7 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 23:08:04 -0700 Subject: [PATCH 5/8] Revert "Revert "fix: remove root path prefix when root_path is set on app"" This reverts commit b345f6f27e43c53b0f08c278fbfcdde6401d2200. --- stac_fastapi/pgstac/models/links.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/stac_fastapi/pgstac/models/links.py b/stac_fastapi/pgstac/models/links.py index ae7e3cf..31bf920 100644 --- a/stac_fastapi/pgstac/models/links.py +++ b/stac_fastapi/pgstac/models/links.py @@ -65,9 +65,7 @@ def url(self): # We need to remove the root path prefix from the path before # joining the base_url and path to get the full url to avoid # having root_path twice in the url - if ( - root_path := self.request.scope.get("root_path") - ) and not self.request.app.root_path: + if root_path := self.request.scope.get("root_path"): # self.request.app.root_path is set by FastAPI when running with FastAPI(root_path="...") # If self.request.app.root_path is not set but self.request.scope.get("root_path") is set, # then the root path is set by uvicorn From b466a3b42957b05458ec780e46c109521d50b4b0 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Sat, 19 Jul 2025 23:43:49 -0700 Subject: [PATCH 6/8] refactor: use starlette.testclient.TestClient, simplify tests via parametrize --- tests/api/test_links_with_root_path.py | 100 ++++++++++--------------- 1 file changed, 40 insertions(+), 60 deletions(-) diff --git a/tests/api/test_links_with_root_path.py b/tests/api/test_links_with_root_path.py index 46e1803..3418ab9 100644 --- a/tests/api/test_links_with_root_path.py +++ b/tests/api/test_links_with_root_path.py @@ -1,14 +1,12 @@ import importlib import pytest -from httpx import ASGITransport, AsyncClient +from starlette.testclient import TestClient from stac_fastapi.pgstac.db import close_db_connection, connect_to_db -root_path_value = "/stac/v1" - -# Append the root path to the base URL, this is key to reproducing the issue where the root path appears twice in some links -base_url = f"http://api.acme.com{root_path_value}" +BASE_URL = "http://api.acme.com" +ROOT_PATH = "/stac/v1" @pytest.fixture(scope="function") @@ -18,7 +16,7 @@ async def app_with_root_path(database, monkeypatch): specific ROOT_PATH environment variable and connected to the test database. """ - monkeypatch.setenv("ROOT_PATH", root_path_value) + monkeypatch.setenv("ROOT_PATH", ROOT_PATH) monkeypatch.setenv("PGUSER", database.user) monkeypatch.setenv("PGPASSWORD", database.password) monkeypatch.setenv("PGHOST", database.host) @@ -35,8 +33,8 @@ async def app_with_root_path(database, monkeypatch): # Ensure the app's root_path is configured as expected assert ( - app.root_path == root_path_value - ), f"app_with_root_path fixture: app.root_path is '{app.root_path}', expected '{root_path_value}'" + app.root_path == ROOT_PATH + ), f"app_with_root_path fixture: app.root_path is '{app.root_path}', expected '{ROOT_PATH}'" await connect_to_db(app, add_write_connection_pool=with_transactions) yield app @@ -44,30 +42,31 @@ async def app_with_root_path(database, monkeypatch): @pytest.fixture(scope="function") -async def client_with_root_path(app_with_root_path): - async with AsyncClient( - transport=ASGITransport(app=app_with_root_path), - base_url=base_url, +def client_with_root_path(app_with_root_path): + with TestClient( + app_with_root_path, + base_url=BASE_URL, + root_path=ROOT_PATH, ) as c: yield c @pytest.fixture(scope="function") -async def loaded_client(client_with_root_path, load_test_data): +def loaded_client(client_with_root_path, load_test_data): col = load_test_data("test_collection.json") - resp = await client_with_root_path.post( + resp = client_with_root_path.post( "/collections", json=col, ) assert resp.status_code == 201 item = load_test_data("test_item.json") - resp = await client_with_root_path.post( + resp = client_with_root_path.post( f"/collections/{col['id']}/items", json=item, ) assert resp.status_code == 201 item = load_test_data("test_item2.json") - resp = await client_with_root_path.post( + resp = client_with_root_path.post( f"/collections/{col['id']}/items", json=item, ) @@ -75,68 +74,49 @@ async def loaded_client(client_with_root_path, load_test_data): yield client_with_root_path -async def test_search_links_are_valid(loaded_client): - resp = await loaded_client.get("/search?limit=1") - assert resp.status_code == 200 - response_json = resp.json() - assert_links_href(response_json.get("links", []), base_url) - - -async def test_collection_links_are_valid(loaded_client): - resp = await loaded_client.get("/collections?limit=1") +@pytest.mark.parametrize( + "path", + [ + "/search?limit=1", + "/collections?limit=1", + "/collections/test-collection/items?limit=1", + ], +) +def test_search_links_are_valid(loaded_client, path): + resp = loaded_client.get(path) assert resp.status_code == 200 response_json = resp.json() - assert_links_href(response_json.get("links", []), base_url) - - -async def test_items_collection_links_are_valid(loaded_client): - resp = await loaded_client.get("/collections/test-collection/items?limit=1") - assert resp.status_code == 200 - response_json = resp.json() - assert_links_href(response_json.get("links", []), base_url) - - -def assert_links_href(links, url_prefix): - """ - Ensure all links start with the expected URL prefix and check that - there is no root_path duplicated in the URL. - - Args: - links: List of link dictionaries with 'href' keys - url_prefix: Expected URL prefix (e.g., 'http://test/stac/v1') - """ - from urllib.parse import urlparse + # Ensure all links start with the expected URL prefix and check that + # there is no root_path duplicated in the URL. failed_links = [] - parsed_prefix = urlparse(url_prefix) - root_path = parsed_prefix.path # e.g., '/stac/v1' + expected_prefix = f"{BASE_URL}{ROOT_PATH}" - for link in links: + for link in response_json.get("links", []): href = link["href"] rel = link.get("rel", "unknown") # Check if link starts with the expected prefix - if not href.startswith(url_prefix): + if not href.startswith(expected_prefix): failed_links.append( { "rel": rel, "href": href, - "error": f"does not start with expected prefix '{url_prefix}'", + "error": f"does not start with expected prefix '{expected_prefix}'", } ) continue # Check for duplicated root path - if root_path and root_path != "/": - remainder = href[len(url_prefix) :] - if remainder.startswith(root_path): - failed_links.append( - { - "rel": rel, - "href": href, - "error": f"contains duplicated root path '{root_path}'", - } - ) + remainder = href[len(expected_prefix) :] + if remainder.startswith(ROOT_PATH): + failed_links.append( + { + "rel": rel, + "href": href, + "error": f"contains duplicated root path '{ROOT_PATH}'", + } + ) # If there are failed links, create a detailed error report if failed_links: From f08a6ffafd2f24c8cb546a8296b00b18f6674910 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 22 Jul 2025 10:20:01 -0700 Subject: [PATCH 7/8] Reduce comments --- stac_fastapi/pgstac/models/links.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/stac_fastapi/pgstac/models/links.py b/stac_fastapi/pgstac/models/links.py index 31bf920..42b5cf7 100644 --- a/stac_fastapi/pgstac/models/links.py +++ b/stac_fastapi/pgstac/models/links.py @@ -58,19 +58,10 @@ def url(self): # - by uvicorn when running with --root-path # - by FastAPI when running with FastAPI(root_path="...") # - # When root path is set by uvicorn, request.url.path will have the root path prefix. - # eg. if root path is "/api" and the path is "/collections", - # the request.url.path will be "/api/collections" - # # We need to remove the root path prefix from the path before # joining the base_url and path to get the full url to avoid # having root_path twice in the url if root_path := self.request.scope.get("root_path"): - # self.request.app.root_path is set by FastAPI when running with FastAPI(root_path="...") - # If self.request.app.root_path is not set but self.request.scope.get("root_path") is set, - # then the root path is set by uvicorn - # So we need to remove the root path prefix from the path before - # joining the base_url and path to get the full url if path.startswith(root_path): path = path[len(root_path) :] From 86e848fb9f55ec0ae20aaf8af468a36453f5c3c4 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 22 Jul 2025 10:23:27 -0700 Subject: [PATCH 8/8] Update CHANGES.md --- CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index b838437..24f6a38 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- fix root-path handling when setting via env var or on app instance + ### Changed - rename `POSTGRES_HOST_READER` to `PGHOST` in config **breaking change**