From 176f97b0f79acc90ac78aa94d4a65ba174510ce9 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 29 Dec 2022 11:06:23 -0700 Subject: [PATCH 1/7] feat: add validation via stac-api-validator Includes a local validation script and a CI job. --- .github/workflows/cicd.yaml | 68 ++++++++++++++++++++++++++++++++++--- scripts/validate | 28 +++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) create mode 100755 scripts/validate diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index b1f9765a3..3015a28d6 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -1,16 +1,16 @@ name: stac-fastapi on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] jobs: test: runs-on: ubuntu-latest strategy: matrix: - python-version: [ '3.8', '3.9', '3.10'] + python-version: ["3.8", "3.9", "3.10"] timeout-minutes: 10 services: @@ -118,9 +118,69 @@ jobs: POSTGRES_HOST_READER: localhost POSTGRES_HOST_WRITER: localhost POSTGRES_PORT: 5432 + + validate: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + backend: ["sqlalchemy", "pgstac"] + services: + pgstac: + image: ghcr.io/stac-utils/pgstac:v0.6.11 + env: + POSTGRES_USER: username + POSTGRES_PASSWORD: password + POSTGRES_DB: postgis + PGUSER: username + PGPASSWORD: password + PGDATABASE: postgis + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - name: Check out repository code + uses: actions/checkout@v3 + - name: Setup Python + uses: actions/setup-python@v3 + with: + python-version: "3.10" + cache: pip + cache-dependency-path: stac_fastapi/pgstac/setup.cfg + - name: Install stac-fastapi and stac-api-validator + run: pip install ./stac_fastapi/api ./stac_fastapi/types ./stac_fastapi/${{ matrix.backend }}[server] stac-api-validator==0.4.1 + - name: Run migration + if: ${{ matrix.backend == 'sqlalchemy' }} + run: cd stac_fastapi/sqlalchemy && alembic upgrade head + env: + POSTGRES_USER: username + POSTGRES_PASS: password + POSTGRES_DBNAME: postgis + POSTGRES_HOST: localhost + POSTGRES_PORT: 5432 + - name: Load data and validate + run: python -m stac_fastapi.${{ matrix.backend }}.app & ./scripts/wait-for-it.sh localhost:8080 && python ./scripts/ingest_joplin.py http://localhost:8080 && ./scripts/validate http://localhost:8080 + env: + POSTGRES_USER: username + POSTGRES_PASS: password + POSTGRES_DBNAME: postgis + POSTGRES_HOST_READER: localhost + POSTGRES_HOST_WRITER: localhost + POSTGRES_PORT: 5432 + PGUSER: username + PGPASSWORD: password + PGHOST: localhost + PGDATABASE: postgis + APP_HOST: 0.0.0.0 + APP_PORT: 8080 + test-docs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - name: Test generating docs - run: make docs \ No newline at end of file + run: make docs diff --git a/scripts/validate b/scripts/validate new file mode 100755 index 000000000..3431ac36f --- /dev/null +++ b/scripts/validate @@ -0,0 +1,28 @@ +#!/usr/bin/env sh +# +# Validate a STAC server using [stac-api-validator](https://github.com/stac-utils/stac-api-validator). +# +# Assumptions: +# +# - You have stac-api-validator installed, e.g. via `pip install stac-api-validator` +# - You've loaded the joplin data, probably using `python ./scripts/ingest_joplin.py http://localhost:8080`` +# +# Currently, item-search is not checked, because it crashes stac-api-validator (probably a problem on our side). + +set -e + +if [ $# -eq 0 ]; then + root_url=http://localhost:8080 +else + root_url="$1" +fi +geometry='{"type":"Polygon","coordinates":[[[-94.6884155,37.0595608],[-94.6884155,37.0332547],[-94.6554565,37.0332547],[-94.6554565,37.0595608],[-94.6884155,37.0595608]]]}' + +stac-api-validator --root-url "$root_url" \ + --conformance core \ + --conformance collections \ + --conformance features \ + --conformance filter \ + --collection joplin \ + --geometry "$geometry" + # --conformance item-search # currently breaks stac-api-validator From cc1990a23428d975249966cf3efc3d6fd05b9e12 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 10 Jan 2023 16:11:33 -0700 Subject: [PATCH 2/7] fix: get_item response is geojson --- stac_fastapi/api/stac_fastapi/api/app.py | 4 ++-- stac_fastapi/pgstac/tests/api/test_api.py | 9 +++++++++ stac_fastapi/sqlalchemy/tests/api/test_api.py | 13 +++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/stac_fastapi/api/stac_fastapi/api/app.py b/stac_fastapi/api/stac_fastapi/api/app.py index 1f2768ee2..808afe275 100644 --- a/stac_fastapi/api/stac_fastapi/api/app.py +++ b/stac_fastapi/api/stac_fastapi/api/app.py @@ -158,12 +158,12 @@ def register_get_item(self): name="Get Item", path="/collections/{collection_id}/items/{item_id}", response_model=Item if self.settings.enable_response_models else None, - response_class=self.response_class, + response_class=GeoJSONResponse, response_model_exclude_unset=True, response_model_exclude_none=True, methods=["GET"], endpoint=create_async_endpoint( - self.client.get_item, ItemUri, self.response_class + self.client.get_item, ItemUri, GeoJSONResponse ), ) diff --git a/stac_fastapi/pgstac/tests/api/test_api.py b/stac_fastapi/pgstac/tests/api/test_api.py index 1f7fbe371..cbe1a2202 100644 --- a/stac_fastapi/pgstac/tests/api/test_api.py +++ b/stac_fastapi/pgstac/tests/api/test_api.py @@ -51,6 +51,15 @@ async def test_get_features_content_type(app_client, load_test_collection): assert resp.headers["content-type"] == "application/geo+json" +async def test_get_feature_content_type( + app_client, load_test_collection, load_test_item +): + resp = await app_client.get( + f"collections/{load_test_collection.id}/items/{load_test_item.id}" + ) + assert resp.headers["content-type"] == "application/geo+json" + + async def test_api_headers(app_client): resp = await app_client.get("/api") assert ( diff --git a/stac_fastapi/sqlalchemy/tests/api/test_api.py b/stac_fastapi/sqlalchemy/tests/api/test_api.py index de4f5d734..7922bba9d 100644 --- a/stac_fastapi/sqlalchemy/tests/api/test_api.py +++ b/stac_fastapi/sqlalchemy/tests/api/test_api.py @@ -445,9 +445,18 @@ def test_app_search_response_duplicate_forwarded_headers( assert link["href"].startswith("https://testserver:1234/") -async def test_get_features_content_type(app_client, load_test_data): +def test_get_features_content_type(app_client, load_test_data): item = load_test_data("test_item.json") - resp = await app_client.get(f"collections/{item['collection']}/items") + resp = app_client.get(f"collections/{item['collection']}/items") + assert resp.headers["content-type"] == "application/geo+json" + + +def test_get_feature_content_type(app_client, load_test_data, postgres_transactions): + item = load_test_data("test_item.json") + postgres_transactions.create_item( + item["collection"], item, request=MockStarletteRequest + ) + resp = app_client.get(f"collections/{item['collection']}/items/{item['id']}") assert resp.headers["content-type"] == "application/geo+json" From b0e1542b462ce0e9059ee36e2e14dc6b9bdff7ab Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 10 Jan 2023 16:41:10 -0700 Subject: [PATCH 3/7] fix, pgstac: self link, item collection endpoint --- .../pgstac/stac_fastapi/pgstac/core.py | 9 ++++++-- .../stac_fastapi/pgstac/models/links.py | 21 +++++++++++++++++++ stac_fastapi/pgstac/tests/api/test_api.py | 12 +++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/stac_fastapi/pgstac/stac_fastapi/pgstac/core.py b/stac_fastapi/pgstac/stac_fastapi/pgstac/core.py index 9b194f70e..e074a9f9c 100644 --- a/stac_fastapi/pgstac/stac_fastapi/pgstac/core.py +++ b/stac_fastapi/pgstac/stac_fastapi/pgstac/core.py @@ -18,7 +18,12 @@ from starlette.requests import Request from stac_fastapi.pgstac.config import Settings -from stac_fastapi.pgstac.models.links import CollectionLinks, ItemLinks, PagingLinks +from stac_fastapi.pgstac.models.links import ( + CollectionLinks, + ItemCollectionLinks, + ItemLinks, + PagingLinks, +) from stac_fastapi.pgstac.types.search import PgstacSearch from stac_fastapi.pgstac.utils import filter_fields from stac_fastapi.types.core import AsyncBaseCoreClient @@ -286,7 +291,7 @@ async def item_collection( **clean, ) item_collection = await self._search_base(req, **kwargs) - links = await CollectionLinks( + links = await ItemCollectionLinks( collection_id=collection_id, request=kwargs["request"] ).get_links(extra_links=item_collection["links"]) item_collection["links"] = links diff --git a/stac_fastapi/pgstac/stac_fastapi/pgstac/models/links.py b/stac_fastapi/pgstac/stac_fastapi/pgstac/models/links.py index 798db5426..c59891876 100644 --- a/stac_fastapi/pgstac/stac_fastapi/pgstac/models/links.py +++ b/stac_fastapi/pgstac/stac_fastapi/pgstac/models/links.py @@ -206,6 +206,27 @@ def link_items(self) -> Dict: ) +@attr.s +class ItemCollectionLinks(CollectionLinksBase): + """Create inferred links specific to collections.""" + + def link_self(self) -> Dict: + """Return the self link.""" + return dict( + rel=Relations.self.value, + type=MimeTypes.geojson.value, + href=self.resolve(f"collections/{self.collection_id}/items"), + ) + + def link_parent(self) -> Dict: + """Create the `parent` link.""" + return self.collection_link(rel=Relations.parent.value) + + def link_collection(self) -> Dict: + """Create the `collection` link.""" + return self.collection_link() + + @attr.s class ItemLinks(CollectionLinksBase): """Create inferred links specific to items.""" diff --git a/stac_fastapi/pgstac/tests/api/test_api.py b/stac_fastapi/pgstac/tests/api/test_api.py index cbe1a2202..1ac5e9701 100644 --- a/stac_fastapi/pgstac/tests/api/test_api.py +++ b/stac_fastapi/pgstac/tests/api/test_api.py @@ -51,6 +51,18 @@ async def test_get_features_content_type(app_client, load_test_collection): assert resp.headers["content-type"] == "application/geo+json" +async def test_get_features_self_link(app_client, load_test_collection): + # https://github.com/stac-utils/stac-fastapi/issues/483 + resp = await app_client.get(f"collections/{load_test_collection.id}/items") + assert resp.status_code == 200 + resp_json = resp.json() + self_link = next( + (link for link in resp_json["links"] if link["rel"] == "self"), None + ) + assert self_link is not None + assert self_link["href"].endswith("/items") + + async def test_get_feature_content_type( app_client, load_test_collection, load_test_item ): From 6d1e67135075e11fb3bd62f0b6cb3b8ba27d2fcc Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 12 Jan 2023 13:32:37 -0700 Subject: [PATCH 4/7] fix: don't add context extension to landing page --- stac_fastapi/pgstac/tests/api/test_api.py | 7 +++++++ stac_fastapi/sqlalchemy/tests/api/test_api.py | 7 +++++++ stac_fastapi/types/stac_fastapi/types/core.py | 10 ++-------- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/stac_fastapi/pgstac/tests/api/test_api.py b/stac_fastapi/pgstac/tests/api/test_api.py index 1ac5e9701..06a80675e 100644 --- a/stac_fastapi/pgstac/tests/api/test_api.py +++ b/stac_fastapi/pgstac/tests/api/test_api.py @@ -92,6 +92,13 @@ async def test_core_router(api_client, app): assert not core_routes - api_routes +async def test_landing_page_stac_extensions(app_client): + resp = await app_client.get("/") + assert resp.status_code == 200 + resp_json = resp.json() + assert not resp_json["stac_extensions"] + + async def test_transactions_router(api_client, app): transaction_routes = set() for transaction_route in STAC_TRANSACTION_ROUTES: diff --git a/stac_fastapi/sqlalchemy/tests/api/test_api.py b/stac_fastapi/sqlalchemy/tests/api/test_api.py index 7922bba9d..cbab5bfc2 100644 --- a/stac_fastapi/sqlalchemy/tests/api/test_api.py +++ b/stac_fastapi/sqlalchemy/tests/api/test_api.py @@ -53,6 +53,13 @@ def test_core_router(api_client): assert not core_routes - api_routes +def test_landing_page_stac_extensions(app_client): + resp = app_client.get("/") + assert resp.status_code == 200 + resp_json = resp.json() + assert not resp_json["stac_extensions"] + + def test_transactions_router(api_client): transaction_routes = set(STAC_TRANSACTION_ROUTES) api_routes = set( diff --git a/stac_fastapi/types/stac_fastapi/types/core.py b/stac_fastapi/types/stac_fastapi/types/core.py index 258cb93a0..8470bec08 100644 --- a/stac_fastapi/types/stac_fastapi/types/core.py +++ b/stac_fastapi/types/stac_fastapi/types/core.py @@ -351,13 +351,10 @@ def landing_page(self, **kwargs) -> stac_types.LandingPage: """ request: Request = kwargs["request"] base_url = get_base_url(request) - extension_schemas = [ - schema.schema_href for schema in self.extensions if schema.schema_href - ] landing_page = self._landing_page( base_url=base_url, conformance_classes=self.conformance_classes(), - extension_schemas=extension_schemas, + extension_schemas=[], ) # Add Collections links @@ -550,13 +547,10 @@ async def landing_page(self, **kwargs) -> stac_types.LandingPage: """ request: Request = kwargs["request"] base_url = get_base_url(request) - extension_schemas = [ - schema.schema_href for schema in self.extensions if schema.schema_href - ] landing_page = self._landing_page( base_url=base_url, conformance_classes=self.conformance_classes(), - extension_schemas=extension_schemas, + extension_schemas=[], ) collections = await self.all_collections(request=kwargs["request"]) for collection in collections["collections"]: From 7d0fb5712b15c72d9b27d30889141b74e3395863 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Fri, 13 Jan 2023 07:58:01 -0700 Subject: [PATCH 5/7] deps: update uvicorn to non-yanked version --- stac_fastapi/pgstac/setup.py | 2 +- stac_fastapi/sqlalchemy/setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stac_fastapi/pgstac/setup.py b/stac_fastapi/pgstac/setup.py index 5d47424e5..00e8f7478 100644 --- a/stac_fastapi/pgstac/setup.py +++ b/stac_fastapi/pgstac/setup.py @@ -31,7 +31,7 @@ "httpx", ], "docs": ["mkdocs", "mkdocs-material", "pdocs"], - "server": ["uvicorn[standard]==0.17.0"], + "server": ["uvicorn[standard]==0.17.0.post1"], "awslambda": ["mangum"], } diff --git a/stac_fastapi/sqlalchemy/setup.py b/stac_fastapi/sqlalchemy/setup.py index c7d6e98b6..00b6ab414 100644 --- a/stac_fastapi/sqlalchemy/setup.py +++ b/stac_fastapi/sqlalchemy/setup.py @@ -29,7 +29,7 @@ "requests", ], "docs": ["mkdocs", "mkdocs-material", "pdocs"], - "server": ["uvicorn[standard]==0.17.0"], + "server": ["uvicorn[standard]==0.17.0.post1"], } From 3bff04401c7f39c747538bf6b8db6697d49cc94e Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Fri, 13 Jan 2023 08:46:20 -0700 Subject: [PATCH 6/7] fix: exclude optional collection values If we're not using response models, we can't automagically exclude none values. --- .../stac_fastapi/sqlalchemy/serializers.py | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/serializers.py b/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/serializers.py index 948d06e30..5a272634e 100644 --- a/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/serializers.py +++ b/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/serializers.py @@ -146,22 +146,28 @@ def db_to_stac(cls, db_model: database.Collection, base_url: str) -> TypedDict: if db_links: collection_links += resolve_links(db_links, base_url) - stac_extensions = db_model.stac_extensions or [] - - return stac_types.Collection( + collection = stac_types.Collection( type="Collection", id=db_model.id, - stac_extensions=stac_extensions, stac_version=db_model.stac_version, - title=db_model.title, description=db_model.description, - keywords=db_model.keywords, license=db_model.license, - providers=db_model.providers, - summaries=db_model.summaries, extent=db_model.extent, links=collection_links, ) + # We need to manually include optional values to ensure they are + # excluded if we're not using response models. + if db_model.stac_extensions: + collection["stac_extensions"] = db_model.stac_extensions + if db_model.title: + collection["title"] = db_model.title + if db_model.keywords: + collection["keywords"] = db_model.keywords + if db_model.providers: + collection["providers"] = db_model.providers + if db_model.summaries: + collection["summaries"] = db_model.summaries + return collection @classmethod def stac_to_db( From b69c2213fb2348a7540d83322c3897cbf77b3a49 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Fri, 13 Jan 2023 08:56:09 -0700 Subject: [PATCH 7/7] fix: add required links to item collection sqlalchemy --- .../sqlalchemy/stac_fastapi/sqlalchemy/core.py | 18 +++++++++++++++++- stac_fastapi/sqlalchemy/tests/conftest.py | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/core.py b/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/core.py index 68c96f2de..a0a99d044 100644 --- a/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/core.py +++ b/stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/core.py @@ -169,7 +169,23 @@ def item_collection( else None ) - links = [] + links = [ + { + "rel": Relations.self.value, + "type": "application/geo+json", + "href": str(kwargs["request"].url), + }, + { + "rel": Relations.root.value, + "type": "application/json", + "href": str(kwargs["request"].base_url), + }, + { + "rel": Relations.parent.value, + "type": "application/json", + "href": str(kwargs["request"].base_url), + }, + ] if page.next: links.append( { diff --git a/stac_fastapi/sqlalchemy/tests/conftest.py b/stac_fastapi/sqlalchemy/tests/conftest.py index 7abd9150f..86984a12f 100644 --- a/stac_fastapi/sqlalchemy/tests/conftest.py +++ b/stac_fastapi/sqlalchemy/tests/conftest.py @@ -70,6 +70,7 @@ def load_file(filename: str) -> Dict: class MockStarletteRequest: base_url = "http://test-server" + url = "http://test-server/some/endpoint" @pytest.fixture