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!: Closure-based reader-writer serde for JSON, FormURL #696

Merged
merged 58 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
3ee7e8d
Generated code builds
jbelkins Apr 5, 2024
0a6cbb5
Protocol tests now run with failures
jbelkins Apr 5, 2024
255be51
All readers/writers implemented, 74 test failures
jbelkins Apr 10, 2024
ef6afbc
Lint cleanup, codegen test fixes
jbelkins Apr 11, 2024
1b71f03
Handle sparse collections in response
jbelkins Apr 11, 2024
1042248
Error tests fixed
jbelkins Apr 12, 2024
e4958ba
Remove the detach() interface from Reader & Writer
jbelkins Apr 13, 2024
b6ea1d8
Fix protocol tests with non-JSON body, fix lint
jbelkins Apr 15, 2024
e2d41a6
Cleanup, fix codegen tests
jbelkins Apr 15, 2024
7bb92f5
Fix codegen tests
jbelkins Apr 15, 2024
9d8090f
Fix Swift tests
jbelkins Apr 15, 2024
c0be78e
Fix ktlint
jbelkins Apr 15, 2024
70eb9c7
Fix Swift tests
jbelkins Apr 15, 2024
9049fbf
Merge branch 'main' into jbe/readwrite
jbelkins Apr 15, 2024
7766c88
Add Int8, Int16 writing closures
jbelkins Apr 15, 2024
d8511a3
Add test dirs to swiftlint config
jbelkins Apr 16, 2024
ce56c31
Ignore union members without content
jbelkins Apr 16, 2024
a6f6a96
Fix codegen tests
jbelkins Apr 16, 2024
d8158b5
Merge branch 'main' into jbe/readwrite
jbelkins Apr 16, 2024
0d3ebf1
Fix test broken after merge
jbelkins Apr 16, 2024
0cb4b04
Fix Weather test project, rename & reorg serde files
jbelkins Apr 17, 2024
6ada476
Add WeatherSDK files
jbelkins Apr 17, 2024
7655987
Add protocol to base error, use base error to make unknown
jbelkins Apr 17, 2024
23248ea
Code cleanup
jbelkins Apr 17, 2024
19fb75d
Refactoring
jbelkins Apr 18, 2024
77ac55f
More refactoring and eliminating dead code
jbelkins Apr 18, 2024
8b4212f
ktlint fix, code cleanup
jbelkins Apr 18, 2024
efdabef
Fixes from code review
jbelkins Apr 18, 2024
54498c3
Add custom error to BaseError, eliminate unneeded closures
jbelkins Apr 18, 2024
73b1ae3
Fix ktlint
jbelkins Apr 18, 2024
3ec9818
Eliminated document closures
jbelkins Apr 19, 2024
74051e3
Fix tests
jbelkins Apr 19, 2024
ecd9c94
Merge branch 'main' into jbe/readwrite
jbelkins Apr 19, 2024
9b643b8
Fix writer format mismatch in marshal generation
jbelkins Apr 19, 2024
a76bed8
Eliminate Decodable method on enums
jbelkins Apr 20, 2024
a2f8612
Fix codegen tests
jbelkins Apr 20, 2024
975c2d6
Provide service error customization point
jbelkins Apr 22, 2024
a3caac2
Merge remote-tracking branch 'origin/main' into jbe/readwrite
jbelkins Apr 23, 2024
05ecd95
Fix ktlint
jbelkins Apr 23, 2024
b1d6f94
Add service error customization, adapt initial response events
jbelkins Apr 23, 2024
9a2a983
Fix codegen tests & ktlint
jbelkins Apr 23, 2024
7ca1ebf
Fix initial request & response methods
jbelkins Apr 24, 2024
70f18e1
Merge remote-tracking branch 'origin/main' into jbe/readwrite
jbelkins Apr 24, 2024
12f1ed1
Eliminate timestamp & ByteStream Codable conformance
jbelkins Apr 25, 2024
9ffbd39
Fix SmithyTestUtil
jbelkins Apr 25, 2024
8a4df3e
Merge branch 'main' into jbe/readwrite
jbelkins Apr 25, 2024
cf6b8ab
Merge branch 'main' into jbe/readwrite
jbelkins Apr 26, 2024
126195e
Remove Codable conformance from Enums & IntEnums, clean up enum defin…
jbelkins Apr 27, 2024
39e539b
Fix code review comments
jbelkins Apr 28, 2024
c8c6aa2
Merge branch 'main' into jbe/readwrite
jbelkins May 6, 2024
84721ae
Merge branch 'main' into jbe/readwrite
jbelkins May 6, 2024
6eef981
Merge remote-tracking branch 'origin/main' into jbe/readwrite
jbelkins May 9, 2024
05d6d33
Passes protocol tests after merge
jbelkins May 9, 2024
3bd83cc
Fix codegen tests after merge
jbelkins May 10, 2024
042b734
Regenerate WeatherSDK
jbelkins May 10, 2024
532977b
Fix Swift tests
jbelkins May 10, 2024
0a35bdf
Merge branch 'main' into jbe/readwrite
jbelkins May 10, 2024
3bd533a
Fix call to initial request message creation
jbelkins May 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions .swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ excluded:
- Tests/ClientRuntimeTests/*
- Tests/SmithyTestUtilTests/*
- Tests/SmithyXMLTests/*
- Tests/SmithyJSONTests/*
- Tests/SmithyFormURLTests/*
- Tests/SmithyTimestampsTests/*
- Tests/WeatherSDKTests/*
- smithy-swift-codegen-test/build/*
Expand Down
33 changes: 32 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ let package = Package(
.library(name: "ClientRuntime", targets: ["ClientRuntime"]),
.library(name: "SmithyReadWrite", targets: ["SmithyReadWrite"]),
.library(name: "SmithyXML", targets: ["SmithyXML"]),
.library(name: "SmithyJSON", targets: ["SmithyJSON"]),
.library(name: "SmithyFormURL", targets: ["SmithyFormURL"]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create new Swift modules for the JSON and FormURL readers/writers.

.library(name: "SmithyTestUtil", targets: ["SmithyTestUtil"]),
],
dependencies: [
Expand All @@ -42,14 +44,21 @@ let package = Package(
name: "ClientRuntime",
dependencies: [
"SmithyXML",
"SmithyJSON",
"SmithyFormURL",
.product(name: "AwsCommonRuntimeKit", package: "aws-crt-swift"),
.product(name: "Logging", package: "swift-log"),
],
resources: [
.copy("PrivacyInfo.xcprivacy")
]
),
.target(name: "SmithyReadWrite"),
.target(
name: "SmithyReadWrite",
dependencies: [
"SmithyTimestamps"
]
),
.target(
name: "SmithyXML",
dependencies: [
Expand All @@ -58,6 +67,20 @@ let package = Package(
libXML2DependencyOrNil
].compactMap { $0 }
),
.target(
name: "SmithyJSON",
dependencies: [
"SmithyReadWrite",
"SmithyTimestamps"
]
),
.target(
name: "SmithyFormURL",
dependencies: [
"SmithyReadWrite",
"SmithyTimestamps"
]
),
libXML2TargetOrNil,
.target(
name: "SmithyTimestamps"
Expand All @@ -74,6 +97,14 @@ let package = Package(
name: "SmithyXMLTests",
dependencies: ["SmithyXML", "ClientRuntime"]
),
.testTarget(
name: "SmithyJSONTests",
dependencies: ["SmithyJSON", "ClientRuntime"]
),
.testTarget(
name: "SmithyFormURLTests",
dependencies: ["SmithyFormURL", "ClientRuntime"]
),
.testTarget(
name: "SmithyTimestampsTests",
dependencies: ["SmithyTimestamps"]
Expand Down
12 changes: 0 additions & 12 deletions Sources/ClientRuntime/Config/DefaultSDKRuntimeConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,6 @@ public struct DefaultSDKRuntimeConfiguration<DefaultSDKRuntimeRetryStrategy: Ret
/// The name of the client this config configures.
public var clientName: String

/// The encoder to be used for encoding models.
///
/// If none is provided, a default encoder will be used.
public var encoder: RequestEncoder?

/// The decoder to be used for decoding models.
///
/// If none is provided, a default decoder will be used.
public var decoder: ResponseDecoder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Encoders/decoders are no longer used & are removed from config.

/// The HTTP client to be used for HTTP connections.
///
/// If none is provided, the AWS CRT HTTP client will be used.
Expand Down Expand Up @@ -70,8 +60,6 @@ public struct DefaultSDKRuntimeConfiguration<DefaultSDKRuntimeRetryStrategy: Ret
) throws {
self.serviceName = clientName
self.clientName = clientName
self.encoder = nil
self.decoder = nil
self.httpClientConfiguration = Self.defaultHttpClientConfiguration
self.httpClientEngine = Self.makeClient(httpClientConfiguration: self.httpClientConfiguration)
self.idempotencyTokenGenerator = Self.defaultIdempotencyTokenGenerator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import struct Foundation.Data

extension EventStream {
/// Stream adapter that encodes input into `Data` objects.
public class DefaultMessageEncoderStream<Event: MessageMarshallable>: MessageEncoderStream, Stream {
public class DefaultMessageEncoderStream<Event>: MessageEncoderStream, Stream {
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 MessageMarshallable protocol is removed in favor of marshal/unmarshal closures.

let stream: AsyncThrowingStream<Event, Error>
let messageEncoder: MessageEncoder
sichanyoo marked this conversation as resolved.
Show resolved Hide resolved
let messageSigner: MessageSigner
Expand Down
23 changes: 0 additions & 23 deletions Sources/ClientRuntime/EventStream/MessageMarshallable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,4 @@
// SPDX-License-Identifier: Apache-2.0
//

/// Marshals an event stream event into a `Message`.
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 MessageMarshallable protocol and the helper method are no longer needed since closures are used to marshal & unmarshal.

/// Codgegen generates a conformance to this protocol for each event stream event input.
public protocol MessageMarshallable {
/// Marshals a event stream event into a `Message`.
/// - Parameters:
/// - encoder: RequestEncoder to use to encode the event stream event.
/// Note: event type may contain nested types that need to be encoded
/// using the same encoder.
/// - Returns: The marshalled `Message`.
func marshall(encoder: RequestEncoder) throws -> EventStream.Message
}

public typealias MarshalClosure<T> = (T) throws -> (EventStream.Message)

/// Provides a `MarshalClosure` for event payloads that are Swift `Encodable`.
/// - Parameter requestEncoder: The Swift `Encoder` to be used for encoding this event payload.
/// - Returns: A `MarshalClosure` that uses the provided encoder to encode event payloads.
public func jsonMarshalClosure<T: MessageMarshallable>(
requestEncoder: RequestEncoder
) -> MarshalClosure<T> {
return { eventStream in
try eventStream.marshall(encoder: requestEncoder)
}
}
21 changes: 0 additions & 21 deletions Sources/ClientRuntime/EventStream/MessageUnmarshallable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,4 @@
// SPDX-License-Identifier: Apache-2.0
//

/// Unmarshals a `Message` into a event stream event.
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 MessageUnmarshallable protocol and the helper method are no longer needed since closures are used to marshal & unmarshal.

/// Codgegen generates a conformance to this protocol for each event stream event output.
public protocol MessageUnmarshallable {
/// Unmarshals a `Message` into a event stream event.
/// - Parameters:
/// - message: The message to unmarshal.
/// - decoder: ResponseDecoder to use to decode the event stream event.
/// Note: event type may contain nested types that need to be decoded
/// using the same decoder.
init(message: EventStream.Message, decoder: ResponseDecoder) throws
}

public typealias UnmarshalClosure<T> = (EventStream.Message) throws -> T

/// Provides an `UnmarshalClosure` for event payloads that are Swift `Decodable`.
/// - Parameter responseDecoder: The Swift `Decoder` to be used for decoding this event payload.
/// - Returns: An `UnmarshalClosure` that uses the provided decoder to decode event payloads.
public func jsonUnmarshalClosure<T: MessageUnmarshallable>(responseDecoder: ResponseDecoder) -> UnmarshalClosure<T> {
return { message in
try T(message: message, decoder: responseDecoder)
}
}
29 changes: 29 additions & 0 deletions Sources/ClientRuntime/Networking/BaseError/BaseError.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

public protocol BaseError {
var httpResponse: HttpResponse { get }
var code: String { get }
var message: String? { get }
var requestID: String? { get }

func customError() -> Error?
}

public extension BaseError {

/// Returns a custom error from the error response, if any.
///
/// By default, a `BaseError` returns no custom error unless
/// the implementation provides its own implementation of this method.
/// - Returns: Some custom `Error` or `nil` if none.
func customError() -> Error? { nil }
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This protocol provides a way to expose info common to all service errors. Each AWS protocol provides a custom type that conforms to this protocol. There is also a "test" base error used by the WeatherSDK.


public enum BaseErrorDecodeError: Error {
case missingRequiredData
}
38 changes: 0 additions & 38 deletions Sources/ClientRuntime/Networking/Http/HTTPResponseClosure.swift
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to SmithyReadWrite (and made generic to remove the dependency on HTTP).

This file was deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to SmithyReadWrite (and made generic to remove the dependency on HTTP).

This file was deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to SmithyReadWrite (and made generic to remove the dependency on HTTP).

This file was deleted.

22 changes: 0 additions & 22 deletions Sources/ClientRuntime/Networking/Http/HttpContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ public class HttpContext: MiddlewareContext {
return attributes.get(key: AttributeKeys.isChunkedEligibleStream)
}

public func getDecoder() -> ResponseDecoder {
Copy link
Contributor Author

@jbelkins jbelkins Apr 16, 2024

Choose a reason for hiding this comment

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

Encoders & decoders are no longer used, and removed from context objects.

return attributes.get(key: AttributeKeys.decoder)!
}

public func getEncoder() -> RequestEncoder {
return attributes.get(key: AttributeKeys.encoder)!
}

public func getExpiration() -> TimeInterval {
return attributes.get(key: AttributeKeys.expiration) ?? 0
}
Expand Down Expand Up @@ -178,18 +170,6 @@ public class HttpContextBuilder {
return self
}

@discardableResult
public func withDecoder(value: ResponseDecoder) -> HttpContextBuilder {
self.attributes.set(key: AttributeKeys.decoder, value: value)
return self
}

@discardableResult
public func withEncoder(value: RequestEncoder) -> HttpContextBuilder {
self.attributes.set(key: AttributeKeys.encoder, value: value)
return self
}

@discardableResult
public func withExpiration(value: TimeInterval) -> HttpContextBuilder {
self.attributes.set(key: AttributeKeys.expiration, value: value)
Expand Down Expand Up @@ -329,8 +309,6 @@ public enum AttributeKeys {
public static let authSchemeResolver = AttributeKey<AuthSchemeResolver>(name: "AuthSchemeResolver")
public static let authSchemes = AttributeKey<Attributes>(name: "AuthSchemes")
public static let bidirectionalStreaming = AttributeKey<Bool>(name: "BidirectionalStreaming")
public static let decoder = AttributeKey<ResponseDecoder>(name: "Decoder")
public static let encoder = AttributeKey<RequestEncoder>(name: "Encoder")
public static let flowType = AttributeKey<FlowType>(name: "FlowType")
public static let host = AttributeKey<String>(name: "Host")
public static let hostPrefix = AttributeKey<String>(name: "HostPrefix")
Expand Down
21 changes: 17 additions & 4 deletions Sources/ClientRuntime/Networking/Http/HttpResponse.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import protocol SmithyReadWrite.WireDataProviding
import AwsCommonRuntimeKit

public class HttpResponse: HttpUrlResponse {
Expand Down Expand Up @@ -32,6 +36,15 @@ extension HttpResponse: CustomDebugStringConvertible {
}
}

extension HttpResponse: WireDataProviding {

public func data() async throws -> Data {
let data = try await body.readData()
body = .data(data)
return data ?? Data()
}
}

extension ByteStream {
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 HttpResponse type is extended with a method to get the data out of it. This allows SmithyReadWrite to get data from an HTTP response without depending on the type.


// Convert the body stream to a ValidatingFileStream to check checksums
Expand Down
13 changes: 0 additions & 13 deletions Sources/ClientRuntime/Networking/Http/HttpResponseBinding.swift
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted since this supported encoder/decoder serde.

This file was deleted.

Loading
Loading