fix: hook proxies are not added when defined with hooks options (#8664)

lib/hooks.js

@@ -66,33 +66,41 @@ const getProxiedHooks = hookType =>
     : [hookType]
     : [hookType]
 ;
 ;
 
 
+function getHooks(hookType) {
+  return (this.options.hooks || {})[hookType] || [];
+};
+
 const Hooks = {
 const Hooks = {
-  replaceHookAliases(hooks) {
-    _.each(hooks, (hooksArray, name) => {
-      // Does an alias for this hook name exist?
-      const realHookName = hookAliases[name];
-      if (realHookName) {
-        // Add the hooks to the actual hook
-        hooks[realHookName] = (hooks[realHookName] || []).concat(hooksArray);
-
-        // Delete the alias
-        delete hooks[name];
-      }
+  /**
+   * Process user supplied hooks definition
+   *
+   * @param {Object} hooks
+   *
+   * @private
+   * @memberOf Sequelize
+   * @memberOf Sequelize.Model
+   */
+  _setupHooks(hooks) {
+    this.options.hooks = {};
+    _.map(hooks || {}, (hooksArray, hookName) => {
+      if (!_.isArray(hooksArray)) hooksArray = [hooksArray];
+      hooksArray.forEach(hookFn => this.addHook(hookName, hookFn));
     });
     });
-
-    return hooks;
   },
   },
 
 
   runHooks(hooks) {
   runHooks(hooks) {
-    if (!hooks) throw new Error('runHooks requires atleast 1 argument');
+    if (!hooks) throw new Error('runHooks requires at least 1 argument');
 
 
     const hookArgs = Utils.sliceArgs(arguments, 1);
     const hookArgs = Utils.sliceArgs(arguments, 1);
     let hookType;
     let hookType;
 
 
     if (typeof hooks === 'string') {
     if (typeof hooks === 'string') {
       hookType = hooks;
       hookType = hooks;
-      hooks = this.options.hooks[hookType] || [];
-      if (this.sequelize) hooks = hooks.concat(this.sequelize.options.hooks[hookType] || []);
+      hooks = getHooks.call(this, hookType);
+
+      if (this.sequelize) {
+        hooks = hooks.concat(getHooks.call(this.sequelize, hookType));
+      }
     }
     }
 
 
     if (!Array.isArray(hooks)) {
     if (!Array.isArray(hooks)) {
@@ -150,7 +158,7 @@ const Hooks = {
     hookType = getProxiedHooks(hookType);
     hookType = getProxiedHooks(hookType);
 
 
     _.each(hookType, type => {
     _.each(hookType, type => {
-      this.options.hooks[type] = this.options.hooks[type] || [];
+      this.options.hooks[type] = getHooks.call(this, type);
       this.options.hooks[type].push(name ? {name, fn} : fn);
       this.options.hooks[type].push(name ? {name, fn} : fn);
     });
     });
 
 

lib/model.js

@@ -761,7 +761,6 @@ class Model {
       schemaDelimiter: '',
       schemaDelimiter: '',
       defaultScope: {},
       defaultScope: {},
       scopes: [],
       scopes: [],
-      hooks: {},
       indexes: []
       indexes: []
     }, options);
     }, options);
 
 
@@ -771,10 +770,7 @@ class Model {
     }
     }
 
 
     this.associations = {};
     this.associations = {};
-    this.options.hooks = _.mapValues(this.replaceHookAliases(this.options.hooks), hooks => {
-      if (!Array.isArray(hooks)) hooks = [hooks];
-      return hooks;
-    });
+    this._setupHooks(options.hooks);
 
 
     this.underscored = this.underscored || this.underscoredAll;
     this.underscored = this.underscored || this.underscoredAll;
 
 

lib/sequelize.js

@@ -145,7 +145,7 @@ class Sequelize {
 
 
     Sequelize.runHooks('beforeInit', config, options);
     Sequelize.runHooks('beforeInit', config, options);
 
 
-    this.options = _.extend({
+    this.options = Object.assign({
       dialect: null,
       dialect: null,
       dialectModulePath: null,
       dialectModulePath: null,
       host: 'localhost',
       host: 'localhost',
@@ -188,7 +188,7 @@ class Sequelize {
       this.options.logging = console.log;
       this.options.logging = console.log;
     }
     }
 
 
-    this.options.hooks = this.replaceHookAliases(this.options.hooks);
+    this._setupHooks(options.hooks);
 
 
     if (['', null, false].indexOf(config.password) > -1 || typeof config.password === 'undefined') {
     if (['', null, false].indexOf(config.password) > -1 || typeof config.password === 'undefined') {
       config.password = null;
       config.password = null;

test/integration/hooks/hooks.test.js

@@ -467,6 +467,5 @@ describe(Support.getTestDialectTeaser('Hooks'), () => {
         expect(narutoHook).to.have.been.calledTwice;
         expect(narutoHook).to.have.been.calledTwice;
       });
       });
     });
     });
-
   });
   });
 });
 });

