Skip to content

[Runtime] Support unexploded query items #35

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

Merged
merged 5 commits into from
Aug 8, 2023

Conversation

czechboy0
Copy link
Contributor

Motivation

Fixes apple/swift-openapi-generator#52.

By default, query items are encoded as exploded (key=value1&key=value2), but OpenAPI allows explicitly requesting them unexploded (key=value1,value2). This feature missing has shown up in a few OpenAPI documents recently.

Modifications

Add support for unexploded query items by expanding the helper functions and allow passing in style and explode parameters. The style is in preparation of also supporting alternative styles, but for now we just validate that only the supported style is provided.

Result

Generated code can use these improved helper functions to encode/decode unexploded query items.

Test Plan

Updated/added unit tests for unexploded query items.

@czechboy0 czechboy0 merged commit b4c4ada into apple:main Aug 8, 2023
@czechboy0 czechboy0 deleted the hd-unexploded-query-items branch August 8, 2023 07:37
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Aug 8, 2023
czechboy0 added a commit to apple/swift-openapi-generator that referenced this pull request Aug 8, 2023
[Generator] Support unexploded query items

### Motivation

Depends on apple/swift-openapi-runtime#35.

Fixes #52.

By default, query items are encoded as exploded (`key=value1&key=value2`), but OpenAPI allows explicitly requesting them unexploded (`key=value1,value2`). This feature missing has shown up in a few OpenAPI documents recently.

### Modifications

Adapt the generator to provide the two new `style` and `explode` parameters to query item encoding/decoding functions.

### Result

We now support unexploded query items.

### Test Plan

Expanded snippet-based tests to allow generating not just types, but also parts of the client/server. This has allowed us to not have to expand file-based reference tests here.


Reviewed by: glbrntt

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#171
} else {
processedValues = untypedValues.flatMap { multiValue in
multiValue
.split(separator: ",", omittingEmptySubsequences: false)
Copy link

Choose a reason for hiding this comment

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

I'm sorry for the late comment, but in theory I assume it would be possible to have values with commas in them. Would the commas be escaped or percent-encoded in this case? Just want to make sure this split doesn't split the string at incorrect places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah exactly, if the comma is part of the value, it is percent escaped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature "Unexploded query params" is not supported.
3 participants