From 5399b9e30d8d68dd1a86fad31128fe4a33ad0703 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Mon, 31 Jul 2023 10:55:12 +0200 Subject: [PATCH 1/3] Design away EncodableBodyContent --- .../RequestBody/translateRequestBody.swift | 103 ++++---- .../Responses/translateResponseOutcome.swift | 106 ++++----- .../ReferenceSources/Petstore/Client.swift | 61 ++--- .../ReferenceSources/Petstore/Server.swift | 223 ++++++++---------- 4 files changed, 219 insertions(+), 274 deletions(-) diff --git a/Sources/_OpenAPIGeneratorCore/Translator/RequestBody/translateRequestBody.swift b/Sources/_OpenAPIGeneratorCore/Translator/RequestBody/translateRequestBody.swift index bc2b4bc2..68d7b988 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/RequestBody/translateRequestBody.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/RequestBody/translateRequestBody.swift @@ -175,63 +175,60 @@ extension ClientFileTranslator { requestVariableName: String, inputVariableName: String ) throws -> Expression { + let contents = [requestBody.content] + var cases: [SwitchCaseDescription] = contents.map { typedContent in + let content = typedContent.content + let contentType = content.contentType + let contentTypeIdentifier = contentSwiftName(contentType) + let contentTypeHeaderValue = contentType.headerValueForSending - let typedContent = requestBody.content - let content = typedContent.content - let contentType = content.contentType - let contentTypeIdentifier = contentSwiftName(contentType) - let contentTypeHeaderValue = contentType.headerValueForSending - - let transformReturnExpr: Expression = .return( - .dot("init") - .call([ - .init( - label: "value", - expression: .identifier("value") - ), - .init( - label: "contentType", - expression: .literal(contentTypeHeaderValue) - ), - ]) - ) - let caseDecl: SwitchCaseDescription = .init( - kind: .case(.dot(contentTypeIdentifier), ["value"]), - body: [ - .expression(transformReturnExpr) - ] - ) - let transformExpr: Expression = .closureInvocation( - argumentNames: ["wrapped"], - body: [ - .expression( - .switch( - switchedExpression: .identifier("wrapped"), - cases: [ - caseDecl - ] - ) + let bodyAssignExpr: Expression = .assignment( + left: .identifier(requestVariableName).dot("body"), + right: .try( + .identifier("converter") + .dot( + "set\(requestBody.request.required ? "Required" : "Optional")RequestBodyAs\(contentType.codingStrategy.runtimeName)" + ) + .call([ + .init(label: nil, expression: .identifier("value")), + .init( + label: "headerFields", + expression: .inOut( + .identifier(requestVariableName).dot("headerFields") + ) + ), + .init( + label: "contentType", + expression: .literal(contentTypeHeaderValue) + ), + ]) ) - ] - ) - return .assignment( - left: .identifier(requestVariableName).dot("body"), - right: .try( - .identifier("converter") - .dot( - "set\(requestBody.request.required ? "Required" : "Optional")RequestBodyAs\(contentType.codingStrategy.runtimeName)" + ) + let caseDesc: SwitchCaseDescription = .init( + kind: .case(.dot(contentTypeIdentifier), ["value"]), + body: [ + .expression(bodyAssignExpr) + ] + ) + return caseDesc + } + if !requestBody.request.required { + let noneCase: SwitchCaseDescription = .init( + kind: .case(.dot("none")), + body: [ + .expression( + .assignment( + left: .identifier(requestVariableName).dot("body"), + right: .literal(.nil) + ) ) - .call([ - .init(label: nil, expression: .identifier(inputVariableName).dot("body")), - .init( - label: "headerFields", - expression: .inOut( - .identifier(requestVariableName).dot("headerFields") - ) - ), - .init(label: "transforming", expression: transformExpr), - ]) + ] ) + cases.insert(noneCase, at: 0) + } + return .switch( + switchedExpression: .identifier(inputVariableName).dot("body"), + cases: cases ) } } diff --git a/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift b/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift index 422042d1..025001ea 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift @@ -306,73 +306,63 @@ extension ServerFileTranslator { } codeBlocks.append(contentsOf: headerExprs.map { .expression($0) }) - if let typedContent = try bestSingleTypedContent( + let typedContents = [try bestSingleTypedContent( typedResponse.response.content, inParent: bodyTypeName - ) { - let contentTypeHeaderValue = typedContent.content.contentType.headerValueForValidation - let validateAcceptHeader: Expression = .try( - .identifier("converter").dot("validateAcceptIfPresent") - .call([ - .init(label: nil, expression: .literal(contentTypeHeaderValue)), - .init(label: "in", expression: .identifier("request").dot("headerFields")), - ]) - ) - codeBlocks.append(.expression(validateAcceptHeader)) + )].compactMap { $0 } - let contentType = typedContent.content.contentType - let switchContentCases: [SwitchCaseDescription] = [ - .init( + if !typedContents.isEmpty { + let switchContentCases: [SwitchCaseDescription] = typedContents.map { typedContent in + + var caseCodeBlocks: [CodeBlock] = [] + + let contentTypeHeaderValue = typedContent.content.contentType.headerValueForValidation + let validateAcceptHeader: Expression = .try( + .identifier("converter").dot("validateAcceptIfPresent") + .call([ + .init(label: nil, expression: .literal(contentTypeHeaderValue)), + .init(label: "in", expression: .identifier("request").dot("headerFields")), + ]) + ) + caseCodeBlocks.append(.expression(validateAcceptHeader)) + + let contentType = typedContent.content.contentType + let assignBodyExpr: Expression = .assignment( + left: .identifier("response").dot("body"), + right: .try( + .identifier("converter") + .dot("setResponseBodyAs\(contentType.codingStrategy.runtimeName)") + .call([ + .init(label: nil, expression: .identifier("value")), + .init( + label: "headerFields", + expression: .inOut( + .identifier("response").dot("headerFields") + ) + ), + .init( + label: "contentType", + expression: .literal(contentType.headerValueForSending) + ), + ]) + ) + ) + caseCodeBlocks.append(.expression(assignBodyExpr)) + + return .init( kind: .case(.dot(contentSwiftName(contentType)), ["value"]), - body: [ - .expression( - .return( - .dot("init") - .call([ - .init( - label: "value", - expression: .identifier("value") - ), - .init( - label: "contentType", - expression: .literal(contentType.headerValueForSending) - ), - ]) - ) - ) - ] + body: caseCodeBlocks ) - ] + } - let transformExpr: Expression = .closureInvocation( - argumentNames: ["wrapped"], - body: [ - .expression( - .switch( - switchedExpression: .identifier("wrapped"), - cases: switchContentCases - ) + codeBlocks.append( + .expression( + .switch( + switchedExpression: .identifier("value").dot("body"), + cases: switchContentCases ) - ] - ) - let assignBodyExpr: Expression = .assignment( - left: .identifier("response").dot("body"), - right: .try( - .identifier("converter") - .dot("setResponseBodyAs\(contentType.codingStrategy.runtimeName)") - .call([ - .init(label: nil, expression: .identifier("value").dot("body")), - .init( - label: "headerFields", - expression: .inOut( - .identifier("response").dot("headerFields") - ) - ), - .init(label: "transforming", expression: transformExpr), - ]) ) ) - codeBlocks.append(.expression(assignBodyExpr)) } let returnExpr: Expression = .return( diff --git a/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Client.swift b/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Client.swift index d8cd6c51..7a534d9a 100644 --- a/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Client.swift +++ b/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Client.swift @@ -151,19 +151,14 @@ public struct Client: APIProtocol { name: "accept", value: "application/json" ) - request.body = try converter.setRequiredRequestBodyAsJSON( - input.body, - headerFields: &request.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch input.body { + case let .json(value): + request.body = try converter.setRequiredRequestBodyAsJSON( + value, + headerFields: &request.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return request }, deserializer: { response in @@ -255,19 +250,15 @@ public struct Client: APIProtocol { name: "accept", value: "application/json" ) - request.body = try converter.setOptionalRequestBodyAsJSON( - input.body, - headerFields: &request.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch input.body { + case .none: request.body = nil + case let .json(value): + request.body = try converter.setOptionalRequestBodyAsJSON( + value, + headerFields: &request.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return request }, deserializer: { response in @@ -315,16 +306,14 @@ public struct Client: APIProtocol { name: "accept", value: "application/octet-stream, application/json, text/plain" ) - request.body = try converter.setRequiredRequestBodyAsBinary( - input.body, - headerFields: &request.headerFields, - transforming: { wrapped in - switch wrapped { - case let .binary(value): - return .init(value: value, contentType: "application/octet-stream") - } - } - ) + switch input.body { + case let .binary(value): + request.body = try converter.setRequiredRequestBodyAsBinary( + value, + headerFields: &request.headerFields, + contentType: "application/octet-stream" + ) + } return request }, deserializer: { response in diff --git a/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Server.swift b/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Server.swift index fef0daf4..b242b973 100644 --- a/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Server.swift +++ b/Tests/OpenAPIGeneratorReferenceTests/Resources/ReferenceSources/Petstore/Server.swift @@ -126,45 +126,35 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol { name: "My-Tracing-Header", value: value.headers.My_Tracing_Header ) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response case let .`default`(statusCode, value): suppressUnusedWarning(value) var response: Response = .init(statusCode: statusCode) suppressMutabilityWarning(&response) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response } } @@ -220,23 +210,18 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol { name: "X-Extra-Arguments", value: value.headers.X_Extra_Arguments ) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response case let .badRequest(value): suppressUnusedWarning(value) @@ -247,23 +232,18 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol { name: "X-Reason", value: value.headers.X_Reason ) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response case let .undocumented(statusCode, _): return .init(statusCode: statusCode) } @@ -353,23 +333,18 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol { suppressUnusedWarning(value) var response: Response = .init(statusCode: 400) suppressMutabilityWarning(&response) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response case let .undocumented(statusCode, _): return .init(statusCode: statusCode) } @@ -423,58 +398,52 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol { suppressUnusedWarning(value) var response: Response = .init(statusCode: 200) suppressMutabilityWarning(&response) - try converter.validateAcceptIfPresent( - "application/octet-stream", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsBinary( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .binary(value): - return .init(value: value, contentType: "application/octet-stream") - } - } - ) + switch value.body { + case let .binary(value): + try converter.validateAcceptIfPresent( + "application/octet-stream", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsBinary( + value, + headerFields: &response.headerFields, + contentType: "application/octet-stream" + ) + } return response case let .preconditionFailed(value): suppressUnusedWarning(value) var response: Response = .init(statusCode: 412) suppressMutabilityWarning(&response) - try converter.validateAcceptIfPresent( - "application/json", - in: request.headerFields - ) - response.body = try converter.setResponseBodyAsJSON( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .json(value): - return .init( - value: value, - contentType: "application/json; charset=utf-8" - ) - } - } - ) + switch value.body { + case let .json(value): + try converter.validateAcceptIfPresent( + "application/json", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsJSON( + value, + headerFields: &response.headerFields, + contentType: "application/json; charset=utf-8" + ) + } return response case let .internalServerError(value): suppressUnusedWarning(value) var response: Response = .init(statusCode: 500) suppressMutabilityWarning(&response) - try converter.validateAcceptIfPresent("text/plain", in: request.headerFields) - response.body = try converter.setResponseBodyAsText( - value.body, - headerFields: &response.headerFields, - transforming: { wrapped in - switch wrapped { - case let .text(value): - return .init(value: value, contentType: "text/plain") - } - } - ) + switch value.body { + case let .text(value): + try converter.validateAcceptIfPresent( + "text/plain", + in: request.headerFields + ) + response.body = try converter.setResponseBodyAsText( + value, + headerFields: &response.headerFields, + contentType: "text/plain" + ) + } return response case let .undocumented(statusCode, _): return .init(statusCode: statusCode) } From 9f41abdbf4d0232bd9bc5c328f02025d516e30d2 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Mon, 31 Jul 2023 17:50:45 +0200 Subject: [PATCH 2/3] Bump the runtime dependency to 0.1.6 --- Package.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index 8256a45b..11cdeb7a 100644 --- a/Package.swift +++ b/Package.swift @@ -78,7 +78,7 @@ let package = Package( ), // Tests-only: Runtime library linked by generated code - .package(url: "https://github.com/apple/swift-openapi-runtime", .upToNextMinor(from: "0.1.3")), + .package(url: "https://github.com/apple/swift-openapi-runtime", .upToNextMinor(from: "0.1.6")), // Build and preview docs .package(url: "https://github.com/apple/swift-docc-plugin", from: "1.0.0"), From c56d2ecb0c68708d60bded6855e2c96d6c0df97a Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Mon, 31 Jul 2023 18:47:02 +0200 Subject: [PATCH 3/3] Fix soundness --- .../Responses/translateResponseOutcome.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift b/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift index 025001ea..829f7589 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift @@ -306,10 +306,13 @@ extension ServerFileTranslator { } codeBlocks.append(contentsOf: headerExprs.map { .expression($0) }) - let typedContents = [try bestSingleTypedContent( - typedResponse.response.content, - inParent: bodyTypeName - )].compactMap { $0 } + let typedContents = [ + try bestSingleTypedContent( + typedResponse.response.content, + inParent: bodyTypeName + ) + ] + .compactMap { $0 } if !typedContents.isEmpty { let switchContentCases: [SwitchCaseDescription] = typedContents.map { typedContent in