Merge pull request #17 from luislavena/fix-sorting-named-parameters

Lower named and glob node priority to fix lookup issue

CHANGELOG.md

@@ -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

spec/radix/node_spec.cr

@@ -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
 

spec/radix/tree_spec.cr

@@ -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

src/radix/node.cr

@@ -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 }