From cd89299163a65c2b2bfd014e9b5cec36fe8b808d Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 12 Dec 2014 18:15:24 -0800 Subject: [PATCH] Make sure middleware layer sorting is stable See https://github.com/strongloop/strong-arc/issues/767 --- lib/server-app.js | 25 ++++++++++++++++++++++--- test/app.test.js | 13 +++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/server-app.js b/lib/server-app.js index 7788dd9e..46070df4 100644 --- a/lib/server-app.js +++ b/lib/server-app.js @@ -214,13 +214,22 @@ proto._sortLayersByPhase = function() { }); var router = this._router; + + // See https://code.google.com/p/v8/issues/detail?id=90 + router.stack.forEach(function(layer, ix) { + layer.order = ix; + }); router.stack.sort(compareLayers); function compareLayers(left, right) { var leftPhase = left.phase; var rightPhase = right.phase; - if (leftPhase === rightPhase) return 0; + var result; + if (leftPhase === rightPhase) { + // Keep the order (stable sorting) + return left.order - right.order; + } // Builtin middleware is always first if (leftPhase === BUILTIN_MIDDLEWARE) return -1; @@ -232,13 +241,23 @@ proto._sortLayersByPhase = function() { if (rightPhase === 'routes') return -1; - return phaseOrder['routes'] - phaseOrder[rightPhase]; + result = phaseOrder['routes'] - phaseOrder[rightPhase]; + if (result === 0) { + return left.order - right.order; + } else { + return result; + } } if (rightPhase === undefined) return -compareLayers(right, left); // Layers registered via `app.middleware` are compared via phase & hook - return phaseOrder[leftPhase] - phaseOrder[rightPhase]; + result = phaseOrder[leftPhase] - phaseOrder[rightPhase]; + if (result === 0) { + return left.order - right.order; + } else { + return result; + } } }; diff --git a/test/app.test.js b/test/app.test.js index 978773ac..946342f3 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -297,6 +297,19 @@ describe('app', function() { }); }); + it('keep orders of middleware added by app.use()', function(done) { + var expectedSteps = []; + for (var i = 0; i < 10; i++) { + app.use(namedHandler('m' + i)); + expectedSteps.push('m' + i); + } + executeMiddlewareHandlers(app, function(err) { + if (err) return done(err); + expect(steps).to.eql(expectedSteps); + done(); + }); + }); + function namedHandler(name) { return function(req, res, next) { steps.push(name);