Merge connection pool cache feature

Author: Christopher Jones

doc/api.md

@@ -159,23 +159,19 @@ function rollback(rollbackCb) {
 rollbackPromisified = nodbUtil.promisify(rollback);
 
 // This release function is used to override the release method of the Connection
-// class, which is defined in the C layer. Currently the main difference is that
-// connections obtained from a pool need to invoke the pool's _onConnectionRelease
-// method so the pool can dequeue the next connection request.
+// class, which is defined in the C layer.
 function release(releaseCb) {
   var self = this;
 
   nodbUtil.assert(arguments.length === 1, 'NJS-009');
   nodbUtil.assert(typeof releaseCb === 'function', 'NJS-006', 1);
 
   self._release(function(err) {
-    releaseCb(err);
-
-    // pool will only exist for connections obtained from a pool.
-    if (self._pool && self._pool.queueRequests !== false) {
-      self._pool._onConnectionRelease();
+    if (!err) {
+      self.emit('_after_close');
     }
 
+    releaseCb(err);
   });
 }
 
@@ -198,6 +194,8 @@ breakPromisified = nodbUtil.promisify(module.break);
 // custom properties and method overrides. References to the original methods are
 // maintained so they can be invoked by the overriding method at the right time.
 function extend(conn, oracledb, pool) {
+  nodbUtil.makeEventEmitter(conn);
+
   // Using Object.defineProperties to add properties to the Connection instance with
   // special properties, such as enumerable but not writable.
   Object.defineProperties(
@@ -206,7 +204,7 @@ function extend(conn, oracledb, pool) {
       _oracledb: { // storing a reference to the base instance to avoid circular references with require
         value: oracledb
       },
-      _pool: {
+      _pool: { // storing a reference to the pool, if any, from which the connection was obtained
         value: pool
       },
       _execute: {

lib/connection.js

@@ -27,6 +27,9 @@ var connection = require('./connection.js');
 var nodbUtil = require('./util.js');
 var createPoolPromisified;
 var getConnectionPromisified;
+var poolCache = {};
+var tempUsedPoolAliases = {};
+var defaultPoolAlias = 'default';
 
 try {
   oracledbCLib =  require('../build/Release/oracledb');
@@ -55,45 +58,163 @@ oracledbCLib.Oracledb.prototype.newLob = function(iLob) {
 // things like extend out the pool instance prior to passing it to the caller.
 function createPool(poolAttrs, createPoolCb) {
   var self = this;
+  var poolAlias;
 
+  // Initial argument count and type checks are done first and throw in the same
+  // call stack.
   nodbUtil.assert(arguments.length === 2, 'NJS-009');
   nodbUtil.assert(nodbUtil.isObject(poolAttrs), 'NJS-006', 1);
   nodbUtil.assert(typeof createPoolCb === 'function', 'NJS-006', 2);
 
+  // Additional validations should pass errors via the callback. Need to ensure
+  // that errors are raised prior to actually creating the pool via _createPool.
+  if (poolAttrs.poolAlias !== undefined) {
+    if (typeof poolAttrs.poolAlias !== 'string' || poolAttrs.poolAlias.length === 0) {
+      createPoolCb(new Error(nodbUtil.getErrorMessage('NJS-004', 'poolAttrs.poolAlias')));
+      return;
+    }
+
+    poolAlias = poolAttrs.poolAlias;
+  } else if (poolAttrs.poolAlias === undefined
+    && !poolCache[defaultPoolAlias]
+    && !tempUsedPoolAliases[defaultPoolAlias]
+  ) {
+    poolAlias = defaultPoolAlias;
+  }
+
+  if (poolCache[poolAlias] || tempUsedPoolAliases[poolAlias]) {
+    createPoolCb(new Error(nodbUtil.getErrorMessage('NJS-046', poolAlias)));
+    return;
+  }
+
+  // Need to prevent another call in the same stack from succeeding, otherwise
+  // two pools could be created with the same poolAlias and the second one that
+  // comes back would overwrite the first in the cache.
+  if (poolAlias) {
+    tempUsedPoolAliases[poolAlias] = true;
+  }
+
   self._createPool(poolAttrs, function(err, poolInst) {
     if (err) {
+      // We need to free this up since the creation of the pool failed.
+      if (poolAlias) {
+        delete tempUsedPoolAliases[poolAlias];
+      }
+
       createPoolCb(err);
+
       return;
     }
 
-    pool.extend(poolInst, poolAttrs, self);
+    if (poolAlias) {
+      poolCache[poolAlias] = poolInst;
+
+      // It's now safe to remove this alias from the tempUsedPoolAliases.
+      delete tempUsedPoolAliases[poolAlias];
+    }
+
+    pool.extend(poolInst, poolAttrs, poolAlias, self);
+
+    poolInst.on('_after_close', function() {
+      var pool = this;
+
+      if (pool.poolAlias) {
+        delete poolCache[pool.poolAlias];
+      }
+    });
 
     createPoolCb(null, poolInst);
   });
 }
 
 createPoolPromisified = nodbUtil.promisify(createPool);
 
+// The getPool function is a synchronous method used to retrieve pools from the
+// pool cache.
+function getPool(poolAlias) {
+  var pool;
+
+  nodbUtil.assert(arguments.length < 2, 'NJS-009');
+
+  if (poolAlias) {
+    nodbUtil.assert(typeof poolAlias === 'string' || typeof poolAlias === 'number', 'NJS-006', 1);
+  }
+
+  poolAlias = poolAlias || defaultPoolAlias;
+
+  pool = poolCache[poolAlias];
+
+  if (!pool) {
+    throw new Error(nodbUtil.getErrorMessage('NJS-047', poolAlias));
+  }
+
+  return pool;
+}
+
 // This getConnection function is used the override the getConnection method of the
 // Oracledb class, which is defined in the C layer. The override allows us to do
 // things like extend out the connection instance prior to passing it to the caller.
-function getConnection(connAttrs, createConnectionCb) {
+function getConnection(a1, a2) {
   var self = this;
+  var pool;
+  var poolAlias;
+  var connAttrs;
+  var getConnectionCb;
 
-  nodbUtil.assert(arguments.length === 2, 'NJS-009');
-  nodbUtil.assert(nodbUtil.isObject(connAttrs), 'NJS-006', 1);
-  nodbUtil.assert(typeof createConnectionCb === 'function', 'NJS-006', 2);
+  nodbUtil.assert(arguments.length < 3, 'NJS-009');
 
-  self._getConnection(connAttrs, function(err, connInst) {
-    if (err) {
-      createConnectionCb(err);
+  // Verify the number and types of arguments, then initialize the local poolAlias,
+  // connAttrs, and getConnectionCb variables based on the arguments.
+  switch (arguments.length) {
+    case 1:
+      nodbUtil.assert(typeof a1 === 'function', 'NJS-006', 1);
+
+      poolAlias = defaultPoolAlias;
+      getConnectionCb = a1;
+
+      break;
+    case 2:
+      nodbUtil.assert(typeof a1 === 'string' || nodbUtil.isObject(a1), 'NJS-006', 1);
+      nodbUtil.assert(typeof a2 === 'function', 'NJS-006', 2);
+
+      if (typeof a1 === 'string') {
+        poolAlias = a1;
+      } else if (nodbUtil.isObject(a1)) {
+        connAttrs = a1;
+
+        if (connAttrs.poolAlias) {
+          poolAlias = connAttrs.poolAlias;
+        }
+      }
+
+      getConnectionCb = a2;
+
+      break;
+  }
+
+  // Proceed to execution based on values in local variables. Look for the poolAlias
+  // first and only attempt to use connAttrs if the poolAlias isn't set.
+  if (poolAlias) {
+    pool = poolCache[poolAlias];
+
+    if (!pool) {
+      getConnectionCb(new Error(nodbUtil.getErrorMessage('NJS-047', poolAlias)));
       return;
     }
 
-    connection.extend(connInst, self);
+    pool.getConnection(getConnectionCb);
+  } else {
+    self._getConnection(connAttrs, function(err, connInst) {
+      if (err) {
+        getConnectionCb(err);
+        return;
+      }
+
+      connection.extend(connInst, self);
 
-    createConnectionCb(null, connInst);
-  });
+      getConnectionCb(null, connInst);
+    });
+  }
 }
 
 getConnectionPromisified = nodbUtil.promisify(getConnection);
@@ -258,6 +379,11 @@ function extend(oracledb) {
         enumerable: true,
         writable: true
       },
+      getPool: {
+        value: getPool,
+        enumerable: true,
+        writable: true
+      },
       _getConnection: {
         value: oracledb.getConnection
       },

lib/oracledb.js

@@ -55,6 +55,12 @@ function completeConnectionRequest(getConnectionCb) {
 
     connection.extend(connInst, self._oracledb, self);
 
+    connInst.on('_after_close', function() {
+      self._connectionsOut -= 1;
+
+      checkRequestQueue.call(self);
+    });
+
     getConnectionCb(null, connInst);
   });
 }
@@ -95,16 +101,6 @@ function checkRequestQueue() {
   completeConnectionRequest.call(self, payload.getConnectionCb);
 }
 
-// onConnectionRelease contains the logic that should be executed when a connection
-// that was obtained from a pool is released back to the pool.
-function onConnectionRelease() {
-  var self = this;
-
-  self._connectionsOut -= 1;
-
-  checkRequestQueue.call(self);
-}
-
 // onRequestTimeout is used to prevent requests for connections from sitting in the
 // queue for too long. The number of milliseconds can be set via queueTimeout
 // property of the poolAttrs used when creating a pool.
@@ -219,6 +215,8 @@ function terminate(terminateCb) {
   self._terminate(function(err) {
     if (!err) {
       self._isValid = false;
+
+      self.emit('_after_close', self);
     }
 
     terminateCb(err);
@@ -265,6 +263,7 @@ function logStats() {
   console.log('...pool connections in use:', self.connectionsInUse);
   console.log('...pool connections open:', self.connectionsOpen);
   console.log('Related pool attributes:');
+  console.log('...poolAlias:', self.poolAlias);
   console.log('...queueRequests:', self.queueRequests);
   console.log('...queueTimeout (milliseconds):', self.queueTimeout);
   console.log('...poolMin:', self.poolMin);
@@ -279,7 +278,7 @@ function logStats() {
 // The extend method is used to extend Pool instances from the C layer with custom
 // properties, methods, and method overrides. References to the original methods are
 // maintained so they can be invoked by the overriding method at the right time.
-function extend(pool, poolAttrs, oracledb) {
+function extend(pool, poolAttrs, poolAlias, oracledb) {
   var queueRequests;
   var queueTimeout;
 
@@ -295,6 +294,8 @@ function extend(pool, poolAttrs, oracledb) {
     queueTimeout = oracledb.queueTimeout;
   }
 
+  nodbUtil.makeEventEmitter(pool);
+
   // Using Object.defineProperties to add properties to the Pool instance with special
   // properties, such as enumerable but not writable.
   Object.defineProperties(
@@ -389,12 +390,18 @@ function extend(pool, poolAttrs, oracledb) {
         value: {},
         writable: true
       },
-      _onConnectionRelease: {
-        value: onConnectionRelease
-      },
       _getConnection: {
         value: pool.getConnection
       },
+      poolAlias: {
+        enumerable: true,
+        get: function() {
+          return poolAlias;
+        },
+        set: function() {
+          throw new Error(nodbUtil.getErrorMessage('NJS-014', 'poolAlias'));
+        }
+      },
       getConnection: {
         value: getConnectionPromisified,
         enumerable: true,

lib/pool.js

@@ -19,10 +19,17 @@
 
 var util = require('util');
 
+var EventEmitter = require('events').EventEmitter;
+var eventEmitterKeys = Object.keys(EventEmitter.prototype);
+var eventEmitterFuncKeys = eventEmitterKeys.filter(function(key) {
+  return typeof EventEmitter.prototype[key] === 'function';
+});
+
 // errorMessages is a temporary duplication of error messages defined in the C
 // layer that will be removed once a function to fetch from the C layer is added.
 var errorMessages = {
   'NJS-002': 'NJS-002: invalid pool',
+  'NJS-004': 'NJS-004: invalid value for property %s',
   'NJS-005': 'NJS-005: invalid value for parameter %d',
   'NJS-006': 'NJS-006: invalid type for parameter %d',
   'NJS-009': 'NJS-009: invalid number of parameters',
@@ -31,10 +38,26 @@ var errorMessages = {
   'NJS-040': 'NJS-040: connection request timeout',
   'NJS-041': 'NJS-041: cannot convert ResultSet to QueryStream after invoking methods',
   'NJS-042': 'NJS-042: cannot invoke ResultSet methods after converting to QueryStream',
-  'NJS-043': "NJS-043: ResultSet already converted to QueryStream",
-  'NJS-045': "NJS-045: cannot load the oracledb add-on binary"
+  'NJS-043': 'NJS-043: ResultSet already converted to QueryStream',
+  'NJS-045': 'NJS-045: cannot load the oracledb add-on binary',
+  'NJS-046': 'NJS-046: poolAlias "%s" already exists in the connection pool cache',
+  'NJS-047': 'NJS-047: poolAlias "%s" not found in the connection pool cache'
 };
 
+// makeEventEmitter is used to make class instances inherit from the EventEmitter
+// class. This is needed because we extend instances from the C layer and thus
+// don't have JavaScript constructor functions we can use for more traditional
+// inheritance.
+function makeEventEmitter(instance){
+  eventEmitterFuncKeys.forEach(function(key) {
+    instance[key] = EventEmitter.prototype[key];
+  });
+
+  EventEmitter.call(instance);
+}
+
+module.exports.makeEventEmitter = makeEventEmitter;
+
 // getErrorMessage is used to get and format error messages to make throwing errors
 // a little more convenient.
 function getErrorMessage(errorCode, messageArg1) {

lib/util.js

@@ -82,6 +82,8 @@ static const char *errMsg[] =
   "NJS-043: ResultSet already converted to QueryStream", // errResultSetAlreadyConverted
   "NJS-044: named JSON object is not expected in this context", // errNamedJSON
   "NJS-045: cannot load the oracledb add-on binary", // errCannotLoadBinary
+  "NJS-046: pool alias \"%s\" already exists in the connection pool cache", // errPoolWithAliasAlreadyExists
+  "NJS-047: pool alias \"%s\" not found in connection pool cache", // errPoolWithAliasNotFound
 };
 
 string NJSMessages::getErrorMsg ( NJSErrorType err, ... )

src/njs/src/njsMessages.cpp

@@ -81,6 +81,8 @@ typedef enum
   errResultSetAlreadyConverted,
   errNamedJSON,
   errCannotLoadBinary,
+  errPoolWithAliasAlreadyExists,
+  errPoolWithAliasNotFound,
 
   // New ones should be added here