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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ enum Constants {

/// The substring used in method names for the multipart coding strategy.
static let multipart: String = "Multipart"

/// The substring used in method names for the XML coding strategy.
static let xml: String = "XML"
}

/// Constants related to types used in many components.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ enum CodingStrategy: String, Hashable, Sendable {
/// A strategy using multipart/form-data.
case multipart

/// A strategy using optional CustomCoder.
case xml

/// The name of the coding strategy in the runtime library.
var runtimeName: String {
switch self {
Expand All @@ -38,6 +41,7 @@ enum CodingStrategy: String, Hashable, Sendable {
case .binary: return Constants.CodingStrategy.binary
case .urlEncodedForm: return Constants.CodingStrategy.urlEncodedForm
case .multipart: return Constants.CodingStrategy.multipart
case .xml: return Constants.CodingStrategy.xml
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ extension FileTranslator {
///
/// Priority:
/// 1. JSON
/// 2. text
/// 3. binary
/// 2. XML
/// 3. text
/// 4. binary
///
/// - Parameters:
/// - map: The content map from the OpenAPI document.
Expand Down Expand Up @@ -108,8 +109,9 @@ extension FileTranslator {
///
/// Priority:
/// 1. JSON
/// 2. text
/// 3. binary
/// 2. XML
/// 3. text
/// 4. binary
///
/// - Parameters:
/// - map: The content map from the OpenAPI document.
Expand All @@ -127,7 +129,7 @@ extension FileTranslator {
let mapWithContentTypes = try map.map { key, content in try (type: key.asGeneratorContentType, value: content) }

let chosenContent: (type: ContentType, schema: SchemaContent, content: OpenAPI.Content)?
if let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isJSON }) {
if let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isJSON || $0.type.isXml }) {
chosenContent = (contentType, .init(contentType: contentType, schema: contentValue.schema), contentValue)
} else if !excludeBinary,
let (contentType, contentValue) = mapWithContentTypes.first(where: { $0.type.isBinary })
Expand Down Expand Up @@ -160,6 +162,7 @@ extension FileTranslator {
///
/// Priority of checking for known MIME types:
/// 1. JSON
/// 2. XML
/// 2. text
/// 3. binary
///
Expand Down Expand Up @@ -188,6 +191,7 @@ extension FileTranslator {
)
}
if contentType.isJSON { return .init(contentType: contentType, schema: contentValue.schema) }
if contentType.isXml { return .init(contentType: contentType, schema: contentValue.schema) }
if contentType.isUrlEncodedForm { return .init(contentType: contentType, schema: contentValue.schema) }
if contentType.isMultipart {
guard isRequired else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,14 @@ struct ContentType: Hashable {
/// The type is encoded as an async sequence of parts.
case multipart

/// A content type for XML.
///
/// The bytes are provided to a CustomCoder.
case xml

/// Creates a category from the provided type and subtype.
///
/// First checks if the provided content type is a JSON, then text,
/// First checks if the provided content type is a JSON, then XML, then text,
/// and uses binary if none of the two match.
/// - Parameters:
/// - lowercasedType: The first component of the MIME type.
Expand All @@ -68,6 +73,10 @@ struct ContentType: Hashable {
if (lowercasedType == "application" && lowercasedSubtype == "json") || lowercasedSubtype.hasSuffix("+json")
{
self = .json
} else if (lowercasedType == "application" && lowercasedSubtype == "xml")
|| lowercasedSubtype.hasSuffix("+xml")
{
self = .xml
} else if lowercasedType == "application" && lowercasedSubtype == "x-www-form-urlencoded" {
self = .urlEncodedForm
} else if lowercasedType == "multipart" && lowercasedSubtype == "form-data" {
Expand All @@ -84,6 +93,7 @@ struct ContentType: Hashable {
case .binary: return .binary
case .urlEncodedForm: return .urlEncodedForm
case .multipart: return .multipart
case .xml: return .xml
}
}
}
Expand Down Expand Up @@ -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.


/// The content type `text/plain`.
static var textPlain: Self { try! .init(string: "text/plain") }

/// The content type `application/json`.
static var applicationJSON: Self { try! .init(string: "application/json") }
/// The content type `application/xml`.
static var applicationXML: Self { try! .init(string: "application/xml") }

/// The content type `application/octet-stream`.
static var applicationOctetStream: Self { try! .init(string: "application/octet-stream") }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ extension FileTranslator {
])
let bodyExpr: Expression
switch codingStrategy {
case .json, .uri, .urlEncodedForm:
case .json, .xml, .uri, .urlEncodedForm:
// Buffering.
bodyExpr = .try(.await(converterExpr))
case .binary, .multipart:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ extension ServerFileTranslator {
)
let bodyExpr: Expression
switch codingStrategy {
case .json, .uri, .urlEncodedForm:
case .json, .xml, .uri, .urlEncodedForm:
// Buffering.
bodyExpr = .try(.await(converterExpr))
case .binary, .multipart:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ extension ClientFileTranslator {
)
let bodyExpr: Expression
switch codingStrategy {
case .json, .uri, .urlEncodedForm:
case .json, .xml, .uri, .urlEncodedForm:
// Buffering.
bodyExpr = .try(.await(converterExpr))
case .binary, .multipart:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ For any other formats, the payload is provided as raw bytes (using the `HTTPBody
- when content type is `application/x-www-form-urlencoded`
- [x] multipart
- for details, see [SOAR-0009](https://swiftpackageindex.com/apple/swift-openapi-generator/main/documentation/swift-openapi-generator/soar-0009)
- [ ] 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`


### OpenAPI specification features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ At the time of writing, the list of coders used is as follows.
| Format | Encoder | Decoder | Supported in |
| ------ | ------- | ------- | ----- |
| JSON | `Foundation.JSONEncoder` | `Foundation.JSONDecoder` | Bodies, headers |
| URI (†) | `OpenAPIRuntime.URIEncoder` | `OpenAPIRuntime.URIDecoder` | Path, query, headers |
| XML (†) | `OpenAPIRuntime.CustomCoder` | `OpenAPIRuntime.CustomCoder` | Bodies |
| URI (††) | `OpenAPIRuntime.URIEncoder` | `OpenAPIRuntime.URIDecoder` | Path, query, headers |
| 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.


> ††: 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>.

While the generator attempts to catch invalid inputs at generation time, there are still combinations of `Codable` types and locations that aren't compatible, and will only get caught at runtime by the specific coder implementation. For example, one could ask the `StringEncoder` to encode an array, but the encoder will throw an error, as containers are not supported in that encoder.

Expand All @@ -50,6 +53,9 @@ Below is a list of the "dimensions" across which the helper methods differ:
- `JSON`
- example content type: `application/json` and any with the `+json` suffix
- `{"color": "red", "power": 24}`
- `XML`
- example content type: `application/xml` and any with the `+xml` suffix
- `<root><color>red</color><power>24</power></root>`
- `URI`
- example: query, path, header parameters
- `color=red&power=24`
Expand Down Expand Up @@ -93,24 +99,30 @@ method parameters: value or type of value
| client | set | request query | URI | both | setQueryItemAsURI |
| client | set | request body | JSON | optional | setOptionalRequestBodyAsJSON |
| client | set | request body | JSON | required | setRequiredRequestBodyAsJSON |
| client | set | request body | XML | optional | setOptionalRequestBodyAsXML |
| client | set | request body | XML | required | setRequiredRequestBodyAsXML |
| client | set | request body | binary | optional | setOptionalRequestBodyAsBinary |
| client | set | request body | binary | required | setRequiredRequestBodyAsBinary |
| client | set | request body | urlEncodedForm | optional | setOptionalRequestBodyAsURLEncodedForm |
| client | set | request body | urlEncodedForm | required | setRequiredRequestBodyAsURLEncodedForm |
| client | set | request body | multipart | required | setRequiredRequestBodyAsMultipart |
| client | get | response body | JSON | required | getResponseBodyAsJSON |
| client | get | response body | XML | required | getResponseBodyAsXML |
| client | get | response body | binary | required | getResponseBodyAsBinary |
| client | get | response body | multipart | required | getResponseBodyAsMultipart |
| server | get | request path | URI | required | getPathParameterAsURI |
| server | get | request query | URI | optional | getOptionalQueryItemAsURI |
| server | get | request query | URI | required | getRequiredQueryItemAsURI |
| server | get | request body | JSON | optional | getOptionalRequestBodyAsJSON |
| server | get | request body | JSON | required | getRequiredRequestBodyAsJSON |
| server | get | request body | XML | optional | getOptionalRequestBodyAsXML |
| server | get | request body | XML | required | getRequiredRequestBodyAsXML |
| server | get | request body | binary | optional | getOptionalRequestBodyAsBinary |
| server | get | request body | binary | required | getRequiredRequestBodyAsBinary |
| server | get | request body | urlEncodedForm | optional | getOptionalRequestBodyAsURLEncodedForm |
| server | get | request body | urlEncodedForm | required | getRequiredRequestBodyAsURLEncodedForm |
| server | get | request body | multipart | required | getRequiredRequestBodyAsMultipart |
| server | set | response body | JSON | required | setResponseBodyAsJSON |
| server | set | response body | XML | required | setResponseBodyAsXML |
| server | set | response body | binary | required | setResponseBodyAsBinary |
| server | set | response body | multipart | required | setResponseBodyAsMultipart |
Original file line number Diff line number Diff line change
Expand Up @@ -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.

),
(
Expand Down