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

Improve compatibility with negroni and other http-based middleware. #6

Merged
merged 1 commit into from
Sep 4, 2014
Merged

Conversation

albrow
Copy link
Contributor

@albrow albrow commented Sep 1, 2014

Throttle looks like a really handy tool, and I'm interested in adapting it to work with negroni. This small change makes it easier to write a negroni-compatible middleware wrapper around the throttle handler.

For example, I can now write negroni-compatible middleware (which uses throttle behind the scenes) in just a few lines:

func Policy(quota *throttle.Quota, options ...*throttle.Options) negroni.HandlerFunc {
    return func(res http.ResponseWriter, req *http.Request, next http.HandlerFunc) {
        handler := throttle.Policy(quota, options...)
        handler(res, req)
        if res.Header().Get("X-Ratelimit-Remaining") == "0" {
            // the policy was triggered and the request should fail
            // in this case, we should not call next
        } else {
            // the policy was not triggered, and the request should continue
            // on to the next handler
            next(res, req)
        }
    }
}

I expect that this will also make throttle more compatible for those who prefer to roll their own middleware micro-frameworks. The tests still pass, and Policy still returns a handler which is directly compatible with martini, so as far as I know this change comes with no downsides.

@beatrichartz beatrichartz merged commit c262e5c into martini-contrib:master Sep 4, 2014
@beatrichartz
Copy link
Member

Nice work!

I don't see any downsides either, merging this.

@albrow albrow deleted the compatibility branch September 7, 2014 21:59
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

2 participants