Use middleware in a group of handlers? about goji HOT 13 CLOSED

Hi - see this recent issue: #64

Specifically, this line in the example: https://github.com/zenazn/goji/blob/master/example/main.go#L48

admin := web.New()
    goji.Handle("/admin/*", admin)
    admin.Use(SuperSecure)

    // Goji's routing, like Sinatra's, is exact: no effort is made to
    // normalize trailing slashes.
    goji.Get("/admin", http.RedirectHandler("/admin/", 301))

    // Set up admin routes. Note that sub-routes do *not* mutate the path in
    // any way, so we need to supply full ("/admin/" prefixed) paths.
    admin.Get("/admin/", AdminRoot)
    admin.Get("/admin/finances", AdminFinances)

Use sub-routers to apply middleware to certain route groups. If you want to apply middleware to a single route only you can just wrap it - i.e. goji.Get("/something", SomeMiddleware(YourHandler)).

from goji.

The issue I have with that example is that the sub-router has to specify the parent path /admin/ again.. I think it would be better if you could do..

admin := web.New()
    goji.Handle("/admin/*", admin)
    admin.Use(SuperSecure)

    // Goji's routing, like Sinatra's, is exact: no effort is made to
    // normalize trailing slashes.
    goji.Get("/admin", http.RedirectHandler("/admin/", 301))

    // Set up admin routes. Note that sub-routes do *not* mutate the path in
    // any way, so we need to supply full ("/admin/" prefixed) paths.
    admin.Get("/", AdminRoot)
    admin.Get("/finances", AdminFinances)

which would be equivalent and the default..

from goji.

@pkieltyka — would http://golang.org/pkg/net/http/#StripPrefix be helpful?

It's also long been on my list of things to treat * specially. Hopefully I'll have some time to work on that soon, which should make sub-routes less painful.

from goji.

@zenazn hey, ah, perhaps but its just extra clutter.

The issue is just how sub-routes are defined and stacked ontop of parents. Currently /admin has to be specified in the sub-routes, yet the parent that mounts the sub-component specifies /admin/*. The goji mux should just notice the *, and when its Handle()ing another mux, it should join those routes so the parent specifies /admin/* and the sub-mux dont need the /admin stuff and will be just / and /finances.. the end-result is the api has routes: /admin, /admin/ and /admin/finances you will probably want to check for "//" and just change it to "/" whenever the routes are joined

from goji.

Yeah, it's a little more annoying to do sub-routes in Goji than other frameworks, but I think the fact that everything is explicit (and non-magical) is a point in Goji's favor. For instance, in your proposal, if I have two muxes a and b and write a.Handle("/admin", b) it does one thing, but if I have a.Handle("/admin", SomeMiddleware(b)) suddenly everything breaks. Or as another example, how would you deal with regular expression patterns or even custom patterns?

I think if nothing else, it's extremely obvious what Goji will do with any given routing invocation. Yes, there are other APIs that have less "extra clutter," but with magic comes corner cases and composability constraints, and I'm not especially interested in that.

from goji.

I really like that Goji stays away from the magic, which is why I refactored all of our services from martini. I hear what you're saying, but I think as long as the routes and rules blend well together, then its okay IMO, it would take thought to the different edge cases to make sure its not adding magic or breaking interoperability with the stdlib.

When I initially tried to do this, intuitively I thought the sub-routes would join with the parent, just like any normal folder structure. I spent probably 2 hours until I realized its actually not a parent at all, and may as well be mounted on "/".

For a.Handle("/admin", SomeMiddleware(b)) there are a few options, the answer is in the details to make sure its clean. Say b was a mux with "/" and "/finances" routes+handlers, you could make a.Handle() create routes as "/admin/" "/admin/finances", and the middleware would apply to those handlers. You could also only join the parent-child routes if a * is specified in the parent route like "/admin/*", otherwise the behavior stays as it is now. If the b mux specifies some exp like "/hi/:x", well then its the same thing.. a.Handle("/admin/*", M(b)) will just create a route called "/admin/hi/:x" to b's handler for "/hi/:x". Thats the way I see it anyways.. whatever you decide is all good with me, just my 2 cents.

from goji.

Alright, I just changed the wildcard behavior to make doing stuff like this a teensy bit easier: 2fe5c3e

I also plan on writing a SubRouter middleware that makes use of the new URLParam key to Do The Right Thing for the standard case.

from goji.

nice! 👍

from goji.

hey @zenazn I just tried this in my app and it didn't seem to work. I really like how you introduced this behavior as middleware.SubRouter btw.

In my case the base mux looks like:

m := web.New()
m.Get("/", someHandler)
m.Handle("/:catchAll*", NewCatchAllHandler())

the NewCatchAllHandler makes its own mux with web.New(), and does:

m.Use(middleware.SubRouter)
m.Get("/", rootHandler)
m.Get("/more", moreHandler)

.. is that supposed work? as well, having the URLParams["catchAll"] in the sub handler

from goji.

The syntax is m.Handle("/*", NewCatchAllHandler()), which will bind c.URLParams["*"] (that's the key that middleware.SubRouter looks for)

from goji.

makes sense! thanks

On Tuesday, November 4, 2014 at 5:54 PM, Carl Jackson wrote:

The syntax is m.Handle("/", NewCatchAllHandler()), which will bind c.URLParams[""](that's the key that middleware.SubRouter looks for)

—
Reply to this email directly or view it on GitHub (#65 (comment)).

from goji.

@zenazn how about this edge case..

the primary mux:

m := web.New()
m.Get("/", someHandler)
m.Handle("/creds*", NewCredsHandler())

the NewCredsHandler makes its own mux with web.New(), and does:

m.Use(middleware.SubRouter)
m.Get("/", rootHandler)
m.Get("/:id", resourceHandler)

.. this works fine, except you must specify the ending "/" like: /creds/ even if you expect /creds to work. I tried to use m.Get("", rootHandler) but this doesn't work.. any suggestions?

from goji.

what would be nice.. as those route strings are being built, the builder would see /creds* and the route "/" would allow for /creds/ and /creds

from goji.