Support multiple request body content types #7
Assignees
Labels
area/openapi
Adding/updating a feature defined in OpenAPI.
kind/feature
New feature.
size/M
Medium task. (A couple of days of work.)
Comments
czechboy0
added
area/openapi
3 tasks
czechboy0
added a commit
that referenced
this issue
Aug 4, 2023
[Generator] Multiple content types support Depends on apple/swift-openapi-runtime#29 ### Motivation Up until now, when an OpenAPI document contained more than one content type in either a request or a response body, we would only generate code for now, and ignored the other ones. However, that was a temporary workaround until we could add proper support for multiple content types, which is what this PR is about. Addresses #6 and #7, except for the "Accept header" propagation, which will be addressed separately and have its own PR and even a proposal. That's one of the reasons this feature is landing disabled, hidden behind the `multipleContentTypes` feature flag. A tip for reviewing this change: first check out how the test OpenAPI document and the reference files changed, that shows the details of how we encode and decode multiple content types. I also added comments in the code for easier review. ### Modifications - Main: all supported content types found in a `content` map in request and response bodies are now generated in the `Body` enum. - Added a method `supportedTypedContents` that returns all supported content types, used now instead of the `bestTypedContent` method we used before. What is a "supported" content type? One that has a schema that doesn't return `false` to `isSchemaSupported`. (This is a pre-existing concept.) - Updated the logic for generating request and response bodies to use the new method, both on client and server. - Updated content type -> Swift enum case name logic, but that will go through a full proposal, so please hold feedback for the proposal, this is just a first pass based on some collected data of most commonly used content types. - ### Open Questions - What to do in an operation with multiple request/response content types when _no_ `content-type` header is provided? Previously, with up to 1 content type support, we still went ahead and tried to parse it. But here, we don't know which content to parse it as, so we just try it with the first in the list (we respect the order in the YAML dictionary, so what the user put first will be what we try to use when no `content-type` header is provided). If an invalid `content-type` value is provided, we throw an error (pre-existing behavior). Q: Is choosing the first reasonable? What would be better? Note that, unfortunately, many servers don't send `content-type` today. ### Result When an adopter provides multiple content types in their OpenAPI document, we generate one case in the Body enum for each! However, the new functionality is disabled by default, has to be explicitly requested by enabling the `multipleContentTypes` feature flag either on the CLI or in the config file. ### Test Plan Added `setStats` and `getStats` operations that employ multiple content types, one in a request body, the other in a response body. Added unit tests for it in `PetstoreConsumerTests` to verify it all works correctly at runtime. Deleted `Tests/OpenAPIGeneratorCoreTests/Translator/RequestBody/Test_translateRequestBody.swift` as snippet tests do this job better, and I didn't want to spend the time updating the test. Adds `PetstoreConsumerTests_FF_MultipleContentTypes`, a variant of `PetstoreConsumerTests` for when the `multipleContentTypes` feature flag is enabled, so we test both how the existing logic handles multiple content types (it picks one), and the new logic (it generates code for all supported ones it found). ## TODOs - [x] rename `IfConditionPair` to `IfBranch` - [x] break out the first `if` from the subsequence `else if`s in `IfStatement`, to 3 properties: first if, then an array of else ifs, then an optional else; to avoid creating invalid code - [x] create an SPI type `ContentType` to avoid repeatedly parsing the received content type Reviewed by: gjcairo, 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. #146
|
Landed in main under the |
|
Now released in 0.1.7 under the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Similar to #6 but for request content.
Should be simpler, as it doesn't require the integration with the Accept header. It'll just be an extension of the existing N=1 media types into N>1. The generated type is already an enum though, so shouldn't require structural changes, just handling all the supported media types instead of picking the first "best" one (like the generator does today).
The text was updated successfully, but these errors were encountered: