add collection search extension #136
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,5 +9,7 @@ coverage.xml | |
| *.log | ||
| .git | ||
| .envrc | ||
| *egg-info | ||
|
|
||
| venv | ||
| env | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,12 +14,11 @@ | |
| from pygeofilter.parsers.cql2_text import parse as parse_cql2_text | ||
| from pypgstac.hydration import hydrate | ||
| from stac_fastapi.api.models import JSONResponse | ||
| from stac_fastapi.types.core import AsyncBaseCoreClient, Relations | ||
| from stac_fastapi.types.errors import InvalidQueryParameter, NotFoundError | ||
| from stac_fastapi.types.requests import get_base_url | ||
| from stac_fastapi.types.rfc3339 import DateTimeType | ||
| from stac_fastapi.types.stac import Collection, Collections, Item, ItemCollection | ||
| from stac_pydantic.shared import BBox, MimeTypes | ||
|
|
||
| from stac_fastapi.pgstac.config import Settings | ||
|
|
@@ -39,17 +38,100 @@ | |
| class CoreCrudClient(AsyncBaseCoreClient): | ||
| """Client for core endpoints defined by stac.""" | ||
|
|
||
| async def all_collections( # noqa: C901 | ||
| self, | ||
| request: Request, | ||
| # Extensions | ||
| bbox: Optional[BBox] = None, | ||
| datetime: Optional[DateTimeType] = None, | ||
| limit: Optional[int] = None, | ||
| query: Optional[str] = None, | ||
| token: Optional[str] = None, | ||
| fields: Optional[List[str]] = None, | ||
| sortby: Optional[str] = None, | ||
| filter: Optional[str] = None, | ||
| filter_lang: Optional[str] = None, | ||
| **kwargs, | ||
| ) -> Collections: | ||
| """Cross catalog search (GET). | ||
| Called with `GET /collections`. | ||
| Returns: | ||
| Collections which match the search criteria, returns all | ||
| collections by default. | ||
| """ | ||
|
|
||
| # Parse request parameters | ||
| base_args = { | ||
| "bbox": bbox, | ||
| "limit": limit, | ||
| "token": token, | ||
| "query": orjson.loads(unquote_plus(query)) if query else query, | ||
| } | ||
|
|
||
| clean = clean_search_args( | ||
| base_args=base_args, | ||
| datetime=datetime, | ||
| fields=fields, | ||
| sortby=sortby, | ||
| filter=filter, | ||
| filter_lang=filter_lang, | ||
| ) | ||
|
|
||
| # Do the request | ||
| try: | ||
| search_request = self.post_request_model(**clean) | ||
|
There was a problem hiding this comment. 🤔 😬 the issue is that here we don't have to the model used, we should add a There was a problem hiding this comment. If I understand the problem correctly, we are just trying to dump the args into a json that pgstac will accept, but we are using a model for item search that is defined in There was a problem hiding this comment. it's kinda weird because we will pass the Client to the CollectionSearch extension, which kinda result in a circular dependency, but that's how it's done for the
But yes I think we will have to do that! There was a problem hiding this comment. I tried adding a Here is what I tried: @attr.s
class CoreCrudClient(AsyncBaseCoreClient):
"""Client for core endpoints defined by stac."""
collections_get_request_model: BaseCollectionSearchGetRequest = attr.ib(
default=BaseCollectionSearchGetRequest
)
async def all_collections( # noqa: C901
self,
request: Request,
# Extensions
bbox: Optional[BBox] = None,
datetime: Optional[DateTimeType] = None,
limit: Optional[int] = None,
query: Optional[str] = None,
token: Optional[str] = None,
fields: Optional[List[str]] = None,
sortby: Optional[str] = None,
filter: Optional[str] = None,
filter_lang: Optional[str] = None,
**kwargs,
) -> Collections:
"""Cross catalog search (GET).
Called with `GET /collections`.
Returns:
Collections which match the search criteria, returns all
collections by default.
"""
# Parse request parameters
base_args = {
"bbox": bbox,
"limit": limit,
"token": token,
"query": orjson.loads(unquote_plus(query)) if query else query,
}
clean = clean_search_args(
base_args=base_args,
datetime=datetime,
fields=fields,
sortby=sortby,
filter=filter,
filter_lang=filter_lang,
)
# Do the request
try:
search_request = self.collections_get_request_model(**clean)
except ValidationError as e:
raise HTTPException(
status_code=400, detail=f"Invalid parameters provided {e}"
) from e
return await self._collection_search_base(search_request, request=request)
So I think I will need to get the json for the pgstac query using a different approach. There was a problem hiding this comment. I got this working by using There was a problem hiding this comment. Ah, I think I see the path. I probably need to use There was a problem hiding this comment.
yes because pydantic model are interpreted as body parameter for endpoints There was a problem hiding this comment. I think the stac-fastapi-pgstac/stac_fastapi/pgstac/core.py Lines 466 to 471 in cb08b0f There was a problem hiding this comment. Thanks for your patience while I figure this out. I think I need to double back and make sure I understand how all of these models are getting used. Upon closer examination, it looks like stac-fastapi-pgstac is just using the We provide a stac-fastapi-pgstac/stac_fastapi/pgstac/app.py Lines 67 to 78 in cb08b0f But I can't see how There was a problem hiding this comment. I guess I am pretty sure we can use it for the |
||
| except ValidationError as e: | ||
| raise HTTPException( | ||
| status_code=400, detail=f"Invalid parameters provided {e}" | ||
| ) from e | ||
|
|
||
| return await self._collection_search_base(search_request, request=request) | ||
|
|
||
| async def _collection_search_base( # noqa: C901 | ||
| self, | ||
| search_request: PgstacSearch, | ||
| request: Request, | ||
| ) -> Collections: | ||
| """Cross catalog search (GET). | ||
| Called with `GET /search`. | ||
| Args: | ||
| search_request: search request parameters. | ||
| Returns: | ||
| All collections which match the search criteria. | ||
| """ | ||
| base_url = get_base_url(request) | ||
| search_request_json = search_request.model_dump_json( | ||
| exclude_none=True, by_alias=True | ||
| ) | ||
|
|
||
| try: | ||
| async with request.app.state.get_connection(request, "r") as conn: | ||
| q, p = render( | ||
| """ | ||
| SELECT * FROM collection_search(:req::text::jsonb); | ||
| """, | ||
| req=search_request_json, | ||
| ) | ||
| collections_result: Collections = await conn.fetchval(q, *p) | ||
| except InvalidDatetimeFormatError as e: | ||
| raise InvalidQueryParameter( | ||
| f"Datetime parameter {search_request.datetime} is invalid." | ||
| ) from e | ||
|
|
||
| next: Optional[str] = None | ||
| prev: Optional[str] = None | ||
|
|
||
| if links := collections_result.get("links"): | ||
| next = collections_result["links"].pop("next") | ||
| prev = collections_result["links"].pop("prev") | ||
|
|
||
| linked_collections: List[Collection] = [] | ||
| collections = collections_result["collections"] | ||
| if collections is not None and len(collections) > 0: | ||
| for c in collections: | ||
| coll = Collection(**c) | ||
|
|
@@ -71,25 +153,16 @@ async def all_collections(self, request: Request, **kwargs) -> Collections: | |
|
|
||
| linked_collections.append(coll) | ||
|
|
||
| links = await PagingLinks( | ||
| request=request, | ||
| next=next, | ||
| prev=prev, | ||
| ).get_links() | ||
|
|
||
| return Collections( | ||
| collections=linked_collections or [], | ||
| links=links, | ||
| ) | ||
|
|
||
| async def get_collection( | ||
| self, collection_id: str, request: Request, **kwargs | ||
|
|
@@ -383,7 +456,7 @@ async def post_search( | |
|
|
||
| return ItemCollection(**item_collection) | ||
|
|
||
| async def get_search( | ||
| self, | ||
| request: Request, | ||
| collections: Optional[List[str]] = None, | ||
|
|
@@ -418,49 +491,15 @@ async def get_search( # noqa: C901 | |
| "query": orjson.loads(unquote_plus(query)) if query else query, | ||
| } | ||
|
|
||
| clean = clean_search_args( | ||
| base_args=base_args, | ||
| intersects=intersects, | ||
| datetime=datetime, | ||
| fields=fields, | ||
| sortby=sortby, | ||
| filter=filter, | ||
| filter_lang=filter_lang, | ||
| ) | ||
|
|
||
| # Do the request | ||
| try: | ||
|
|
@@ -471,3 +510,60 @@ async def get_search( # noqa: C901 | |
| ) from e | ||
|
|
||
| return await self.post_search(search_request, request=request) | ||
|
|
||
|
|
||
| def clean_search_args( # noqa: C901 | ||
| base_args: Dict[str, Any], | ||
| intersects: Optional[str] = None, | ||
| datetime: Optional[DateTimeType] = None, | ||
| fields: Optional[List[str]] = None, | ||
| sortby: Optional[str] = None, | ||
| filter: Optional[str] = None, | ||
| filter_lang: Optional[str] = None, | ||
| ) -> Dict[str, Any]: | ||
| """Clean up search arguments to match format expected by pgstac""" | ||
| if filter: | ||
| if filter_lang == "cql2-text": | ||
| ast = parse_cql2_text(filter) | ||
| base_args["filter"] = orjson.loads(to_cql2(ast)) | ||
| base_args["filter-lang"] = "cql2-json" | ||
|
|
||
| if datetime: | ||
| base_args["datetime"] = format_datetime_range(datetime) | ||
|
|
||
| if intersects: | ||
| base_args["intersects"] = orjson.loads(unquote_plus(intersects)) | ||
|
|
||
| if sortby: | ||
| # https://github.com/radiantearth/stac-spec/tree/master/api-spec/extensions/sort#http-get-or-post-form | ||
| sort_param = [] | ||
| for sort in sortby: | ||
| sortparts = re.match(r"^([+-]?)(.*)$", sort) | ||
| if sortparts: | ||
| sort_param.append( | ||
| { | ||
| "field": sortparts.group(2).strip(), | ||
| "direction": "desc" if sortparts.group(1) == "-" else "asc", | ||
| } | ||
| ) | ||
| base_args["sortby"] = sort_param | ||
|
|
||
| if fields: | ||
| includes = set() | ||
| excludes = set() | ||
| for field in fields: | ||
| if field[0] == "-": | ||
| excludes.add(field[1:]) | ||
| elif field[0] == "+": | ||
| includes.add(field[1:]) | ||
| else: | ||
| includes.add(field) | ||
| base_args["fields"] = {"include": includes, "exclude": excludes} | ||
|
|
||
| # Remove None values from dict | ||
| clean = {} | ||
| for k, v in base_args.items(): | ||
| if v is not None and v != []: | ||
| clean[k] = v | ||
|
|
||
| return clean | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts from discussion: consider adding
idto the endpoint, despite it being omitted from the Collection Search Extension.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that the same as just calling /collections/:id ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use-case for including an
idsparameter would be to limit the scope of a search in the context of scoped authentication for a STAC API, but we discussed some more and it probably makes more sense to use thefilterextension for injecting scope limits in a search request.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
idswould be a better parameter name thanidto evoke the ability to provide multiple as a filter. And for our specific needs today, we will make use offilteras @hrodmn said. I believe it was @bitner who suggested adding the ability to filter by ID parameter, I'll let him weigh in on whether we should go without.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense just to be parallel to the items spec (and yes, "ids" plural which is how it works in items)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you add it, I'd recommend to add a separate conformance class for it so that clients actually know whether it's supported or not.
PS: ids is not included in collection search as we just inherit from OGC API - Records, which doesn't have it. It's orthogonal to how ids is not part of OGC API - Features for items. ids is a STAC-specific thing.