Simple IoC Support for swaggerRouter

[steve-gray]

At the moment the swaggerRouter assumes that all required() modules are object-returns, with slots for each controller method. This means that you cannot effectively pass-in test control flags, value substitutions or pluggable modules (such as database providers).

This is leading some fairly evil brain-surgery type hacks in code that implement this, such as hooking into the controllers collection and dynamically rebuilding it with different instances loaded from different folders.

[confuser]

:+1:

This would allow for more flexible design patterns. However with #335, do alternatives such as sway provide such functionality?

[steve-gray]

Could folks not use sway to just build the injected value? I've submitted a PR for this feature, but didn't realise VScode accidentally checked in 3 build outputs. But it's there to see if you want to look.

Sent from my iPhone

On 4 Feb 2016, at 20:32, James Mortemore [email protected] wrote:

This would allow for more flexible design patterns. However with #335, do alternatives such as sway provide such functionality?

— Reply to this email directly or view it on GitHub.

[whitlockjc]

IoC seems to be outside the scope of this middleware. If we need it, I'm for it but in all honesty, swagger-router already provides a way for you to programmatically create the route handler map so for testing, you could generate this as part of your test setup. The swagger-router documentation says that if options.controllers is an object, it will be used as the route handler mapping instead of generating one based on the on-disk controller files.

I use this exact feature in swagger-tools' router tests to simplify things. Is this not enough?

[steve-gray]

How do you validate the real route handler calls the right implementations from outside your app, with a dummy back end.

For any sizeable app, there's a lot more going on than just your route handler and reaching into connects brain to swap out a link in the middleware chain is fairly yucky.

Sent from my iPhone

On 5 Feb 2016, at 04:36, Jeremy Whitlock [email protected] wrote:

IoC seems to be outside the scope of this middleware. If we need it, I'm for it but in all honesty, swagger-router already provides a way for you to programmatically create the route handler map so for testing, you could generate this as part of your test setup. The swagger-router documentation says that if options.controllers is an object, it will be used as the route handler mapping instead of generating one based on the on-disk controller files.

I use this exact feature in swagger-tools' router tests to simplify things. Is this not enough?

— Reply to this email directly or view it on GitHub.

[whitlockjc]

I'm not disagreeing, I'm just saying that this has nothing to do with Swagger so I question whether swagger-tools should be involved. If your route handler has some dependencies, creating the routing map should be enough to be able to instantiate/wrap your routers so that their dependencies are met.

Let me look at your PR to see what it's doing so I can have a better idea of the picture. Keep the conversation going, I'm not pushing back for anything more than to better understand so I make the right decision.

[steve-gray]

Essentially the problem I'm having is that by having the controller class effectively a parameterless singleton, you can't then readily use faked 'thing' it talks to. You can work around this by having the controller responsible for creating the dependencies - but then you end up with:

• a quantity of untestable code in those real controllers
• the repetition of dependency code in every controller
• can't test app-level behaviour with supertest without effectively mirroring the entire server root module and surrounding code where I build up the middleware.

Whereas with an IoC model I just fire up my whole server.js with a fake back end and throw it into supertest.

Does that make it a little clearer? If there's some way to achieve an equivalent outcome I missed, would appreciate the nudge in the direction. The straight up controller = require seems to be the lynchpin of the issue.

Sent from my iPhone

On 5 Feb 2016, at 05:12, Jeremy Whitlock [email protected] wrote:

I'm not disagreeing, I'm just saying that this has nothing to do with Swagger so I question whether swagger-tools should be involved. If your route handler has some dependencies, creating the routing map should be enough to be able to instantiate/wrap your routers so that their dependencies are met.

Let me look at your PR to see what it's doing so I can have a better idea of the picture. Keep the conversation going, I'm not pushing back for anything more than to better understand so I make the right decision.

— Reply to this email directly or view it on GitHub.

[whitlockjc]

How do you do this now? It sounds like you have some sort of initialization process for controllers and/or route handlers. Can I see an example?

[steve-gray]

