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

fix(hydra): use correctly enable_docs #7062

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

alanpoulain
Copy link
Member

@alanpoulain alanpoulain commented Apr 2, 2025

Q A
Branch? 4.1
Tickets N/A
License MIT
Doc PR N/A

This PR aims to fix definitively a recurring issue: JSON-LD documentation is mandatory but users don't want to expose their endpoints when the API is private: #1568

The enable_docs configuration parameter removes the api_doc route: the documentation is not exposed anymore, but the URL is needed in some parts of the API Platform code: JSON-LD context, Hydra links, etc.
It regularly causes some bugs.

A relating PR was done in 2018:
#1731

This PR was removing the code adding the Hydra headers when the docs is disabled.
IMO @dunglas's comment was right:

IMO we must never disable the Hydra docs (or throw) when JSON-LD is enabled. The hydra "doc" is mandatory for JSON-LD (because we use Hydra types everywhere in JSON-LD documents) and Hydra compliance.

In 2023 another PRs were done but never merged:
#5676
#5862

@dunglas's comments were having the same point:

#5676 (comment)

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

IMHO we have two options:

  • 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)
  • 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.

#5862 (comment)

I wonder if we cannot do that in a cleaner way: for instance, we could set docs_formats in the DI Extension to jsonld if enabled_docs is false, so we can simplify the routing part and always register the route, as it will work only for JSON-LD.

My proposal:

  • Always enable the route.
  • If enable_docs is set to false, only the jsonld documentation format is registered.
  • Since the JSON-LD/Hydra documentation is always exposed, if enable_docs is false, hydra:supportedClass is not filled: the API resources are not included in the documentation but all the other keys stay (@context, @vocab...). To do this, it uses the new hideHydraOperation introduced in feat(metadata): hide an hydra endpoint #6871.
  • Bonus: if the entrypoint is also disabled (enable_entrypoint set to false), the entrypoint is not added to hydra:entrypoint and hydra:supportedClass.

@alanpoulain alanpoulain requested review from dunglas and soyuka April 2, 2025 14:38
@alanpoulain alanpoulain force-pushed the fix-hydra-enable-docs branch from 9a5eac4 to b7033a4 Compare April 2, 2025 14:50
@alanpoulain alanpoulain force-pushed the fix-hydra-enable-docs branch 2 times, most recently from 61e9427 to dab63f4 Compare April 9, 2025 09:50
@alanpoulain alanpoulain force-pushed the fix-hydra-enable-docs branch from dab63f4 to a81f8e4 Compare April 9, 2025 11:08
@alanpoulain alanpoulain force-pushed the fix-hydra-enable-docs branch from a81f8e4 to 5bc4eb0 Compare April 9, 2025 12:28
@soyuka soyuka merged commit 615bdd6 into api-platform:4.1 Apr 10, 2025
78 checks passed
soyuka pushed a commit to soyuka/core that referenced this pull request Apr 10, 2025
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