Relative self hrefs break link resolution

[gadomski]

Related Issue(s): #571

Description: Items created from relative paths can have relative self links, which breaks assumptions about self hrefs. Here's one place where the self href is assumed to be absolute:

pystac/pystac/link.py

Lines 234 to 242 in 76ef6a3

	start_href = self.owner.get_self_href()
	if start_href is None:
	raise pystac.STACError(
	"Relative path {} encountered "
	'without owner "self" link set.'.format(target_href)
	)
	target_href = make_absolute_href(target_href, start_href)

I did confirm that the issue was present in https://github.com/stac-utils/pystac/releases/tag/v1.0.0-rc.3, which means that #555 is likely not the cause (we thought it might be since it did muck around with how links worked).

Opening as a draft b/c it's just a failing test right now, will convert to a real pull request when there's a resolution.

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.

[gadomski]

I've come up with a "strong as possible" solution by gating Link creation when rel == self. However, this breaks a test:

pystac/tests/test_link.py

Lines 137 to 138 in 76ef6a3

	# Special case.
	{"rel": "self", "href": "t"},

The test breaks because the self href is turned into an absolute one by this PR. This test case is labelled "special case" by @schwehr (https://github.com/stac-utils/pystac/blame/76ef6a3b9bc3412cfcbc7f502d8fc1f56310b779/tests/test_link.py#L137-L138). What was the special case that necessitated this test?

[schwehr]

I've come up with a "strong as possible" solution by gating Link creation when rel == self. However, this breaks a test:

pystac/tests/test_link.py

Lines 137 to 138 in 76ef6a3

	# Special case.
	{"rel": "self", "href": "t"},

The test breaks because the self href is turned into an absolute one by this PR. This test case is labelled "special case" by @schwehr (https://github.com/stac-utils/pystac/blame/76ef6a3b9bc3412cfcbc7f502d8fc1f56310b779/tests/test_link.py#L137-L138). What was the special case that necessitated this test?

I wish I knew what was weird about the handling of self links. Clearly my comment wasn't good enough ☹️. Maybe @lossyrob can remember?

[duckontheweb]

I've come up with a "strong as possible" solution by gating Link creation when rel == self. However, this breaks a test:

pystac/tests/test_link.py

Lines 137 to 138 in 76ef6a3

	# Special case.
	{"rel": "self", "href": "t"},

The test breaks because the self href is turned into an absolute one by this PR. This test case is labelled "special case" by @schwehr (https://github.com/stac-utils/pystac/blame/76ef6a3b9bc3412cfcbc7f502d8fc1f56310b779/tests/test_link.py#L137-L138). What was the special case that necessitated this test?

I wish I knew what was weird about the handling of self links. Clearly my comment wasn't good enough ☹️. Maybe @lossyrob can remember?

Looking back at the commit that added that comment it looks like the only thing that was special about that case is the fact that "self" links were always turned into absolute links (there is a test case later on in that commit that tests for this, but it looks like it has been removed since then). I'm in favor of updating this test to reflect the changes in this PR.

[codecov-commenter]

Codecov Report

Merging #574 (c593d3f) into main (76ef6a3) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #574      +/-   ##
==========================================
+ Coverage   94.67%   94.69%   +0.02%     
==========================================
  Files          75       75              
  Lines       10732    10776      +44     
  Branches     1048     1053       +5     
==========================================
+ Hits        10160    10204      +44     
  Misses        396      396              
  Partials      176      176              

Impacted Files	Coverage Δ
pystac/link.py	90.30% <100.00%> (+0.24%)	⬆️
tests/test_link.py	100.00% <100.00%> (ø)
tests/test_catalog.py	99.17% <0.00%> (+0.01%)	⬆️
pystac/catalog.py	91.93% <0.00%> (+0.29%)	⬆️

---

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 76ef6a3...c593d3f. Read the comment docs.

[duckontheweb]

Just a minor type annotation change (that I think was actually introduced in an earlier PR...), but otherwise looks good to me. Thanks for the fix!