Merge pull request #1225 from saschagehlich/feature/pass-instance-to-hooks

Now passing instances to hooks

lib/dao-factory.js

@@ -53,6 +53,7 @@ module.exports = (function() {
     }
 
     attributes = replaceReferencesWithTableNames(attributes)
+    this.options.hooks = this.replaceHookAliases(this.options.hooks)
 
     this.rawAttributes     = attributes
     this.daoFactoryManager = null // defined in init function

lib/dao-validator.js

@@ -23,12 +23,11 @@ DaoValidator.prototype.hookValidate = function() {
     , errors = {}
 
   return new Utils.CustomEventEmitter(function(emitter) {
-    self.model.daoFactory.runHooks('beforeValidate', self.model.dataValues, function(err, newValues) {
+    self.model.daoFactory.runHooks('beforeValidate', self.model, function(err) {
       if (!!err) {
         return emitter.emit('error', err)
       }
 
-      self.model.dataValues = newValues || self.model.dataValues
       errors = Utils._.extend(errors, validateAttributes.call(self))
       errors = Utils._.extend(errors, validateModel.call(self))
 
@@ -36,12 +35,11 @@ DaoValidator.prototype.hookValidate = function() {
         return emitter.emit('error', errors)
       }
 
-      self.model.daoFactory.runHooks('afterValidate', self.model.dataValues, function(err, newValues) {
+      self.model.daoFactory.runHooks('afterValidate', self.model, function(err) {
         if (!!err) {
           return emitter.emit('error', err)
         }
 
-        self.model.dataValues = newValues || self.model.dataValues
         emitter.emit('success', self.model)
       })
     })

lib/dao.js

@@ -12,7 +12,7 @@ module.exports = (function() {
     this.__options                   = this.__factory.options
     this.options                     = options
     this.hasPrimaryKeys              = this.__factory.options.hasPrimaryKeys
-    // What is selected values even used for? 
+    // What is selected values even used for?
     this.selectedValues              = options.include ? _.omit(values, options.includeNames) : values
     this.__eagerlyLoadedAssociations = []
     this.isNewRecord                 = options.isNewRecord
@@ -157,7 +157,7 @@ module.exports = (function() {
       // If not raw, and there's a customer setter
       if (!options.raw && this._customSetters[key]) {
         this._customSetters[key].call(this, value, key)
-      } else { 
+      } else {
         // Check if we have included models, and if this key matches the include model names/aliases
 
         if (this.options && this.options.include && this.options.includeNames.indexOf(key) !== -1) {
@@ -375,35 +375,40 @@ module.exports = (function() {
           hook          = 'Update'
         }
 
-        self.__factory.runHooks('before' + hook, values, function(err, newValues) {
+        // Add the values to the DAO
+        self.dataValues = _.extend(self.dataValues, values)
+
+        // Run the beforeCreate / beforeUpdate hook
+        self.__factory.runHooks('before' + hook, self, function(err) {
           if (!!err) {
             return emitter.emit('error', err)
           }
 
-          args[2] = values = newValues || values
-          // Newest values need to be on the dao since the dao is returned from the query interface
-          self.dataValues = _.extend(self.dataValues, values)
+          // dataValues might have changed inside the hook, rebuild
+          // the values hash
+          values = {}
+
+          options.fields.forEach(function(field) {
+            if (self.dataValues[field] !== undefined) {
+              values[field] = self.dataValues[field]
+            }
+          })
+          args[2] = values
 
           self.QueryInterface[query].apply(self.QueryInterface, args)
             .proxy(emitter, {events: ['sql', 'error']})
             .success(function(result) {
               // Transfer database generated values (defaults, autoincrement, etc)
-              values = _.extend(values, result.values) 
+              values = _.extend(values, result.dataValues)
 
               // Ensure new values are on DAO, and reset previousDataValues
               result.dataValues = _.extend(result.dataValues, values)
               result._previousDataValues = _.clone(result.dataValues)
 
-              self.__factory.runHooks('after' + hook, values, function(err, newValues) {
+              self.__factory.runHooks('after' + hook, result, function(err) {
                 if (!!err) {
                   return emitter.emit('error', err)
                 }
-
-                if (newValues) {
-                  // Repeat value assignment incase afterHook changed anything
-                  result.dataValues = _.extend(result.dataValues, newValues)
-                  result._previousDataValues = _.clone(result.dataValues)
-                }
                 emitter.emit('success', result)
               })
             })
@@ -486,7 +491,7 @@ module.exports = (function() {
       , query = null
 
     return new Utils.CustomEventEmitter(function(emitter) {
-      self.daoFactory.runHooks(self.daoFactory.options.hooks.beforeDestroy, self.dataValues, function(err) {
+      self.daoFactory.runHooks(self.daoFactory.options.hooks.beforeDestroy, self, function(err) {
         if (!!err) {
           return emitter.emit('error', err)
         }
@@ -507,7 +512,7 @@ module.exports = (function() {
           emitter.emit('error', err)
         })
         .success(function(results) {
-          self.daoFactory.runHooks(self.daoFactory.options.hooks.afterDestroy, self.dataValues, function(err) {
+          self.daoFactory.runHooks(self.daoFactory.options.hooks.afterDestroy, self, function(err) {
             if (!!err) {
               return emitter.emit('error', err)
             }

lib/hooks.js

+var Utils = require("./utils")
 var Hooks = module.exports = function(){}
+var hookAliases = {
+  beforeDelete: "beforeDestroy",
+  afterDelete: "afterDestroy"
+}
+
+Hooks.replaceHookAliases = function(hooks) {
+  var realHookName
+
+  Utils._.each(hooks, function(hooksArray, name) {
+    // Does an alias for this hook name exist?
+    if(realHookName = hookAliases[name]) {
+      // Add the hooks to the actual hook
+      hooks[realHookName] = (hooks[realHookName] || []).concat(hooksArray)
+
+      // Delete the alias
+      delete hooks[name]
+    }
+  })
+
+  return hooks
+}
 
 Hooks.runHooks = function() {
   var self  = this
@@ -43,16 +65,9 @@ Hooks.runHooks = function() {
   run(hooks[tick])
 }
 
-Hooks.hook = function(hookType, name, fn) {
-  // For aliases, we may want to incorporate some sort of way to mitigate this
-  if (hookType === "beforeDelete") {
-    hookType = 'beforeDestroy'
-  }
-  else if (hookType === "afterDelete") {
-    hookType = 'afterDestroy'
-  }
-
-  Hooks.addHook.call(this, hookType, name, fn)
+// Alias for `.addHook`
+Hooks.hook = function() {
+  Hooks.addHook.apply(this, arguments)
 }
 
 Hooks.addHook = function(hookType, name, fn) {
@@ -65,6 +80,9 @@ Hooks.addHook = function(hookType, name, fn) {
     fn.apply(this, Array.prototype.slice.call(arguments, 0, arguments.length-1).concat(arguments[arguments.length-1]))
   }
 
+  // Aliases
+  hookType = hookAliases[hookType] || hookType
+
   // Just in case if we override the default DAOFactory.options
   this.options.hooks[hookType] = this.options.hooks[hookType] || []
   this.options.hooks[hookType][this.options.hooks[hookType].length] = !!name ? {name: name, fn: method} : method
@@ -95,11 +113,11 @@ Hooks.afterDestroy = function(name, fn) {
 }
 
 Hooks.beforeDelete = function(name, fn) {
-  Hooks.addHook.call(this, 'beforeDestroy', name, fn)
+  Hooks.addHook.call(this, 'beforeDelete', name, fn)
 }
 
 Hooks.afterDelete = function(name, fn) {
-  Hooks.addHook.call(this, 'afterDestroy', name, fn)
+  Hooks.addHook.call(this, 'afterDelete', name, fn)
 }
 
 Hooks.beforeUpdate = function(name, fn) {

package.json

@@ -56,7 +56,7 @@
   },
   "devDependencies": {
     "sqlite3": "~2.1.12",
-    "mysql": "~2.0.0-alpha9",
+    "mysql": "2.0.0-alpha9",
     "pg": "~2.8.1",
     "watchr": "~2.4.3",
     "yuidocjs": "~0.3.36",
@@ -89,4 +89,4 @@
     "node": ">=0.6.21"
   },
   "license": "MIT"
-}
\ No newline at end of file
+}

test/dao-factory.test.js

@@ -1686,10 +1686,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
     beforeEach(function(done) {
       this.User = this.sequelize.define('User', {username: DataTypes.STRING }, { paranoid: true })
       this.Project = this.sequelize.define('Project', { title: DataTypes.STRING }, { paranoid: true })
-      
+
       this.Project.hasMany(this.User)
       this.User.hasMany(this.Project)
-      
+
       var self = this
       this.sequelize.sync({ force: true }).success(function() {
         self.User.bulkCreate([{
@@ -1777,7 +1777,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         }
       }).error(done)
     })
-  
+
   })
 
 })

test/hooks.test.js

@@ -6994,4 +6994,168 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
       })
     })
   })
+
+  describe('passing DAO instances', function() {
+
+    describe('beforeValidate / afterValidate', function() {
+      it('should pass a DAO instance to the hook', function(done){
+        var beforeHooked = false
+        var afterHooked = false
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        }, {
+          hooks: {
+            beforeValidate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              beforeHooked = true
+              fn()
+            },
+            afterValidate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              afterHooked = true
+              fn()
+            }
+          }
+        })
+
+        User.sync({ force: true }).success(function() {
+          User.create({ username: 'bob' }).success(function(user) {
+            expect(beforeHooked).to.be.true
+            expect(afterHooked).to.be.true
+            done()
+          })
+        })
+      })
+    })
+
+    describe('beforeCreate / afterCreate', function() {
+      it('should pass a DAO instance to the hook', function(done){
+        var beforeHooked = false
+        var afterHooked = false
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        }, {
+          hooks: {
+            beforeCreate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              beforeHooked = true
+              fn()
+            },
+            afterCreate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              afterHooked = true
+              fn()
+            }
+          }
+        })
+
+        User.sync({ force: true }).success(function() {
+          User.create({ username: 'bob' }).success(function(user) {
+            expect(beforeHooked).to.be.true
+            expect(afterHooked).to.be.true
+            done()
+          })
+        })
+      })
+    })
+
+    describe('beforeDestroy / afterDestroy', function() {
+      it('should pass a DAO instance to the hook', function(done){
+        var beforeHooked = false
+        var afterHooked = false
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        }, {
+          hooks: {
+            beforeDestroy: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              beforeHooked = true
+              fn()
+            },
+            afterDestroy: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              afterHooked = true
+              fn()
+            }
+          }
+        })
+
+        User.sync({ force: true }).success(function() {
+          User.create({ username: 'bob' }).success(function(user) {
+            user.destroy().success(function() {
+              expect(beforeHooked).to.be.true
+              expect(afterHooked).to.be.true
+              done()
+            })
+          })
+        })
+      })
+    })
+
+    describe('beforeDelete / afterDelete', function() {
+      it('should pass a DAO instance to the hook', function(done){
+        var beforeHooked = false
+        var afterHooked = false
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        }, {
+          hooks: {
+            beforeDelete: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              beforeHooked = true
+              fn()
+            },
+            afterDelete: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              afterHooked = true
+              fn()
+            }
+          }
+        })
+
+        User.sync({ force: true }).success(function() {
+          User.create({ username: 'bob' }).success(function(user) {
+            user.destroy().success(function() {
+              expect(beforeHooked).to.be.true
+              expect(afterHooked).to.be.true
+              done()
+            })
+          })
+        })
+      })
+    })
+
+    describe('beforeUpdate / afterUpdate', function() {
+      it('should pass a DAO instance to the hook', function(done){
+        var beforeHooked = false
+        var afterHooked = false
+        var User = this.sequelize.define('User', {
+          username: DataTypes.STRING
+        }, {
+          hooks: {
+            beforeUpdate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              beforeHooked = true
+              fn()
+            },
+            afterUpdate: function(user, fn) {
+              expect(user).to.be.instanceof(User.DAO)
+              afterHooked = true
+              fn()
+            }
+          }
+        })
+
+        User.sync({ force: true }).success(function() {
+          User.create({ username: 'bob' }).success(function(user) {
+            user.save({ username: 'bawb' }).success(function() {
+              expect(beforeHooked).to.be.true
+              expect(afterHooked).to.be.true
+              done()
+            })
+          })
+        })
+      })
+    })
+  })
 })