Merge pull request #165 from strongloop/feature/allow-hook-skip-next

Pass callback to hooks so that next can be skipped

lib/dao.js

@@ -178,8 +178,8 @@ DataAccessObject.create = function (data, callback) {
             });
           });
         }, obj);
-      }, obj);
-    }, obj);
+      }, obj, callback);
+    }, obj, callback);
   }
 
   // for chaining
@@ -1032,8 +1032,8 @@ DataAccessObject.prototype.save = function (options, callback) {
             });
           });
         });
-      }, data);
-    }, data);
+      }, data, callback);
+    }, data, callback);
   }
 };
 
@@ -1136,7 +1136,7 @@ DataAccessObject.prototype.remove =
             Model.emit('deleted', id);
           });
         }.bind(this));
-      });
+      }, null, cb);
     };
 
 /**
@@ -1198,7 +1198,8 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
             typedData[key] = inst[key];
           }
 
-          inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst), inst.constructor._forDB(typedData), function (err) {
+          inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst),
+            inst.constructor._forDB(typedData), function (err) {
             done.call(inst, function () {
               saveDone.call(inst, function () {
                 if(cb) cb(err, inst);
@@ -1206,8 +1207,8 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
               });
             });
           });
-        }, data);
-      }, data);
+        }, data, cb);
+      }, data, cb);
     }
   }, data);
 };

lib/hooks.js

@@ -27,7 +27,7 @@ Hookable.beforeDestroy = null;
 Hookable.afterDestroy = null;
 
 // TODO: Evaluate https://github.com/bnoguchi/hooks-js/
-Hookable.prototype.trigger = function trigger(actionName, work, data) {
+Hookable.prototype.trigger = function trigger(actionName, work, data, callback) {
   var capitalizedName = capitalize(actionName);
   var beforeHook = this.constructor["before" + capitalizedName]
     || this.constructor["pre" + capitalizedName];
@@ -42,8 +42,13 @@ Hookable.prototype.trigger = function trigger(actionName, work, data) {
   // we only call "before" hook when we have actual action (work) to perform
   if (work) {
     if (beforeHook) {
-      // before hook should be called on instance with one param: callback
+      // before hook should be called on instance with two parameters: next and data
       beforeHook.call(inst, function () {
+        // Check arguments to next(err, result)
+        if (arguments.length) {
+          return callback && callback.apply(null, arguments);
+        }
+        // No err & result is present, proceed with the real work
         // actual action also have one param: callback
         work.call(inst, next);
       }, data);

lib/validations.js

@@ -416,7 +416,7 @@ Validatable.prototype.isValid = function (callback, data) {
         validationsDone.call(inst, function () {
           callback(valid);
         });
-      });
+      }, data, callback);
     }
     return valid;
   }
@@ -467,7 +467,7 @@ Validatable.prototype.isValid = function (callback, data) {
       }
     }
 
-  }, data);
+  }, data, callback);
 
   if (async) {
     // in case of async validation we should return undefined here,

test/hooks.test.js

@@ -81,7 +81,27 @@ describe('hooks', function () {
       }
 
       User.afterCreate = function () {
-        throw new Error('shouldn\'t be called')
+        throw new Error('shouldn\'t be called');
+      };
+      User.create(function (err, user) {
+        User.dataSource.connector.create = old;
+        done();
+      });
+    });
+
+    it('afterCreate should not be triggered on failed beforeCreate', function (done) {
+      User.beforeCreate = function (next, data) {
+        // Skip next()
+        next(new Error('fail in beforeCreate'));
+      };
+
+      var old = User.dataSource.connector.create;
+      User.dataSource.connector.create = function (modelName, id, cb) {
+        throw new Error('shouldn\'t be called');
+      }
+
+      User.afterCreate = function () {
+        throw new Error('shouldn\'t be called');
       };
       User.create(function (err, user) {
         User.dataSource.connector.create = old;
@@ -173,6 +193,18 @@ describe('hooks', function () {
       });
     });
 
+    it('beforeSave should be able to skip next', function (done) {
+      User.create(function (err, user) {
+        User.beforeSave = function (next, data) {
+          next(null, 'XYZ');
+        };
+        user.save(function(err, result) {
+          result.should.be.eql('XYZ');
+          done();
+        });
+      });
+    });
+
   });
 
   describe('update', function () {
@@ -221,7 +253,7 @@ describe('hooks', function () {
 
     it('should not trigger after-hook on failed save', function (done) {
       User.afterUpdate = function () {
-        should.fail('afterUpdate shouldn\'t be called')
+        should.fail('afterUpdate shouldn\'t be called');
       };
       User.create(function (err, user) {
         var save = User.dataSource.connector.save;