Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 248 patch - Update JMESPaths #256

Merged
merged 2 commits into from
Dec 10, 2022

Conversation

digitaldogsbody
Copy link
Contributor

This PR updates the JMESPath constants for both IIIF Presentation v2 and v3 to fix some potential issues.

Presentation v2:

Problem: Non-Image services could be picked up by the selector.
Fix: Add a filter clause to make sure the @context block of the service is for an Image API version. The reason for using this field is per Presentation 2.1.1 section 5.4: "A reference to the Image API context document MUST be included", so on a valid Manifest it will be present.
Playgrounds:

  1. Current code, picking up Search service in third canvas: http://play.jmespath.org/?u=926ccfd3-0b1f-43b2-b431-b9c9b3bdbf93
  2. Fixed code, picking up Image services for Canvas 1+2, and not the Search service in Canvas 3: http://play.jmespath.org/?u=cccfd941-f3c7-418f-844f-44b85ddee0f4

Presentation v3:

Problem 1: Current selector only works if all services are v3 (i.e with id and type). Having even a single v2 service (with @type) causes the whole selector to fail on the contains conditional and return a single null for the entire manifest (not just the one block).
Problem 2: Assuming above fixed, current selector only returns id for the service, meaning it misses v2 services with @id.
Fix 1: Push both type and @type into a hash, and use the not_null function to select the one that actually exists, and then assign it to type, then use this for the conditional selection of only Image services.
Fix 2: Repeat the same push-to-hash/not_null trick with id and @id, ensuring that id will then have the value from whichever one exists.
Playgrounds: (n.b. each of these has a third canvas which should never get a service_id as it only contains a Search service)

  1. Current code, working with only v3 services: https://play.jmespath.org/?u=1a017e4f-064e-48fb-b469-1cbfac6f8b71
  2. Current code, failing to null when one v2 service exists: https://play.jmespath.org/?u=07998489-58d0-47cb-b4ba-5b7dd5278431
  3. Fixed code, working with both v2 and v3 services and returning consistent output regardless of whether @ is present in input keys or not: https://play.jmespath.org/?u=b0d17ada-8b30-4f8f-a2cd-9f8296b90b78

@DiegoPino
Copy link
Member

Amazing @digitaldogsbody thanks so much. Merging!

@DiegoPino DiegoPino merged commit ae4a5a4 into esmero:ISSUE-248 Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants