feat: add multiple parametric nodes support (#320)

* feat: add multiple parametric nodes support

* docs: update nodes maching order documentation

Author: ivan-tymoshenko

README.md

@@ -283,25 +283,34 @@ Having a route with multiple parameters may affect negatively the performance, s
 <a name="match-order"></a>
 ##### Match order
 
-The routing algorithm matches one chunk at a time (where the chunk is a string between two slashes),
+The routing algorithm matches one node at a time (where the node is a string between two slashes),
 this means that it cannot know if a route is static or dynamic until it finishes to match the URL.
 
-The chunks are matched in the following order:
+The nodes are matched in the following order:
 
 1. static
-1. parametric
-1. wildcards
-1. parametric(regex)
-1. multi parametric(regex)
+2. parametric node with static ending
+3. parametric(regex)/multi-parametric
+4. parametric
+5. wildcard
 
 So if you declare the following routes
 
-- `/:userId/foo/bar`
-- `/33/:a(^.*$)/:b`
+- `/foo/filename.png` - static route
+- `/foo/:filename.png` - route with param `filename` and static ending `.png`
+- `/foo/:filename.:ext` - route with two params `filename` and `ext`
+- `/foo/:filename` - route with one param `filename`
+- `/*` - wildcard route
+
+You will have next matching rules:
+- the static node would have the highest priority. It will be matched only if incoming URL equals  `/foo/filename.png`
+- the parametric node with a static ending would have the higher priority than other parametric nodes without it. This node would match any filenames with `.png` extension. If one node static ending ends with another node static ending, the node with a longer static ending would have higher priority.
+  - `/foo/:filename.png.png` - higher priority, more specific route
+  - `/foo/:filename.png` - lower priority
+- the multi-parametric node (or any regexp node) without static ending would have lower priority than parametric node with static ending and higher priority than generic parametric node. You can declare only one node like that for the same route (see [caveats](#caveats)). It would match any filenames with any extensions.
+- the parametric node has lower priority than any other parametric node. It would match any filenames, even if they don't have an extension.
+- the wildcard node has the lowest priority of all nodes.
 
-and the URL of the incoming request is /33/foo/bar,
-the second route will be matched because the first chunk (33) matches the static chunk.
-If the URL would have been /32/foo/bar, the first route would have been matched.
 Once a url has been matched, `find-my-way` will figure out which handler registered for that path matches the request if there are any constraints.
 `find-my-way` will check the most constrained handlers first, which means the handlers with the most keys in the `constraints` object.
 

custom_node.js

@@ -61,7 +61,7 @@ class StaticNode extends ParentNode {
     this._compilePrefixMatch()
   }
 
-  createParametricChild (regex) {
+  createParametricChild (regex, staticSuffix) {
     const regexpSource = regex && regex.source
 
     let parametricChild = this.parametricChildren.find(child => {
@@ -73,12 +73,21 @@ class StaticNode extends ParentNode {
       return parametricChild
     }
 
-    parametricChild = new ParametricNode(regex)
-    if (regex) {
-      this.parametricChildren.unshift(parametricChild)
-    } else {
-      this.parametricChildren.push(parametricChild)
-    }
+    parametricChild = new ParametricNode(regex, staticSuffix)
+    this.parametricChildren.push(parametricChild)
+    this.parametricChildren.sort((child1, child2) => {
+      if (!child1.isRegex) return 1
+      if (!child2.isRegex) return -1
+
+      if (child1.staticSuffix === null) return 1
+      if (child2.staticSuffix === null) return -1
+
+      if (child2.staticSuffix.endsWith(child1.staticSuffix)) return 1
+      if (child1.staticSuffix.endsWith(child2.staticSuffix)) return -1
+
+      return 0
+    })
+
     return parametricChild
   }
 
@@ -153,10 +162,11 @@ class StaticNode extends ParentNode {
 }
 
 class ParametricNode extends ParentNode {
-  constructor (regex) {
+  constructor (regex, staticSuffix) {
     super()
-    this.regex = regex || null
     this.isRegex = !!regex
+    this.regex = regex || null
+    this.staticSuffix = staticSuffix || null
     this.kind = NODE_TYPES.PARAMETRIC
   }
 

index.js

@@ -89,7 +89,7 @@ function Router (opts) {
   this.trees = {}
   this.constrainer = new Constrainer(opts.constraints)
 
-  this._routesPatterns = []
+  this._routesPatterns = {}
 }
 
 Router.prototype.on = function on (method, path, opts, handler, store) {
@@ -156,6 +156,7 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
   // Boot the tree for this method if it doesn't exist yet
   if (this.trees[method] === undefined) {
     this.trees[method] = new StaticNode('/')
+    this._routesPatterns[method] = []
   }
 
   if (path === '*' && this.trees[method].prefix.length !== 0) {
@@ -193,18 +194,21 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
       let isRegexNode = false
       const regexps = []
 
-      let staticEndingLength = 0
       let lastParamStartIndex = i + 1
       for (let j = lastParamStartIndex; ; j++) {
         const charCode = path.charCodeAt(j)
 
-        if (charCode === 40 || charCode === 45 || charCode === 46) {
-          isRegexNode = true
+        const isRegexParam = charCode === 40
+        const isStaticPart = charCode === 45 || charCode === 46
+        const isEndOfNode = charCode === 47 || j === path.length
 
+        if (isRegexParam || isStaticPart || isEndOfNode) {
           const paramName = path.slice(lastParamStartIndex, j)
           params.push(paramName)
 
-          if (charCode === 40) {
+          isRegexNode = isRegexNode || isRegexParam || isStaticPart
+
+          if (isRegexParam) {
             const endOfRegexIndex = getClosingParenthensePosition(path, j)
             const regexString = path.slice(j, endOfRegexIndex + 1)
 
@@ -219,53 +223,39 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
             regexps.push('(.*?)')
           }
 
-          let lastParamEndIndex = j
-          for (; lastParamEndIndex < path.length; lastParamEndIndex++) {
-            const charCode = path.charCodeAt(lastParamEndIndex)
-            const nextCharCode = path.charCodeAt(lastParamEndIndex + 1)
-            if (charCode === 58 && nextCharCode === 58) {
-              lastParamEndIndex++
-              continue
+          const staticPartStartIndex = j
+          for (; j < path.length; j++) {
+            const charCode = path.charCodeAt(j)
+            if (charCode === 47) break
+            if (charCode === 58) {
+              const nextCharCode = path.charCodeAt(j + 1)
+              if (nextCharCode === 58) j++
+              else break
             }
-            if (charCode === 58 || charCode === 47) break
           }
 
-          let staticPart = path.slice(j, lastParamEndIndex)
+          let staticPart = path.slice(staticPartStartIndex, j)
           if (staticPart) {
             staticPart = staticPart.split('::').join(':')
             staticPart = staticPart.split('%').join('%25')
             regexps.push(escapeRegExp(staticPart))
           }
 
-          lastParamStartIndex = lastParamEndIndex + 1
-          j = lastParamEndIndex
+          lastParamStartIndex = j + 1
 
-          if (path.charCodeAt(j) === 47 || j === path.length) {
-            staticEndingLength = staticPart.length
-          }
-        } else if (charCode === 47 || j === path.length) {
-          const paramName = path.slice(lastParamStartIndex, j)
-          params.push(paramName)
+          if (isEndOfNode || path.charCodeAt(j) === 47 || j === path.length) {
+            const nodePattern = isRegexNode ? '()' + staticPart : staticPart
 
-          if (regexps.length !== 0) {
-            regexps.push('(.*?)')
-          }
-        }
+            path = path.slice(0, i + 1) + nodePattern + path.slice(j)
+            i += nodePattern.length
 
-        if (path.charCodeAt(j) === 47 || j === path.length) {
-          path = path.slice(0, i + 1) + path.slice(j - staticEndingLength)
-          i += staticEndingLength
-          break
+            const regex = isRegexNode ? new RegExp('^' + regexps.join('') + '$') : null
+            currentNode = currentNode.createParametricChild(regex, staticPart || null)
+            parentNodePathIndex = i + 1
+            break
+          }
         }
       }
-
-      let regex = null
-      if (isRegexNode) {
-        regex = new RegExp('^' + regexps.join('') + '$')
-      }
-
-      currentNode = currentNode.createParametricChild(regex)
-      parentNodePathIndex = i + 1
     } else if (isWildcardNode) {
       // add the wildcard parameter
       params.push('*')
@@ -282,25 +272,16 @@ Router.prototype._on = function _on (method, path, opts, handler, store) {
     path = path.toLowerCase()
   }
 
-  const isRootWildcard = path === '*' || path === '/*'
-  for (const existRoute of this._routesPatterns) {
-    let samePath = false
-
-    if (existRoute.path === path) {
-      samePath = true
-    } else if (isRootWildcard && (existRoute.path === '/*' || existRoute.path === '*')) {
-      samePath = true
-    }
+  if (path === '*') {
+    path = '/*'
+  }
 
-    if (
-      samePath &&
-      existRoute.method === method &&
-      deepEqual(existRoute.constraints, constraints)
-    ) {
+  for (const existRoute of this._routesPatterns[method]) {
+    if (existRoute.path === path && 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 })
+  this._routesPatterns[method].push({ path, params, constraints })
 
   currentNode.handlerStorage.addHandler(handler, params, store, this.constrainer, constraints)
 }
@@ -317,7 +298,7 @@ Router.prototype.addConstraintStrategy = function (constraints) {
 Router.prototype.reset = function reset () {
   this.trees = {}
   this.routes = []
-  this._routesPatterns = []
+  this._routesPatterns = {}
 }
 
 Router.prototype.off = function off (method, path, constraints) {

test/errors.test.js

@@ -254,7 +254,7 @@ test('Method already declared if * is used', t => {
     findMyWay.on('GET', '*', () => {})
     t.fail('should throw error')
   } catch (e) {
-    t.equal(e.message, 'Method \'GET\' already declared for route \'*\' with constraints \'{}\'')
+    t.equal(e.message, 'Method \'GET\' already declared for route \'/*\' with constraints \'{}\'')
   }
 })
 

test/params-collisions.test.js

@@ -0,0 +1,127 @@
+'use strict'
+
+const t = require('tap')
+const test = t.test
+const FindMyWay = require('..')
+
+test('should setup parametric and regexp node', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler = () => {}
+  const regexpHandler = () => {}
+
+  findMyWay.on('GET', '/foo/:bar', paramHandler)
+  findMyWay.on('GET', '/foo/:bar(123)', regexpHandler)
+
+  t.equal(findMyWay.find('GET', '/foo/value').handler, paramHandler)
+  t.equal(findMyWay.find('GET', '/foo/123').handler, regexpHandler)
+})
+
+test('should setup parametric and multi-parametric node', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler = () => {}
+  const regexpHandler = () => {}
+
+  findMyWay.on('GET', '/foo/:bar', paramHandler)
+  findMyWay.on('GET', '/foo/:bar.png', regexpHandler)
+
+  t.equal(findMyWay.find('GET', '/foo/value').handler, paramHandler)
+  t.equal(findMyWay.find('GET', '/foo/value.png').handler, regexpHandler)
+})
+
+test('should throw when set upping two parametric nodes', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay()
+  findMyWay.on('GET', '/foo/:bar', () => {})
+
+  t.throws(() => findMyWay.on('GET', '/foo/:baz', () => {}))
+})
+
+test('should throw when set upping two regexp nodes', t => {
+  t.plan(1)
+
+  const findMyWay = FindMyWay()
+  findMyWay.on('GET', '/foo/:bar(123)', () => {})
+
+  t.throws(() => findMyWay.on('GET', '/foo/:bar(456)', () => {}))
+})
+
+test('should set up two parametric nodes with static ending', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler1 = () => {}
+  const paramHandler2 = () => {}
+
+  findMyWay.on('GET', '/foo/:bar.png', paramHandler1)
+  findMyWay.on('GET', '/foo/:bar.jpeg', paramHandler2)
+
+  t.equal(findMyWay.find('GET', '/foo/value.png').handler, paramHandler1)
+  t.equal(findMyWay.find('GET', '/foo/value.jpeg').handler, paramHandler2)
+})
+
+test('should set up two regexp nodes with static ending', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler1 = () => {}
+  const paramHandler2 = () => {}
+
+  findMyWay.on('GET', '/foo/:bar(123).png', paramHandler1)
+  findMyWay.on('GET', '/foo/:bar(456).jpeg', paramHandler2)
+
+  t.equal(findMyWay.find('GET', '/foo/123.png').handler, paramHandler1)
+  t.equal(findMyWay.find('GET', '/foo/456.jpeg').handler, paramHandler2)
+})
+
+test('node with longer static suffix should have higher priority', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler1 = () => {}
+  const paramHandler2 = () => {}
+
+  findMyWay.on('GET', '/foo/:bar.png', paramHandler1)
+  findMyWay.on('GET', '/foo/:bar.png.png', paramHandler2)
+
+  t.equal(findMyWay.find('GET', '/foo/value.png').handler, paramHandler1)
+  t.equal(findMyWay.find('GET', '/foo/value.png.png').handler, paramHandler2)
+})
+
+test('node with longer static suffix should have higher priority', t => {
+  t.plan(2)
+
+  const findMyWay = FindMyWay()
+
+  const paramHandler1 = () => {}
+  const paramHandler2 = () => {}
+
+  findMyWay.on('GET', '/foo/:bar.png.png', paramHandler2)
+  findMyWay.on('GET', '/foo/:bar.png', paramHandler1)
+
+  t.equal(findMyWay.find('GET', '/foo/value.png').handler, paramHandler1)
+  t.equal(findMyWay.find('GET', '/foo/value.png.png').handler, paramHandler2)
+})
+
+test('should set up regexp node and node with static ending', t => {
+  t.plan(2)
+
+  const regexHandler = () => {}
+  const multiParamHandler = () => {}
+
+  const findMyWay = FindMyWay()
+  findMyWay.on('GET', '/foo/:bar(123)', regexHandler)
+  findMyWay.on('GET', '/foo/:bar(123).jpeg', multiParamHandler)
+
+  t.equal(findMyWay.find('GET', '/foo/123.jpeg').handler, multiParamHandler)
+  t.equal(findMyWay.find('GET', '/foo/123').handler, regexHandler)
+})