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(openapi): do not throw error with non standard HTTP verb #4304

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

qboot
Copy link
Contributor

@qboot qboot commented Jun 3, 2021

Q A
Branch current stable (2.6)
License MIT

Hello!

I'm trying to use a non standard HTTP verb in my ApiResource operations:

collectionOperations={
 *          "test":{
 *              "method": "TEST",
 *              "path": "/something",
 *              "controller": TesterController::class,
 *          },
 *      },

Everything works great, but when I want to access the Swagger UI (/docs), I got an error:

Attempted to call an undefined method named "withTEST" of class "ApiPlatform\Core\OpenApi\Model\PathItem".

It seems that we don't check the HTTP verb before creating the related PathItem method here:

$pathItem = $pathItem->{'with' . ucfirst($method)}(new Model\Operation(

https://github.com/api-platform/core/blob/2.6/src/OpenApi/Factory/OpenApiFactory.php#L271

OpenApi only supports: ['GET', 'PUT', 'POST', 'DELETE', 'OPTIONS', 'HEAD', 'PATCH', 'TRACE'].
So I propose this fix: continue if the HTTP verb is unknown, which fix the above error.

Thanks

@alanpoulain
Copy link
Member

I don't think it's a good idea to use a non-standard HTTP verb.
What is your use case?

@qboot
Copy link
Contributor Author

qboot commented Jun 3, 2021

There is a lot more HTTP verb than the few from RFC7231 accepted by OpenApi. It's actually in debate to accept all of them in this comment OAI/OpenAPI-Specification#1747.

IMO it's weird to be able to use a non-standard HTTP verb, without problem, but then be constrained by the documentation's generation.

A same PR has been made in NelmioApiDocBundle: nelmio/NelmioApiDocBundle#1294

@lyrixx
Copy link
Contributor

lyrixx commented Jun 3, 2021

I don't think it's a good idea to use a non-standard HTTP verb.

But it's all about REST. you can use the verb you want... there is no limit.

And in my use case is the following: an endpoint to test something: you configure some credential (like slack) token, you click on "test" on the interface, and it will emit a slack notification. It could not be a GET one, since there is a body and browser does not support body + GET. And if I use POST, APIP will try to save the new configuration. So TEST verb is a good one here.

@soyuka
Copy link
Member

soyuka commented Jun 4, 2021

👍 on this, see comment though

@qboot qboot requested a review from soyuka June 4, 2021 14:10
@soyuka soyuka merged commit e32b4e8 into api-platform:2.6 Jun 4, 2021
@soyuka
Copy link
Member

soyuka commented Jun 4, 2021

Thanks @qboot !

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.

4 participants