Skip to content

fix(jsonld): check if docs enabled when generate vocab in context #5676

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

Closed

Conversation

Romaixn
Copy link
Contributor

@Romaixn Romaixn commented Jul 18, 2023

Q A
Branch 3.1
Tickets #5528
License MIT

Check if enable_docs option is enabled before adding @vocab in JSON-LD context to avoid error route not found.

Transform this :

{
	"@context": {
		"hydra": "http:\/\/www.w3.org\/ns\/hydra\/core#",
		"@vocab": "https:\/\/localhost\/docs.jsonld#",
		"isbn": "Book\/isbn",
		"title": "Book\/title",
		"description": "Book\/description",
		"author": "Book\/author",
		"publicationDate": "Book\/publicationDate",
		"reviews": "Book\/reviews",
		"cover": "Book\/cover",
		"archivedAt": "Book\/archivedAt"
	}
}

To this :

{
	"@context": {
		"hydra": "http:\/\/www.w3.org\/ns\/hydra\/core#",
		"isbn": "Book\/isbn",
		"title": "Book\/title",
		"description": "Book\/description",
		"author": "Book\/author",
		"publicationDate": "Book\/publicationDate",
		"reviews": "Book\/reviews",
		"cover": "Book\/cover",
		"archivedAt": "Book\/archivedAt"
	}
}

@dunglas
Copy link
Member

dunglas commented Jul 20, 2023

This doesn't look good to me. RDF types are now wrong. Even if it's considered a best practice to use dereferenceable URI as types, it's not mandatory. RDF parsers will not even try to hit the URL. It's just a namespace.

I would just keep things as-is.

@Romaixn
Copy link
Contributor Author

Romaixn commented Jul 20, 2023

Alternatively, we could attempt to replace the @vocab with something else. The issue is that if the enable_docs parameter is set to false, the URL /contexts returns an error since the route to the documentation doesn't exist.

Or we could assume that if we disable the documentation, we also disable the /contexts route ?

@dunglas
Copy link
Member

dunglas commented Jul 20, 2023

No, contexts must always exist according to the JSON-LD spec.

The error is likely because the docs route doesn't exist in the Symfony router.

IMHO we have two options:

  1. Always register the docs route but "manually" throw a NotFoundHttpException if enable_docs is set to false (consequently the route will technically exists in the Symfony router and the error will be prevented)
  2. Always generate the Hydra "docs" if JSON-LD is enabled, even if enable_docs is set to false (but continue to disable the OpenAPI endpoint, Swagger UI etc)

IMHO option 2 is more correct and easier to implement.

WDYT?

@Romaixn
Copy link
Contributor Author

Romaixn commented Jul 20, 2023

I think you're right; the second option is the best. I will modify my PR accordingly, by enabling only the route to /docs.jsonld when enable_docs is set to false.

Always add jsonld version of doc to prevent route not found when getting context of resource
@stale
Copy link

stale bot commented Sep 25, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 25, 2023
@stale stale bot closed this Oct 2, 2023
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