Let filter take a promise form. (#341)

Author: monkpow

app/steps/filterUserRequest.js

@@ -0,0 +1,24 @@
+'use strict';
+
+// No-op version of filter.  Allows everything!
+
+function defaultFilter(proxyReqOptBuilder, userReq) { // eslint-disable-line
+  return true;
+}
+
+function filterUserRequest(container) {
+  var resolverFn = container.options.filter || defaultFilter;
+
+  return Promise
+    .resolve(resolverFn(container.proxy.reqBuilder, container.user.req))
+    .then(function (shouldIContinue) {
+      if (shouldIContinue) {
+        return Promise.resolve(container);
+      } else {
+        return Promise.reject(); // reject with no args should simply call next()
+      }
+    });
+}
+
+module.exports = filterUserRequest;
+

index.js

@@ -4,7 +4,6 @@
 // * Uses Promises to support async.
 // * Uses a quasi-Global called Container to tidy up the argument passing between the major work-flow steps.
 
-
 var ScopeContainer = require('./lib/scopeContainer');
 var assert = require('assert');
 var debug = require('debug')('express-http-proxy');
@@ -15,7 +14,8 @@ var decorateProxyReqBody         = require('./app/steps/decorateProxyReqBody');
 var decorateProxyReqOpts         = require('./app/steps/decorateProxyReqOpts');
 var decorateUserRes              = require('./app/steps/decorateUserRes');
 var decorateUserResHeaders       = require('./app/steps/decorateUserResHeaders');
-var handleProxyErrors          = require('./app/steps/handleProxyErrors');
+var filterUserRequest            = require('./app/steps/filterUserRequest');
+var handleProxyErrors            = require('./app/steps/handleProxyErrors');
 var maybeSkipToNextHandler       = require('./app/steps/maybeSkipToNextHandler');
 var prepareProxyReq              = require('./app/steps/prepareProxyReq');
 var resolveProxyHost             = require('./app/steps/resolveProxyHost');
@@ -33,9 +33,10 @@ module.exports = function proxy(host, userOptions) {
     // Skip proxy if filter is falsey.  Loose equality so filters can return
     // false, null, undefined, etc.
 
-    if (!container.options.filter(req, res)) { return next(); }
+    //if (!container.options.filter(req, res)) { return next(); }
 
-    buildProxyReq(container)
+    filterUserRequest(container)
+      .then(buildProxyReq)
       .then(resolveProxyHost)
       .then(decorateProxyReqOpts)
       .then(resolveProxyReqPath)
@@ -48,10 +49,17 @@ module.exports = function proxy(host, userOptions) {
       .then(decorateUserRes)
       .then(sendUserRes)
       .catch(function (err) {
-        var resolver = (container.options.proxyErrorHandler) ?
-          container.options.proxyErrorHandler :
-          handleProxyErrors;
-        resolver(err, res, next);
+        // I sometimes reject without an error to shortcircuit the remaining
+        // steps and return control to the host application.
+
+        if (err) {
+          var resolver = (container.options.proxyErrorHandler) ?
+            container.options.proxyErrorHandler :
+            handleProxyErrors;
+          resolver(err, res, next);
+        } else {
+          next();
+        }
       });
   };
 };

lib/resolveOptions.js

@@ -50,7 +50,7 @@ function resolveOptions(options) {
     userResDecorator: options.userResDecorator || options.intercept,
     userResHeaderDecorator: options.userResHeaderDecorator,
     proxyErrorHandler: options.proxyErrorHandler,
-    filter: options.filter || defaultFilter,
+    filter: options.filter,
     // For backwards compatability, we default to legacy behavior for newly added settings.
 
     parseReqBody: isUnset(options.parseReqBody) ? true : options.parseReqBody,
@@ -76,10 +76,4 @@ function resolveOptions(options) {
   return resolved;
 }
 
-function defaultFilter() {
-  // No-op version of filter.  Allows everything!
-
-  return true;
-}
-
 module.exports = resolveOptions;

test/filter.js

@@ -0,0 +1,113 @@
+'use strict';
+
+var express = require('express');
+var request = require('supertest');
+var proxy = require('../');
+var proxyTarget = require('../test/support/proxyTarget');
+var proxyRouteFn = [{
+  method: 'get',
+  path: '/',
+  fn: function (req, res) {
+    return res.status(209).send('Proxy Server');
+  }
+}];
+
+function nextMethod(req, res, next) {
+  res.status(201).send('Client Application');
+  next();
+}
+
+describe('filter', function () {
+  var app = express();
+  var proxyServer;
+
+  beforeEach(function () {
+    proxyServer = proxyTarget(12346, 100, proxyRouteFn);
+    app = express();
+  });
+
+  afterEach(function () {
+    proxyServer.close();
+  });
+
+  describe('when filter function returns true', function () {
+    it('continues route processing', function (done) {
+      // should capture every possible path
+
+      app.use(proxy('localhost:12346', {
+        filter: function () { return true; }
+      }));
+
+      // because prior line should *always* fire and return, we should never get here.
+
+      app.use(nextMethod);
+
+      request(app)
+        .get('/')
+        .expect(209)
+        .end(done);
+    });
+  });
+
+  describe('when filter function returns false', function () {
+
+    it('hanldes route processing', function (done) {
+      // should capture every possible path
+
+      app.use(proxy('localhost:12346', {
+        filter: function () { return false; }
+      }));
+
+      // because prior line should *always* fire skip, we should get here.
+
+      app.use(nextMethod);
+
+      request(app)
+        .get('/')
+        .expect(201)
+        .end(done);
+    });
+  });
+
+  describe('promise form', function () {
+    describe('when filter function returns true', function () {
+      it('continues route processing', function (done) {
+        // should capture every possible path
+
+        app.use(proxy('localhost:12346', {
+          filter: function () { return new Promise(function (resolve) { resolve(true); }); }
+        }));
+
+        // because prior line should *always* fire and return, we should never get here.
+
+        app.use(nextMethod);
+
+        request(app)
+          .get('/')
+          .expect(209)
+          .end(done);
+      });
+    });
+
+    describe('when filter function returns false', function () {
+
+      it('hanldes route processing', function (done) {
+        // should capture every possible path
+
+        app.use(proxy('localhost:12346', {
+          filter: function () { return new Promise(function (resolve) { resolve(false); }); }
+        }));
+
+        // because prior line should *always* fire skip, we should get here.
+
+        app.use(nextMethod);
+
+        request(app)
+          .get('/')
+          .expect(201)
+          .end(done);
+      });
+    });
+  });
+
+});