From c275ca7bff70973ec0f18e7643bb78c38a3cf04c Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 13 Jul 2021 09:35:44 -0600 Subject: [PATCH 1/4] Allow resolved self links When getting a STAC object's self href, there was a cast that assumed that the object's self link's target was a string, which was not always the case (the self link's target could be a string or another STAC object). This commit removes that cast by splitting a link's target into two separate "private" attributes, `_target_href` and `_target_object`. Fixes #499. --- pystac/link.py | 68 ++++++++++++++++++++++++++++++------------- pystac/stac_object.py | 2 +- tests/test_link.py | 13 +++++++++ 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/pystac/link.py b/pystac/link.py index 5f27d56ff..345714f11 100644 --- a/pystac/link.py +++ b/pystac/link.py @@ -1,5 +1,5 @@ from copy import copy -from typing import Any, Dict, Optional, TYPE_CHECKING, Union, cast +from typing import Any, Dict, Optional, TYPE_CHECKING, Union import pystac from pystac.utils import make_absolute_href, make_relative_href, is_absolute_href @@ -52,11 +52,6 @@ class Link: """The relation of the link (e.g. 'child', 'item'). Registered rel Types are preferred. See :class:`~pystac.RelType` for common media types.""" - target: Union[str, "STACObject_Type"] - """The target of the link. If the link is unresolved, or the link is to something - that is not a STACObject, the target is an HREF. If resolved, the target is a - STACObject.""" - media_type: Optional[str] """Optional description of the media type. Registered Media Types are preferred. See :class:`~pystac.MediaType` for common media types.""" @@ -82,7 +77,12 @@ def __init__( extra_fields: Optional[Dict[str, Any]] = None, ) -> None: self.rel = rel - self.target = target + if isinstance(target, str): + self._target_href: Optional[str] = target + self._target_object = None + else: + self._target_href = None + self._target_object = target self.media_type = media_type self.title = title self.extra_fields = extra_fields or {} @@ -119,10 +119,10 @@ def get_href(self) -> Optional[str]: In all other cases, this method will return an absolute HREF. """ # get the self href - if self.is_resolved(): - href = cast(pystac.STACObject, self.target).get_self_href() + if self._target_object: + href = self._target_object.get_self_href() else: - href = cast(Optional[str], self.target) + href = self._target_href if href and is_absolute_href(href) and self.owner and self.owner.get_root(): root = self.owner.get_root() @@ -158,16 +158,42 @@ def get_absolute_href(self) -> Optional[str]: from this link; however, if the link is relative, has no owner, and has an unresolved target, this will return a relative HREF. """ - if self.is_resolved(): - href = cast(pystac.STACObject, self.target).get_self_href() + if self._target_object: + href = self._target_object.get_self_href() else: - href = cast(Optional[str], self.target) + href = self._target_href if href is not None and self.owner is not None: href = make_absolute_href(href, self.owner.get_self_href()) return href + @property + def target(self) -> Union[str, "STACObject_Type"]: + """The target of the link. If the link is unresolved, or the link is to something + that is not a STACObject, the target is an HREF. If resolved, the target is a + STACObject.""" + if self._target_object: + return self._target_object + elif self._target_href: + return self._target_href + else: + raise ValueError("No target defined for link.") + + @target.setter + def target(self, target: Union[str, "STACObject_Type"]) -> None: + """Sets this link's target to a string or a STAC object.""" + if isinstance(target, str): + self._target_href = target + self._target_object = None + else: + self._target_href = None + self._target_object = target + + def get_target_str(self) -> Optional[str]: + """Returns this link's target as a string.""" + return self._target_href + def __repr__(self) -> str: return "".format(self.rel, self.target) @@ -180,8 +206,10 @@ def resolve_stac_object(self, root: Optional["Catalog_Type"] = None) -> "Link": If provided, the root's resolved object cache is used to search for previously resolved instances of the STAC object. """ - if isinstance(self.target, str): - target_href = self.target + if self._target_object: + pass + elif self._target_href: + target_href = self._target_href # If it's a relative link, base it off the parent. if not is_absolute_href(target_href): @@ -221,17 +249,17 @@ def resolve_stac_object(self, root: Optional["Catalog_Type"] = None) -> "Link": if root is not None: obj = root._resolved_objects.get_or_cache(obj) obj.set_root(root) + self._target_object = obj else: - obj = self.target - - self.target = obj + raise ValueError("Cannot resolve STAC object without a target") if ( self.owner and self.rel in [pystac.RelType.CHILD, pystac.RelType.ITEM] and isinstance(self.owner, pystac.Catalog) ): - self.target.set_parent(self.owner) + assert self._target_object + self._target_object.set_parent(self.owner) return self @@ -241,7 +269,7 @@ def is_resolved(self) -> bool: Returns: bool: True if this link is resolved. """ - return not isinstance(self.target, str) + return self._target_object is not None def to_dict(self) -> Dict[str, Any]: """Generate a dictionary representing the JSON of this serialized Link. diff --git a/pystac/stac_object.py b/pystac/stac_object.py index 29e26f5c5..400b5fdea 100644 --- a/pystac/stac_object.py +++ b/pystac/stac_object.py @@ -159,7 +159,7 @@ def get_self_href(self) -> Optional[str]: """ self_link = self.get_single_link(pystac.RelType.SELF) if self_link: - return cast(str, self_link.target) + return self_link.get_target_str() else: return None diff --git a/tests/test_link.py b/tests/test_link.py index 6d76d45ef..8a2e81b6c 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -1,5 +1,7 @@ import datetime +import os.path import unittest +from tempfile import TemporaryDirectory from typing import Any, Dict, List import pystac @@ -82,6 +84,17 @@ def test_resolve_stac_object_no_root_and_target_is_item(self) -> None: link = pystac.Link("my rel", target=self.item) link.resolve_stac_object() + def test_resolved_self_href(self) -> None: + catalog = pystac.Catalog(id="test", description="test desc") + with TemporaryDirectory() as temporary_directory: + catalog.normalize_and_save(temporary_directory) + path = os.path.join(temporary_directory, "catalog.json") + catalog = pystac.Catalog.from_file(path) + link = catalog.get_single_link(pystac.RelType.SELF) + assert link + link.resolve_stac_object() + self.assertEqual(link.get_absolute_href(), path) + class StaticLinkTest(unittest.TestCase): def setUp(self) -> None: From 46556a24e9e1b17d0d0e96a4d4c22292fb011960 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Tue, 13 Jul 2021 09:53:07 -0600 Subject: [PATCH 2/4] Update changelog for #555 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78a677758..8155507f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ ### Fixed - Added `Collections` as a type that can be extended for extensions whose fields can appear in collection summaries ([#547](https://github.com/stac-utils/pystac/pull/547)) +- Allow resolved self links when getting an object's self href ([#555](https://github.com/stac-utils/pystac/pull/555)) ### Deprecated From 3561a5c718471da5716f52f917698089b84edd59 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 15 Jul 2021 07:31:47 -0600 Subject: [PATCH 3/4] Update `get_target_str` to fall back to object --- pystac/link.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pystac/link.py b/pystac/link.py index 345714f11..49e31985c 100644 --- a/pystac/link.py +++ b/pystac/link.py @@ -191,8 +191,17 @@ def target(self, target: Union[str, "STACObject_Type"]) -> None: self._target_object = target def get_target_str(self) -> Optional[str]: - """Returns this link's target as a string.""" - return self._target_href + """Returns this link's target as a string. + + If a string href was provided, returns that. If not, tries to resolve + the self link of the target object. + """ + if self._target_href: + return self._target_href + elif self._target_object: + return self._target_object.get_self_href() + else: + return None def __repr__(self) -> str: return "".format(self.rel, self.target) From 21feb889d0e05fd29a96096d112fdc639024c669 Mon Sep 17 00:00:00 2001 From: Pete Gadomski Date: Thu, 15 Jul 2021 07:59:52 -0600 Subject: [PATCH 4/4] Add tests and recursion check for link str --- pystac/link.py | 4 ++++ pystac/stac_object.py | 2 +- tests/test_link.py | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pystac/link.py b/pystac/link.py index 49e31985c..b0a20664d 100644 --- a/pystac/link.py +++ b/pystac/link.py @@ -203,6 +203,10 @@ def get_target_str(self) -> Optional[str]: else: return None + def has_target_href(self) -> bool: + """Returns true if this link has a string href in its target information.""" + return self._target_href is not None + def __repr__(self) -> str: return "".format(self.rel, self.target) diff --git a/pystac/stac_object.py b/pystac/stac_object.py index 400b5fdea..e7aeb5104 100644 --- a/pystac/stac_object.py +++ b/pystac/stac_object.py @@ -158,7 +158,7 @@ def get_self_href(self) -> Optional[str]: links have absolute (as opposed to relative) HREFs. """ self_link = self.get_single_link(pystac.RelType.SELF) - if self_link: + if self_link and self_link.has_target_href(): return self_link.get_target_str() else: return None diff --git a/tests/test_link.py b/tests/test_link.py index 8a2e81b6c..3710e0d53 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -95,6 +95,24 @@ def test_resolved_self_href(self) -> None: link.resolve_stac_object() self.assertEqual(link.get_absolute_href(), path) + def test_target_getter_setter(self) -> None: + link = pystac.Link("my rel", target="./foo/bar.json") + self.assertEqual(link.target, "./foo/bar.json") + self.assertEqual(link.get_target_str(), "./foo/bar.json") + + link.target = self.item + self.assertEqual(link.target, self.item) + self.assertEqual(link.get_target_str(), self.item.get_self_href()) + + link.target = "./bar/foo.json" + self.assertEqual(link.target, "./bar/foo.json") + + def test_get_target_str_no_href(self) -> None: + self.item.remove_links("self") + link = pystac.Link("self", target=self.item) + self.item.add_link(link) + self.assertIsNone(link.get_target_str()) + class StaticLinkTest(unittest.TestCase): def setUp(self) -> None: