Browse Source

Reduce code duplication between builders and configuration (#907)

Motivation:

The client and server builders each eventually produce a `Configuration`.
Each `Configuration` has a bunch of defaults which were duplicated
in the builder.

Modifications:

- Builders use configuration rather than relying on their own values and
  then building a configuration

Result:

- Less code duplication; lower chance of drift in defaults between
  configuration and builders
George Barnett 5 years ago
parent
commit
47cb50187b

+ 2 - 2
Sources/GRPC/ClientConnection.swift

@@ -536,8 +536,8 @@ extension ClientConnection {
     /// - Parameter callStartBehavior: The behavior used to determine when a call should start in
     ///     relation to its underlying connection. Defaults to `waitsForConnectivity`.
     /// - Parameter httpTargetWindowSize: The HTTP/2 flow control target window size.
-    /// - Parameter logger: A logger for background information (such as connectivity state).
-    ///     Defaults to a no-op logger.
+    /// - Parameter backgroundActivityLogger: A logger for background information (such as
+    ///     connectivity state). Defaults to a no-op logger.
     public init(
       target: ConnectionTarget,
       eventLoopGroup: EventLoopGroup,

+ 18 - 33
Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift

@@ -32,39 +32,24 @@ extension ClientConnection {
 
 extension ClientConnection {
   public class Builder {
-    private let group: EventLoopGroup
+    private var configuration: ClientConnection.Configuration
     private var maybeTLS: ClientConnection.Configuration.TLS? { return nil }
+
     private var connectionBackoff = ConnectionBackoff()
     private var connectionBackoffIsEnabled = true
-    private var errorDelegate: ClientErrorDelegate?
-    private var connectivityStateDelegate: ConnectivityStateDelegate?
-    private var connectivityStateDelegateQueue: DispatchQueue?
-    private var connectionKeepalive = ClientConnectionKeepalive()
-    private var connectionIdleTimeout: TimeAmount = .minutes(5)
-    private var callStartBehavior: CallStartBehavior = .waitsForConnectivity
-    private var httpTargetWindowSize: Int = 65535
-    private var logger: Logger = Logger(label: "io.grpc", factory: { _ in SwiftLogNoOpLogHandler() })
 
     fileprivate init(group: EventLoopGroup) {
-      self.group = group
+      // This is okay: the configuration is only consumed on a call to `connect` which sets the host
+      // and port.
+      self.configuration = Configuration(target: .hostAndPort("", .max), eventLoopGroup: group)
     }
 
     public func connect(host: String, port: Int) -> ClientConnection {
-      let configuration = ClientConnection.Configuration(
-        target: .hostAndPort(host, port),
-        eventLoopGroup: self.group,
-        errorDelegate: self.errorDelegate,
-        connectivityStateDelegate: self.connectivityStateDelegate,
-        connectivityStateDelegateQueue: self.connectivityStateDelegateQueue,
-        tls: self.maybeTLS,
-        connectionBackoff: self.connectionBackoffIsEnabled ? self.connectionBackoff : nil,
-        connectionKeepalive: self.connectionKeepalive,
-        connectionIdleTimeout: self.connectionIdleTimeout,
-        callStartBehavior: self.callStartBehavior,
-        httpTargetWindowSize: self.httpTargetWindowSize,
-        backgroundActivityLogger: self.logger
-      )
-      return ClientConnection(configuration: configuration)
+      // Finish setting up the configuration.
+      self.configuration.target = .hostAndPort(host, port)
+      self.configuration.connectionBackoff = self.connectionBackoffIsEnabled ? self.connectionBackoff : nil
+      self.configuration.tls = maybeTLS
+      return ClientConnection(configuration: self.configuration)
     }
   }
 }
@@ -158,7 +143,7 @@ extension ClientConnection.Builder {
   /// [documentation] (https://github.com/grpc/grpc/blob/master/doc/keepalive.md).
   @discardableResult
   public func withKeepalive(_ keepalive: ClientConnectionKeepalive) -> Self {
-    self.connectionKeepalive = keepalive
+    self.configuration.connectionKeepalive = keepalive
     return self
   }
 
@@ -168,7 +153,7 @@ extension ClientConnection.Builder {
   /// Defaults to 5 minutes if not set.
   @discardableResult
   public func withConnectionIdleTimeout(_ timeout: TimeAmount) -> Self {
-    self.connectionIdleTimeout = timeout
+    self.configuration.connectionIdleTimeout = timeout
     return self
   }
 
@@ -177,7 +162,7 @@ extension ClientConnection.Builder {
   /// use `.waitsForConnectivity` by default.
   @discardableResult
   public func withCallStartBehavior(_ behavior: CallStartBehavior) -> Self {
-    self.callStartBehavior = behavior
+    self.configuration.callStartBehavior = behavior
     return self
   }
 }
@@ -186,7 +171,7 @@ extension ClientConnection.Builder {
   /// Sets the client error delegate.
   @discardableResult
   public func withErrorDelegate(_ delegate: ClientErrorDelegate?) -> Self {
-    self.errorDelegate = delegate
+    self.configuration.errorDelegate = delegate
     return self
   }
 }
@@ -200,8 +185,8 @@ extension ClientConnection.Builder {
     _ delegate: ConnectivityStateDelegate?,
     executingOn queue: DispatchQueue? = nil
   ) -> Self {
-    self.connectivityStateDelegate = delegate
-    self.connectivityStateDelegateQueue = queue
+    self.configuration.connectivityStateDelegate = delegate
+    self.configuration.connectivityStateDelegateQueue = queue
     return self
   }
 }
@@ -247,7 +232,7 @@ extension ClientConnection.Builder {
   /// Sets the HTTP/2 flow control target window size. Defaults to 65,535 if not explicitly set.
   @discardableResult
   public func withHTTPTargetWindowSize(_ httpTargetWindowSize: Int) -> Self {
-    self.httpTargetWindowSize = httpTargetWindowSize
+    self.configuration.httpTargetWindowSize = httpTargetWindowSize
     return self
   }
 }
@@ -260,7 +245,7 @@ extension ClientConnection.Builder {
   /// here.
   @discardableResult
   public func withBackgroundActivityLogger(_ logger: Logger) -> Self {
-    self.logger = logger
+    self.configuration.backgroundActivityLogger = logger
     return self
   }
 }

+ 19 - 29
Sources/GRPC/ServerBuilder.swift

@@ -19,18 +19,17 @@ import Logging
 
 extension Server {
   public class Builder {
-    private let group: EventLoopGroup
+    private var configuration: Server.Configuration
     private var maybeTLS: Server.Configuration.TLS? { return nil }
-    private var providers: [CallHandlerProvider] = []
-    private var errorDelegate: ServerErrorDelegate?
-    private var messageEncoding: ServerMessageEncoding = .disabled
-    private var connectionKeepalive: ServerConnectionKeepalive = ServerConnectionKeepalive()
-    private var connectionIdleTimeout: TimeAmount = .minutes(5)
-    private var httpTargetWindowSize: Int = 65535
-    private var logger: Logger = Logger(label: "io.grpc", factory: { _ in SwiftLogNoOpLogHandler() })
 
     fileprivate init(group: EventLoopGroup) {
-      self.group = group
+      self.configuration = Configuration(
+        // This is okay: the configuration is only consumed on a call to `bind` which sets the host
+        // and port.
+        target: .hostAndPort("", .max),
+        eventLoopGroup: group,
+        serviceProviders: []
+      )
     }
 
     public class Secure: Builder {
@@ -49,19 +48,10 @@ extension Server {
     }
 
     public func bind(host: String, port: Int) -> EventLoopFuture<Server> {
-      let configuration = Server.Configuration(
-        target: .hostAndPort(host, port),
-        eventLoopGroup: self.group,
-        serviceProviders: self.providers,
-        errorDelegate: self.errorDelegate,
-        tls: self.maybeTLS,
-        connectionKeepalive: self.connectionKeepalive,
-        connectionIdleTimeout: self.connectionIdleTimeout,
-        messageEncoding: self.messageEncoding,
-        httpTargetWindowSize: self.httpTargetWindowSize,
-        logger: self.logger
-      )
-      return Server.start(configuration: configuration)
+      // Finish setting up the configuration.
+      self.configuration.target = .hostAndPort(host, port)
+      self.configuration.tls = self.maybeTLS
+      return Server.start(configuration: self.configuration)
     }
   }
 }
@@ -70,7 +60,7 @@ extension Server.Builder {
   /// Sets the server error delegate.
   @discardableResult
   public func withErrorDelegate(_ delegate: ServerErrorDelegate?) -> Self {
-    self.errorDelegate = delegate
+    self.configuration.errorDelegate = delegate
     return self
   }
 }
@@ -80,7 +70,7 @@ extension Server.Builder {
   /// times will override any previously set providers.
   @discardableResult
   public func withServiceProviders(_ providers: [CallHandlerProvider]) -> Self {
-    self.providers = providers
+    self.configuration.serviceProviders = providers
     return self
   }
 }
@@ -88,7 +78,7 @@ extension Server.Builder {
 extension Server.Builder {
   @discardableResult
   public func withKeepalive(_ keepalive: ServerConnectionKeepalive) -> Self {
-    self.connectionKeepalive = keepalive
+    self.configuration.connectionKeepalive = keepalive
     return self
   }
 }
@@ -99,7 +89,7 @@ extension Server.Builder {
   /// 5 minutes if not set.
   @discardableResult
   public func withConnectionIdleTimeout(_ timeout: TimeAmount) -> Self {
-    self.connectionIdleTimeout = timeout
+    self.configuration.connectionIdleTimeout = timeout
     return self
   }
 }
@@ -109,7 +99,7 @@ extension Server.Builder {
   /// and any RPCs using compression will not be accepted.
   @discardableResult
   public func withMessageCompression(_ encoding: ServerMessageEncoding) -> Self {
-    self.messageEncoding = encoding
+    self.configuration.messageEncoding = encoding
     return self
   }
 }
@@ -136,7 +126,7 @@ extension Server.Builder {
   /// Sets the HTTP/2 flow control target window size. Defaults to 65,535 if not explicitly set.
   @discardableResult
   public func withHTTPTargetWindowSize(_ httpTargetWindowSize: Int) -> Self {
-    self.httpTargetWindowSize = httpTargetWindowSize
+    self.configuration.httpTargetWindowSize = httpTargetWindowSize
     return self
   }
 }
@@ -147,7 +137,7 @@ extension Server.Builder {
   /// available to service providers via `context`. Defaults to a no-op logger.
   @discardableResult
   public func withLogger(_ logger: Logger) -> Self {
-    self.logger = logger
+    self.configuration.logger = logger
     return self
   }
 }