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

Identifier should be optional when using only a POST operation #4037

Closed
soyuka opened this issue Feb 8, 2021 · 8 comments
Closed

Identifier should be optional when using only a POST operation #4037

soyuka opened this issue Feb 8, 2021 · 8 comments
Milestone

Comments

@soyuka
Copy link
Member

soyuka commented Feb 8, 2021

API Platform version(s) affected: 2.6.2

Description

POST should allow not having any identifiers #3975

@soyuka soyuka added this to the 2.6.3 milestone Feb 8, 2021
@maks-rafalko
Copy link
Contributor

maks-rafalko commented Feb 8, 2021

Here you described why identifier is needed.

Could you please explain why it was decided to allow resources without identifiers again, to understand the whole picture? Probably, it will answer the question where we should use custom controllers and where we should use api-platform resources with no identifiers.

During upgrading from 2.5 to 2.6, we had to replace all DTOs marked with @ApiResource with custom controllers, because we didn't use identifiers there (POST-only custom operations)

@dunglas
Copy link
Member

dunglas commented Feb 8, 2021

@maks-rafalko I discussed this privately with @soyuka and I think that it's a regression. The POST verb explicitly allows this kind of RPC-like operations, and so API Platform should too.
POST is a special verb allowing to send payloads that may lead to side-effects (such as creating new resources). Most POST payloads don't contain identifiers, and so if a class mapped with #[ApiResource] only has a POST operation, it looks sensitive to not make the ID mandatory (also because, in this case, it will be used nowhere).

Also, it's a BC break (I'm very sorry for the inconvenience) that I noticed lately. It breaks some of our internal projects too. We refrain from introducing BC breaks in minor versions (we patched most of them on 2.6.2).

@phpjob
Copy link

phpjob commented Feb 9, 2021

@dunglas I am really interested in the discussion and the arguments of you and @soyuka. Maybe you can publish them?

In the HTTP-RFC
https://tools.ietf.org/html/rfc2616#section-9.5

it says among other things.

The action performed by the POST method might not result in a
resource that can be identified by a URI. In this case, either 200
(OK) or 204 (No Content) is the appropriate response status,
depending on whether or not the response includes an entity that
describes the result.

which sounds plausible to me. Not always a created resource can or must be represented by a URI (or IRI). E.g. if the developer or the use case does not want that.

I'm really not deep in the philosphy of REST (https://www.ics.uci.edu/~fielding/pubs/dissertation/rest_arch_style.htm), but I would say in summary myself that the request always has the full state, so the server can process the request stateless.

In the example of a message that is posted and created in the backend service, but not represented as URL/URI /GET, this may happen. Since the request represents the full state.

I wonder if content is returned, this must contain an ID in any case? Or does REST only say, if you want to link to a resource, how this linking has to be done?!

@soyuka
Copy link
Member Author

soyuka commented Feb 11, 2021

Most POST payloads don't contain identifiers, and so if a class mapped with #[ApiResource] only has a POST operation, it looks sensitive to not make the ID mandatory (also because, in this case, it will be used nowhere).

On one hand we say that a Resource MUST have a corresponding IRI (hence the disabling of operations must still have a GET /cars/1, even if its status is 404). This, to respect JSON-LD specification.
On the other hand, we say that well, to match the HTTP spec, this is not needed.

I'd really suggest a new annotation like #[RPC] (can be named differently) for the 3.0 to remove this confusion. Indeed, a Resource, in REST terms should have a representation right? #3915

I wonder if content is returned, this must contain an ID in any case? Or does REST only say, if you want to link to a resource, how this linking has to be done?!

Indeed, to me if it has a representation, it should have an identifier. The JSON-LD format is really good for that as it enforces the use of @id for a resource. In API Platform we added generated IRIs when they were not available. In 2.7 I want us to be able to configure this identifier even on non-resources (https://github.com/api-platform/core/pull/3946/files).

@soyuka
Copy link
Member Author

soyuka commented Feb 11, 2021

see related PR and https://github.com/api-platform/core/pull/4052/files#diff-a10d35714d202fd78e981792557f650588d7028209a316e33fa7ffa545a5ffc4R1-R36

WDYT ?

soyuka added a commit to soyuka/core that referenced this issue Feb 11, 2021
soyuka added a commit to soyuka/core that referenced this issue Feb 13, 2021
soyuka added a commit that referenced this issue Feb 15, 2021
* Fix #4037 allow POST operations without identifiers

* Better

* remove line

* Update features/main/overridden_operation.feature

Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

* fix review

* Fix mongodb

Co-authored-by: Alan Poulain <contact@alanpoulain.eu>
@soyuka
Copy link
Member Author

soyuka commented Feb 15, 2021

Could anyone try the 2.6 branch see if it resolves the issue?

@antograssiot
Copy link
Contributor

It does for us @soyuka

@soyuka
Copy link
Member Author

soyuka commented Feb 17, 2021

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants