Skip to content

Commit

Permalink
[Generator] Include partial errors in oneOf/anyOf decoding errors (#350)
Browse files Browse the repository at this point in the history 
[Generator] Include partial errors in oneOf/anyOf decoding errors

### Motivation

Fixes #275.

Depends on apple/swift-openapi-runtime#66.

### Modifications

Adapt the generator to emit code that collects individual errors during oneOf/anyOf decoding and includes them in the final error.

### Result

Easier debugging of oneOf/anyOf decoding issues.

### Test Plan

Adapted tests and verified that the errors look better now and include the partial errors.


Reviewed by: simonjbeaumont

Builds:
     ✔︎ pull request validation (5.10) - Build finished. 
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (compatibility test) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#350
  • Loading branch information
@czechboy0
czechboy0 authored Oct 30, 2023
1 parent 6057654 commit 9e62a21
Show file tree
Hide file tree
Showing 10 changed files with 521 additions and 269 deletions.
2 changes: 1 addition & 1 deletion Package.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ let package = Package(
// Tests-only: Runtime library linked by generated code, and also
// helps keep the runtime library new enough to work with the generated
// code.
.package(url: "https://github.com/apple/swift-openapi-runtime", .upToNextMinor(from: "0.3.5")),
.package(url: "https://github.com/apple/swift-openapi-runtime", .upToNextMinor(from: "0.3.6")),

// Build and preview docs
.package(url: "https://github.com/apple/swift-docc-plugin", from: "1.0.0"),
Expand Down
10 changes: 10 additions & 0 deletions Sources/_OpenAPIGeneratorCore/Layers/StructuredSwiftRepresentation.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,16 @@ extension Expression {
/// - Returns: A new expression with the `yield` keyword placed before the expression.
static func `yield`(_ expression: Expression) -> Self { .unaryKeyword(kind: .yield, expression: expression) }

/// Returns a new expression that puts the provided code blocks into
/// a do/catch block.
/// - Parameter:
/// - doStatement: The code blocks in the `do` statement body.
/// - catchBody: The code blocks in the `catch` statement.
/// - Returns: The expression.
static func `do`(_ doStatement: [CodeBlock], catchBody: [CodeBlock]? = nil) -> Self {
.doStatement(.init(doStatement: doStatement, catchBody: catchBody))
}

/// Returns a new value binding used in enums with associated values.
///
/// For example: `let foo(bar)`.
Expand Down
96 changes: 75 additions & 21 deletions Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateCodable.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,15 @@ extension FileTranslator {
func translateStructBlueprintAnyOfDecoder(properties: [(property: PropertyBlueprint, isKeyValuePair: Bool)])
-> Declaration
{
let assignExprs: [Expression] = properties.map { (property, isKeyValuePair) in
let errorArrayDecl: Declaration = .createErrorArrayDecl()
let assignBlocks: [CodeBlock] = properties.map { (property, isKeyValuePair) in
let decoderExpr: Expression =
isKeyValuePair ? .initFromDecoderExpr() : .decodeFromSingleValueContainerExpr()
return .assignment(left: .identifierPattern(property.swiftSafeName), right: .optionalTry(decoderExpr))
let assignExpr: Expression = .assignment(
left: .identifierPattern(property.swiftSafeName),
right: .try(decoderExpr)
)
return .expression(assignExpr.wrapInDoCatchAppendArrayExpr())
}
let atLeastOneNotNilCheckExpr: Expression = .try(
.identifierType(TypeName.decodingError).dot("verifyAtLeastOneSchemaIsNotNil")
Expand All @@ -220,9 +225,12 @@ extension FileTranslator {
expression: .literal(.array(properties.map { .identifierPattern($0.property.swiftSafeName) }))
), .init(label: "type", expression: .identifierPattern("Self").dot("self")),
.init(label: "codingPath", expression: .identifierPattern("decoder").dot("codingPath")),
.init(label: "errors", expression: .identifierPattern("errors")),
])
)
return decoderInitializer(body: assignExprs.map { .expression($0) } + [.expression(atLeastOneNotNilCheckExpr)])
return decoderInitializer(
body: [.declaration(errorArrayDecl)] + assignBlocks + [.expression(atLeastOneNotNilCheckExpr)]
)
}

/// Returns a declaration of an anyOf encoder implementation.
Expand Down Expand Up @@ -254,37 +262,51 @@ extension FileTranslator {
/// - Parameter cases: The names of the cases to be decoded.
/// - Returns: A `Declaration` representing the `OneOf` decoder implementation.
func translateOneOfWithoutDiscriminatorDecoder(cases: [(name: String, isKeyValuePair: Bool)]) -> Declaration {
let errorArrayDecl: Declaration = .createErrorArrayDecl()
let assignExprs: [Expression] = cases.map { (caseName, isKeyValuePair) in
let decoderExpr: Expression =
isKeyValuePair ? .initFromDecoderExpr() : .decodeFromSingleValueContainerExpr()
return .doStatement(
.init(
doStatement: [
.expression(
.assignment(
left: .identifierPattern("self"),
right: .dot(caseName).call([.init(label: nil, expression: .try(decoderExpr))])
)
), .expression(.return()),
],
catchBody: []
)
)
let body: [CodeBlock] = [
.expression(
.assignment(
left: .identifierPattern("self"),
right: .dot(caseName).call([.init(label: nil, expression: .try(decoderExpr))])
)
), .expression(.return()),
]
return body.wrapInDoCatchAppendArrayExpr()
}

let otherExprs: [CodeBlock] = [.expression(translateOneOfDecoderThrowOnUnknownExpr())]
return decoderInitializer(body: (assignExprs).map { .expression($0) } + otherExprs)
let otherExprs: [CodeBlock] = [.expression(translateOneOfDecoderThrowOnNoCaseDecodedExpr())]
return decoderInitializer(
body: [.declaration(errorArrayDecl)] + (assignExprs).map { .expression($0) } + otherExprs
)
}

/// Returns an expression that throws an error when a oneOf discriminator
/// failed to match any known cases.
func translateOneOfDecoderThrowOnUnknownExpr(discriminatorSwiftName: String) -> Expression {
.unaryKeyword(
kind: .throw,
expression: .identifierType(TypeName.decodingError).dot("unknownOneOfDiscriminator")
.call([
.init(
label: "discriminatorKey",
expression: .identifierPattern(Constants.Codable.codingKeysName).dot(discriminatorSwiftName)
), .init(label: "discriminatorValue", expression: .identifierPattern("discriminator")),
.init(label: "codingPath", expression: .identifierPattern("decoder").dot("codingPath")),
])
)
}
/// Returns an expression that throws an error when a oneOf failed
/// to match any documented cases.
func translateOneOfDecoderThrowOnUnknownExpr() -> Expression {
func translateOneOfDecoderThrowOnNoCaseDecodedExpr() -> Expression {
.unaryKeyword(
kind: .throw,
expression: .identifierType(TypeName.decodingError).dot("failedToDecodeOneOfSchema")
.call([
.init(label: "type", expression: .identifierPattern("Self").dot("self")),
.init(label: "codingPath", expression: .identifierPattern("decoder").dot("codingPath")),
.init(label: "errors", expression: .identifierPattern("errors")),
])
)
}
Expand All @@ -311,7 +333,9 @@ extension FileTranslator {
]
)
}
let otherExprs: [CodeBlock] = [.expression(translateOneOfDecoderThrowOnUnknownExpr())]
let otherExprs: [CodeBlock] = [
.expression(translateOneOfDecoderThrowOnUnknownExpr(discriminatorSwiftName: discriminatorName))
]
let body: [CodeBlock] = [
.declaration(.decoderContainerOfKeysVar()),
.declaration(
Expand Down Expand Up @@ -408,6 +432,31 @@ fileprivate extension Expression {
static func decodeFromSingleValueContainerExpr() -> Expression {
.identifierPattern("decoder").dot("decodeFromSingleValueContainer").call([])
}
/// Returns a new expression that wraps the provided expression in
/// a do/catch block where the error is appended to an array.
///
/// Assumes the existence of an "errors" variable in the current scope.
/// - Returns: The expression.
func wrapInDoCatchAppendArrayExpr() -> Expression { [CodeBlock.expression(self)].wrapInDoCatchAppendArrayExpr() }
}

fileprivate extension Array where Element == CodeBlock {
/// Returns a new expression that wraps the provided code blocks in
/// a do/catch block where the error is appended to an array.
///
/// Assumes the existence of an "errors" variable in the current scope.
/// - Returns: The expression.
func wrapInDoCatchAppendArrayExpr() -> Expression {
.do(
self,
catchBody: [
.expression(
.identifierPattern("errors").dot("append")
.call([.init(label: nil, expression: .identifierPattern("error"))])
)
]
)
}
}

fileprivate extension Declaration {
Expand All @@ -424,6 +473,11 @@ fileprivate extension Declaration {
)
)
}
/// Creates a new declaration that creates a local array of errors.
/// - Returns: The declaration.
static func createErrorArrayDecl() -> Declaration {
.variable(kind: .var, left: "errors", type: .array(.any(.member("Error"))), right: .literal(.array([])))
}
}

fileprivate extension FileTranslator {
Expand Down
80 changes: 37 additions & 43 deletions Sources/_OpenAPIGeneratorCore/Translator/RequestBody/translateRequestBody.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,24 @@ extension ServerFileTranslator {
.declaration(.variable(kind: .let, left: bodyVariableName, type: .init(requestBody.typeUsage)))
)

func makeIfBranch(typedContent: TypedSchemaContent, isFirstBranch: Bool) -> IfBranch {
let isMatchingContentTypeExpr: Expression = .identifierPattern("converter").dot("isMatchingContentType")
.call([
.init(label: "received", expression: .identifierPattern("contentType")),
.init(
label: "expectedRaw",
expression: .literal(typedContent.content.contentType.headerValueForValidation)
),
])
let condition: Expression
if isFirstBranch {
condition = .binaryOperation(
left: .binaryOperation(
left: .identifierPattern("contentType"),
operation: .equals,
right: .literal(.nil)
),
operation: .booleanOr,
right: isMatchingContentTypeExpr
)
} else {
condition = isMatchingContentTypeExpr
}
let typedContents = requestBody.contents
let contentTypeOptions = typedContents.map { typedContent in
typedContent.content.contentType.headerValueForValidation
}
let chosenContentTypeDecl: Declaration = .variable(
kind: .let,
left: "chosenContentType",
right: .try(
.identifierPattern("converter").dot("bestContentType")
.call([
.init(label: "received", expression: .identifierPattern("contentType")),
.init(label: "options", expression: .literal(.array(contentTypeOptions.map { .literal($0) }))),
])
)
)
codeBlocks.append(.declaration(chosenContentTypeDecl))

func makeCase(typedContent: TypedSchemaContent) -> SwitchCaseDescription {
let contentTypeUsage = typedContent.resolvedTypeUsage
let content = typedContent.content
let contentType = content.contentType
Expand Down Expand Up @@ -264,35 +259,34 @@ extension ServerFileTranslator {
} else {
bodyExpr = .try(.await(converterExpr))
}
let bodyAssignExpr: Expression = .assignment(left: .identifierPattern("body"), right: bodyExpr)
return .init(
condition: .try(condition),
body: [.expression(.assignment(left: .identifierPattern("body"), right: bodyExpr))]
kind: .case(.literal(typedContent.content.contentType.headerValueForValidation)),
body: [.expression(bodyAssignExpr)]
)
}

let typedContents = requestBody.contents

let primaryIfBranch = makeIfBranch(typedContent: typedContents[0], isFirstBranch: true)
let elseIfBranches = typedContents.dropFirst()
.map { typedContent in makeIfBranch(typedContent: typedContent, isFirstBranch: false) }

codeBlocks.append(
.expression(
.ifStatement(
ifBranch: primaryIfBranch,
elseIfBranches: elseIfBranches,
elseBody: [
let cases = typedContents.map(makeCase)
let switchExpr: Expression = .switch(
switchedExpression: .identifierPattern("chosenContentType"),
cases: cases + [
.init(
kind: .default,
body: [
.expression(
.unaryKeyword(
kind: .throw,
expression: .identifierPattern("converter").dot("makeUnexpectedContentTypeError")
.call([.init(label: "contentType", expression: .identifierPattern("contentType"))])
)
.identifierPattern("preconditionFailure")
.call([
.init(
label: nil,
expression: .literal("bestContentType chose an invalid content type.")
)
])
)
]
)
)
]
)
codeBlocks.append(.expression(switchExpr))
return codeBlocks
}
}
84 changes: 39 additions & 45 deletions Sources/_OpenAPIGeneratorCore/Translator/Responses/translateResponseOutcome.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,29 +191,26 @@ extension ClientFileTranslator {
let bodyDecl: Declaration = .variable(kind: .let, left: "body", type: .init(bodyTypeName))
codeBlocks.append(.declaration(bodyDecl))

func makeIfBranch(typedContent: TypedSchemaContent, isFirstBranch: Bool) -> IfBranch {
let isMatchingContentTypeExpr: Expression = .identifierPattern("converter").dot("isMatchingContentType")
.call([
.init(label: "received", expression: .identifierPattern("contentType")),
.init(
label: "expectedRaw",
expression: .literal(typedContent.content.contentType.headerValueForValidation)
),
])
let condition: Expression
if isFirstBranch {
condition = .binaryOperation(
left: .binaryOperation(
left: .identifierPattern("contentType"),
operation: .equals,
right: .literal(.nil)
),
operation: .booleanOr,
right: isMatchingContentTypeExpr
)
} else {
condition = isMatchingContentTypeExpr
}
let contentTypeOptions = typedContents.map { typedContent in
typedContent.content.contentType.headerValueForValidation
}
let chosenContentTypeDecl: Declaration = .variable(
kind: .let,
left: "chosenContentType",
right: .try(
.identifierPattern("converter").dot("bestContentType")
.call([
.init(label: "received", expression: .identifierPattern("contentType")),
.init(
label: "options",
expression: .literal(.array(contentTypeOptions.map { .literal($0) }))
),
])
)
)
codeBlocks.append(.declaration(chosenContentTypeDecl))

func makeCase(typedContent: TypedSchemaContent) -> SwitchCaseDescription {
let contentTypeUsage = typedContent.resolvedTypeUsage
let transformExpr: Expression = .closureInvocation(
argumentNames: ["value"],
Expand All @@ -238,36 +235,33 @@ extension ClientFileTranslator {
} else {
bodyExpr = .try(.await(converterExpr))
}
let bodyAssignExpr: Expression = .assignment(left: .identifierPattern("body"), right: bodyExpr)
return .init(
condition: .try(condition),
body: [.expression(.assignment(left: .identifierPattern("body"), right: bodyExpr))]
kind: .case(.literal(typedContent.content.contentType.headerValueForValidation)),
body: [.expression(bodyAssignExpr)]
)
}

let primaryIfBranch = makeIfBranch(typedContent: typedContents[0], isFirstBranch: true)
let elseIfBranches = typedContents.dropFirst()
.map { typedContent in makeIfBranch(typedContent: typedContent, isFirstBranch: false) }

codeBlocks.append(
.expression(
.ifStatement(
ifBranch: primaryIfBranch,
elseIfBranches: elseIfBranches,
elseBody: [
let cases = typedContents.map(makeCase)
let switchExpr: Expression = .switch(
switchedExpression: .identifierPattern("chosenContentType"),
cases: cases + [
.init(
kind: .default,
body: [
.expression(
.unaryKeyword(
kind: .throw,
expression: .identifierPattern("converter").dot("makeUnexpectedContentTypeError")
.call([
.init(label: "contentType", expression: .identifierPattern("contentType"))
])
)
.identifierPattern("preconditionFailure")
.call([
.init(
label: nil,
expression: .literal("bestContentType chose an invalid content type.")
)
])
)
]
)
)
]
)

codeBlocks.append(.expression(switchExpr))
bodyVarExpr = .identifierPattern("body")
} else {
bodyVarExpr = nil
Expand Down
Loading

0 comments on commit 9e62a21

Please sign in to comment.