Add multiple regexp parametric children (#291)

custom_node.js

@@ -56,18 +56,21 @@ class StaticNode extends ParentNode {
     super()
     this.prefix = prefix
     this.wildcardChild = null
-    this.parametricChild = null
+    this.parametricChildren = []
     this.kind = NODE_TYPES.STATIC
     this._compilePrefixMatch()
   }
 
   createParametricChild (regex) {
-    if (this.parametricChild) {
-      return this.parametricChild
+    let parametricChild = this.parametricChildren.find(child => child.regex === regex)
+
+    if (parametricChild) {
+      return parametricChild
     }
 
-    this.parametricChild = new ParametricNode(regex)
-    return this.parametricChild
+    parametricChild = new ParametricNode(regex)
+    this.parametricChildren.push(parametricChild)
+    return parametricChild
   }
 
   createWildcardChild () {
@@ -93,6 +96,38 @@ class StaticNode extends ParentNode {
     return staticNode
   }
 
+  getNextNode (path, pathIndex, nodeStack, paramsCount) {
+    let node = this.findStaticMatchingChild(path, pathIndex)
+    let parametricBrotherNodeIndex = 0
+
+    if (node === null) {
+      if (this.parametricChildren.length === 0) {
+        return this.wildcardChild
+      }
+
+      node = this.parametricChildren[0]
+      parametricBrotherNodeIndex = 1
+    }
+
+    if (this.wildcardChild !== null) {
+      nodeStack.push({
+        paramsCount,
+        brotherPathIndex: pathIndex,
+        brotherNode: this.wildcardChild
+      })
+    }
+
+    for (let i = parametricBrotherNodeIndex; i < this.parametricChildren.length; i++) {
+      nodeStack.push({
+        paramsCount,
+        brotherPathIndex: pathIndex,
+        brotherNode: this.parametricChildren[i]
+      })
+    }
+
+    return node
+  }
+
   _compilePrefixMatch () {
     if (this.prefix.length === 1) {
       this.matchPrefix = () => true
@@ -115,13 +150,21 @@ class ParametricNode extends ParentNode {
     this.isRegex = !!regex
     this.kind = NODE_TYPES.PARAMETRIC
   }
+
+  getNextNode (path, pathIndex) {
+    return this.findStaticMatchingChild(path, pathIndex)
+  }
 }
 
 class WildcardNode extends Node {
   constructor () {
     super()
     this.kind = NODE_TYPES.WILDCARD
   }
+
+  getNextNode () {
+    return null
+  }
 }
 
 module.exports = { StaticNode, ParametricNode, WildcardNode, NODE_TYPES }

handler_storage.js

@@ -1,7 +1,5 @@
 'use strict'
 
-const deepEqual = require('fast-deep-equal')
-
 class HandlerStorage {
   constructor () {
     this.unconstrainedHandler = null // optimized reference to the handler that will match most of the time
@@ -10,10 +8,6 @@ class HandlerStorage {
     this.constrainedHandlerStores = null
   }
 
-  hasHandler (constraints) {
-    return this.handlers.find(handler => deepEqual(constraints, handler.constraints)) !== undefined
-  }
-
   // This is the hot path for node handler finding -- change with care!
   getMatchingHandler (derivedConstraints) {
     if (derivedConstraints === undefined) {

index.js

@@ -88,8 +88,9 @@ function Router (opts) {
   this.allowUnsafeRegex = opts.allowUnsafeRegex || false
   this.routes = []
   this.trees = {}
-
   this.constrainer = new Constrainer(opts.constraints)
+
+  this._routesPatterns = []
 }
 
 Router.prototype.on = function on (method, path, opts, handler, store) {
@@ -170,25 +171,21 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
   const params = []
   for (let i = 0; i <= path.length; i++) {
     if (path.charCodeAt(i) === 58 && path.charCodeAt(i + 1) === 58) {
-      // It's a double colon. Let's just replace it with a single colon and go ahead
-      path = path.slice(0, i) + path.slice(i + 1)
+      // It's a double colon
+      i++
       continue
     }
 
-    if (path.charCodeAt(i) === 37) {
-      // We need to encode % char to prevent double decoding
-      path = path.slice(0, i + 1) + '25' + path.slice(i + 1)
-      continue
-    }
-
-    const isParametricNode = path.charCodeAt(i) === 58
+    const isParametricNode = path.charCodeAt(i) === 58 && path.charCodeAt(i + 1) !== 58
     const isWildcardNode = path.charCodeAt(i) === 42
 
     if (isParametricNode || isWildcardNode || (i === path.length && i !== parentNodePathIndex)) {
       let staticNodePath = path.slice(parentNodePathIndex, i)
       if (!this.caseSensitive) {
         staticNodePath = staticNodePath.toLowerCase()
       }
+      staticNodePath = staticNodePath.split('::').join(':')
+      staticNodePath = staticNodePath.split('%').join('%25')
       // add the static part of the route to the tree
       currentNode = currentNode.createStaticChild(staticNodePath)
     }
@@ -267,7 +264,21 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
     }
   }
 
-  assert(!currentNode.handlerStorage.hasHandler(constraints), `Method '${method}' already declared for route '${path}' with constraints '${JSON.stringify(constraints)}'`)
+  if (!this.caseSensitive) {
+    path = path.toLowerCase()
+  }
+
+  for (const existRoute of this._routesPatterns) {
+    if (
+      existRoute.path === path &&
+      existRoute.method === method &&
+      deepEqual(existRoute.constraints, constraints)
+    ) {
+      throw new Error(`Method '${method}' already declared for route '${path}' with constraints '${JSON.stringify(constraints)}'`)
+    }
+  }
+  this._routesPatterns.push({ method, path, constraints })
+
   currentNode.handlerStorage.addHandler(handler, params, store, this.constrainer, constraints)
 }
 
@@ -283,6 +294,7 @@ Router.prototype.addConstraintStrategy = function (constraints) {
 Router.prototype.reset = function reset () {
   this.trees = {}
   this.routes = []
+  this._routesPatterns = []
 }
 
 Router.prototype.off = function off (method, path, opts) {
@@ -402,48 +414,7 @@ Router.prototype.find = function find (method, path, derivedConstraints) {
       }
     }
 
-    let node = null
-
-    if (
-      currentNode.kind === NODE_TYPES.STATIC ||
-      currentNode.kind === NODE_TYPES.PARAMETRIC
-    ) {
-      node = currentNode.findStaticMatchingChild(path, pathIndex)
-
-      if (currentNode.kind === NODE_TYPES.STATIC) {
-        if (node === null) {
-          if (currentNode.parametricChild !== null) {
-            node = currentNode.parametricChild
-
-            if (currentNode.wildcardChild !== null) {
-              brothersNodesStack.push({
-                brotherPathIndex: pathIndex,
-                paramsCount: params.length,
-                brotherNode: currentNode.wildcardChild
-              })
-            }
-          } else {
-            node = currentNode.wildcardChild
-          }
-        } else {
-          if (currentNode.wildcardChild !== null) {
-            brothersNodesStack.push({
-              brotherPathIndex: pathIndex,
-              paramsCount: params.length,
-              brotherNode: currentNode.wildcardChild
-            })
-          }
-
-          if (currentNode.parametricChild !== null) {
-            brothersNodesStack.push({
-              brotherPathIndex: pathIndex,
-              paramsCount: params.length,
-              brotherNode: currentNode.parametricChild
-            })
-          }
-        }
-      }
-    }
+    let node = currentNode.getNextNode(path, pathIndex, brothersNodesStack, params.length)
 
     if (node === null) {
       if (brothersNodesStack.length === 0) {

lib/pretty-print.js

@@ -253,15 +253,15 @@ function flattenNode (flattened, node, method) {
     flattened.nodes.push({ method, node })
   }
 
-  if (node.parametricChild) {
+  if (node.parametricChildren && node.parametricChildren[0]) {
     if (!flattened.children[':']) {
       flattened.children[':'] = {
         prefix: ':',
         nodes: [],
         children: {}
       }
     }
-    flattenNode(flattened.children[':'], node.parametricChild, method)
+    flattenNode(flattened.children[':'], node.parametricChildren[0], method)
   }
 
   if (node.wildcardChild) {

test/issue-285.test.js

@@ -0,0 +1,38 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('../')
+
+test('Parametric regex match with similar routes', (t) => {
+  t.plan(2)
+  const findMyWay = FindMyWay()
+
+  findMyWay.on('GET', '/:a(a)', () => {})
+  findMyWay.on('GET', '/:param/static', () => {})
+
+  t.same(findMyWay.find('GET', '/a', {}).params, { a: 'a' })
+  t.same(findMyWay.find('GET', '/param/static', {}).params, { param: 'param' })
+})
+
+test('Parametric regex match with similar routes', (t) => {
+  t.plan(2)
+  const findMyWay = FindMyWay()
+
+  findMyWay.on('GET', '/:a(a)', () => {})
+  findMyWay.on('GET', '/:b(b)/static', () => {})
+
+  t.same(findMyWay.find('GET', '/a', {}).params, { a: 'a' })
+  t.same(findMyWay.find('GET', '/b/static', {}).params, { b: 'b' })
+})
+
+test('Parametric regex match with similar routes', (t) => {
+  t.plan(2)
+  const findMyWay = FindMyWay()
+
+  findMyWay.on('GET', '/:a(a)/static', { constraints: { version: '1.0.0' } }, () => {})
+  findMyWay.on('GET', '/:b(b)/static', { constraints: { version: '2.0.0' } }, () => {})
+
+  t.same(findMyWay.find('GET', '/a/static', { version: '1.0.0' }).params, { a: 'a' })
+  t.same(findMyWay.find('GET', '/b/static', { version: '2.0.0' }).params, { b: 'b' })
+})