-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Request bodies moved out of parameters #670
Conversation
While I think media types need separate considerations (see #146), I think the plural |
@jasonh-n-austin Cool PR 👍 definitely make spec less confusing. You can make POST requests without body parameter, so you should have the ability to write If you use the same schema to describe multiple body parameters, how you will deprecate only one of them? So body need it's own |
@@ -346,7 +346,8 @@ Field Name | Type | Description | |||
<a name="pathItemHost"></a>host | `string` | The host (name or ip) serving the path. This optional value will override the top-level [host](#oasHost) if present. This MUST be the host only and does not include the scheme nor sub-paths. It MAY include a port. If the `host` is not included, the host serving the documentation is to be used (including the port). The `host` does not support [path templating](#pathTemplating). | |||
<a name="pathItemBasePath"></a>basePath | `string` | The base path on which the API is served, which is relative to the [`host`](#pathItemHost). This optional value will override the top-level [basePath](#oasBasePath) if present. If it is not included, the API is served directly under the `host`. The value MUST start with a leading slash (`/`). The `basePath` does not support [path templating](#pathTemplating). | |||
<a name="pathItemSchemes"></a>schemes | [`string`] | The transfer protocol of the API. Values MUST be from the list: `"http"`, `"https"`, `"ws"`, `"wss"`. This optional value will override the top-level [schemes](#oasSchemes) if present. If the `schemes` is not included, the default scheme to be used is the one used to access the OpenAPI definition itself. | |||
<a name="pathItemParameters"></a>parameters | [[Parameter Object](#parameterObject) <span>|</span> [Reference Object](#referenceObject)] | A list of parameters that are applicable for all the operations described under this path. These parameters can be overridden at the operation level, but cannot be removed there. The list MUST NOT include duplicated parameters. A unique parameter is defined by a combination of a [name](#parameterName) and [location](#parameterIn). The list can use the [Reference Object](#referenceObject) to link to parameters that are defined at the [OpenAPI Object's parameters](#oasParameters). There can be one "body" parameter at most. | |||
<a name="pathItemParameters"></a>parameters | [[Parameter Object](#parameterObject) <span>|</span> [Reference Object](#referenceObject)] | A list of parameters that are applicable for all the operations described under this path. These parameters can be overridden at the operation level, but cannot be removed there. The list MUST NOT include duplicated parameters. A unique parameter is defined by a combination of a [name](#parameterName) and [location](#parameterIn). The list can use the [Reference Object](#referenceObject) to link to parameters that are defined at the [OpenAPI Object's parameters](#oasParameters). | |||
<a name="pathItemRequestBody"></a>requestBody | [[Request Body Object](#requestBodyObject) <span>|</span> [Reference Object](#referenceObject)] | The request body applicable for all the operations described under this path. This request body can be overriden at the operation level, but cannot be removed there. |
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.
Actually I would omit it at the path item level, and have it only at the operation level.
I can't think of a single request body definition which would be plausibly the same for multiple, or even all, operations on the same path.
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.
I don't think it's super common, but I could see a case where the same type is used on multiple operations in the same path, with some exceptions. I'm not super tied to it, but since v2.0 has the concept for parameters
, it seemed fair to keep it in splitting out requestBody
.
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.
Adding to the people above and @whitlockjc's comment on the main thread - this just doesn't seem like a real case we need to handle. The idea behind parameters being at the POI was that people won't need to repeat common parameters - this makes sense for path parameters, query parameters and obviously header parameters. This doesn't make sense as much for payloads (form and body). You could argue there's a case where a single body can be used for both POST and PUT, and that a specific path doesn't have a GET/DELETE/... calls as well. So yes, but it feels like the edge of the edge.
Does adding support to it cost much? Probably not. Will it be used by enough APIs to justify it? Probably even less. So in favor of simplifying the tooling and considering there's a very easy alternative, I'd suggest we drop this from the POI.
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.
Agreed, I'll drop it.
Deprecating a request body is only sensible when the operation previously had a body parameter, but in the future should be called without one. (Otherwise you would deprecate the whole operation.) I guess this happens, but not that often, so I'm ambivalent if it is necessary. Which might be happening more often would be passing a different body parameter (maybe with a different Content-Type), and then the old one will be deprecated. Enabling this at all belongs to #146, though. I guess a solution to that will also cover the case of "no body parameter". |
|
||
Describes a single request body. | ||
|
||
If the `requestBody` attribute is not present, or `schema` is not defined, the request body is not required. |
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.
@IvanGoncharov this concept is meant to preclude the need for required
. If no request body is needed, then one should not be specified. You could also specify a schema with nothing specified and get close the same net effect.
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.
I don't get it. If a requestBody
is defined, how can it not have schema
? Seems like schema
needs to be required in this case.
Not sure we can avoid required
here. To me, not stating a requestBody
doesn't make it optional, it makes it 'ignored', 'unaccepted'.
We can argue whether there are cases that a payload can be optional, question is do we really want to not allow specifying it. We can make it implicitly required allowing people to explicitly set it as not-required if needed, covering most cases and reducing bloat.
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.
Adding required
back in.
I'm specifying that it defaults to true
, to avoid having to specify required: true
for every request body (when that is probably the 90+% case).
@IvanGoncharov I think @ePaul got it right here. In the case of an operation with a request body, the entire operation would be deprecated, but a request body would be very unlikely to be deprecated. I intentionally didn't tackle multiple request bodies in this PR, to try and take a bite sized chunk of the complexity. Perhaps when we consider content-type-specific requests the On multiple request bodies (to be addressed in a future PR), to emphasize why it's not in here: |
@@ -445,7 +446,8 @@ Field Name | Type | Description | |||
<a name="operationId"></a>operationId | `string` | Unique string used to identify the operation. The id MUST be unique among all operations described in the API. Tools and libraries MAY use the operationId to uniquely identify an operation, therefore, it is recommended to follow common programming naming conventions. | |||
<a name="operationConsumes"></a>consumes | [`string`] | A list of MIME types the operation can consume. This overrides the [`consumes`](#oasConsumes) definition at the OpenAPI Object. An empty value MAY be used to clear the global definition. Value MUST be as described under [Mime Types](#mimeTypes). | |||
<a name="operationProduces"></a>produces | [`string`] | A list of MIME types the operation can produce. This overrides the [`produces`](#oasProduces) definition at the OpenAPI Object. An empty value MAY be used to clear the global definition. Value MUST be as described under [Mime Types](#mimeTypes). | |||
<a name="operationParameters"></a>parameters | [[Parameter Object](#parameterObject) <span>|</span> [Reference Object](#referenceObject)] | A list of parameters that are applicable for this operation. If a parameter is already defined at the [Path Item](#pathItemParameters), the new definition will override it, but can never remove it. The list MUST NOT include duplicated parameters. A unique parameter is defined by a combination of a [name](#parameterName) and [location](#parameterIn). The list can use the [Reference Object](#referenceObject) to link to parameters that are defined at the [OpenAPI Object's parameters](#oasParameters). There can be one "body" parameter at most. | |||
<a name="operationParameters"></a>parameters | [[Parameter Object](#parameterObject) <span>|</span> [Reference Object](#referenceObject)] | A list of parameters that are applicable for this operation. If a parameter is already defined at the [Path Item](#pathItemParameters), the new definition will override it, but can never remove it. The list MUST NOT include duplicated parameters. A unique parameter is defined by a combination of a [name](#parameterName) and [location](#parameterIn). The list can use the [Reference Object](#referenceObject) to link to parameters that are defined at the [OpenAPI Object's parameters](#oasParameters). | |||
<a name="operationRequestBody"></a>body | [[Request Body Object](#requestBodyObject) <span>|</span> [Reference Object](#referenceObject)] | The request body applicable for this operation. If a request body is already defined at the [Path Item](#pathItemParameters), the new definition will override it, but can never remove it. |
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.
This should be requestBody
instead of body
.
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.
body
should be requestBody
here
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.
If we move formData
parameters to the requestBody
as well (and maybe even if not), perhaps a more suitable name would be payload
(not a big fan of mixed-case names, body
may be too obscure to some).
My personal opinion is that having a path-level Would this require a global |
* Path - Used together with [Path Templating](#pathTemplating), where the parameter value is actually part of the operation's URL. This does not include the host or base path of the API. For example, in `/items/{itemId}`, the path parameter is `itemId`. | ||
* Query - Parameters that are appended to the URL. For example, in `/items?id=###`, the query parameter is `id`. | ||
* Header - Custom headers that are expected as part of the request. | ||
* Body - The payload that's appended to the HTTP request. Since there can only be one payload, there can only be *one* body parameter. The name of the body parameter has no effect on the parameter itself and is used for documentation purposes only. Since Form parameters are also in the payload, body and form parameters cannot exist together for the same operation. | ||
* Form - Used to describe the payload of an HTTP request when either `application/x-www-form-urlencoded`, `multipart/form-data` or both are used as the content type of the request (in the OpenAPI Specification's definition, the [`consumes`](#operationConsumes) property of an operation). This is the only parameter type that can be used to send files, thus supporting the `file` type. Since form parameters are sent in the payload, they cannot be declared together with a body parameter for the same operation. Form parameters have a different format based on the content-type used (for further details, consult http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4): | ||
* `application/x-www-form-urlencoded` - Similar to the format of Query parameters but as a payload. For example, `foo=1&bar=swagger` - both `foo` and `bar` are form parameters. This is normally used for simple parameters that are being transferred. |
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.
@webron to look at how to deal with form urlencoded parameters.
This is a good proposal. I'll add my comments to specific sections tackling the proposal as-is, then go over the formData parameters. |
* Path - Used together with [Path Templating](#pathTemplating), where the parameter value is actually part of the operation's URL. This does not include the host or base path of the API. For example, in `/items/{itemId}`, the path parameter is `itemId`. | ||
* Query - Parameters that are appended to the URL. For example, in `/items?id=###`, the query parameter is `id`. | ||
* Header - Custom headers that are expected as part of the request. | ||
* Body - The payload that's appended to the HTTP request. Since there can only be one payload, there can only be *one* body parameter. The name of the body parameter has no effect on the parameter itself and is used for documentation purposes only. Since Form parameters are also in the payload, body and form parameters cannot exist together for the same operation. |
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.
Removing this description removes the restriction of using body parameters alongside formData parameters (which just can't happen). We either need to add it (probably in both places) or discuss moving formData parameters to the requestBody
as well.
Re Re Re @whitlockjc raises an important issue regarding reusable components - since requestBody is no longer a parameter, do we need to allow it under the new components construct? Not sure we can avoid it. |
Now to look at |
I'm trying to collect my thoughts around Hate bringing it up but regarding the name, are we set on |
Ugh, forgot to say, I think this can be merged, and I'll tackle the |
2 cents, please help rid the whole of |
This also resolved #278. |
Field Name | Type | Description | ||
---|:---:|--- | ||
<a name="requestBodyDescription"></a>description | `string` | A brief description of the request body. This could contain examples of use. [GFM syntax](https://help.github.com/articles/github-flavored-markdown) can be used for rich text representation. | ||
<a name="requestBodySchema"></a>schema | [Schema Object](#schemaObject) | The schema defining the type used for the request body. |
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.
Just noticed this - shouldn't the schema field be required? What does a request body mean without a schema?
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.
Some of our APIs use AtomPub-style "media resources", see https://tools.ietf.org/html/rfc5023#section-9.6. In this case the request body of a POST can have any media type, e.g. a picture or PDF document, so there's no schema.
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.
Many media types full describe the semantics of a body.
Problem description
Request bodies are currently specified in
parameters
asin: body
. This has been addressed as an incongruence with other parameters throughout a variety of issues in meta #565, as well as discussions with tooling vendors, as well as many exceptions made for request bodies throughout the spec.A number of issues arise from treating request body like other parameter locations (path, query, header) :
name
is meaningless in body, and is required. To underscore this, from the current spec (I would also argue this serves no documentation purpose):application/json
,application/xml
examples
to the body parameter (Add examples object #636), theexamples
aren’t really applicable to other parameter types.Proposed solution
body
parameter into it’s own key, peer-level toparameters
.name
,in
,required
, anddeprecated
.examples
in parameters (as media types are largely irrelevant). This was already unclear in the spec, and probably needs revision in another PR (as mentioned in Add examples object #636).