From 4e1433b5196470fd7d0677103e6a43c9d141cb80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <miroslav@strongloop.com>
Date: Wed, 5 Nov 2014 20:07:58 +0100
Subject: [PATCH] Middleware phases - initial implementation

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

Predefined phases:

    'initial', 'session', 'auth', 'parse', 'routes', 'files', 'final'

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

API usage:

    app.middleware('initial', compression());
    app.middleware('initial:before', serveFavicon());
    app.middleware('files:after', loopback.urlNotFound());
    app.middleware('final:after', errorHandler());

Middleware flavours:

    // 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/loopback.js   |   2 +-
 lib/server-app.js | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 package.json      |   4 +-
 test/app.test.js  |  98 +++++++++++++++++++++++++++++++++++-
 4 files changed, 226 insertions(+), 3 deletions(-)
 create mode 100644 lib/server-app.js

diff --git a/lib/loopback.js b/lib/loopback.js
index 9d8351ce..b3ee7a6e 100644
--- a/lib/loopback.js
+++ b/lib/loopback.js
@@ -2,7 +2,7 @@
  * Module dependencies.
  */
 
-var express = require('express');
+var express = require('./server-app');
 var proto = require('./application');
 var fs = require('fs');
 var ejs = require('ejs');
diff --git a/lib/server-app.js b/lib/server-app.js
new file mode 100644
index 00000000..07097eb9
--- /dev/null
+++ b/lib/server-app.js
@@ -0,0 +1,125 @@
+var express = require('express');
+var merge = require('util')._extend;
+var PhaseList = require('loopback-phase').PhaseList;
+var debug = require('debug')('loopback:app');
+
+var proto = {};
+
+module.exports = function loopbackExpress() {
+  var app = express();
+  app.__expressLazyRouter = app.lazyrouter;
+  merge(app, proto);
+  return app;
+};
+
+/**
+ * Register a middleware handler to be executed in a given phase.
+ * @param {string} name The phase name, e.g. "init" or "routes".
+ * @param {function} handler The middleware handler, one of
+ *   `function(req, res, next)` or
+ *   `function(err, req, res, next)`
+ * @returns {object} this (fluent API)
+ */
+proto.middleware = function(name, handler) {
+  this.lazyrouter();
+
+  var fullName = name;
+  var handlerName = handler.name || '(anonymous)';
+
+  var hook = 'use';
+  var m = name.match(/^(.+):(before|after)$/);
+  if (m) {
+    name = m[1];
+    hook = m[2];
+  }
+
+  var phase = this._requestHandlingPhases.find(name);
+  if (!phase)
+    throw new Error('Unknown middleware phase ' + name);
+
+  var wrapper;
+  if (handler.length === 4) {
+    // handler is function(err, req, res, next)
+    debug('Add error handler %j to phase %j', handlerName, fullName);
+
+    wrapper = function errorHandler(ctx, next) {
+      if (ctx.err) {
+        var err = ctx.err;
+        ctx.err = undefined;
+        handler(err, ctx.req, ctx.res, storeErrorAndContinue(ctx, next));
+      } else {
+        next();
+      }
+    };
+  } else {
+    // handler is function(req, res, next)
+    debug('Add middleware %j to phase %j', handlerName , fullName);
+    wrapper = function regularHandler(ctx, next) {
+      if (ctx.err) {
+        next();
+      } else {
+        handler(ctx.req, ctx.res, storeErrorAndContinue(ctx, next));
+      }
+    };
+  }
+
+  phase[hook](wrapper);
+  return this;
+};
+
+function storeErrorAndContinue(ctx, next) {
+  return function(err) {
+    if (err) ctx.err = err;
+    next();
+  };
+}
+
+// Install our custom PhaseList-based handler into the app
+proto.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 with the router as `this`
+  // without the need to use slow `call` or `apply`.
+  self._router.__expressHandle = self._router.handle;
+
+  self._requestHandlingPhases = new PhaseList();
+  self._requestHandlingPhases.add([
+    'initial', 'session', 'auth', 'parse',
+    'routes', 'files', 'final'
+  ]);
+
+  // In order to pass error into express router, we have
+  // to pass it to a middleware executed from within the router.
+  // This is achieved by adding a phase-handler that wraps the error
+  // into `req` object and then a router-handler that unwraps the error
+  // and calls `next(err)`.
+  // It is important to register these two handlers at the very beginning,
+  // before any other handlers are added.
+  self.middleware('routes', 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.middleware('routes', function runRootHandlers(req, res, next) {
+    self._router.__expressHandle(req, res, next);
+  });
+
+  // Overwrite the original handle() function provided by express,
+  // replace it with our implementation based on PhaseList
+  self._router.handle = function(req, res, next) {
+    var ctx = { req: req, res: res };
+    self._requestHandlingPhases.run(ctx, function(err) {
+      next(err || ctx.err);
+    });
+  };
+};
diff --git a/package.json b/package.json
index 71276842..09b5936d 100644
--- a/package.json
+++ b/package.json
@@ -38,7 +38,7 @@
     "canonical-json": "0.0.4",
     "debug": "~2.0.0",
     "ejs": "~1.0.0",
