Don't override parent link when determining hrefs #1116

stac-utils · May 11, 2023 · 614199f · 614199f
1 parent 684b317
commit 614199f
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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")