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

[Generator] Choose the serialization method based on content type #48

Merged
merged 15 commits into from
Jun 8, 2023

Conversation

czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented May 31, 2023

Motivation

Fixes #43. Depends on apple/swift-openapi-runtime#12 landing first and tagging a release.

Modifications

Builds on top of the changes to the runtime library.

Result

We now always specify a coding strategy with a Swift type, leading to deterministic and understandable conversion logic.

Test Plan

Updated unit and integration tests, added a 500 case to one of the operations to explicitly test the missing plain text response body.

@czechboy0 czechboy0 requested a review from simonjbeaumont May 31, 2023 08:31
@czechboy0 czechboy0 marked this pull request as draft June 7, 2023 08:10
czechboy0 added a commit to apple/swift-openapi-runtime that referenced this pull request Jun 8, 2023
[Runtime] Choose the serialization method based on content type

### Motivation

Resolves apple/swift-openapi-generator#43. This PR also resolves apple/swift-openapi-generator#42. It's paired with apple/swift-openapi-generator#48.

This PR introduces the initial content-type-aware coding strategy, which is likely to get expanded in the future.

### Modifications

This PR covers the scope described in issue 43 - handling a `type: string` appropriately, based on whether it's wrapped in `application/json` or `text/plan` content.

It introduces new variants of the SPI helper functions, which now has variants per "coding strategy". It's best to read https://github.com/apple/swift-openapi-generator/pull/48/files?short_path=aa92e58#diff-aa92e58fb5a311512cfcd285827a4aa2e6634cae7fdfe0090bd2cfc7b0986140 for details.

Deprecated all of the SPI methods that didn't contain the coding strategy parameter, to be removed in the next breaking version.

### Result

We now have explicit control over the coding strategy for ambiguous types, such as `String`, which conform to both `Codable` and `LosslessStringConvertible`, to be taken advantage of by the generator (see the paired PR).

### Test Plan

Verified that the default behavior now matches version 0.1.0, and when paired with the updated generator, fully resolves issue 43.

Used code coverage to ensure that `Converter+{Common,Client,Server}.swift` are now 100% unit tested.


Reviewed by: simonjbeaumont

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

#12
@czechboy0 czechboy0 merged commit 8b9c564 into apple:main Jun 8, 2023
@czechboy0 czechboy0 deleted the hd-43-content-type-aware-coding branch June 8, 2023 17:06
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jun 8, 2023
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.

Choose the serialization method based on content type
2 participants