From 3050765f883103ba096d6e5b77a9f2d9c098473e Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Mon, 10 Jun 2024 09:35:52 +0700 Subject: [PATCH 1/6] implemented same path wildcard & static conflict fix - implemented support of having exact route definitions for the same path as wildcard; - adjusted test to cover the case; - added .idea to .gitignore; --- .gitignore | 2 + __tests__/tree-spec.js | 7 +- tree.js | 162 +++++++++++++++++++++++------------------ 3 files changed, 99 insertions(+), 72 deletions(-) diff --git a/.gitignore b/.gitignore index ca8527b..2ebcc74 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +.idea + # Logs logs *.log diff --git a/__tests__/tree-spec.js b/__tests__/tree-spec.js index 85e8ece..c5a6649 100644 --- a/__tests__/tree-spec.js +++ b/__tests__/tree-spec.js @@ -106,8 +106,9 @@ describe("Wildcard", () => { const tree = new Tree(); const routes = [ "/", - "/cmd/:tool/:sub", + "/cmd/foo/bar", "/cmd/:tool/", + "/cmd/:tool/:sub", "/src/*filepath", "/search/", "/search/:query", @@ -132,6 +133,10 @@ describe("Wildcard", () => { route: "/", params: [] }, + { + route: "/cmd/foo/bar", + params: [] + }, { route: "/cmd/test/", params: [{ key: "tool", value: "test" }] diff --git a/tree.js b/tree.js index a7ef623..f0d7826 100644 --- a/tree.js +++ b/tree.js @@ -81,6 +81,7 @@ class Node { ) { this.path = path; this.wildChild = wildChild; + this.wildChildIdx = -1; this.type = type; this.indices = indices; this.children = children; @@ -139,7 +140,7 @@ class Node { let i = longestCommonPrefix(path, n.path); // Split edge - if (i < n.path.length) { + if (i && i < n.path.length) { const child = new Node( n.path.slice(i), n.wildChild, @@ -162,16 +163,17 @@ class Node { path = path.slice(i); if (n.wildChild) { - n = n.children[0]; + n = n.children[n.wildChildIdx]; n.priority++; // Check if the wildcard matches if ( - path.length >= n.path.length && - n.path === path.slice(0, n.path.length) && + (n.type === STATIC || ( + (n.path.length >= path.length || path[n.path.length] === "/") && + n.path === path.slice(0, n.path.length) + )) && // Adding a child to a catchAll is not possible - n.type !== CATCH_ALL && - (n.path.length >= path.length || path[n.path.length] === "/") + n.type !== CATCH_ALL ) { continue walk; } else { @@ -261,16 +263,6 @@ class Node { ); } - if (n.children.length > 0) { - throw new Error( - "wildcard route '" + - wildcard + - "' conflicts with existing children in path '" + - fullPath + - "'" - ); - } - if (wildcard[0] === ":") { // param if (i > 0) { @@ -280,8 +272,9 @@ class Node { } n.wildChild = true; + n.wildChildIdx = n.children.length; const child = new Node(wildcard, false, PARAM); - n.children = [child]; + n.children.push(child); n = child; n.priority++; @@ -352,78 +345,105 @@ class Node { } /** * - * @param {string} path + * @param {string} ipath */ - search(path) { - let handle = null; - const params = []; + search(ipath) { + let path = ipath; let n = this; - - walk: while (true) { - if (path.length > n.path.length) { - if (path.slice(0, n.path.length) === n.path) { - path = path.slice(n.path.length); - // If this node does not have a wildcard child, - // we can just look up the next child node and continue - // to walk down the tree - if (!n.wildChild) { - const c = path.charCodeAt(0); - for (let i = 0; i < n.indices.length; i++) { - if (c === n.indices.charCodeAt(i)) { - n = n.children[i]; - continue walk; + let params = []; + let handle = null; + let skip = 0; + let bk = [[path, n, params, handle, skip]]; + + srch: while (bk.length) { + [path, n, params, handle, skip] = bk.pop(); + + walk: while (true) { + const wpath = path + + if (path.length > n.path.length) { + if (path.slice(0, n.path.length) === n.path) { + path = path.slice(n.path.length); + // If this node does not have a wildcard child, + // we can just look up the next child node and continue + // to walk down the tree + if (!n.wildChild) { + const c = path.charCodeAt(0); + for (let i = 0; i < n.indices.length; i++) { + if (c === n.indices.charCodeAt(i)) { + n = n.children[i]; + continue walk; + } } - } - // Nothing found. - return { handle, params }; - } + // Nothing found. + continue srch; + } - // Handle wildcard child - n = n.children[0]; - switch (n.type) { - case PARAM: - // Find param end - let end = 0; - while (end < path.length && path.charCodeAt(end) !== 47) { - end++; + // Handle wildcard child + for (let i = skip; i < n.children.length; i++) { + skip = 0; + if (i < n.children.length - 1) { + // has more children to lookup in case nothing will be found + bk.push([wpath, n, params.slice(0), handle, i + 1]) } - // Save param value - params.push({ key: n.path.slice(1), value: path.slice(0, end) }); + n = n.children[i]; - // We need to go deeper! - if (end < path.length) { - if (n.children.length > 0) { - path = path.slice(end); - n = n.children[0]; - continue walk; - } + switch (n.type) { + case PARAM: + // Find param end + let end = 0; + while (end < path.length && path.charCodeAt(end) !== 47) { + end++; + } - // ... but we can't - return { handle, params }; - } + // Save param value + params.push({ key: n.path.slice(1), value: path.slice(0, end) }); + + // We need to go deeper! + if (end < path.length) { + if (n.children.length > 0) { + path = path.slice(end); + n = n.children[0]; + continue walk; + } + + // ... but we can't + return { handle, params }; + } - handle = n.handle; + handle = n.handle; - return { handle, params }; + if (!handle) { + continue srch; + } - case CATCH_ALL: - params.push({ key: n.path.slice(2), value: path }); + return { handle, params }; - handle = n.handle; - return { handle, params }; + case CATCH_ALL: + params.push({ key: n.path.slice(2), value: path }); - default: - throw new Error("invalid node type"); + handle = n.handle; + return { handle, params }; + + default: + continue walk; + } + } } + } else if (path === n.path) { + handle = n.handle; + } + + if (!handle) { + continue srch; } - } else if (path === n.path) { - handle = n.handle; - } - return { handle, params }; + break srch; + } } + return { handle, params }; } } From 6b7f845f1efa3012a5c9a29eda34b5712047bf7d Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Thu, 13 Jun 2024 07:50:44 +0700 Subject: [PATCH 2/6] fixed invalid type check --- tree.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tree.js b/tree.js index f0d7826..9db9ca4 100644 --- a/tree.js +++ b/tree.js @@ -427,8 +427,11 @@ class Node { handle = n.handle; return { handle, params }; - default: + case STATIC: continue walk; + + default: + throw new Error("invalid node type"); } } } From f14b5292f7a64a27c17e99499618c08c7c8434b4 Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Thu, 13 Jun 2024 08:41:10 +0700 Subject: [PATCH 3/6] rollbacked invalid wildcard duplicates check --- tree.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tree.js b/tree.js index 9db9ca4..fb58e0f 100644 --- a/tree.js +++ b/tree.js @@ -168,12 +168,13 @@ class Node { // Check if the wildcard matches if ( - (n.type === STATIC || ( - (n.path.length >= path.length || path[n.path.length] === "/") && - n.path === path.slice(0, n.path.length) - )) && // Adding a child to a catchAll is not possible - n.type !== CATCH_ALL + n.type !== CATCH_ALL && + path.length >= n.path.length && + // exactly matches wildcard and ...v + n.path === path.slice(0, n.path.length) && + // either exact match or starts a new subpath by slash + (n.path.length === path.length || path[n.path.length] === "/") ) { continue walk; } else { From 6314db6c3f71d0593d204b34bc1f48e408441719 Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Thu, 13 Jun 2024 08:42:08 +0700 Subject: [PATCH 4/6] improved tests to check that found handle is correct --- __tests__/tree-spec.js | 186 ++++++++++++++++++++++++++++------------- 1 file changed, 126 insertions(+), 60 deletions(-) diff --git a/__tests__/tree-spec.js b/__tests__/tree-spec.js index c5a6649..db9ad06 100644 --- a/__tests__/tree-spec.js +++ b/__tests__/tree-spec.js @@ -105,96 +105,162 @@ describe("Add and get", () => { describe("Wildcard", () => { const tree = new Tree(); const routes = [ - "/", - "/cmd/foo/bar", - "/cmd/:tool/", - "/cmd/:tool/:sub", - "/src/*filepath", - "/search/", - "/search/:query", - "/user_:name", - "/user_:name/about", - "/files/:dir/*filepath", - "/doc/", - "/doc/node_faq.html", - "/doc/node1.html", - "/info/:user/public", - "/info/:user/project/:project" - ]; - - routes.forEach(route => { - tree.addRoute(route, noOp); - }); - - // tree.printTree(); - - const foundData = [ { route: "/", - params: [] + found: [ + { + route: "/", + params: [] + }, + ], }, { route: "/cmd/foo/bar", - params: [] + found: [ + { + route: "/cmd/foo/bar", + params: [] + }, + ], }, { - route: "/cmd/test/", - params: [{ key: "tool", value: "test" }] + route: "/cmd/a_:tool", + found: [ + { + route: "/cmd/a_test", + params: [{ key: "tool", value: "test" }] + }, + ], }, { - route: "/cmd/test/3", - params: [{ key: "tool", value: "test" }, { key: "sub", value: "3" }] + route: "/cmd/:tool/", + found: [ + { + route: "/cmd/test/", + params: [{ key: "tool", value: "test" }] + }, + ], }, { - route: "/src/", - params: [{ key: "filepath", value: "/" }] + route: "/cmd/:tool/:sub", + found: [ + { + route: "/cmd/test/3", + params: [{ key: "tool", value: "test" }, { key: "sub", value: "3" }] + }, + ], }, { - route: "/src/some/file.png", - params: [{ key: "filepath", value: "/some/file.png" }] + route: "/src/*filepath", + found: [ + { + route: "/src/", + params: [{ key: "filepath", value: "/" }] + }, + { + route: "/src/some/file.png", + params: [{ key: "filepath", value: "/some/file.png" }] + }, + ], }, { route: "/search/", - params: [] + found: [ + { + route: "/search/", + params: [] + }, + ], }, { - route: "/search/中文", - params: [{ key: "query", value: "中文" }] + route: "/search/:query", + found: [ + { + route: "/search/中文", + params: [{ key: "query", value: "中文" }] + }, + ], }, { - route: "/user_noder", - params: [{ key: "name", value: "noder" }] + route: "/user_:name", + found: [ + { + route: "/user_noder", + params: [{ key: "name", value: "noder" }] + }, + ], }, { - route: "/user_noder/about", - params: [{ key: "name", value: "noder" }] + route: "/user_:name/about", + found: [ + { + route: "/user_noder/about", + params: [{ key: "name", value: "noder" }] + }, + ], }, { - route: "/files/js/inc/framework.js", - params: [ - { key: "dir", value: "js" }, - { key: "filepath", value: "/inc/framework.js" } - ] + route: "/files/:dir/*filepath", + found: [ + { + route: "/files/js/inc/framework.js", + params: [ + { key: "dir", value: "js" }, + { key: "filepath", value: "/inc/framework.js" } + ] + }, + ], }, { - route: "/info/gordon/public", - params: [{ key: "user", value: "gordon" }] + route: "/doc/", + found: [], }, { - route: "/info/gordon/project/node", - params: [ - { key: "user", value: "gordon" }, - { key: "project", value: "node" } - ] - } + route: "/doc/node_faq.html", + found: [], + }, + { + route: "/doc/node1.html", + found: [], + }, + { + route: "/info/:user/public", + found: [ + { + route: "/info/gordon/public", + params: [{ key: "user", value: "gordon" }] + }, + ], + }, + { + route: "/info/:user/project/:project", + found: [ + { + route: "/info/gordon/project/node", + params: [ + { key: "user", value: "gordon" }, + { key: "project", value: "node" } + ] + }, + ], + }, ]; - foundData.forEach(data => { - it(data.route, () => { - const { handle, params } = tree.search(data.route); - expect(handle).toBeTruthy(); - expect(params).toMatchObject(data.params); - }); + routes.forEach((route) => { + route.handle = noOp.slice(0) // creating an unique handle + tree.addRoute(route.route, route.handle); + }); + + // tree.printTree(); + + routes.forEach((route) => { + route.found.forEach((data) => { + it(data.route, () => { + const { handle, params } = tree.search(data.route); + expect(handle).toBe(route.handle); + expect(params).toMatchObject(data.params); + }); + }) }); const noHandlerData = [ From d08adc3ba959a2845078c9de68826a7efbb43e20 Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Fri, 14 Jun 2024 17:06:54 +0700 Subject: [PATCH 5/6] optimizations iter 1 --- tree.js | 156 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 80 insertions(+), 76 deletions(-) diff --git a/tree.js b/tree.js index fb58e0f..2d76fa8 100644 --- a/tree.js +++ b/tree.js @@ -346,108 +346,112 @@ class Node { } /** * - * @param {string} ipath + * @param {string} path */ - search(ipath) { - let path = ipath; + search(path) { let n = this; let params = []; let handle = null; let skip = 0; - let bk = [[path, n, params, handle, skip]]; - - srch: while (bk.length) { - [path, n, params, handle, skip] = bk.pop(); - - walk: while (true) { - const wpath = path - - if (path.length > n.path.length) { - if (path.slice(0, n.path.length) === n.path) { - path = path.slice(n.path.length); - // If this node does not have a wildcard child, - // we can just look up the next child node and continue - // to walk down the tree - if (!n.wildChild) { - const c = path.charCodeAt(0); - for (let i = 0; i < n.indices.length; i++) { - if (c === n.indices.charCodeAt(i)) { - n = n.children[i]; - continue walk; - } + let bk = []; + let reset = false; + + walk: while (true) { + if (reset && bk.length) { + ({path, n, params, handle, skip} = bk.pop()); + reset = false; + } + + const wpath = path + + if (path.length > n.path.length) { + if (path.indexOf(n.path) === 0) { + path = path.slice(n.path.length); + // If this node does not have a wildcard child, + // we can just look up the next child node and continue + // to walk down the tree + if (!n.wildChild) { + const c = path.charCodeAt(0); + for (let i = 0; i < n.indices.length; i++) { + if (c === n.indices.charCodeAt(i)) { + n = n.children[i]; + continue walk; } + } - // Nothing found. - continue srch; + // Nothing found. + if (bk.length) { + reset = true; + continue walk; } + return { handle, params }; + } - // Handle wildcard child - for (let i = skip; i < n.children.length; i++) { - skip = 0; - if (i < n.children.length - 1) { - // has more children to lookup in case nothing will be found - bk.push([wpath, n, params.slice(0), handle, i + 1]) - } + if (skip < n.wildChildIdx) { + // has more children to lookup in case nothing will be found + bk.push({ path: wpath, n, params: params.slice(0), handle, skip: skip + 1 }) + } - n = n.children[i]; + n = n.children[skip]; + skip = 0; - switch (n.type) { - case PARAM: - // Find param end - let end = 0; - while (end < path.length && path.charCodeAt(end) !== 47) { - end++; - } + // Handle wildcard child + switch (n.type) { + case PARAM: + // Find param end + let end = 0; + while (end < path.length && path.charCodeAt(end) !== 47) { + end++; + } - // Save param value - params.push({ key: n.path.slice(1), value: path.slice(0, end) }); + // Save param value + params.push({ key: n.path.slice(1), value: path.slice(0, end) }); - // We need to go deeper! - if (end < path.length) { - if (n.children.length > 0) { - path = path.slice(end); - n = n.children[0]; - continue walk; - } + // We need to go deeper! + if (end < path.length) { + if (n.children.length > 0) { + path = path.slice(end); + n = n.children[0]; + continue walk; + } - // ... but we can't - return { handle, params }; - } + // ... but we can't + return { handle, params }; + } - handle = n.handle; + handle = n.handle; - if (!handle) { - continue srch; - } + if (handle === null && bk.length) { + reset = true; + continue walk; + } - return { handle, params }; + return { handle, params }; - case CATCH_ALL: - params.push({ key: n.path.slice(2), value: path }); + case CATCH_ALL: + params.push({ key: n.path.slice(2), value: path }); - handle = n.handle; - return { handle, params }; + handle = n.handle; + return { handle, params }; - case STATIC: - continue walk; + case STATIC: + continue walk; - default: - throw new Error("invalid node type"); - } - } + default: + throw new Error("invalid node type"); } - } else if (path === n.path) { - handle = n.handle; - } - - if (!handle) { - continue srch; } + } else if (path === n.path) { + handle = n.handle; + } - break srch; + if (handle === null && bk.length) { + reset = true; + continue walk; } + + return { handle, params }; } - return { handle, params }; } } From c277e3a535f48f902c0498fc88299eeeb66d3552 Mon Sep 17 00:00:00 2001 From: Philipp Malkov Date: Fri, 14 Jun 2024 17:21:28 +0700 Subject: [PATCH 6/6] optimizations iter 2 --- tree.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tree.js b/tree.js index 2d76fa8..653c16c 100644 --- a/tree.js +++ b/tree.js @@ -349,15 +349,15 @@ class Node { * @param {string} path */ search(path) { - let n = this; - let params = []; + let bk = null; let handle = null; + let params = []; + let n = this; let skip = 0; - let bk = []; let reset = false; walk: while (true) { - if (reset && bk.length) { + if (reset) { ({path, n, params, handle, skip} = bk.pop()); reset = false; } @@ -365,7 +365,7 @@ class Node { const wpath = path if (path.length > n.path.length) { - if (path.indexOf(n.path) === 0) { + if (path.slice(0, n.path.length) === n.path) { path = path.slice(n.path.length); // If this node does not have a wildcard child, // we can just look up the next child node and continue @@ -380,7 +380,7 @@ class Node { } // Nothing found. - if (bk.length) { + if (bk && bk.length) { reset = true; continue walk; } @@ -389,6 +389,9 @@ class Node { if (skip < n.wildChildIdx) { // has more children to lookup in case nothing will be found + if (!bk) { + bk = [] + } bk.push({ path: wpath, n, params: params.slice(0), handle, skip: skip + 1 }) } @@ -421,7 +424,7 @@ class Node { handle = n.handle; - if (handle === null && bk.length) { + if (!handle && bk && bk.length) { reset = true; continue walk; } @@ -445,7 +448,7 @@ class Node { handle = n.handle; } - if (handle === null && bk.length) { + if (!handle && bk && bk.length) { reset = true; continue walk; }