Mismatched route
crisward opened this issue · 9 comments
I'm using kemal, and I've discovered a bug which seems to come from radix.
The below should output three, but is not found. However if I change contact to anything which doesn't being with a c
or a o
it works as expected. Any ideas?
require "radix"
tree = Radix::Tree(Symbol).new
tree.add "/get/", :one
tree.add "/head/", :two
tree.add "/get/*", :three
tree.add "/head/*", :four
tree.add "/get/open", :five
tree.add "/head/open", :six
tree.add "/get/open/*", :seven
tree.add "/head/open/*", :eight
tree.add "/get/close", :nine
tree.add "/head/close", :ten
tree.add "/get/close/*", :eleven
tree.add "/head/close/*", :twelve
result = tree.find "/get/hello"
if result.found?
puts result.payload
else
puts "not found"
end
This works for me using Radix 0.3.4
icr(0.19.4) > result.found?
=> true
icr(0.19.4) > result.payload
=> :three
icr(0.19.4) > tree.root.key
=> "/"
icr(0.19.4) > tree.root.children.size
=> 2
icr(0.19.4) > tree.root.children[0].key
=> "head/"
icr(0.19.4) > tree.root.children[1].key
=> "get/"
icr(0.19.4) > tree.root.children[1].children.size
=> 3
icr(0.19.4) > tree.root.children[1].children[0].key
=> "close"
icr(0.19.4) > tree.root.children[1].children[1].key
=> "open"
icr(0.19.4) > tree.root.children[1].children[2].key
=> "*"
Make sure you pull from Kemal on master in your shards.yml. It has the latest radix in it.
Hello @crisward, thank you for your report.
I was unable to reproduce this with latest version 0.3.4
, released a few days ago:
p Radix::VERSION # => 0.3.4
result = tree.find "/get/hello"
if result.found?
puts result.payload # => three
puts result.params # => {"" => "hello"}
else
puts "not found"
end
Please check if possible on your end to update your Kemal dependency so latest Radix is available.
Cheers.
Thanks for you help on this.
I just read through my explanation, possibly wasn't very good.
I'm using sdogruyol/kemal master - radix 0.3.4. The below test I ran on radix 0.3.4 - Crystal 0.19.4 (2016-10-21). Mac Sierra, 10.12.1. I also tried deleting radix out of my deps, the doing another crystal deps update
to make sure I had the correct version. I also double checked the source code in the lib folder incase this is an issue with crystal shards.
# radix-test.cr
require "radix"
tree = Radix::Tree(Symbol).new
tree.add "/get/", :one
tree.add "/head/", :two
tree.add "/get/*", :three
tree.add "/head/*", :four
tree.add "/get/open", :five
tree.add "/head/open", :six
tree.add "/get/open/*", :seven
tree.add "/head/open/*", :eight
tree.add "/get/close", :nine
tree.add "/head/close", :ten
tree.add "/get/close/*", :eleven
tree.add "/head/close/*", :twelve
result = tree.find "/get/ola" # this doesn't work, I get not found
if result.found?
puts result.payload
else
puts "not found"
end
result = tree.find "/get/hello" # this works fine, I get 'three'
if result.found?
puts result.payload
else
puts "not found"
end
Then I ran, and got the below output
$ crystal radix-test.cr
not found
three
If you still can't recreate this, let me know and I'll fork and add a failing test, I could even try to get it to pass if I'm feeling brave...
Ah, that makes more sense @crisward. Sorry didn't understood your problem from the initial description 😄
Will take a look later today and get back to you.
Thank you for your patience.
It's ok, I wrote it last night at 11:30pm, been up since 6am, so my descriptive powers had deserted me. Thanks again for your help.
No worries.
Got a spec that reproduces the issue:
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")
end
Need a bit more of investigation since this might impact other lookup paths.
Cheers.
Hello @crisward, thank you for your report and your patience.
I've released a new version that corrects this issue.
Cheers!
Cool, thanks for you work on this. I'll check if kemal has updated and perhaps nudge / do a pull request for the latest version.
He's already done it... that was quick!