Refactor deconstructURL and scheme parsing (#504)

dnadoba · web-flow · commit 99bd384b3857 · 2021-12-01T13:03:09.000+01:00
* make `Scheme` a type

* introduce new Endpoint type

* use endpoint as storage in `HTTPClient.Request`

* fix merge conflicts

* rename Endpoint to DeconstructedURL

* swift-format

* make `DeconstructedURL` properties `var`'s

* move scheme into global namespace

- rename `useTLS` to `usesTLS` where posible without breaking public API
- only import Foundation.URL

* fix review comments
diff --git a/Sources/AsyncHTTPClient/ConnectionPool.swift b/Sources/AsyncHTTPClient/ConnectionPool.swift
@@ -24,52 +24,13 @@ enum ConnectionPool {
         private var tlsConfiguration: BestEffortHashableTLSConfiguration?
 
         init(_ request: HTTPClient.Request) {
-            self.connectionTarget = request.connectionTarget
-            switch request.scheme {
-            case "http":
-                self.scheme = .http
-            case "https":
-                self.scheme = .https
-            case "unix":
-                self.scheme = .unix
-            case "http+unix":
-                self.scheme = .http_unix
-            case "https+unix":
-                self.scheme = .https_unix
-            default:
-                fatalError("HTTPClient.Request scheme should already be a valid one")
-            }
+            self.scheme = request.deconstructedURL.scheme
+            self.connectionTarget = request.deconstructedURL.connectionTarget
             if let tls = request.tlsConfiguration {
                 self.tlsConfiguration = BestEffortHashableTLSConfiguration(wrapping: tls)
             }
         }
 
-        enum Scheme: Hashable {
-            case http
-            case https
-            case unix
-            case http_unix
-            case https_unix
-
-            var requiresTLS: Bool {
-                switch self {
-                case .https, .https_unix:
-                    return true
-                default:
-                    return false
-                }
-            }
-        }
-
-        /// Returns a key-specific `HTTPClient.Configuration` by overriding the properties of `base`
-        func config(overriding base: HTTPClient.Configuration) -> HTTPClient.Configuration {
-            var config = base
-            if let tlsConfiguration = self.tlsConfiguration {
-                config.tlsConfiguration = tlsConfiguration.base
-            }
-            return config
-        }
-
         var description: String {
             var hasher = Hasher()
             self.tlsConfiguration?.hash(into: &hasher)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift
@@ -185,9 +185,9 @@ extension HTTPConnectionPool.ConnectionFactory {
         logger: Logger
     ) -> EventLoopFuture<NegotiatedProtocol> {
         switch self.key.scheme {
-        case .http, .http_unix, .unix:
+        case .http, .httpUnix, .unix:
             return self.makePlainChannel(deadline: deadline, eventLoop: eventLoop).map { .http1_1($0) }
-        case .https, .https_unix:
+        case .https, .httpsUnix:
             return self.makeTLSChannel(deadline: deadline, eventLoop: eventLoop, logger: logger).flatMapThrowing {
                 channel, negotiated in
 
@@ -197,7 +197,7 @@ extension HTTPConnectionPool.ConnectionFactory {
     }
 
     private func makePlainChannel(deadline: NIODeadline, eventLoop: EventLoop) -> EventLoopFuture<Channel> {
-        precondition(!self.key.scheme.requiresTLS, "Unexpected scheme")
+        precondition(!self.key.scheme.usesTLS, "Unexpected scheme")
         return self.makePlainBootstrap(deadline: deadline, eventLoop: eventLoop).connect(target: self.key.connectionTarget)
     }
 
@@ -283,7 +283,7 @@ extension HTTPConnectionPool.ConnectionFactory {
         logger: Logger
     ) -> EventLoopFuture<NegotiatedProtocol> {
         switch self.key.scheme {
-        case .unix, .http_unix, .https_unix:
+        case .unix, .httpUnix, .httpsUnix:
             preconditionFailure("Unexpected scheme. Not supported for proxy!")
         case .http:
             return channel.eventLoop.makeSucceededFuture(.http1_1(channel))
@@ -356,7 +356,7 @@ extension HTTPConnectionPool.ConnectionFactory {
     }
 
     private func makeTLSChannel(deadline: NIODeadline, eventLoop: EventLoop, logger: Logger) -> EventLoopFuture<(Channel, String?)> {
-        precondition(self.key.scheme.requiresTLS, "Unexpected scheme")
+        precondition(self.key.scheme.usesTLS, "Unexpected scheme")
         let bootstrapFuture = self.makeTLSBootstrap(
             deadline: deadline,
             eventLoop: eventLoop,
@@ -470,12 +470,12 @@ extension HTTPConnectionPool.ConnectionFactory {
     }
 }
 
-extension ConnectionPool.Key.Scheme {
+extension Scheme {
     var isProxyable: Bool {
         switch self {
         case .http, .https:
             return true
-        case .unix, .http_unix, .https_unix:
+        case .unix, .httpUnix, .httpsUnix:
             return false
         }
     }
diff --git a/Sources/AsyncHTTPClient/DeconstructedURL.swift b/Sources/AsyncHTTPClient/DeconstructedURL.swift
@@ -0,0 +1,76 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the AsyncHTTPClient open source project
+//
+// Copyright (c) 2021 Apple Inc. and the AsyncHTTPClient project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of AsyncHTTPClient project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+import struct Foundation.URL
+
+struct DeconstructedURL {
+    var scheme: Scheme
+    var connectionTarget: ConnectionTarget
+    var uri: String
+
+    init(
+        scheme: Scheme,
+        connectionTarget: ConnectionTarget,
+        uri: String
+    ) {
+        self.scheme = scheme
+        self.connectionTarget = connectionTarget
+        self.uri = uri
+    }
+}
+
+extension DeconstructedURL {
+    init(url: URL) throws {
+        guard let schemeString = url.scheme else {
+            throw HTTPClientError.emptyScheme
+        }
+        guard let scheme = Scheme(rawValue: schemeString.lowercased()) else {
+            throw HTTPClientError.unsupportedScheme(schemeString)
+        }
+
+        switch scheme {
+        case .http, .https:
+            guard let host = url.host, !host.isEmpty else {
+                throw HTTPClientError.emptyHost
+            }
+            self.init(
+                scheme: scheme,
+                connectionTarget: .init(remoteHost: host, port: url.port ?? scheme.defaultPort),
+                uri: url.uri
+            )
+
+        case .httpUnix, .httpsUnix:
+            guard let socketPath = url.host, !socketPath.isEmpty else {
+                throw HTTPClientError.missingSocketPath
+            }
+            self.init(
+                scheme: scheme,
+                connectionTarget: .unixSocket(path: socketPath),
+                uri: url.uri
+            )
+
+        case .unix:
+            let socketPath = url.baseURL?.path ?? url.path
+            let uri = url.baseURL != nil ? url.uri : "/"
+            guard !socketPath.isEmpty else {
+                throw HTTPClientError.missingSocketPath
+            }
+            self.init(
+                scheme: scheme,
+                connectionTarget: .unixSocket(path: socketPath),
+                uri: uri
+            )
+        }
+    }
+}
diff --git a/Sources/AsyncHTTPClient/HTTPHandler.swift b/Sources/AsyncHTTPClient/HTTPHandler.swift
@@ -92,82 +92,16 @@ extension HTTPClient {
 
     /// Represent HTTP request.
     public struct Request {
-        /// Represent kind of Request
-        enum Kind: Equatable {
-            enum UnixScheme: Equatable {
-                case baseURL
-                case http_unix
-                case https_unix
-            }
-
-            /// Remote host request.
-            case host
-            /// UNIX Domain Socket HTTP request.
-            case unixSocket(_ scheme: UnixScheme)
-
-            private static var hostRestrictedSchemes: Set = ["http", "https"]
-            private static var allSupportedSchemes: Set = ["http", "https", "unix", "http+unix", "https+unix"]
-
-            func supportsRedirects(to scheme: String?) -> Bool {
-                guard let scheme = scheme?.lowercased() else { return false }
-
-                switch self {
-                case .host:
-                    return Kind.hostRestrictedSchemes.contains(scheme)
-                case .unixSocket:
-                    return Kind.allSupportedSchemes.contains(scheme)
-                }
-            }
-        }
-
-        static func useTLS(_ scheme: String) -> Bool {
-            return scheme == "https" || scheme == "https+unix"
-        }
-
-        static func deconstructURL(
-            _ url: URL
-        ) throws -> (kind: Kind, scheme: String, connectionTarget: ConnectionTarget, uri: String) {
-            guard let scheme = url.scheme?.lowercased() else {
-                throw HTTPClientError.emptyScheme
-            }
-            switch scheme {
-            case "http", "https":
-                guard let host = url.host, !host.isEmpty else {
-                    throw HTTPClientError.emptyHost
-                }
-                let defaultPort = self.useTLS(scheme) ? 443 : 80
-                let hostTarget = ConnectionTarget(remoteHost: host, port: url.port ?? defaultPort)
-                return (.host, scheme, hostTarget, url.uri)
-            case "http+unix", "https+unix":
-                guard let socketPath = url.host, !socketPath.isEmpty else {
-                    throw HTTPClientError.missingSocketPath
-                }
-                let socketTarget = ConnectionTarget.unixSocket(path: socketPath)
-                let kind = self.useTLS(scheme) ? Kind.UnixScheme.https_unix : .http_unix
-                return (.unixSocket(kind), scheme, socketTarget, url.uri)
-            case "unix":
-                let socketPath = url.baseURL?.path ?? url.path
-                let uri = url.baseURL != nil ? url.uri : "/"
-                guard !socketPath.isEmpty else {
-                    throw HTTPClientError.missingSocketPath
-                }
-                let socketTarget = ConnectionTarget.unixSocket(path: socketPath)
-                return (.unixSocket(.baseURL), scheme, socketTarget, uri)
-            default:
-                throw HTTPClientError.unsupportedScheme(url.scheme!)
-            }
-        }
-
         /// Request HTTP method, defaults to `GET`.
         public let method: HTTPMethod
         /// Remote URL.
         public let url: URL
+
         /// Remote HTTP scheme, resolved from `URL`.
-        public let scheme: String
-        /// The connection target, resolved from `URL`.
-        let connectionTarget: ConnectionTarget
-        /// URI composed of the path and query, resolved from `URL`.
-        let uri: String
+        public var scheme: String {
+            self.deconstructedURL.scheme.rawValue
+        }
+
         /// Request custom HTTP Headers, defaults to no headers.
         public var headers: HTTPHeaders
         /// Request body, defaults to no body.
@@ -180,8 +114,10 @@ extension HTTPClient {
             var visited: Set<URL>?
         }
 
+        /// Parsed, validated and deconstructed URL.
+        let deconstructedURL: DeconstructedURL
+
         var redirectState: RedirectState?
-        let kind: Kind
 
         /// Create HTTP request.
         ///
@@ -252,7 +188,8 @@ extension HTTPClient {
         ///     - `emptyHost` if URL does not contains a host.
         ///     - `missingSocketPath` if URL does not contains a socketPath as an encoded host.
         public init(url: URL, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil, tlsConfiguration: TLSConfiguration?) throws {
-            (self.kind, self.scheme, self.connectionTarget, self.uri) = try Request.deconstructURL(url)
+            self.deconstructedURL = try DeconstructedURL(url: url)
+
             self.redirectState = nil
             self.url = url
             self.method = method
@@ -261,14 +198,9 @@ extension HTTPClient {
             self.tlsConfiguration = tlsConfiguration
         }
 
-        /// Whether request will be executed using secure socket.
-        public var useTLS: Bool {
-            return Request.useTLS(self.scheme)
-        }
-
         /// Remote host, resolved from `URL`.
         public var host: String {
-            switch self.connectionTarget {
+            switch self.deconstructedURL.connectionTarget {
             case .ipAddress(let serialization, _): return serialization
             case .domain(let name, _): return name
             case .unixSocket: return ""
@@ -277,25 +209,28 @@ extension HTTPClient {
 
         /// Resolved port.
         public var port: Int {
-            switch self.connectionTarget {
+            switch self.deconstructedURL.connectionTarget {
             case .ipAddress(_, let address): return address.port!
             case .domain(_, let port): return port
-            case .unixSocket: return Request.useTLS(self.scheme) ? 443 : 80
+            case .unixSocket: return self.deconstructedURL.scheme.defaultPort
             }
         }
 
+        /// Whether request will be executed using secure socket.
+        public var useTLS: Bool { self.deconstructedURL.scheme.usesTLS }
+
         func createRequestHead() throws -> (HTTPRequestHead, RequestFramingMetadata) {
             var head = HTTPRequestHead(
                 version: .http1_1,
                 method: self.method,
-                uri: self.uri,
+                uri: self.deconstructedURL.uri,
                 headers: self.headers
             )
 
             if !head.headers.contains(name: "host") {
                 let port = self.port
                 var host = self.host
-                if !(port == 80 && self.scheme == "http"), !(port == 443 && self.scheme == "https") {
+                if !(port == 80 && self.deconstructedURL.scheme == .http), !(port == 443 && self.deconstructedURL.scheme == .https) {
                     host += ":\(port)"
                 }
                 head.headers.add(name: "host", value: host)
@@ -740,7 +675,7 @@ internal struct RedirectHandler<ResponseType> {
             return nil
         }
 
-        guard self.request.kind.supportsRedirects(to: url.scheme) else {
+        guard self.request.deconstructedURL.scheme.supportsRedirects(to: url.scheme) else {
             return nil
         }
 
diff --git a/Sources/AsyncHTTPClient/Scheme.swift b/Sources/AsyncHTTPClient/Scheme.swift
diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift
