From f1767535eb98a08d1189fcdd1950b021c7bd11a7 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 8 May 2024 10:06:33 -0700 Subject: [PATCH] chore: update http requests, responses and endpoint with uri --- .../ClientRuntime/Networking/Endpoint.swift | 51 +++++++++++-------- .../Networking/Http/HttpResponse.swift | 8 ++- .../Networking/Http/HttpUrlResponse.swift | 1 + .../Networking/Http/SdkHttpRequest.swift | 37 +++++--------- .../ClientRuntime/Networking/Http/URI.swift | 30 +++++++++++ .../ExpectedSdkHttpRequest.swift | 13 ++--- .../Http/HttpRequestTests.swift | 12 ++--- .../Http/SdkRequestBuilderTests.swift | 2 +- 8 files changed, 91 insertions(+), 63 deletions(-) create mode 100644 Sources/ClientRuntime/Networking/Http/URI.swift diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index 2beffb33b..cb8337198 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -6,16 +6,17 @@ import Foundation public struct Endpoint: Hashable { - public let path: String - public let queryItems: [SDKURLQueryItem]? + public let uri: URI public let protocolType: ProtocolType? - public let host: String - public let port: Int16 - public let headers: Headers? + public var queryItems: [SDKURLQueryItem] { uri.query } + public var path: String { uri.path } + public var host: String { uri.host } + public var port: Int16 { uri.port } + public var headers: Headers public let properties: [String: AnyHashable] public init(urlString: String, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { guard let url = URL(string: urlString) else { throw ClientError.unknownError("invalid url \(urlString)") @@ -25,17 +26,19 @@ public struct Endpoint: Hashable { } public init(url: URL, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { guard let host = url.host else { throw ClientError.unknownError("invalid host \(String(describing: url.host))") } let protocolType = ProtocolType(rawValue: url.scheme ?? "") ?? .https - self.init(host: host, - path: url.path, - port: Int16(url.port ?? protocolType.port), - queryItems: url.toQueryItems(), + let uri = URI(scheme: protocolType.rawValue, + path: url.path, + host: host, + port: Int16(url.port ?? protocolType.port), + query: url.toQueryItems() ?? []) + self.init(uri: uri, protocolType: protocolType, headers: headers, properties: properties) @@ -45,13 +48,17 @@ public struct Endpoint: Hashable { path: String = "/", port: Int16 = 443, queryItems: [SDKURLQueryItem]? = nil, + headers: Headers = Headers(), + protocolType: ProtocolType = .https) { + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems ?? []) + self.init(uri: uri, protocolType: protocolType, headers: headers) + } + + public init(uri: URI, protocolType: ProtocolType? = .https, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) { - self.host = host - self.path = path - self.port = port - self.queryItems = queryItems + self.uri = uri self.protocolType = protocolType self.headers = headers self.properties = properties @@ -64,14 +71,16 @@ extension Endpoint { public var url: URL? { var components = URLComponents() components.scheme = protocolType?.rawValue - components.host = host.isEmpty ? nil : host // If host is empty, URL is invalid - components.percentEncodedPath = path - components.percentEncodedQuery = queryItemString + components.host = uri.host.isEmpty ? nil : uri.host // If host is empty, URL is invalid + components.percentEncodedPath = uri.path + components.percentEncodedQuery = query return (components.host == nil || components.scheme == nil) ? nil : components.url } - var queryItemString: String? { - guard let queryItems = queryItems else { return nil } + var query: String? { + if (queryItems.isEmpty) { + return nil + } return queryItems.map { queryItem in return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") diff --git a/Sources/ClientRuntime/Networking/Http/HttpResponse.swift b/Sources/ClientRuntime/Networking/Http/HttpResponse.swift index dd9611f0f..32206165c 100644 --- a/Sources/ClientRuntime/Networking/Http/HttpResponse.swift +++ b/Sources/ClientRuntime/Networking/Http/HttpResponse.swift @@ -9,17 +9,23 @@ public class HttpResponse: HttpUrlResponse { public var headers: Headers public var body: ByteStream public var statusCode: HttpStatusCode + public var reason: String? - public init(headers: Headers = .init(), statusCode: HttpStatusCode = .processing, body: ByteStream = .noStream) { + public init(headers: Headers = .init(), + statusCode: HttpStatusCode = .processing, + body: ByteStream = .noStream, + reason: String? = nil) { self.headers = headers self.statusCode = statusCode self.body = body + self.reason = reason } public init(headers: Headers = .init(), body: ByteStream, statusCode: HttpStatusCode) { self.body = body self.statusCode = statusCode self.headers = headers + self.reason = nil } } diff --git a/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift b/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift index c379e7e24..aebd2ce42 100644 --- a/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift +++ b/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift @@ -9,4 +9,5 @@ protocol HttpUrlResponse { var headers: Headers { get set } var body: ByteStream { get set} var statusCode: HttpStatusCode {get set} + var reason: String? {get set} } diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index 2b43c1780..b0fbc4a8c 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -17,18 +17,14 @@ import struct Foundation.URLRequest // in the CRT engine so that is why it's a class public class SdkHttpRequest { public var body: ByteStream - public let endpoint: Endpoint + public var endpoint: Endpoint public let method: HttpMethodType - private var additionalHeaders: Headers = Headers() - public var headers: Headers { - var allHeaders = endpoint.headers ?? Headers() - allHeaders.addAll(headers: additionalHeaders) - return allHeaders - } - public var trailingHeaders: Headers = Headers() + public var destination: URI { endpoint.uri } + public var headers: Headers { endpoint.headers } public var path: String { endpoint.path } public var host: String { endpoint.host } public var queryItems: [SDKURLQueryItem]? { endpoint.queryItems } + public var trailingHeaders: Headers = Headers() public init(method: HttpMethodType, endpoint: Endpoint, @@ -55,11 +51,11 @@ public class SdkHttpRequest { } public func withHeader(name: String, value: String) { - self.additionalHeaders.add(name: name, value: value) + self.endpoint.headers.add(name: name, value: value) } public func withoutHeader(name: String) { - self.additionalHeaders.remove(name: name) + self.endpoint.headers.remove(name: name) } public func withBody(_ body: ByteStream) { @@ -92,7 +88,7 @@ extension SdkHttpRequest { public func toHttpRequest() throws -> HTTPRequest { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) httpRequest.body = isChunked ? nil : StreamableHttpBody(body: body) // body needs to be nil to use writeChunk() return httpRequest @@ -104,7 +100,7 @@ extension SdkHttpRequest { public func toHttp2Request() throws -> HTTPRequestBase { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) // Remove the "Transfer-Encoding" header if it exists since h2 does not support it @@ -198,7 +194,7 @@ public class SdkHttpRequestBuilder { var host: String = "" var path: String = "/" var body: ByteStream = .noStream - var queryItems: [SDKURLQueryItem]? + var queryItems: [SDKURLQueryItem] = [] var port: Int16 = 443 var protocolType: ProtocolType = .https var trailingHeaders: Headers = Headers() @@ -267,8 +263,7 @@ public class SdkHttpRequestBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> SdkHttpRequestBuilder { - self.queryItems = self.queryItems ?? [] - self.queryItems?.append(contentsOf: value) + self.queryItems.append(contentsOf: value) return self } @@ -290,15 +285,9 @@ public class SdkHttpRequestBuilder { } public func build() -> SdkHttpRequest { - let endpoint = Endpoint(host: host, - path: path, - port: port, - queryItems: queryItems, - protocolType: protocolType, - headers: headers) - return SdkHttpRequest(method: methodType, - endpoint: endpoint, - body: body) + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) + let endpoint = Endpoint(uri: uri, protocolType: protocolType, headers: headers) + return SdkHttpRequest(method: methodType, endpoint: endpoint, body: body) } } diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift new file mode 100644 index 000000000..69980bbfa --- /dev/null +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -0,0 +1,30 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +public struct URI: Hashable { + public let scheme: String + public let path: String + public let host: String + public let port: Int16 + public let query: [SDKURLQueryItem] + public let username: String? + public let password: String? + + public init(scheme: String, + path: String, + host: String, + port: Int16, + query: [SDKURLQueryItem], + username: String? = nil, + password: String? = nil) { + self.scheme = scheme + self.path = path + self.host = host + self.port = port + self.query = query + self.username = username + self.password = password + } +} diff --git a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift index d8ba0506b..2f23ad7c9 100644 --- a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift +++ b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift @@ -12,7 +12,7 @@ public struct ExpectedSdkHttpRequest { public var headers: Headers? public var forbiddenHeaders: [String]? public var requiredHeaders: [String]? - public let queryItems: [SDKURLQueryItem]? + public var queryItems: [SDKURLQueryItem] { endpoint.queryItems } public let forbiddenQueryItems: [SDKURLQueryItem]? public let requiredQueryItems: [SDKURLQueryItem]? public let endpoint: Endpoint @@ -23,7 +23,6 @@ public struct ExpectedSdkHttpRequest { headers: Headers? = nil, forbiddenHeaders: [String]? = nil, requiredHeaders: [String]? = nil, - queryItems: [SDKURLQueryItem]? = nil, forbiddenQueryItems: [SDKURLQueryItem]? = nil, requiredQueryItems: [SDKURLQueryItem]? = nil, body: ByteStream = ByteStream.noStream) { @@ -32,7 +31,6 @@ public struct ExpectedSdkHttpRequest { self.headers = headers self.forbiddenHeaders = forbiddenHeaders self.requiredHeaders = requiredHeaders - self.queryItems = queryItems self.forbiddenQueryItems = forbiddenQueryItems self.requiredQueryItems = requiredQueryItems self.body = body @@ -139,12 +137,8 @@ public class ExpectedSdkHttpRequestBuilder { } public func build() -> ExpectedSdkHttpRequest { - let endpoint = Endpoint(host: host, - path: path, - port: port, - queryItems: queryItems, - protocolType: protocolType) - let queryItems = !queryItems.isEmpty ? queryItems : nil + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) + let endpoint = Endpoint(uri: uri, protocolType: protocolType) let forbiddenQueryItems = !forbiddenQueryItems.isEmpty ? forbiddenQueryItems : nil let requiredQueryItems = !requiredQueryItems.isEmpty ? requiredQueryItems : nil @@ -156,7 +150,6 @@ public class ExpectedSdkHttpRequestBuilder { headers: headers, forbiddenHeaders: forbiddenHeaders, requiredHeaders: requiredHeaders, - queryItems: queryItems, forbiddenQueryItems: forbiddenQueryItems, requiredQueryItems: requiredQueryItems, body: body) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index 1b7d5a433..c91399658 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -118,17 +118,17 @@ class HttpRequestTests: NetworkingTestUtils { .withQueryItem(queryItem2) .withHeader(name: "Content-Length", value: "6") - XCTAssert(builder.queryItems?.count == 2) + XCTAssert(builder.queryItems.count == 2) let httpRequest = try builder.build().toHttpRequest() httpRequest.path = "/hello?foo=bar&quz=bar&signedthing=signed" let updatedRequest = builder.update(from: httpRequest, originalRequest: builder.build()) XCTAssert(updatedRequest.path == "/hello") - XCTAssert(updatedRequest.queryItems?.count == 3) - XCTAssert(updatedRequest.queryItems?.contains(queryItem1) ?? false) - XCTAssert(updatedRequest.queryItems?.contains(queryItem2) ?? false) - XCTAssert(updatedRequest.queryItems?.contains(SDKURLQueryItem(name: "signedthing", value: "signed")) ?? false) + XCTAssert(updatedRequest.queryItems.count == 3) + XCTAssert(updatedRequest.queryItems.contains(queryItem1) ?? false) + XCTAssert(updatedRequest.queryItems.contains(queryItem2) ?? false) + XCTAssert(updatedRequest.queryItems.contains(SDKURLQueryItem(name: "signedthing", value: "signed")) ?? false) } func testPathInInHttpRequestIsNotAltered() throws { @@ -143,7 +143,7 @@ class HttpRequestTests: NetworkingTestUtils { func testConversionToUrlRequestFailsWithInvalidEndpoint() { // Testing with an invalid endpoint where host is empty, // path is empty, and protocolType is nil. - let endpoint = Endpoint(host: "", path: "", protocolType: nil) + let endpoint = Endpoint(host: "", path: "") XCTAssertNil(endpoint.url, "An invalid endpoint should result in a nil URL.") } } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index d6b18d1c3..60d7478c5 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -17,7 +17,7 @@ class SdkRequestBuilderTests: XCTestCase { crtRequest.path = pathToMatch let updatedRequest = SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() - let updatedPath = [updatedRequest.endpoint.path, updatedRequest.endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + let updatedPath = [updatedRequest.endpoint.path, updatedRequest.endpoint.query].compactMap { $0 }.joined(separator: "?") XCTAssertEqual(pathToMatch, updatedPath) XCTAssertEqual(url, updatedRequest.endpoint.url?.absoluteString) }