Different request in wrapHandler than in handler #144

Closed
basvandijk opened this Issue Jun 22, 2012 · 8 comments

Comments

Projects
None yet
4 participants
Contributor

basvandijk commented Jun 22, 2012

I expected the following program to print:

wrapHandlers("/foo/bar/","")
handler("/foo/bar/","")

However it prints:

wrapHandlers("/","foo/bar")
handler("/foo/bar/","")

when called with curl localhost:8000/foo/bar.

{-# LANGUAGE OverloadedStrings #-}

import Snap

main :: IO ()
main = serveSnaplet defaultConfig $
         makeSnaplet "test" "Test" Nothing $ do
           addRoutes [("/foo/bar", printPaths "handler")]
           wrapHandlers $ (printPaths "wrapHandlers" >>)

printPaths str = withRequest $ \req ->
                   liftIO $ putStrLn $ str ++
                     show (rqContextPath req, rqPathInfo req)

My motivation is that I have a bunch of routes that I want to wrap with a check that if rqPathInfo is not empty the route passes. However, as shown above, this is not possible since rqPathInfo equals the route path itself within wrapHandlers.

The workaround of course is the manually wrap each route by mapping over the routes. However, I thought this was the job of wrapHandlers.

Owner

mightybyte commented Jun 22, 2012

No, that was not the intent of wrapHandlers. addRoutes passes all of its handlers to the route function, generating one big route [...site routes...] handler for the site. wrapHandlers is meant as a mechanism to perform transformations on your top-level site handler, not individual route handlers. rqContextPath is set inside of the route function, so wrapHandlers doesn't have any knowledge of the context path that you want.

Now that I think about it, I guess the name wrapHandlers being plural might be slightly misleading. But at the same time, the singular "wrapHandler" doesn't really communicate the all-encompassing nature that we want either.

Owner

gregorycollins commented Jun 22, 2012

Let's deprecate "wrapHandlers" and pick a better name --- agree that this is confusing. Bas, do you have any suggestions re: what a better name for this would be?

Contributor

basvandijk commented Jun 22, 2012

wrapTopLevelHandler maybe?

Owner

mightybyte commented Jun 22, 2012

wrapSiteHandler ?

Member

ozataman commented Jun 22, 2012

How about wrapSite or wrapRoutes or wrapAll or wrapEverything ?

Owner

mightybyte commented Jun 22, 2012

I kind of like wrapSite actually. Having "Handler" in the name might not be necessary since it's in the type signature. I think wrapRoutes and wrapAll likely have the same problem as wrapHandlers.

Contributor

basvandijk commented Jun 22, 2012

+1 for wrapSite

Owner

mightybyte commented Jun 22, 2012

Done. I went with wrapSite.

mightybyte closed this Jun 22, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment