Merge pull request #1841 from overlookmotel/hooks2

Changes to hooks

lib/associations/has-many.js

@@ -455,7 +455,7 @@ module.exports = (function() {
       if (Object(association.through) === association.through) {
         // Create the related model instance
         return association.target.create(values, fieldsOrOptions).then(function(newAssociatedObject) {
-          return instance[association.accessors.add](newAssociatedObject, options).return (newAssociatedObject);
+          return instance[association.accessors.add](newAssociatedObject, options).return(newAssociatedObject);
         });
       } else {
         values[association.identifier] = instance.get(association.source.primaryKeyAttribute);

lib/instance-validator.js

@@ -174,7 +174,7 @@ InstanceValidator.prototype.hookValidate = function() {
     });
   }).then(function() {
     return self.modelInstance.Model.runHooks('afterValidate', self.modelInstance);
-  }).return (self.modelInstance);
+  }).return(self.modelInstance);
 };
 
 /**
@@ -223,7 +223,7 @@ InstanceValidator.prototype._customValidators = function() {
 
     var valprom = self._invokeCustomValidator(validator, validatorType)
       // errors are handled in settling, stub this
-      .catch (noop);
+      .catch(noop);
 
     validators.push(valprom);
   });
@@ -302,10 +302,10 @@ InstanceValidator.prototype._invokeCustomValidator = Promise.method(function(val
       validatorFunction = Promise.promisify(validator.bind(this.modelInstance));
     }
     return validatorFunction()
-      .catch (this._pushError.bind(this, false, errorKey));
+      .catch(this._pushError.bind(this, false, errorKey));
   } else {
     return Promise.try(validator.bind(this.modelInstance, invokeArgs))
-      .catch (this._pushError.bind(this, false, errorKey));
+      .catch(this._pushError.bind(this, false, errorKey));
   }
 });
 

lib/instance.js

@@ -4,6 +4,7 @@ var Utils = require('./utils')
   , Mixin = require('./associations/mixin')
   , InstanceValidator = require('./instance-validator')
   , DataTypes = require('./data-types')
+  , Promise = require("./promise")
   , _ = require('lodash')
   , defaultsOptions = { raw: true };
 
@@ -415,7 +416,9 @@ module.exports = (function() {
       fieldsOrOptions = { fields: fieldsOrOptions };
     }
 
-    options = Utils._.extend({}, options, fieldsOrOptions);
+    options = Utils._.extend({
+      hooks: true
+    }, options, fieldsOrOptions);
 
     if (!options.fields) {
       options.fields = Object.keys(this.Model.attributes);
@@ -450,8 +453,11 @@ module.exports = (function() {
       options.fields.push(createdAtAttr);
     }
 
-    return self.hookValidate({
-      skip: _.difference(Object.keys(self.rawAttributes), options.fields)
+    return Promise.try(function() {
+      // Validate
+      if (options.hooks) {
+        return self.hookValidate({skip: _.difference(Object.keys(self.rawAttributes), options.fields)});
+      }
     }).then(function() {
       options.fields.forEach(function(field) {
         if (self.dataValues[field] !== undefined) {
@@ -488,7 +494,8 @@ module.exports = (function() {
             && !!self.Model.rawAttributes[updatedAtAttr].defaultValue
           )
           ? self.Model.rawAttributes[updatedAtAttr].defaultValue
-          : Utils.now(self.sequelize.options.dialect));
+          : Utils.now(self.sequelize.options.dialect)
+        );
       }
 
       if (self.isNewRecord && createdAtAttr && !values[createdAtAttr]) {
@@ -498,7 +505,8 @@ module.exports = (function() {
             && !!self.Model.rawAttributes[createdAtAttr].defaultValue
           )
           ? self.Model.rawAttributes[createdAtAttr].defaultValue
-          : Utils.now(self.sequelize.options.dialect));
+          : Utils.now(self.sequelize.options.dialect)
+        );
       }
 
       var query = null
@@ -534,9 +542,11 @@ module.exports = (function() {
       // Add the values to the Instance
       self.dataValues = _.extend(self.dataValues, values);
 
+      return Promise.try(function() {
+        // Run before hook
+        if (options.hooks) {
           return self.Model.runHooks('before' + hook, self).then(function() {
-        // dataValues might have changed inside the hook, rebuild
-        // the values hash
+            // dataValues might have changed inside the hook, rebuild the values hash
             values = {};
 
             options.fields.forEach(function(attr) {
@@ -552,8 +562,10 @@ module.exports = (function() {
             });
 
             args[2] = values;
-
-        return self.QueryInterface[query].apply(self.QueryInterface, args).catch (function(err) {
+          });
+        }
+      }).then(function() {
+        return self.QueryInterface[query].apply(self.QueryInterface, args).catch(function(err) {
           if (!!self.__options.uniqueKeys && err.code && self.QueryInterface.QueryGenerator.uniqueConstraintMapping.code === err.code) {
             var fields = self.QueryInterface.QueryGenerator.uniqueConstraintMapping.map(err.toString());
 
@@ -568,15 +580,20 @@ module.exports = (function() {
           }
 
           throw err;
-        }).then(function(result) {
+        }).tap(function(result) {
           // Transfer database generated values (defaults, autoincrement, etc)
           values = _.extend(values, result.dataValues);
 
           // Ensure new values are on Instance, and reset previousDataValues
           result.dataValues = _.extend(result.dataValues, values);
           result._previousDataValues = _.clone(result.dataValues);
-
-          return self.Model.runHooks('after' + hook, result).return (result);
+        }).tap(function(result) {
+          // Run before hook
+          if (options.hooks) {
+            return self.Model.runHooks('after' + hook, result);
+          }	      
+        }).then(function(result) {
+          return result;
         });
       });
     });
@@ -604,7 +621,7 @@ module.exports = (function() {
       include: this.options.include || null
     }, options).then(function(reload) {
       self.set(reload.dataValues, {raw: true, reset: true});
-    }).return (self);
+    }).return(self);
   };
 
   /*
@@ -660,24 +677,37 @@ module.exports = (function() {
    * @return {Promise<undefined>}
    */
   Instance.prototype.destroy = function(options) {
-    options = options || {};
-    options.force = options.force === undefined ? false : Boolean(options.force);
+    options = Utils._.extend({
+      hooks: true,
+      force: false
+    }, options || {});
 
     var self = this;
 
     // This semi awkward syntax where we can't return the chain directly but have to return the last .then() call is to allow sql proxying
-    return self.Model.runHooks(self.Model.options.hooks.beforeDestroy, self).then(function() {
+    return Promise.try(function() {
+      // Run before hook
+      if (options.hooks) {
+        return self.Model.runHooks('beforeDestroy', self);
+      }
+    }).then(function() {
       var identifier;
 
       if (self.Model._timestampAttributes.deletedAt && options.force === false) {
         self.dataValues[self.Model._timestampAttributes.deletedAt] = new Date();
+        options.hooks = false;
         return self.save(options);
       } else {
         identifier = self.__options.hasPrimaryKeys ? self.primaryKeyValues : { id: self.id };
         return self.QueryInterface.delete(self, self.QueryInterface.QueryGenerator.addSchema(self.Model), identifier, options);
       }
-    }).then(function(results) {
-      return self.Model.runHooks(self.Model.options.hooks.afterDestroy, self).return (results);
+    }).tap(function(result) {
+      // Run after hook
+      if (options.hooks) {
+        return self.Model.runHooks('afterDestroy', self);
+      }
+    }).then(function(result) {
+      return result;
     });
   };
 

lib/sequelize.js

@@ -594,7 +594,7 @@ module.exports = (function() {
    * @return {Promise}
    */
   Sequelize.prototype.authenticate = function() {
-    return this.query('SELECT 1+1 AS result', null, { raw: true, plain: true }).return ().catch (function(err) {
+    return this.query('SELECT 1+1 AS result', null, { raw: true, plain: true }).return().catch(function(err) {
       throw new Error(err);
     });
   };

test/dao-factory.test.js

@@ -791,15 +791,13 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         , done = _.after(2, _done)
 
       this.User.bulkCreate(data).success(function() {
-        self.User.update({username: 'Bill'}, {secretValue: '42'}).done(function(err, affectedRows) {
-          expect(err).not.to.be.ok
+        self.User.update({username: 'Bill'}, {secretValue: '42'}).spread(function(affectedRows) {
           expect(affectedRows).to.equal(2)
           
           done()
         })
 
-        self.User.update({username: 'Bill'}, {secretValue: '44'}).done(function(err, affectedRows) {
-          expect(err).not.to.be.ok
+        self.User.update({username: 'Bill'}, {secretValue: '44'}).spread(function(affectedRows) {
           expect(affectedRows).to.equal(0)
           
           done()
@@ -815,8 +813,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
                     { username: 'Peter', secretValue: '42' }]
 
         this.User.bulkCreate(data).success(function () {
-          self.User.update({secretValue: '43'}, {username: 'Peter'}, {limit: 1}).done(function (err, affectedRows) {
-            expect(err).not.to.be.ok
+          self.User.update({secretValue: '43'}, {username: 'Peter'}, {limit: 1}).spread(function(affectedRows) {
             expect(affectedRows).to.equal(1)
             done()
           })

test/hooks.test.js

@@ -103,7 +103,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
                   this.User.bulkCreate([
                     {username: 'Bob', mood: 'cold'},
                     {username: 'Tobi', mood: 'hot'}
-                  ], { fields: [], hooks: true }).success(function(bulkUsers) {
+                  ], { fields: [], individualHooks: true }).success(function(bulkUsers) {
                     expect(beforeBulkCreate).to.be.true
                     expect(afterBulkCreate).to.be.true
                     expect(bulkUsers).to.be.instanceof(Array)
@@ -130,7 +130,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
                 return this.User.bulkCreate([
                   {username: 'Bob', mood: 'cold'},
                   {username: 'Tobi', mood: 'hot'}
-                ], { fields: [], hooks: false }).success(function(bulkUsers) {
+                ], { fields: [], individualHooks: false }).success(function(bulkUsers) {
                   return self.User.all().success(function(users) {
                     expect(users[0].mood).to.equal(null)
                     expect(users[1].mood).to.equal(null)
@@ -288,7 +288,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
                   this.User.bulkCreate([
                     {username: 'Bob', mood: 'cold'},
                     {username: 'Tobi', mood: 'hot'}
-                  ], { hooks: true }).success(function(bulkUsers) {
+                  ], { individualHooks: true }).success(function(bulkUsers) {
                     expect(beforeBulkCreate).to.be.true
                     expect(afterBulkCreate).to.be.true
                     expect(bulkUsers).to.be.instanceof(Array)
@@ -4343,7 +4343,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
       })
     })
 
-    describe('with the {hooks: true} option', function() {
+    describe('with the {individualHooks: true} option', function() {
       beforeEach(function(done) {
         this.User = this.sequelize.define('User', {
           username: {
@@ -4389,7 +4389,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
           fn()
         })
 
-        this.User.bulkCreate([{aNumber: 5}, {aNumber: 7}, {aNumber: 3}], { fields: ['aNumber'], hooks: true }).success(function(records) {
+        this.User.bulkCreate([{aNumber: 5}, {aNumber: 7}, {aNumber: 3}], { fields: ['aNumber'], individualHooks: true }).success(function(records) {
           records.forEach(function(record) {
             expect(record.username).to.equal('User' + record.id)
             expect(record.beforeHookTest).to.be.true
@@ -4423,7 +4423,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
           fn()
         })
 
-        this.User.bulkCreate([{aNumber: 5}, {aNumber: 7}, {aNumber: 3}], { fields: ['aNumber'], hooks: true }).error(function(err) {
+        this.User.bulkCreate([{aNumber: 5}, {aNumber: 7}, {aNumber: 3}], { fields: ['aNumber'], individualHooks: true }).error(function(err) {
           expect(err).to.equal('You shall not pass!')
           expect(beforeBulkCreate).to.be.true
           expect(afterBulkCreate).to.be.false
@@ -5254,7 +5254,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
       })
     })
 
-    describe('with the {hooks: true} option', function() {
+    describe('with the {individualHooks: true} option', function() {
       beforeEach(function(done) {
         this.User = this.sequelize.define('User', {
           username: {
@@ -5301,11 +5301,10 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
           fn()
         })
 
-
         this.User.bulkCreate([
           {aNumber: 1}, {aNumber: 1}, {aNumber: 1}
         ]).success(function() {
-          self.User.update({aNumber: 10}, {aNumber: 1}, {hooks: true}).success(function(records) {
+          self.User.update({aNumber: 10}, {aNumber: 1}, {individualHooks: true}).spread(function(affectedRows, records) {
             records.forEach(function(record) {
               expect(record.username).to.equal('User' + record.id)
               expect(record.beforeHookTest).to.be.true
@@ -5342,7 +5341,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
         })
 
         this.User.bulkCreate([{aNumber: 1}, {aNumber: 1}, {aNumber: 1}], { fields: ['aNumber'] }).success(function() {
-          self.User.update({aNumber: 10}, {aNumber: 1}, {hooks: true}).error(function(err) {
+          self.User.update({aNumber: 10}, {aNumber: 1}, {individualHooks: true}).error(function(err) {
             expect(err).to.equal('You shall not pass!')
             expect(beforeBulk).to.be.true
             expect(afterBulk).to.be.false
@@ -6038,7 +6037,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
       })
     })
 
-    describe('with the {hooks: true} option', function() {
+    describe('with the {individualHooks: true} option', function() {
       beforeEach(function(done) {
         this.User = this.sequelize.define('User', {
           username: {
@@ -6091,7 +6090,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
         this.User.bulkCreate([
           {aNumber: 1}, {aNumber: 1}, {aNumber: 1}
         ]).success(function() {
-          self.User.destroy({aNumber: 1}, {hooks: true}).success(function() {
+          self.User.destroy({aNumber: 1}, {individualHooks: true}).success(function() {
             expect(beforeBulk).to.be.true
             expect(afterBulk).to.be.true
             expect(beforeHook).to.be.true
@@ -6129,7 +6128,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
         })
 
         this.User.bulkCreate([{aNumber: 1}, {aNumber: 1}, {aNumber: 1}], { fields: ['aNumber'] }).success(function() {
-          self.User.destroy({aNumber: 1}, {hooks: true}).error(function(err) {
+          self.User.destroy({aNumber: 1}, {individualHooks: true}).error(function(err) {
             expect(err).to.equal('You shall not pass!')
             expect(beforeBulk).to.be.true
             expect(beforeHook).to.be.true
@@ -7243,7 +7242,7 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
       return this.User.bulkCreate([
         {username: 'Bob', mood: 'cold'},
         {username: 'Tobi', mood: 'hot'}
-      ], { fields: [], hooks: false }).success(function(bulkUsers) {
+      ], { fields: [], individualHooks: false }).success(function(bulkUsers) {
         return self.User.all().success(function(users) {
           expect(users[0].mood).to.equal(null)
           expect(users[1].mood).to.equal(null)