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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture names affects finding nested routes #252

Closed
twe4ked opened this issue Feb 9, 2023 · 2 comments
Closed

Capture names affects finding nested routes #252

twe4ked opened this issue Feb 9, 2023 · 2 comments
Assignees

Comments

@twe4ked
Copy link

twe4ked commented Feb 9, 2023

Hey!

I've run into some unexpected (to me) behaviour when finding nested routes with different capture names.

Here's an example that shows the behaviour:

  • Define a GET route at /foo/:id
  • Define a GET route at /foo/:foo_id/bar
  • /foo/:foo_id/bar 404s
  • Define a GET route at /foo/:foo_id (uncomment the line)
  • /foo/:foo_id/bar now returns 200
require "hanami/router"

# gem "hanami-router", "2.0.2"

router = Hanami::Router.new do
  # get "/foo/:foo_id", to: ->(env) { [200, {}, ["/foo/:foo_id #{ env["router.params"] }"]] }
  get "/foo/:id", to: ->(env) { [200, {}, ["/foo/:id #{ env["router.params"] }"]] }
  get "/foo/:foo_id/bar", to: ->(env) { [200, {}, ["/foo/:foo_id/bar #{ env["router.params"] }"]] }
end

req = ->(path) {
  puts "Requesting #{path}"
  response = router.call(
    "REQUEST_METHOD" => "GET",
    "PATH_INFO" => path,
    "SCRIPT_NAME" => "something",
  )
  puts "\tStatus #{response[0]}"
}

req.("/foo/123") # => 200
req.("/foo/123/bar") # => 400

Is this behaviour expected?

Extra context: We're using openapi_first which automatically generates a hanami-router based off the OpenAPI definition. Internally it converts the OpenAPI path names like this: /foo/{bar}/baz -> /foo/:bar/baz and uses those to generate the routes. For now we're going to ensure all our OpenAPI paths use common capture names. We could also look at patching openapi_first to ensure the names match across nested routes.

Thanks :)

@jodosha jodosha self-assigned this Feb 9, 2023
@jodosha
Copy link
Member

jodosha commented Feb 9, 2023

@twe4ked That is because get "/foo/:id" is shadowing get "/foo/:foo_id/bar".

I suggest two alternative approaches:

  1. Reintroduce the commented route get "/foo/:foo_id".
  2. Change the order to have get "/foo/:foo_id/bar" first

@jodosha jodosha closed this as completed Feb 9, 2023
@twe4ked
Copy link
Author

twe4ked commented Feb 9, 2023

Thanks for the help, Luca. 🙏

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

2 participants