Add an extra SELF_CONTAINED catalog test (#658)

* Add an extra SELF_CONTAINED catalog test

* Improve unit test

* Run pre-commit

* Make linter happy

* Update test to avoid checking 'self' link

'self' link is always absolute; PySTAC avoids writing
the link if the Catalog is self contained or relative published
(besides the root catalog in the latter case).

* Inline test issue-specific catalog; check raw json

* Set root on resolved links when setting on Catalog

When setting the root on a Catalog, also set the root on
any child or item objects that are resolved. This handles the case
where child items and catalogs were created, added to a catalog,
and then that catalog added to another catalog, invalidating the
originally set root links of the leaf items and catalogs.

* Close file properly in test

* Message which catalog failed for better debugging

* Don't recursively clear the root of children/items

When clearing the root link of a Catalog, avoid clearing the
root link of the children/items. This is to avoid the root link clearing
in the "clone" method to avoid touching the un-cloned linked STAC Objects.

* Use STACObject.set_root in full_copy

This avoids logic in the child class implementations that are incompatible
with the full_copy logic, such as the Catalog's call of set_root
on of child and item links. This crawling logic is handled by full_copy
itself.

* Update CHANGELOG

Co-authored-by: Rob Emanuele <rdemanuele@gmail.com>

CHANGELOG.md

@@ -23,9 +23,10 @@
 
 ### Fixed
 
-- `generate_subcatalogs` can include multiple template values in a single subfolder layer 
+- `generate_subcatalogs` can include multiple template values in a single subfolder layer
   ([#595](https://github.com/stac-utils/pystac/pull/595))
 - Avoid implicit re-exports ([#591](https://github.com/stac-utils/pystac/pull/591))
+- Fix issue that caused incorrect root links when constructing multi-leveled catalogs ([#658](https://github.com/stac-utils/pystac/pull/658))
 - Regression where string `Enum` values were not serialized properly in methods like `Link.to_dict` ([#654](https://github.com/stac-utils/pystac/pull/654))
 
 ### Deprecated

pystac/catalog.py

@@ -191,6 +191,13 @@ def set_root(self, root: Optional["Catalog"]) -> None:
                 root._resolved_objects, self._resolved_objects
             )
 
+            # Walk through resolved object links and update the root
+            for link in self.links:
+                if link.rel == pystac.RelType.CHILD or link.rel == pystac.RelType.ITEM:
+                    target = link.target
+                    if isinstance(target, STACObject):
+                        target.set_root(root)
+
     def is_relative(self) -> bool:
         return self.catalog_type in [
             CatalogType.RELATIVE_PUBLISHED,

pystac/stac_object.py

@@ -362,7 +362,11 @@ def full_copy(
         if root is None and isinstance(clone, pystac.Catalog):
             root = clone
 
-        clone.set_root(cast(pystac.Catalog, root))
+        # Set the root of the STAC Object using the base class,
+        # avoiding child class overrides
+        # extra logic which can be incompatible with the full copy.
+        STACObject.set_root(clone, cast(pystac.Catalog, root))
+
         if parent:
             clone.set_parent(parent)
 

tests/test_catalog.py

@@ -837,6 +837,54 @@ def check_all_absolute(cat: Catalog) -> None:
                 c2.catalog_type = CatalogType.ABSOLUTE_PUBLISHED
                 check_all_absolute(c2)
 
+    def test_self_contained_catalog_collection_item_links(self) -> None:
+        """See issue https://github.com/stac-utils/pystac/issues/657"""
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            catalog = pystac.Catalog(
+                id="catalog-issue-657", description="catalog-issue-657"
+            )
+            collection = pystac.Collection(
+                "collection-issue-657",
+                "collection-issue-657",
+                pystac.Extent(
+                    spatial=pystac.SpatialExtent([[-180.0, -90.0, 180.0, 90.0]]),
+                    temporal=pystac.TemporalExtent([[datetime(2021, 11, 1), None]]),
+                ),
+                license="proprietary",
+            )
+
+            item = pystac.Item(
+                id="item-issue-657",
+                stac_extensions=[],
+                geometry=ARBITRARY_GEOM,
+                bbox=ARBITRARY_BBOX,
+                datetime=datetime(2021, 11, 1),
+                properties={},
+            )
+
+            collection.add_item(item)
+            catalog.add_child(collection)
+
+            catalog.normalize_hrefs(tmp_dir)
+            catalog.validate_all()
+
+            catalog.save(catalog_type=CatalogType.SELF_CONTAINED)
+            with open(
+                f"{tmp_dir}/collection-issue-657/item-issue-657/item-issue-657.json"
+            ) as f:
+                item_json = json.load(f)
+
+            for link in item_json["links"]:
+                # self links are always absolute
+                if link["rel"] == "self":
+                    continue
+
+                href = link["href"]
+                self.assertFalse(
+                    is_absolute_href(href),
+                    msg=f"Link with rel={link['rel']} is absolute!",
+                )
+
     def test_full_copy_and_normalize_works_with_created_stac(self) -> None:
         cat = TestCases.test_case_3()
         cat_copy = cat.full_copy()
@@ -1071,7 +1119,7 @@ def check_item(self, item: Item, tag: str) -> None:
             self.check_link(link, tag)
 
     def check_catalog(self, c: Catalog, tag: str) -> None:
-        self.assertEqual(len(c.get_links("root")), 1)
+        self.assertEqual(len(c.get_links("root")), 1, msg=f"{c}")
 
         for link in c.links:
             self.check_link(link, tag)