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

Redirect root context ref to systemContextUri #1482

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

niklasl
Copy link
Member

@niklasl niklasl commented Sep 23, 2024

[Untested Draft]

Currently, we're not serving working JSON-LD, since the /context.jsonld reference used both in the embedded @context
reference:

curl -s -L -HAccept:application/ld+json https://libris.kb.se/fxql7jqr38b1dkf

and the Linke header variant served for plain JSON:

curl -v -HAccept:application/json https://libris.kb.se/fxql7jqr38b1dkf
< link: </context.jsonld>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"

(This was pointed out during the BIBFRAME conference.)

This Untested Draft PR simply redirects to the chosen system context, from the previously working "convenience" URI (the root /context.jsonld resource; i.e. resolving to either https://libris.kb.se/context.jsonld and https://id.kb.se/context.jsonld).

If you so decide, it could set the whelk.systemContextUri directly (it's not as "nice", but more explicit, and may be easier depending on web server configuration complexity).

@andersju
Copy link
Member

Ouch! This looks fine to me but wouldn't itself solve the issue because https://libris.kb.se/context.jsonld is passed to websok anyway. id.kb.se's nginx has this:

  location = /context.jsonld {
    proxy_pass http://xl-api-cluster/https://id.kb.se/vocab/context/data.jsonld;
  }

Should simply add the same to libris.kb.se's nginx, no? Then we can fix the issue immediately. We used to have the equivalent in the Apache config but it got lost in the migration. I'll add some tests to https://github.com/libris/lxl_api_tests.

@niklasl
Copy link
Member Author

niklasl commented Sep 24, 2024

Ah; yes; adding that would fix this right now, which would be great! Then we can proceed to solve this properly in this PR (to avoid too much domain logic in nginx)? And 💯 👍 for tests!

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