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.

Sign up for GitHub

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

Jump to bottom

Support for XML request body and response body #556

Open
ugocottin opened this issue Mar 28, 2024 · 10 comments · May be fixed by #664
Open

Support for XML request body and response body #556

ugocottin opened this issue Mar 28, 2024 · 10 comments · May be fixed by #664
Assignees
@ugocottin
Labels
area/generator Affects: plugin, CLI, config file. area/runtime Affects: the runtime library. kind/feature New feature. size/M Medium task. (A couple of days of work.)
Milestone
Post-1.0

Comments

@ugocottin
Copy link

ugocottin commented Mar 28, 2024 •
edited
Loading

Motivation

I want to be able de generate client and server from an OpenAPI specification, for an API which use XML as content type for request and response body.

Currently, application/xml contents for request and response are not generated by swift-openapi-generator, and only OpenAPIRuntime.HTTPBody enum associated value is available for xml content.

A Workaround is to use an external XML encoder and decoder, like CoreOffice XMLCoder, and make the encoding from / decoding to HTTPBody.

struct Credentials: Encodable {
    var username: String
    var password_hash: String
}

struct Token: Decodable {
    var token: String
}
let credentials = Credentials(username: "johnapplessed", password_hash: "d2hhdCBkaWQgeW91IGV4cGVjdD8=")
let encoder = XMLEncoder()
let encoded = try encoder.encode(credentials)
let response = try await client.authenticate(body: .xml(HTTPBody(encoded)))
let xmlBody = try response.ok.body.xml
let xmlData = try await Data(collecting: xmlBody, upTo: .max)
let decoder = XMLDecoder()
let decoded = try decoder.decode(Token.self, from: xmlData)
let token = decoded.token

Proposed solution

One quick solution could be to implement a new CodingStrategy for xml in swift-openapi-generator.
For swift-openapi-runtime, we could add the following methods to Converter:

Client/server Set/get Schema location Coding strategy Optional/required Method name
client set request body XML optional setOptionalRequestBodyAsXML
client set request body XML required setRequiredRequestBodyAsXML
client get response body XML required getResponseBodyAsXML
server get request body XML optional getOptionalRequestBodyAsXML
server get request body XML required getRequiredRequestBodyAsXML
server set response body XML required setResponseBodyAsXML

Encoding / Decoding logic can be implemented into these methods, with an XML encoder / decoder.
As Converter have encoder and decoder properties as type JSONEncoder and JSONDecoder, we could:

  1. Rename encoder to jsonEncoder and decoder to jsonDecoder, and instantiate a xmlEncoder and xmlDecoder in Converter init.
  2. Instantiate a XMLEncoder or XMLDecoder in call of methods above.

Example:

extension Converter {
    public func setRequiredRequestBodyAsXML<T: Encodable>(
        _ value: T,
        headerFields: inout HTTPFields,
        contentType: String
    ) throws -> HTTPBody {
        try setRequiredRequestBody(
            value,
            headerFields: &headerFields,
            contentType: contentType,
            convert: convertBodyCodableToXML
        )
    }
}

Alternatives considered

I found that Converter struct is not suited for adding support of new content type. With the proposed solution, the Converter struct will have types of <#HTTPMediaType#>Encoder and <#HTTPMediaType#>Decoder, while not necessarily needed.
For XML only API, Converter still need a JSONEncoder and JSONDecoder.

I thought that, maybe, and I would like to have your opinion on this, we must implement a Converter for a specific http media type, like Vapor is doing with ContentEncoder and ContentDecoder. Each http media type would be assigned to a specific Converter.

What's your thoughts about this?

Additional information

No response
@ugocottin ugocottin added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Mar 28, 2024
@czechboy0
Copy link
Contributor

Hi @ugocottin,

yes XML is officially supported by the OpenAPI specification, so it'd be reasonable for us to support it here.

Since we already generate Codable types, I think the main piece of work would be to create an XMLEncoder and XMLDecoder that can work with Codable types, and then we can update the generator.

Once that's in place, your plan above with extending the Converter, adding a CodingStrategy is exactly right.

A heads up - this would need to be contributed by you, and we'd help steer the code through pull request review.

If that sounds good, go ahead! 🙏

@czechboy0
Copy link
Contributor

Oh I missed that you already linked to such encoder/decoder in https://github.com/CoreOffice/XMLCoder.

The only concern there is that it's a dependency that isn't API-stable.

I wonder if we could somehow offer this as explicitly opt-in, where only adopters who want XML would include that dependency.

Maybe some new optional conversion methods on OpenAPIRuntime.Configuration, able to convert between HTTPBody and Codable using a custom encoder/decoder.

@ugocottin ugocottin mentioned this issue Apr 2, 2024
Merged
@ugocottin
Copy link
Author

Hello @czechboy0

Thanks for the feedback. I've submitted a first pull request to apple/swift-openapi-runtime#102.

As you suggested, I added the possibility to define custom encoding / decoding behaviour through OpenAPIRuntime.Configuration, with CustomCoder protocol.

Thanks!

@czechboy0
Copy link
Contributor

Looks good - feel free to open a PR for the generator as well (even if it won't pass until the runtime change lands and is released), it's easier to review both changes together.

czechboy0 added a commit to apple/swift-openapi-runtime that referenced this issue Apr 5, 2024
@ugocottin @czechboy0
Add support for application/xml body (#102)
6e2f9b9 
### Motivation

See
[apple/swift-openapi-generator#556](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](https://github.com/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 <[email protected]>
Co-authored-by: Honza Dvorsky <[email protected]>
@czechboy0 czechboy0 added area/generator Affects: plugin, CLI, config file. area/runtime Affects: the runtime library. size/M Medium task. (A couple of days of work.) and removed status/triage Collecting information required to triage the issue. labels Apr 16, 2024
@czechboy0 czechboy0 added this to the Post-1.0 milestone Apr 16, 2024
@czechboy0
Copy link
Contributor

Ok @ugocottin, the runtime changes landed in swift-openapi-runtime 1.4.0, so please bump the dependency of the generator's Package.swift and you should be able to finish the generator work.

@czechboy0
Copy link
Contributor

As part of the generator PR, it'd be great to also add an example project that shows how to bring your own XML encoder/decoder and plug it all together (see the Examples directory).

@ugocottin
Copy link
Author

Hi @czechboy0, thanks for letting me know. I will make a pr soon

@czechboy0
Copy link
Contributor

Hi @ugocottin, do you expect to still be able to finish this support, or should I unassign it for now and let someone else pick it up? 🙂

@czechboy0
Copy link
Contributor

Hi @ugocottin, pinging one more time - are you planning to finish the generator implementation, or should we unassign it and have someone else pick it up? 🙂

@ugocottin
Copy link
Author

Hi @czechboy0, sorry for the delay. I will finish the implementation this week

@ugocottin ugocottin linked a pull request Oct 31, 2024 that will close this issue
Open
@czechboy0 czechboy0 linked a pull request Oct 31, 2024 that will close this issue
Support for XML request body and response body #664
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ugocottin ugocottin

Labels
area/generator Affects: plugin, CLI, config file. area/runtime Affects: the runtime library. kind/feature New feature. size/M Medium task. (A couple of days of work.)
Projects
None yet
Milestone
Post-1.0
Development

Successfully merging a pull request may close this issue.

Support for XML request body and response body ugocottin/swift-openapi-generator
2 participants
@czechboy0 @ugocottin