Skip to content

Conversation

bfrearson
Copy link
Contributor

@bfrearson bfrearson commented Sep 18, 2023

Motivation

Issue 182 Supporting runtime update for add URL form encoder & decoder

Modifications

Add converter methods for Codable <--> URLEncodedForm.

Note: Trying to encode a primitive collection (e.g. an array of String) form a Codable type fails because the URIEncoder rejects nested object types. I didn't make any changes to the Encoder here, as I think there may be a larger discussion needed about how to approach this. It would be beneficial to support this, as there may be a form which contains multiple values for the same key.

Decoding an exploded key from to a Codable type containing an array works already, but I've not included the test in this PR, as that should be included in the above.

Result

Converter methods for URLEncodedForms are now available for use in generated content.

Test Plan

Update tests for encoders and decoders.

@bfrearson bfrearson marked this pull request as ready for review September 19, 2023 09:33
@czechboy0
Copy link
Contributor

Note: Trying to encode a primitive collection (e.g. an array of String) form a Codable type fails because the URIEncoder rejects nested object types. I didn't make any changes to the Encoder here, as I think there may be a larger discussion needed about how to approach this. It would be beneficial to support this, as there may be a form which contains multiple values for the same key.

@bfrearson, please file a separate issue about that if it's specific to the URI encoder/decoder and not specific to using it for bodies here. If it doesn't work, it'd affect query items as well, which can commonly be arrays.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Looks great, just a few minor suggestions, otherwise almost ready to go.

@bfrearson
Copy link
Contributor Author

@czechboy0 All changes complete on both generator & runtime 👍🏻

@czechboy0 czechboy0 merged commit 9ac0b62 into apple:main Sep 25, 2023
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Sep 25, 2023
czechboy0 added a commit to apple/swift-openapi-generator that referenced this pull request Sep 25, 2023
### Motivation

#182 Currently, the generated interface for request bodies that use
URLEncodedForms accepts a `Data` object. This PR uses the
URIEncoder/Decoder to instead provide an interface using codable types
in the same way that a JSON body would.

### Modifications
Adds a new coding strategy that works with the corresponding [runtime
converter
updates](apple/swift-openapi-runtime#53).

**Note:** This PR does not support encoding collections of primitives
(i.e. an array of string values for a single key). This should be
addressed in a future update, but it raised questions about the correct
use of URIEncoder, so I didn't include it in this PR. Further discussion
of the issue is in the [runtime
PR](apple/swift-openapi-runtime#53).

### Result
Generated interfaces now directly accept URLEncodedForm Codable types
rather than untyped `Data`.

### Test Plan

Updated reference tests to include a request using a form body.

---------

Co-authored-by: benfrearson <[email protected]>
Co-authored-by: bfrearson <>
Co-authored-by: Honza Dvorsky <[email protected]>
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.

3 participants