Middleware phases - initial implementation

Modify the app and router implementation, so that the middleware is
executed in order defined by phases.

Predefined phases: 'init', 'routes', 'files', 'final'

Methods defined via `app.use`, `app.route` and friends are executed
as the first thing in 'routes' phase.

API usage:

    // get a Phase object
    app.phase('init');

    // register middleware handlers in a phase
    app.phase('init').before(handler);
    app.phase('init').use(handler);
    app.phase('init').after(errorHandler);

    // regular handler
    function handler(req, res, next) {
      // do stuff
      next();
    }

    // error handler
    function errorHandler(err, req, res, next) {
      // handle error and/or call next
      next(err);
    }

lib/application.js

@@ -11,6 +11,7 @@ var _ = require('underscore');
 var RemoteObjects = require('strong-remoting');
 var stringUtils = require('underscore.string');
 var path = require('path');
+var runtime = require('./runtime');
 
 /**
  * The `App` object represents a Loopback application.
@@ -43,6 +44,64 @@ function App() {
 
 var app = module.exports = {};
 
+if (runtime.isServer) {
+  var PhaseList = require('loopback-phase').PhaseList;
+  var MiddlewarePhase = require('./middleware-phase');
+
+  /**
+   * Get a middleware phase for the given name
+   * @param {string} name The phase name, e.g. "init" or "routes".
+   * @returns {MiddlewarePhase}
+   */
+  app.phase = function(name) {
+    this.lazyrouter();
+
+    // TODO(bajtos) We need .get(name) that throws a descriptive error
+    // when the phase was not found
+    return this._phases.find(name);
+  };
+
+  app.lazyrouter = function() {
+    var self = this;
+    if (self._router) return;
+
+    self.__expressLazyRouter();
+
+    // Storing the fn in another property of the router object
+    // allows us to call the method the router as `this`
+    // without then need to use slow `call` or `apply`.
+    self._router.__expressHandle = self._router.handle;
+
+    self._phases = new PhaseList();
+    self._phases.add(
+      ['init', 'routes', 'files', 'error']
+        .map(function(name) {
+          // TODO(bajtos) self._phase.add('foo') must create MiddlewarePhase
+          return new MiddlewarePhase(name);
+        })
+    );
+
+    self._phases.find('routes').use(function wrapError(err, req, res, next) {
+      req.__err = err;
+      next();
+    });
+
+    self.use(function unwrapError(req, res, next) {
+      var err = req.__err;
+      req.__err = undefined;
+      next(err);
+    });
+
+    self._phases.find('routes').use(function runRootHandlers(req, res, next) {
+      self._router.__expressHandle(req, res, next);
+    });
+
+    self._router.handle = function(req, res, next) {
+      self._phases.run({ req: req, res: res }, next);
+    };
+  };
+}
+
 /**
  * Lazily load a set of [remote objects](http://apidocs.strongloop.com/strong-remoting/#remoteobjectsoptions).
  *

lib/loopback.js

@@ -54,6 +54,8 @@ loopback.mime = express.mime;
 function createApplication() {
   var app = express();
 
+  app.__expressLazyRouter = app.lazyrouter;
+
   merge(app, proto);
 
   app.loopback = loopback;

lib/middleware-phase.js

@@ -0,0 +1,53 @@
+var assert = require('assert');
+var Phase = require('loopback-phase').Phase;
+var inherits = require('util').inherits;
+
+/**
+ * MiddlewarePhase accepts middleware functions for `use`, `before` and `after`.
+ *
+ * @param {string} id The phase name (id).
+ * @class MiddlewarePhase
+ */
+function MiddlewarePhase(id) {
+  Phase.apply(this, arguments);
+}
+
+module.exports = MiddlewarePhase;
+
+inherits(MiddlewarePhase, Phase);
+
+// TODO(bajtos) add a customization hook to Phase
+['before', 'use', 'after'].forEach(function decorate(name) {
+  MiddlewarePhase.prototype[name] = function(fn) {
+    var wrapper;
+
+    if (fn.length === 4) {
+      wrapper = function errorHandler(ctx, next) {
+        if (ctx.err) {
+          var err = ctx.err;
+          ctx.err = undefined;
+          fn(err, ctx.req, ctx.res, storeErrorAndContinue(ctx, next));
+        } else {
+          next();
+        }
+      };
+    } else {
+      wrapper = function regularHandler(ctx, next) {
+        if (ctx.err) {
+          next();
+        } else {
+          fn(ctx.req, ctx.res, storeErrorAndContinue(ctx, next));
+        }
+      };
+    }
+
+    MiddlewarePhase.super_.prototype[name].call(this, wrapper);
+  };
+});
+
+function storeErrorAndContinue(ctx, next) {
+  return function(err) {
+    if (err) ctx.err = err;
+    next();
+  };
+}

