Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routes with multiple variables not working in common nested route scenario #255

Open
Drowze opened this issue Mar 2, 2023 · 2 comments
Open

Comments

@Drowze
Copy link

Drowze commented Mar 2, 2023

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 Drowze changed the title Routes with multiple variables not working in common scenario Routes with multiple variables not working in common nested route scenario Mar 2, 2023
@eriklott
Copy link

eriklott commented Mar 9, 2023

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"]]

@parndt
Copy link

parndt commented Sep 12, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants