perf(lucid:hooks): resolve hooks when adding

resolving hooks on add will be a one time process to whereas earlier they were resolved on each

action

package.json

@@ -42,6 +42,7 @@
     }
   },
   "dependencies": {
+    "adonis-binding-resolver": "^1.0.1",
     "auto-loader": "git+https://github.com/thetutlage/node-auto-loader.git",
     "cat-log": "^1.0.2",
     "chance": "^1.0.3",

src/Lucid/Model/Mixins/Hooks.js

@@ -11,7 +11,6 @@
 
 const Hooks = exports = module.exports = {}
 const _ = require('lodash')
-const Ioc = require('adonis-fold').Ioc
 
 /**
  * compose hooks for a given type by reading values
@@ -25,18 +24,9 @@ const Ioc = require('adonis-fold').Ioc
  * @public
  */
 Hooks.getHooks = function (type, handler) {
-  let beforeHooks = []
-  let afterHooks = []
-  const modelHooks = this.constructor.$modelHooks
-  if (!modelHooks) {
-    return [handler]
-  }
-  if (modelHooks[`before${type}`]) {
-    beforeHooks = _.map(modelHooks[`before${type}`], function (hook) { return hook.handler })
-  }
-  if (modelHooks[`after${type}`]) {
-    afterHooks = _.map(modelHooks[`after${type}`], function (hook) { return hook.handler })
-  }
+  const modelHooks = this.constructor.$modelHooks || {}
+  const beforeHooks = _.map(modelHooks[`before${type}`], 'handler') || []
+  const afterHooks = _.map(modelHooks[`after${type}`], 'handler') || []
   return beforeHooks.concat([handler]).concat(afterHooks)
 }
 
@@ -53,21 +43,17 @@ Hooks.getHooks = function (type, handler) {
  * @public
  */
 Hooks.composeHooks = function (scope, hooks) {
-  const Helpers = Ioc.use('Adonis/Src/Helpers')
-  const hookNameSpace = 'Model/Hooks'
-
   function * noop () {}
+
   return function * (next) {
     next = next || noop()
-    let i = hooks.length
-    while (i--) {
-      let hook = hooks[i]
-      if (typeof (hooks[i]) === 'string') {
-        const resolvedHook = Ioc.makeFunc(`${Helpers.makeNameSpace(hookNameSpace, hooks[i])}`)
-        hook = resolvedHook.instance[resolvedHook.method]
+    _.forEachRight(hooks, (hook) => {
+      if (typeof (hook) === 'function') {
+        next = hook.apply(scope, [next])
+      } else if (hook.instance && hook.method) {
+        next = hook.instance[hook.method].apply(scope, [next])
       }
-      next = hook.apply(scope, [next])
-    }
+    })
     yield * next
   }
 }

src/Lucid/Model/index.js

@@ -22,6 +22,11 @@ const proxyHandler = require('./proxyHandler')
 const Mixins = require('./Mixins')
 const Relations = require('../Relations')
 const BaseSerializer = require('../QueryBuilder/Serializers/Base')
+const Ioc = require('adonis-fold').Ioc
+const Resolver = require('adonis-binding-resolver')
+const resolver = new Resolver(Ioc)
+
+const hookNameSpace = 'Model/Hooks'
 
 /**
  * returns a function that be executed with a key/value
@@ -104,6 +109,22 @@ class Model {
     this.setJSON(values)
   }
 
+  /**
+   * validates whether hooks is of valid type
+   * or not
+   *
+   * @method  _validateIsValidHookType
+   *
+   * @param   {String}                 type
+   *
+   * @private
+   */
+  static _validateIsValidHookType (type) {
+    if (validHookTypes.indexOf(type) <= -1) {
+      throw CE.InvalidArgumentException.invalidParameter(`${type} is not a valid hook type`)
+    }
+  }
+
   /**
    * adds a new hook for a given type for a model. Note
    * this method has no way of checking duplicate
@@ -123,30 +144,19 @@ class Model {
    * @public
    */
   static addHook (type, name, handler) {
-    if (validHookTypes.indexOf(type) <= -1) {
-      throw CE.InvalidArgumentException.invalidParameter(`${type} is not a valid hook type`)
-    }
+    this._validateIsValidHookType(type)
+    const Helpers = Ioc.use('Adonis/Src/Helpers')
 
-    /**
-     * if handler is not defined, set name as handler. It is required coz
-     * we have 2nd parameter optional.
-     */
     if (!handler) {
       handler = name
       name = null
     }
 
-    /**
-     * handler should be a reference to a string or a valid function. Strings are
-     * assumed to be a reference to hook namespace and if autoloading fails an
-     * error will be thrown when calling hook
-     */
-    if (typeof (handler) !== 'function' && typeof (handler) !== 'string') {
-      throw CE.InvalidArgumentException.invalidParameter('hook handler must point to a valid generator method')
-    }
+    const resolvedHandler = typeof (handler) === 'string' ? Helpers.makeNameSpace(hookNameSpace, handler) : handler
+    resolver.validateBinding(resolvedHandler)
 
     this.$modelHooks[type] = this.$modelHooks[type] || []
-    this.$modelHooks[type].push({handler, name})
+    this.$modelHooks[type].push({handler: resolver.resolveBinding(resolvedHandler), name})
   }
 
   /**

test/unit/app/Model/Hooks/Users.js

@@ -6,3 +6,7 @@ UsersHooks.validate = function * (next) {
   this.username = 'viahook'
   yield next
 }
+
+UsersHooks.log = function * (next) {
+  yield next
+}

test/unit/lucid.spec.js

@@ -1306,7 +1306,7 @@ describe('Lucid', function () {
       const fn = function () {
         User.addHook('beforeCreate')
       }
-      expect(fn).to.throw('InvalidArgumentException: E_INVALID_PARAMETER: hook handler must point to a valid generator method')
+      expect(fn).to.throw('InvalidArgumentException: E_INVALID_IOC_BINDING: Handler must point to a valid namespace or a closure')
     })
 
     it('should add a hook for a given type', function () {
@@ -1356,22 +1356,26 @@ describe('Lucid', function () {
     it('should be able to define multiple hooks in a go', function () {
       class User extends Model {}
       User.bootIfNotBooted()
-      User.defineHooks('beforeCreate', 'UsersHook.validate', 'UsersHook.log')
+      User.defineHooks('beforeCreate', 'Users.validate', 'Users.log')
       expect(User.$modelHooks['beforeCreate']).to.be.an('array')
       expect(User.$modelHooks['beforeCreate'].length).to.equal(2)
-      expect(User.$modelHooks['beforeCreate'][0].handler).to.equal('UsersHook.validate')
-      expect(User.$modelHooks['beforeCreate'][1].handler).to.equal('UsersHook.log')
+      expect(User.$modelHooks['beforeCreate'][0].handler).to.have.property('instance')
+      expect(User.$modelHooks['beforeCreate'][0].handler).to.have.property('method')
+      expect(User.$modelHooks['beforeCreate'][1].handler).to.have.property('instance')
+      expect(User.$modelHooks['beforeCreate'][1].handler).to.have.property('method')
     })
 
     it('should override existing hooks when calling defineHooks', function () {
       class User extends Model {}
       User.bootIfNotBooted()
       User.addHook('beforeCreate', function * () {})
-      User.defineHooks('beforeCreate', 'UsersHook.validate', 'UsersHook.log')
+      User.defineHooks('beforeCreate', 'Users.validate', 'Users.log')
       expect(User.$modelHooks['beforeCreate']).to.be.an('array')
       expect(User.$modelHooks['beforeCreate'].length).to.equal(2)
-      expect(User.$modelHooks['beforeCreate'][0].handler).to.equal('UsersHook.validate')
-      expect(User.$modelHooks['beforeCreate'][1].handler).to.equal('UsersHook.log')
+      expect(User.$modelHooks['beforeCreate'][0].handler).to.have.property('instance')
+      expect(User.$modelHooks['beforeCreate'][0].handler).to.have.property('method')
+      expect(User.$modelHooks['beforeCreate'][1].handler).to.have.property('instance')
+      expect(User.$modelHooks['beforeCreate'][1].handler).to.have.property('method')
     })
 
     it('should execute beforeCreate hook when a model is saved to the database', function * () {