From da4cb2f90aac299e3d958b8ebd08888fc76044d5 Mon Sep 17 00:00:00 2001 From: Albert Sverdlov Date: Tue, 2 Jul 2019 00:37:30 +0300 Subject: [PATCH] router: route specificity by symbols known Now we prioritize route (a) over (b) for some request that matches both: 1. if it has more "constant" symbols (that is symbols hardcoded in the path and not matched in regexp group generated from : or * stashes), or, if this number is equal, 2. if (a) has more specific filter on HTTP method --- http/router/matching.lua | 18 ++++++++++++++---- test/http.test.lua | 31 +++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/http/router/matching.lua b/http/router/matching.lua index 322c29c..2733779 100644 --- a/http/router/matching.lua +++ b/http/router/matching.lua @@ -77,9 +77,19 @@ local function matches(r, filter) return false end + -- how many symbols were not known (were hidden behind : and * patterns) + local symbols_didnt_know = 0 + for _, matched_part in ipairs(regex_groups_matched) do + symbols_didnt_know = symbols_didnt_know + #matched_part + end + return true, { route = r, stash = regex_groups_matched, + + -- the more symbols were known in advance by route, + -- the more priority we give the route + specificity = -symbols_didnt_know, } end @@ -91,10 +101,10 @@ local function better_than(newmatch, oldmatch) return true end - -- current match (route) is prioritized iff: - -- 1. it has less matched words, or - -- 2. if current match (route) has more specific method filter - if #oldmatch.stash > #newmatch.stash then + -- newmatch route is prioritized over oldmatch iff: + -- 1. its' path is more specific (see matches() function), or + -- 2. if current route has more specific method filter + if newmatch.specificity > oldmatch.specificity then return true end return newmatch.route.method ~= oldmatch.route.method and diff --git a/test/http.test.lua b/test/http.test.lua index a1fc1e9..4324c89 100755 --- a/test/http.test.lua +++ b/test/http.test.lua @@ -257,7 +257,7 @@ test:test("server url_for", function(test) end) test:test("server requests", function(test) - test:plan(42) + test:plan(43) local httpd, router = cfgserv() httpd:start() @@ -501,7 +501,6 @@ test:test("server requests", function(test) test:is(parsed_body.read_cached, 'hello mister', 'non-json req:read_cached()') end) - if is_builtin_test() then test:test('post body', function(test) test:plan(2) @@ -569,6 +568,34 @@ test:test("server requests", function(test) test:ok(true, 'pong received - ignored on NGINX') end + test:test('prioritization of more specific routes', function(test) + test:plan(4) + + router:route({method = 'GET', path = '*stashname'}, function(_) + return { + status = 200, + body = 'GET *', + } + end) + local r = http_client.get('http://127.0.0.1:12345/a/b/c') + test:is(r.status, 200, '/a/b/c request returns 200') + test:is(r.body, 'GET *', 'GET * matches') + + router:route({method = 'ANY', path = '/a/:foo/:bar'}, function(_) + return { + status = 200, + body = 'ANY /a/:foo/:bar', + } + end) + local r = http_client.get('http://127.0.0.1:12345/a/b/c') + test:is(r.status, 200, '/a/b/c request returns 200') + test:is( + r.body, + 'ANY /a/:foo/:bar', + '# of stashes matched doesnt matter - only # of known symbols by the route matters' + ) + end) + httpd:stop() end)