Skip to content

Commit

Permalink
fix conformance class checking (#223)
Browse files Browse the repository at this point in the history
* fix incorrect conformance class for collections, and add checking for all ItemSearch features
* remove uses of stac_io isinstance
  • Loading branch information
Phil Varner authored Jun 6, 2022
1 parent 6a1295e commit 49de56d
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 46 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Search query parameter now has correct typing and handles Query Extension JSON format. [#220](https://github.com/stac-utils/pystac-client/pull/220)
- Search sortby parameter now has correct typing and handles both GET and POST JSON parameter formats. [#175](https://github.com/stac-utils/pystac-client/pull/175)
- Search fields parameter now has correct typing and handles both GET and POST JSON parameter formats. [#184](https://github.com/stac-utils/pystac-client/pull/184)
- Use pytest configuration to skip benchmarks by default (instead of a `skip` mark) [#168](https://github.com/stac-utils/pystac-client/pull/168)
- Use pytest configuration to skip benchmarks by default (instead of a `skip` mark). [#168](https://github.com/stac-utils/pystac-client/pull/168)
- Methods retrieving collections incorrectly checked the existence of the OAFeat OpenAPI 3.0 conformance class
(http://www.opengis.net/spec/ogcapi-features-1/1.0/conf/oas30) instead of the `STAC API - Collections`
(https://api.stacspec.org/v1.0.0-beta.1/collections) or `STAC API - Features`
(https://api.stacspec.org/v1.0.0-beta.1/ogcapi-features) conformance classes. [223](https://github.com/stac-utils/pystac-client/pull/223)

## [v0.3.5] - 2022-05-26

Expand Down
34 changes: 24 additions & 10 deletions pystac_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ class Client(pystac.Catalog):
such as searching items (e.g., /search endpoint).
"""

def __repr__(self):
def __repr__(self) -> str:
return "<Client id={}>".format(self.id)

@classmethod
def open(
cls,
url: str,
headers: Dict[str, str] = None,
headers: Optional[Dict[str, str]] = None,
parameters: Optional[Dict[str, Any]] = None,
ignore_conformance: bool = False,
) -> "Client":
Expand All @@ -49,6 +49,8 @@ def open(
`STAC_URL` environment variable.
headers : A dictionary of additional headers to use in all requests
made to any part of this Catalog/API.
parameters: Optional dictionary of query string parameters to
include in all requests.
ignore_conformance : Ignore any advertised Conformance Classes in this
Catalog/API. This means that
functions will skip checking conformance, and may throw an unknown
Expand Down Expand Up @@ -77,8 +79,8 @@ def from_file(
cls,
href: str,
stac_io: Optional[pystac.StacIO] = None,
headers: Optional[Dict] = {},
parameters: Optional[Dict] = None,
headers: Optional[Dict[str, str]] = None,
parameters: Optional[Dict[str, Any]] = None,
) -> "Client":
"""Open a STAC Catalog/API
Expand All @@ -94,6 +96,15 @@ def from_file(

return cat

def _supports_collections(self) -> bool:
return self._conforms_to(ConformanceClasses.COLLECTIONS) or self._conforms_to(
ConformanceClasses.FEATURES
)

# TODO: fix this with the stac_api_io() method in a future PR
def _conforms_to(self, conformance_class: ConformanceClasses) -> bool:
return self._stac_io.conforms_to(conformance_class) # type: ignore

@classmethod
def from_dict(
cls,
Expand Down Expand Up @@ -123,7 +134,7 @@ def get_collection(self, collection_id: str) -> CollectionClient:
Returns:
CollectionClient: A STAC Collection
"""
if self._stac_io.conforms_to(ConformanceClasses.COLLECTIONS):
if self._supports_collections():
url = f"{self.get_self_href()}/collections/{collection_id}"
collection = CollectionClient.from_dict(
self._stac_io.read_json(url), root=self
Expand All @@ -143,7 +154,7 @@ def get_collections(self) -> Iterable[CollectionClient]:
Return:
Iterable[CollectionClient]: Iterator through Collections in Catalog/API
"""
if self._stac_io.conforms_to(ConformanceClasses.COLLECTIONS):
if self._supports_collections():
url = self.get_self_href() + "/collections"
for page in self._stac_io.get_pages(url):
if "collections" not in page:
Expand All @@ -160,7 +171,7 @@ def get_items(self) -> Iterable["Item_Type"]:
Return:
Iterable[Item]:: Generator of items whose parent is this catalog.
"""
if self._stac_io.conforms_to(ConformanceClasses.ITEM_SEARCH):
if self._conforms_to(ConformanceClasses.ITEM_SEARCH):
search = self.search()
yield from search.items()
else:
Expand All @@ -175,7 +186,7 @@ def get_all_items(self) -> Iterable["Item_Type"]:
catalogs or collections connected to this catalog through
child links.
"""
if self._stac_io.conforms_to(ConformanceClasses.ITEM_SEARCH):
if self._conforms_to(ConformanceClasses.ITEM_SEARCH):
yield from self.get_items()
else:
yield from super().get_items()
Expand Down Expand Up @@ -211,7 +222,7 @@ def search(self, **kwargs: Any) -> ItemSearch:
or does not have a link with
a ``"rel"`` type of ``"search"``.
"""
if not self._stac_io.conforms_to(ConformanceClasses.ITEM_SEARCH):
if not self._conforms_to(ConformanceClasses.ITEM_SEARCH):
raise NotImplementedError(
"This catalog does not support search because it "
f'does not conform to "{ConformanceClasses.ITEM_SEARCH}"'
Expand All @@ -223,7 +234,10 @@ def search(self, **kwargs: Any) -> ItemSearch:
)

return ItemSearch(
search_link.target, stac_io=self._stac_io, client=self, **kwargs
search_link.target,
stac_io=self._stac_io,
client=self,
**kwargs,
)

def get_search_link(self) -> Optional[pystac.Link]:
Expand Down
5 changes: 1 addition & 4 deletions pystac_client/collection_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,7 @@ def get_item(self, id: str, recursive: bool = False) -> Optional["Item_Type"]:
assert stac_io
assert isinstance(stac_io, StacApiIO)
link = self.get_single_link("items")
if (
stac_io.conforms_to(ConformanceClasses.OGCAPI_FEATURES)
and link is not None
):
if stac_io.conforms_to(ConformanceClasses.FEATURES) and link is not None:
url = f"{link.href}/{id}"
try:
item = stac_io.read_stac_object(url, root=self)
Expand Down
10 changes: 6 additions & 4 deletions pystac_client/conformance.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@ class ConformanceClasses(Enum):

# defined conformance classes regexes
CORE = rf"{stac_prefix}(.*){re.escape('/core')}"
COLLECTIONS = rf"{stac_prefix}(.*){re.escape('/collections')}"

# this is ogcapi-features instead of just features for historical reasons,
# even thought this is a STAC API conformance class
FEATURES = rf"{stac_prefix}(.*){re.escape('/ogcapi-features')}"
ITEM_SEARCH = rf"{stac_prefix}(.*){re.escape('/item-search')}"

CONTEXT = rf"{stac_prefix}(.*){re.escape('/item-search#context')}"
FIELDS = rf"{stac_prefix}(.*){re.escape('/item-search#fields')}"
SORT = rf"{stac_prefix}(.*){re.escape('/item-search#sort')}"
QUERY = rf"{stac_prefix}(.*){re.escape('/item-search#query')}"
FILTER = rf"{stac_prefix}(.*){re.escape('/item-search#filter')}"
COLLECTIONS = re.escape(
"http://www.opengis.net/spec/ogcapi-features-1/1.0/conf/oas30"
)
OGCAPI_FEATURES = rf"{stac_prefix}(.*){re.escape('/ogcapi-features')}"


CONFORMANCE_URIS = {c.name: c.value for c in ConformanceClasses}
48 changes: 28 additions & 20 deletions pystac_client/item_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
QueryLike = Union[Query, List[str]]

FilterLangLike = str
FilterLike = Union[dict, str]
FilterLike = Union[Dict[str, Any], str]

Sortby = List[Dict[str, str]]
SortbyLike = Union[Sortby, str, List[str]]
Expand Down Expand Up @@ -240,7 +240,8 @@ def __init__(
self._stac_io = stac_io
else:
self._stac_io = StacApiIO()
self._stac_io.assert_conforms_to(ConformanceClasses.ITEM_SEARCH)

self._assert_conforms_to(ConformanceClasses.ITEM_SEARCH)

self._max_items = max_items
if self._max_items is not None and limit is not None:
Expand All @@ -267,6 +268,10 @@ def __init__(

self._parameters = {k: v for k, v in params.items() if v is not None}

# TODO: fix this with the stac_api_io() method in a future PR
def _assert_conforms_to(self, conformance_class: ConformanceClasses) -> None:
self._stac_io.assert_conforms_to(conformance_class) # type: ignore

def get_parameters(self) -> Dict[str, Any]:
if self.method == "POST":
return self._parameters
Expand All @@ -281,18 +286,20 @@ def get_parameters(self) -> Dict[str, Any]:
if "intersects" in params:
params["intersects"] = json.dumps(params["intersects"])
if "sortby" in params:
params["sortby"] = self.sortby_dict_to_str(params["sortby"])
params["sortby"] = self._sortby_dict_to_str(params["sortby"])
if "fields" in params:
params["fields"] = self.fields_dict_to_str(params["fields"])
params["fields"] = self._fields_dict_to_str(params["fields"])
return params
else:
raise Exception(f"Unsupported method {self.method}")

@staticmethod
def _format_query(value: QueryLike) -> Optional[Dict[str, Any]]:
def _format_query(self, value: QueryLike) -> Optional[Dict[str, Any]]:
if value is None:
return None
elif isinstance(value, dict):

self._assert_conforms_to(ConformanceClasses.QUERY)

if isinstance(value, dict):
return value
elif isinstance(value, list):
query: Dict[str, Any] = {}
Expand All @@ -319,7 +326,7 @@ def _format_query(value: QueryLike) -> Optional[Dict[str, Any]]:

@staticmethod
def _format_filter_lang(
_filter: FilterLike, value: FilterLangLike
_filter: Optional[FilterLike], value: Optional[FilterLangLike]
) -> Optional[str]:
if _filter is None:
return None
Expand All @@ -335,11 +342,12 @@ def _format_filter_lang(

return None

def _format_filter(self, value: FilterLike) -> Optional[dict]:
def _format_filter(self, value: Optional[FilterLike]) -> Optional[FilterLike]:
if value is None:
return None

self._stac_io.assert_conforms_to(ConformanceClasses.FILTER)
self._assert_conforms_to(ConformanceClasses.FILTER)

return value

@staticmethod
Expand Down Expand Up @@ -475,14 +483,14 @@ def _format_sortby(self, value: Optional[SortbyLike]) -> Optional[Sortby]:
if value is None:
return None

self._stac_io.assert_conforms_to(ConformanceClasses.SORT)
self._assert_conforms_to(ConformanceClasses.SORT)

if isinstance(value, str):
return [self.sortby_part_to_dict(part) for part in value.split(",")]
return [self._sortby_part_to_dict(part) for part in value.split(",")]

if isinstance(value, list):
if value and isinstance(value[0], str):
return [self.sortby_part_to_dict(v) for v in value]
return [self._sortby_part_to_dict(v) for v in value]
elif value and isinstance(value[0], dict):
return value

Expand All @@ -491,7 +499,7 @@ def _format_sortby(self, value: Optional[SortbyLike]) -> Optional[Sortby]:
)

@staticmethod
def sortby_part_to_dict(part: str) -> Dict[str, str]:
def _sortby_part_to_dict(part: str) -> Dict[str, str]:
if part.startswith("-"):
return {"field": part[1:], "direction": "desc"}
elif part.startswith("+"):
Expand All @@ -500,7 +508,7 @@ def sortby_part_to_dict(part: str) -> Dict[str, str]:
return {"field": part, "direction": "asc"}

@staticmethod
def sortby_dict_to_str(sortby: Sortby) -> str:
def _sortby_dict_to_str(sortby: Sortby) -> str:
return ",".join(
[
f"{'+' if sort['direction'] == 'asc' else '-'}{sort['field']}"
Expand All @@ -512,12 +520,12 @@ def _format_fields(self, value: Optional[FieldsLike]) -> Optional[Fields]:
if value is None:
return None

self._stac_io.assert_conforms_to(ConformanceClasses.FIELDS)
self._assert_conforms_to(ConformanceClasses.FIELDS)

if isinstance(value, str):
return self.fields_to_dict(value.split(","))
return self._fields_to_dict(value.split(","))
if isinstance(value, list):
return self.fields_to_dict(value)
return self._fields_to_dict(value)
if isinstance(value, dict):
return value

Expand All @@ -526,7 +534,7 @@ def _format_fields(self, value: Optional[FieldsLike]) -> Optional[Fields]:
)

@staticmethod
def fields_to_dict(fields: List[str]) -> Fields:
def _fields_to_dict(fields: List[str]) -> Fields:
includes: List[str] = []
excludes: List[str] = []
for field in fields:
Expand All @@ -539,7 +547,7 @@ def fields_to_dict(fields: List[str]) -> Fields:
return {"includes": includes, "excludes": excludes}

@staticmethod
def fields_dict_to_str(fields: Fields) -> str:
def _fields_dict_to_str(fields: Fields) -> str:
includes = [f"+{x}" for x in fields.get("includes", [])]
excludes = [f"-{x}" for x in fields.get("excludes", [])]
return ",".join(chain(includes, excludes))
Expand Down
1 change: 1 addition & 0 deletions tests/data/planetary-computer-root.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
],
"conformsTo": [
"https://api.stacspec.org/v1.0.0-beta.1/core",
"https://api.stacspec.org/v1.0.0-beta.1/collections",
"https://api.stacspec.org/v1.0.0-beta.1/item-search",
"http://www.opengis.net/spec/ogcapi-features-1/1.0/conf/core",
"http://www.opengis.net/spec/ogcapi-features-1/1.0/conf/oas30",
Expand Down
2 changes: 1 addition & 1 deletion tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def test_get_collections_without_conformance(self, requests_mock):

# Remove the collections conformance class
pc_root_dict["conformsTo"].remove(
"http://www.opengis.net/spec/ogcapi-features-1/1.0/conf/oas30"
"https://api.stacspec.org/v1.0.0-beta.1/collections"
)

# Remove all child links except for the collection that we are mocking
Expand Down
13 changes: 7 additions & 6 deletions tests/test_item_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -667,15 +667,16 @@ def test_query_json_syntax(self) -> None:


def test_query_json_syntax() -> None:
assert ItemSearch._format_query(['{"eo:cloud_cover": { "gte": 0, "lte": 1 }}']) == {
item_search = ItemSearch("")
assert item_search._format_query(
['{"eo:cloud_cover": { "gte": 0, "lte": 1 }}']
) == {"eo:cloud_cover": {"gte": 0, "lte": 1}}
assert item_search._format_query({"eo:cloud_cover": {"gte": 0, "lte": 1}}) == {
"eo:cloud_cover": {"gte": 0, "lte": 1}
}
assert ItemSearch._format_query({"eo:cloud_cover": {"gte": 0, "lte": 1}}) == {
"eo:cloud_cover": {"gte": 0, "lte": 1}
}
assert ItemSearch._format_query(["eo:cloud_cover<=1"]) == {
assert item_search._format_query(["eo:cloud_cover<=1"]) == {
"eo:cloud_cover": {"lte": "1"}
}
assert ItemSearch._format_query(["eo:cloud_cover<=1", "eo:cloud_cover>0"]) == {
assert item_search._format_query(["eo:cloud_cover<=1", "eo:cloud_cover>0"]) == {
"eo:cloud_cover": {"lte": "1", "gt": "0"}
}

0 comments on commit 49de56d

Please sign in to comment.