-
Notifications
You must be signed in to change notification settings - Fork 9.2k
v3.3: edits to parameter sections #5122
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
base: v3.3-dev
Are you sure you want to change the base?
v3.3: edits to parameter sections #5122
Conversation
490500a to
2eed149
Compare
handrews
left a comment
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.
@karenetheridge thanks for continuing to work through all of these details! Some good catches here. I hope you don't mind me commenting on the draft as it sounds like you are adding more rather than likely to change what you have already posted. Let me know if you would prefer me to wait in the future.
I have a few questions and possible suggestions, and one very reluctant "it's not what I would do but it's what was decided" objection.
| #### Fixed Fields | ||
|
|
||
| The rules for serialization of the parameter are specified in one of two ways. | ||
| The rules for serialization and deserialization of the parameter are specified in one of two ways. |
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 think in other places we have used "serialization and parsing" or "parsing and serialization." Do you think "deserialization" is a better term here? Is there a distinction between that and "parsing"? I have no idea what the answer is, I either copied "parsing" or picked it without much thought, so I'm open to changing it as long as we're consistent.
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 searched the document and found that "deserialize" was used more often than "parse". I've standardized on the former in my implementation (after waffling a few times) as I think the symmetry makes more sense. Also I think "parse" is a little more vague -- you parse a string for any number of reasons, but what we're really doing here when we parse a parameter string is deserializing it into the json document model so we can then validate it against a json schema, or pass it to the user application in a format closer to its final form.
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 searched the document and found that "deserialize" was used more often than "parse".
I doublechecked this assertion and it's false. We talk a lot about "parsing an OAD", and that's definitely the correct language there, but we say both "parse a parameter" and "deserialize a parameter".
I think I still prefer "deserialize" for its extra specificity though, when it is applicable -- an OAD is parsed to interpret its rules, and parts of an HTTP message are deserialized to be applied against the OAD rules.
| | form | false | <span style="white-space: nowrap;">color=</span> | <span style="white-space: nowrap;">color=blue</span> | <span style="white-space: nowrap;">color=blue,black,brown</span> | <span style="white-space: nowrap;">color=R,100,G,200,B,150</span> | | ||
| | form | true | <span style="white-space: nowrap;">color=</span> | <span style="white-space: nowrap;">color=blue</span> | <span style="white-space: nowrap;">color=blue&color=black&color=brown</span> | <span style="white-space: nowrap;">R=100&G=200&B=150</span> | | ||
| | spaceDelimited</span> | false | _n/a_ | _n/a_ | <span style="white-space: nowrap;">color=blue%20black%20brown</span> | <span style="white-space: nowrap;">color=R%20100%20G%20200%20B%20150</span> | | ||
| | form | false | color= | color=blue | color=blue,black,brown | color=R,100,G,200,B,150 | | ||
| | form | true | color= | color=blue | color=blue&color=black&color=brown | R=100&G=200&B=150 | | ||
| | spaceDelimited | false | _n/a_ | _n/a_ | color=blue%20black%20brown | color=R%20100%20G%20200%20B%20150 | |
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.
My recollection is that we need these because otherwise the text will wrap at = and/or & without this, but I'm not at all certain. I tried to build locally to check but I'm having problems with that. Have you verified that it won't wrap without this?
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.
It doesn't appear that there are any characters in the examples that would cause a wrap now, but a check should be done on the final copy before merging -- what's the process for that? I can look at the rendered document on github but that's not the same processing that our publication process uses for html generation.
this was added in commit 9739dfe, but now that the leading "?" has been removed, this is no longer needed
..such as an empty array or object
|
Force-pushing a new version after checking all commits; more test cases have been added for the schema changes. |
6050bd6 to
423d8d3
Compare
423d8d3 to
1372357
Compare
|
As this PR contains some fixes to things that are incorrect, IMO those commits at least should be backported to 3.2 and 3.1. |
685302e to
7de9fda
Compare
namely: in=path, in=query, in=cookie+style=form, but not in=header, in=querystring or in=cookie+style=cookie and not in encoding objects where contentType is used (none of style, explode, allowReserved are present) this brings in some work that didn't get merged in OAI#4904 ..and also fixes a bad $ref URI
… objects - explode defaults were wrong for in: cookie, style: cookie, and for encoding object - allowReserved defaults were wrong for encoding object in parameter objects: - explode: always false for "in: path", "in: header" - explode: always true for "in: cookie" (both "style: form" and "style: cookie" default to "explode: true") - explode: only true for "in: query" when "style: form" (the default style for this location) in encoding objects: - style: default is "form", but only when "explode" or "allowReserved" are present - explode: default is true when "style: form" (the default style) and otherwise false, and not included at all unless "style" or "allowReserved" are present - allowReserved: default is false, but only when "style" or "explode" are present that is: when none of style, explode or allowReserved are present, "contentType" is used (or a default is calculated), so none of style, explode or allowReserved shall have default values
…ll defaults omitted
tested with:
$ openapi-validate --with-defaults tests/schema/pass/style-defaults.yaml
{
"defaults" : {
"/components/mediaTypes/encoding_object_defaults/encoding/allowReserved/explode" : true,
"/components/mediaTypes/encoding_object_defaults/encoding/allowReserved/style" : "form",
"/components/mediaTypes/encoding_object_defaults/encoding/explode/allowReserved" : false,
"/components/mediaTypes/encoding_object_defaults/encoding/explode/style" : "form",
"/components/mediaTypes/encoding_object_defaults/encoding/style_form/allowReserved" : false,
"/components/mediaTypes/encoding_object_defaults/encoding/style_form/explode" : true,
"/components/mediaTypes/encoding_object_defaults/encoding/style_spaceDelimited/allowReserved" : false,
"/components/mediaTypes/encoding_object_defaults/encoding/style_spaceDelimited/explode" : false,
"/components/parameters/cookie_cookie/deprecated" : false,
"/components/parameters/cookie_cookie/explode" : true,
"/components/parameters/cookie_cookie/required" : false,
"/components/parameters/cookie_form/allowReserved" : false,
"/components/parameters/cookie_form/deprecated" : false,
"/components/parameters/cookie_form/explode" : true,
"/components/parameters/cookie_form/required" : false,
"/components/parameters/cookie_form/style" : "form",
"/components/parameters/cookie_media_type/deprecated" : false,
"/components/parameters/cookie_media_type/required" : false,
"/components/parameters/header/allowReserved" : false,
"/components/parameters/header/deprecated" : false,
"/components/parameters/header/explode" : false,
"/components/parameters/header/style" : "simple",
"/components/parameters/path_label/allowReserved" : false,
"/components/parameters/path_label/deprecated" : false,
"/components/parameters/path_label/explode" : false,
"/components/parameters/path_matrix/allowReserved" : false,
"/components/parameters/path_matrix/deprecated" : false,
"/components/parameters/path_matrix/explode" : false,
"/components/parameters/path_media_type/deprecated" : false,
"/components/parameters/path_simple/allowReserved" : false,
"/components/parameters/path_simple/deprecated" : false,
"/components/parameters/path_simple/explode" : false,
"/components/parameters/path_simple/style" : "simple",
"/components/parameters/query_deepObject/allowEmptyValue" : false,
"/components/parameters/query_deepObject/allowReserved" : false,
"/components/parameters/query_deepObject/deprecated" : false,
"/components/parameters/query_deepObject/explode" : false,
"/components/parameters/query_deepObject/required" : false,
"/components/parameters/query_form/allowEmptyValue" : false,
"/components/parameters/query_form/allowReserved" : false,
"/components/parameters/query_form/deprecated" : false,
"/components/parameters/query_form/explode" : true,
"/components/parameters/query_form/required" : false,
"/components/parameters/query_form/style" : "form",
"/components/parameters/query_media_type/allowEmptyValue" : false,
"/components/parameters/query_media_type/deprecated" : false,
"/components/parameters/query_media_type/required" : false,
"/components/parameters/query_pipeDelimited/allowEmptyValue" : false,
"/components/parameters/query_pipeDelimited/allowReserved" : false,
"/components/parameters/query_pipeDelimited/deprecated" : false,
"/components/parameters/query_pipeDelimited/explode" : false,
"/components/parameters/query_pipeDelimited/required" : false,
"/components/parameters/query_spaceDelimited/allowEmptyValue" : false,
"/components/parameters/query_spaceDelimited/allowReserved" : false,
"/components/parameters/query_spaceDelimited/deprecated" : false,
"/components/parameters/query_spaceDelimited/explode" : false,
"/components/parameters/query_spaceDelimited/required" : false,
"/jsonSchemaDialect" : "https://spec.openapis.org/oas/3.2/dialect/WORK-IN-PROGRESS",
"/servers" : [
{
"url" : "/"
}
]
},
"valid" : true
}
(executable is part of https://github.com/karenetheridge/OpenAPI-Modern)
..and remove example that is specific to a particular style, explode and schema type configuration
185f7d4 to
6603f70
Compare
Various fixes and wording clarifications to the sections discussing parameters.