Use only search link supporting the json mimeType

[mneagul]

Related Issue(s):
N/A

Description:
pystac allways selects the first search link without taking into consideration the mimeType. For STAC catalogs offering multiple search links with different mimeTypes this might result in erroneous selection of the search link (eg. in case the first search link points to an endpoint returning text/html or a different represenation)

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.

[duckontheweb]

@mneagul Thanks for submitting this.

I think that changing get_single_link to only return JSON links is too restrictive since the type field is not required for Links and this method could conceivably be used by users or in downstream code to get a non-JSON link. I believe the test failure we're seeing here is due to the first case (a missing type parameter in one of the links), but I have not verified that.

However, I think it would be useful to add an optional media_type argument to get_single_link that only returns links with the given type value if media_type is not None and maintains the current behavior of only filtering on rel if media_type is None. If the media_type argument defaulted to None, then this should be backward compatible but still allow users to filter by type if needed.

[duckontheweb]

Regarding how this applies specifically to search links...

The search link is defined in the STAC API spec and this library has mostly limited its scope to concepts and constructs from the core STAC Spec and the STAC Extensions hosted in STAC Extensions organization. If we add the media_type argument to get_single_link then it might also be worth a PR on pystac-client to ensure it is using that argument to filter the links where appropriate.

It might also be worth adding a media_type argument to the get_links method so that users can filter multiple links in the same way.

[mneagul]

Thank you for your feedback and sorry for the late response. I will try to reflect this in the pull request.

The pull request was mainly motivated by the the need to use the STAC implementation in geoserver

[duckontheweb]

@mneagul Could you run the test & lint checks by running ./scripts/test at the top level of the repo? I'm getting a few formatting and type hint errors when I run that.

It also looks like there is a conflict in CHANGELOG.md so you will probably need to rebase your branch onto stac-utils/main.

Thanks!

[duckontheweb]

@lossyrob @gadomski Any idea why the CI wouldn't be running for this (it did when the PR was originally opened)? I don't see an option to approve the workflow like I normally do for new contributors even after removing the required status checks in the branch protection settings.

[lossyrob]

@duckontheweb I see "This branch has conflicts that must be resolved" so looks like it needs to merge upstream?

[codecov-commenter]

Codecov Report

Merging #704 (a08f4b1) into main (2d175d1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #704   +/-   ##
=======================================
  Coverage   94.54%   94.55%           
=======================================
  Files          79       79           
  Lines       11606    11621   +15     
  Branches     1370     1370           
=======================================
+ Hits        10973    10988   +15     
  Misses        455      455           
  Partials      178      178           

Impacted Files	Coverage Δ
pystac/stac_object.py	90.81% <100.00%> (ø)
tests/test_catalog.py	99.01% <100.00%> (+0.01%)	⬆️

---

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 2d175d1...a08f4b1. Read the comment docs.

[mneagul]

I created a new PR in pystack-client: stac-utils/pystac-client#142 using the functionality added in this PR.

[duckontheweb]

@mneagul Thanks for contributing this, looks good! Lets add a couple of tests to the CatalogTest class in tests/test_catalog.py that use the new media_type argument in get_links and get_single_link and then I think this is ready to merge.

[mneagul]

I added tests for both functions, including a sample catalog.

[duckontheweb]

Sorry, @mneagul, I feel like I'm putting you through the ringer with this PR...

I left some suggested changes and comments regarding creating a new test case, but the changes look good otherwise.

[mneagul]

I was unaware of the interaction between mypy and the assert behaviour :D I apologies for this :D

[duckontheweb]

@mneagul Once tests/data-files/catalogs/test-case-9/catalog.json has been removed this should be ready to approve and merge.

[duckontheweb]

Thanks again. I'll include this in the 1.4.0 release.