proposal feedback (#26)

* add swift log / metrics

* remove PostgresConnection.requestTLS

* make error code's camelCase

* remove tableoid from PostgresRow

* update PostgresRequest protocol

* remove test prints

* only run perf tests in release mode

* rebuild test linuxmain

* add psql db to perf tests

* bump

Author: tanner0101

Package.swift

@@ -10,10 +10,12 @@ let package = Package(
     dependencies: [
         .package(url: "https://github.com/apple/swift-nio.git", from: "2.0.0"),
         .package(url: "https://github.com/apple/swift-nio-ssl.git", from: "2.0.0"),
+        .package(url: "https://github.com/apple/swift-metrics.git", from: "1.0.0-a"),
+        .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"),
     ],
     targets: [
         .target(name: "CMD5", dependencies: []),
-        .target(name: "NIOPostgres", dependencies: ["CMD5", "NIO", "NIOSSL"]),
+        .target(name: "NIOPostgres", dependencies: ["CMD5", "Logging", "Metrics", "NIO", "NIOSSL"]),
         .target(name: "NIOPostgresBenchmark", dependencies: ["NIOPostgres"]),
         .testTarget(name: "NIOPostgresTests", dependencies: ["NIOPostgres"]),
     ]

…/Connection/PostgresDatabase+Query.swift …es/Connection/PostgresClient+Query.swiftSources/NIOPostgres/Connection/PostgresDatabase+Query.swift renamed to Sources/NIOPostgres/Connection/PostgresClient+Query.swift Sources/NIOPostgres/Connection/PostgresDatabase+Query.swift renamed to Sources/NIOPostgres/Connection/PostgresClient+Query.swift

@@ -1,4 +1,5 @@
 import NIO
+import Logging
 
 extension PostgresClient {
     public func query(_ string: String, _ binds: [PostgresData] = []) -> EventLoopFuture<[PostgresRow]> {
@@ -14,7 +15,7 @@ extension PostgresClient {
 
 // MARK: Private
 
-private final class PostgresParameterizedQuery: PostgresRequestHandler {
+private final class PostgresParameterizedQuery: PostgresRequest {
     let query: String
     let binds: [PostgresData]
     var onRow: (PostgresRow) throws -> ()
@@ -32,7 +33,14 @@ private final class PostgresParameterizedQuery: PostgresRequestHandler {
         self.resultFormatCodes = [.binary]
     }
 
+    func log(to logger: Logger) {
+        logger.debug("\(self.query) \(self.binds)")
+    }
+    
     func respond(to message: PostgresMessage) throws -> [PostgresMessage]? {
+        if case .error = message.identifier {
+            return nil
+        }
         switch message.identifier {
         case .bindComplete:
             return []
@@ -57,6 +65,8 @@ private final class PostgresParameterizedQuery: PostgresRequestHandler {
             return []
         case .commandComplete:
             return []
+        case .notice:
+            return []
         case .readyForQuery:
             return nil
         default: throw PostgresError.protocol("Unexpected message during query: \(message)")

…ction/PostgresDatabase+SimpleQuery.swift …nection/PostgresClient+SimpleQuery.swiftSources/NIOPostgres/Connection/PostgresDatabase+SimpleQuery.swift renamed to Sources/NIOPostgres/Connection/PostgresClient+SimpleQuery.swift Sources/NIOPostgres/Connection/PostgresDatabase+SimpleQuery.swift renamed to Sources/NIOPostgres/Connection/PostgresClient+SimpleQuery.swift

@@ -1,4 +1,5 @@
 import NIO
+import Logging
 
 extension PostgresClient {
     public func simpleQuery(_ string: String) -> EventLoopFuture<[PostgresRow]> {
@@ -14,7 +15,7 @@ extension PostgresClient {
 
 // MARK: Private
 
-private final class PostgresSimpleQuery: PostgresRequestHandler {
+private final class PostgresSimpleQuery: PostgresRequest {
     var query: String
     var onRow: (PostgresRow) throws -> ()
     var rowLookupTable: PostgresRow.LookupTable?
@@ -24,7 +25,14 @@ private final class PostgresSimpleQuery: PostgresRequestHandler {
         self.onRow = onRow
     }
 
+    func log(to logger: Logger) {
+        logger.debug("\(self.query)")
+    }
+    
     func respond(to message: PostgresMessage) throws -> [PostgresMessage]? {
+        if case .error = message.identifier {
+            return nil
+        }
         switch message.identifier {
         case .dataRow:
             let data = try PostgresMessage.DataRow(message: message)
@@ -43,6 +51,8 @@ private final class PostgresSimpleQuery: PostgresRequestHandler {
             return []
         case .readyForQuery:
             return nil
+        case .notice:
+            return []
         default:
             throw PostgresError.protocol("Unexpected message during simple query: \(message)")
         }

Sources/NIOPostgres/Connection/PostgresClient.swift

@@ -1,7 +1,4 @@
-#warning("TODO: make this protocol more general, not depend on PostgresRequestHandler")
 public protocol PostgresClient {
     var eventLoop: EventLoop { get }
-    func send(_ request: PostgresRequestHandler) -> EventLoopFuture<Void>
+    func send(_ request: PostgresRequest) -> EventLoopFuture<Void>
 }
-
-extension PostgresConnection: PostgresClient { }

Sources/NIOPostgres/Connection/PostgresConnection+Authenticate.swift

@@ -1,5 +1,6 @@
 import CMD5
 import NIO
+import Logging
 
 extension PostgresConnection {
     public func authenticate(username: String, database: String? = nil, password: String? = nil) -> EventLoopFuture<Void> {
@@ -14,7 +15,7 @@ extension PostgresConnection {
 
 // MARK: Private
 
-private final class PostgresAuthenticationRequest: PostgresRequestHandler {
+private final class PostgresAuthenticationRequest: PostgresRequest {
     enum State {
         case ready
         case done
@@ -25,18 +26,23 @@ private final class PostgresAuthenticationRequest: PostgresRequestHandler {
     let password: String?
     var state: State
 
-    var errorMessageIsFinal: Bool {
-        return true
-    }
-
     init(username: String, database: String?, password: String?) {
         self.state = .ready
         self.username = username
         self.database = database
         self.password = password
     }
 
+    func log(to logger: Logger) {
+        logger.debug("Logging into Postgres db \(self.database ?? "nil") as \(self.username)")
+    }
+    
     func respond(to message: PostgresMessage) throws -> [PostgresMessage]? {
+        if case .error = message.identifier {
+            // terminate immediately on error
+            return nil
+        }
+        
         switch self.state {
         case .ready:
             switch message.identifier {

Sources/NIOPostgres/Connection/PostgresConnection+Connect.swift

@@ -1,18 +1,49 @@
+import Logging
 import NIO
 
 extension PostgresConnection {
-    public static func connect(to socketAddress: SocketAddress, on eventLoop: EventLoop) -> EventLoopFuture<PostgresConnection> {
+    public static func connect(
+        to socketAddress: SocketAddress,
+        tlsConfiguration: TLSConfiguration? = nil,
+        serverHostname: String? = nil,
+        on eventLoop: EventLoop
+    ) -> EventLoopFuture<PostgresConnection> {
         let bootstrap = ClientBootstrap(group: eventLoop)
             .channelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
 
+        let logger = Logger(label: "codes.vapor.nio-postgres")
         return bootstrap.connect(to: socketAddress).flatMap { channel in
             return channel.pipeline.addHandlers([
                 ByteToMessageHandler(PostgresMessageDecoder()),
                 MessageToByteHandler(PostgresMessageEncoder()),
-                PostgresConnectionHandler(),
+                PostgresRequestHandler(logger: logger),
+                PostgresErrorHandler(logger: logger)
             ]).map {
-                return .init(channel: channel)
+                return PostgresConnection(channel: channel, logger: logger)
+            }
+        }.flatMap { conn in
+            if let tlsConfiguration = tlsConfiguration {
+                return conn.requestTLS(using: tlsConfiguration, serverHostname: serverHostname)
+                    .map { conn }
+            } else {
+                return eventLoop.makeSucceededFuture(conn)
             }
         }
     }
 }
+
+
+private final class PostgresErrorHandler: ChannelInboundHandler {
+    typealias InboundIn = Never
+    
+    let logger: Logger
+    init(logger: Logger) {
+        self.logger = logger
+    }
+    
+    func errorCaught(context: ChannelHandlerContext, error: Error) {
+        self.logger.error("Uncaught error: \(error)")
+        context.close(promise: nil)
+        context.fireErrorCaught(error)
+    }
+}

Sources/NIOPostgres/Connection/PostgresConnection+RequestTLS.swift

@@ -1,28 +1,33 @@
 import NIOSSL
+import Logging
 
 extension PostgresConnection {
-    public func requestTLS(using tlsConfig: TLSConfiguration, serverHostname: String?) -> EventLoopFuture<Bool> {
+    internal func requestTLS(using tlsConfig: TLSConfiguration, serverHostname: String?) -> EventLoopFuture<Void> {
         let tls = RequestTLSQuery()
         return self.send(tls).flatMapThrowing { _ in
-            if tls.isSupported {
-                let sslContext = try NIOSSLContext(configuration: tlsConfig)
-                let handler = try NIOSSLClientHandler(context: sslContext, serverHostname: serverHostname)
-                _ = self.channel.pipeline.addHandler(handler, position: .first)
+            guard tls.isSupported else {
+                throw PostgresError.protocol("Server does not support TLS")
             }
-            return tls.isSupported
+            let sslContext = try NIOSSLContext(configuration: tlsConfig)
+            let handler = try NIOSSLClientHandler(context: sslContext, serverHostname: serverHostname)
+            _ = self.channel.pipeline.addHandler(handler, position: .first)
         }
     }
 }
 
 // MARK: Private
 
-private final class RequestTLSQuery: PostgresRequestHandler {
+private final class RequestTLSQuery: PostgresRequest {
     var isSupported: Bool
 
     init() {
         self.isSupported = false
     }
 
+    func log(to logger: Logger) {
+        logger.debug("Requesting TLS")
+    }
+    
     func respond(to message: PostgresMessage) throws -> [PostgresMessage]? {
         switch message.identifier {
         case .sslSupported:

Sources/NIOPostgres/Connection/PostgresConnection.swift

@@ -1,9 +1,7 @@
 import Foundation
+import Logging
+
 public final class PostgresConnection {
-//    #warning("publicize these values?")
-//    public var status: [String: String]
-//    var processID: Int32?
-//    var secretKey: Int32?
     let channel: Channel
 
     public var eventLoop: EventLoop {
@@ -14,18 +12,20 @@ public final class PostgresConnection {
         return channel.closeFuture
     }
 
-    init(channel: Channel) {
+    public var logger: Logger
+    
+    init(channel: Channel, logger: Logger) {
         self.channel = channel
+        self.logger = logger
     }
 
     public func close() -> EventLoopFuture<Void> {
         guard self.channel.isActive else {
             return self.eventLoop.makeSucceededFuture(())
         }
-        return self.channel.close(mode: .all)
+        return self.channel.close()
     }
 
-    #warning("TODO: add error handler that closes connection")
     deinit {
         if self.channel.isActive {
             assertionFailure("PostgresConnection deinitialized before being closed.")