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

feat!: XML response deserialization #1299

Merged
merged 40 commits into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3e78b74
Mostly working XML reader
jbelkins Jan 9, 2024
cf9bb15
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 9, 2024
569d5de
Code cleanup
jbelkins Jan 10, 2024
34a52d4
Revert generated code to main
jbelkins Jan 10, 2024
d7a592b
Revert to main branch models
jbelkins Jan 10, 2024
1b7e7b9
Fix integration tests
jbelkins Jan 11, 2024
2562b24
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 11, 2024
5bae0ee
Eliminate encoders/decoders where not used
jbelkins Jan 15, 2024
4f2b458
Delete duplicate tests
jbelkins Jan 16, 2024
a0aba18
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 16, 2024
0711195
Restore main
jbelkins Jan 16, 2024
4d327a1
Revert Sources/Services to main branch
jbelkins Jan 16, 2024
5a3cb57
Revert models
jbelkins Jan 16, 2024
4de9974
Revert Package.swift
jbelkins Jan 16, 2024
3ba40ad
Code cleanup
jbelkins Jan 16, 2024
ca01cb4
Fix codegen tests
jbelkins Jan 17, 2024
833360c
Fix tests
jbelkins Jan 18, 2024
d1304ad
fix lint
jbelkins Jan 18, 2024
b99378a
Merge branch 'main' into jbe/xml_reader
jbelkins Jan 18, 2024
fe2fb19
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 18, 2024
491ef6d
Merge branch 'jbe/xml_reader' of github.com:awslabs/aws-sdk-swift int…
jbelkins Jan 18, 2024
de4a4ec
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 18, 2024
d8beae9
Merge remote-tracking branch 'origin/main' into jbe/xml_reader
jbelkins Jan 22, 2024
5a25bbf
Code review fixes
jbelkins Jan 23, 2024
90ffa69
Merge branch 'main' into jbe/xml_reader
jbelkins Jan 27, 2024
e638413
Update RestXML error
jbelkins Jan 27, 2024
aec6f45
Fix error customizations
jbelkins Feb 5, 2024
837bb87
Fix codegen tests
jbelkins Feb 5, 2024
4f483bf
Fix swiftlint issue
jbelkins Feb 5, 2024
d8f3a81
Update EC2 error customization
jbelkins Feb 5, 2024
9d8e96d
Merge branch 'main' into jbe/xml_reader
jbelkins Feb 6, 2024
0168c60
Merge branch 'main' into jbe/xml_reader
jbelkins Feb 6, 2024
65bfcf4
Fix codegen, adopt closure changes
jbelkins Feb 7, 2024
1341b98
Merge branch 'main' into jbe/xml_reader
jbelkins Feb 7, 2024
f2a2899
Fix getOrNull usage
jbelkins Feb 7, 2024
bf13ee8
Revert manifest
jbelkins Feb 7, 2024
48bd940
Merge branch 'main' into jbe/xml_reader
jbelkins Feb 9, 2024
372806a
Delete unused CodingKeys
jbelkins Feb 9, 2024
1abe22d
Update RestXMLError doc comment
jbelkins Feb 9, 2024
de92168
Merge branch 'main' into jbe/xml_reader
jbelkins Feb 9, 2024
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 @@ -22,10 +22,10 @@ final class S3StreamTests: S3XCTestCase {
case .data(let dataOrNil):
let data = try XCTUnwrap(dataOrNil)
let actual = String(data: data, encoding: .utf8)
XCTAssertEqual(actual, expected)
XCTAssertEqual(expected, actual)
case .stream(let stream):
let actual = String(data: try await stream.readToEndAsync()!, encoding: .utf8)
XCTAssertEqual(actual, expected)
let actual = String(data: try await stream.readToEndAsync() ?? Data(), encoding: .utf8)
XCTAssertEqual(expected, actual)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactoring

case .noStream:
XCTFail("Expected stream")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class S3XCTestCase: XCTestCase {
let data = try XCTUnwrap(dataOrNil)
return String(data: data, encoding: .utf8)
case .stream(let stream):
return String(data: try await stream.readToEndAsync()!, encoding: .utf8)
return String(data: try await stream.readToEndAsync() ?? Data(), encoding: .utf8)
case .noStream:
return nil
}
Expand Down
17 changes: 9 additions & 8 deletions Sources/Core/AWSClientRuntime/Errors/RestXMLError+AWS.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import ClientRuntime
import class SmithyXML.Reader

extension RestXMLError {
/// Makes a `RestXMLError` from the provided `HttpResponse`.
sichanyoo marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -15,13 +16,13 @@ extension RestXMLError {
/// - Returns: A`RestXMLError` instance with an error code of "NotFound" if the response body is empty and the status code is 404. Otherwise returns a `RestXMLError` by calling ``RestXMLError.init(httpResponse: HttpResponse)``.
///
/// - Throws: An error if it fails to decode the response body.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert this error customization to use a XML reader

public static func makeError(from response: HttpResponse) async throws -> RestXMLError {
response.statusCodeIsNotFoundAndBodyIsEmpty
? .makeNotFoundError(requestID: response.requestId)
: try await .init(httpResponse: response)
}

static func makeNotFoundError(requestID: String?) -> RestXMLError {
return RestXMLError(errorCode: "NotFound", requestId: requestID)
public static func makeError(
from httpResponse: HttpResponse,
responseReader: SmithyXML.Reader,
noErrorWrapping: Bool
) async throws -> RestXMLError {
return httpResponse.statusCodeIsNotFoundAndBodyIsEmpty
? .init(code: "NotFound", message: "404 Not Found", requestID: httpResponse.requestId)
: try .init(responseReader: responseReader, noErrorWrapping: noErrorWrapping)
}
}
19 changes: 13 additions & 6 deletions Sources/Core/AWSClientRuntime/Protocols/Ec2Query/Ec2Error.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@
// SPDX-License-Identifier: Apache-2.0
//

public struct Ec2Error: Decodable {
public let code: String
public let message: String
import SmithyReadWrite
import SmithyXML

enum CodingKeys: String, CodingKey {
case code = "Code"
case message = "Message"
public struct Ec2Error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to use Reader to get error fields instead of Decoder

public var code: String?
public var message: String?

static var readingClosure: ReadingClosure<Ec2Error, Reader> {
return { reader in
var value = Ec2Error()
value.code = try reader["Code"].readIfPresent()
value.message = try reader["Message"].readIfPresent()
return value
}
}
}
15 changes: 11 additions & 4 deletions Sources/Core/AWSClientRuntime/Protocols/Ec2Query/Ec2Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@
// SPDX-License-Identifier: Apache-2.0
//

public struct Ec2Errors: Decodable {
public let error: Ec2Error
import SmithyReadWrite
import SmithyXML

enum CodingKeys: String, CodingKey {
case error = "Error"
public struct Ec2Errors {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as with previous type

sichanyoo marked this conversation as resolved.
Show resolved Hide resolved
public var error: Ec2Error?

static var readingClosure: ReadingClosure<Ec2Errors, Reader> {
return { reader in
var value = Ec2Errors()
value.error = try reader["Error"].readIfPresent(readingClosure: Ec2Error.readingClosure)
return value
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,19 @@
// SPDX-License-Identifier: Apache-2.0
//

import ClientRuntime
import class ClientRuntime.HttpResponse
import class SmithyXML.Reader
import var ClientRuntime.responseDocumentBinding

public struct Ec2QueryError {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert this type to use a XML reader

public let errorCode: String?
public let requestId: String?
public let message: String?
public var errorCode: String?
public var requestId: String?
public var message: String?

public init(httpResponse: HttpResponse) async throws {
guard let data = try await httpResponse.body.readData() else {
errorCode = nil
requestId = nil
message = nil
return
}
let decoded: Ec2Response = try XMLDecoder().decode(responseBody: data)
self.errorCode = decoded.errors.error.code
self.message = decoded.errors.error.message
self.requestId = decoded.requestId
let response = try await Ec2Response.httpBinding(httpResponse, responseDocumentBinding)
self.errorCode = response.errors?.error?.code
self.message = response.errors?.error?.message
self.requestId = response.requestId
}
}
26 changes: 15 additions & 11 deletions Sources/Core/AWSClientRuntime/Protocols/Ec2Query/Ec2Response.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,27 @@
// SPDX-License-Identifier: Apache-2.0
//

public struct Ec2Response: Decodable {
public let errors: Ec2Errors
public let requestId: String
import SmithyReadWrite
import SmithyXML
@testable import ClientRuntime

public struct Ec2Response {
public var errors: Ec2Errors?
public var requestId: String?

enum CodingKeys: String, CodingKey {
case errors = "Errors"
case requestId = "RequestId"
case requestID = "RequestID"
}
sichanyoo marked this conversation as resolved.
Show resolved Hide resolved

public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
self.errors = try container.decode(Ec2Errors.self, forKey: .errors)

// Attempt to decode the requestId with the key "RequestID"
// if that is not present, then fallback to the key "RequestId"
self.requestId = try container.decodeIfPresent(String.self, forKey: .requestID)
?? container.decode(String.self, forKey: .requestId)
public static var httpBinding: HTTPResponseOutputBinding<Ec2Response, Reader> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses a closure binding instead of the Decodable protocol

return { httpResponse, responseDocumentBinding in
let reader = try await responseDocumentBinding(httpResponse)
var value = Ec2Response()
value.errors = try reader["Errors"].readIfPresent(readingClosure: Ec2Errors.readingClosure)
value.requestId = try reader["RequestId"].readIfPresent() ?? reader["RequestID"].readIfPresent()
return value
}
}
}
48 changes: 24 additions & 24 deletions Sources/Core/AWSClientRuntime/Protocols/RestXML/RestXMLError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,37 @@
// SPDX-License-Identifier: Apache-2.0
//

import ClientRuntime
import class SmithyXML.Reader

public struct RestXMLError {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracts common fields from Rest XML errors. Can read from "unwrapped" errors as well.

public let errorCode: String?
public let requestId: String?
public let code: String
public let message: String?
public let requestID: String?

public init(httpResponse: HttpResponse) async throws {
guard let data = try await httpResponse.body.readData() else {
errorCode = nil
requestId = nil
message = nil
return
}
do {
let decoded: ErrorResponseContainer<RestXMLErrorPayload>
decoded = try XMLDecoder().decode(responseBody: data)
self.errorCode = decoded.error.errorCode
self.message = decoded.error.message
self.requestId = decoded.requestId
} catch {
let decoded: RestXMLErrorNoErrorWrappingPayload = try XMLDecoder().decode(responseBody: data)
self.errorCode = decoded.errorCode
self.message = decoded.message
self.requestId = decoded.requestId
public static func errorBodyReader(responseReader: Reader, noErrorWrapping: Bool) -> Reader {
noErrorWrapping ? responseReader : responseReader["Error"]
}

public init(responseReader: Reader, noErrorWrapping: Bool) throws {
let reader = Self.errorBodyReader(responseReader: responseReader, noErrorWrapping: noErrorWrapping)
let code: String? = try reader["Code"].readIfPresent()
let message: String? = try reader["Message"].readIfPresent()
let requestID: String? = try responseReader["RequestId"].readIfPresent()
guard let code else {
throw RestXMLDecodeError.missingRequiredData
}
self.code = code
self.message = message
self.requestID = requestID
}

public init(errorCode: String? = nil, requestId: String? = nil, message: String? = nil) {
self.errorCode = errorCode
self.requestId = requestId
public init(code: String, message: String?, requestID: String?) {
self.code = code
self.message = message
self.requestID = requestID
}
}

public enum RestXMLDecodeError: Error {
case missingRequiredData
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ final class AWSMessageDecoderStreamTests: XCTestCase {
let bufferedStream = BufferedStream(data: validMessageDataWithAllHeaders + validMessageDataEmptyPayload + validMessageDataNoHeaders,
isClosed: true)
let messageDecoder = AWSEventStream.AWSMessageDecoder()
let sut = EventStream.DefaultMessageDecoderStream<TestEvent>(stream: bufferedStream,
messageDecoder: messageDecoder,
responseDecoder: JSONDecoder())
let sut = EventStream.DefaultMessageDecoderStream<TestEvent>(
stream: bufferedStream,
messageDecoder: messageDecoder,
unmarshalClosure: jsonUnmarshalClosure(responseDecoder: JSONDecoder())
)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted to use closures for event streams.


var events: [TestEvent] = []
for try await evnt in sut {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Sha256TreeHashMiddlewareTests: XCTestCase {
var stack = OperationStack<MockStreamInput, MockOutput>(id: "TreeHashMiddlewareTestStack")
stack.serializeStep.intercept(position: .before, middleware: MockSerializeStreamMiddleware())
let mockHttpResponse = HttpResponse(body: .noStream, statusCode: .accepted)
let mockOutput = try MockOutput(httpResponse: mockHttpResponse, decoder: nil)
let mockOutput = MockOutput()
let output = OperationOutput<MockOutput>(httpResponse: mockHttpResponse, output: mockOutput)
stack.finalizeStep.intercept(position: .after, middleware: Sha256TreeHashMiddleware<MockOutput>())
_ = try await stack.handleMiddleware(context: context, input: streamInput, next: MockHandler(handleCallback: { context, input in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
import ClientRuntime
import SmithyTestUtil
import XCTest
import SmithyXML
@testable import AWSClientRuntime

class Ec2ErrorRequestIdTests: XCTestCase {

func testEc2ResponseDecodesRequestID() throws {
let data = """
func testEc2ResponseDecodesRequestID() async throws {
let data = Data("""
<Ec2Response>
<Errors>
<Error>
Expand All @@ -24,13 +25,14 @@ class Ec2ErrorRequestIdTests: XCTestCase {
</Errors>
<RequestID>abcdefg12345</RequestID>
</Ec2Response>
""".data(using: .utf8)!
let response = try XMLDecoder().decode(Ec2Response.self, from: data)
""".utf8)
let httpResponse = HttpResponse(body: .data(data), statusCode: .ok)
let response = try await responseClosure(Ec2Response.httpBinding, responseDocumentBinding)(httpResponse)
XCTAssertEqual(response.requestId, "abcdefg12345")
}

func testEc2ResponseDecodesRequestId() throws {
let data = """
func testEc2ResponseDecodesRequestId() async throws {
let data = Data("""
<Ec2Response>
<Errors>
<Error>
Expand All @@ -40,34 +42,37 @@ class Ec2ErrorRequestIdTests: XCTestCase {
</Errors>
<RequestId>abcdefg12345</RequestId>
</Ec2Response>
""".data(using: .utf8)!
let response = try XMLDecoder().decode(Ec2Response.self, from: data)
""".utf8)
let httpResponse = HttpResponse(body: .data(data), statusCode: .ok)
let response = try await responseClosure(Ec2Response.httpBinding, responseDocumentBinding)(httpResponse)
XCTAssertEqual(response.requestId, "abcdefg12345")
}

func testEc2NarrowedResponseDecodesRequestID() throws {
let data = """
func testEc2NarrowedResponseDecodesRequestID() async throws {
let data = Data("""
<Ec2NarrowedResponse>
<Errors>
<Error>Sample Error</Error>
</Errors>
<RequestID>abcdefg12345</RequestID>
</Ec2NarrowedResponse>
""".data(using: .utf8)!
let response = try XMLDecoder().decode(Ec2NarrowedResponse<String>.self, from: data)
""".utf8)
let httpResponse = HttpResponse(body: .data(data), statusCode: .ok)
let response = try await responseClosure(Ec2Response.httpBinding, responseDocumentBinding)(httpResponse)
XCTAssertEqual(response.requestId, "abcdefg12345")
}

func testEc2NarrowResponseDecodesRequestId() throws {
let data = """
func testEc2NarrowResponseDecodesRequestId() async throws {
let data = Data("""
<Ec2Response>
<Errors>
<Error>Sample Error</Error>
</Errors>
<RequestId>abcdefg12345</RequestId>
</Ec2Response>
""".data(using: .utf8)!
let response = try XMLDecoder().decode(Ec2NarrowedResponse<String>.self, from: data)
""".utf8)
let httpResponse = HttpResponse(body: .data(data), statusCode: .ok)
let response = try await responseClosure(Ec2Response.httpBinding, responseDocumentBinding)(httpResponse)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests for EC2 errors above are converted to use closures instead of decoders to create the errors.

The tests for the several following deleted files were removed because they appear to have been copied from protocol tests. I suspect they were used in the early days of establishing protocol tests in this project.

The same tests still exist in the protocol tests suite, and they are being removed here as redundant.

XCTAssertEqual(response.requestId, "abcdefg12345")
}
}

This file was deleted.

Loading
Loading