fix(Tree): corrects lookup issue with catch all and shared key

Lookup was failing when part of the given path matched a key at the
first character, even when the rest of the key didn't match.

It was not possible place a catch all an another key at the same
level that started with same character.

This change ensures that all shared part between path and key is
compared prior assuming usage of that node as part of the lookup.

Closes #14
This commit is contained in:
Luis Lavena 2016-11-24 20:52:07 -03:00
parent 6880b341a6
commit 95b6638f1a
3 changed files with 62 additions and 3 deletions

View file

@ -5,6 +5,8 @@ This project aims to comply with [Semantic Versioning](http://semver.org/),
so please check *Changed* and *Removed* notes before upgrading. so please check *Changed* and *Removed* notes before upgrading.
## [Unreleased] ## [Unreleased]
### Fixed
- Correct lookup issue when dealing with catch all and shared partial key (@crisward)
## [0.3.4] - 2016-11-12 ## [0.3.4] - 2016-11-12
### Fixed ### Fixed

View file

@ -404,6 +404,18 @@ module Radix
result.found?.should be_true result.found?.should be_true
result.key.should eq("/members") result.key.should eq("/members")
end end
it "does prefer catch all over specific key with partially shared key" do
tree = Tree(Symbol).new
tree.add "/orders/*anything", :orders_catch_all
tree.add "/orders/closed", :closed_orders
result = tree.find("/orders/cancelled")
result.found?.should be_true
result.key.should eq("/orders/*anything")
result.params.has_key?("anything").should be_true
result.params["anything"].should eq("cancelled")
end
end end
context "dealing with named parameters" do context "dealing with named parameters" do

View file

@ -313,9 +313,10 @@ module Radix
# not found in current node, check inside children nodes # not found in current node, check inside children nodes
new_path = path_reader.string.byte_slice(path_reader.pos) new_path = path_reader.string.byte_slice(path_reader.pos)
node.children.each do |child| node.children.each do |child|
# check if child first character matches the new path # check if child key is a named parameter, catch all or shares parts
if child.key[0]? == new_path[0]? || # with new path
child.key[0]? == '*' || child.key[0]? == ':' if (child.key[0]? == '*' || child.key[0]? == ':') ||
_shared_key?(new_path, child.key)
# consider this node for key but don't use payload # consider this node for key but don't use payload
result.use node, payload: false result.use node, payload: false
@ -376,6 +377,16 @@ module Radix
count count
end end
# Internal: allow inline comparison of *char* against 3 defined markers:
#
# - Path separator (`/`)
# - Named parameter (`:`)
# - Catch all (`*`)
@[AlwaysInline]
private def _check_markers(char)
(char == '/' || char == ':' || char == '*')
end
# Internal: Compares *path* against *key* for differences until the # Internal: Compares *path* against *key* for differences until the
# following criteria is met: # following criteria is met:
# #
@ -409,6 +420,40 @@ module Radix
(path_reader.current_char == '/' || !path_reader.has_next?) (path_reader.current_char == '/' || !path_reader.has_next?)
end end
# Internal: Compares *path* against *key* for equality until one of the
# following criterias is met:
#
# - End of *path* or *key* is reached.
# - A separator (`/`) is found.
# - A named parameter (`:`) or catch all (`*`) is found.
# - A character in *path* differs from *key*
#
# ```
# _shared_key?("foo", "bar") # => false (mismatch at 1st character)
# _shared_key?("foo/bar", "foo/baz") # => true (only `foo` is compared)
# _shared_key?("zipcode", "zip") # => true (only `zip` is compared)
# ```
private def _shared_key?(path, key)
path_reader = Char::Reader.new(path)
key_reader = Char::Reader.new(key)
different = false
while (path_reader.has_next? && !_check_markers(path_reader.current_char)) &&
(key_reader.has_next? && !_check_markers(key_reader.current_char))
if path_reader.current_char != key_reader.current_char
different = true
break
end
path_reader.next_char
key_reader.next_char
end
(!different) &&
(!key_reader.has_next? || _check_markers(key_reader.current_char))
end
# :nodoc: # :nodoc:
private def deprecation(message : String) private def deprecation(message : String)
STDERR.puts message STDERR.puts message