agrafix / Spock

Another Haskell web framework for rapid development
https://www.spock.li
679 stars 56 forks source link

Order of route handlers returned by the routing function from Reroute.runRegistry #168

Closed moll closed 3 years ago

moll commented 3 years ago

Hey!

Thanks for Spock and Reroute!

I'm using Reroute outside of Spock and I'm a little confused as to the order of the route handlers returned by the matching from Reroute.runRegistry (i.e the second element of the three-element tuple). Mind clarifying, what order are the handlers returned if there are multiple matches for a given route? That is, if one defines multiple handlers to the same path with Reroute.hookRoute. From the source code I believe the wildcard handlers are returned last, but if I'm not mistaken, multiple definitions to the same route are returned in reverse — the ones hooked later are first in the list.

Thanks in advance!

agrafix commented 3 years ago

Glad to hear they are useful for you!

I'll need to dig around in the code a bit, my recommendation would be to add a test here: https://github.com/agrafix/Spock/blob/b215cc3cfe1e26d815308e9dff24c24a4491e5b1/reroute/test/Web/Routing/SafeRoutingSpec.hs#L102

That way we can document the current behavior :)

agrafix commented 3 years ago

It looks like they are returned in order, see should handle multiple possible matches correctly tests.

moll commented 3 years ago

Hey! Thanks, though in what order? My quick skimming of should handle multiple possible matches correctly at https://github.com/agrafix/Spock/blob/1639051b1ceedfba8f3667c12c7344700d0859e4/reroute/test/Web/Routing/SafeRoutingSpec.hs#L78-L82 tells me that the tested values are actually sorted before tested, making the test a little pointless. :)

    checkRoute' :: T.Text -> Bool -> [ReturnVar] -> Expectation
    checkRoute' r b x =
      let matches = handleFun b (pieces r)
       in sort (map runIdentity matches) `shouldBe` sort x

    checkRoute :: T.Text -> [ReturnVar] -> Expectation
    checkRoute = flip checkRoute' True
moll commented 2 years ago

Hey again! I think this issue should be re-opened based on what I spotted in https://github.com/agrafix/Spock/issues/168#issuecomment-962590613. ;)