rgrinberg / opium

Sinatra like web toolkit for OCaml
MIT License
755 stars 67 forks source link

Documentation on how routes are prioritized #45

Open hcarty opened 8 years ago

hcarty commented 8 years ago

Given a modified version of the README example:

open Opium.Std

let print_param = get "/hello/:name" begin fun req ->
    `String ("Hello " ^ param req "name") |> respond'
  end

let print_other = get "**" begin fun req ->
    `String "Hello everyone else" |> respond'
  end

let () =
  App.empty
  (* Order matters! *)
  |> print_other
  |> print_param
  (* Order matters! *)
  |> App.run_command

With this code I see "Hello test" if I GET /hello/test and "Hello everyone else" if I use other paths. However, if I swap the lines between the comments above then the only response "Hello everyone else". OCaml 4.02.3 + opium 0.13.3 + cohttp 0.19.3 from opam.

I'm not sure what the right approach is here. It's useful to be able to have specific routes with an catch-all to handle all other requests. It would be useful if the right way to do this were shown in the README or somewhere similarly visible.

rgrinberg commented 8 years ago

OK, I see the problem. I think that the ** route should always match last in such a case but as you've said it might be useful to have an explicit catch all route.

hcarty commented 8 years ago

Agreed, ** matching last is what I was hoping for. In general it would be nice if the most precise route were picked. For example the route /hello/test would be selected before /hello/:name which would be selected before **.

cullophid commented 8 years ago

Could you not just change the order of the routes? This is the same behaviour you see in sinatra and express.js and generally its quite nice that you have to be specific about what order the routes get evaluated in

jochasinga commented 4 years ago

Could you not just change the order of the routes? This is the same behaviour you see in sinatra and express.js and generally its quite nice that you have to be specific about what order the routes get evaluated in

To add to @cullophid I agree it's nice about this explicit ordering plus the |> operator already does a great job of being semantic. An emphasized doc on this is all is needed. I think adding another way other than using |> for implicit route matching might be appropriate if needed.

rgrinberg commented 3 years ago

Will be fixed by #215 . We're not going to allow the default router to have ambiguous routing.