Smarter href construction for clients

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

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
@@ -131,7 +132,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 +141,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
@@ -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,36 @@ def get_search_link(self) -> Optional[pystac.Link]:
             ),
             None,
         )
+
+    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")
+            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
+        else:
+            collections_href = f"{self_href.rstrip('/')}/collections"
+
+        if not pystac.utils.is_absolute_href(collections_href):
+            collections_href = self._make_absolute_href(collections_href)
+
+        if collection_id is None:
+            return collections_href
+        else:
+            return f"{collections_href.rstrip('/')}/{collection_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)

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(