From bd5b8177d6e6cdda7b65c3c284a0532055913201 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 17 Jan 2023 08:29:54 -0700 Subject: [PATCH 1/6] docs, fix: spelling --- pystac_client/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pystac_client/client.py b/pystac_client/client.py index 7e734bdd..fae5a202 100644 --- a/pystac_client/client.py +++ b/pystac_client/client.py @@ -131,7 +131,7 @@ def open( After getting a child collection with, e.g. :meth:`Client.get_collection`, the child items of that collection will still be signed with ``modifier``. - request_modifier: A callable that eitehr modifies a `Request` instance or + request_modifier: A callable that either modifies a `Request` instance or returns a new one. This can be useful for injecting Authentication headers and/or signing fully-formed requests (e.g. signing requests using AWS SigV4). @@ -140,7 +140,7 @@ def open( of :class:`requests.Request`. If the callable returns a `requests.Request`, that will be used. - Alternately, the calable may simply modify the provided request object + Alternately, the callable may simply modify the provided request object and return `None`. stac_io: A `StacApiIO` object to use for I/O requests. Generally, leave this to the default. However in cases where customized I/O processing From 36b0d57d127484580c8c10d65f71adb627734df1 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 17 Jan 2023 08:36:41 -0700 Subject: [PATCH 2/6] fix: don't strip slash from root href Some servers require it (per https://github.com/stac-utils/pystac-client/pull/373#issuecomment-1384088271). --- pystac_client/client.py | 47 +++++++++++++++++++++--- tests/test_client.py | 80 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/pystac_client/client.py b/pystac_client/client.py index fae5a202..9cdeda14 100644 --- a/pystac_client/client.py +++ b/pystac_client/client.py @@ -12,6 +12,7 @@ ) import pystac +import pystac.utils import pystac.validation from pystac import CatalogType, Collection from requests import Request @@ -149,7 +150,6 @@ def open( Return: catalog : A :class:`Client` instance for this Catalog/API """ - url = url.rstrip("/") client: Client = cls.from_file( url, headers=headers, @@ -254,7 +254,7 @@ def get_collection(self, collection_id: str) -> Optional[Collection]: CollectionClient: A STAC Collection """ if self._supports_collections() and self._stac_io: - url = f"{self.get_self_href()}/collections/{collection_id}" + url = self._get_collections_href(collection_id) collection = CollectionClient.from_dict( self._stac_io.read_json(url), root=self, @@ -281,9 +281,9 @@ def get_collections(self) -> Iterator[Collection]: """ collection: Union[Collection, CollectionClient] - if self._supports_collections() and self.get_self_href() is not None: - url = f"{self.get_self_href()}/collections" - for page in self._stac_io.get_pages(url): # type: ignore + if self._supports_collections() and self._stac_io: + url = self._get_collections_href() + for page in self._stac_io.get_pages(url): if "collections" not in page: raise APIError("Invalid response from /collections") for col in page["collections"]: @@ -504,3 +504,40 @@ def get_search_link(self) -> Optional[pystac.Link]: ), None, ) + + def _get_collections_href(self, id: Optional[str] = None) -> str: + self_href = self.get_self_href() + if self_href is None: + data_link = self.get_single_link("data") + if data_link is None: + raise ValueError( + "cannot build a collections href without a self href or a data link" + ) + else: + collections_href = data_link.href + elif self_href.endswith("/"): + collections_href = f"{self_href}collections" + else: + collections_href = f"{self_href}/collections" + + if not pystac.utils.is_absolute_href(collections_href): + collections_href = self._make_absolute_href(collections_href) + + if id is None: + return collections_href + elif collections_href.endswith("/"): + return f"{collections_href}{id}" + else: + return f"{collections_href}/{id}" + + def _make_absolute_href(self, href: str) -> str: + self_link = self.get_single_link("self") + if self_link is None: + raise ValueError("cannot build an absolute href without a self link") + elif not pystac.utils.is_absolute_href(self_link.href): + raise ValueError( + "cannot build an absolute href from " + f"a relative self link: {self_link.href}" + ) + else: + return pystac.utils.make_absolute_href(href, self_link.href) diff --git a/tests/test_client.py b/tests/test_client.py index ff07a486..d9a7f66a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -3,7 +3,7 @@ import warnings from datetime import datetime from tempfile import TemporaryDirectory -from typing import Any +from typing import Any, Dict from urllib.parse import parse_qs, urlsplit import pystac @@ -146,6 +146,84 @@ def test_get_collections_single_slash(self, requests_mock: Mocker) -> None: assert len(history) == 2 assert history[1].url == f"{root_url}collections" + def test_keep_trailing_slash_on_root(self, requests_mock: Mocker) -> None: + pc_root_text = read_data_file("planetary-computer-root.json") + root_url = "http://pystac-client.test/" + requests_mock.get(root_url, status_code=200, text=pc_root_text) + client = Client.open(root_url) + self_href = client.get_self_href() + assert self_href + assert self_href.endswith("/") + + def test_fall_back_to_data_link_for_collections( + self, requests_mock: Mocker + ) -> None: + pc_root_text = read_data_file("planetary-computer-root.json") + root_url = "http://pystac-client.test/" + requests_mock.get(root_url, status_code=200, text=pc_root_text) + api = Client.open(root_url) + api.set_self_href(None) + pc_collection_dict = read_data_file( + "planetary-computer-aster-l1t-collection.json", parse_json=True + ) + requests_mock.get( + # the href of the data link + "https://planetarycomputer.microsoft.com/api/stac/v1/collections", + status_code=200, + json={"collections": [pc_collection_dict], "links": []}, + ) + _ = next(api.get_collections()) + history = requests_mock.request_history + assert len(history) == 2 + assert ( + history[1].url + == "https://planetarycomputer.microsoft.com/api/stac/v1/collections" + ) + + def test_build_absolute_href_from_data_link(self, requests_mock: Mocker) -> None: + pc_root = read_data_file("planetary-computer-root.json", parse_json=True) + assert isinstance(pc_root, Dict) + for link in pc_root["links"]: + if link["rel"] == "data": + link["href"] = "./collections" + root_url = "http://pystac-client.test/" + requests_mock.get(root_url, status_code=200, text=json.dumps(pc_root)) + api = Client.open(root_url) + api.set_self_href(None) + api.add_link( + pystac.Link( + rel="self", + target="https://planetarycomputer.microsoft.com/api/stac/v1/", + ) + ) + pc_collection_dict = read_data_file( + "planetary-computer-aster-l1t-collection.json", parse_json=True + ) + requests_mock.get( + # the href of the data link + "https://planetarycomputer.microsoft.com/api/stac/v1/collections", + status_code=200, + json={"collections": [pc_collection_dict], "links": []}, + ) + _ = next(api.get_collections()) + history = requests_mock.request_history + assert len(history) == 2 + assert ( + history[1].url + == "https://planetarycomputer.microsoft.com/api/stac/v1/collections" + ) + + def test_error_if_no_self_href_or_data_link(self, requests_mock: Mocker) -> None: + pc_root = read_data_file("planetary-computer-root.json", parse_json=True) + assert isinstance(pc_root, Dict) + pc_root["links"] = [link for link in pc_root["links"] if link["rel"] != "data"] + root_url = "http://pystac-client.test/" + requests_mock.get(root_url, status_code=200, text=json.dumps(pc_root)) + api = Client.open(root_url) + api.set_self_href(None) + with pytest.raises(ValueError): + _ = api.get_collection("an-id") + def test_custom_request_parameters(self, requests_mock: Mocker) -> None: pc_root_text = read_data_file("planetary-computer-root.json") pc_collection_dict = read_data_file( From 6f722598b13cafdc830c9ab98a31499f75510280 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 17 Jan 2023 08:40:54 -0700 Subject: [PATCH 3/6] chore: update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f157b9..e28b3cb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Some mishandled cases for datetime intervals [#363](https://github.com/stac-utils/pystac-client/pull/363) -- Collection requests when the Client's url ends in a '/' [#373](https://github.com/stac-utils/pystac-client/pull/373) +- Collection requests when the Client's url ends in a '/' [#373](https://github.com/stac-utils/pystac-client/pull/373), [#405](https://github.com/stac-utils/pystac-client/pull/405) - Parse datetimes more strictly [#364](https://github.com/stac-utils/pystac-client/pull/364) ### Removed From 858b97c6e1613694eb812aff2cdb06f6033b2829 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 19 Jan 2023 05:38:37 -0700 Subject: [PATCH 4/6] fixup: prevent double-slashes from self_href Co-authored-by: Abhishek Manandhar --- pystac_client/client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pystac_client/client.py b/pystac_client/client.py index 9cdeda14..daf17251 100644 --- a/pystac_client/client.py +++ b/pystac_client/client.py @@ -515,10 +515,8 @@ def _get_collections_href(self, id: Optional[str] = None) -> str: ) else: collections_href = data_link.href - elif self_href.endswith("/"): - collections_href = f"{self_href}collections" else: - collections_href = f"{self_href}/collections" + collections_href = f"{self_href.rstrip('/')}/collections" if not pystac.utils.is_absolute_href(collections_href): collections_href = self._make_absolute_href(collections_href) From 90b1dbd4ac0932a45699b2db0811aa11d06dce04 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 19 Jan 2023 05:38:57 -0700 Subject: [PATCH 5/6] fixup: prevent double-slashes from collections_href Co-authored-by: Abhishek Manandhar --- pystac_client/client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pystac_client/client.py b/pystac_client/client.py index daf17251..c0fb7e08 100644 --- a/pystac_client/client.py +++ b/pystac_client/client.py @@ -523,10 +523,8 @@ def _get_collections_href(self, id: Optional[str] = None) -> str: if id is None: return collections_href - elif collections_href.endswith("/"): - return f"{collections_href}{id}" else: - return f"{collections_href}/{id}" + return f"{collections_href.rstrip('/')}/{id}" def _make_absolute_href(self, href: str) -> str: self_link = self.get_single_link("self") From d6e8d84b7d33f651305cadaf61db3bfe31dd921d Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 19 Jan 2023 05:42:21 -0700 Subject: [PATCH 6/6] fixup: use `collection_id`, not `id` --- pystac_client/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pystac_client/client.py b/pystac_client/client.py index c0fb7e08..655b02b1 100644 --- a/pystac_client/client.py +++ b/pystac_client/client.py @@ -505,7 +505,7 @@ def get_search_link(self) -> Optional[pystac.Link]: None, ) - def _get_collections_href(self, id: Optional[str] = None) -> str: + def _get_collections_href(self, collection_id: Optional[str] = None) -> str: self_href = self.get_self_href() if self_href is None: data_link = self.get_single_link("data") @@ -521,10 +521,10 @@ def _get_collections_href(self, id: Optional[str] = None) -> str: if not pystac.utils.is_absolute_href(collections_href): collections_href = self._make_absolute_href(collections_href) - if id is None: + if collection_id is None: return collections_href else: - return f"{collections_href.rstrip('/')}/{id}" + return f"{collections_href.rstrip('/')}/{collection_id}" def _make_absolute_href(self, href: str) -> str: self_link = self.get_single_link("self")