fix(core): Support multiple handlers for same paths (#141)

* fix(core): Support multiple handlers for same paths * fix: Don't make a copy of the handlers for the route * test: Fix failing tests
Netflix · Nov 26, 2018 · 79e04b8 · 79e04b8
1 parent cf0d755
commit 79e04b8
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 25 deletions.
diff --git a/packages/@pollyjs/core/src/server/index.js b/packages/@pollyjs/core/src/server/index.js
@@ -12,8 +12,13 @@ const HOST = Symbol();
 const NAMESPACES = Symbol();
 const REGISTRY = Symbol();
 const MIDDLEWARE = Symbol();
-const SLASH = '/';
-const STAR = '*';
+const HANDLERS = Symbol();
+
+const CHARS = {
+  SLASH: '/',
+  STAR: '*',
+  COLON: ':'
+};
 
 const METHODS = ['GET', 'PUT', 'POST', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'];
 
@@ -25,7 +30,7 @@ function parseUrl(url) {
     Use the full origin (http://hostname:port) if the host exists. If there
     is no host, URL.origin returns "null" (null as a string) so set host to '/'
   */
-  const host = path.host ? path.origin : SLASH;
+  const host = path.host ? path.origin : CHARS.SLASH;
   const href = removeHostFromUrl(path).href;
 
   return { host, path: href };
@@ -109,8 +114,15 @@ export default class Server {
     castArray(routes).forEach(route => {
       const { host, path } = parseUrl(this._buildUrl(route));
       const registry = this._registryForHost(host);
+      const name = this._nameForPath(path);
+      const router = registry[method.toUpperCase()];
 
-      registry[method.toUpperCase()].add([{ path, handler }]);
+      if (router[HANDLERS].has(name)) {
+        router[HANDLERS].get(name).push(handler);
+      } else {
+        router[HANDLERS].set(name, [handler]);
+        router.add([{ path, handler: router[HANDLERS].get(name) }]);
+      }
     });
 
     return handler;
@@ -126,7 +138,7 @@ export default class Server {
         specified, treat the middleware as global so it will match all routes.
       */
       if (
-        (!route || route === STAR) &&
+        (!route || route === CHARS.STAR) &&
         !this[HOST] &&
         this[NAMESPACES].length === 0
       ) {
@@ -159,12 +171,39 @@ export default class Server {
     return buildUrl(this[HOST], ...this[NAMESPACES], path);
   }
 
+  /**
+   * Converts a url path into a name used to combine route handlers by
+   * normalizing dynamic and star segments
+   * @param {String} path
+   * @returns {String}
+   */
+  _nameForPath(path = '') {
+    return path
+      .split(CHARS.SLASH)
+      .map(segment => {
+        switch (segment.charAt(0)) {
+          // If this is a dynamic segment (e.g. :id), then just return `:`
+          // since /path/:id is the same as /path/:uuid
+          case CHARS.COLON:
+            return CHARS.COLON;
+          // If this is a star segment (e.g. *path), then just return `*`
+          // since /path/*path is the same as /path/*all
+          case CHARS.STAR:
+            return CHARS.STAR;
+          default:
+            return segment;
+        }
+      })
+      .join(CHARS.SLASH);
+  }
+
   _registryForHost(host) {
-    host = host || SLASH;
+    host = host || CHARS.SLASH;
 
     if (!this[REGISTRY][host]) {
       this[REGISTRY][host] = METHODS.reduce((acc, method) => {
         acc[method] = new RouteRecognizer();
+        acc[method][HANDLERS] = new Map();
 
         return acc;
       }, {});

diff --git a/packages/@pollyjs/core/src/server/middleware.js b/packages/@pollyjs/core/src/server/middleware.js
@@ -12,7 +12,9 @@ export default class Middleware {
     this.paths = this.global ? [GLOBAL] : paths;
     this._routeRecognizer = new RouteRecognizer();
 
-    this.paths.forEach(path => this._routeRecognizer.add([{ path, handler }]));
+    this.paths.forEach(path =>
+      this._routeRecognizer.add([{ path, handler: [handler] }])
+    );
   }
 
   match(host, path) {

diff --git a/packages/@pollyjs/core/src/server/route.js b/packages/@pollyjs/core/src/server/route.js
@@ -1,7 +1,5 @@
 import mergeOptions from 'merge-options';
 
-import Handler from './handler';
-
 async function invoke(fn, route, req, ...args) {
   if (typeof fn !== 'function') {
     return;
@@ -29,10 +27,12 @@ async function invoke(fn, route, req, ...args) {
 }
 
 async function emit(route, eventName, ...args) {
-  const listeners = route.handler._eventEmitter.listeners(eventName);
+  for (const handler of route.handlers) {
+    const listeners = handler._eventEmitter.listeners(eventName);
 
-  for (const listener of listeners) {
-    await invoke(listener, route, ...args);
+    for (const listener of listeners) {
+      await invoke(listener, route, ...args);
+    }
   }
 }
 
@@ -47,15 +47,14 @@ export default class Route {
 
     this.params = {};
     this.queryParams = {};
+    this.handlers = [];
     this.middleware = middleware || [];
 
     if (result) {
-      this.handler = result.handler;
+      this.handlers = result.handler;
       this.params = { ...result.params };
       this.queryParams = recognizeResults.queryParams;
     }
-
-    this.handler = this.handler || new Handler();
   }
 
   shouldPassthrough() {
@@ -72,7 +71,7 @@ export default class Route {
 
   config() {
     return mergeOptions(
-      ...this._orderedRoutes().map(r => r.handler.get('config'))
+      ...this._orderedHandlers().map(handler => handler.get('config'))
     );
   }
 
@@ -83,9 +82,9 @@ export default class Route {
    * @param {Interceptor} interceptor
    */
   async intercept(req, res, interceptor) {
-    for (const route of this._orderedRoutes()) {
-      if (route.handler.has('intercept') && interceptor.shouldIntercept) {
-        await invoke(route.handler.get('intercept'), this, ...arguments);
+    for (const handler of this._orderedHandlers()) {
+      if (handler.has('intercept') && interceptor.shouldIntercept) {
+        await invoke(handler.get('intercept'), this, ...arguments);
       }
     }
   }
@@ -104,16 +103,20 @@ export default class Route {
     await emit(this, ...arguments);
   }
 
-  _orderedRoutes() {
-    return [...this.middleware, this];
+  _orderedHandlers() {
+    return [...this.middleware, this].reduce((handlers, route) => {
+      handlers.push(...route.handlers);
+
+      return handlers;
+    }, []);
   }
 
   _valueFor(key) {
     let value;
 
-    for (const route of this._orderedRoutes()) {
-      if (route.handler.has(key)) {
-        value = route.handler.get(key);
+    for (const handler of this._orderedHandlers()) {
+      if (handler.has(key)) {
+        value = handler.get(key);
       }
     }
 

diff --git a/packages/@pollyjs/core/tests/unit/server/server-test.js b/packages/@pollyjs/core/tests/unit/server/server-test.js
@@ -5,7 +5,7 @@ const METHODS = ['GET', 'PUT', 'POST', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'];
 let server;
 
 function request(method, path) {
-  return server.lookup(method, path).handler.get('intercept')();
+  return server.lookup(method, path).handlers[0].get('intercept')();
 }
 
 describe('Unit | Server', function() {
@@ -109,4 +109,63 @@ describe('Unit | Server', function() {
       expect(request('GET', '/api/v2/bar')).to.equal('bar');
     });
   });
+
+  describe('Route Matching', function() {
+    beforeEach(function() {
+      server = new Server();
+    });
+
+    function addHandlers(url) {
+      server.get(url).on('request', () => {});
+      server.get(url).on('response', () => {});
+      server.get(url).intercept(() => {});
+    }
+
+    it('should concat handlers for same paths', async function() {
+      [
+        '/ping',
+        '/ping/:id',
+        '/ping/*path',
+        'http://ping.com',
+        'http://ping.com/pong/:id',
+        'http://ping.com/pong/*path'
+      ].forEach(url => {
+        addHandlers(url);
+        expect(server.lookup('GET', url).handlers).to.have.lengthOf(3);
+      });
+    });
+
+    it('should concat handlers for same paths with different dynamic segment names', async function() {
+      addHandlers('/ping/:id');
+      expect(server.lookup('GET', '/ping/:id').handlers).to.have.lengthOf(3);
+
+      addHandlers('/ping/:uuid');
+      expect(server.lookup('GET', '/ping/:id').handlers).to.have.lengthOf(6);
+      expect(server.lookup('GET', '/ping/:uuid').handlers).to.have.lengthOf(6);
+    });
+
+    it('should concat handlers for same paths with different star segment names', async function() {
+      addHandlers('/ping/*path');
+      expect(server.lookup('GET', '/ping/*path').handlers).to.have.lengthOf(3);
+
+      addHandlers('/ping/*rest');
+      expect(server.lookup('GET', '/ping/*path').handlers).to.have.lengthOf(6);
+      expect(server.lookup('GET', '/ping/*rest').handlers).to.have.lengthOf(6);
+    });
+
+    it('should concat handlers for same paths with different dynamic and star segment names', async function() {
+      addHandlers('/ping/:id/pong/*path');
+      expect(
+        server.lookup('GET', '/ping/:id/pong/*path').handlers
+      ).to.have.lengthOf(3);
+
+      addHandlers('/ping/:uuid/pong/*rest');
+      expect(
+        server.lookup('GET', '/ping/:id/pong/*path').handlers
+      ).to.have.lengthOf(6);
+      expect(
+        server.lookup('GET', '/ping/:uuid/pong/*rest').handlers
+      ).to.have.lengthOf(6);
+    });
+  });
 });