Request handling phases

[bajtos]

Now that we have loopback-phase, we should put it to use in loopback core and simplify the task of registering middleware in the correct order.

See strongloop/loopback-boot#46 for background discussion.

Tasks

• Implement a prototype of PhasedRouter in LoopBack - see RFC: phased router [do not merge] #757
• Get the prototype reviewed, decide whether it should be moved
  to a standalone project (e.g. strong-router).
• Finish the prototype, make it production-ready - see Middleware phases - initial implementation #767
• Add more predefined phases - see Middleware phases - initial implementation #767
• Add API for adding custom phases - see Implement app.defineMiddlewarePhases #786
• Clean up the implementation - see app.middleware: verify serial exec of handlers #792, Simplify app.defineMiddlewarePhases #790 and phase-list: implement .zipMerge(names) loopback-phase#7
• cleanup round 2 - Cleanup middleware config opts #796
• Write down a draft of the documentation - see Document middleware phases #863 and https://gist.github.com/bajtos/e7eaba736ff096916b71

Related

• Middleware Mounting loopback-boot#44 Middleware Mounting in loopback-boot

[bajtos]

As I was pondering the best implementation, I realised that preserving
backwards compatiblity is one of the the difficult parts.

Consider a simplified server/server.js application:

var app = loopback();
app.use(loopback.favicon());
app.use(loopback.compress());

boot(app, __dirname);
  // boot scripts add the following middleware
  app.use('/api', loopback.rest());
  app.use('/explorer', explorer(app));
  // most of the phased middleware is registered by boot too

app.use(loopback.static(path.resolve(__dirname, '../client')));

app.use(loopback.urlNotFound());
app.use(loopback.errorHandler());

The implementation of middleware phases must correctly blend with middleware
registered the old way.

Another challenge is to support the rich API provided by express Router, e.g.
route param parsing, handling of computed params, OPTIONS requests, etc.

My plan (proposal?) is to extend express.Router with phase API while
preserving the existing methods like use and post. All handlers registered
using the old API will be run as the first task in the phase route
(or perhaps main). This can be easily implemented by adding router.handle
function as the first phase callback. Such implementation makes it easy to
support all bells and whistles provided by the express router, as we are
not reimplementing the old API, it is handled by the express router itself.

Considering the example above, the middleware will be executed in the following
order:

// phase: initial
// phase: preprocess
// phase: session
// phase: auth
// phase: parse
// phase: log
// phase: route (or main)
favicon
compress
/api loopback.rest
/explorer explorer
loopback.static
loopback.urlNotFound
loopback.errorHandler
// phase: error

Let's add some middleware registered via phases to the mix:

var app = loopback();
app.use(loopback.favicon());
app.use(loopback.compress());

boot(app, __dirname);
  // boot scripts add the following middleware
  app.phase('auth').use(loopback.token());
  app.use('/api', loopback.rest());
  app.phase('log').use(morgan());
  app.use('/explorer', explorer(app));
  app.phase('session').use(session());

app.use(loopback.static(path.resolve(__dirname, '../client')));

app.use(loopback.urlNotFound());
app.use(loopback.errorHandler());

Now the middleware will be executed in the following order:

// phase: initial
// phase: preprocess
// phase: session
session
// phase: auth
loopback.token
// phase: parse
// phase: log
morgan
// phase: route (or main)
favicon
compress
/api loopback.rest
/explorer explorer
loopback.static
loopback.urlNotFound
loopback.errorHandler
// phase: error

Here are the cons of this solution: favicon should have been executed
much sooner and the error phase is practically a no-op (i.e. any handlers
registered with the error phase are ignored). The favicon problem should
not affect the functionality of the app at all, it's just an performance
optimization.

The issue with the ignored error phase should be encountered rather
infrequently, as most errors are handled inside loopback.rest and never hit
the global error handler.

My conclusion is that both shortcomings are minor and it is an acceptable price to pay for backwards compatibility.

Router design

/* Constructor */
var router = loopback.PhasedRouter();
  // stores original handler fn in router.expressHandler
  // calls router.phase('main').use(router.expressHandler);

/* Using the router as a middleware in express */
app.use(router)
  // express calls route.handle(req, res, next);
  // router.handle invokes all phases

/* Compatibility API */
router.use(compress());
  // calls `use` from express router

router.post('/foo', handleFn);
  // calls `post` from express router

/* Phase API */
router.phase('auth').use(function(req, res, next) { ... });
  // finds phase "auth"
  // calls phase.use(fn)

LoopBack integration

The express app created by loopback() will be configured to use
PhasedRouter instead of the default express.Router.

Next steps

• Implement a prototype of PhasedRouter in LoopBack
• Get the prototype reviewed, decide whether it should be moved
  to a standalone project (e.g. strong-router).
• Finish the prototype, make it production-ready
• Write down a draft of the documentation

[raymondfeng]

Some thoughts around your proposal:

1. We might be able to avoid PhasedRouter as follows:

Each phase will be backed by a router. There will be 3 child routers for each phase.

this.router = express.Router();
this.beforeRouter = express.Router();
this.useRouter = express.Router();
this.afterRouter = express.Router();

this.router.use(this.beforeRouter);
this.router.use(this.useRouter);
this.router.use(this.afterRouter);

phase.before(hander1) ==> phase.beforeRouter.use(handler1)

1. We should move middleware from server/server.js to boot scripts. The middleware should be registered with phases in boot scripts.

[bajtos]

Each phase will be backed by a router. There will be 3 child routers for each phase.

That's an interesting concept, I'll give it some though to figure out whether it is viable.

I guess my concern is about the complexity it brings to the table, as you will be able to specify route handlers in and sub-site middleware in phases too. Will the developer be able to make a mental picture of what middleware/route is invoked in which order for a given path? My proposal kept it simpler - phases are for site-wide middleware only.

We should move middleware from server/server.js to boot scripts. The middleware should be registered with phases in boot scripts.

Of course, that's the bright future where we are going to. However, we must support users that have an application scaffolded yesterday or even in the days of loopback 1.x.

[bajtos]

We should move middleware from server/server.js to boot scripts. The middleware should be registered with phases in boot scripts.

Actually, the middleware should be declared in a JSON file and loaded by loopback-boot, see strongloop/loopback-boot#44. You should not need any boot scripts for that unless you are doing something too advanced.

[raymondfeng]

Even better.