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.

Sign up for GitHub

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

Jump to bottom

Parse Query Params From Path in Addition to Method #71

Merged
merged 6 commits into from
Aug 27, 2021
Merged

Parse Query Params From Path in Addition to Method #71

merged 6 commits into from
Aug 27, 2021

Conversation

bpross
Copy link
Contributor

@bpross bpross commented Aug 12, 2021 •
edited
Loading

Parse any query params that are defined on the path object in addition to parsing them on the method.

Included

  • Parse query params that are included on the path instead of just the method. This allows code snippets to include any params that are required for all methods on a path.
  • Small refactor to reduce duplicate code required to parse parameters on the method and the path.
  • Test to verify that the new behavior works correctly and the snippet generated contains the correct information.
  • Test to verify that overriding query string params work
  • Test to verify that GET snippets with no query strings work as well

@bpross
Copy link
Contributor Author

bpross commented Aug 24, 2021

@ErikWittern do you mind taking a look when you have a change? Thanks!

ErikWittern
ErikWittern approved these changes Aug 27, 2021
View reviewed changes
Copy link
Owner

@ErikWittern ErikWittern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bpross Hi Benjamin, this is an excellent PR, thank you very much for it!!!

I will merge it, and then look at #72.

@ErikWittern ErikWittern merged commit 859be04 into ErikWittern:main Aug 27, 2021
@ErikWittern ErikWittern mentioned this pull request Aug 27, 2021
Merged
@bpross bpross deleted the query-string-from-path branch August 27, 2021 15:31
brianstrauch added a commit to confluentinc/openapi-snippet that referenced this pull request Jan 15, 2025
@brianstrauch @d-silva
@dsilva-alviere @ErikWittern @bpross @restyler @michaelgwelch @dependabot
Merge ErikWittern/openapi-snippet (#13)
701844b 
* Add x-www-form-urlencoded media type examples support (ErikWittern#67)

* Add x-www-form-urlencoded media type

* ditch querystring dependency

* escape keys and values

* add test

* fix: replace all '%20' occurrences

* fix: use replace function with wildcard instead of replaceAll

* improve syntax

Co-authored-by: Diogo Silva <[email protected]>

* Bump version; fix typo

* Parse Query Params From Path in Addition to Method  (ErikWittern#71)

* Update query string parsing to look for parameters defined on the path in addition to the method.

* Add return to method comment block

* Fix comment formatting

* Add tests to verify that method params override path params. Update query strings so method params override path params

* Format all code

* Generate Snippets for Multiple Content-Types (ErikWittern#72)

* Add support for multipart/form-data snippet generation. Add tests to verify behavior

* Add functionality to generate multiple snippets for an endpoint method if there are multiple content-type request bodies. Refactor generation methods to adjust for return type changes. Refactor header parsing to move content-type header to come from the postData parsing

* Set mimeType to contentType instead of using magic string

* Don't copy sample

* Fix formatting

* DRY payloads method up a bit

* Move snippet creation into its own function. Only set mimeType if har.comment is undefined, not set to undefined. Fix some formatting

* run prettier on code

* Bump version

* Fix bug when generating multipart/form-data requests (ErikWittern#73)

Form data must be of type string, so stringify objects and arrays

* Bump version

* bump httpsnippet to latest v2 version (ErikWittern#75)

* Bump version

* Add csharp_httpclient to readme and link (ErikWittern#79)

* Revise parameter handling for snippets (ErikWittern#86)

* Revise example swagger to match test

There are three test cases dealing with array value query parameters
in test/parameter_example_swagger.json. The assertion for each of
these is wrong as it assumes the array should not be exploded.

However, two of these parameters are explicitly form style. The third
is implicitly form style because that is the default for query parameters.
None of them specify a value for explode, so this defaults to true.
Therefore the assertions are wrong.

See https://spec.openapis.org/oas/v3.0.3#fixed-fields-9

This commit does not fix the logic in the code, rather (for now) it
just updates the spec that is being used for the test to set explode
equal to false.

* create function createHarParameterObjects

This function takes an OpenAPI parameter and a sample value and
creates a HAR "parameter" object which is just a {name: "", value: ""}
object.

The function handles all of the variatons outlined in the spec
and this commit adds all the test cases that are presented
here: https://swagger.io/docs/specification/serialization

This function will be used in future commits to handle every
type of parrameter while generating snippets.

* refactor getParameterValues in terms of createHarParameterObjects

This doesn't change the functionality yet. This is just in preparation
for providing full support for all variations of parameters.

This commit breaks no tests but prepares us for more test cases.

* Add support for exploded values in query parameters

* Add support for simple style exploded values in path

* Add tests to demonstrate working label style path params

* Add tests to demonstrate working matrix style path params

* Add support for pipe and space delimited path params

* Add tests to demonstrate working object and array values in headers

* Re-order checks for candidate example param values

Per the swagger guide: https://swagger.io/docs/specification/describing-parameters/

"Using default to specify a sample value. This is not intended use of
default and can lead to unexpected behavior in some Swagger tools.
Use the example or examples keyword for this purpose instead."

Per this I think `default` should be the sample value of last resort
instead favoring `example` and `examples` first.

* Add support for samples in examples key

This works for header, path, and query parameter which
will now be discovered if the sample is in the examles key.

The snippet will choose the first example in examples.

* refactor getQueryStrings in terms of reusable method

getQueryStrings was doing something special that getFullPath
and getHeadersArray are not doing: considering both path
and operation level defined parameters. By parameterizing this
method on `in` we can later use it for `path`, `header` and
`cookie` parameters as well.

* Refactor getFullPath and getHeadersArray

These functions are now written in terms of
getParameterCollectionIn which does the following correctly

* Considers path and operation parameters and overrides
  appropriately
* Resolves parameter references
* Resolves schema references
* Looks at example, examples, schema.example and default keys
  looking for sample value

So now getFullPath and getHeadersArray benefit from all of this
logic. Each had one or more short comings.

* Add support for cookie parameters

* Resolve examples objects as needed

* Bump shell-quote from 1.7.2 to 1.7.3 (ErikWittern#88)

Bumps [shell-quote](https://github.com/substack/node-shell-quote) from 1.7.2 to 1.7.3.
- [Release notes](https://github.com/substack/node-shell-quote/releases)
- [Changelog](https://github.com/substack/node-shell-quote/blob/master/CHANGELOG.md)
- [Commits](https://github.com/substack/node-shell-quote/compare/v1.7.2...1.7.3)

---
updated-dependencies:
- dependency-name: shell-quote
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump cached-path-relative from 1.0.2 to 1.1.0 (ErikWittern#76)

Bumps [cached-path-relative](https://github.com/ashaffer/cached-path-relative) from 1.0.2 to 1.1.0.
- [Release notes](https://github.com/ashaffer/cached-path-relative/releases)
- [Commits](https://github.com/ashaffer/cached-path-relative/commits)

---
updated-dependencies:
- dependency-name: cached-path-relative
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump ajv from 6.12.0 to 6.12.6 (ErikWittern#77)

Bumps [ajv](https://github.com/ajv-validator/ajv) from 6.12.0 to 6.12.6.
- [Release notes](https://github.com/ajv-validator/ajv/releases)
- [Commits](ajv-validator/ajv@v6.12.0...v6.12.6)

---
updated-dependencies:
- dependency-name: ajv
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Increase version; update dependencies

---------

Co-authored-by: Diogo Silva <[email protected]>
Co-authored-by: Diogo Silva <[email protected]>
Co-authored-by: ErikWittern <[email protected]>
Co-authored-by: Benjamin Ross <[email protected]>
Co-authored-by: Benjamin Ross <[email protected]>
Co-authored-by: restyler <[email protected]>
Co-authored-by: Davis Martin <[email protected]>
Co-authored-by: Michael Welch <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ErikWittern ErikWittern ErikWittern approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bpross @ErikWittern