package.json

@@ -78,6 +78,7 @@
     "karma-script-launcher": "~0.1.0",
     "loopback-boot": "^1.1.0",
     "loopback-datasource-juggler": "^2.8.0",
+    "loopback-phase": "^1.0.0",
     "loopback-testing": "~0.2.0",
     "mocha": "~1.21.4",
     "serve-favicon": "~2.1.3",

test/app.test.js

@@ -7,6 +7,72 @@ var describe = require('./util/describe');
 var it = require('./util/it');
 
 describe('app', function() {
+  describe.onServer('router', function() {
+    var reqStub;
+    var resStub;
+    var steps;
+
+    beforeEach(function setup() {
+      reqStub = { url: '/test/url', verb: 'GET' };
+      resStub = {
+        setHeader: function() {}
+      };
+      steps = [];
+    });
+
+    function namedHandler(name) {
+      return function(req, res, next) {
+        steps.push(name);
+        next();
+      };
+    }
+
+    it('supports phases', function(done) {
+      var app = loopback();
+      app.phase('init').use(namedHandler('init'));
+      app.phase('files').use(namedHandler('files'));
+      app.use(namedHandler('main'));
+
+      app.handle(reqStub, resStub, safeDone(done, function(err) {
+        if (err) return done(err);
+        expect(steps).to.eql(['init', 'main', 'files']);
+        done();
+      }));
+    });
+
+    it('injects error from previous phases into router', function(done) {
+      var app = loopback();
+      var expectedError = new Error('expected error');
+
+      app.phase('init').use(function(req, res, next) {
+        steps.push('init');
+        next(expectedError);
+      });
+
+      // legacy solution for handling errors
+      app.use(function errorHandler(err, req, res, next) {
+        steps.push('error');
+        next();
+      });
+
+        app.handle(reqStub, resStub, safeDone(done, function(err) {
+        if (err) return done(err);
+        expect(steps).to.eql(['init', 'error']);
+        done();
+      }));
+    });
+
+    // Workaround for https://github.com/strongloop/expressjs.com/issues/270
+    function safeDone(realDone, fn) {
+      return function(err) {
+        try {
+          return fn(err);
+        } catch (e) {
+          return realDone(e);
+        }
+      };
+    }
+  });
 
   describe('app.model(Model)', function() {
     var app, db;

test/middleware-phase.test.js

@@ -0,0 +1,65 @@
+var extend = require('util')._extend;
+var MiddlewarePhase = require('../lib/middleware-phase');
+var expect = require('chai').expect;
+
+describe('middleware-phase', function() {
+  it('executes middleware in the correct order', function(done) {
+    var phase = new MiddlewarePhase();
+    var steps = [];
+    var ctx = givenRouterContext();
+
+    var handlerFn = function(name) {
+      return function(req, res, next) {
+        expect(req, name + '.req').to.equal(ctx.req);
+        expect(res, name + '.res').to.equal(ctx.res);
+        steps.push(name);
+        next();
+      };
+    };
+
+    phase.use(handlerFn('use'));
+    phase.before(handlerFn('before'));
+    phase.after(handlerFn('after'));
+
+    phase.run(ctx, function verify(err) {
+      if (err) return done(err);
+      expect(steps).to.eql(['before', 'use', 'after']);
+      done();
+    });
+  });
+
+  it('passes errors to the next handler', function(done) {
+    var phase = new MiddlewarePhase();
+    var expectedError = new Error('expected error');
+
+    phase.before(function(req, res, next) {
+      next(expectedError);
+    });
+
+    phase.after(function(err, req, res, next) {
+      expect(err).to.equal(expectedError);
+      done();
+    });
+
+    phase.run(givenRouterContext(), function(err) {
+      if (err && err !== expectedError) return done(err);
+      done(new Error(
+        'The handler chain should have been stopped by error handler'));
+    });
+  });
+});
+
+function givenRequest(props) {
+  return extend({ url: '/test/url', method: 'GET' }, props);
+}
+
+function givenResponse(props) {
+  return extend({}, props);
+}
+
+function givenRouterContext(req, res) {
+  return {
+    req: givenRequest(req),
+    res: givenResponse(res)
+  };
+}