I can't post actual examples as its work code: Ill mock an example when I'm at my desk in 90 minutes (it's 5am here ;)). The PR is basically clean-roomed at home.

The controller has code at the top that has to run creating dependencies - which has to be called, then return before the module finishes initialising. You then can't test controller because those dependencies are 'for life' and created on a path from require().

To address this you can make every method swap it's dependencies based on an externally shared state (process.env....) and go and flush the require() function internal state and cache. That was fairly evil though.

I did experiment with just hooking require() at test startup, but you find that measuring coverage with gulp-istabnbul gets very problematic because it also just flat out rewrites require to ensure the instrumented code is written, and changes paths.

Sent from my iPhone

On 5 Feb 2016, at 05:33, Jeremy Whitlock [email protected] wrote:

How do you do this now? It sounds like you have some sort of initialization process for controllers and/or route handlers. Can I see an example?

— Reply to this email directly or view it on GitHub.

[steve-gray]

Here's an es6'd controller, for example:

    thing-doer.js
        class ThingDoer {
            constructor(widget) {
                this.widget = widget;
            }
            handlePutWidget(req, res) {
                    if (!this.widget.exists(req.swagger.params.widgetKey.value)) 
                        this.widget.throb(req.swagger.params.widgetValue);
            }
        }
        module.exports = ThingDoer;

    somecontroller.js
        const ThingDoer = require('thing-doer');
        module.exports = function generator(injected) {
            return new ThingDoer(inejected.widget);
        }

With this pattern I could literally build my instances using any external IoC container (or just pass in some simple instances as keys for example). To use an IoC container, you'd pass that through as an injected property and then your generator function is actually just asking the IoC container to build your instance, instead of pulling out members - but its all application-level selectable because people can do whatever they want in their generators.

[steve-gray]

I was also going to put in some ES6 promises support, but I suspect that's possibly going to muddy the waters - for example, if my controller needed to wait for MongoDB to come up. I've worked around this by putting a 'status' operation on things, so I can test if it's up/down. That'll provide a load balancer health-test point too. Maybe one for the rewrite? :)

[whitlockjc]

Why not make your controllers created via a factory? Then async and dependencies are not an issue. And then you could also drive the options.controllers feature of swagger-router. Just a thought.

[steve-gray]

Hmm. Could do that doh! It'll trade in the ability to do dynamic discovery of modules though, but I can probably live with that.

[whitlockjc]

I'll continue to think as well.

[steve-gray]

The issue I'm having with the 'controllers' approach is that it expects inputs to be a function map of all resolved actions, so you can't just pass in a single controller and have the operations mapped to that. Effectively I need to build my instance, walk the functions, generate a map of controllerName_functionName to (req, res) => instance.functionName(req, res)

Am I missing something?

[whitlockjc]

I understand but that is how the routing middleware was written. The current approach for swagger-tools' routing middleware is to be capable of creating a route handler map based on controllers that exist in a configurable location and that export route handler functions. But, knowing that this isn't the only pattern out there, swagger-tools also allows you to compute your own route handler map. The goal was to make the simple case a configuration thing, or something we can figure out by convention, but still allow you to do things your own way with the chance that you could still use the routing middleware. I don't think it works for every case out there and I'm not sure it's possible to achieve such a thing. Here is a simplified example:

var MyController = require('./controllers/my').init(myDependencies);

app.use(middleware.swaggerRouter({
  controllers: {
    getAllUsers: MyController.getAllusers
  }
});

[josemf]

I'll just leave a possible solution to this issue. Needed some way to override the default controller/action routing mechanisms since this wouldn't work out in a framework I'm currently building.

Swagger routing controllers option makes it possible to pass an object that map <Controller>_<action> to action methods, it's possible to use the Proxy pattern such that the routing is totally customised.

function controllerLoader = (controller) => {
//... locate controller module, require it, initialise it, do any custom logic.
// return new (require(locateControllerModule(controller)))(Inject Something);
};

let router = new Proxy({ }, {
    get(target, propertyKey, receiver) {
        let controllerAction = propertyKey.split('_'),
             controller = controllerLoader(controllerAction[0]);
 
        // return an action callback from the controller
        return controller[controllerAction[1]].bind(controller);
    }
})

// ... validate swagger document, setup middlewares
app.use(middleware.swaggerRouter({
  controllers: router
});

Hope this will be of some use.