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

Add SKOS relations to JSON-LD serialization #748

Merged
merged 4 commits into from
Apr 13, 2018

Conversation

danmichaelo
Copy link
Contributor

Realized earlier today that skos:relatedMatch is returned as related:Match in the JSON-LD serialization.. example 🌵 Ouch!

As a start, I added a failing test case to document it.

I think the easiest fix is to add the various SKOS mapping properties (exactMatch, closematch, etc.) to the default JSON-LD context). Let me know what you think.

@osma osma added the bug label Mar 27, 2018
@osma osma added this to the 2.0 milestone Mar 27, 2018
@osma
Copy link
Member

osma commented Mar 27, 2018

Ouch indeed! Thanks for spotting!

A failing test is a great way to report an issue :)

The proposed solution sounds good. I think this will only affect skos:relatedMatch since it's the only property whose URI starts with another SKOS property. But defining all the SKOS mapping properties is better for clarity and consistency.

danmichaelo added a commit to danmichaelo/skosmos that referenced this pull request Apr 3, 2018
@danmichaelo danmichaelo changed the title Add test case for failing skos:relatedMatch JSON-LD serialization Add SKOS relations to JSON-LD serialization Apr 3, 2018
@danmichaelo
Copy link
Contributor Author

Ok, added the mapping relations to the context for the data endpoint.

I also wanted the output from the search endpoint to be more consistent with the output from the data endpoint, so I added any additional fields being requested using the fieldsparameter to the JSON-LD context there. Note that this change is technically breaking (if users just rely on the 'skos:' prefix being present in the keys in the resulting JSON without taking the JSON-LD context into account), but I guess the impact should be low since it only affects additional fields requested using the fields parameter.

If additional fields are requested using the 'fields' parameter,
add them to the JSON-LD context.

Note: This will break any implementation that relies on the 'skos:'
prefix being part of the keys.
@osma
Copy link
Member

osma commented Apr 13, 2018

Excellent work! I see you even added a RestControllerTest class, which we didn't have before.

The only problem I see is that the Travis tests failed due to an unrelated PHP version compatibility issue. That should be fixed in master now. I will try merging this first into a separate branch to make sure it doesn't actually break the tests.

@osma osma merged commit 1a5cc6c into NatLibFi:master Apr 13, 2018
@osma
Copy link
Member

osma commented Apr 13, 2018

Tests OK after merge. Thanks a lot!

@danmichaelo danmichaelo deleted the test-relatedmatch branch April 13, 2018 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants