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

Middleware (handler wrapping) #36

Closed
wants to merge 1 commit into from
Closed

Conversation

scttnlsn
Copy link

@scttnlsn scttnlsn commented Oct 7, 2013

This adds support for wrapping handler functions with additional functionality. Example:

middleware := func(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
        fmt.Println("before")
        next.ServeHTTP(w, req)
        fmt.Println("after")
    })
}

r := NewRouter().Use(middleware)

I find this pattern very useful for implementing auth, logging, etc. Is this something you'd be interested in merging into mux?

@kisielk
Copy link
Contributor

kisielk commented Oct 7, 2013

It's already possible to do this two ways:

router := mux.NewRouter()
router.Handle("/foo", handler)
http.Handle("/", middleware(router))

or

router := mux.NewRouter()
router.Handle("/foo", middleware(handler))
http.Handle("/", router)

apart from saving a bit of typing in the second case, are there any other benefits from this approach?

@scttnlsn
Copy link
Author

scttnlsn commented Oct 7, 2013

@kisielk I use it frequently with subrouters.

@kisielk
Copy link
Contributor

kisielk commented Oct 7, 2013

I guess only the second approach I outlined above works in the case of subrouters, but that still works doesn't it?

@scttnlsn
Copy link
Author

scttnlsn commented Oct 7, 2013

Yeah, the second approach works with subrouters but you have to apply the middleware to each handler:

sub := router.PathPrefix("/sub").Subrouter()
sub.Handle("/foo", middleware2(middleware1(fooHandler)))
sub.Handle("/bar", middleware2(middleware1(barHandler)))

Instead of:

sub := router.PathPrefix("/sub").Subrouter().Use(middleware1, middleware2)
sub.Handle("/foo", fooHandler)
sub.Handle("/bar", barHandler)

@kisielk
Copy link
Contributor

kisielk commented Oct 7, 2013

You can do it without growing the mux API:

func Use(handler http.Handler, middleware ...func(http.Handler) http.Handler) http.Handler {
    for _, m := range middleware {
        handler = m(handler)
    }
    return handler
}

middleware := []func(http.Handler) http.Handler{middleware1, middleware2}
sub := router.PathPrefix("/sub").SubRouter()
sub.Handle("/foo", Use(fooHandler, middleware...))
sub.Handle("/bar", Use(barHandler, middleware...))

Sorry for being a stickler, I would just prefer to avoid adding unnecessary API functions to mux and growing the internal complexity.

@scttnlsn
Copy link
Author

scttnlsn commented Oct 7, 2013

Yeah, fair enough. Thanks for talking through it.

@marpaia
Copy link

marpaia commented Oct 9, 2013

FWIW, although I understand the desire to not want to add unnecessary API functionality for complexity reasons, I feel like it would be much cleaner and idiomatic to be able to define a subrouter and immediately define all of it's middlewares once, in a single place via a clean syntax.

Complexity issues are often a real concern, but this is a 56 loc change WITH tests; I don't think that anyone's ability to understand the codebase will ever be affected by a change of this size.

As a mux user, I would really like to see this merged.

@sinni800
Copy link

sinni800 commented Oct 9, 2013

I think this is great because it gives you an easy way to push multiple middleware functions onto a stack.

Though I think this might be workable with one less layer of nesting if the thing it accepts wasn't a http.Handler, but rather something else (I am thinking HandlerFunc with the "next" parameter right in it). It's not a regular handler anyway because it needs to call the "next" middleware. Unless I am overseeing something, of course.

@kisielk
Copy link
Contributor

kisielk commented Oct 9, 2013

I think that illustrates another problem, everyone is going to want to have some different way to want to stack their handlers. Instead of committing to one, I think we should leave it open. mux is not a framework, it's merely an HTTP router. If you need handler stacking or other functionality like that, that's to be implemented elsewhere.

I discussed the idea with some of the other maintainers and I think we are in agreement.

Apart from that, the implementation of this suffers from a few problems in that it rebuilds the function call stack on every single call to ServeHTTP, the other approach I posted here avoids that.

@kisielk kisielk closed this Oct 9, 2013
@kisielk kisielk mentioned this pull request Nov 13, 2013
@kevinburke
Copy link

@kisielk would be great to add docs around this middleware based approach

@mrhwick
Copy link

mrhwick commented Apr 27, 2017

For anyone who is still frustrated by not having a clean way to apply a set of middleware to all Routes in a Subrouter, here's a solution that allows you to specify everything without repeating yourself.

It's a bit complicated, but you can walk the Route tree starting at a certain Subrouter and apply your middleware to all of the Routes below that Subrouter.

func Use(handler http.Handler, middleware ...func(http.Handler) http.Handler) http.Handler {
    for _, m := range middleware {
        handler = m(handler)
    }
    return handler
}

middleware := []func(http.Handler) http.Handler{middleware1, middleware2}

sub := router.PathPrefix("/sub").Subrouter()
sub.Handle("/foo", fooHandler)
sub.Handle("/bar", barHandler)

sub.Walk(func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error {
	route.Handler(Use(route.GetHandler(), middleware...))
	return nil
})

@0xdevalias
Copy link

In case google or something else leads you here like it did for me.. it seems there is a Use function now

https://github.com/gorilla/mux/blob/master/middleware.go#L23-L28 :

// Use appends a MiddlewareFunc to the chain. Middleware can be used to intercept or otherwise modify requests and/or responses, and are executed in the order that they are applied to the Router.
func (r *Router) Use(mwf ...MiddlewareFunc) {
	for _, fn := range mwf {
		r.middlewares = append(r.middlewares, fn)
	}
}

It appears to exist for both routers, and subrouters.

@mrhwick
Copy link

mrhwick commented May 23, 2018

Thanks for highlighting that new function.

Here's the godoc in case the source tree changes in the future: https://godoc.org/github.com/gorilla/mux#Router.Use

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

Successfully merging this pull request may close these issues.

None yet

7 participants