Re-use connection from pool after conn.changeUser is used

closes #837
closes #1088

Author: seangarner

Changes.md

@@ -8,6 +8,7 @@ you spot any mistakes.
 
 * Fix a sync callback when sequence enqueue fails #1147
 * Provide static require analysis
+* Re-use connection from pool after `conn.changeUser` is used #837 #1088
 
 ## v2.7.0 (2015-05-27)
 

lib/Pool.js

@@ -74,11 +74,12 @@ Pool.prototype.acquireConnection = function acquireConnection(connection, cb) {
     throw new Error('Connection acquired from wrong pool.');
   }
 
-  var pool = this;
+  var changeUser = this._needsChangeUser(connection);
+  var pool       = this;
 
   this._acquiringConnections.push(connection);
 
-  connection.ping({timeout: this.config.acquireTimeout}, function onPing(err) {
+  function onOperationComplete(err) {
     spliceConnection(pool._acquiringConnections, connection);
 
     if (pool._closed) {
@@ -91,12 +92,26 @@ Pool.prototype.acquireConnection = function acquireConnection(connection, cb) {
       return;
     }
 
+    if (changeUser) {
+      pool.emit('connection', connection);
+    }
+
     cb(null, connection);
-  });
+  }
+
+  if (changeUser) {
+    // restore user back to pool configuration
+    connection.config = this.config.newConnectionConfig();
+    connection.changeUser({timeout: this.config.acquireTimeout}, onOperationComplete);
+  } else {
+    // ping connection
+    connection.ping({timeout: this.config.acquireTimeout}, onOperationComplete);
+  }
 };
 
 Pool.prototype.releaseConnection = function releaseConnection(connection) {
   var cb;
+  var pool = this;
 
   if (this._acquiringConnections.indexOf(connection) !== -1) {
     // connection is being acquired
@@ -108,11 +123,7 @@ Pool.prototype.releaseConnection = function releaseConnection(connection) {
       throw new Error('Connection released to wrong pool');
     }
 
-    if (connection._purge) {
-      // purge connection from pool
-      this._purgeConnection(connection);
-      return;
-    } else if (this._freeConnections.indexOf(connection) !== -1) {
+    if (this._freeConnections.indexOf(connection) !== -1) {
       // connection already in free connection pool
       // this won't catch all double-release cases
       throw new Error('Connection already released');
@@ -218,6 +229,17 @@ Pool.prototype._enqueueCallback = function _enqueueCallback(callback) {
   this.emit('enqueue');
 };
 
+Pool.prototype._needsChangeUser = function _needsChangeUser(connection) {
+  var connConfig = connection.config;
+  var poolConfig = this.config.connectionConfig;
+
+  // check if changeUser values are different
+  return connConfig.user !== poolConfig.user
+    || connConfig.database !== poolConfig.database
+    || connConfig.password !== poolConfig.password
+    || connConfig.charsetNumber !== poolConfig.charsetNumber;
+}
+
 Pool.prototype._purgeConnection = function _purgeConnection(connection, callback) {
   var cb = callback || function () {};
 

lib/PoolConnection.js

@@ -1,14 +1,12 @@
 var inherits = require('util').inherits;
 var Connection = require('./Connection')
-var __changeUser = Connection.prototype.changeUser;
 
 module.exports = PoolConnection;
 inherits(PoolConnection, Connection);
 
 function PoolConnection(pool, options) {
   Connection.call(this, options);
   this._pool  = pool;
-  this._purge = false
 
   // When a fatal error occurs the connection's protocol ends, which will cause
   // the connection to end as well, thus we only need to watch for the end event
@@ -21,14 +19,9 @@ function PoolConnection(pool, options) {
   });
 }
 
-PoolConnection.prototype.changeUser = function changeUser(options, callback) {
-  this._purge = true;
-
-  return __changeUser.apply(this, arguments);
-};
-
 PoolConnection.prototype.release = function release() {
   var pool = this._pool;
+  var connection = this;
 
   if (!pool || pool._closed) {
     return;

test/unit/pool/test-change-user-eject.js renamed to test/unit/pool/test-change-user-restore.js

@@ -19,38 +19,49 @@ server.listen(common.fakeServerPort, function(err) {
     assert.ifError(err);
     assert.ok(conn.threadId === 1 || conn.threadId === 2);
     conn0 = conn;
-    threadId = conn.threadId;
   });
 
   pool.getConnection(function(err, conn) {
     assert.ifError(err);
     assert.ok(conn.threadId === 1 || conn.threadId === 2);
-
-    var threadId = conn.threadId;
+    threadId = conn.threadId;
 
     conn.changeUser({user: 'user_2'}, function(err) {
       assert.ifError(err);
       assert.strictEqual(conn.threadId, threadId);
-      conn.release();
-      conn0.release();
+
+      conn.query('SELECT CURRENT_USER()', function (err, rows) {
+        assert.ifError(err);
+        assert.strictEqual(rows.length, 1);
+        assert.strictEqual(rows[0]['CURRENT_USER()'], 'user_2@localhost');
+        conn.release();
+      });
     });
   });
 
   pool.getConnection(function(err, conn1) {
     assert.ifError(err);
-    assert.strictEqual(conn1.threadId, 3);
+    assert.strictEqual(conn1.threadId, threadId);
 
-    pool.getConnection(function(err, conn2) {
+    conn1.query('SELECT CURRENT_USER()', function (err, rows) {
       assert.ifError(err);
-      assert.strictEqual(conn2.threadId, threadId);
-      conn1.release();
-      conn2.release();
+      assert.strictEqual(rows.length, 1);
+      assert.strictEqual(rows[0]['CURRENT_USER()'], 'user_1@localhost');
 
-      pool.end(function(err) {
+      pool.getConnection(function(err, conn2) {
         assert.ifError(err);
-        assert.strictEqual(closed, 3);
-        server.destroy();
+        assert.ok(conn2.threadId === 1 || conn2.threadId === 2);
+        conn1.release();
+        conn2.release();
+
+        pool.end(function(err) {
+          assert.ifError(err);
+          assert.strictEqual(closed, 2);
+          server.destroy();
+        });
       });
+
+      conn0.release();
     });
   });
 });