diff --git a/CHANGELOG.md b/CHANGELOG.md index 38aa01d..15c56ab 100644 --- a/CHANGELOG.md +++ b/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 prioritization of node's children using combination of kind and + priority, allowing partial shared keys to coexist and resolve lookup. ## [0.3.6] - 2017-01-18 ### Fixed diff --git a/spec/radix/node_spec.cr b/spec/radix/node_spec.cr index 4be39c9..e43ffaa 100644 --- a/spec/radix/node_spec.cr +++ b/spec/radix/node_spec.cr @@ -2,6 +2,16 @@ require "../spec_helper" module Radix describe Node do + describe "#glob?" do + it "returns true when key contains a glob parameter (catch all)" do + node = Node(Nil).new("a") + node.glob?.should be_false + + node = Node(Nil).new("*filepath") + node.glob?.should be_true + end + end + describe "#key=" do it "accepts change of key after initialization" do node = Node(Nil).new("abc") @@ -10,6 +20,38 @@ module Radix node.key = "xyz" node.key.should eq("xyz") end + + it "also changes kind when modified" do + node = Node(Nil).new("abc") + node.normal?.should be_true + + node.key = ":query" + node.normal?.should be_false + node.named?.should be_true + end + end + + describe "#named?" do + it "returns true when key contains a named parameter" do + node = Node(Nil).new("a") + node.named?.should be_false + + node = Node(Nil).new(":query") + node.named?.should be_true + end + end + + describe "#normal?" do + it "returns true when key does not contain named or glob parameters" do + node = Node(Nil).new("a") + node.normal?.should be_true + + node = Node(Nil).new(":query") + node.normal?.should be_false + + node = Node(Nil).new("*filepath") + node.normal?.should be_false + end end describe "#payload" do @@ -36,7 +78,7 @@ module Radix end describe "#priority" do - it "calculates it based on key size" do + it "calculates it based on key length" do node = Node(Nil).new("a") node.priority.should eq(1) @@ -44,20 +86,20 @@ module Radix node.priority.should eq(3) end - it "returns zero for catch all (globbed) key" do - node = Node(Nil).new("*filepath") - node.priority.should eq(-2) + it "considers key length up until named parameter presence" do + node = Node(Nil).new("/posts/:id") + node.priority.should eq(7) - node = Node(Nil).new("/src/*filepath") - node.priority.should eq(-2) + node = Node(Nil).new("/u/:username") + node.priority.should eq(3) end - it "returns one for keys with named parameters" do - node = Node(Nil).new(":query") - node.priority.should eq(-1) + it "considers key length up until glob parameter presence" do + node = Node(Nil).new("/search/*query") + node.priority.should eq(8) - node = Node(Nil).new("/search/:query") - node.priority.should eq(-1) + node = Node(Nil).new("/*anything") + node.priority.should eq(1) end it "changes when key changes" do @@ -67,16 +109,16 @@ module Radix node.key = "abc" node.priority.should eq(3) - node.key = "*filepath" - node.priority.should eq(-2) + node.key = "/src/*filepath" + node.priority.should eq(5) - node.key = ":query" - node.priority.should eq(-1) + node.key = "/search/:query" + node.priority.should eq(8) end end describe "#sort!" do - it "orders children by priority" do + it "orders children" do root = Node(Int32).new("/") node1 = Node(Int32).new("a", 1) node2 = Node(Int32).new("bc", 2) @@ -90,7 +132,7 @@ module Radix root.children[2].should eq(node1) end - it "orders catch all and named parameters lower than others" do + it "orders catch all and named parameters lower than normal nodes" do root = Node(Int32).new("/") node1 = Node(Int32).new("*filepath", 1) node2 = Node(Int32).new("abc", 2) diff --git a/spec/radix/tree_spec.cr b/spec/radix/tree_spec.cr index a59132e..4c8ce04 100644 --- a/spec/radix/tree_spec.cr +++ b/spec/radix/tree_spec.cr @@ -544,6 +544,19 @@ module Radix result.payload.should eq(:featured) end end + + context "dealing with named parameters and shared key" do + it "finds matching path" do + tree = Tree(Symbol).new + tree.add "/one/:id", :one + tree.add "/one-longer/:id", :two + + result = tree.find "/one-longer/10" + result.found?.should be_true + result.key.should eq("/one-longer/:id") + result.params["id"].should eq("10") + end + end end end end diff --git a/src/radix/node.cr b/src/radix/node.cr index 985c323..a87d39e 100644 --- a/src/radix/node.cr +++ b/src/radix/node.cr @@ -4,42 +4,34 @@ module Radix # Carries a *payload* and might also contain references to other nodes # down in the organization inside *children*. # - # Each node also carries a *priority* number, which might indicate the - # weight of this node depending on characteristics like catch all - # (globbing), named parameters or simply the size of the Node's *key*. - # - # Referenced nodes inside *children* can be sorted by *priority*. + # Each node also carries identification in relation to the kind of key it + # contains, which helps with characteristics of the node like named + # parameters or catch all kind (globbing). # # Is not expected direct usage of a node but instead manipulation via # methods within `Tree`. - # - # ``` - # node = Radix::Node.new("/", :root) - # node.children << Radix::Node.new("a", :a) - # node.children << Radix::Node.new("bc", :bc) - # node.children << Radix::Node.new("def", :def) - # node.sort! - # - # node.priority - # # => 1 - # - # node.children.map &.priority - # # => [3, 2, 1] - # ``` class Node(T) + include Comparable(self) + + # :nodoc: + enum Kind : UInt8 + Normal + Named + Glob + end + getter key getter? placeholder + property children = [] of Node(T) property! payload : T | Nil - property children + + # :nodoc: + protected getter kind = Kind::Normal # Returns the priority of the Node based on it's *key* # - # This value will be directly associated to the key size or special - # elements of it. - # - # * 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. + # This value will be directly associated to the key size up until a + # special elements is found. # # ``` # Radix::Node(Nil).new("a").priority @@ -48,11 +40,11 @@ module Radix # Radix::Node(Nil).new("abc").priority # # => 3 # - # Radix::Node(Nil).new("*filepath").priority - # # => -2 + # Radix::Node(Nil).new("/src/*filepath").priority + # # => 5 # - # Radix::Node(Nil).new(":query").priority - # # => -1 + # Radix::Node(Nil).new("/search/:query").priority + # # => 8 # ``` getter priority : Int32 @@ -75,10 +67,62 @@ module Radix # node = Radix::Node.new("/") # ``` def initialize(@key : String, @payload : T? = nil, @placeholder = false) - @children = [] of Node(T) @priority = compute_priority end + # Compares this node against *other*, returning `-1`, `0` or `1` depending + # on whether this node differentiates from *other*. + # + # Comparison is done combining node's `kind` and `priority`. Nodes of + # same kind are compared by priority. Nodes of different kind are + # ranked. + # + # ### Normal nodes + # + # ``` + # node1 = Radix::Node(Nil).new("a") # normal + # node2 = Radix::Node(Nil).new("bc") # normal + # node1 <=> node2 # => 1 + # ``` + # + # ### Normal vs named or glob nodes + # + # ``` + # node1 = Radix::Node(Nil).new("a") # normal + # node2 = Radix::Node(Nil).new(":query") # named + # node3 = Radix::Node(Nil).new("*filepath") # glob + # node1 <=> node2 # => -1 + # node1 <=> node3 # => -1 + # ``` + # + # ### Named vs glob nodes + # + # ``` + # node1 = Radix::Node(Nil).new(":query") # named + # node2 = Radix::Node(Nil).new("*filepath") # glob + # node1 <=> node2 # => -1 + # ``` + def <=>(other : self) + result = kind <=> other.kind + return result if result != 0 + + other.priority <=> priority + end + + # Returns `true` if the node key contains a glob parameter in it + # (catch all) + # + # ``` + # node = Radix::Node(Nil).new("*filepath") + # node.glob? # => true + # + # node = Radix::Node(Nil).new("abc") + # node.glob? # => false + # ``` + def glob? + kind.glob? + end + # Changes current *key* # # ``` @@ -91,7 +135,7 @@ module Radix # # => "b" # ``` # - # This will also result in a new priority for the node. + # This will also result in change of node's `priority` # # ``` # node = Radix::Node(Nil).new("a") @@ -103,9 +147,38 @@ module Radix # # => 6 # ``` def key=(@key) + # reset kind on change of key + @kind = Kind::Normal @priority = compute_priority end + # Returns `true` if the node key contains a named parameter in it + # + # ``` + # node = Radix::Node(Nil).new(":query") + # node.named? # => true + # + # node = Radix::Node(Nil).new("abc") + # node.named? # => false + # ``` + def named? + kind.named? + end + + # Returns `true` if the node key does not contain an special parameter + # (named or glob) + # + # ``` + # node = Radix::Node(Nil).new("a") + # node.normal? # => true + # + # node = Radix::Node(Nil).new(":query") + # node.normal? # => false + # ``` + def normal? + kind.normal? + end + # :nodoc: private def compute_priority reader = Char::Reader.new(@key) @@ -113,9 +186,11 @@ module Radix while reader.has_next? case reader.current_char when '*' - return -2 + @kind = Kind::Glob + break when ':' - return -1 + @kind = Kind::Named + break else reader.next_char end @@ -124,23 +199,9 @@ module Radix reader.pos end - # Changes the order of Node's children based on each node priority. - # - # This ensures highest priority nodes are listed before others. - # - # ``` - # root = Radix::Node(Nil).new("/") - # 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, -2] - # ``` - def sort! - @children.sort_by! { |node| -node.priority } + # :nodoc: + protected def sort! + @children.sort! end end end