From 651b4d5ea5baaa05876bdea83b910bf4aa0d99ac Mon Sep 17 00:00:00 2001 From: Niclas Axelsson Date: Mon, 18 Apr 2022 22:46:01 +0200 Subject: [PATCH 1/5] Add case for when we have trailing slash --- src/routing_tree.erl | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/routing_tree.erl b/src/routing_tree.erl index 3aa02da..c51cfaf 100644 --- a/src/routing_tree.erl +++ b/src/routing_tree.erl @@ -70,7 +70,8 @@ lookup_path([Segment|Tl], Comparator, Tree, {Bindings, _}) -> end. -lookup_binary(<<>>, Comparator, Tree, {Bindings, PrevNode}, Ack) -> +lookup_binary(Empty, Comparator, Tree, {Bindings, PrevNode}, Ack) when Empty == <<>> orelse + Empty == <<"/">> -> Node = case Ack of <<>> -> PrevNode; @@ -493,6 +494,16 @@ dash_in_path_test() -> ?assertEqual(Expected, C). +trailing_slash_test() -> + A = new(#{convert_to_binary => true}), + B = insert('_', "/my_app/", "GET", "ONE", A), + C = lookup(<<"my_host">>, <<"/my_app">>, "GET", B), + D = lookup(<<"my_host2">>, <<"/my_app/">>, "GET", B), + Expected = {ok, #{}, "ONE"}, + Expected0 = {ok, #{}, "ONE"}, + ?assertEqual(Expected, C), + ?assertEqual(Expected0, D). + get_random_string(Length, AllowedChars) -> lists:foldl(fun(_, Acc) -> [lists:nth(rand:uniform(length(AllowedChars)), @@ -509,5 +520,11 @@ insert_10000_inserts_test() -> insert('_', Path, "GET", "PAYLOAD", T) end, new(), Paths)). +insert_1000_inserts_test() -> + Paths = [ + lists:flatten([ [$/|get_random_string(20, lists:seq($a, $z))] || _X <- lists:seq(0, 20)]) || _Y <- lists:seq(0, 1000) ], + ?debugTime("Inserting 1000 paths into same tree", lists:foldl(fun(Path, T) -> + insert('_', Path, "GET", "PAYLOAD", T) + end, new(), Paths)). -endif. From 752c28877464c72b474b9169d2bbda21de78ce88 Mon Sep 17 00:00:00 2001 From: Niclas Axelsson Date: Mon, 18 Apr 2022 22:47:11 +0200 Subject: [PATCH 2/5] Add case for trailing slashes --- src/routing_tree.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/routing_tree.erl b/src/routing_tree.erl index c51cfaf..36c8e68 100644 --- a/src/routing_tree.erl +++ b/src/routing_tree.erl @@ -70,8 +70,9 @@ lookup_path([Segment|Tl], Comparator, Tree, {Bindings, _}) -> end. -lookup_binary(Empty, Comparator, Tree, {Bindings, PrevNode}, Ack) when Empty == <<>> orelse - Empty == <<"/">> -> +lookup_binary(Empty, Comparator, Tree, {Bindings, PrevNode}, Ack) when Ack /= <<>> andalso + (Empty == <<>> orelse + Empty == <<"/">>) -> Node = case Ack of <<>> -> PrevNode; From f4eaff5344c9e94a4e3aded6068fe4feab2f044b Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Sat, 23 Apr 2022 11:40:44 +0200 Subject: [PATCH 3/5] moved lists:keyfind in insert to a own function called lookup_node for more debugging options --- src/routing_tree.erl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/routing_tree.erl b/src/routing_tree.erl index 36c8e68..e6602a4 100644 --- a/src/routing_tree.erl +++ b/src/routing_tree.erl @@ -190,7 +190,7 @@ insert([{Type, Ident}|Tl], CompNode, Siblings, Options = #{use_strict := UseStri ok end, - case lists:keyfind(Ident, #node.segment, Siblings) of + case lookup_node(Ident, Options, Siblings) of false -> %% Nothing found - Just add the tree case Tl of @@ -299,7 +299,13 @@ check_conflicting_nodes_binding(#node{is_wildcard = true}, _, _) -> true; check_conflicting_nodes_binding(#node{value = Value} = Node, CompNode, _) -> {[ X || #node_comp{comparator = C, value = X} <- Value, C == CompNode#node_comp.comparator ] /= [], {conflict, Node}}. +value(Value, #{convert_to_binary := true}) when is_list(Value) -> + erlang:list_to_binary(Value); +value(Value, _) -> + Value. +lookup_node(Ident, Options, Siblings) -> + lists:keyfind(value(Ident, Options), #node.segment, Siblings). %%======================================== %% EUnit tests %%======================================== From f7bc125c01e7c5efab362d20987c7d9488737bcb Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Mon, 2 May 2022 21:17:18 +0200 Subject: [PATCH 4/5] rebased on master --- src/routing_tree.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routing_tree.erl b/src/routing_tree.erl index e6602a4..e8498a6 100644 --- a/src/routing_tree.erl +++ b/src/routing_tree.erl @@ -206,7 +206,7 @@ insert([{Type, Ident}|Tl], CompNode, Siblings, Options = #{use_strict := UseStri end; Node -> case Tl of - [] -> + List when ( List == []) orelse (List == [{segment, 1, []}]) -> case find_comparator(CompNode#node_comp.comparator, Node#node.value) of {error, not_found} -> [Node#node{value = [CompNode|Node#node.value], is_binding = Type == binding, From eaec263cec77867f73ad4abef1a055478d6514e4 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Mon, 2 May 2022 21:23:28 +0200 Subject: [PATCH 5/5] found issue after rebase on master --- src/routing_tree.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routing_tree.erl b/src/routing_tree.erl index e8498a6..5ba450b 100644 --- a/src/routing_tree.erl +++ b/src/routing_tree.erl @@ -197,7 +197,7 @@ insert([{Type, Ident}|Tl], CompNode, Siblings, Options = #{use_strict := UseStri [] -> [#node{segment = Ident, is_binding = Type == binding, is_wildcard = Type == wildcard, value = [CompNode]} | Siblings]; - [{segment, 1, []}] -> + [{segment, <<>>}] -> [#node{segment = Ident, is_binding = Type == binding, is_wildcard = Type == wildcard, value = [CompNode]} | Siblings]; _ -> @@ -206,7 +206,7 @@ insert([{Type, Ident}|Tl], CompNode, Siblings, Options = #{use_strict := UseStri end; Node -> case Tl of - List when ( List == []) orelse (List == [{segment, 1, []}]) -> + List when ( List == []) orelse (List == [{segment, <<>>}]) -> case find_comparator(CompNode#node_comp.comparator, Node#node.value) of {error, not_found} -> [Node#node{value = [CompNode|Node#node.value], is_binding = Type == binding,