From 93247cd33a5525af9786d7c86b20d85b129bed01 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Sat, 30 Jan 2021 22:25:26 -0300 Subject: [PATCH] Correct lookup with shared key and glob Properly skip nodes and continue lookup when the key to be looked up shares partial elements with others. With the following scenario: tree = Radix::Tree(Symbol).new tree.add "/*glob", :catch_all tree.add "/resources", :resources tree.add "/robots.txt", :robots When attempt to lookup for `/reviews`, it will now correctly return `:catch_all` as found. Fixes #23 --- CHANGELOG.md | 3 +++ spec/radix/tree_spec.cr | 13 +++++++++++++ src/radix/tree.cr | 16 ++++++++++------ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e929196..c5f44d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ so please check *Changed* and *Removed* notes before upgrading. ## [Unreleased] +### Fixed +- Correct lookup issue caused by partial shared key with glob [#23](https://github.com/luislavena/radix/issues/23) + ### Removed - Remove `Radix::Result#key` since exposes internal details about structure (breaking change) diff --git a/spec/radix/tree_spec.cr b/spec/radix/tree_spec.cr index d24ac95..f848f86 100644 --- a/spec/radix/tree_spec.cr +++ b/spec/radix/tree_spec.cr @@ -451,6 +451,19 @@ module Radix result.params.has_key?("anything").should be_true result.params["anything"].should eq("cancelled") end + + it "does prefer root catch all over specific partially shared key" do + tree = Tree(Symbol).new + tree.add "/*anything", :root_catch_all + tree.add "/robots.txt", :robots + tree.add "/resources", :resources + + result = tree.find("/reviews") + result.found?.should be_true + result.payload.should eq(:root_catch_all) + result.params.has_key?("anything").should be_true + result.params["anything"].should eq("reviews") + end end context "dealing with named parameters" do diff --git a/src/radix/tree.cr b/src/radix/tree.cr index 96c8276..3d930dd 100644 --- a/src/radix/tree.cr +++ b/src/radix/tree.cr @@ -315,13 +315,17 @@ module Radix node.children.each do |child| # check if child key is a named parameter, catch all or shares parts # with new path - if (child.key[0]? == '*' || child.key[0]? == ':') || - _shared_key?(new_path, child.key) - # consider this node for key but don't use payload - result.use node, payload: false - + if (child.glob? || child.named?) || _shared_key?(new_path, child.key) + # traverse branch to determine if valid find new_path, result, child - return + + if result.found? + # stop iterating over nodes + return + else + # move to next child + next + end end end