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: overhaul protocol test generation to use actual client #767

Merged
merged 22 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ let package = Package(
),
.target(
name: "SmithyTestUtil",
dependencies: ["ClientRuntime"]
dependencies: ["ClientRuntime", "SmithyHTTPAPI"]
),
.target(
name: "SmithyIdentity",
Expand Down
21 changes: 21 additions & 0 deletions Sources/ClientRuntime/Endpoints/StaticEndpointResolver.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//
// Copyright Amazon.com Inc. or its affiliates.
// All Rights Reserved.
//
// SPDX-License-Identifier: Apache-2.0
//

import struct SmithyHTTPAPI.Endpoint

public struct StaticEndpointResolver<Params: EndpointsRequestContextProviding> {

private let endpoint: SmithyHTTPAPI.Endpoint

public init(endpoint: SmithyHTTPAPI.Endpoint) {
self.endpoint = endpoint
}

public func resolve(params: Params) throws -> SmithyHTTPAPI.Endpoint {
return endpoint
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import enum Smithy.ClientError
import Foundation
import SmithyHTTPAPI
import SmithyHTTPAuthAPI
import struct Smithy.AttributeKey

public struct SignerMiddleware<OperationStackOutput>: Middleware {
public let id: String = "SignerMiddleware"
Expand Down Expand Up @@ -69,10 +70,17 @@ extension SignerMiddleware: ApplySigner {
)
}

// Check if CRT should be provided a pre-computed Sha256 SignedBodyValue
var updatedSigningProperties = signingProperties
let sha256: String? = attributes.get(key: AttributeKey(name: "X-Amz-Content-Sha256"))
if let bodyValue = sha256 {
updatedSigningProperties.set(key: AttributeKey(name: "SignedBodyValue"), value: bodyValue)
}

let signed = try await signer.signRequest(
requestBuilder: request.toBuilder(),
identity: identity,
signingProperties: signingProperties
signingProperties: updatedSigningProperties
)

// The saved signature is used to sign event stream messages if needed.
Expand Down
1 change: 1 addition & 0 deletions Sources/SmithyHTTPAuth/CRTAdapters.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ extension AWSSignedBodyValue {
case .streamingSha256Events: return .streamingSha256Events
case .streamingSha256PayloadTrailer: return .streamingSha256PayloadTrailer
case .streamingUnsignedPayloadTrailer: return .streamingUnSignedPayloadTrailer
case .precomputed(let value): return .precomputedSha256(value)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ public enum AWSSignedBodyValue {
case streamingSha256Events
case streamingSha256PayloadTrailer
case streamingUnsignedPayloadTrailer
case precomputed(String)
}
31 changes: 31 additions & 0 deletions Sources/SmithyTestUtil/ProtocolTestClient.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

import protocol SmithyHTTPAPI.HTTPClient
import class SmithyHTTPAPI.SdkHttpRequest
import class SmithyHTTPAPI.HttpResponse
import ClientRuntime

public class ProtocolTestClient {
public init() {}
}

public enum TestCheckError: Error {
case actual(SdkHttpRequest)
}

extension ProtocolTestClient: HTTPClient {
public func send(request: SdkHttpRequest) async throws -> HttpResponse {
throw TestCheckError.actual(request)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

throwing an error with the request will allow tests to do checks against the result

}
}

public class ProtocolTestIdempotencyTokenGenerator: ClientRuntime.IdempotencyTokenGenerator {
public init() {}

public func generateToken() -> String {
return "00000000-0000-4000-8000-000000000000"
}
}
50 changes: 47 additions & 3 deletions Sources/SmithyTestUtil/RequestTestUtil/HttpRequestTestBase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,32 @@ open class HttpRequestTestBase: XCTestCase {
try await assertEqualHttpBody?(expected.body, actual.body)
}

private func checkStreamLengthEqualityIfUnseekableActualStreamHasBeenRead(
expected: ByteStream,
actual: ByteStream,
file: StaticString = #filePath,
line: UInt = #line
) -> Bool {
switch actual {
case .stream(let actualStream):
if actualStream.position == actualStream.length, !actualStream.isSeekable {
switch expected {
case .stream(let expectedStream):
if actualStream.length == expectedStream.length {
return true // Streams are considered equal
} else {
XCTFail("Actual stream is a different size than expected!", file: file, line: line)
}
default:
break // This is only applicable to streams
}
}
default:
break // This is only applicable to streams
}
return false
}

public func genericAssertEqualHttpBodyData(
expected: ByteStream,
actual: ByteStream,
Expand All @@ -237,6 +263,17 @@ open class HttpRequestTestBase: XCTestCase {
) async throws {
let expectedData = try await expected.readData()
let actualData = try await actual.readData()

// Unseekable streams may have already been read by Signer middleware and cannot be read again
// Compare stream lengths if ByteStream is a .stream and actualData is nil
if checkStreamLengthEqualityIfUnseekableActualStreamHasBeenRead(
expected: expected, actual: actual, file: file, line: line
) {
// Stream lengths were checked and comparing data will result in failure due to above conditions
return
}
Comment on lines +267 to +274
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an edge case where the SignerMiddleware has already read an unseekable stream. In which case by the time all middlewares are executed we cannot re-read the data. In that case let's check if the actual and expected stream lengths are the same.

Note: This comparison should only fail if the actualStream.position == actualStream.length and !actualStream.isSeekable


// Compare the data
compareData(contentType: contentType, expectedData, actualData, file: file, line: line)
}

Expand Down Expand Up @@ -304,10 +341,17 @@ open class HttpRequestTestBase: XCTestCase {
return
}

let actualValue = actual.values(for: header.name)?.joined(separator: ", ")
let actualValue = actual.values(for: header.name)?
.joined(separator: ", ")
.components(separatedBy: .whitespaces)
.joined()
XCTAssertNotNil(actualValue, file: file, line: line)

let expectedValue = header.value.joined(separator: ", ")
let expectedValue = header.value
.joined(separator: ", ")
.components(separatedBy: .whitespaces)
.joined()

XCTAssertEqual(actualValue, expectedValue, file: file, line: line)
}
}
Expand Down Expand Up @@ -371,7 +415,7 @@ open class HttpRequestTestBase: XCTestCase {
}

for expectedQueryItem in expectedQueryItems {
let values = actualQueryItems.filter {$0.name == expectedQueryItem.name}.map { $0.value}
let values = actualQueryItems.filter {$0.name == expectedQueryItem.name}.map { $0.value }
XCTAssertNotNil(
values,
"expected query parameter \(expectedQueryItem.name); no values found",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class SwiftDependency(
val CRT = SwiftDependency(
"AwsCommonRuntimeKit",
null,
"0.30.0",
"0.31.0",
"https://github.com/awslabs/aws-crt-swift",
"",
"aws-crt-swift",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class EndpointResolverGenerator(
renderResolverProtocol(it)
it.write("")
renderResolver(it, ruleSet)
renderStaticResolver(it)
val inputSymbol = Symbol.builder().name("SdkHttpRequestBuilder").build()
val outputSymbol = Symbol.builder().name("OperationStackOutput").build()
val outputErrorSymbol = Symbol.builder().name("OperationStackError").build()
Expand Down Expand Up @@ -62,4 +63,14 @@ class EndpointResolverGenerator(
writer.write("")
writer.write("extension DefaultEndpointResolver: EndpointResolver {}")
}

private fun renderStaticResolver(writer: SwiftWriter) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Static Resolver will be used by tests but are also part of SRA to allow users to bypass endpoint resolution and provide their own endpoint

writer.write("")
writer.write(
"typealias StaticEndpointResolver = \$N<EndpointParams>",
ClientRuntimeTypes.Core.StaticEndpointResolver,
)
writer.write("")
writer.write("extension StaticEndpointResolver: EndpointResolver {}")
}
}
Loading
Loading