fix(transaction): fix issue introduced in commit 7801903d where child transaction…

…s would not properly use a parent connection

lib/dialects/abstract/query-generator.js

@@ -1705,8 +1705,8 @@ var QueryGenerator = {
    * @param  {Object} options An object with options.
    * @return {String}         The generated sql query.
    */
-  startTransactionQuery: function(transaction, options) {
-    if (options.parent) {
+  startTransactionQuery: function(transaction) {
+    if (transaction.parent) {
       return 'SAVEPOINT ' + this.quoteIdentifier(transaction.name) + ';';
     }
 
@@ -1732,8 +1732,8 @@ var QueryGenerator = {
    * @param  {Object} options An object with options.
    * @return {String}         The generated sql query.
    */
-  commitTransactionQuery: function(options) {
-    if (options.parent) {
+  commitTransactionQuery: function(transaction) {
+    if (transaction.parent) {
       return;
     }
 
@@ -1747,8 +1747,8 @@ var QueryGenerator = {
    * @param  {Object} options An object with options.
    * @return {String}         The generated sql query.
    */
-  rollbackTransactionQuery: function(transaction, options) {
-    if (options.parent) {
+  rollbackTransactionQuery: function(transaction) {
+    if (transaction.parent) {
       return 'ROLLBACK TO SAVEPOINT ' + this.quoteIdentifier(transaction.name) + ';';
     }
 

lib/model.js

@@ -1919,7 +1919,7 @@ Model.prototype.findOrCreate = function(options) {
     });
   }).finally(function () {
     if (internalTransaction && transaction) {
-      // If we created a transaction internally, we should clean it up
+      // If we created a transaction internally (and not just a savepoint), we should clean it up
       return transaction.commit();
     }
   });

lib/query-interface.js

@@ -821,37 +821,44 @@ QueryInterface.prototype.setAutocommit = function(transaction, value, options) {
   if (!transaction || !(transaction instanceof Transaction)) {
     throw new Error('Unable to set autocommit for a transaction without transaction object!');
   }
+  if (transaction.parent) {
+    // Not possible to set a seperate isolation level for savepoints
+    return Promise.resolve();
+  }
 
   options = _.assign({}, options || {}, {
-    transaction: transaction,
-    parent: options.transaction
+    transaction: transaction.parent || transaction
   });
 
-  var sql = this.QueryGenerator.setAutocommitQuery(value, options);
-  if (sql) {
-    return this.sequelize.query(sql, options);
-  } else {
-    return Promise.resolve();
-  }
+  var sql = this.QueryGenerator.setAutocommitQuery(value, {
+    parent: transaction.parent
+  });
+
+  if (!sql) return Promise.resolve();
+
+  return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.setIsolationLevel = function(transaction, value, options) {
   if (!transaction || !(transaction instanceof Transaction)) {
     throw new Error('Unable to set isolation level for a transaction without transaction object!');
   }
+  if (transaction.parent) {
+    // Not possible to set a seperate isolation level for savepoints
+    return Promise.resolve();
+  }
 
   options = _.assign({}, options || {}, {
-    transaction: transaction,
-    parent: options.transaction
+    transaction: transaction.parent || transaction
   });
 
-  var sql = this.QueryGenerator.setIsolationLevelQuery(value, options);
+  var sql = this.QueryGenerator.setIsolationLevelQuery(value, {
+    parent: transaction.parent
+  });
 
-  if (sql) {
-    return this.sequelize.query(sql, options);
-  } else {
-    return Promise.resolve();
-  }
+  if (!sql) return Promise.resolve();
+
+  return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.startTransaction = function(transaction, options) {
@@ -860,21 +867,22 @@ QueryInterface.prototype.startTransaction = function(transaction, options) {
   }
 
   options = _.assign({}, options || {}, {
-    transaction: transaction,
-    parent: options.transaction
+    transaction: transaction.parent || transaction
   });
 
-  var sql = this.QueryGenerator.startTransactionQuery(transaction, options);
+  var sql = this.QueryGenerator.startTransactionQuery(transaction);
+
   return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.deferConstraints = function (transaction, options) {
   options = Utils._.extend({
-    transaction: transaction,
-    parent: options.transaction
+    transaction: transaction.parent || transaction
   }, options || {});
 
-  var sql = this.QueryGenerator.deferConstraintsQuery(options);
+  var sql = this.QueryGenerator.deferConstraintsQuery({
+    parent: transaction.parent
+  });
 
   if (sql) {
     return this.sequelize.query(sql, options);
@@ -887,21 +895,19 @@ QueryInterface.prototype.commitTransaction = function(transaction, options) {
   if (!transaction || !(transaction instanceof Transaction)) {
     throw new Error('Unable to commit a transaction without transaction object!');
   }
-
+  if (transaction.parent) {
+    // Savepoints cannot be committed
+    return Promise.resolve();
+  }
 
   options = _.assign({}, options || {}, {
-    transaction: transaction,
-    parent: options.transaction,
+    transaction: transaction.parent || transaction,
     supportsSearchPath: false
   });
 
-  var sql = this.QueryGenerator.commitTransactionQuery(options);
+  var sql = this.QueryGenerator.commitTransactionQuery(transaction);
 
-  if (sql) {
-    return this.sequelize.query(sql, options);
-  } else {
-    return Promise.resolve();
-  }
+  return this.sequelize.query(sql, options);
 };
 
 QueryInterface.prototype.rollbackTransaction = function(transaction, options) {
@@ -910,12 +916,12 @@ QueryInterface.prototype.rollbackTransaction = function(transaction, options) {
   }
 
   options = _.assign({}, options || {}, {
-    transaction: transaction,
-    parent: options.transaction,
+    transaction: transaction.parent || transaction,
     supportsSearchPath: false
   });
 
-  var sql = this.QueryGenerator.rollbackTransactionQuery(transaction, options);
+  var sql = this.QueryGenerator.rollbackTransactionQuery(transaction);
+
   return this.sequelize.query(sql, options);
 };
 

lib/sequelize.js

@@ -789,6 +789,7 @@ Sequelize.prototype.query = function(sql, options) {
   return Promise.resolve(
     options.transaction ? options.transaction.connection : self.connectionManager.getConnection(options)
   ).then(function (connection) {
+
     var query = new self.dialect.Query(connection, self, options);
     return query.run(sql, bindParameters).finally(function() {
       if (options.transaction) return;

lib/transaction.js

@@ -23,13 +23,14 @@ var Transaction = module.exports = function(sequelize, options) {
     autocommit: true,
     isolationLevel: sequelize.options.isolationLevel
   }, options || {});
-  this.id = this.options.transaction ? this.options.transaction.id : uuid.v4();
 
-  if (this.options.transaction) {
-    this.id = this.options.transaction.id;
-    this.options.transaction.savepoints.push(this);
-    this.name = this.id + '-savepoint-' + this.options.transaction.savepoints.length;
-    this.parent = this.options.transaction;
+  this.parent = this.options.transaction
+  this.id = this.parent ? this.parent.id : uuid.v4();
+
+  if (this.parent) {
+    this.id = this.parent.id;
+    this.parent.savepoints.push(this);
+    this.name = this.id + '-savepoint-' + this.parent.savepoints.length;
   } else {
     this.id = this.name = uuid.v4();
   }
@@ -178,7 +179,7 @@ Transaction.prototype.prepareEnvironment = function() {
   var self = this;
 
   return Utils.Promise.resolve(
-    self.options.transaction ? self.options.transaction.connection : self.sequelize.connectionManager.getConnection({ uuid: self.id })
+    self.parent ? self.parent.connection : self.sequelize.connectionManager.getConnection({ uuid: self.id })
   ).then(function (connection) {
     self.connection = connection;
     self.connection.uuid = self.id;

package.json

 {
   "name": "sequelize",
   "description": "Multi dialect ORM for Node.JS/io.js",
-  "version": "3.14.0",
+  "version": "3.14.1",
   "author": "Sascha Depold <sascha@depold.com>",
   "contributors": [
     {

test/integration/model/create.test.js

@@ -45,9 +45,17 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       it('supports transactions', function() {
         var self = this;
         return this.sequelize.transaction().then(function(t) {
-          return self.User.findOrCreate({ where: { username: 'Username' }, defaults: { data: 'some data' }, transaction: t }).then(function() {
+          return self.User.findOrCreate({
+            where: {
+              username: 'Username'
+            },
+            defaults: {
+              data: 'some data'
+            },
+            transaction: t
+          }).then(function() {
             return self.User.count().then(function(count) {
-              expect(count).to.equal(0);
+             expect(count).to.equal(0);
               return t.commit().then(function() {
                 return self.User.count().then(function(count) {
                   expect(count).to.equal(1);