Refactor RedisConnection

Motivation:

During proposal review, and while working within the codebase, several issues were identified with how `RedisConnection` was architectured.

Modifications:

- Change implementation of `RedisConnection` in some areas for new logic of internal `ConnectionState`
- Change behavior of logging in a few places
- The initializer for `RedisConnection` is now **internal**
- How users can override the default `ClientBootstrap` for a connection is by passing an instance to the `.connect` static method
- Change unit tests to inherit from a common XCTestCase class that handles creation and cleanup of `RedisConnection` in tests
- Remove Redis namespace enum

Result:

The API for `RedisConnection` should be much simpler, with the implementation being less buggy.

This resolves issues #49, #54, and #57.

Author: Mordil

README.md

@@ -24,26 +24,28 @@ To install **RedisNIO**, just add the package as a dependency in your [**Package
 
 ```swift
 dependencies: [
-    .package(url: "https://github.com/Mordil/swift-redis-nio-client.git", from: "1.0.0-alpha.1")
+    .package(url: "https://github.com/Mordil/swift-redis-nio-client.git", from: "1.0.0-alpha.4")
 ]
 ```
 
 and run the following command: `swift package resolve`
 
 ## :zap: Getting Started
 
-**RedisNIO** is ready to use right after installation.
+**RedisNIO** is quick to use - all you need is an [`EventLoop`](https://apple.github.io/swift-nio/docs/current/NIO/Protocols/EventLoop.html) from **SwiftNIO**.
 
 ```swift
+import NIO
 import RedisNIO
 
-let connection = Redis.makeConnection(
+let eventLoop: EventLoop = ...
+let connection = RedisConnection.connect(
     to: try .init(ipAddress: "127.0.0.1", port: RedisConnection.defaultPort),
-    password: "my_pass"
+    on: eventLoop
 ).wait()
 
 let result = try connection.set("my_key", to: "some value")
-    .flatMap { return connection.get("my_key" }
+    .flatMap { return connection.get("my_key") }
     .wait()
 
 print(result) // Optional("some value")

Sources/RedisNIO/Redis.swift

@@ -12,12 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-import struct Foundation.UUID
-import struct Dispatch.DispatchTime
-import Logging
-import Metrics
 import NIO
-import NIOConcurrencyHelpers
 
 /// An object capable of sending commands and receiving responses.
 ///
@@ -46,144 +41,3 @@ extension RedisClient {
         return self.send(command: command, with: [])
     }
 }
-
-private let loggingKeyID = "RedisConnection"
-
-/// A `RedisClient` implementation that represents an individual connection
-/// to a Redis database instance.
-///
-/// `RedisConnection` comes with logging by default.
-///
-/// See `RedisClient`
-public final class RedisConnection: RedisClient {
-    public static let defaultPort = 6379
-
-    private enum ConnectionState {
-        case open
-        case closed
-    }
-
-    /// See `RedisClient.eventLoop`
-    public var eventLoop: EventLoop { return channel.eventLoop }
-    /// Is the client still connected to Redis?
-    public var isConnected: Bool { return state != .closed }
-    /// Controls the timing behavior of sending commands over this connection. The default is `true`.
-    ///
-    /// When set to `false`, the host will "queue" commands and determine when to send all at once,
-    /// while `true` will force each command to be sent as soon as they are "queued".
-    /// - Note: Setting this to `true` will trigger all "queued" commands to be sent.
-    public var sendCommandsImmediately: Bool {
-        get { return autoflush.load() }
-        set(newValue) {
-            if newValue { channel.flush() }
-            autoflush.store(newValue)
-        }
-    }
-
-    let channel: Channel
-    private var logger: Logger
-
-    private let autoflush = Atomic<Bool>(value: true)
-    private let _stateLock = Lock()
-    private var _state: ConnectionState
-    private var state: ConnectionState {
-        get { return _stateLock.withLock { self._state } }
-        set(newValue) { _stateLock.withLockVoid { self._state = newValue } }
-    }
-
-    deinit {
-        if isConnected {
-            assertionFailure("close() was not called before deinit!")
-            logger.warning("RedisConnection did not properly shutdown before deinit!")
-        }
-    }
-
-    /// Creates a new connection on the provided `Channel`.
-    /// - Important: Call `close()` before deinitializing to properly cleanup resources.
-    /// - Note: This connection will take ownership of the channel.
-    /// - Parameters:
-    ///     - channel: The `Channel` to read and write from.
-    ///     - logger: The `Logger` instance to use for all logging purposes.
-    public init(channel: Channel, logger: Logger = Logger(label: "RedisNIO.RedisConnection")) {
-        self.channel = channel
-        self.logger = logger
-
-        self.logger[metadataKey: loggingKeyID] = "\(UUID())"
-        self.logger.debug("Connection created.")
-        self._state = .open
-        RedisMetrics.activeConnectionCount += 1
-        RedisMetrics.totalConnectionCount.increment()
-    }
-
-    /// Sends a `QUIT` command, then closes the `Channel` this instance was initialized with.
-    ///
-    /// See [https://redis.io/commands/quit](https://redis.io/commands/quit)
-    /// - Returns: An `EventLoopFuture` that resolves when the connection has been closed.
-    @discardableResult
-    public func close() -> EventLoopFuture<Void> {
-        guard isConnected else {
-            logger.notice("Connection received more than one close() request.")
-            return channel.eventLoop.makeSucceededFuture(())
-        }
-
-        let result = send(command: "QUIT")
-            .flatMap { _ in
-                let promise = self.channel.eventLoop.makePromise(of: Void.self)
-                self.channel.close(promise: promise)
-                return promise.futureResult
-            }
-            .map {
-                self.logger.debug("Connection closed.")
-                RedisMetrics.activeConnectionCount -= 1
-            }
-            .recover {
-                self.logger.error("Encountered error during close(): \($0)")
-                self.state = .open
-            }
-
-        // setting it to closed now prevents multiple close() chains, but doesn't stop the QUIT command
-        // if the connection wasn't closed, it's reset in the callback chain
-        state = .closed
-
-        return result
-    }
-
-    /// Sends commands to the Redis instance this connection is tied to.
-    ///
-    /// See `RedisClient.send(command:with:)`
-    ///
-    /// - Note: The timing of when commands are actually sent to Redis are controlled by
-    ///     the `sendCommandsImmediately` property.
-    public func send(
-        command: String,
-        with arguments: [RESPValue]
-    ) -> EventLoopFuture<RESPValue> {
-        guard isConnected else {
-            logger.error("\(RedisNIOError.connectionClosed.localizedDescription)")
-            return channel.eventLoop.makeFailedFuture(RedisNIOError.connectionClosed)
-        }
-
-        var commandParts: [RESPValue] = [.init(bulk: command)]
-        commandParts.append(contentsOf: arguments)
-
-        let promise = channel.eventLoop.makePromise(of: RESPValue.self)
-        let context = RedisCommand(
-            command: .array(commandParts),
-            promise: promise
-        )
-
-        let startTime = DispatchTime.now().uptimeNanoseconds
-        promise.futureResult.whenComplete { result in
-            let duration = DispatchTime.now().uptimeNanoseconds - startTime
-            RedisMetrics.commandRoundTripTime.recordNanoseconds(duration)
-            guard case let .failure(error) = result else { return }
-            self.logger.error("\(error.localizedDescription)")
-        }
-        logger.debug("Sending command \"\(command)\" with \(arguments)")
-
-        guard sendCommandsImmediately else {
-            return channel.write(context).flatMap { promise.futureResult }
-        }
-        return channel.writeAndFlush(context).flatMap { promise.futureResult }
-    }
-}