Skip to content

Commit 0fc3d48

Browse files
authored
fix: Fix Hashable compliance on Headers & Header types, disable tests that use httpbin.org (#548)
1 parent fc6d682 commit 0fc3d48

File tree

4 files changed

+89
-10
lines changed

4 files changed

+89
-10
lines changed

Sources/ClientRuntime/Networking/Http/Headers.swift

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import AwsCommonRuntimeKit
77

8-
public struct Headers: Hashable {
8+
public struct Headers {
99
public var headers: [Header] = []
1010

1111
/// Creates an empty instance.
@@ -172,6 +172,13 @@ extension Headers: Equatable {
172172
}
173173
}
174174

175+
extension Headers: Hashable {
176+
177+
public func hash(into hasher: inout Hasher) {
178+
hasher.combine(headers.sorted())
179+
}
180+
}
181+
175182
extension Array where Element == Header {
176183
/// Case-insensitively finds the index of an `Header` with the provided name, if it exists.
177184
func index(of name: String) -> Int? {
@@ -186,7 +193,7 @@ extension Array where Element == Header {
186193
}
187194
}
188195

189-
public struct Header: Hashable {
196+
public struct Header {
190197
public var name: String
191198
public var value: [String]
192199

@@ -207,6 +214,14 @@ extension Header: Equatable {
207214
}
208215
}
209216

217+
extension Header: Hashable {
218+
219+
public func hash(into hasher: inout Hasher) {
220+
hasher.combine(name)
221+
hasher.combine(value.sorted())
222+
}
223+
}
224+
210225
extension Header: Comparable {
211226
/// Compares two `Header` instances by name.
212227
/// - Parameters:

Tests/ClientRuntimeTests/ClientRuntimeTests/NetworkingTests/HttpHeadersTests.swift

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,62 @@ class HttpHeadersTests: XCTestCase {
6464

6565
XCTAssertEqual(httpHeaders.dictionary.count, 0)
6666
}
67+
68+
// MARK: - Equatable & Hashable implementations
69+
70+
func test_headers_equatableAndHashable_nonIdenticalHeaders() {
71+
var headersA = Headers()
72+
headersA.add(name: "A", values: ["X", "Y"])
73+
var headersB = Headers()
74+
headersB.add(name: "B", values: ["X", "Y"])
75+
76+
XCTAssertNotEqual(headersA, headersB)
77+
}
78+
79+
func test_headers_equatableAndHashable_identicalHeaders() {
80+
var headersA = Headers()
81+
headersA.add(name: "A", values: ["X", "Y"])
82+
headersA.add(name: "B", values: ["X", "Y"])
83+
var headersB = Headers()
84+
headersB.add(name: "A", values: ["X", "Y"])
85+
headersB.add(name: "B", values: ["X", "Y"])
86+
87+
XCTAssertEqual(headersA, headersB)
88+
XCTAssertEqual(headersA.hashValue, headersB.hashValue)
89+
}
90+
91+
func test_headers_equatableAndHashable_outOfOrderHeaders() {
92+
var headersA = Headers()
93+
headersA.add(name: "A", values: ["X", "Y"])
94+
headersA.add(name: "B", values: ["X", "Y"])
95+
var headersB = Headers()
96+
headersB.add(name: "B", values: ["X", "Y"])
97+
headersB.add(name: "A", values: ["X", "Y"])
98+
99+
XCTAssertEqual(headersA, headersB)
100+
XCTAssertEqual(headersA.hashValue, headersB.hashValue)
101+
}
102+
103+
func test_header_equatableAndHashable_nonIdenticalValues() {
104+
let headerA = Header(name: "A", values: ["X", "Y"])
105+
let headerB = Header(name: "B", values: ["X", "Y"])
106+
107+
XCTAssertNotEqual(headerA, headerB)
108+
}
109+
110+
func test_header_equatableAndHashable_identicalValues() {
111+
let headerA = Header(name: "A", values: ["X", "Y"])
112+
let headerB = Header(name: "A", values: ["X", "Y"])
113+
114+
XCTAssertEqual(headerA, headerB)
115+
XCTAssertEqual(headerA.hashValue, headerB.hashValue)
116+
}
117+
118+
func test_header_equatableAndHashable_outOfOrderValues() {
119+
let headerA = Header(name: "A", values: ["X", "Y"])
120+
let headerB = Header(name: "A", values: ["Y", "X"])
121+
122+
XCTAssertEqual(headerA, headerB)
123+
XCTAssertEqual(headerA.hashValue, headerB.hashValue)
124+
}
67125
}

Tests/ClientRuntimeTests/MiddlewareTests/MiddlewareStackTests.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ class MiddlewareStackTests: XCTestCase {
7171
XCTAssert(result.value == 200)
7272
}
7373

74-
func testFullBlownOperationRequestWithClientHandler() async throws {
74+
// This test is disabled because unreliability of httpbin.org is causing spurious failures.
75+
// Github issue to track correction of these tests: https://github.com/awslabs/aws-sdk-swift/issues/962
76+
77+
func xtestFullBlownOperationRequestWithClientHandler() async throws {
7578
let httpClientConfiguration = HttpClientConfiguration()
7679
let clientEngine = CRTClientEngine()
7780
let httpClient = SdkHttpClient(engine: clientEngine, config: httpClientConfiguration)

Tests/ClientRuntimeTests/NetworkingTests/CRTClientEngineIntegrationTests.swift

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import XCTest
77
@testable import ClientRuntime
88
import AwsCommonRuntimeKit
99

10+
// These tests are disabled because unreliability of httpbin.org is causing spurious failures.
11+
// Github issue to track correction of these tests: https://github.com/awslabs/aws-sdk-swift/issues/962
12+
1013
class CRTClientEngineIntegrationTests: NetworkingTestUtils {
1114

1215
var httpClient: SdkHttpClient!
@@ -22,7 +25,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
2225
super.tearDown()
2326
}
2427

25-
func testMakeHttpGetRequest() async throws {
28+
func xtestMakeHttpGetRequest() async throws {
2629
var headers = Headers()
2730
headers.add(name: "Content-type", value: "application/json")
2831
headers.add(name: "Host", value: "httpbin.org")
@@ -33,7 +36,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
3336
XCTAssert(response.statusCode == HttpStatusCode.ok)
3437
}
3538

36-
func testMakeHttpPostRequest() async throws {
39+
func xtestMakeHttpPostRequest() async throws {
3740
//used https://httpbin.org
3841
var headers = Headers()
3942
headers.add(name: "Content-type", value: "application/json")
@@ -50,7 +53,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
5053
XCTAssert(response.statusCode == HttpStatusCode.ok)
5154
}
5255

53-
func testMakeHttpStreamRequestDynamicReceive() async throws {
56+
func xtestMakeHttpStreamRequestDynamicReceive() async throws {
5457
//used https://httpbin.org
5558
var headers = Headers()
5659
headers.add(name: "Content-type", value: "application/json")
@@ -64,7 +67,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
6467
XCTAssert(response.statusCode == HttpStatusCode.ok)
6568
}
6669

67-
func testMakeHttpStreamRequestReceive() async throws {
70+
func xtestMakeHttpStreamRequestReceive() async throws {
6871
//used https://httpbin.org
6972
var headers = Headers()
7073
headers.add(name: "Content-type", value: "application/json")
@@ -84,7 +87,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
8487
XCTAssert(response.statusCode == HttpStatusCode.ok)
8588
}
8689

87-
func testMakeHttpStreamRequestReceiveOneByte() async throws {
90+
func xtestMakeHttpStreamRequestReceiveOneByte() async throws {
8891
//used https://httpbin.org
8992
var headers = Headers()
9093
headers.add(name: "Content-type", value: "application/json")
@@ -104,7 +107,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
104107
XCTAssert(response.statusCode == HttpStatusCode.ok)
105108
}
106109

107-
func testMakeHttpStreamRequestReceive3ThousandBytes() async throws {
110+
func xtestMakeHttpStreamRequestReceive3ThousandBytes() async throws {
108111
//used https://httpbin.org
109112
var headers = Headers()
110113
headers.add(name: "Content-type", value: "application/json")
@@ -124,7 +127,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils {
124127
XCTAssert(response.statusCode == HttpStatusCode.ok)
125128
}
126129

127-
func testMakeHttpStreamRequestFromData() async throws {
130+
func xtestMakeHttpStreamRequestFromData() async throws {
128131
//used https://httpbin.org
129132
var headers = Headers()
130133
headers.add(name: "Content-type", value: "application/json")

0 commit comments

Comments
 (0)