test/unit/hooks.test.js

@@ -26,6 +26,87 @@ describe(Support.getTestDialectTeaser('Hooks'), () => {
     });
     });
   });
   });
 
 
+  describe('proxies', () => {
+    beforeEach(() => {
+      sinon.stub(current, 'query').returns(Promise.resolve([{
+        _previousDataValues: {},
+        dataValues: {id: 1, name: 'abc'}
+      }]));
+    });
+
+    afterEach(() => {
+      current.query.restore();
+    });
+
+    describe('defined by options.hooks', () => {
+      beforeEach(() => {
+        this.beforeSaveHook = sinon.spy();
+        this.afterSaveHook = sinon.spy();
+        this.afterCreateHook = sinon.spy();
+
+        this.Model = current.define('m', {
+          name: Support.Sequelize.STRING
+        }, {
+          hooks: {
+            beforeSave: this.beforeSaveHook,
+            afterSave: this.afterSaveHook,
+            afterCreate: this.afterCreateHook
+          }
+        });
+      });
+
+      it('calls beforeSave/afterSave', () => {
+        return this.Model.create({}).then(() => {
+          expect(this.afterCreateHook).to.have.been.calledOnce;
+          expect(this.beforeSaveHook).to.have.been.calledOnce;
+          expect(this.afterSaveHook).to.have.been.calledOnce;
+        });
+      });
+    });
+
+    describe('defined by addHook method', () => {
+      beforeEach(() => {
+        this.beforeSaveHook = sinon.spy();
+        this.afterSaveHook = sinon.spy();
+
+        this.Model = current.define('m', {
+          name: Support.Sequelize.STRING
+        });
+
+        this.Model.addHook('beforeSave', this.beforeSaveHook);
+        this.Model.addHook('afterSave', this.afterSaveHook);
+      });
+
+      it('calls beforeSave/afterSave', () => {
+        return this.Model.create({}).then(() => {
+          expect(this.beforeSaveHook).to.have.been.calledOnce;
+          expect(this.afterSaveHook).to.have.been.calledOnce;
+        });
+      });
+    });
+
+    describe('defined by hook method', () => {
+      beforeEach(() => {
+        this.beforeSaveHook = sinon.spy();
+        this.afterSaveHook = sinon.spy();
+
+        this.Model = current.define('m', {
+          name: Support.Sequelize.STRING
+        });
+
+        this.Model.hook('beforeSave', this.beforeSaveHook);
+        this.Model.hook('afterSave', this.afterSaveHook);
+      });
+
+      it('calls beforeSave/afterSave', () => {
+        return this.Model.create({}).then(() => {
+          expect(this.beforeSaveHook).to.have.been.calledOnce;
+          expect(this.afterSaveHook).to.have.been.calledOnce;
+        });
+      });
+    });
+  });
+
   describe('multiple hooks', () => {
   describe('multiple hooks', () => {
     beforeEach(function() {
     beforeEach(function() {
       this.hook1 = sinon.spy();
       this.hook1 = sinon.spy();