Added proper testing for CLS, and finalised implementation

cls-test.js

-"use strict";
-
-var Sequelize = require('./index');
-var DataTypes = Sequelize;
-var inflection = require('inflection');
-var Bluebird = require('q');
-
-var Promise = Sequelize.Promise;
-var db, sequelize;
-
-// db = sequelize = new Sequelize('sequelize_test', 'postgres', 'postgres', {
-    // dialect: 'postgres',
-var sequelize = new Sequelize('sequelize_test', 'root', null, {
-    // dialect: 'sqlite',
-    // dialect: 'mariadb',
-    omitNull: true,
-    // logging: console.log,
-    // logging: false,
-    host: '127.0.0.1',
-    define: {
-      // freezeTableName:true,
-      // underscoredAll: true,
-      underscored: false,
-      timestamps: true,
-    }
-});
-
-
-
-
-
-
-var cls = require('continuation-local-storage');
-var sequelize_cls = cls.getNamespace('sequelize');
-
-var User = sequelize.define('User', {});
-// var Project = sequelize.define('project', {});
-
-sequelize.authenticate({
-  // logging: console.log,
-  force: true
-}).then(function() {
-// sequelize.Promise.resolve().then(function () {
-  sequelize.transaction(function () {
-    console.log('inside transaction 1');
-    console.log(sequelize_cls.get('transaction').id);
-
-    return User.findAll();
-  });
-
-  sequelize.transaction(function () {
-    console.log('inside transaction 2');
-    console.log(sequelize_cls.get('transaction').id);
-
-    return User.findAll();
-  });
-  sequelize.Promise.delay(2000).then(function () {
-    console.log('in a totally different context');
-    console.log(!!sequelize_cls.get('transaction'));
-  });
-
-  sequelize.transaction(function () {
-    console.log('inside transaction 3');
-    console.log(sequelize_cls.get('transaction').id);
-
-    return sequelize.Promise.delay(2000).then(function () {
-      console.log('inside transaction 3, very delayed');
-      console.log(sequelize_cls.get('transaction').id);
-    });
-  }).then(function () {
-    console.log('transaction done ')
-    console.log(!!sequelize_cls.get('transaction'));
-  });
-
-
-  sequelize.transaction(function () {
-    console.log('inside transaction 4');
-    console.log(sequelize_cls.get('transaction').id);
-
-    User.findAll();
-
-    return sequelize.Promise.delay(200).then(function () {
-      console.log('inside transaction 4, delayed');
-      console.log(sequelize_cls.get('transaction').id);
-
-      return User.findAll();
-    })
-  })
-
-  // sequelize.authenticate().then(function () {
-  //   sequelize.transaction(function() {
-  //     console.log('inside authenticate, inside transaction')
-  //     console.log(sequelize_cls.get('transaction').id);
-
-  //     return User.findAll();
-  //   })
-  // })
-
-
-}).then(function(user) {
-
-}).catch(function (e) {
-  console.log('catcher!')
-  console.log(e);
-}).done();

lib/dialects/abstract/connection-manager.js

@@ -185,7 +185,6 @@ ConnectionManager.prototype.initPools = function () {
       idleTimeoutMillis: config.pool.idle
       idleTimeoutMillis: config.pool.idle
     });
     });
   }
   }
-
 };
 };
 
 
 ConnectionManager.prototype.getConnection = function(options) {
 ConnectionManager.prototype.getConnection = function(options) {

lib/dialects/mysql/query.js

@@ -40,7 +40,6 @@ module.exports = (function() {
           resolve(self.formatResults(results));
           resolve(self.formatResults(results));
         }
         }
       }).setMaxListeners(100);
       }).setMaxListeners(100);
-
     });
     });
 
 
     return promise;
     return promise;

lib/model.js

@@ -685,10 +685,6 @@ module.exports = (function() {
       hooks: true
       hooks: true
     });
     });
 
 
