Skip to content

Commit

Permalink
feat: remove links from default include list
Browse files Browse the repository at this point in the history
**sqlalchemy** uses `FieldsExtension.default_includes`, so its fix was pretty
*easy
-- I did have to add an explicit include+exclude check to the search function to
ensure the fields extension was applied _only if_ either `include` or `exclude`
was set. **pgstac** was a little trickier, because it doesn't use the
`default_includes` list.

I did modify one existing test -- `test_app_fields_extension` in the
**sqlalchemy** tests was assuming fields would be stripped just by providing a
`collections` parameter to the query, which I think was incorrect -- I added an
explicit `include` field to the test.
  • Loading branch information
gadomski committed Feb 7, 2023
1 parent 94f6e2c commit 5dc6eb8
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ class FieldsExtension(ApiExtension):
"stac_version",
"geometry",
"bbox",
"links",
"assets",
"properties.datetime",
"collection",
Expand Down
34 changes: 23 additions & 11 deletions stac_fastapi/pgstac/stac_fastapi/pgstac/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,29 +190,41 @@ async def _search_base(
if include and len(include) == 0:
include = None

async def _add_item_links(
features = collection.get("features", [])
if include and features and not (set(include) & set(features[0]["properties"])):
# If the `include` list is only non-existent fields, keep links
exclude_links = False
else:
exclude_links = (
search_request.fields.exclude
and "links" in search_request.fields.exclude
) or (
search_request.fields.include
and "links" not in search_request.fields.include
)

async def _update_item_links(
feature: Item,
collection_id: Optional[str] = None,
item_id: Optional[str] = None,
) -> None:
"""Add ItemLinks to the Item.
"""Update this item's links.
If the fields extension is excluding links, then don't add them.
Also skip links if the item doesn't provide collection and item ids.
If the fields extension is excluding links, or including other
fields but _not_ links, then remove all links.
Also skip adding links if the item doesn't provide collection and item ids.
"""
collection_id = feature.get("collection") or collection_id
item_id = feature.get("id") or item_id

if (
search_request.fields.exclude is None
or "links" not in search_request.fields.exclude
and all([collection_id, item_id])
):
if not exclude_links and all([collection_id, item_id]):
feature["links"] = await ItemLinks(
collection_id=collection_id,
item_id=item_id,
request=request,
).get_links(extra_links=feature.get("links"))
elif exclude_links and "links" in feature:
del feature["links"]

cleaned_features: List[Item] = []

Expand All @@ -234,12 +246,12 @@ async def _get_base_item(collection_id: str) -> Dict[str, Any]:
item_id = feature.get("id")

feature = filter_fields(feature, include, exclude)
await _add_item_links(feature, collection_id, item_id)
await _update_item_links(feature, collection_id, item_id)

cleaned_features.append(feature)
else:
for feature in collection.get("features") or []:
await _add_item_links(feature)
await _update_item_links(feature)
cleaned_features.append(feature)

collection["features"] = cleaned_features
Expand Down
15 changes: 13 additions & 2 deletions stac_fastapi/pgstac/tests/resources/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -1200,11 +1200,22 @@ async def test_field_extension_exclude_links(
assert "links" not in resp_json["features"][0]


async def test_field_extension_include_but_not_links(
app_client, load_test_item, load_test_collection
):
# https://github.com/stac-utils/stac-fastapi/issues/395
body = {"fields": {"include": ["properties.eo:cloud_cover"]}}
resp = await app_client.post("/search", json=body)
assert resp.status_code == 200
resp_json = resp.json()
assert "links" not in resp_json["features"][0]


async def test_field_extension_include_only_non_existant_field(
app_client, load_test_item, load_test_collection
):
"""Including only a non-existant field should return the full item"""
body = {"fields": {"include": ["non_existant_field"]}}
"""Including only a non-existent field should return the full item"""
body = {"fields": {"include": ["non_existent_field"]}}

resp = await app_client.post("/search", json=body)
assert resp.status_code == 200
Expand Down
4 changes: 3 additions & 1 deletion stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,9 @@ def post_search(
)

# Use pydantic includes/excludes syntax to implement fields extension
if self.extension_is_enabled("FieldsExtension"):
if self.extension_is_enabled("FieldsExtension") and (
search_request.fields.include or search_request.fields.exclude
):
if search_request.query is not None:
query_include: Set[str] = set(
[
Expand Down
8 changes: 7 additions & 1 deletion stac_fastapi/sqlalchemy/tests/api/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,13 @@ def test_app_fields_extension(load_test_data, app_client, postgres_transactions)
item["collection"], item, request=MockStarletteRequest
)

resp = app_client.get("/search", params={"collections": ["test-collection"]})
resp = app_client.post(
"/search",
json={
"collections": ["test-collection"],
"fields": {"include": ["datetime"]},
},
)
assert resp.status_code == 200
resp_json = resp.json()
assert list(resp_json["features"][0]["properties"]) == ["datetime"]
Expand Down
14 changes: 14 additions & 0 deletions stac_fastapi/sqlalchemy/tests/resources/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,20 @@ def test_field_extension_exclude_default_includes(app_client, load_test_data):
assert "geometry" not in resp_json["features"][0]


def test_field_extension_include_but_not_links(app_client, load_test_data):
# https://github.com/stac-utils/stac-fastapi/issues/395
test_item = load_test_data("test_item.json")
resp = app_client.post(
f"/collections/{test_item['collection']}/items", json=test_item
)
assert resp.status_code == 200
body = {"fields": {"include": ["properties.eo:cloud_cover"]}}
resp = app_client.post("/search", json=body)
assert resp.status_code == 200
resp_json = resp.json()
assert "links" not in resp_json["features"][0]


def test_search_intersects_and_bbox(app_client):
"""Test POST search intersects and bbox are mutually exclusive (core)"""
bbox = [-118, 34, -117, 35]
Expand Down

0 comments on commit 5dc6eb8

Please sign in to comment.