From 614199f827dfff1e9240947cc1aa265009b4a333 Mon Sep 17 00:00:00 2001 From: Matthias Mohr Date: Fri, 12 May 2023 00:08:34 +0200 Subject: [PATCH] Don't override parent link when determining hrefs #1116 --- CHANGELOG.md | 5 +++++ pystac/catalog.py | 14 +++++++++++--- pystac/link.py | 3 ++- tests/test_catalog.py | 22 ++++++++++++++++++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8677c4c1..710395dca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ - Catalog `get_item()`. Use `get_items(id)` instead ([#1075](https://github.com/stac-utils/pystac/pull/1075)) - Catalog and Collection `get_all_items`. Use `get_items(recursive=True)` instead ([#1075](https://github.com/stac-utils/pystac/pull/1075)) +### Fixed + +- If a child catalog is linked to from two parents, the creation folder depends on the parent link instead of relying on the order of add_child calls ([#1118](https://github.com/stac-utils/pystac/pull/1118)) +- Don't override an existing parent link when determining the href's ([#1118](https://github.com/stac-utils/pystac/pull/1118)) + ## [v1.7.3] ### Fixed diff --git a/pystac/catalog.py b/pystac/catalog.py index da74b0b39..4f12798b8 100644 --- a/pystac/catalog.py +++ b/pystac/catalog.py @@ -718,7 +718,9 @@ def normalize_hrefs( if not is_absolute_href(root_href): root_href = make_absolute_href(root_href, os.getcwd(), start_is_dir=True) - def process_item(item: Item, _root_href: str) -> Callable[[], None]: + def process_item( + item: Item, _root_href: str, parent: Catalog + ) -> Callable[[], None]: if not skip_unresolved: item.resolve_links() @@ -730,7 +732,7 @@ def fn() -> None: return fn def process_catalog( - cat: Catalog, _root_href: str, is_root: bool + cat: Catalog, _root_href: str, is_root: bool, parent: Catalog = None ) -> List[Callable[[], None]]: setter_funcs: List[Callable[[], None]] = [] @@ -740,13 +742,18 @@ def process_catalog( new_self_href = _strategy.get_href(cat, _root_href, is_root) new_root = os.path.dirname(new_self_href) + # Abort as the intended parent is not the actual parent + # https://github.com/stac-utils/pystac/issues/1116 + if parent is not None and cat.get_parent() != parent: + return setter_funcs + for link in cat.get_links(): if skip_unresolved and not link.is_resolved(): continue elif link.rel == pystac.RelType.ITEM: link.resolve_stac_object(root=self.get_root()) setter_funcs.append( - process_item(cast(pystac.Item, link.target), new_root) + process_item(cast(pystac.Item, link.target), new_root, cat) ) elif link.rel == pystac.RelType.CHILD: link.resolve_stac_object(root=self.get_root()) @@ -755,6 +762,7 @@ def process_catalog( cast(Union[pystac.Catalog, pystac.Collection], link.target), new_root, is_root=False, + parent=cat, ) ) diff --git a/pystac/link.py b/pystac/link.py index 33572a5ae..7aaf4cc39 100644 --- a/pystac/link.py +++ b/pystac/link.py @@ -334,7 +334,8 @@ def resolve_stac_object(self, root: Optional[Catalog] = None) -> Link: and isinstance(self.owner, pystac.Catalog) ): assert self._target_object - self._target_object.set_parent(self.owner) + if self._target_object.get_parent() is None: + self._target_object.set_parent(self.owner) return self diff --git a/tests/test_catalog.py b/tests/test_catalog.py index 090bb22c7..d6dd9f64e 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -264,6 +264,28 @@ def test_add_item_override_parent(self) -> None: parent2.add_item(child) assert child.get_parent() is parent2 + def test_add_child_store_at_parent_location(self) -> None: + root = pystac.Catalog("root", "root") + left = pystac.Catalog("left", "left") + right = pystac.Catalog("right", "right") + + root.add_child(left) + root.add_child(right) + + child = pystac.Catalog("child", "child") + + left.add_child(child) + right.add_child(child, keep_parent=True) + + with tempfile.TemporaryDirectory() as temporary_directory: + root.normalize_and_save( + temporary_directory, pystac.CatalogType.ABSOLUTE_PUBLISHED + ) + correct_path = os.path.join(temporary_directory, "left/child/catalog.json") + wrong_path = os.path.join(temporary_directory, "right/child/catalog.json") + assert os.path.exists(correct_path) + assert not os.path.exists(wrong_path) + def test_add_item_keep_parent(self) -> None: parent1 = Catalog(id="parent1", description="test1") parent2 = Catalog(id="parent2", description="test2")