Lowers named and glob node priority to fix lookup issue

Given two similar keys, one short and one with named parameter, lookup
was incorrectly picking up the named parameter version instead of the
specific one:

    tree = Radix::Tree(Symbol).new
    tree.add "/tag-edit/:id", :edit_tag
    tree.add "/tag-edit2", :alternate_edit_tag

    result = tree.find("/tag-edit2")
    result.found? # => false

The order of insertion (named before specific) was causing the
lookup mechanism to validate it before checking the other options.

With the changes present here, short or long keys will be affected
anymore by named or globbed keys present when sharing part of the
key.

Fixes kemalcr/kemal#293
This commit is contained in:
Luis Lavena 2017-01-17 14:30:20 -03:00
parent 40b8e26ba5
commit 9163860e4d
4 changed files with 40 additions and 15 deletions

View file

@ -5,6 +5,9 @@ This project aims to comply with [Semantic Versioning](http://semver.org/),
so please check *Changed* and *Removed* notes before upgrading.
## [Unreleased]
### Fixed
- Correct lookup issue caused by similar priority between named paramter and
shared partial key [kemalcr/kemal#293](https://github.com/kemalcr/kemal/issues/293)
## [0.3.5] - 2016-11-24
### Fixed

View file

@ -46,18 +46,18 @@ module Radix
it "returns zero for catch all (globbed) key" do
node = Node(Nil).new("*filepath")
node.priority.should eq(0)
node.priority.should eq(-2)
node = Node(Nil).new("/src/*filepath")
node.priority.should eq(0)
node.priority.should eq(-2)
end
it "returns one for keys with named parameters" do
node = Node(Nil).new(":query")
node.priority.should eq(1)
node.priority.should eq(-1)
node = Node(Nil).new("/search/:query")
node.priority.should eq(1)
node.priority.should eq(-1)
end
it "changes when key changes" do
@ -68,10 +68,10 @@ module Radix
node.priority.should eq(3)
node.key = "*filepath"
node.priority.should eq(0)
node.priority.should eq(-2)
node.key = ":query"
node.priority.should eq(1)
node.priority.should eq(-1)
end
end

View file

@ -466,6 +466,28 @@ module Radix
result.params.has_key?("name").should be_true
result.params["name"].should eq("日本語")
end
it "does prefer specific path over named parameters one if both are present" do
tree = Tree(Symbol).new
tree.add "/tag-edit/:tag", :edit_tag
tree.add "/tag-edit2", :alternate_tag_edit
result = tree.find("/tag-edit2")
result.found?.should be_true
result.key.should eq("/tag-edit2")
end
it "does prefer named parameter over specific key with partially shared key" do
tree = Tree(Symbol).new
tree.add "/orders/:id", :specific_order
tree.add "/orders/closed", :closed_orders
result = tree.find("/orders/10")
result.found?.should be_true
result.key.should eq("/orders/:id")
result.params.has_key?("id").should be_true
result.params["id"].should eq("10")
end
end
context "dealing with multiple named parameters" do

View file

@ -37,8 +37,8 @@ module Radix
# This value will be directly associated to the key size or special
# elements of it.
#
# * A catch all (globbed) key will receive lowest priority (`0`)
# * A named parameter key will receive priority above catch all (`1`)
# * A catch all (globbed) key will receive lowest priority (`-2`)
# * A named parameter key will receive priority above catch all (`-1`)
# * Any other type of key will receive priority based on its size.
#
# ```
@ -49,10 +49,10 @@ module Radix
# # => 3
#
# Radix::Node(Nil).new("*filepath").priority
# # => 0
# # => -2
#
# Radix::Node(Nil).new(":query").priority
# # => 1
# # => -1
# ```
getter priority : Int32
@ -113,9 +113,9 @@ module Radix
while reader.has_next?
case reader.current_char
when '*'
return 0
return -2
when ':'
return 1
return -1
else
reader.next_char
end
@ -130,14 +130,14 @@ module Radix
#
# ```
# root = Radix::Node(Nil).new("/")
# root.children << Radix::Node(Nil).new("*filepath") # node.priority => 0
# root.children << Radix::Node(Nil).new(":query") # node.priority => 1
# root.children << Radix::Node(Nil).new("*filepath") # node.priority => -2
# root.children << Radix::Node(Nil).new(":query") # node.priority => -1
# root.children << Radix::Node(Nil).new("a") # node.priority => 1
# root.children << Radix::Node(Nil).new("bc") # node.priority => 2
# root.sort!
#
# root.children.map &.priority
# # => [2, 1, 1, 0]
# # => [2, 1, -1, -2]
# ```
def sort!
@children.sort_by! { |node| -node.priority }