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

Support for XML request body and response body #664

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ugocottin
Copy link

Motivation

See this issue for more.

Modifications

Based on modifications in the swift-openapi-runtime, I've added support for XML coding strategy and content-type category.

Result

Bodies with XML content-type can be encoded and decoded through xmlCoder in OpenAPIRuntime.Configuration.

Test Plan

I've updated test for ContentType parsing with application/xml content-type.
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.

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, thank you @ugocottin!

Added a few suggestions, and I wonder if you could also add an XML client and server example to the Examples directory? To show how to connect all the pieces to actually have it all working. You can add a dependency on an existing XML coder in those example packages.

@@ -214,12 +224,17 @@ struct ContentType: Hashable {
/// A Boolean value that indicates whether the content type
/// is a multipart form.
var isMultipart: Bool { category == .multipart }
/// A Boolean value that indicates whether the content type
/// is a type of XML.
var isXml: Bool { category == .xml }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var isXml: Bool { category == .xml }
var isXML: Bool { category == .xml }

Nit: casing, to be consistent with JSON.

- [ ] XML
- [X] XML
- when content type is `application/xml` or ends with `+xml`
- xmlCoder must be defined in `OpenAPIRuntime.Configuration`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- xmlCoder must be defined in `OpenAPIRuntime.Configuration`
- xmlCoder must be provided to `OpenAPIRuntime.Configuration`

| Plain text | `OpenAPIRuntime.StringEncoder` | `OpenAPIRuntime.StringDecoder` | Bodies |

> †: Configurable implementation of variable expansion from URI Template (RFC 6570), the `application/x-www-form-urlencoded` serialization from RFC 1866, and OpenAPI 3.0.3. For details of the supported combinations, review <doc:Supported-OpenAPI-features>.
> †: XML support is optional, and not enabled by default. Encoding and decoding requires to define a `OpenAPIRuntime.CustomCoder` in `OpenAPIRuntime.Configuration` initializer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> †: XML support is optional, and not enabled by default. Encoding and decoding requires to define a `OpenAPIRuntime.CustomCoder` in `OpenAPIRuntime.Configuration` initializer.
> †: XML support is optional, and not enabled by default. Encoding and decoding requires providing a `OpenAPIRuntime.CustomCoder` to the `OpenAPIRuntime.Configuration` initializer.

@@ -46,7 +46,7 @@ final class Test_ContentType: Test_Core {
), ("text/plain", .binary, "text", "plain", "", "text/plain", "text/plain", "text/plain"),
("*/*", .binary, "*", "*", "", "*/*", "*/*", "*/*"),
(
"application/xml", .binary, "application", "xml", "", "application/xml", "application/xml",
"application/xml", .xml, "application", "xml", "", "application/xml", "application/xml",
"application/xml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep the .binary case, just change the example value to something else, like application/customType.

But also add an explicit XML case above.

@czechboy0 czechboy0 linked an issue Oct 31, 2024 that may be closed by this pull request
@czechboy0
Copy link
Contributor

@ugocottin could you also review this section of OpenAPI to see if we should be taking that info into account? https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#xml-object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for XML request body and response body
2 participants