Provide 'default' static functions for making client/server configuration (#1198)

Motivation:

The `ClientConnection` and `Server` configuration objects are reasonably
large and contain many deafult initialized fields. Whenever we add a
field we need to add a new initializer and deprecate the old one in
order to not break API. Providing a static 'default' function which only
accepts the required properties allows us to add defaulted values without
doing the deprecate and replace.

Modifications:

- Add `default` methods to `ClientConnection.Configuration` and
  `Server.Configuration`
- Deprecate `init`s.

Result:

Easier to add new fields in the future.

FuzzTesting/Sources/ServerFuzzer/main.swift

@@ -26,7 +26,7 @@ public func test(_ start: UnsafeRawPointer, _ count: Int) -> CInt {
     _ = try? channel.finish()
   }
 
-  let configuration = Server.Configuration(
+  let configuration = Server.Configuration.default(
     target: .unixDomainSocket("/ignored"),
     eventLoopGroup: channel.eventLoop,
     serviceProviders: [EchoProvider()]

Sources/GRPC/ClientConnection.swift

@@ -290,14 +290,14 @@ extension ClientConnection {
 
     /// An error delegate which is called when errors are caught. Provided delegates **must not
     /// maintain a strong reference to this `ClientConnection`**. Doing so will cause a retain
-    /// cycle.
-    public var errorDelegate: ClientErrorDelegate?
+    /// cycle. Defaults to `LoggingClientErrorDelegate`.
+    public var errorDelegate: ClientErrorDelegate? = LoggingClientErrorDelegate.shared
 
-    /// A delegate which is called when the connectivity state is changed.
+    /// A delegate which is called when the connectivity state is changed. Defaults to `nil`.
     public var connectivityStateDelegate: ConnectivityStateDelegate?
 
     /// The `DispatchQueue` on which to call the connectivity state delegate. If a delegate is
-    /// provided but the queue is `nil` then one will be created by gRPC.
+    /// provided but the queue is `nil` then one will be created by gRPC. Defaults to `nil`.
     public var connectivityStateDelegateQueue: DispatchQueue?
 
     /// TLS configuration for this connection. `nil` if TLS is not desired.
@@ -305,23 +305,27 @@ extension ClientConnection {
 
     /// The connection backoff configuration. If no connection retrying is required then this should
     /// be `nil`.
-    public var connectionBackoff: ConnectionBackoff?
+    public var connectionBackoff: ConnectionBackoff? = ConnectionBackoff()
 
     /// The connection keepalive configuration.
-    public var connectionKeepalive: ClientConnectionKeepalive
+    public var connectionKeepalive = ClientConnectionKeepalive()
 
     /// The amount of time to wait before closing the connection. The idle timeout will start only
     /// if there are no RPCs in progress and will be cancelled as soon as any RPCs start.
     ///
     /// If a connection becomes idle, starting a new RPC will automatically create a new connection.
-    public var connectionIdleTimeout: TimeAmount
+    ///
+    /// Defaults to 30 minutes.
+    public var connectionIdleTimeout: TimeAmount = .minutes(30)
 
     /// The behavior used to determine when an RPC should start. That is, whether it should wait for
     /// an active connection or fail quickly if no connection is currently available.
-    public var callStartBehavior: CallStartBehavior
+    ///
+    /// Defaults to `waitsForConnectivity`.
+    public var callStartBehavior: CallStartBehavior = .waitsForConnectivity
 
-    /// The HTTP/2 flow control target window size.
-    public var httpTargetWindowSize: Int
+    /// The HTTP/2 flow control target window size. Defaults to 65535.
+    public var httpTargetWindowSize = 65535
 
     /// The HTTP protocol used for this connection.
     public var httpProtocol: HTTP2FramePayloadToHTTP1ClientCodec.HTTPProtocol {
@@ -332,7 +336,10 @@ extension ClientConnection {
     /// requests may be provided in the `CallOptions`.
     ///
     /// Defaults to a no-op logger.
-    public var backgroundActivityLogger: Logger
+    public var backgroundActivityLogger = Logger(
+      label: "io.grpc",
+      factory: { _ in SwiftLogNoOpLogHandler() }
+    )
 
     /// A channel initializer which will be run after gRPC has initialized each channel. This may be
     /// used to add additional handlers to the pipeline and is intended for debugging.
@@ -362,6 +369,7 @@ extension ClientConnection {
     ///     connectivity state). Defaults to a no-op logger.
     /// - Parameter debugChannelInitializer: A channel initializer will be called after gRPC has
     ///     initialized the channel. Defaults to `nil`.
+    @available(*, deprecated, renamed: "default(target:eventLoopGroup:)")
     public init(
       target: ConnectionTarget,
       eventLoopGroup: EventLoopGroup,
@@ -394,6 +402,25 @@ extension ClientConnection {
       self.backgroundActivityLogger = backgroundActivityLogger
       self.debugChannelInitializer = debugChannelInitializer
     }
+
+    private init(eventLoopGroup: EventLoopGroup, target: ConnectionTarget) {
+      self.eventLoopGroup = eventLoopGroup
+      self.target = target
+    }
+
+    /// Make a new configuration using default values.
+    ///
+    /// - Parameters:
+    ///   - target: The target to connect to.
+    ///   - eventLoopGroup: The `EventLoopGroup` providing an `EventLoop` for the connection to
+    ///       run on.
+    /// - Returns: A configuration with default values set.
+    public static func `default`(
+      target: ConnectionTarget,
+      eventLoopGroup: EventLoopGroup
+    ) -> Configuration {
+      return .init(eventLoopGroup: eventLoopGroup, target: target)
+    }
   }
 }
 

Sources/GRPC/ClientErrorDelegate.swift

@@ -42,7 +42,10 @@ extension ClientErrorDelegate {
 }
 
 /// A `ClientErrorDelegate` which logs errors.
-public class LoggingClientErrorDelegate: ClientErrorDelegate {
+public final class LoggingClientErrorDelegate: ClientErrorDelegate {
+  /// A shared instance of this class.
+  public static let shared = LoggingClientErrorDelegate()
+
   public init() {}
 
   public func didCatchError(_ error: Error, logger: Logger, file: StaticString, line: Int) {

Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift

@@ -41,7 +41,7 @@ extension ClientConnection {
     fileprivate init(group: EventLoopGroup) {
       // 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)
+      self.configuration = .default(target: .hostAndPort("", .max), eventLoopGroup: group)
     }
 
     public func connect(host: String, port: Int) -> ClientConnection {

Sources/GRPC/Server.swift

@@ -258,11 +258,11 @@ extension Server {
     public var tls: TLS?
 
     /// The connection keepalive configuration.
-    public var connectionKeepalive: ServerConnectionKeepalive
+    public var connectionKeepalive = ServerConnectionKeepalive()
 
     /// The amount of time to wait before closing connections. The idle timeout will start only
     /// if there are no RPCs in progress and will be cancelled as soon as any RPCs start.
-    public var connectionIdleTimeout: TimeAmount
+    public var connectionIdleTimeout: TimeAmount = .nanoseconds(.max)
 
     /// The compression configuration for requests and responses.
     ///
@@ -272,15 +272,17 @@ extension Server {
     /// Compression may also be disabled at the message-level for streaming responses (i.e. server
     /// streaming and bidirectional streaming RPCs) by passing setting `compression` to `.disabled`
     /// in `sendResponse(_:compression)`.
-    public var messageEncoding: ServerMessageEncoding
+    ///
+    /// Defaults to `.disabled`.
+    public var messageEncoding: ServerMessageEncoding = .disabled
 
-    /// The HTTP/2 flow control target window size.
-    public var httpTargetWindowSize: Int
+    /// The HTTP/2 flow control target window size. Defaults to 65535.
+    public var httpTargetWindowSize: Int = 65535
 
     /// The root server logger. Accepted connections will branch from this logger and RPCs on
     /// each connection will use a logger branched from the connections logger. This logger is made
     /// available to service providers via `context`. Defaults to a no-op logger.
-    public var logger: Logger
+    public var logger = Logger(label: "io.grpc", factory: { _ in SwiftLogNoOpLogHandler() })
 
     /// A channel initializer which will be run after gRPC has initialized each accepted channel.
     /// This may be used to add additional handlers to the pipeline and is intended for debugging.
@@ -313,6 +315,7 @@ extension Server {
     ///   - logger: A logger. Defaults to a no-op logger.
     ///   - debugChannelInitializer: A channel initializer which will be called for each connection
     ///     the server accepts after gRPC has initialized the channel. Defaults to `nil`.
+    @available(*, deprecated, renamed: "default(target:eventLoopGroup:serviceProviders:)")
     public init(
       target: BindTarget,
       eventLoopGroup: EventLoopGroup,
@@ -342,6 +345,38 @@ extension Server {
       self.logger = logger
       self.debugChannelInitializer = debugChannelInitializer
     }
+
+    private init(
+      eventLoopGroup: EventLoopGroup,
+      target: BindTarget,
+      serviceProviders: [CallHandlerProvider]
+    ) {
+      self.eventLoopGroup = eventLoopGroup
+      self.target = target
+      self.serviceProvidersByName = Dictionary(uniqueKeysWithValues: serviceProviders.map {
+        ($0.serviceName, $0)
+      })
+    }
+
+    /// Make a new configuration using default values.
+    ///
+    /// - Parameters:
+    ///   - target: The target to bind to.
+    ///   - eventLoopGroup: The `EventLoopGroup` the server should run on.
+    ///   - serviceProviders: An array of `CallHandlerProvider`s which the server should use
+    ///       to handle requests.
+    /// - Returns: A configuration with default values set.
+    public static func `default`(
+      target: BindTarget,
+      eventLoopGroup: EventLoopGroup,
+      serviceProviders: [CallHandlerProvider]
+    ) -> Configuration {
+      return .init(
+        eventLoopGroup: eventLoopGroup,
+        target: target,
+        serviceProviders: serviceProviders
+      )
+    }
   }
 }
 

Sources/GRPC/ServerBuilder.swift

@@ -23,7 +23,7 @@ extension Server {
     private var maybeTLS: Server.Configuration.TLS? { return nil }
 
     fileprivate init(group: EventLoopGroup) {
-      self.configuration = Configuration(
+      self.configuration = .default(
         // This is okay: the configuration is only consumed on a call to `bind` which sets the host
         // and port.
         target: .hostAndPort("", .max),

Tests/GRPCTests/ClientTLSFailureTests.swift

@@ -71,14 +71,17 @@ class ClientTLSFailureTests: GRPCTestCase {
   func makeClientConfiguration(
     tls: ClientConnection.Configuration.TLS
   ) -> ClientConnection.Configuration {
-    return .init(
+    var configuration = ClientConnection.Configuration.default(
       target: .hostAndPort("localhost", self.port),
-      eventLoopGroup: self.clientEventLoopGroup,
-      tls: tls,
-      // No need to retry connecting.
-      connectionBackoff: nil,
-      backgroundActivityLogger: self.clientLogger
+      eventLoopGroup: self.clientEventLoopGroup
     )
+
+    configuration.tls = tls
+    // No need to retry connecting.
+    configuration.connectionBackoff = nil
+    configuration.backgroundActivityLogger = self.clientLogger
+
+    return configuration
   }
 
   func makeClientConnectionExpectation() -> XCTestExpectation {

Tests/GRPCTests/ConnectionManagerTests.swift

@@ -26,13 +26,15 @@ class ConnectionManagerTests: GRPCTestCase {
   private var monitor: ConnectivityStateMonitor!
 
   private var defaultConfiguration: ClientConnection.Configuration {
-    return ClientConnection.Configuration(
+    var configuration = ClientConnection.Configuration.default(
       target: .unixDomainSocket("/ignored"),
-      eventLoopGroup: self.loop,
-      connectivityStateDelegate: nil,
-      connectionBackoff: nil,
-      backgroundActivityLogger: self.clientLogger
+      eventLoopGroup: self.loop
     )
+
+    configuration.connectionBackoff = nil
+    configuration.backgroundActivityLogger = self.clientLogger
+
+    return configuration
   }
 
   override func setUp() {

Tests/GRPCTests/GRPCServerPipelineConfiguratorTests.swift

@@ -46,13 +46,14 @@ class GRPCServerPipelineConfiguratorTests: GRPCTestCase {
   private func setUp(tls: Bool, requireALPN: Bool = true) {
     self.channel = EmbeddedChannel()
 
-    var configuration = Server.Configuration(
+    var configuration = Server.Configuration.default(
       target: .unixDomainSocket("/ignored"),
       eventLoopGroup: self.channel.eventLoop,
-      serviceProviders: [],
-      logger: self.serverLogger
+      serviceProviders: []
     )
 
+    configuration.logger = self.serverLogger
+
     if tls {
       configuration.tls = .init(
         certificateChain: [],

Tests/GRPCTests/ServerFuzzingRegressionTests.swift

@@ -34,7 +34,7 @@ final class ServerFuzzingRegressionTests: GRPCTestCase {
       _ = try? channel.finish()
     }
 
-    let configuration = Server.Configuration(
+    let configuration = Server.Configuration.default(
       target: .unixDomainSocket("/ignored"),
       eventLoopGroup: channel.eventLoop,
       serviceProviders: [EchoProvider()]

Tests/GRPCTests/ServerTLSErrorTests.swift

@@ -53,13 +53,16 @@ class ServerTLSErrorTests: GRPCTestCase {
     tls: ClientConnection.Configuration.TLS,
     port: Int
   ) -> ClientConnection.Configuration {
-    return .init(
+    var configuration = ClientConnection.Configuration.default(
       target: .hostAndPort("localhost", port),
-      eventLoopGroup: self.clientEventLoopGroup,
-      tls: tls,
-      // No need to retry connecting.
-      connectionBackoff: nil
+      eventLoopGroup: self.clientEventLoopGroup
     )
+
+    configuration.tls = tls
+    // No need to retry connecting.
+    configuration.connectionBackoff = nil
+
+    return configuration
   }
 
   func makeClientConnectionExpectation() -> XCTestExpectation {