switch to node-pool from custom client pool

Author: brianc

lib/index.js

@@ -1,17 +1,81 @@
 var EventEmitter = require('events').EventEmitter;
 var Client = require(__dirname+'/client');
 var defaults =  require(__dirname + '/defaults');
-var pool = require(__dirname + "/client-pool").init(Client);
+var genericPool = require('generic-pool');
+
+//cache of existing client pools
+var pools = {};
+
+//returns connect function using supplied client constructor
+var makeConnectFunction = function(ClientConstructor) {
+  return function(config, callback) {
+    var c = config;
+    var cb = callback;
+    //allow for no config to be passed
+    if(typeof c === 'function') {
+      cb = c;
+      c = defaults;
+    }
+    //get unique pool name if using a config object instead of config string
+    var poolName = typeof(c) === 'string' ? c : c.user+c.host+c.port+c.database;
+    var pool = pools[poolName];
+    if(pool) return pool.acquire(cb);
+    var pool = pools[poolName] = genericPool.Pool({
+      name: poolName,
+      create: function(callback) {
+        var client = new Client(c);
+        client.connect();
+        var connectError = function(err) {
+          client.removeListener('connect', connectSuccess);
+          callback(err, null);
+        };
+        var connectSuccess = function() {
+          client.removeListener('error', connectError);
+          callback(null, client);
+        };
+        client.once('connect', connectSuccess);
+        client.once('error', connectError);
+        client.on('drain', function() {
+          pool.release(client);
+        });
+      },
+      destroy: function(client) {
+        client.end();
+      },
+      max: defaults.poolSize
+    });
+    return pool.acquire(cb);
+  }
+}
+
+var end = function() {
+  Object.keys(pools).forEach(function(name) {
+    var pool = pools[name];
+    pool.drain(function() {
+      pool.destroyAllNow();
+    });
+  })
+};
 
 module.exports = {
   Client: Client,
   Connection: require(__dirname + '/connection'),
-  connect: pool.connect,
-  end: pool.end,
+  connect: makeConnectFunction(Client),
+  end: end,
   defaults: defaults
 }
 
+var nativeExport = null;
 //lazy require native module...the c++ may not have been compiled
 module.exports.__defineGetter__("native", function() {
-  return require(__dirname + '/native');
+  if(nativeExport === null) {
+    var NativeClient = require(__dirname + '/native');
+    nativeExport = {
+      Client: NativeClient,
+      connect: makeConnectFunction(NativeClient),
+      end: end,
+      defaults: defaults
+    }
+  }
+  return nativeExport;
 })

lib/native.js

@@ -178,9 +178,4 @@ p.handleReadyForQuery = function() {
 
 var pool = require(__dirname + '/client-pool').init(ctor);
 
-module.exports = {
-  Client: ctor,
-  connect: pool.connect,
-  end: pool.end,
-  defaults: require(__dirname + '/defaults')
-};
+module.exports = ctor;

test/integration/client/api-tests.js

@@ -2,7 +2,7 @@ var helper = require(__dirname + '/../test-helper');
 var pg = require(__dirname + '/../../../lib');
 
 if(helper.args.native) {
-  pg = require(__dirname + '/../../../lib/native')
+  pg = require(__dirname + '/../../../lib').native;
 }
 
 if(helper.args.libpq) {

test/integration/client/transaction-tests.js

@@ -1,10 +1,11 @@
 var helper = require(__dirname + '/test-helper');
 
+var sink = new helper.Sink(2, function() {
+  helper.pg.end();
+});
+
 test('a single connection transaction', function() {
   var connectionString = helper.connectionString();
-  var sink = new helper.Sink(1, function() {
-    helper.pg.end();
-  });
 
   helper.pg.connect(connectionString, assert.calls(function(err, client) {
     assert.isNull(err);
@@ -43,7 +44,6 @@ test('a single connection transaction', function() {
         sink.add();
       }))
     })
-
   }))
 })
 
@@ -68,7 +68,8 @@ test('gh#36', function() {
       if(err) throw err;
       assert.equal(result.rows.length, 1);
     }))
-    client.query("COMMIT")
-    client.on('drain', client.end.bind(client))
+    client.query("COMMIT", function() {
+      sink.add();
+    })
   })
 })

test/integration/client/type-coercion-tests.js

@@ -83,7 +83,7 @@ var valueCount = 0;
 types.forEach(function(type) {
   valueCount += type.values.length;
 })
-sink = new helper.Sink(types.length, function() {
+sink = new helper.Sink(types.length + 1, function() {
   helper.pg.end();
 })
 
@@ -135,6 +135,6 @@ helper.pg.connect(helper.connectionString(), assert.calls(function(err, client)
   client.query('select 7 <> $1 as res;',[null], function(err, res) {
     assert.isNull(err);
     assert.strictEqual(res.rows[0].res, null);
-    client.end();
+    sink.add();
   })
 }))

test/integration/test-helper.js

@@ -2,7 +2,7 @@ var helper = require(__dirname + '/../test-helper');
 
 //TODO would this be better served set at ../test-helper?
 if(helper.args.native) {
-  Client = require(__dirname + '/../../lib/native').Client;
+  Client = require(__dirname + '/../../lib/native');
   helper.pg = helper.pg.native;
 }
 //export parent helper stuffs

test/native/callback-api-tests.js

@@ -1,5 +1,5 @@
 var helper = require(__dirname + "/../test-helper");
-var Client = require(__dirname + "/../../lib/native").Client;
+var Client = require(__dirname + "/../../lib/native");
 var conString = helper.connectionString();
 
 test('fires callback with results', function() {

test/native/connection-tests.js

@@ -1,5 +1,5 @@
 var helper = require(__dirname + "/../test-helper");
-var Client = require(__dirname + "/../../lib/native").Client;
+var Client = require(__dirname + "/../../lib/native");
 
 test('connecting with wrong parameters', function() {
   var con = new Client("user=asldfkj hostaddr=127.0.0.1 port=5432 dbname=asldkfj");

test/native/error-tests.js

@@ -1,5 +1,5 @@
 var helper = require(__dirname + "/../test-helper");
-var Client = require(__dirname + "/../../lib/native").Client;
+var Client = require(__dirname + "/../../lib/native");
 var conString = helper.connectionString();
 
 test('query with non-text as first parameter throws error', function() {

test/native/evented-api-tests.js

@@ -1,5 +1,5 @@
 var helper = require(__dirname + "/../test-helper");
-var Client = require(__dirname + "/../../lib/native").Client;
+var Client = require(__dirname + "/../../lib/native");
 var conString = helper.connectionString();
 
 var setupClient = function() {