Makes node prioritization insertion-independent
The order in which nodes were inserted into a tree might produce failures in lookup mechanism. tree = Radix::Tree(Symbol).new tree.add "/one/:id", :one tree.add "/one-longer/:id", :two result = tree.find "/one-longer/10" # expected `true` result.found? # => false In above example, reversing the order of insertion solved the issue, but exposed the naive sorting/prioritization mechanism used. This change improves that by properly identifying the kind of node being evaluated and compared against others of the same kind. It is now possible to know in advance if a node contains an special condition (named parameter or globbing) or is a normal one: node = Radix::Node(Nil).new("a") node.normal? # => true node = Radix::Node(Nil).new(":query") node.named? # => true node = Radix::Node(Nil).new("*filepath") node.glob? # => true Which helps out with prioritization of nodes: - A normal node ranks higher than a named one - A named node ranks higher than a glob one - On two of same kind, ranks are based on priority With this change in place, above example works as expected: tree = Radix::Tree(Symbol).new tree.add "/one/:id", :one tree.add "/one-longer/:id", :two result = tree.find "/one-longer/10" result.found? # => true Fixes #18
This commit is contained in:
parent
0dc6e174d2
commit
1764332123
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue