-
-
Notifications
You must be signed in to change notification settings - Fork 889
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
feat(openapi): allow optional request body content #6374
feat(openapi): allow optional request body content #6374
Conversation
0eb4e2b
to
5ce5ec3
Compare
5ce5ec3
to
8b57125
Compare
$openapiOperation = $openapiOperation->withRequestBody(new RequestBody( | ||
description: $openapiOperation->getRequestBody()?->getDescription() ?? sprintf('The %s %s resource', 'POST' === $method ? 'new' : 'updated', $resourceShortName), | ||
content: $content, | ||
required: $openapiOperation->getRequestBody()?->getRequired() ?? true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to understand this correctly the main change is that true
is now the default value when there's a request body ?
I think that early on we used false
as a default value to comply with: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#request-body-object I agree that this is better thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to understand this correctly the main change is that true is now the default value when there's a request body ?
In the current code, the constructor set Model\RequestBody
's required
to false
by default. But the OpenFactory
force the required
to be true
if there were no RequestBody
and if this was a write operation.
Here the PR allow the developer to fine-tune the values as he/she wants.
So it's even possible now to set a required
to false
even in a write operation.
@@ -17,16 +17,16 @@ final class RequestBody | |||
{ | |||
use ExtensionTrait; | |||
|
|||
public function __construct(private string $description = '', private ?\ArrayObject $content = null, private bool $required = false) | |||
public function __construct(private ?string $description = null, private ?\ArrayObject $content = null, private bool $required = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
funny enough content
is Required in the spec: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#request-body-object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR's originated where I wanted to edit the description of an endpoint, but by doing so, I needed to rewrite the whole content
.
The PR code make the Model\RequestBody
itself differs from OpenAPI
own specification.
But the whole OpenApiFactory
will bring back the compliance while the specification by overwriting the content
if it's set to null
.
{ | ||
return $this->description; | ||
} | ||
|
||
public function getContent(): \ArrayObject | ||
public function getContent(): ?\ArrayObject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see my previous comment this was correct but the constructor is not
Allow to generate a default request content or description if not defined in the extension of OpenApi using
Model\RequestBody
.For example, on a
POST /greetings/test
operation on aGreeting
entity with aid
and aname
, the following:Will generate the following OpenApi, with the default generated content: