Skip to content

Commit

Permalink
[Generator] Choose the serialization method based on content type (#48)
Browse files Browse the repository at this point in the history
[Generator] Choose the serialization method based on content type

### Motivation

Fixes #43. Depends on apple/swift-openapi-runtime#12 landing first and tagging a release.

### Modifications

Builds on top of the changes to the runtime library.

### Result

We now always specify a coding strategy with a Swift type, leading to deterministic and understandable conversion logic.

### Test Plan

Updated unit and integration tests, added a `500` case to one of the operations to explicitly test the missing plain text response body.


Reviewed by: simonjbeaumont

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#48
  • Loading branch information
czechboy0 authored Jun 8, 2023
1 parent 7186fd5 commit 8b9c564
Show file tree
Hide file tree
Showing 26 changed files with 518 additions and 255 deletions.
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,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.0")),
.package(url: "https://github.com/apple/swift-openapi-runtime", .upToNextMinor(from: "0.1.1")),

// Build and preview docs
.package(url: "https://github.com/apple/swift-docc-plugin", from: "1.0.0"),
Expand Down
12 changes: 1 addition & 11 deletions Sources/_OpenAPIGeneratorCore/Extensions/Foundation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,7 @@ extension Data {
/// - Throws: When data is not valid UTF-8.
var swiftFormatted: Data {
get throws {
struct FormattingError: Error, LocalizedError, CustomStringConvertible {
var description: String {
"Invalid UTF-8 data"
}
var errorDescription: String? {
description
}
}
guard let string = String(data: self, encoding: .utf8) else {
throw FormattingError()
}
let string = String(decoding: self, as: UTF8.self)
return try Self(string.swiftFormatted.utf8)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct TextBasedRenderer: RendererProtocol {

/// Renders the specified Swift file.
func renderFile(_ description: FileDescription) -> Data {
renderedFile(description).data(using: .utf8)!
Data(renderedFile(description).utf8)
}

/// Renders the specified comment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,28 @@ extension ClientFileTranslator {
_ description: OperationDescription
) throws -> Expression {

let clientPathTemplate = try translatePathParameterInClient(
let (pathTemplate, pathParamsArrayExpr) = try translatePathParameterInClient(
description: description
)
let pathDecl: Declaration = .variable(
kind: .let,
left: "path",
right: .try(
.identifier("converter")
.dot("renderedRequestPath")
.call([
.init(label: "template", expression: .literal(pathTemplate)),
.init(label: "parameters", expression: pathParamsArrayExpr),
])
)
)
let requestDecl: Declaration = .variable(
kind: .var,
left: "request",
type: TypeName.request.fullyQualifiedSwiftName,
right: .dot("init")
.call([
.init(label: "path", expression: .literal(clientPathTemplate)),
.init(label: "path", expression: .identifier("path")),
.init(label: "method", expression: .dot(description.httpMethodLowercased)),
])
)
Expand Down Expand Up @@ -65,9 +77,12 @@ extension ClientFileTranslator {
.map(\.headerValueForValidation)
.joined(separator: ", ")
let addAcceptHeaderExpr: Expression = .try(
.identifier("converter").dot("headerFieldAdd")
.identifier("converter").dot("setHeaderFieldAsText")
.call([
.init(label: "in", expression: .inOut(.identifier("request").dot("headerFields"))),
.init(
label: "in",
expression: .inOut(.identifier("request").dot("headerFields"))
),
.init(label: "name", expression: "accept"),
.init(label: "value", expression: .literal(acceptValue)),
])
Expand All @@ -91,6 +106,7 @@ extension ClientFileTranslator {
"input"
],
body: [
.declaration(pathDecl),
.declaration(requestDecl),
.expression(requestDecl.suppressMutabilityWarningExpr),
] + requestExprs.map { .expression($0) } + [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,19 @@ enum Constants {
]
}

/// Constants related to the coding strategy.
enum CodingStrategy {

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

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

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

/// Constants related to types used in many components.
enum Global {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftOpenAPIGenerator open source project
//
// Copyright (c) 2023 Apple Inc. and the SwiftOpenAPIGenerator project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

/// Describes the underlying coding strategy.
enum CodingStrategy: String, Equatable, Hashable, Sendable {

/// A strategy using JSONEncoder/JSONDecoder.
case json

/// A strategy using LosslessStringConvertible.
case text

/// A strategy that passes through the data unmodified.
case binary

/// The name of the coding strategy in the runtime library.
var runtimeName: String {
switch self {
case .json:
return Constants.CodingStrategy.json
case .text:
return Constants.CodingStrategy.text
case .binary:
return Constants.CodingStrategy.binary
}
}
}
12 changes: 12 additions & 0 deletions Sources/_OpenAPIGeneratorCore/Translator/Content/ContentType.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ enum ContentType: Hashable {
}
}

/// The coding strategy appropriate for this content type.
var codingStrategy: CodingStrategy {
switch self {
case .json:
return .json
case .text:
return .text
case .binary:
return .binary
}
}

/// A Boolean value that indicates whether the content type
/// is a type of JSON.
var isJSON: Bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,23 +232,28 @@ extension OperationDescription {
}

/// Returns a string that contains the template to be generated for
/// the client that fills in path parameters.
/// the client that fills in path parameters, and an array expression
/// with the parameter values.
///
/// For example, `/cats/\(input.catId)`.
var templatedPathForClient: String {
/// For example, `/cats/{}` and `[input.catId]`.
var templatedPathForClient: (String, Expression) {
get throws {
let path = self.path.rawValue
let pathParameters = try allResolvedParameters.filter { $0.location == .path }
guard !pathParameters.isEmpty else {
return path
}
// replace "{foo}" with "\(input.foo)" for each parameter
return pathParameters.reduce(into: path) { partialResult, parameter in
// replace "{foo}" with "{}" for each parameter
let template = pathParameters.reduce(into: path) { partialResult, parameter in
partialResult = partialResult.replacingOccurrences(
of: "{\(parameter.name)}",
with: "\\(input.path.\(parameter.name.asSwiftSafeName))"
with: "{}"
)
}
let names: [Expression] =
pathParameters
.map { param in
.identifier("input.path.\(param.name.asSwiftSafeName)")
}
let arrayExpr: Expression = .literal(.array(names))
return (template, arrayExpr)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ struct TypedParameter {

/// The computed type usage.
var typeUsage: TypeUsage

/// The coding strategy appropriate for this parameter.
var codingStrategy: CodingStrategy
}

extension TypedParameter: CustomStringConvertible {
Expand Down Expand Up @@ -126,9 +129,11 @@ extension FileTranslator {
let foundIn = "\(locationTypeName.description)/\(parameter.name)"

let schema: Either<JSONReference<JSONSchema>, JSONSchema>
let codingStrategy: CodingStrategy
switch parameter.schemaOrContent {
case let .a(schemaContext):
schema = schemaContext.schema
codingStrategy = .text

// Check supported exploded/style types
let location = parameter.location
Expand Down Expand Up @@ -175,6 +180,11 @@ extension FileTranslator {
return nil
}
schema = typedContent.content.schema ?? .b(.fragment)
codingStrategy =
typedContent
.content
.contentType
.codingStrategy
}

// Check if the underlying schema is supported
Expand Down Expand Up @@ -207,7 +217,8 @@ extension FileTranslator {
return .init(
parameter: parameter,
schema: schema,
typeUsage: usage
typeUsage: usage,
codingStrategy: codingStrategy
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ extension TypesFileTranslator {
extension ClientFileTranslator {

/// Returns a templated string that includes all path parameters in
/// the specified operation.
/// the specified operation, and an expression of an array literal
/// with all those parameters.
/// - Parameter description: The OpenAPI operation.
func translatePathParameterInClient(
description: OperationDescription
) throws -> String {
) throws -> (String, Expression) {
try description.templatedPathForClient
}

Expand All @@ -115,10 +116,10 @@ extension ClientFileTranslator {
let containerExpr: Expression
switch parameter.location {
case .header:
methodPrefix = "headerField"
methodPrefix = "HeaderField"
containerExpr = .identifier(requestVariableName).dot("headerFields")
case .query:
methodPrefix = "query"
methodPrefix = "QueryItem"
containerExpr = .identifier(requestVariableName)
default:
diagnostics.emitUnsupported(
Expand All @@ -129,20 +130,22 @@ extension ClientFileTranslator {
}
return .try(
.identifier("converter")
.dot("\(methodPrefix)Add")
.call([
.init(
label: "in",
expression: .inOut(containerExpr)
),
.init(label: "name", expression: .literal(parameter.name)),
.init(
label: "value",
expression: .identifier(inputVariableName)
.dot(parameter.location.shortVariableName)
.dot(parameter.variableName)
),
])
.dot("set\(methodPrefix)As\(parameter.codingStrategy.runtimeName)")
.call(
[
.init(
label: "in",
expression: .inOut(containerExpr)
),
.init(label: "name", expression: .literal(parameter.name)),
.init(
label: "value",
expression: .identifier(inputVariableName)
.dot(parameter.location.shortVariableName)
.dot(parameter.variableName)
),
]
)
)
}
}
Expand All @@ -160,17 +163,21 @@ extension ServerFileTranslator {
.typeUsage
.fullyQualifiedNonOptionalSwiftName

func methodName(_ parameterLocationName: String, _ requiresOptionality: Bool = true) -> String {
let optionality: String
if requiresOptionality {
optionality = parameter.required ? "Required" : "Optional"
} else {
optionality = ""
}
return "get\(optionality)\(parameterLocationName)As\(typedParameter.codingStrategy.runtimeName)"
}

let convertExpr: Expression
switch parameter.location {
case .path:
let methodName: String
if parameter.required {
methodName = "pathGetRequired"
} else {
methodName = "pathGetOptional"
}
convertExpr = .try(
.identifier("converter").dot(methodName)
.identifier("converter").dot(methodName("PathParameter", false))
.call([
.init(label: "in", expression: .identifier("metadata").dot("pathParameters")),
.init(label: "name", expression: .literal(parameter.name)),
Expand All @@ -181,14 +188,8 @@ extension ServerFileTranslator {
])
)
case .query:
let methodName: String
if parameter.required {
methodName = "queryGetRequired"
} else {
methodName = "queryGetOptional"
}
convertExpr = .try(
.identifier("converter").dot(methodName)
.identifier("converter").dot(methodName("QueryItem"))
.call([
.init(label: "in", expression: .identifier("metadata").dot("queryParameters")),
.init(label: "name", expression: .literal(parameter.name)),
Expand All @@ -199,15 +200,9 @@ extension ServerFileTranslator {
])
)
case .header:
let methodName: String
if parameter.required {
methodName = "headerFieldGetRequired"
} else {
methodName = "headerFieldGetOptional"
}
convertExpr = .try(
.identifier("converter")
.dot(methodName)
.dot(methodName("HeaderField"))
.call([
.init(label: "in", expression: .identifier("request").dot("headerFields")),
.init(label: "name", expression: .literal(parameter.name)),
Expand Down
Loading

0 comments on commit 8b9c564

Please sign in to comment.