Fix for proper handling of relative links for assets of network-loaded STAC items

[spinto]

Related Issue(s):

• Proper handling of root relative hrefs #1597

Description:
when a STAC Item is loaded from a remote URL, pystac was not resolving correctly relative URL references (e.g. /d/myfile) to absolute paths.

This was due to the utils.is_absolute_href function checking always via os.path.isabs, even if the STAC Item is from a non-filesystem URL.

The patch proposed here applies the same principle of "utils.make_absolute_href", so to use the STAC item self value to determine if this is filesystem or network url and act accordingly (for network urls, the presence of the schema in front is considered absolute URL, instead a relative URL).

PR Checklist:

• Pre-commit hooks pass (run pre-commit run --all-files)
• Tests pass (run pytest)
• 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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.30%. Comparing base (608972a) to head (5e08cc8).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1599      +/-   ##
==========================================
+ Coverage   92.29%   92.30%   +0.01%     
==========================================
  Files          55       55              
  Lines        8395     8397       +2     
  Branches      971      971              
==========================================
+ Hits         7748     7751       +3     
  Misses        458      458              
+ Partials      189      188       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gadomski]

This PR looks like it only fixes href resolution for assets. Any reason to not also consider href resolution for traversing links, e.g. resolving a "child" or "item" link?

Also, can you add a CHANGELOG entry? Thanks.

[spinto]

This PR looks like it only fixes href resolution for assets. Any reason to not also consider href resolution for traversing links, e.g. resolving a "child" or "item" link?

I was unsure it would be easy to make it work for the 'links', because it uses the get_self_href function to determine if we are loading from the network or from the filesystem, and you may get to parse the 'self' links later then the other links... also STAC API Catalogues I know will generate always absolute values for the links (although they may be relative according to the STAC Item specifications)... if that is ok, I would propose to start with just the asset and see later for the links...

[spinto]

I have added the CHANGELOG

[gadomski]

I agree that implementing this for links will be much tricker, so I'm ok with just handling assets for now.

Can you add a test for get_absolute_href, since that's the API change that you're mention in the CHANGELOG? Thanks!

[spinto]

Can you add a test for get_absolute_href, since that's the API change that you're mention in the CHANGELOG? Thanks!

Well, the issue in get_absolute_href was caused by a problem in is_absolute_href... anyway I totally agree that more testing is better, so I have added also explicit tests for get_absolute_href too in the test_asset (for both windows and linux)

[spinto]

o the previous get_absolute_href test had some problems in windows. It should work now.

[gadomski]

Cool, thanks for the additional test, I think it helps illustrate the desired functionality well.

Just one question about logic to make sure I'm understanding things correctly.

[gadomski]

I struggled to understand this at first. Can it be more clearly expressed as this?

Suggested change

	parsed_start_scheme = "" if start_href is None else safe_urlparse(start_href).scheme
	if parsed_start_scheme in ["", "file"]:
	return parsed.scheme not in ["", "file"] or os.path.isabs(parsed.path)
	else:
	return parsed.scheme not in ["", "file"]
	if parsed.scheme not in ["", "file"]:
	return True
	else:
	parsed_start_scheme = "" if start_href is None else safe_urlparse(start_href).scheme
	return parsed_start_scheme in ["", "file"] and os.path.isabs(parsed.path)