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 a new attribute for "placeholder" routes #3051

Closed
dunglas opened this issue Sep 5, 2019 · 8 comments
Closed

Add a new attribute for "placeholder" routes #3051

dunglas opened this issue Sep 5, 2019 · 8 comments

Comments

@dunglas
Copy link
Member

dunglas commented Sep 5, 2019

Because in API Platform (and when following the web architecture in general), it's better to identify every resources using an IRI, it's what does API Platform by default. While not mandatory, it's also considered a best practice for an IRI to be "dereferencable". When requesting the IRI, a representation of the resource, or the associated documentation should be displayed.

However, sometimes, you need to use reference to resources that aren't publicly available. Because API Platform internally uses the Symfony router to map objects to URL (forth and back), a route must exist for any resource type exposed through the API (or, in PHP term, for any class marked with @ApiResource). A common problem with API Platform is that because of this the GET operation must always exist, but sometimes you don't want this route (used to generate IRIs) to return anything. In this case, also want to this route to be hidden from the OpenAPI documentation, because it is useless for humans and code generator. This kind of routes are just placeholders used to generate identifiers.

My proposal is to introduce a new operation attribute, called iriOnly. When set to true, the route will exist but will return a 404 not found status code, and no PHP code, nor SQL request will be triggered. Also, such operations will be automatically hidden from OpenAPI and Swagger documentations. Optionally, a HTTP status code could be associated with this attribute, such as iriOnly=204.

@teohhanhui
Copy link
Contributor

teohhanhui commented Sep 5, 2019

I think it's better to provide a URN or a Skolem IRI (see below) in such cases. We avoid the whole mess altogether.

@teohhanhui
Copy link
Contributor

Note

As noted in § 1.1 How to Read this Document, IRIs can often be confused with URLs (Uniform Resource Locators), the primary distinction is that a URL locates a resource on the web, an IRI identifies a resource. While it is a good practice for resource identifiers to be dereferenceable, sometimes this is not practical. In particular, note the [URN] scheme for Uniform Resource Names, such as UUID. An example UUID is urn:uuid:f81d4fae-7dec-11d0-a765-00a0c91e6bf6.

Authors are strongly encouraged to avoid labeling properties using blank node identifiers, instead, consider one of the following mechanisms:

  • a relative IRI, either relative to the document or the vocabulary (see § 4.1.2.1 Using the Document Base for the Default Vocabulary for a discussion on using the document base as part of the vocabulary mapping).
  • a URN such as urn:example:1, see [URN], or
  • a "Skolem IRI" as per Replacing Blank Nodes with IRIs of [RDF11-CONCEPTS].

https://www.w3.org/TR/json-ld11/

@dunglas
Copy link
Member Author

dunglas commented Sep 5, 2019

"Cool IRIs don't change". If at some point you want to expose this resource through the web, (and it often happen in the world of web APIs), using an URI from the very beginning (even if it isn't dereferenceable yet) is better.
Also, regarding the implementation details, relying on the router for everything is easier. Patching the router to support Skolem IRIs or URN will not be easy. Let's not introduce unneeded complexity.

@teohhanhui
Copy link
Contributor

Completely disagree here. I'm not sure that returning HTTP 404 even qualifies as a "dereferenceable" IRI.

"Cool IRIs don't change".

That's only applicable when we're talking about web IRIs. It's irrelevant when we're using a URN or a Skolem IRI.

Patching the router to support Skolem IRIs or URN will not be easy. Let's not introduce unneeded complexity.

It should not even hit the router at all. 😉

@dunglas
Copy link
Member Author

dunglas commented Sep 5, 2019

That's only applicable when we're talking about web IRIs.

My point is that we're actually talking about that!

@teohhanhui
Copy link
Contributor

teohhanhui commented Sep 6, 2019

After our offline discussion yesterday, we've agreed that returning HTTP 404 is the right thing to do.

However, I think ideally we could avoid having a new attribute, by always auto-generating a private get item operation ("read"=false, "output"=false, "status"=404) when it's not declared. And of course only when not using the default set of item operations (i.e. not declaring itemOperations at all), as in that case it'd be a public get item operation.

It means we always do the right thing. WDYT?

@teohhanhui
Copy link
Contributor

teohhanhui commented Sep 6, 2019

/cc @Deuchnord on this problem:

if I use the output=false trick, the model is not exposed anymore in the API (I mean: it's does not appear in the API doc).

Is the output model missing even when there's another operation that uses it (including in the case of embedded relations)? If so, it sounds like a bug in the Swagger DocumentationNormalizer. (And do you still see the same issue in master?) 😄

@soyuka
Copy link
Member

soyuka commented Sep 13, 2019

Also somehow related #2954

soyuka added a commit to soyuka/core that referenced this issue Sep 18, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 18, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 18, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 20, 2019
soyuka added a commit to soyuka/core that referenced this issue Sep 21, 2019
@soyuka soyuka closed this as completed Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants