Transactions for SQL Server (#6972)

* [MSSQL] Uses crypto randomBytes to generate 20 character trasaction identifier, enabled trasaction tests for mssql dialect

* Added missing break, changelog entry

* Added READ_COMMITTED_SNAPSHOT ON script for tests, fixes skipped tests, transaction savepoints, uses sequelize_test as default database on appveyor

* Added sqlcmd database query to appveyor-setup.ps1

* Added sequelize_test as default db in test/config/config for MSSQL dialect

* Moved READ_COMMITTED_SNAPSHOT ON query to appveyor ps script

* Replaced lodash includes with startsWith, review fixes, closes #6954

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)
 - [FIXED] Issue with sync hooks (before/afterInit, before/afterDefine) [#6680](https://github.com/sequelize/sequelize/issues/6680)
 - [FIXED] MSSQL handle large bulk inserts [#6866](https://github.com/sequelize/sequelize/issues/6866)
 - [FIXED] describeTable returns a wrong value for primaryKey [#5756] (https://github.com/sequelize/sequelize/issues/5756)

lib/dialects/abstract/query-generator.js

@@ -1577,6 +1577,10 @@ const QueryGenerator = {
     return 'SET SESSION TRANSACTION ISOLATION LEVEL ' + value + ';';
   },
 
+  generateTransactionId() {
+    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 */
@@ -718,6 +719,10 @@ var QueryGenerator = {
     return 'SET TRANSACTION ISOLATION LEVEL ' + value + ';';
   },
 
+  generateTransactionId() {
+    return randomBytes(10).toString('hex');
+  },
+
   startTransactionQuery(transaction, options) {
     if (transaction.parent) {
       return 'SAVE TRANSACTION ' + this.quoteIdentifier(transaction.name) + ';';

lib/dialects/mssql/query.js

@@ -37,46 +37,53 @@ class Query extends AbstractQuery {
     if (benchmark) {
       queryBegin = Date.now();
     } else {
-      this.sequelize.log('Executing (' + (connection.uuid || 'default') + '): ' + this.sql, this.options);
+      this.sequelize.log('Executing (' + (this.connection.uuid || 'default') + '): ' + this.sql, this.options);
     }
 
-    debug(`executing(${connection.uuid || 'default'}) : ${this.sql}`);
+    debug(`executing(${this.connection.uuid || 'default'}) : ${this.sql}`);
 
     return new Promise((resolve, reject) => {
         // TRANSACTION SUPPORT
-      if (_.includes(this.sql, 'BEGIN TRANSACTION')) {
+      if (_.startsWith(this.sql, 'BEGIN TRANSACTION')) {
         connection.beginTransaction(err => {
-          if (!!err) {
+          if (err) {
             reject(this.formatError(err));
           } else {
             resolve(this.formatResults());
           }
-        } /* name, isolation_level */);
-      } else if (_.includes(this.sql, 'COMMIT TRANSACTION')) {
+        }, this.options.transaction.name, this.options.isolationLevel);
+      } else if (_.startsWith(this.sql, 'COMMIT TRANSACTION')) {
         connection.commitTransaction(err => {
-          if (!!err) {
+          if (err) {
             reject(this.formatError(err));
           } else {
             resolve(this.formatResults());
           }
         });
-      } else if (_.includes(this.sql, 'ROLLBACK TRANSACTION')) {
+      } else if (_.startsWith(this.sql, 'ROLLBACK TRANSACTION')) {
         connection.rollbackTransaction(err => {
-          if (!!err) {
+          if (err) {
             reject(this.formatError(err));
           } else {
             resolve(this.formatResults());
           }
-        });
+        }, this.options.transaction.name);
+      } else if (_.startsWith(this.sql, 'SAVE TRANSACTION')) {
+        connection.saveTransaction(err => {
+          if (err) {
+            reject(this.formatError(err));
+          } else {
+            resolve(this.formatResults());
+          }
+        }, this.options.transaction.name);
       } else {
-          // QUERY SUPPORT
         const results = [];
         const request = new connection.lib.Request(this.sql, (err, rowCount) => {
 
-          debug(`executed(${connection.uuid || 'default'}) : ${this.sql}`);
+          debug(`executed(${this.connection.uuid || 'default'}) : ${this.sql}`);
 
           if (benchmark) {
-            this.sequelize.log('Executed (' + (connection.uuid || 'default') + '): ' + this.sql, (Date.now() - queryBegin), this.options);
+            this.sequelize.log('Executed (' + (this.connection.uuid || 'default') + '): ' + this.sql, (Date.now() - queryBegin), this.options);
           }
 
           if (err) {

lib/query-interface.js

@@ -847,7 +847,7 @@ class QueryInterface {
     options = _.assign({}, options, {
       transaction: transaction.parent || transaction
     });
-
+    options.transaction.name = transaction.parent ? transaction.name : undefined;
     const sql = this.QueryGenerator.startTransactionQuery(transaction);
 
     return this.sequelize.query(sql, options);
@@ -898,7 +898,7 @@ class QueryInterface {
       transaction: transaction.parent || transaction,
       supportsSearchPath: false
     });
-
+    options.transaction.name = transaction.parent ? transaction.name : undefined;
     const sql = this.QueryGenerator.rollbackTransactionQuery(transaction);
     const promise = this.sequelize.query(sql, options);
 

lib/transaction.js

 'use strict';
 
 const Utils = require('./utils');
-const uuid = require('uuid');
 
 /**
  * The transaction object is used to identify a running transaction. It is created by calling `Sequelize.transaction()`.
@@ -26,6 +25,7 @@ class Transaction {
 
     // get dialect specific transaction options
     const transactionOptions = sequelize.dialect.supports.transactionOptions || {};
+    const generateTransactionId = this.sequelize.dialect.QueryGenerator.generateTransactionId;
 
     this.options = Utils._.extend({
       autocommit: transactionOptions.autocommit || null,
@@ -34,14 +34,14 @@ class Transaction {
     }, 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/config/config.js

@@ -23,17 +23,12 @@ module.exports = {
   },
 
   mssql: mssqlConfig || {
-    database: process.env.SEQ_MSSQL_DB   || process.env.SEQ_DB   || (function () {
-      var db = 'sequelize-test-' + ~~(Math.random() * 100);
-      console.log('Using database: ', db);
-      return db;
-    }()),
+    database: process.env.SEQ_MSSQL_DB   || process.env.SEQ_DB   || 'sequelize_test',
     username: process.env.SEQ_MSSQL_USER || process.env.SEQ_USER || 'sequelize',
     password: process.env.SEQ_MSSQL_PW   || process.env.SEQ_PW   || 'nEGkLma26gXVHFUAHJxcmsrK',
     host:     process.env.SEQ_MSSQL_HOST || process.env.SEQ_HOST || 'mssql.sequelizejs.com',
     port:     process.env.SEQ_MSSQL_PORT || process.env.SEQ_PORT || 1433,
     dialectOptions: {
-      instanceName:  process.env.MSSQL_INSTANCE ? process.env.MSSQL_INSTANCE : 'SQLEXPRESS',
       // big insert queries need a while
       requestTimeout: 60000
     },

test/integration/model/create.test.js

@@ -1244,6 +1244,28 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
     });
 
+    it('Works even when SQL query has a values of transaction keywords such as BEGIN TRANSACTION', function () {
+      const Task = this.sequelize.define('task', {
+        title: DataTypes.STRING
+      });
+      return Task.sync({ force: true })
+        .then(() => {
+          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(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
@@ -1366,20 +1388,26 @@ 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 })
-            .then(function() {
-              return self.User.count().then(function(count1) {
-                return self.User.count({ transaction: t }).then(function(count2) {
-                  expect(count1).to.equal(0);
-                  expect(count2).to.equal(2);
-                  return t.rollback();
-                });
-              });
-            });
-        });
+        const User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        });
+        let transaction, count1;
+        return User.sync({ force: true })
+          .then(() => this.sequelize.transaction())
+          .then(t => {
+            transaction = t;
+            return User.bulkCreate([{ username: 'foo' }, { username: 'bar' }], { transaction });
+          })
+          .then(() => User.count())
+          .then((count) => {
+            count1 = count;
+            return User.count({ transaction });
+          })
+          .then((count2) => {
+              expect(count1).to.equal(0);
+              expect(count2).to.equal(2);
+              return transaction.rollback();
+          });
       });
     }
 

test/integration/sequelize.transaction.test.js

@@ -71,6 +71,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() {
+      const transaction = new Transaction(this.sequelize);
+      expect(transaction.id).to.exist;      
+      if (dialect === 'mssql') {
+        expect(transaction.id).to.have.lengthOf(20);
+      }
+    });
   });
 
   describe('commit', function() {
@@ -72,7 +80,8 @@ describe(Support.getTestDialectTeaser('Transaction'), function() {
       });
     });
 
-    if (dialect === 'postgres' || dialect === 'mssql') {
+    //Promise rejection test is specifc to postgres
+    if (dialect === 'postgres') {
       it('do not rollback if already committed', function() {
         var SumSumSum = this.sequelize.define('transaction', {
               value: {

test/support.js

@@ -218,6 +218,5 @@ if (typeof beforeEach !== 'undefined') {
     this.sequelize = Support.sequelize;
   });
 }
-
 Support.sequelize = Support.createSequelizeInstance();
 module.exports = Support;