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

Add support for application/xml body #102

Merged
merged 18 commits into from
Apr 5, 2024
Merged

Add support for application/xml body #102

merged 18 commits into from
Apr 5, 2024

Conversation

ugocottin
Copy link
Contributor

Motivation

See apple/swift-openapi-generator#556 for more.

Modifications

Add converter methods for encoding and decoding XML request and response body.
Add CustomCoder protocol, allows to use other Encoder and Decoder for other content-type body.

User can define custom coder, and assign a custom coder to a specific content-type within Configuration.customCoders dictionary.

Result

It's now possible to define custom encoder and decoder for supported content-type.

Test Plan

Added converter methods are tested with a mock custom coder for application/xml content-type. To avoid adding a dependency to a XMLCoder like CoreOffice/XMLCoder, mock custom coder uses JSONEncoder and JSONDecoder.

Encoding and decoding to XML are out of scope of the tests, because encoding and decoding logic must be provided by user through custom coder implementation.

Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Signed-off-by: Ugo Cottin <ugo.cottin@gmail.com>
Copy link
Collaborator

@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 really good! A few initial comments, mostly minor suggestions.

@czechboy0
Copy link
Collaborator

@swift-server-bot test this please

@ugocottin
Copy link
Contributor Author

@czechboy0 I've made the suggested changes, the PR on swift-openapi-generator will follow soon, thanks

@czechboy0
Copy link
Collaborator

@swift-server-bot test this please

@czechboy0
Copy link
Collaborator

@swift-server-bot test this please

@ugocottin
Copy link
Contributor Author

ugocottin commented Apr 2, 2024

Jenkins CI reports that OpenAPIMIMEType struct is not Sendable, and must be to be use in RuntimeError.missingCoderForCustomContentType(contentType:).

OpenAPIMIMEType seems thread safe to me. Should I make OpenAPIMIMEType (and OpenAPIMIMEType.Kind) conform to Sendable, or use String type in RuntimeError.missingCoderForCustomContentType(contentType:) ?

@czechboy0
Copy link
Collaborator

Just convert it to string when constructing the error.

@ugocottin
Copy link
Contributor Author

@swift-server-bot test this please

@czechboy0
Copy link
Collaborator

@swift-server-bot test this please

@czechboy0
Copy link
Collaborator

@swift-server-bot test this please

@czechboy0
Copy link
Collaborator

Looks great, @ugocottin - will merge after CI passes. Feel free to move onto the generator PR. We'll do a release of the runtime early next week, at which point your generator PR should start passing on CI.

@czechboy0 czechboy0 enabled auto-merge (squash) April 5, 2024 06:20
@czechboy0 czechboy0 merged commit 6e2f9b9 into apple:main Apr 5, 2024
8 checks passed
@czechboy0 czechboy0 added the semver/minor Adds new public API. label Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants