Skip to content

Commit 581320d

Browse files
author
Ed Paulosky
authored
Fixes XML encoding issues (#488)
* Verifies serialized data matches expected data in tests * Handle xmlName trait - Adds code to properly set the xml root key if the object has a custom xml name set - Sets the xmlName trait for operation shapes since we modify operation shape names * No longer encodes floats and doubles as strings * Updates tests * ktlintformat * Addresses PR feedback * Adds explicit FoundationXML import for linux * Checks Any objects for equality * Cleans up code and adds docs
1 parent 0cca10a commit 581320d

File tree

17 files changed

+295
-34
lines changed

17 files changed

+295
-34
lines changed

‎Packages/ClientRuntime/Sources/Networking/Http/Middlewares/SerializableBodyMiddleware.swift‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ public struct SerializableBodyMiddleware<OperationStackInput: Encodable,
99
OperationStackOutput: HttpResponseBinding>: Middleware {
1010
public let id: Swift.String = "\(String(describing: OperationStackInput.self))BodyMiddleware"
1111

12-
public init() {}
12+
let xmlName: String?
13+
14+
public init(xmlName: String? = nil) {
15+
self.xmlName = xmlName
16+
}
1317

1418
public func handle<H>(context: Context,
1519
input: SerializeStepInput<OperationStackInput>,
@@ -20,7 +24,12 @@ public struct SerializableBodyMiddleware<OperationStackInput: Encodable,
2024
Self.Context == H.Context {
2125
do {
2226
let encoder = context.getEncoder()
23-
let data = try encoder.encode(input.operationInput)
27+
let data: Data
28+
if let xmlName = xmlName, let xmlEncoder = encoder as? XMLEncoder {
29+
data = try xmlEncoder.encode(input.operationInput, withRootKey: xmlName)
30+
} else {
31+
data = try encoder.encode(input.operationInput)
32+
}
2433
let body = HttpBody.data(data)
2534
input.builder.withBody(body)
2635
} catch let err {

‎Packages/SmithyTestUtil/Sources/JSONComparator.swift‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import Foundation
9+
10+
public struct JSONComparator {
11+
/// Returns true if the JSON documents, for the corresponding data objects, are equal.
12+
/// - Parameters:
13+
/// - dataA: The first data object to compare to the second data object.
14+
/// - dataB: The second data object to compare to the first data object.
15+
/// - Returns: Returns true if the JSON documents, for the corresponding data objects, are equal.
16+
public static func jsonData(_ dataA: Data, isEqualTo dataB: Data) throws -> Bool {
17+
let jsonDictA = try JSONSerialization.jsonObject(with: dataA)
18+
let jsonDictB = try JSONSerialization.jsonObject(with: dataB)
19+
return anyValuesAreEqual(jsonDictA, jsonDictB)
20+
}
21+
}
22+
23+
fileprivate func anyDictsAreEqual(_ lhs: [String: Any], _ rhs: [String: Any]) -> Bool {
24+
guard lhs.keys == rhs.keys else { return false }
25+
for key in lhs.keys {
26+
if !anyValuesAreEqual(lhs[key], rhs[key]) {
27+
return false
28+
}
29+
}
30+
return true
31+
}
32+
33+
fileprivate func anyArraysAreEqual(_ lhs: [Any], _ rhs: [Any]) -> Bool {
34+
guard lhs.count == rhs.count else { return false }
35+
for i in 0..<lhs.count {
36+
if !anyValuesAreEqual(lhs[i], rhs[i]) {
37+
return false
38+
}
39+
}
40+
return true
41+
}
42+
43+
fileprivate func anyValuesAreEqual(_ lhs: Any?, _ rhs: Any?) -> Bool {
44+
if lhs == nil && rhs == nil { return true }
45+
guard let lhs = lhs, let rhs = rhs else { return false }
46+
if let lhsDict = lhs as? [String: Any], let rhsDict = rhs as? [String: Any] {
47+
return anyDictsAreEqual(lhsDict, rhsDict)
48+
} else if let lhsArray = lhs as? [Any], let rhsArray = rhs as? [Any] {
49+
return anyArraysAreEqual(lhsArray, rhsArray)
50+
} else {
51+
return type(of: lhs) == type(of: rhs) && "\(lhs)" == "\(rhs)"
52+
}
53+
}

‎Packages/SmithyTestUtil/Sources/RequestTestUtil/HttpRequestTestBase.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ open class HttpRequestTestBase: XCTestCase {
231231
public func genericAssertEqualHttpBodyData(
232232
_ expected: HttpBody,
233233
_ actual: HttpBody,
234+
_ encoder: Any,
234235
_ callback: (Data, Data) -> Void,
235236
file: StaticString = #filePath,
236237
line: UInt = #line
@@ -244,6 +245,11 @@ open class HttpRequestTestBase: XCTestCase {
244245
return
245246
}
246247
if shouldCompareData(expectedData, actualData) {
248+
if encoder is XMLEncoder {
249+
XCTAssertXMLDataEqual(actualData!, expectedData!, file: file, line: line)
250+
} else if encoder is JSONEncoder {
251+
XCTAssertJSONDataEqual(actualData!, expectedData!, file: file, line: line)
252+
}
247253
callback(expectedData!, actualData!)
248254
}
249255
}

‎Packages/SmithyTestUtil/Sources/XCTAssertions.swift‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import Foundation
9+
import XCTest
10+
11+
public func XCTAssertJSONDataEqual(
12+
_ expression1: @autoclosure () throws -> Data,
13+
_ expression2: @autoclosure () throws -> Data,
14+
_ message: @autoclosure () -> String = "",
15+
file: StaticString = #filePath,
16+
line: UInt = #line
17+
) {
18+
do {
19+
let data1 = try expression1()
20+
let data2 = try expression2()
21+
guard data1 != data2 else { return }
22+
XCTAssertTrue(
23+
try JSONComparator.jsonData(data1, isEqualTo: data2),
24+
message(),
25+
file: file,
26+
line: line
27+
)
28+
} catch {
29+
XCTFail("Failed to evaluate JSON with error: \(error)", file: file, line: line)
30+
}
31+
}
32+
33+
public func XCTAssertXMLDataEqual(
34+
_ expression1: @autoclosure () throws -> Data,
35+
_ expression2: @autoclosure () throws -> Data,
36+
_ message: @autoclosure () -> String = "",
37+
file: StaticString = #filePath,
38+
line: UInt = #line
39+
) {
40+
do {
41+
let data1 = try expression1()
42+
let data2 = try expression2()
43+
guard data1 != data2 else { return }
44+
XCTAssertTrue(XMLComparator.xmlData(data1, isEqualTo: data2), message(), file: file, line: line)
45+
} catch {
46+
XCTFail("Failed to evaluate XML with error: \(error)", file: file, line: line)
47+
}
48+
}

‎Packages/SmithyTestUtil/Sources/XMLComparator.swift‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import Foundation
9+
#if canImport(FoundationXML)
10+
// As of Swift 5.1, the Foundation module on Linux only has the same set of dependencies as the Swift standard library itself
11+
// Therefore, we need to explicitly import FoundationXML on linux.
12+
// The preferred way to do this, is to check if FoundationXML can be imported.
13+
// https://github.com/apple/swift-corelibs-foundation/blob/main/Docs/ReleaseNotes_Swift5.md
14+
import FoundationXML
15+
#endif
16+
17+
public struct XMLComparator {
18+
/// Returns true if the XML documents, for the corresponding data objects, are equal.
19+
/// Order of elements within the document do not affect equality.
20+
/// - Parameters:
21+
/// - dataA: The first data object to compare to the second data object.
22+
/// - dataB: The second data object to compare to the first data object.
23+
/// - Returns: Returns true if the XML documents, for the corresponding data objects, are equal.
24+
public static func xmlData(_ dataA: Data, isEqualTo dataB: Data) -> Bool {
25+
let rootA = XMLConverter.xmlTree(with: dataA)
26+
let rootB = XMLConverter.xmlTree(with: dataB)
27+
return rootA == rootB
28+
}
29+
}
30+
31+
private struct XMLElement: Hashable {
32+
var name: String?
33+
var attributes: [String : String]?
34+
var string: String?
35+
var elements: Set<XMLElement> = []
36+
}
37+
38+
private class XMLConverter: NSObject {
39+
/// Keeps track of the value since `foundCharacters` can be called multiple times for the same element
40+
private var valueBuffer = ""
41+
private var stack: [XMLElement] = []
42+
43+
static func xmlTree(with data: Data) -> XMLElement {
44+
let converter = XMLConverter()
45+
converter.stack.append(XMLElement())
46+
47+
let parser = XMLParser(data: data)
48+
parser.delegate = converter
49+
parser.parse()
50+
51+
return converter.stack.first!
52+
}
53+
}
54+
55+
extension XMLConverter: XMLParserDelegate {
56+
func parser(
57+
_ parser: XMLParser,
58+
didStartElement elementName: String,
59+
namespaceURI: String?,
60+
qualifiedName qName: String?,
61+
attributes attributeDict: [String : String] = [:]
62+
) {
63+
let parent = stack.last!
64+
let element = XMLElement(
65+
name: elementName,
66+
attributes: attributeDict
67+
)
68+
69+
stack.append(element)
70+
}
71+
72+
func parser(_ parser: XMLParser, foundCharacters string: String) {
73+
let trimmedString = string.trimmingCharacters(in: .whitespacesAndNewlines)
74+
valueBuffer.append(trimmedString)
75+
}
76+
77+
func parser(
78+
_ parser: XMLParser, didEndElement
79+
elementName: String,
80+
namespaceURI: String?,
81+
qualifiedName qName: String?
82+
) {
83+
var element = stack.popLast()!
84+
var parent = stack.last!
85+
86+
element.string = valueBuffer
87+
88+
var elements = parent.elements
89+
elements.insert(element)
90+
parent.elements = elements
91+
92+
stack[stack.endIndex - 1] = parent
93+
valueBuffer = ""
94+
}
95+
}

‎Packages/SmithyTestUtil/Tests/RequestTestUtilTests/HttpRequestTestBaseTests.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase {
214214
self.assertEqual(expected, actual, { (expectedHttpBody, actualHttpBody) -> Void in
215215
XCTAssertNotNil(actualHttpBody, "The actual HttpBody is nil")
216216
XCTAssertNotNil(expectedHttpBody, "The expected HttpBody is nil")
217-
self.genericAssertEqualHttpBodyData(expectedHttpBody!, actualHttpBody!) { (expectedData, actualData) in
217+
self.genericAssertEqualHttpBodyData(expectedHttpBody!, actualHttpBody!, JSONEncoder()) { (expectedData, actualData) in
218218
do {
219219
let decoder = JSONDecoder()
220220
let expectedObj = try decoder.decode(SayHelloInputBody.self, from: expectedData)

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/HttpProtocolUnitTestRequestGenerator.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
147147
writer.write("XCTAssertNotNil(expectedHttpBody, \"The expected HttpBody is nil\")")
148148
val expectedData = "expectedData"
149149
val actualData = "actualData"
150-
writer.openBlock("self.genericAssertEqualHttpBodyData(expectedHttpBody!, actualHttpBody!) { $expectedData, $actualData in ", "}") {
150+
writer.openBlock("self.genericAssertEqualHttpBodyData(expectedHttpBody!, actualHttpBody!, encoder) { $expectedData, $actualData in ", "}") {
151151
val httpPayloadShape = inputShape.members().firstOrNull { it.hasTrait(HttpPayloadTrait::class.java) }
152152

153153
httpPayloadShape?.let {
@@ -204,6 +204,11 @@ open class HttpProtocolUnitTestRequestGenerator protected constructor(builder: B
204204
writer.write("}")
205205
}
206206

207+
private fun renderDataComparison(writer: SwiftWriter, expectedData: String, actualData: String) {
208+
val assertionMethod = "XCTAssertJSONDataEqual"
209+
writer.write("\$L(\$L, \$L, \"Some error message\")", assertionMethod, actualData, expectedData)
210+
}
211+
207212
protected open fun renderAssertions(test: HttpRequestTestCase, outputShape: Shape) {
208213
val members = outputShape.members().filterNot { it.hasTrait(HttpQueryTrait::class.java) }
209214
.filterNot { it.hasTrait(HttpHeaderTrait::class.java) }

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/OperationInputBodyMiddleware.kt‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ package software.amazon.smithy.swift.codegen.integration.middlewares
33
import software.amazon.smithy.codegen.core.SymbolProvider
44
import software.amazon.smithy.model.Model
55
import software.amazon.smithy.model.shapes.OperationShape
6+
import software.amazon.smithy.model.traits.XmlNameTrait
67
import software.amazon.smithy.swift.codegen.ClientRuntimeTypes
78
import software.amazon.smithy.swift.codegen.SwiftWriter
89
import software.amazon.smithy.swift.codegen.integration.middlewares.handlers.MiddlewareShapeUtils
910
import software.amazon.smithy.swift.codegen.middleware.MiddlewarePosition
1011
import software.amazon.smithy.swift.codegen.middleware.MiddlewareRenderable
1112
import software.amazon.smithy.swift.codegen.middleware.MiddlewareStep
13+
import software.amazon.smithy.swift.codegen.model.getTrait
1214

1315
class OperationInputBodyMiddleware(
1416
val model: Model,
@@ -27,15 +29,38 @@ class OperationInputBodyMiddleware(
2729
op: OperationShape,
2830
operationStackName: String,
2931
) {
32+
val inputShape = MiddlewareShapeUtils.inputShape(model, op)
3033
val inputShapeName = MiddlewareShapeUtils.inputSymbol(symbolProvider, model, op).name
3134
val outputShapeName = MiddlewareShapeUtils.outputSymbol(symbolProvider, model, op).name
35+
val xmlName = inputShape.getTrait<XmlNameTrait>()?.value
36+
3237
if (alwaysSendBody) {
33-
writer.write("$operationStackName.${middlewareStep.stringValue()}.intercept(position: ${position.stringValue()}, middleware: \$N<$inputShapeName, $outputShapeName>())", ClientRuntimeTypes.Middleware.SerializableBodyMiddleware)
38+
if (xmlName != null) {
39+
writer.write(
40+
"\$L.\$L.intercept(position: \$L, middleware: \$N<\$L, \$L>(xmlName: \"\$L\"))",
41+
operationStackName, middlewareStep.stringValue(), position.stringValue(), ClientRuntimeTypes.Middleware.SerializableBodyMiddleware, inputShapeName, outputShapeName, xmlName
42+
)
43+
} else {
44+
writer.write(
45+
"\$L.\$L.intercept(position: \$L, middleware: \$N<\$L, \$L>())",
46+
operationStackName, middlewareStep.stringValue(), position.stringValue(), ClientRuntimeTypes.Middleware.SerializableBodyMiddleware, inputShapeName, outputShapeName
47+
)
48+
}
3449
} else if (MiddlewareShapeUtils.hasHttpBody(model, op)) {
3550
if (MiddlewareShapeUtils.bodyIsHttpPayload(model, op)) {
3651
writer.write("$operationStackName.${middlewareStep.stringValue()}.intercept(position: ${position.stringValue()}, middleware: ${inputShapeName}BodyMiddleware())")
3752
} else {
38-
writer.write("$operationStackName.${middlewareStep.stringValue()}.intercept(position: ${position.stringValue()}, middleware: \$N<$inputShapeName, $outputShapeName>())", ClientRuntimeTypes.Middleware.SerializableBodyMiddleware)
53+
if (xmlName != null) {
54+
writer.write(
55+
"\$L.\$L.intercept(position: \$L, middleware: \$N<\$L, \$L>(xmlName: \"\$L\"))",
56+
operationStackName, middlewareStep.stringValue(), position.stringValue(), ClientRuntimeTypes.Middleware.SerializableBodyMiddleware, inputShapeName, outputShapeName, xmlName
57+
)
58+
} else {
59+
writer.write(
60+
"\$L.\$L.intercept(position: \$L, middleware: \$N<\$L, \$L>())",
61+
operationStackName, middlewareStep.stringValue(), position.stringValue(), ClientRuntimeTypes.Middleware.SerializableBodyMiddleware, inputShapeName, outputShapeName
62+
)
63+
}
3964
}
4065
}
4166
}

‎smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/handlers/HttpBodyMiddleware.kt‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55

66
package software.amazon.smithy.swift.codegen.integration.middlewares.handlers
77

8+
import software.amazon.smithy.aws.traits.protocols.RestXmlTrait
89
import software.amazon.smithy.codegen.core.CodegenException
910
import software.amazon.smithy.codegen.core.Symbol
1011
import software.amazon.smithy.model.knowledge.HttpBinding
1112
import software.amazon.smithy.model.shapes.OperationShape
1213
import software.amazon.smithy.model.shapes.ShapeType
1314
import software.amazon.smithy.model.traits.EnumTrait
1415
import software.amazon.smithy.model.traits.StreamingTrait
16+
import software.amazon.smithy.model.traits.XmlNameTrait
1517
import software.amazon.smithy.swift.codegen.ClientRuntimeTypes
1618
import software.amazon.smithy.swift.codegen.Middleware
1719
import software.amazon.smithy.swift.codegen.MiddlewareGenerator
@@ -21,6 +23,7 @@ import software.amazon.smithy.swift.codegen.integration.HttpBindingDescriptor
2123
import software.amazon.smithy.swift.codegen.integration.HttpBindingResolver
2224
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
2325
import software.amazon.smithy.swift.codegen.integration.steps.OperationSerializeStep
26+
import software.amazon.smithy.swift.codegen.model.getTrait
2427
import software.amazon.smithy.swift.codegen.model.hasTrait
2528

2629
class HttpBodyMiddleware(
@@ -107,7 +110,19 @@ class HttpBodyMiddleware(
107110
writer.openBlock("do {", "} catch let err {") {
108111
writer.write("let encoder = context.getEncoder()")
109112
writer.openBlock("if let $memberName = input.operationInput.$memberName {", "} else {") {
110-
writer.write("let $dataDeclaration = try encoder.encode(\$L)", memberName)
113+
114+
val xmlNameTrait = binding.member.getTrait<XmlNameTrait>() ?: target.getTrait<XmlNameTrait>()
115+
if (ctx.protocol == RestXmlTrait.ID && xmlNameTrait != null) {
116+
val xmlName = xmlNameTrait.value
117+
writer.write("let xmlEncoder = encoder as! XMLEncoder")
118+
writer.write(
119+
"let $dataDeclaration = try xmlEncoder.encode(\$L, withRootKey: \"\$L\")",
120+
memberName, xmlName
121+
)
122+
} else {
123+
writer.write("let $dataDeclaration = try encoder.encode(\$L)", memberName)
124+
}
125+
111126
renderEncodedBodyAddedToRequest(bodyDeclaration, dataDeclaration)
112127
}
113128
writer.indent()

0 commit comments

Comments
 (0)