Transactions for SQL Server - V3 backport (#7001)

* Enabled transaction tests for mssql dialect, fixed transaction name too long bug

* Removed passing of isolationLevel to begintransaction method

* Replaced mssql query lodash includes with startsWith, added a test to allow transaction keywords as values

appveyor-setup.ps1

@@ -20,7 +20,7 @@ $config = @{
   username = "sa"
   password = "Password12!"
   port = $port
-  database = "master"
+  database = "sequelize_test"
   dialectOptions = @{
     requestTimeout = 25000
     cryptoCredentialsDetails = @{
@@ -35,5 +35,8 @@ $config = @{
 
 $json = $config | ConvertTo-Json -Depth 3
 
+# Create sequelize_test database
+sqlcmd -S "(local)" -U "sa" -P "Password12!" -d "master" -Q "CREATE DATABASE [sequelize_test]; ALTER DATABASE [sequelize_test] SET READ_COMMITTED_SNAPSHOT ON;"
+
 # cannot use Out-File because it outputs a BOM
 [IO.File]::WriteAllLines((Join-Path $pwd "test\config\mssql.json"), $json)

changelog.md

+# Future
+- [FIXED] Transaction Name too long, transaction savepoints for SQL Server [#6972](https://github.com/sequelize/sequelize/pull/6972)
+
 # 3.28.0
 - [FIXED] Soft-delete not returning number of affected rows on mssql [#6916](https://github.com/sequelize/sequelize/pull/6916)
 - [ADDED] `afterConnect` hook

lib/dialects/abstract/query-generator.js

@@ -1804,6 +1804,10 @@ var QueryGenerator = {
     return 'SET SESSION TRANSACTION ISOLATION LEVEL ' + value + ';';
   },
 
+  generateTransactionId: function() {
+    return uuid.v4();
+  },
+
   /**
    * Returns a query that starts a transaction.
    *

lib/dialects/mssql/index.js

@@ -24,7 +24,7 @@ MssqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.support
   'LIMIT ON UPDATE': true,
   'ORDER NULLS': false,
   lock: false,
-  transactions: false,
+  transactions: true,
   migrations: false,
   upserts: true,
   returnValues: {

lib/dialects/mssql/query-generator.js

@@ -4,6 +4,7 @@
 var Utils = require('../../utils')
   , DataTypes = require('../../data-types')
   , AbstractQueryGenerator = require('../abstract/query-generator')
+  , randomBytes = require('crypto').randomBytes
   , semver = require('semver');
 
 /* istanbul ignore next */
@@ -728,6 +729,10 @@ var QueryGenerator = {
     return 'SET TRANSACTION ISOLATION LEVEL ' + value + ';';
   },
 
+  generateTransactionId: function () {
+    return randomBytes(10).toString('hex');
+  },
+
   startTransactionQuery: function(transaction, options) {
     if (transaction.parent) {
       return 'SAVE TRANSACTION ' + this.quoteIdentifier(transaction.name) + ';';

lib/dialects/mssql/query.js

@@ -38,35 +38,43 @@ Query.prototype._run = function(connection, sql, parameters) {
   if (benchmark) {
     var queryBegin = Date.now();
   } else {
-    this.sequelize.log('Executing (' + (connection.uuid || 'default') + '): ' + this.sql, this.options);
+    this.sequelize.log('Executing (' + (self.connection.uuid || 'default') + '): ' + this.sql, this.options);
   }
 
   var promise = new Utils.Promise(function(resolve, reject) {
       // TRANSACTION SUPPORT
-      if (_.includes(self.sql, 'BEGIN TRANSACTION')) {
+      if (_.startsWith(self.sql, 'BEGIN TRANSACTION')) {
         connection.beginTransaction(function(err) {
-          if (!!err) {
+          if (err) {
             reject(self.formatError(err));
           } else {
             resolve(self.formatResults());
           }
-        } /* name, isolation_level */);
-      } else if (_.includes(self.sql, 'COMMIT TRANSACTION')) {
+        }, self.options.transaction.name);
+      } else if (_.startsWith(self.sql, 'COMMIT TRANSACTION')) {
         connection.commitTransaction(function(err) {
-          if (!!err) {
+          if (err) {
             reject(self.formatError(err));
           } else {
             resolve(self.formatResults());
           }
         });
-      } else if (_.includes(self.sql, 'ROLLBACK TRANSACTION')) {
+      } else if (_.startsWith(self.sql, 'ROLLBACK TRANSACTION')) {
         connection.rollbackTransaction(function(err) {
-          if (!!err) {
+          if (err) {
             reject(self.formatError(err));
           } else {
             resolve(self.formatResults());
           }
-        });
+        }, self.options.transaction.name);
+      } else if (_.startsWith(self.sql, 'SAVE TRANSACTION')) {
+        connection.saveTransaction(function(err) {
+          if (err) {
+            reject(self.formatError(err));
+          } else {
+            resolve(self.formatResults());
+          }
+        }, self.options.transaction.name);
       } else {
         // QUERY SUPPORT
         var results = [];
@@ -74,7 +82,7 @@ Query.prototype._run = function(connection, sql, parameters) {
         var request = new connection.lib.Request(self.sql, function(err) {
 
           if (benchmark) {
-            self.sequelize.log('Executed (' + (connection.uuid || 'default') + '): ' + self.sql, (Date.now() - queryBegin), self.options);
+            self.sequelize.log('Executed (' + (self.connection.uuid || 'default') + '): ' + self.sql, (Date.now() - queryBegin), self.options);
           }
 
           if (err) {

lib/query-interface.js

@@ -877,7 +877,7 @@ QueryInterface.prototype.startTransaction = function(transaction, options) {
   options = _.assign({}, options, {
     transaction: transaction.parent || transaction
   });
-
+  options.transaction.name = transaction.parent ? transaction.name : undefined;
   var sql = this.QueryGenerator.startTransactionQuery(transaction);
 
   return this.sequelize.query(sql, options);
@@ -928,7 +928,7 @@ QueryInterface.prototype.rollbackTransaction = function(transaction, options) {
     transaction: transaction.parent || transaction,
     supportsSearchPath: false
   });
-
+  options.transaction.name = transaction.parent ? transaction.name : undefined;
   var sql = this.QueryGenerator.rollbackTransactionQuery(transaction);
   var promise = this.sequelize.query(sql, options);
 

lib/transaction.js

 'use strict';
 
-var Utils = require('./utils')
-  , uuid = require('node-uuid');
+var Utils = require('./utils');
 
 /**
  * The transaction object is used to identify a running transaction. It is created by calling `Sequelize.transaction()`.
@@ -20,6 +19,8 @@ var Utils = require('./utils')
 var Transaction = module.exports = function(sequelize, options) {
   this.sequelize = sequelize;
   this.savepoints = [];
+  var generateTransactionId = this.sequelize.dialect.QueryGenerator.generateTransactionId;
+
   this.options = Utils._.extend({
     autocommit: true,
     type: sequelize.options.transactionType,
@@ -27,14 +28,14 @@ var Transaction = module.exports = function(sequelize, options) {
   }, options || {});
 
   this.parent = this.options.transaction;
-  this.id = this.parent ? this.parent.id : uuid.v4();
+  this.id = this.parent ? this.parent.id : generateTransactionId();
 
   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();
+    this.id = this.name = generateTransactionId();
   }
 
   delete this.options.transaction;

test/integration/model/create.test.js

@@ -1203,6 +1203,28 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
     });
 
+    it('Works even when SQL query has a values of transaction keywords such as BEGIN TRANSACTION', function () {
+      var Task = this.sequelize.define('task', {
+        title: DataTypes.STRING
+      });
+      return Task.sync({ force: true })
+        .then(function () {
+          return Sequelize.Promise.all([
+            Task.create({ title: 'BEGIN TRANSACTION' }),
+            Task.create({ title: 'COMMIT TRANSACTION' }),
+            Task.create({ title: 'ROLLBACK TRANSACTION' }),
+            Task.create({ title: 'SAVE TRANSACTION' }),
+          ]);
+        })
+        .then(function (newTasks) {
+          expect(newTasks).to.have.lengthOf(4);
+          expect(newTasks[0].title).to.equal('BEGIN TRANSACTION');
+          expect(newTasks[1].title).to.equal('COMMIT TRANSACTION');
+          expect(newTasks[2].title).to.equal('ROLLBACK TRANSACTION');
+          expect(newTasks[3].title).to.equal('SAVE TRANSACTION');
+        });
+    });
+
     describe('enums', function() {
       it('correctly restores enum values', function() {
         var self = this
@@ -1325,19 +1347,29 @@ describe(Support.getTestDialectTeaser('Model'), function() {
   describe('bulkCreate', function() {
     if (current.dialect.supports.transactions) {
       it('supports transactions', function() {
-        var self = this;
-        return this.sequelize.transaction().then(function(t) {
-          return self.User
-            .bulkCreate([{ username: 'foo' }, { username: 'bar' }], { transaction: t })
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        });
+        return User.sync({ force: true })
+          .bind(this)
+          .then(function () {
+             return this.sequelize.transaction();
+          })
+          .then(function(t) {
+            this.transaction = t;
+            return User.bulkCreate([{ username: 'foo' }, { username: 'bar' }], { transaction: this.transaction });
+          })
           .then(function() {
-              return self.User.count().then(function(count1) {
-                return self.User.count({ transaction: t }).then(function(count2) {
-                  expect(count1).to.equal(0);
+            return User.count();
+          })
+          .then(function(count1) {
+            this.count1 = count1;
+            return User.count({ transaction: this.transaction });
+          })
+          .then(function(count2) {
+            expect(this.count1).to.equal(0);
             expect(count2).to.equal(2);
-                  return t.rollback();
-                });
-              });
-            });
+            return this.transaction.rollback();
           });
       });
     }

test/integration/sequelize.transaction.test.js

@@ -72,6 +72,9 @@ describe(Support.getTestDialectTeaser('Sequelize#transaction'), function() {
           case 'sqlite':
             query = 'select sqlite3_sleep(2000);';
             break;
+          case 'mssql':
+            query = 'WAITFOR DELAY \'00:00:02\';';
+            break;
           default:
             break;
           }

test/integration/transaction.test.js

@@ -31,6 +31,14 @@ describe(Support.getTestDialectTeaser('Transaction'), function() {
       var transaction = new Transaction(this.sequelize);
       expect(transaction.id).to.exist;
     });
+
+    it('should call dialect specific generateTransactionId method', function() {
+      var transaction = new Transaction(this.sequelize);
+      expect(transaction.id).to.exist;      
+      if (dialect === 'mssql') {
+        expect(transaction.id).to.have.lengthOf(20);
+      }
+    });
   });
 
   describe('commit', function() {
@@ -81,7 +89,7 @@ describe(Support.getTestDialectTeaser('Transaction'), function() {
       });
     });
 
-    if (dialect === 'postgres' || dialect === 'mssql') {
+    if (dialect === 'postgres') {
       it('do not rollback if already committed', function() {
         var SumSumSum = this.sequelize.define('transaction', {
               value: {