TDD Introduce dependency injection

[sielay]

TDD Introduce dependency injection

Type

• Feature Request

  Description

This would be Phase 1 of migrating to DI based structure.

Currently we load controllers, models and other asynchronous modules in following way

module.exports = require(MODEL_PATH)
    .then(Model => {
      class Controller {
         // do something with model
      }
      return Controller;
   });

or for more dependencies

module.exports = SUtils.deps(
   'Controller Name',
    require(MODEL_PATH),
    require(MODEL2_PATH)
    .then((Model, Model2) => {
      class Controller {
         // do something with model
      }
      return Controller;
   });

To allow easier Unit Tests and more flexible code we should move dependencies one level higher. For beginning we would start with controllers. This way we'd be able to test controller itself using model mockups only (no actual operations on database).

Examples

Option 1 - controller factory

// controller
module.exports = (model1, model2) => {
   class Controller {
   }
   return Controller;
};

// route
Promise
    .all([
        require(MODEL1_PATH),
        require(MODEL2_PATH)
    )   
   .then(models => require(CONTROLLER_PATH)(models[0], models[1])
   .then(controller => server.route('/path').get(controller.someMethod.bind(controller));

Option 2 - utils helper

// controller
module.exports = (model1, model2) => {
   class Controller {
   }
   return Controller;
};

// route
SUtils
    .controller(MODULE, CONTROLLER, model1, model2)
   .then(controller => server.route('/path').get(controller.someMethod.bind(controller));

Option 3 - method factory

// parent class
class Injectable {
    static bind(...promises) { ... } // makes internally function.bind(this)
}
// controller
class Controller extends Injectable {
    static methodA(req, res, next, id, model1, model2) { ... }
}
module.exports = Controller;

// route

const
    model1Promise = require(...),
    model1Promise = require(...);  // or SUtils.cmsMod(module).model(modelName) or SUtils.model(module, modelName) as shorthand

server.route('/path').get(controller.methodA.bind(model1Promise, model2Promise));

[sielay]

Examples https://blog.risingstack.com/dependency-injection-in-node-js/

[sielay]

One lib doing Angular way (required name dependencies, https://github.com/gobwas/dm.js)

[sielay]

another similar, https://github.com/archiejs/archiejs don't like in both that they hide required content in closure.

[sielay]

I think now more about

// controller
class Controller {
    static methodA(req, res, next, id, model1, model2) { ... }
}
module.exports = Controller; // to not lock inheritance

// route

const
    model1Promise = require(...),
    model1Promise = require(...);  // or SUtils.cmsMod(module).model(modelName) or SUtils.model(module, modelName) as shorthand

server.route('/path').get(inject(controller, controller.methodA, model1Promise, model2Promise));

Pros

• Doesn't block inheritance
• Doesn't force wrap class in any magic
• Allow injecting to standalone functions

[sielay]

My suggestion https://github.com/sielay/ditoolkit

[sielay]

Other idea based on http://www.devtrends.co.uk/blog/how-not-to-do-dependency-injection-the-static-or-singleton-container

class Controller extends IoCContainer {
    constructor(options) {
        super(options);
        (..)
    }
    getList(req, res) {
        this.resolve('model1')
        then(model1 => ...);
    }
}

new Controller({
        dependencies: {
            model1Factory : () => ...
        }
});

but this still cause situation that, if method have many dependencies you end up with pyramid of then.

We could use DITookit way, inject dependencies to class constructor and override methods to be executed once all dependencies are resolved so it would look like:

class Controller extends IoCContainer {
    constructor(options) {
        super(options);
        (..)
    }
    getList(req, res) {
        this.dependencies.model1.doSomething();
    }
}

new Controller({
        dependencies: {
            model1Factory : () => ... // but even factory is not needed in that case, Promise is enough
        }
});

[sielay]

To experiment I added support for DI in form similar to how Symphony does it - by configuration.

Configuration that specify what to inject https://github.com/sielay/lackey-cms/blob/35ee3fe54a9806f0e69bc588db955e71d1286033/modules/analytics/module.yml#L8

Controller that receive injections https://github.com/sielay/lackey-cms/blob/35ee3fe54a9806f0e69bc588db955e71d1286033/modules/analytics/server/controllers/dashboard.js#L34

Thanks to this way

• we limit number of untestable lines (routes are trickiest to test)
• we allow injecting mockups to controllers for testing
• we allow async/promised dependencies, making controller code cleaner and more streamline

If no one oppose this way, we'd move to this form with all lackey routes before version 0.10.0 and keep support for old ones until 1.0.0

[sielay]

Implemented