-    if (!options.transaction && sequelize_cls.get('transaction')) {
-      options.transaction = sequelize_cls.get('transaction');
-    }
-
     return Promise.bind(this).then(function() {
     return Promise.bind(this).then(function() {
       conformOptions(options);
       conformOptions(options);
 
 
@@ -1052,9 +1048,7 @@ module.exports = (function() {
       options = { fields: options };
       options = { fields: options };
     }
     }
 
 
-    options = Utils._.extend({
-      transaction: null
-    }, options || {});
+    options = options || {};
 
 
     return this.build(values, {
     return this.build(values, {
       isNewRecord: true,
       isNewRecord: true,

lib/promise.js

@@ -11,10 +11,6 @@ var Promise = require('bluebird/js/main/promise')() // use this syntax to be abl
     deprecatedSeen[message] = true;
     deprecatedSeen[message] = true;
   };
   };
 
 
-var cls = require('continuation-local-storage')
-  , ns = cls.createNamespace('sequelize');
-
-
 /**
 /**
  * A slightly modified version of bluebird promises. This means that, on top of the methods below, you can also call all the methods listed on the link below.
  * A slightly modified version of bluebird promises. This means that, on top of the methods below, you can also call all the methods listed on the link below.
  *
  *
@@ -43,23 +39,24 @@ var SequelizePromise = function (resolver) {
   return promise;
   return promise;
 };
 };
 
 
+var util = require('util');
+util.inherits(SequelizePromise, Promise);
+
 for (var method in Promise) {
 for (var method in Promise) {
   if (Promise.hasOwnProperty(method)) {
   if (Promise.hasOwnProperty(method)) {
     SequelizePromise[method] = Promise[method];
     SequelizePromise[method] = Promise[method];
   }
   }
 }
 }
 
 
-// var bluebird_addcb = Promise.prototype._addCallbacks;
-// Promise.prototype._addCallbacks = function (fulfill, reject, progress, promise, receiver) {
-//   if (typeof fulfill === 'function') fulfill = ns.bind(fulfill);
-//   if (typeof reject === 'function') reject = ns.bind(reject);
-//   if (typeof progress === 'function') progress = ns.bind(progress);
-
-//   return bluebird_addcb.call(this, fulfill, reject, progress, promise, receiver);
-// };
-
 var bluebirdThen = Promise.prototype._then;
 var bluebirdThen = Promise.prototype._then;
 Promise.prototype._then = function (didFulfill, didReject, didProgress, receiver, internalData) {
 Promise.prototype._then = function (didFulfill, didReject, didProgress, receiver, internalData) {
+  if (SequelizePromise.prototype.sequelize.options.namespace) {
+    var ns = SequelizePromise.prototype.sequelize.options.namespace;
+    if (typeof didFulfill === 'function') didFulfill = ns.bind(didFulfill);
+    if (typeof didReject === 'function') didReject = ns.bind(didReject);
+    if (typeof didProgress === 'function') didProgress = ns.bind(didProgress);
+  }
+
   var ret = bluebirdThen.call(this, didFulfill, didReject, didProgress, receiver, internalData);
   var ret = bluebirdThen.call(this, didFulfill, didReject, didProgress, receiver, internalData);
 
 
   // Needed to transfer sql events accross .then() calls
   // Needed to transfer sql events accross .then() calls

lib/sequelize.js

@@ -199,6 +199,8 @@ module.exports = (function() {
     this.importCache = {};
     this.importCache = {};
 
 
     Sequelize.runHooks('afterInit', this);
     Sequelize.runHooks('afterInit', this);
+
+    Promise.prototype.sequelize = this;
   };
   };
 
 
   Sequelize.options = {hooks: {}};
   Sequelize.options = {hooks: {}};
@@ -660,14 +662,18 @@ module.exports = (function() {
       type: (sql.toLowerCase().indexOf('select') === 0) ? QueryTypes.SELECT : false
       type: (sql.toLowerCase().indexOf('select') === 0) ? QueryTypes.SELECT : false
     });
     });
 
 
+    if (
+      (!options.transaction && options.transaction !== null)
+      && this.options.namespace
+      && this.options.namespace.get('transaction')
+    ) {
+      options.transaction = this.options.namespace.get('transaction');
+    }
+
     if (options.transaction && options.transaction.finished) {
     if (options.transaction && options.transaction.finished) {
       return Promise.reject(options.transaction.finished+' has been called on this transaction, you can no longer use it');
       return Promise.reject(options.transaction.finished+' has been called on this transaction, you can no longer use it');
     }
     }
 
 
-    var cls = require('continuation-local-storage');
-    var sequelize_cls = cls.getNamespace('sequelize');
-    return sequelize_cls.bind(function () {
-      sequelize_cls.set('test', true)
     return Promise.resolve(
     return Promise.resolve(
       options.transaction ? options.transaction.connection : self.connectionManager.getConnection(options)
       options.transaction ? options.transaction.connection : self.connectionManager.getConnection(options)
     ).then(function (connection) {
     ).then(function (connection) {
@@ -677,7 +683,6 @@ module.exports = (function() {
         return self.connectionManager.releaseConnection(connection);
         return self.connectionManager.releaseConnection(connection);
       });
       });
     });
     });
-    })();
   };
   };
 
 
   /**
   /**
@@ -1036,12 +1041,18 @@ module.exports = (function() {
       options = undefined;
       options = undefined;
     }
     }
 
 
-    var transaction = new Transaction(this, options);
+    var transaction = new Transaction(this, options)
+      , ns = this.options.namespace;
 
 
     if (autoCallback) {
     if (autoCallback) {
       deprecated('Note: When passing a callback to a transaction a promise chain is expected in return, the transaction will be committed or rejected based on the promise chain returned to the callback.');
       deprecated('Note: When passing a callback to a transaction a promise chain is expected in return, the transaction will be committed or rejected based on the promise chain returned to the callback.');
-      return new Promise(function (resolve, reject) {
+
+      var transactionResolver = function (resolve, reject) {
         transaction.prepareEnvironment().then(function () {
         transaction.prepareEnvironment().then(function () {
+          if (ns) {
+            autoCallback = ns.bind(autoCallback);
+          }
+
           var result = autoCallback(transaction);
           var result = autoCallback(transaction);
           if (!result) return reject(new Error('You need to return a promise chain to the sequelize.transaction() callback'));
           if (!result) return reject(new Error('You need to return a promise chain to the sequelize.transaction() callback'));
 
 
@@ -1057,10 +1068,33 @@ module.exports = (function() {
             });
             });
           });
           });
         }).catch(reject);
         }).catch(reject);
-      });
+      };
+
+      if (ns) {
+        transactionResolver = ns.bind(transactionResolver, ns.createContext());
+      }
+
+      return new Promise(transactionResolver);
+    } else {
+      if (ns) {
+        var context = ns.createContext();
+
+        return ns.bind(function () {
+          var ret =  transaction.prepareEnvironment().return(transaction);
+
+          ret.then = function (didFulfill, didReject, didProgress) {
+            // We manually pass a context here, because the right context has to be available even though the .then callback is not strictly within the same callback chain
+            didFulfill = ns.bind(didFulfill, context);
+
+            return Sequelize.Promise.prototype.then.call(this, didFulfill, didReject, didProgress);
+          };
+
+          return ret;
+        }, context)();
       } else {
       } else {
         return transaction.prepareEnvironment().return(transaction);
         return transaction.prepareEnvironment().return(transaction);
       }
       }
+    }
   };
   };
 
 
   Sequelize.prototype.log = function() {
   Sequelize.prototype.log = function() {

lib/transaction.js

 'use strict';
 'use strict';
 
 
 var Utils = require('./utils')
 var Utils = require('./utils')
-  , util = require('util')
-  , cls = require('continuation-local-storage');
+  , util = require('util');
 
 
 /**
 /**
  * The transaction object is used to identify a running transaction. It is created by calling `Sequelize.transaction()`.
  * The transaction object is used to identify a running transaction. It is created by calling `Sequelize.transaction()`.
@@ -111,20 +110,18 @@ Transaction.prototype.rollback = function() {
     });
     });
 };
 };
 
 
-var sequelize_cls = cls.getNamespace('sequelize');
 Transaction.prototype.prepareEnvironment = function() {
 Transaction.prototype.prepareEnvironment = function() {
   var self = this;
   var self = this;
 
 
-  return sequelize_cls.bind(function () {
   return Utils.Promise.resolve(
   return Utils.Promise.resolve(
     self.options.transaction ? self.options.transaction.connection : self.sequelize.connectionManager.getConnection({ uuid: self.id })
     self.options.transaction ? self.options.transaction.connection : self.sequelize.connectionManager.getConnection({ uuid: self.id })
   ).then(function (connection) {
   ).then(function (connection) {
     self.connection = connection;
     self.connection = connection;
     self.connection.uuid = self.id;
     self.connection.uuid = self.id;
-      sequelize_cls.set('transaction', self);
-    }).catch(function () {
-      console.log('yes, this is a noop')
-      sequelize_cls.set('transaction', self);
+
+    if (self.sequelize.options.namespace) {
+      self.sequelize.options.namespace.set('transaction', self);
+    }
   }).then(function () {
   }).then(function () {
     return self.begin();
     return self.begin();
   }).then(function () {
   }).then(function () {
@@ -132,7 +129,6 @@ Transaction.prototype.prepareEnvironment = function() {
   }).then(function () {
   }).then(function () {
     return self.setAutocommit();
     return self.setAutocommit();
   });
   });
-  })();
 };
 };
 
 
 Transaction.prototype.begin = function() {
 Transaction.prototype.begin = function() {
@@ -159,7 +155,5 @@ Transaction.prototype.setIsolationLevel = function() {
 Transaction.prototype.cleanup = function() {
 Transaction.prototype.cleanup = function() {
   this.connection.uuid = undefined;
   this.connection.uuid = undefined;
 
 
-  return this.sequelize.connectionManager.releaseConnection(this.connection).tap(function () {
-    cls.getNamespace('sequelize').set('transaction', undefined);
-  });
+  return this.sequelize.connectionManager.releaseConnection(this.connection);
 };
 };

test/cls.test.js

+"use strict";
+
+/* jshint camelcase: false */
+var chai      = require('chai')
+  , sinon     = require('sinon')
+  , expect    = chai.expect
+  , Support   = require(__dirname + '/support')
+  , Sequelize = Support.Sequelize
+  , Promise   = Sequelize.Promise
+  , cls       = require('continuation-local-storage');
+
+chai.config.includeStack = true;
+
+describe(Support.getTestDialectTeaser("Continuation local storage"), function () {
+  before(function () {
+    this.sequelize = Support.createSequelizeInstance({
+      namespace: cls.createNamespace('sequelize')
+    });
+  });
+
+  beforeEach(function () {
+    this.ns = cls.getNamespace('sequelize');
+
+    this.User = this.sequelize.define('user', {
+      name: Sequelize.STRING
+    });
+    return this.sequelize.sync({ force: true });
+  });
+
+  var autoCallback = function autoCallback(sequelize, cb) {
+    return sequelize.transaction(cb);
+  };
+  var thenCallback = function thenCallback(sequelize, cb) {
+    return sequelize.transaction().then(function (t) {
+      cb().then(function () {
+        t.commit();
+      });
+    });
+  };
+
+  [autoCallback, thenCallback].forEach(function (cb) {
+    describe(cb.name, function () {
+      describe('context', function () {
+        it('supports several concurrent transactions', function () {
+          var t1id, t2id, self = this;
+
+          return Promise.join(
+            cb(this.sequelize, function () {
+              t1id = self.ns.get('transaction').id;
+
+              return Promise.resolve();
+            }),
+            cb(this.sequelize, function () {
+              t2id = self.ns.get('transaction').id;
+
+              return Promise.resolve();
+            }),
+            function () {
+              expect(t1id).not.to.equal(t2id);
+            }
+          );
+        });
+
+        it('supports nested promise chains', function () {
+          var self = this;
+
+          return cb(this.sequelize, function () {
+            var tid = self.ns.get('transaction').id;
+
+            return self.User.findAll().then(function () {
+              expect(self.ns.get('transaction').id).to.be.ok;
+              expect(self.ns.get('transaction').id).to.equal(tid);
+            });
+          });
+        });
+
+        it('does not leak variables to the outer scope', function () {
+          // This is a little tricky. We want to check the values in the outer scope, when the transaction has been successfully set up, but before it has been comitted.
+          // We can't just call another function from inside that transaction, since that would transfer the context to that function - exactly what we are trying to prevent;
+
+          var self = this
+            , transactionSetup = false
+            , transactionEnded = false;
+
+          cb(this.sequelize, function () {
+            transactionSetup = true;
+
+            return Promise.delay(500).then(function () {
+              expect(self.ns.get('transaction')).to.be.ok;
+              transactionEnded = true;
+            });
+          });
+
+          // Wait for 500 ms - should be enough time to get everything set up
+          return Promise.delay(400).bind(this).then(function () {
+            expect(transactionSetup).to.be.ok;
+            expect(transactionEnded).not.to.be.ok;
+
+            expect(this.ns.get('transaction')).not.to.be.ok;
+          });
+        });
+
+        it('does not leak variables to the following promise chain', function () {
+          return cb(this.sequelize, function () {
+            return Promise.resolve();
+          }).bind(this).then(function () {
+            expect(this.ns.get('transaction')).not.to.be.ok;
+          });
+        });
+      });
+
+      describe('sequelize.query integration', function () {
+        it('automagically uses the transaction in all calls', function () {
+          var self = this;
+          return cb(this.sequelize, function () {
+            return self.User.create({ name: 'bob' }).then(function () {
+              return Promise.all([
+                expect(self.User.findAll({}, { transaction: null })).to.eventually.have.length(0),
+                expect(self.User.findAll({})).to.eventually.have.length(1)
+              ]);
+            });
+          });
+        });
+      });
+    });
+  });
+});