Merge pull request #3667 from BridgeAR/fix-transactions

Fix build and regression

Merge pull request #3667 from BridgeAR/fix-transactions
Fix build and regression
Mick Hansen
Commit 08782303 authored May 07, 2015 by Mick Hansen
Showing with 77 additions and 59 deletions
.travis.yml
changelog.md
lib/model.js
lib/sequelize.js
lib/utils.js
test/integration/associations/belongs-to-many.test.js
test/integration/include/findAll.test.js
test/integration/model/and-or-where.test.js
test/integration/model/update.test.js
test/integration/transaction.test.js
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,7 +3,8 @@ language: node_js
 node_js:
  - "0.10"
  - "0.12"
-  - "iojs"
+  - "iojs-v1.8"
+ # - "iojs" # iojs 2.0 is failing because of pg-native and nan
 sudo: false

--- a/changelog.md
+++ b/changelog.md
+# next
+- [BUG] Fix regression introduced in 2.1.2: updatedAt not set anymore [3667](https://github.com/sequelize/sequelize/pull/3667)
+- [BUG] Fix managed transactions not rolling back if no thenable was provided in the transaction block [3667](https://github.com/sequelize/sequelize/pull/3667)
 # 2.1.2
 - [BUG] `Model.create()/update()` no longer attempts to save undefined fields.
 # 2.1.1
 - [BUG] .get() now passes along options correctly when using a custom getter
+- [BUG] Fix managed transactions not rolling back if an error occured the transaction block [3661](https://github.com/sequelize/sequelize/pull/3661)
+- [BUG] Fix a node-webkit issue [3650](https://github.com/sequelize/sequelize/pull/3650)
 - [FEATURE] Lock modes in Postgres now support `OF table`
 - [FEATURE] New transaction lock modes `FOR KEY SHARE` and `NO KEY UPDATE` for Postgres 9.3+
 - [FEATURE/REFACTOR] Rewritten scopes with complete support for includes and scopes across associations

--- a/lib/model.js
+++ b/lib/model.js
@@ -1630,7 +1630,11 @@ module.exports = (function() {
        }
      });
    } else {
+      var updatedAtAttr = this._timestampAttributes.updatedAt;
      options.fields = _.intersection(Object.keys(values), Object.keys(this.tableAttributes));
+      if (updatedAtAttr && options.fields.indexOf(updatedAtAttr) === -1) {
+        options.fields.push(updatedAtAttr);
+      }
    }
    if (this._timestampAttributes.updatedAt) {

--- a/lib/sequelize.js
+++ b/lib/sequelize.js
@@ -1140,7 +1140,7 @@ module.exports = (function() {
          }
          var result = autoCallback(transaction);
-          if (!result || !result.then) return reject(new Error('You need to return a promise chain/thenable to the sequelize.transaction() callback'));
+          if (!result || !result.then) throw new Error('You need to return a promise chain/thenable to the sequelize.transaction() callback');
          return result.then(function (result) {
            return transaction.commit().then(function () {
@@ -1148,9 +1148,7 @@ module.exports = (function() {
            });
          });
        }).catch(function(err) {
-          transaction.rollback().then(function () {
+          transaction.rollback().finally(function () {
-            reject(err);
-          }, function () {
            reject(err);
          });
        });

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -164,7 +164,6 @@ var Utils = module.exports = {
    fields.forEach(function(attr) {
      if (dataValues[attr] !== undefined && !Model._isVirtualAttribute(attr)) {
        values[attr] = dataValues[attr];
        // Field name mapping
        if (Model.rawAttributes[attr] && Model.rawAttributes[attr].field && Model.rawAttributes[attr].field !== attr) {

--- a/test/integration/associations/belongs-to-many.test.js
+++ b/test/integration/associations/belongs-to-many.test.js
@@ -1881,15 +1881,14 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
    describe('allows the user to provide an attribute definition object as foreignKey', function() {
      it('works when taking a column directly from the object', function() {
        var Project = this.sequelize.define('project', {})
-        , User = this.sequelize.define('user', {
+          , User = this.sequelize.define('user', {
            uid: {
              type: Sequelize.INTEGER,
              primaryKey: true
            }
          });
-        var UserProjects = User.belongsToMany(Project, { foreignKey: { name: 'user_id', defaultValue: 42 }});
+        var UserProjects = User.belongsToMany(Project, { foreignKey: { name: 'user_id', defaultValue: 42 }, through: 'UserProjects' });
        expect(UserProjects.through.model.rawAttributes.user_id).to.be.ok;
        expect(UserProjects.through.model.rawAttributes.user_id.references).to.equal(User.getTableName());
        expect(UserProjects.through.model.rawAttributes.user_id.referencesKey).to.equal('uid');
@@ -1902,7 +1901,7 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
            user: Sequelize.INTEGER
          });
-      expect(User.belongsToMany.bind(User, User, { as: 'user' })).to
+      expect(User.belongsToMany.bind(User, User, { as: 'user', through: 'UserUser' })).to
        .throw ('Naming collision between attribute \'user\' and association \'user\' on model user. To remedy this, change either foreignKey or as in your association definition');
    });
  });

--- a/test/integration/include/findAll.test.js
+++ b/test/integration/include/findAll.test.js
@@ -1817,8 +1817,8 @@ describe(Support.getTestDialectTeaser('Include'), function() {
        , Group = this.sequelize.define('Group', {});
      User.belongsTo(Company);
-      User.belongsToMany(Group);
+      User.belongsToMany(Group, { through: 'UsersGroups' });
-      Group.belongsToMany(User);
+      Group.belongsToMany(User, { through: 'UsersGroups' });
      return this.sequelize.sync({force: true}).then(function () {
        return User.findAll({

--- a/test/integration/model/and-or-where.test.js
+++ b/test/integration/model/and-or-where.test.js
@@ -25,7 +25,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
 (['or', 'and']).forEach(function(method) {
    var word = method.toUpperCase();
-    describe.skip('Sequelize.' + method, function() {
+    describe('Sequelize.' + method, function() {
      it('can handle plain strings', function() {
        return this.User.find({
          where: Sequelize[method]('1=1', '2=2')
@@ -33,38 +33,38 @@ describe(Support.getTestDialectTeaser('Model'), function() {
          logging: function(sql) {
            if (dialect === 'mssql') {
              expect(sql).to.contain('WHERE (1=1 ' + word + ' 2=2)');
-            }else {
+            } else {
              expect(sql).to.contain('WHERE (1=1 ' + word + ' 2=2) LIMIT 1');
            }
          }
        });
      });
-      it('can handle arrays', function() {
+      it.skip('can handle arrays', function() {
        return this.User.find({
          where: Sequelize[method](['1=?', 1], ['2=?', 2])
        }, {
          logging: function(sql) {
            if (dialect === 'mssql') {
              expect(sql).to.contain('WHERE (1=1 ' + word + ' 2=2)');
-            }else {
+            } else {
              expect(sql).to.contain('WHERE (1=1 ' + word + ' 2=2) LIMIT 1');
            }
          }
        });
      });
-      it('can handle objects', function() {
+      it.skip('can handle objects', function() {
        return this.User.find({
          where: Sequelize[method]({ username: 'foo', intVal: 2 }, { secretValue: 'bar' })
        }, {
          logging: function(sql) {
            var expectation = ({
-              mysql: "WHERE (`User`.`username`='foo' AND `User`.`intVal`=2 " + word + " `User`.`secretValue`='bar')",
+              mysql: "WHERE (`User`.`username` = 'foo' AND `User`.`intVal` = 2 " + word + " `User`.`secretValue` = 'bar')",
-              mssql: 'WHERE ([User].[username]=\'foo\' AND [User].[intVal]=2 ' + word + ' [User].[secretValue]=\'bar\')',
+              mssql: 'WHERE ([User].[username] = \'foo\' AND [User].[intVal] = 2 ' + word + ' [User].[secretValue] = \'bar\')',
-              sqlite: "WHERE (`User`.`username`='foo' AND `User`.`intVal`=2 " + word + " `User`.`secretValue`='bar')",
+              sqlite: "WHERE (`User`.`username` = 'foo' AND `User`.`intVal` = 2 " + word + " `User`.`secretValue` = 'bar')",
-              postgres: 'WHERE ("User"."username"=\'foo\' AND "User"."intVal"=2 ' + word + ' "User"."secretValue"=\'bar\')',
+              postgres: 'WHERE ("User"."username" = \'foo\' AND "User"."intVal" = 2 ' + word + ' "User"."secretValue" = \'bar\')',
-              mariadb: "WHERE (`User`.`username`='foo' AND `User`.`intVal`=2 " + word + " `User`.`secretValue`='bar')"
+              mariadb: "WHERE (`User`.`username` = 'foo' AND `User`.`intVal` = 2 " + word + " `User`.`secretValue` = 'bar')"
            })[Support.getTestDialect()];
            if (!expectation) {
@@ -81,11 +81,11 @@ describe(Support.getTestDialectTeaser('Model'), function() {
        }, {
          logging: function(sql) {
            var expectation = ({
-              mysql: 'WHERE (`User`.`id`=1 ' + word + ' `User`.`id`=2)',
+              mysql: 'WHERE (`User`.`id` = 1 ' + word + ' `User`.`id` = 2)',
-              sqlite: 'WHERE (`User`.`id`=1 ' + word + ' `User`.`id`=2)',
+              sqlite: 'WHERE (`User`.`id` = 1 ' + word + ' `User`.`id` = 2)',
-              postgres: 'WHERE ("User"."id"=1 ' + word + ' "User"."id"=2)',
+              postgres: 'WHERE ("User"."id" = 1 ' + word + ' "User"."id" = 2)',
-              mssql: 'WHERE ([User].[id]=1 ' + word + ' [User].[id]=2)',
+              mssql: 'WHERE ([User].[id] = 1 ' + word + ' [User].[id] = 2)',
-              mariadb: 'WHERE (`User`.`id`=1 ' + word + ' `User`.`id`=2)'
+              mariadb: 'WHERE (`User`.`id` = 1 ' + word + ' `User`.`id` = 2)'
            })[Support.getTestDialect()];
            if (!expectation) {
@@ -98,7 +98,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
    });
  });
-  describe.skip('Combinations of Sequelize.and and Sequelize.or', function() {
+  describe('Combinations of Sequelize.and and Sequelize.or', function() {
    it('allows nesting of Sequelize.or', function() {
      return this.User.find({
        where: Sequelize.and(Sequelize.or('1=1', '2=2'), Sequelize.or('3=3', '4=4'))
@@ -115,8 +115,8 @@ describe(Support.getTestDialectTeaser('Model'), function() {
    it('allows nesting of Sequelize.or using object notation', function() {
      return this.User.find({
-        where: Sequelize.and(Sequelize.or({username: {eq: 'foo'}}, {username: {eq: 'bar'}}),
+        where: Sequelize.and(Sequelize.or({username: {$eq: 'foo'}}, {username: {$eq: 'bar'}}),
-                              Sequelize.or({id: {eq: 1}}, {id: {eq: 4}}))
+                              Sequelize.or({id: {$eq: 1}}, {id: {$eq: 4}}))
      }, {
        logging: function(sql) {
          var expectation = ({
@@ -152,8 +152,8 @@ describe(Support.getTestDialectTeaser('Model'), function() {
    it('allows nesting of Sequelize.and using object notation', function() {
      return this.User.find({
-        where: Sequelize.or(Sequelize.and({username: {eq: 'foo'}}, {username: {eq: 'bar'}}),
+        where: Sequelize.or(Sequelize.and({username: {$eq: 'foo'}}, {username: {$eq: 'bar'}}),
-                              Sequelize.and({id: {eq: 1}}, {id: {eq: 4}}))
+                              Sequelize.and({id: {$eq: 1}}, {id: {$eq: 4}}))
      }, {
        logging: function(sql) {
          var expectation = ({
@@ -174,6 +174,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
    });
    if (dialect !== 'postgres') {
+      // TODO: Fix this. There's a weird output produced by SqlString.arrayToList
      it('still allows simple arrays lookups', function() {
        return this.User.find({
          where: ['id IN (?) OR id IN (?)', [1, 2], [3, 4]]
@@ -206,16 +207,16 @@ describe(Support.getTestDialectTeaser('Model'), function() {
            if (dialect === 'postgres') {
              expect(sql).to.contain(
                'WHERE (' + [
-                  '"User"."id"=42 AND 2=2 AND 1=1 AND "User"."username"=\'foo\' AND ',
+                  '"User"."id" = 42 AND 2=2 AND (1=1) AND "User"."username" = \'foo\' AND ',
                    '(',
-                      '"User"."id"=42 OR 2=2 OR 1=1 OR "User"."username"=\'foo\' OR ',
+                      '"User"."id" = 42 OR 2=2 OR (1=1) OR "User"."username" = \'foo\' OR ',
-                      '("User"."id"=42 AND 2=2 AND 1=1 AND "User"."username"=\'foo\') OR ',
+                      '("User"."id" = 42 AND 2=2 AND (1=1) AND "User"."username" = \'foo\') OR ',
-                      '("User"."id"=42 OR 2=2 OR 1=1 OR "User"."username"=\'foo\')',
+                      '("User"."id" = 42 OR 2=2 OR (1=1) OR "User"."username" = \'foo\')',
                    ') AND ',
                    '(',
-                      '"User"."id"=42 AND 2=2 AND 1=1 AND "User"."username"=\'foo\' AND ',
+                      '"User"."id" = 42 AND 2=2 AND (1=1) AND "User"."username" = \'foo\' AND ',
-                      '("User"."id"=42 OR 2=2 OR 1=1 OR "User"."username"=\'foo\') AND ',
+                      '("User"."id" = 42 OR 2=2 OR (1=1) OR "User"."username" = \'foo\') AND ',
-                      '("User"."id"=42 AND 2=2 AND 1=1 AND "User"."username"=\'foo\')',
+                      '("User"."id" = 42 AND 2=2 AND (1=1) AND "User"."username" = \'foo\')',
                    ')'
                  ].join('') +
                ')'
@@ -223,16 +224,16 @@ describe(Support.getTestDialectTeaser('Model'), function() {
            } else if (dialect === 'mssql') {
              expect(sql).to.contain(
                'WHERE (' + [
-                  '[User].[id]=42 AND 2=2 AND 1=1 AND [User].[username]=\'foo\' AND ',
+                  '[User].[id] = 42 AND 2=2 AND (1=1) AND [User].[username] = \'foo\' AND ',
                    '(',
-                      '[User].[id]=42 OR 2=2 OR 1=1 OR [User].[username]=\'foo\' OR ',
+                      '[User].[id] = 42 OR 2=2 OR (1=1) OR [User].[username] = \'foo\' OR ',
-                      '([User].[id]=42 AND 2=2 AND 1=1 AND [User].[username]=\'foo\') OR ',
+                      '([User].[id] = 42 AND 2=2 AND (1=1) AND [User].[username] = \'foo\') OR ',
-                      '([User].[id]=42 OR 2=2 OR 1=1 OR [User].[username]=\'foo\')',
+                      '([User].[id] = 42 OR 2=2 OR (1=1) OR [User].[username] = \'foo\')',
                    ') AND ',
                    '(',
-                      '[User].[id]=42 AND 2=2 AND 1=1 AND [User].[username]=\'foo\' AND ',
+                      '[User].[id] = 42 AND 2=2 AND (1=1) AND [User].[username] = \'foo\' AND ',
-                      '([User].[id]=42 OR 2=2 OR 1=1 OR [User].[username]=\'foo\') AND ',
+                      '([User].[id] = 42 OR 2=2 OR (1=1) OR [User].[username] = \'foo\') AND ',
-                      '([User].[id]=42 AND 2=2 AND 1=1 AND [User].[username]=\'foo\')',
+                      '([User].[id] = 42 AND 2=2 AND (1=1) AND [User].[username] = \'foo\')',
                    ')'
                  ].join('') +
                ')'
@@ -240,16 +241,16 @@ describe(Support.getTestDialectTeaser('Model'), function() {
            } else {
              expect(sql).to.contain(
                'WHERE (' + [
-                  "`User`.`id`=42 AND 2=2 AND 1=1 AND `User`.`username`='foo' AND ",
+                  "`User`.`id` = 42 AND 2=2 AND (1=1) AND `User`.`username` = 'foo' AND ",
                    '(',
-                      "`User`.`id`=42 OR 2=2 OR 1=1 OR `User`.`username`='foo' OR ",
+                      "`User`.`id` = 42 OR 2=2 OR (1=1) OR `User`.`username` = 'foo' OR ",
-                      "(`User`.`id`=42 AND 2=2 AND 1=1 AND `User`.`username`='foo') OR ",
+                      "(`User`.`id` = 42 AND 2=2 AND (1=1) AND `User`.`username` = 'foo') OR ",
-                      "(`User`.`id`=42 OR 2=2 OR 1=1 OR `User`.`username`='foo')",
+                      "(`User`.`id` = 42 OR 2=2 OR (1=1) OR `User`.`username` = 'foo')",
                    ') AND ',
                    '(',
-                      "`User`.`id`=42 AND 2=2 AND 1=1 AND `User`.`username`='foo' AND ",
+                      "`User`.`id` = 42 AND 2=2 AND (1=1) AND `User`.`username` = 'foo' AND ",
-                      "(`User`.`id`=42 OR 2=2 OR 1=1 OR `User`.`username`='foo') AND ",
+                      "(`User`.`id` = 42 OR 2=2 OR (1=1) OR `User`.`username` = 'foo') AND ",
-                      "(`User`.`id`=42 AND 2=2 AND 1=1 AND `User`.`username`='foo')",
+                      "(`User`.`id` = 42 AND 2=2 AND (1=1) AND `User`.`username` = 'foo')",
                    ')'
                  ].join('') +
                ')'

--- a/test/integration/model/update.test.js
+++ b/test/integration/model/update.test.js
@@ -18,7 +18,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
        }
      });
-      return Account.sync({force: true, logging: console.log}).then(function () {
+      return Account.sync({force: true}).then(function () {
        return Account.create({
          ownerId: 2
        }).then(function (account) {

--- a/test/integration/transaction.test.js
+++ b/test/integration/transaction.test.js
@@ -44,6 +44,7 @@ describe(Support.getTestDialectTeaser('Transaction'), function() {
        return Promise.resolve();
      });
    });
    it('supports automatically rolling back with a thrown error', function() {
      var t;
      return (expect(this.sequelize.transaction(function(transaction) {
@@ -53,15 +54,24 @@ describe(Support.getTestDialectTeaser('Transaction'), function() {
        expect(t.finished).to.be.equal('rollback');
      });
    });
    it('supports automatically rolling back with a rejection', function() {
-      return expect(this.sequelize.transaction(function() {
+      var t;
+      return (expect(this.sequelize.transaction(function(transaction) {
+        t = transaction;
        return Promise.reject('Swag');
-      })).to.eventually.be.rejected;
+      })).to.eventually.be.rejected).then(function() {
+        expect(t.finished).to.be.equal('rollback');
+      });
    });
-    it('errors when no promise chain is returned', function() {
-      return expect(this.sequelize.transaction(function() {
-      })).to.eventually.be.rejected;
+    it('errors when no promise chain is returned', function() {
+      var t;
+      return (expect(this.sequelize.transaction(function(transaction) {
+        t = transaction;
+      })).to.eventually.be.rejected).then(function() {
+        expect(t.finished).to.be.equal('rollback');
+      });
    });
  });