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

Add filters #4

Open
h2non opened this issue Mar 28, 2016 · 7 comments
Open

Add filters #4

h2non opened this issue Mar 28, 2016 · 7 comments

Comments

@h2non
Copy link
Member

h2non commented Mar 28, 2016

To determine if a give req should be intercepted or not.

@groyoh
Copy link
Member

groyoh commented Mar 28, 2016

I could probably work on this as my first feature.

@h2non
Copy link
Member Author

h2non commented Mar 28, 2016

Great. Filters should be added here:
https://github.com/vinci-proxy/intercept/blob/master/request.go#L174

Also, the response interceptor should be refactored to return a compatible interface method instead of a function to achieve this. See the request implementation as reference.

@h2non
Copy link
Member Author

h2non commented Apr 2, 2016

@groyoh do you need some help with this?

@groyoh
Copy link
Member

groyoh commented Apr 2, 2016

I was thinking of something like:

+var DefaultFilter = func(req *http.Request) bool {
+       if req.Method == "HEAD" || req.Method == "OPTIONS" {
+               return false
+       }
+       return true
+}
+

 // RequestInterceptor interceps a given http.Request using a custom request modifier function.
 type RequestInterceptor struct {
        Modifier ReqModifierFunc
+       Filters  []Filter
 }

 // Request intercepts an HTTP request and passes it to the given request modifier function.
 func Request(h ReqModifierFunc) *RequestInterceptor {
-       return &RequestInterceptor{Modifier: h}
+       filters := []Filter{DefaultFilter}
+       return &RequestInterceptor{Modifier: h, Filters: filters}
+}
+
+// Filter intercepts an HTTP requests if and only if the given filter returns true.
+func (s *RequestInterceptor) Filter(f Filter) {
+       s.Filters = append(s.Filters, f)
 }

 // HandleHTTP handles the middleware call chain, intercepting the request data if possible.
 // This methods implements the middleware layer compatible interface.
 func (s *RequestInterceptor) HandleHTTP(w http.ResponseWriter, r *http.Request, h http.Handler) {
-       if r.Method == "HEAD" || r.Method == "OPTIONS" {
-               h.ServeHTTP(w, r)
-               return
+       if s.filter(r) {
+               req := NewRequestModifier(r)
+               s.Modifier(req)
        }
+       h.ServeHTTP(w, r)
+}

-       req := NewRequestModifier(r)
-       s.Modifier(req)
-       h.ServeHTTP(w, req.Request)
+func (s RequestInterceptor) filter(req *http.Request) bool {
+       for _, filter := range s.Filters {
+               if !filter(req) {
+                       return false
+               }
+       }
+       return true
 }

Let me know if I'm on the right track here. I will create a PR once I have the test passing.

@h2non
Copy link
Member Author

h2non commented Apr 2, 2016

Looking good. Just a few tips:

Filter() could allow variadic arguments (in some cases useful), like this:

func (s *RequestInterceptor) Filter(f ...Filter) {
       s.Filters = append(s.Filters, f...)
}

The HandleHTTP for Request is fine, but for Response, be sure you're only hijacking the ResponseWriter if all the filters passes.

@groyoh
Copy link
Member

groyoh commented Apr 2, 2016

Alright, thanks. Another question, is it common in Go to have method chaining e.g. interceptor.Filter(someFilter).ANewMethod(someDifferentFunction)?

@h2non
Copy link
Member Author

h2non commented Apr 2, 2016

It's not too much common, but I personally tend to implement fluent interfaces for particular cases when the API consumer might need it for convenience. The Go stdlib doesn't support method chaining, but they're not providing that kind of APIs.

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

No branches or pull requests

2 participants