diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a21d5f16..aa2958d3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - Make `extent` not required for `VerticalSpatialDimension` ([#1596](https://github.com/stac-utils/pystac/pull/1596)) +- `Asset.get_absolute_href()` now properly resolves root relative hrefs ([#1599](https://github.com/stac-utils/pystac/pull/1599)) ## [v1.14.1] - 2025-09-18 diff --git a/pystac/asset.py b/pystac/asset.py index 46d034bf7..ee01df74f 100644 --- a/pystac/asset.py +++ b/pystac/asset.py @@ -105,13 +105,12 @@ def get_absolute_href(self) -> str | None: str: The absolute HREF of this asset, or None if an absolute HREF could not be determined. """ - if utils.is_absolute_href(self.href): + item_self = self.owner.get_self_href() if self.owner is not None else None + if utils.is_absolute_href(self.href, item_self): return self.href else: - if self.owner is not None: - item_self = self.owner.get_self_href() - if item_self is not None: - return utils.make_absolute_href(self.href, item_self) + if item_self is not None: + return utils.make_absolute_href(self.href, item_self) return None def to_dict(self) -> dict[str, Any]: @@ -341,7 +340,7 @@ def make_asset_hrefs_relative(self) -> Assets: """ self_href = self.get_self_href() for asset in self.assets.values(): - if is_absolute_href(asset.href): + if is_absolute_href(asset.href, self_href): if self_href is None: raise STACError( "Cannot make asset HREFs relative if no self_href is set." @@ -360,7 +359,7 @@ def make_asset_hrefs_absolute(self) -> Assets: """ self_href = self.get_self_href() for asset in self.assets.values(): - if not is_absolute_href(asset.href): + if not is_absolute_href(asset.href, self_href): if self_href is None: raise STACError( "Cannot make relative asset HREFs absolute " @@ -380,10 +379,10 @@ def get_self_href(self) -> str | None: def _absolute_href(href: str, owner: Assets | None, action: str = "access") -> str: - if utils.is_absolute_href(href): + item_self = owner.get_self_href() if owner else None + if utils.is_absolute_href(href, item_self): return href else: - item_self = owner.get_self_href() if owner else None if item_self is None: raise ValueError( f"Cannot {action} file if asset href ('{href}') is relative " diff --git a/pystac/utils.py b/pystac/utils.py index 4225fd1d5..ffd368436 100644 --- a/pystac/utils.py +++ b/pystac/utils.py @@ -378,19 +378,27 @@ def make_absolute_href( return _make_absolute_href_path(parsed_source, parsed_start, start_is_dir) -def is_absolute_href(href: str) -> bool: +def is_absolute_href(href: str, start_href: str | None = None) -> bool: """Determines if an HREF is absolute or not. May be used on either local file paths or URLs. Args: href : The HREF to consider. + start_href : The HREF that will be used as the basis for checking if + ``source_href`` is a relative path. Defaults to None. Returns: bool: ``True`` if the given HREF is absolute, ``False`` if it is relative. """ parsed = safe_urlparse(href) - return parsed.scheme not in ["", "file"] or os.path.isabs(parsed.path) + if parsed.scheme not in ["", "file"]: + return True + else: + parsed_start_scheme = ( + "" if start_href is None else safe_urlparse(start_href).scheme + ) + return parsed_start_scheme in ["", "file"] and os.path.isabs(parsed.path) def datetime_to_str(dt: datetime, timespec: str = "auto") -> str: diff --git a/tests/test_asset.py b/tests/test_asset.py index b41113403..b38cdb36f 100644 --- a/tests/test_asset.py +++ b/tests/test_asset.py @@ -102,3 +102,112 @@ def test_delete_asset_relative_no_owner_fails(tmp_asset: pystac.Asset) -> None: assert asset.href in str(e.value) assert os.path.exists(href) + + +@pytest.mark.parametrize( + "self_href, asset_href, expected_href", + ( + ( + "http://test.com/stac/catalog/myitem.json", + "asset.data", + "http://test.com/stac/catalog/asset.data", + ), + ( + "http://test.com/stac/catalog/myitem.json", + "/asset.data", + "http://test.com/asset.data", + ), + ), +) +def test_asset_get_absolute_href( + tmp_asset: pystac.Asset, + self_href: str, + asset_href: str, + expected_href: str, +) -> None: + asset = tmp_asset + item = asset.owner + + if not isinstance(item, pystac.Item): + raise TypeError("Asset must belong to an Item") + + # Set the item HREF as per test + item.set_self_href(self_href) + assert item.get_self_href() == self_href + + # Set the asset HREF as per test and check expected output + asset.href = asset_href + assert asset.get_absolute_href() == expected_href + + +@pytest.mark.skipif(os.name == "nt", reason="Unix only test") +@pytest.mark.parametrize( + "self_href, asset_href, expected_href", + ( + ( + "/local/myitem.json", + "asset.data", + "/local/asset.data", + ), + ( + "/local/myitem.json", + "subdir/asset.data", + "/local/subdir/asset.data", + ), + ( + "/local/myitem.json", + "/absolute/asset.data", + "/absolute/asset.data", + ), + ), +) +def test_asset_get_absolute_href_unix( + tmp_asset: pystac.Asset, + self_href: str, + asset_href: str, + expected_href: str, +) -> None: + test_asset_get_absolute_href(tmp_asset, self_href, asset_href, expected_href) + + +@pytest.mark.skipif(os.name != "nt", reason="Windows only test") +@pytest.mark.parametrize( + "self_href, asset_href, expected_href", + ( + ( + "{tmpdir}/myitem.json", + "asset.data", + "{tmpdir}/asset.data", + ), + ( + "{tmpdir}/myitem.json", + "subdir/asset.data", + "{tmpdir}/subdir/asset.data", + ), + ( + "{tmpdir}/myitem.json", + "c:/absolute/asset.data", + "c:/absolute/asset.data", + ), + ( + "{tmpdir}/myitem.json", + "d:\\absolute\\asset.data", + "d:\\absolute\\asset.data", + ), + ), +) +def test_asset_get_absolute_href_windows( + tmp_path: Path, + tmp_asset: pystac.Asset, + self_href: str, + asset_href: str, + expected_href: str, +) -> None: + # For windows, we need an actual existing temporary directory + tmpdir = tmp_path.as_posix() + test_asset_get_absolute_href( + tmp_asset, + self_href.format(tmpdir=tmpdir), + asset_href.format(tmpdir=tmpdir), + expected_href.format(tmpdir=tmpdir), + ) diff --git a/tests/test_utils.py b/tests/test_utils.py index 8b53daaa6..c9f0407f2 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -197,14 +197,15 @@ def test_make_absolute_href_windows( def test_is_absolute_href() -> None: # Test cases of (href, expected) test_cases = [ - ("item.json", False), - ("./item.json", False), - ("../item.json", False), - ("http://stacspec.org/item.json", True), + ("item.json", False, None), + ("./item.json", False, None), + ("../item.json", False, None), + ("http://stacspec.org/item.json", True, None), + ("http://stacspec.org/item.json", True, "http://stacspec.org/"), ] - for href, expected in test_cases: - actual = is_absolute_href(href) + for href, expected, start_href in test_cases: + actual = is_absolute_href(href, start_href) assert actual == expected @@ -214,15 +215,16 @@ def test_is_absolute_href_os_aware() -> None: is_windows = os.name == "nt" incl_drive_letter = path_includes_drive_letter() test_cases = [ - ("/item.json", not incl_drive_letter), - ("/home/someuser/Downloads/item.json", not incl_drive_letter), - ("file:///home/someuser/Downloads/item.json", not incl_drive_letter), - ("d:/item.json", is_windows), - ("c:/files/more_files/item.json", is_windows), + ("/item.json", not incl_drive_letter, None), + ("/item.json", False, "http://stacspec.org/"), + ("/home/someuser/Downloads/item.json", not incl_drive_letter, None), + ("file:///home/someuser/Downloads/item.json", not incl_drive_letter, None), + ("d:/item.json", is_windows, None), + ("c:/files/more_files/item.json", is_windows, None), ] - for href, expected in test_cases: - actual = is_absolute_href(href) + for href, expected, start_href in test_cases: + actual = is_absolute_href(href, start_href) assert actual == expected @@ -231,15 +233,15 @@ def test_is_absolute_href_windows() -> None: # Test cases of (href, expected) test_cases = [ - ("item.json", False), - (".\\item.json", False), - ("..\\item.json", False), - ("c:\\item.json", True), - ("http://stacspec.org/item.json", True), + ("item.json", False, None), + (".\\item.json", False, None), + ("..\\item.json", False, None), + ("c:\\item.json", True, None), + ("http://stacspec.org/item.json", True, None), ] - for href, expected in test_cases: - actual = is_absolute_href(href) + for href, expected, start_href in test_cases: + actual = is_absolute_href(href, start_href) assert actual == expected