Add an extra SELF_CONTAINED catalog test

[pomadchin]

Related Issue(s): #657

Description:
Adds a unit test that tests #657

I notcied that there is an issue only with catalogs of the full hierarchy:

• catalog
  • collection
    • item

The following one works as expected:

• catalog
  • item

PR Checklist:

• Code is formatted (run pre-commit run --all-files)
• Tests pass (run scripts/test)
• Documentation has been updated to reflect changes, if applicable
• This PR maintains or improves overall codebase code coverage.
• Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

[lossyrob]

I just updated to avoid checking the "self" link, which per the spec is always absolute. This ends up making the test pass. I'm going to try and get it to re-fail in the way @pomadchin mentioned in Gitter (where the rel="collection" link was absolute from a saved self-contained catalog).

[pomadchin]

@lossyrob I can parse it via json.loads to make it more explicit, do you want me to try that?

[lossyrob]

@pomadchin I'm poking around a bit to see if I can trigger it again, will probably use that, but let me take a swing and report back

[lossyrob]

This issue is caused by the order of operations when creating the collection/catalogs, and the logic surrounding deciding whether a link HREF should be absolute or relative based on the link owner's root.

When the Item is first added to the Collection, that Collection is set as the Item's root. When that Collection is added to the Catalog, the Catalog is set as the Collection's root, but the Item's root stays set as the Collection. When the save method is called, it changes the Catalog's type to SELF_CONTAINED, which means that any link owned by a STACObject whose root is that Catalog will return a relative HREF. However, as the Item's root is still the Collection, and that Collection's type never changed from the default ABSOLUTE_PUBLISHED, the links owned by that Item returns an absolute href.

I added some logic that, when setting the root of a Catalog, crawls through the resolved links of that Catalog and recursively sets the root as the new root passed in.

This seems to have caused other errors in other tests, so I'll investigate why that is.

[lossyrob]

Inlined the creation of this catalog as it's specific to that issue, the "test_case_X" methods are meant to represent a set of reusable test Catalogs that are used through many tests.

[codecov-commenter]

Codecov Report

Merging #658 (e4dbf36) into main (c815552) will decrease coverage by 0.00%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##             main     #658      +/-   ##
==========================================
- Coverage   94.21%   94.20%   -0.01%     
==========================================
  Files          77       77              
  Lines       11079    11101      +22     
  Branches     1075     1080       +5     
==========================================
+ Hits        10438    10458      +20     
- Misses        465      466       +1     
- Partials      176      177       +1     

Impacted Files	Coverage Δ
tests/test_catalog.py	98.93% <88.88%> (-0.25%)	⬇️
pystac/catalog.py	92.00% <100.00%> (+0.10%)	⬆️
pystac/stac_object.py	90.81% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c815552...e4dbf36. Read the comment docs.

[lossyrob]

Latest commits fix this error - turns out that during the cloning and full_copy process, setting the children and item's root during the parent set_root call was causing conflicts. For clone, the purpose of calling set_root(None) is to clear only the target object's root link, and shouldn't modify the children or items. The logic in full_copy recurses the links of the clone itself, and so the new logic in set_root on Catalog was interfering with that processing.

I'd consider this a non-ideal fix, and highlights some of the complexity with root-tracking and link logic. @matthewhanson tagging you to take a look as this is an outcome of the hierarchical link refactor, which itself was to reduce some of the complexity of the previous link work. Maybe you can see a better workaround/solution?

[pomadchin]

🚀 thanks!

[duckontheweb]

Thanks @pomadchin and @lossyrob for getting this ironed out. I agree that the root-tracking in these hierarchical links probably needs to be reworked to reduce some of the complexity, but this seems like the best solution until we are able to do that.

[duckontheweb]

@matthewhanson I'm going to merge this so we can go ahead with a new release tomorrow. If your review turns up a better way to handle this just let me know and we can try to sneak in another update before the release.