diff --git a/CHANGELOG.md b/CHANGELOG.md index 41dbf94..a6711b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ 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 when dealing with catch all and shared partial key (@crisward) ## [0.3.4] - 2016-11-12 ### Fixed diff --git a/spec/radix/tree_spec.cr b/spec/radix/tree_spec.cr index 82bdcec..0e4b288 100644 --- a/spec/radix/tree_spec.cr +++ b/spec/radix/tree_spec.cr @@ -404,6 +404,18 @@ module Radix result.found?.should be_true result.key.should eq("/members") 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 context "dealing with named parameters" do diff --git a/src/radix/tree.cr b/src/radix/tree.cr index 8c23e47..49d910b 100644 --- a/src/radix/tree.cr +++ b/src/radix/tree.cr @@ -313,9 +313,10 @@ module Radix # not found in current node, check inside children nodes new_path = path_reader.string.byte_slice(path_reader.pos) node.children.each do |child| - # check if child first character matches the new path - if child.key[0]? == new_path[0]? || - child.key[0]? == '*' || child.key[0]? == ':' + # 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 @@ -376,6 +377,16 @@ module Radix count 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 # following criteria is met: # @@ -409,6 +420,40 @@ module Radix (path_reader.current_char == '/' || !path_reader.has_next?) 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: private def deprecation(message : String) STDERR.puts message