From 4efc5c0e153e501fbb436796176215b3e1a9fc61 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Wed, 14 Apr 2021 10:51:48 +0100 Subject: [PATCH] A more general channel provider Motivation: In #1158 we pulled connection creation of the connection manager into a channel provider in order to loosen the coupling between the connection manager and `ClientConnection`. This change further decouples the `ConnectionManager` from the channel provider pulling out the relevant configuration into a `DefaultChannelProvider`. Modifications: - Refactor `ClientConnection.ChannelProvider` to rely on the bits of configuration it actually requires rather than `ClientConnection.Configuration` - Rename to `DefaultChannelProvider` Result: We can configure channels for the `ConnectionManager` without being tied to `ClientConnection`. --- .swiftformat | 3 + Sources/GRPC/ConnectionManager.swift | 4 +- .../ConnectionManagerChannelProvider.swift | 110 ++++++++++++------ 3 files changed, 80 insertions(+), 37 deletions(-) diff --git a/.swiftformat b/.swiftformat index b39764641..cb0085f4c 100644 --- a/.swiftformat +++ b/.swiftformat @@ -19,6 +19,9 @@ # Don't indent #if blocks --ifdef no-indent +# Don't turn Optional into Foo? +--shortoptionals except-properties + # This rule doesn't always work as we'd expect: specifically when we return a # succeeded future whose type is a closure then that closure is incorrectly # treated as a trailing closure. This is relevant because the service provider diff --git a/Sources/GRPC/ConnectionManager.swift b/Sources/GRPC/ConnectionManager.swift index 6d1b6ffe9..97dc4957e 100644 --- a/Sources/GRPC/ConnectionManager.swift +++ b/Sources/GRPC/ConnectionManager.swift @@ -251,7 +251,7 @@ internal final class ConnectionManager { internal convenience init(configuration: ClientConnection.Configuration, logger: Logger) { self.init( configuration: configuration, - channelProvider: ClientConnection.ChannelProvider(configuration: configuration), + channelProvider: DefaultChannelProvider(configuration: configuration), logger: logger ) } @@ -297,7 +297,7 @@ internal final class ConnectionManager { ) } - private init( + internal init( eventLoop: EventLoop, channelProvider: ConnectionManagerChannelProvider, callStartBehavior: CallStartBehavior.Behavior, diff --git a/Sources/GRPC/ConnectionManagerChannelProvider.swift b/Sources/GRPC/ConnectionManagerChannelProvider.swift index 3a942639e..ec956947d 100644 --- a/Sources/GRPC/ConnectionManagerChannelProvider.swift +++ b/Sources/GRPC/ConnectionManagerChannelProvider.swift @@ -15,6 +15,7 @@ */ import Logging import NIO +import NIOSSL internal protocol ConnectionManagerChannelProvider { /// Make an `EventLoopFuture`. @@ -32,37 +33,79 @@ internal protocol ConnectionManagerChannelProvider { ) -> EventLoopFuture } -extension ClientConnection { - internal struct ChannelProvider { - private var configuration: Configuration +internal struct DefaultChannelProvider: ConnectionManagerChannelProvider { + internal var connectionTarget: ConnectionTarget + internal var connectionKeepalive: ClientConnectionKeepalive + internal var connectionIdleTimeout: TimeAmount - internal init(configuration: Configuration) { - self.configuration = configuration - } + internal var tlsConfiguration: Optional + internal var tlsHostnameOverride: Optional + internal var tlsCustomVerificationCallback: Optional + + internal var httpTargetWindowSize: Int + + internal var errorDelegate: Optional + internal var debugChannelInitializer: Optional<(Channel) -> EventLoopFuture> + + internal init( + connectionTarget: ConnectionTarget, + connectionKeepalive: ClientConnectionKeepalive, + connectionIdleTimeout: TimeAmount, + tlsConfiguration: TLSConfiguration?, + tlsHostnameOverride: String?, + tlsCustomVerificationCallback: NIOSSLCustomVerificationCallback?, + httpTargetWindowSize: Int, + errorDelegate: ClientErrorDelegate?, + debugChannelInitializer: ((Channel) -> EventLoopFuture)? + ) { + self.connectionTarget = connectionTarget + self.connectionKeepalive = connectionKeepalive + self.connectionIdleTimeout = connectionIdleTimeout + + self.tlsConfiguration = tlsConfiguration + self.tlsHostnameOverride = tlsHostnameOverride + self.tlsCustomVerificationCallback = tlsCustomVerificationCallback + + self.httpTargetWindowSize = httpTargetWindowSize + + self.errorDelegate = errorDelegate + self.debugChannelInitializer = debugChannelInitializer + } + + internal init(configuration: ClientConnection.Configuration) { + self.init( + connectionTarget: configuration.target, + connectionKeepalive: configuration.connectionKeepalive, + connectionIdleTimeout: configuration.connectionIdleTimeout, + tlsConfiguration: configuration.tls?.configuration, + tlsHostnameOverride: configuration.tls?.hostnameOverride, + tlsCustomVerificationCallback: configuration.tls?.customVerificationCallback, + httpTargetWindowSize: configuration.httpTargetWindowSize, + errorDelegate: configuration.errorDelegate, + debugChannelInitializer: configuration.debugChannelInitializer + ) + } + + private var serverHostname: String? { + let hostname = self.tlsHostnameOverride ?? self.connectionTarget.host + return hostname.isIPAddress ? nil : hostname + } + + private var hasTLS: Bool { + return self.tlsConfiguration != nil + } + + private func requiresZeroLengthWorkaround(eventLoop: EventLoop) -> Bool { + return PlatformSupport.requiresZeroLengthWriteWorkaround(group: eventLoop, hasTLS: self.hasTLS) } -} -extension ClientConnection.ChannelProvider: ConnectionManagerChannelProvider { internal func makeChannel( managedBy connectionManager: ConnectionManager, onEventLoop eventLoop: EventLoop, connectTimeout: TimeAmount?, logger: Logger ) -> EventLoopFuture { - let serverHostname: String? = self.configuration.tls.flatMap { tls -> String? in - if let hostnameOverride = tls.hostnameOverride { - return hostnameOverride - } else { - return self.configuration.target.host - } - }.flatMap { hostname in - if hostname.isIPAddress { - return nil - } else { - return hostname - } - } - + let needsZeroLengthWriteWorkaround = self.requiresZeroLengthWorkaround(eventLoop: eventLoop) let bootstrap = PlatformSupport.makeClientBootstrap(group: eventLoop, logger: logger) .channelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1) .channelOption(ChannelOptions.socket(IPPROTO_TCP, TCP_NODELAY), value: 1) @@ -72,26 +115,23 @@ extension ClientConnection.ChannelProvider: ConnectionManagerChannelProvider { do { try sync.configureGRPCClient( channel: channel, - httpTargetWindowSize: self.configuration.httpTargetWindowSize, - tlsConfiguration: self.configuration.tls?.configuration, - tlsServerHostname: serverHostname, + httpTargetWindowSize: self.httpTargetWindowSize, + tlsConfiguration: self.tlsConfiguration, + tlsServerHostname: self.tlsHostnameOverride, connectionManager: connectionManager, - connectionKeepalive: self.configuration.connectionKeepalive, - connectionIdleTimeout: self.configuration.connectionIdleTimeout, - errorDelegate: self.configuration.errorDelegate, - requiresZeroLengthWriteWorkaround: PlatformSupport.requiresZeroLengthWriteWorkaround( - group: eventLoop, - hasTLS: self.configuration.tls != nil - ), + connectionKeepalive: self.connectionKeepalive, + connectionIdleTimeout: self.connectionIdleTimeout, + errorDelegate: self.errorDelegate, + requiresZeroLengthWriteWorkaround: needsZeroLengthWriteWorkaround, logger: logger, - customVerificationCallback: self.configuration.tls?.customVerificationCallback + customVerificationCallback: self.tlsCustomVerificationCallback ) } catch { return channel.eventLoop.makeFailedFuture(error) } // Run the debug initializer, if there is one. - if let debugInitializer = self.configuration.debugChannelInitializer { + if let debugInitializer = self.debugChannelInitializer { return debugInitializer(channel) } else { return channel.eventLoop.makeSucceededVoidFuture() @@ -102,6 +142,6 @@ extension ClientConnection.ChannelProvider: ConnectionManagerChannelProvider { _ = bootstrap.connectTimeout(connectTimeout) } - return bootstrap.connect(to: self.configuration.target) + return bootstrap.connect(to: self.connectionTarget) } }