router icon indicating copy to clipboard operation
router copied to clipboard

Routes with multiple variables not working in common nested route scenario

Open Drowze opened this issue 2 years ago • 2 comments

Hello Hanami team 👋

First, thanks for the amazing project. It's a breeze to have projects like Hanami, dry-rb and ROM in the Ruby community 😄

That being said, I've been using Hanami 2 lately I've came across the following issue with hanami-router:

# Here I would expect the posts route to get matched

router = Hanami::Router.new do
  get "/users/:id", to: ->_ {[200, {}, ['show user']]}
  get "/users/:user_id/posts", to: ->_ {[200, {}, ['index posts']]}
end
router.call(Rack::MockRequest.env_for("/users/1/posts"))
# => [404, {"Content-Length"=>"9"}, ["Not Found"]]

Note, however, that this work fine:

router = Hanami::Router.new do
  get "/users/:user_id", to: ->_ {[200, {}, ['show user']]}
  get "/users/:user_id/posts", to: ->_ {[200, {}, ['index posts']]}
end
router.call(Rack::MockRequest.env_for("/users/1/posts"))
# => [200, {}, ["index posts"]]

I would expect the response to be the same in both examples, regardless of the path variable name. Also worth noting, I think naming the last "id" in the route generically as id (as opposed to always specifically calling it with the name of the resource, e.g. "user_id" or "post_id") is a common pattern that is also adopted in Rails by default.

Version used: 2.0.2

Drowze avatar Mar 02 '23 11:03 Drowze

I'm experiencing the same issue with the following routes:

router = Hanami::Router.new do
  get "/purchase_orders/:id", to: ->_ {[200, {}, ['show purchase order']]}
  get "/purchase_orders/:purchase_order_id/items", to: ->_ {[200, {}, ['show purchase order line items']]}
end

router.call(Rack::MockRequest.env_for("/purchase_orders/1"))
# => [200, {}, ["show purchase order"]]

router.call(Rack::MockRequest.env_for("/purchase_orders/1/items"))
# => [404, {"Content-Length"=>"9"}, ["Not Found"]]

It seems only the first matching pattern is being considered. For example, if I reverse the routing order, only the first pattern will be considered, and the second will not be found:

router = Hanami::Router.new do
  get "/purchase_orders/:purchase_order_id/items", to: ->_ {[200, {}, ['show purchase order line items']]}
  get "/purchase_orders/:id", to: ->_ {[200, {}, ['show purchase order']]}
end

router.call(Rack::MockRequest.env_for("/purchase_orders/1"))
# => [404, {"Content-Length"=>"9"}, ["Not Found"]]

router.call(Rack::MockRequest.env_for("/purchase_orders/1/items"))
# => [200, {}, ["show purchase order line items"]]

eriklott avatar Mar 09 '23 17:03 eriklott

I'm having the exact same issue and I believe it's down to Hanami::Router::Trie#find only evaluating the first possible path that it can go down, and when it doesn't match returning nil and not trying the next possible path. I'm seeing if I can work out a failing spec and also whether I can fix it. Time will tell.

parndt avatar Sep 12 '23 23:09 parndt