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.

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

[Runtime] Design away EncodableBodyContent #30

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

czechboy0
Copy link
Contributor

Motivation

Until #12 (Choose the serialization method based on content type), we were still looking for the right way to spell the various encoding/decoding methods that take into account both the type of the underlying schema, and the desired content type. In #12, we found something decent, but we could have simplified the spelling even further, getting rid of a whole closure invocation and a specialized type.

Modifications

In this PR (in preparation for multiple content type support), we do the simplification by completely removing EncodableBodyContent and the associated closure trampoline that we used to propoagate the content type string. We now pass the string directly, and simplified all the helper functions that have to do with encoding body content.

Now, this is done in a backwards compatible way, so we really just deprecated all the existing methods, and the EncodableBodyContent type, but it'll all be cleaned up when we prepare for tagging the next breaking version.

Result

The helper functions have a simpler spelling, which simplifies the generator and reduces the lines of code we have to generate.

(The associated generator PR will be coming in shortly.)

Test Plan

I preserved all the tests for the deprecated variants, and added new unit tests for the new variants. The deprecated tests will also be cleaned up in the next breaking version.

@czechboy0 czechboy0 requested a review from simonjbeaumont July 31, 2023 08:02
@czechboy0 czechboy0 changed the title Design away EncodableBodyContent [Runtime] Design away EncodableBodyContent Jul 31, 2023
@czechboy0 czechboy0 merged commit 5b5b5ef into apple:main Jul 31, 2023
@czechboy0 czechboy0 deleted the hd-remove-EncodableBodyContent branch July 31, 2023 15:35
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jul 31, 2023
czechboy0 added a commit to apple/swift-openapi-generator that referenced this pull request Jul 31, 2023
[Generator] Design away EncodableBodyContent

### Motivation

See apple/swift-openapi-runtime#30

Depends on Runtime 0.1.6

### Modifications

Adapted the generator code for the runtime changes. Also, since this was factored out of the big PR (#146) adding multiple content types, you can see the code was prepared for handling multiple content types, even though still in this PR it only ever gets N=1.

### Result

The generated code uses the new simplified runtime functions, and prepares us for multiple content types.

### Test Plan

Adapted file-based reference tests to generate the new syntax.


Reviewed by: simonjbeaumont

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. 

#152
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.

2 participants