update pagination for collection-search

[vincentsarago]

this PR adds a tests showing that the recent collection-search addition breaks the pagination code

cc @hrodmn

[vincentsarago]

I'm also seeing some warning

  /Users/vincentsarago/Dev/DevSeed/stac-fastapi-off/stac_fastapi/extensions/stac_fastapi/extensions/core/collection_search/collection_search.py:97: UserWarning: Conformance class for `BulkTransactionExtension` extension not found.
    warnings.warn(

coming from

stac-fastapi-pgstac/stac_fastapi/pgstac/app.py

Lines 72 to 76 in 9a3797a

	collection_search_extension = (
	CollectionSearchExtension.from_extensions(extensions)
	if "collection_search" in enabled_extensions
	else None
	)

because the CollectionSearchExtension is trying to use some extension that has nothing to do with the collection-search (I though we splited the extension list at one point to avoid this @hrodmn

[hrodmn]

I'm sorry for missing this in that PR!

because the CollectionSearchExtension is trying to use some extension that has nothing to do with the collection-search (I though we splited the extension list at one point to avoid this @hrodmn

You added that warning in stac-fastapi. We could limit the list of extensions that we pass to .from_extensions but is that necessary if the unimplemented ones are discarded anyways? If we chose to limit the list that is passed along to the class method then stac-fastapi-pgstac will have to stay up-to-date with the list in stac-fastapi which seems like more work!

[hrodmn]

this PR adds a tests showing that the recent collection-search addition breaks the pagination code

The problem is that the pgstac function collection_search returns the next link as a dictionary instead of a hash like the item search function and I didn't test the limit feature 😞 so here we are.

In the short term it probably makes most sense to handle the collection links in a specific way, though it might make sense to update pgstac.collection_search to return a token instead of the full link dictionary.

[hrodmn]

One way to handle it would be to create a new CollectionSearchPagingLinks class:

@attr.s
class CollectionSearchPagingLinks(BaseLinks):
    next: Optional[Dict[str, Any]] = attr.ib(kw_only=True, default=None)
    prev: Optional[Dict[str, Any]] = attr.ib(kw_only=True, default=None)

    def link_next(self) -> Optional[Dict[str, Any]]:
        """Create link for next page."""
        if self.next is not None:
            method = self.request.method
            if method == "GET":
                # if offset is equal to default value (0), drop it
                if self.next["body"].get("offset", -1) == 0:
                    _ = self.next["body"].pop("offset")

                href = merge_params(self.url, self.next["body"])

                # if next link is equal to this link, skip it
                if href == self.url:
                    return None

                link = {
                    "rel": Relations.next.value,
                    "type": MimeTypes.geojson.value,
                    "method": method,
                    "href": href,
                }
                return link

        return None

    def link_prev(self):
        if self.prev is not None:
            method = self.request.method
            if method == "GET":
                # if offset is equal to default value (0), drop it
                if self.prev["body"].get("offset", -1) == 0:
                    _ = self.prev["body"].pop("offset")

                href = merge_params(self.url, self.prev["body"])

                # if prev link is equal to this link, skip it
                if href == self.url:
                    return None
                return {
                    "rel": Relations.previous.value,
                    "type": MimeTypes.geojson.value,
                    "method": method,
                    "href": href,
                }

        return None

It would require some logic to check if we need to pass the next/prev links at a given page (i.e. compare the next link parameters to the request parameters, though.

[hrodmn]

Here is one approach to fixing the problem in stac-fastapi-pgstac: #156. It still needs some work to get the offset parameter to work correctly.

Actually, the request model does not have offset so there is more work to do on that.

[hrodmn]

Thanks for merging #156 @vincentsarago! There is still some work to be done here unfortunately. stac-fastapi expects a token argument in the collection search requests but pgstac is not set up to provide that yet.

I think we either need to add the token functionality to pgstac.collection_search or modify the request model to replace token with offset for collection search.

[hrodmn]

I just noticed that in stac-fastapi we only limit the conformance classes in the from_extensions() method and let all unsupported extensions affect the GET request model:
https://github.com/stac-utils/stac-fastapi/blob/main/stac_fastapi/extensions/stac_fastapi/extensions/core/collection_search/collection_search.py#L94-L109

That is why the token appears in the /collections GET request model. Should we limit the list of extensions that affect the GET request model in addition to the conformance classes?

[hrodmn]

One option is to turn pagination off in stac-fastapi-pgstac collection search for now then either implement next/prev tokens in the pgstac function or modify stac-fastapi to include the offset parameter in the request model.

[vincentsarago]

@hrodmn so there is no hard specification about the pagination in the collection-search

https://github.com/radiantearth/stac-api-spec/blob/main/ogcapi-features/README.md#collection-pagination

but for sure we can't use the TokenPagination extension used for the /search and /items endpoint

we should:

• add a OffsetPagination extension in stac-fastapi
• split the list of extension for the /search, /items and /collections endpoints
• change the all_collections code to accept offset instead of token

OR we could also see if @bitner wants to change pgstac to use token instead of offset 🤷

[vincentsarago]

ref stac-api-extensions/collection-search#19

[vincentsarago]

pgstac seems to return a next link even if returned collection is empty

cc @bitner

[vincentsarago]

there seems to be bugs in pgstac about how next/previous links are created

should open an issue in pgstac repo