-    "express": "4.x",
+    "express": "^4.10.2",
     "inflection": "~1.4.2",
     "loopback-connector-remote": "^1.0.1",
     "nodemailer": "~1.3.0",
@@ -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.1",
     "loopback-testing": "~0.2.0",
     "mocha": "~1.21.4",
     "serve-favicon": "~2.1.3",
@@ -90,6 +91,7 @@
   },
   "browser": {
     "express": "./lib/browser-express.js",
+    "./lib/server-app.js": "./lib/browser-express.js",
     "connect": false,
     "nodemailer": false
   },
diff --git a/test/app.test.js b/test/app.test.js
index a7b2710f..9323ea56 100644
--- a/test/app.test.js
+++ b/test/app.test.js
@@ -1,5 +1,5 @@
 var path = require('path');
-var SIMPLE_APP = path.join(__dirname, 'fixtures', 'simple-app');
+var http = require('http');
 var loopback = require('../');
 var PersistedModel = loopback.PersistedModel;
 
@@ -7,6 +7,102 @@ var describe = require('./util/describe');
 var it = require('./util/it');
 
 describe('app', function() {
+  describe.onServer('.middleware(phase, handler)', function() {
+    var app;
+    var steps;
+
+    beforeEach(function setup() {
+      app = loopback();
+      steps = [];
+    });
+
+    it('runs middleware in phases', function(done) {
+      var PHASES = [
+        'initial', 'session', 'auth', 'parse',
+        'routes', 'files', 'final'
+      ];
+
+      PHASES.forEach(function(name) {
+        app.middleware(name, namedHandler(name));
+      });
+      app.use(namedHandler('main'));
+
+      executeHandlers(function(err) {
+        if (err) return done(err);
+        expect(steps).to.eql([
+          'initial', 'session', 'auth', 'parse',
+          'main', 'routes', 'files', 'final'
+        ]);
+        done();
+      });
+    });
+
+    it('supports "before:" and "after:" prefixes', function(done) {
+      app.middleware('routes:before', namedHandler('routes:before'));
+      app.middleware('routes:after', namedHandler('routes:after'));
+      app.use(namedHandler('main'));
+
+      executeHandlers(function(err) {
+        if (err) return done(err);
+        expect(steps).to.eql(['routes:before', 'main', 'routes:after']);
+        done();
+      });
+    });
+
+    it('injects error from previous phases into the router', function(done) {
+      var expectedError = new Error('expected error');
+
+      app.middleware('initial', function(req, res, next) {
+        steps.push('initial');
+        next(expectedError);
+      });
+
+      // legacy solution for error handling
+      app.use(function errorHandler(err, req, res, next) {
+        expect(err).to.equal(expectedError);
+        steps.push('error');
+        next();
+      });
+
+      executeHandlers(function(err) {
+        if (err) return done(err);
+        expect(steps).to.eql(['initial', 'error']);
+        done();
+      });
+    });
+
+    it('passes unhandled error to callback', function(done) {
+      var expectedError = new Error('expected error');
+
+      app.middleware('initial', function(req, res, next) {
+        next(expectedError);
+      });
+
+      executeHandlers(function(err) {
+        expect(err).to.equal(expectedError);
+        done();
+      });
+    });
+
+    function namedHandler(name) {
+      return function(req, res, next) {
+        steps.push(name);
+        next();
+      };
+    }
+
+    function executeHandlers(callback) {
+      var server = http.createServer(function(req, res) {
+        app.handle(req, res, callback);
+      });
+
+      request(server)
+        .get('/test/url')
+        .end(function(err) {
+          if (err) return callback(err);
+        });
+    }
+  });
 
   describe('app.model(Model)', function() {
     var app, db;