From d33256f1dcba600905bafea151b5efc8d853f6cc Mon Sep 17 00:00:00 2001
From: Matthew Holt <Matthew.Holt+git@gmail.com>
Date: Thu, 2 Apr 2015 23:30:54 -0600
Subject: [PATCH] Refactor: Middleware chain uses Handler instead of
 HandlerFunc

---
 middleware/browse/browse.go         | 12 +++++-----
 middleware/errors/errors.go         |  8 +++----
 middleware/extension/ext.go         |  8 +++----
 middleware/fastcgi/fastcgi.go       |  8 +++----
 middleware/gzip/gzip.go             | 13 +++++-----
 middleware/headers/headers.go       |  4 ++--
 middleware/headers/new.go           |  4 ++--
 middleware/log/log.go               | 10 ++++----
 middleware/markdown/markdown.go     |  8 +++----
 middleware/middleware.go            | 37 ++++++++++++++++++-----------
 middleware/proxy/proxy.go           |  8 +++----
 middleware/redirect/redirect.go     |  8 +++----
 middleware/rewrite/rewrite.go       |  8 +++----
 middleware/websockets/websockets.go |  8 +++----
 server/server.go                    |  7 +++---
 15 files changed, 80 insertions(+), 71 deletions(-)

diff --git a/middleware/browse/browse.go b/middleware/browse/browse.go
index b5a9dd337..eb9413325 100644
--- a/middleware/browse/browse.go
+++ b/middleware/browse/browse.go
@@ -20,7 +20,7 @@ import (
 // Browse is an http.Handler that can show a file listing when
 // directories in the given paths are specified.
 type Browse struct {
-	Next    middleware.HandlerFunc
+	Next    middleware.Handler
 	Root    string
 	Configs []BrowseConfig
 }
@@ -83,9 +83,9 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		Configs: configs,
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
+	return func(next middleware.Handler) middleware.Handler {
 		browse.Next = next
-		return browse.ServeHTTP
+		return browse
 	}, nil
 }
 
@@ -95,11 +95,11 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 
 	info, err := os.Stat(filename)
 	if err != nil {
-		return b.Next(w, r)
+		return b.Next.ServeHTTP(w, r)
 	}
 
 	if !info.IsDir() {
-		return b.Next(w, r)
+		return b.Next.ServeHTTP(w, r)
 	}
 
 	// See if there's a browse configuration to match the path
@@ -192,7 +192,7 @@ func (b Browse) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	}
 
 	// Didn't qualify; pass-thru
-	return b.Next(w, r)
+	return b.Next.ServeHTTP(w, r)
 }
 
 // parse returns a list of browsing configurations
diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go
index 7f2db6f8c..d89a95fde 100644
--- a/middleware/errors/errors.go
+++ b/middleware/errors/errors.go
@@ -39,15 +39,15 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		return nil
 	})
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
+	return func(next middleware.Handler) middleware.Handler {
 		handler.Next = next
-		return handler.ServeHTTP
+		return handler
 	}, nil
 }
 
 // ErrorHandler handles HTTP errors (or errors from other middleware).
 type ErrorHandler struct {
-	Next       middleware.HandlerFunc
+	Next       middleware.Handler
 	ErrorPages map[int]string // map of status code to filename
 	LogFile    string
 	Log        *log.Logger
@@ -61,7 +61,7 @@ func (h ErrorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, er
 		}
 	}()
 
-	status, err := h.Next(w, r)
+	status, err := h.Next.ServeHTTP(w, r)
 
 	if err != nil {
 		h.Log.Printf("[ERROR %d %s] %v", status, r.URL.Path, err)
diff --git a/middleware/extension/ext.go b/middleware/extension/ext.go
index fc599b422..959f22d27 100644
--- a/middleware/extension/ext.go
+++ b/middleware/extension/ext.go
@@ -24,12 +24,12 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		return nil, err
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
+	return func(next middleware.Handler) middleware.Handler {
 		return Ext{
 			Next:       next,
 			Extensions: extensions,
 			Root:       root,
-		}.ServeHTTP
+		}
 	}, nil
 }
 
@@ -37,7 +37,7 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 // It tries extensions in the order listed in Extensions.
 type Ext struct {
 	// Next handler in the chain
-	Next middleware.HandlerFunc
+	Next middleware.Handler
 
 	// Path to ther root of the site
 	Root string
@@ -57,7 +57,7 @@ func (e Ext) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 			}
 		}
 	}
-	return e.Next(w, r)
+	return e.Next.ServeHTTP(w, r)
 }
 
 // parse sets up an instance of extension middleware
diff --git a/middleware/fastcgi/fastcgi.go b/middleware/fastcgi/fastcgi.go
index 281867f1d..844e5dc99 100644
--- a/middleware/fastcgi/fastcgi.go
+++ b/middleware/fastcgi/fastcgi.go
@@ -26,8 +26,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		rules = append(rules, rule)
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) (int, error) {
+	return func(next middleware.Handler) middleware.Handler {
+		return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 
 			servedFcgi := false
 			for _, rule := range rules {
@@ -97,11 +97,11 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 			}
 
 			if !servedFcgi {
-				return next(w, r)
+				return next.ServeHTTP(w, r)
 			}
 
 			return 0, nil
-		}
+		})
 	}, nil
 }
 
diff --git a/middleware/gzip/gzip.go b/middleware/gzip/gzip.go
index cf3457f5b..dd200fda3 100644
--- a/middleware/gzip/gzip.go
+++ b/middleware/gzip/gzip.go
@@ -11,29 +11,28 @@ import (
 	"github.com/mholt/caddy/middleware"
 )
 
-// Gzip is a http.Handler middleware type which gzips HTTP responses.
+// Gzip is a middleware type which gzips HTTP responses.
 type Gzip struct {
-	Next middleware.HandlerFunc
+	Next middleware.Handler
 }
 
 // New creates a new gzip middleware instance.
 func New(c middleware.Controller) (middleware.Middleware, error) {
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		gz := Gzip{Next: next}
-		return gz.ServeHTTP
+	return func(next middleware.Handler) middleware.Handler {
+		return Gzip{Next: next}
 	}, nil
 }
 
 // ServeHTTP serves a gzipped response if the client supports it.
 func (g Gzip) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
-		return g.Next(w, r)
+		return g.Next.ServeHTTP(w, r)
 	}
 	w.Header().Set("Content-Encoding", "gzip")
 	gzipWriter := gzip.NewWriter(w)
 	defer gzipWriter.Close()
 	gz := gzipResponseWriter{Writer: gzipWriter, ResponseWriter: w}
-	return g.Next(gz, r)
+	return g.Next.ServeHTTP(gz, r)
 }
 
 // gzipResponeWriter wraps the underlying Write method
diff --git a/middleware/headers/headers.go b/middleware/headers/headers.go
index 5ef8207cd..1c82076c7 100644
--- a/middleware/headers/headers.go
+++ b/middleware/headers/headers.go
@@ -12,7 +12,7 @@ import (
 // Headers is middleware that adds headers to the responses
 // for requests matching a certain path.
 type Headers struct {
-	Next  middleware.HandlerFunc
+	Next  middleware.Handler
 	Rules []HeaderRule
 }
 
@@ -26,7 +26,7 @@ func (h Headers) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error)
 			}
 		}
 	}
-	return h.Next(w, r)
+	return h.Next.ServeHTTP(w, r)
 }
 
 type (
diff --git a/middleware/headers/new.go b/middleware/headers/new.go
index cb727ef14..28ddd582a 100644
--- a/middleware/headers/new.go
+++ b/middleware/headers/new.go
@@ -10,7 +10,7 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		return nil, err
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return Headers{Next: next, Rules: rules}.ServeHTTP
+	return func(next middleware.Handler) middleware.Handler {
+		return Headers{Next: next, Rules: rules}
 	}, nil
 }
diff --git a/middleware/log/log.go b/middleware/log/log.go
index 7363c4bd5..a5602c027 100644
--- a/middleware/log/log.go
+++ b/middleware/log/log.go
@@ -39,8 +39,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		return nil
 	})
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return Logger{Next: next, Rules: rules}.ServeHTTP
+	return func(next middleware.Handler) middleware.Handler {
+		return Logger{Next: next, Rules: rules}
 	}, nil
 }
 
@@ -48,13 +48,13 @@ func (l Logger) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
 	for _, rule := range l.Rules {
 		if middleware.Path(r.URL.Path).Matches(rule.PathScope) {
 			responseRecorder := middleware.NewResponseRecorder(w)
-			status, err := l.Next(responseRecorder, r)
+			status, err := l.Next.ServeHTTP(responseRecorder, r)
 			rep := middleware.NewReplacer(r, responseRecorder)
 			rule.Log.Println(rep.Replace(rule.Format))
 			return status, err
 		}
 	}
-	return l.Next(w, r)
+	return l.Next.ServeHTTP(w, r)
 }
 
 func parse(c middleware.Controller) ([]LogRule, error) {
@@ -103,7 +103,7 @@ func parse(c middleware.Controller) ([]LogRule, error) {
 }
 
 type Logger struct {
-	Next  middleware.HandlerFunc
+	Next  middleware.Handler
 	Rules []LogRule
 }
 
diff --git a/middleware/markdown/markdown.go b/middleware/markdown/markdown.go
index 14e8f94d8..4d0fc0430 100644
--- a/middleware/markdown/markdown.go
+++ b/middleware/markdown/markdown.go
@@ -21,7 +21,7 @@ type Markdown struct {
 	Root string
 
 	// Next HTTP handler in the chain
-	Next middleware.HandlerFunc
+	Next middleware.Handler
 
 	// The list of markdown configurations
 	Configs []MarkdownConfig
@@ -58,9 +58,9 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		Configs: mdconfigs,
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
+	return func(next middleware.Handler) middleware.Handler {
 		md.Next = next
-		return md.ServeHTTP
+		return md
 	}, nil
 }
 
@@ -122,7 +122,7 @@ func (md Markdown) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error
 	}
 
 	// Didn't qualify to serve as markdown; pass-thru
-	return md.Next(w, r)
+	return md.Next.ServeHTTP(w, r)
 }
 
 // parse creates new instances of Markdown middleware.
diff --git a/middleware/middleware.go b/middleware/middleware.go
index ca66841f6..051fb53fe 100644
--- a/middleware/middleware.go
+++ b/middleware/middleware.go
@@ -6,26 +6,29 @@ import "net/http"
 type (
 	// Generator represents the outer layer of a middleware that
 	// parses tokens to configure the middleware instance.
+	//
+	// Note: This type will be moved into a different package in the future.
 	Generator func(Controller) (Middleware, error)
 
 	// Middleware is the middle layer which represents the traditional
-	// idea of middleware: it is passed the next HandlerFunc in the chain
-	// and returns the inner layer, which is the actual Handler.
-	Middleware func(HandlerFunc) HandlerFunc
+	// idea of middleware: it chains one Handler to the next by being
+	// passed the next Handler in the chain.
+	//
+	// Note: This type will be moved into a different package in the future.
+	Middleware func(Handler) Handler
 
-	// HandlerFunc is like http.HandlerFunc except it returns a status code
-	// and an error. It is the inner-most layer which serves individual
-	// requests. The status code is for the client's benefit; the error
+	// Handler is like http.Handler except ServeHTTP returns a status code
+	// and an error. The status code is for the client's benefit; the error
 	// value is for the server's benefit. The status code will be sent to
 	// the client while the error value will be logged privately. Sometimes,
 	// an error status code (4xx or 5xx) may be returned with a nil error
 	// when there is no reason to log the error on the server.
 	//
 	// If a HandlerFunc returns an error (status >= 400), it should NOT
-	// write to the response. This philosophy makes middleware.HandlerFunc
-	// different from http.HandlerFunc: error handling should happen at
-	// the application layer or in dedicated error-handling middleware
-	// only, rather than with an "every middleware for itself" paradigm.
+	// write to the response. This philosophy makes middleware.Handler
+	// different from http.Handler: error handling should happen at the
+	// application layer or in dedicated error-handling middleware only
+	// rather than with an "every middleware for itself" paradigm.
 	//
 	// The application or error-handling middleware should incorporate logic
 	// to ensure that the client always gets a proper response according to
@@ -38,10 +41,6 @@ type (
 	// response for a status code >= 400. When ANY handler writes to the
 	// response, it should return a status code < 400 to signal others to
 	// NOT write to the response again, which would be erroneous.
-	HandlerFunc func(http.ResponseWriter, *http.Request) (int, error)
-
-	// Handler is like http.Handler except ServeHTTP returns a status code
-	// and an error. See HandlerFunc documentation for more information.
 	Handler interface {
 		ServeHTTP(http.ResponseWriter, *http.Request) (int, error)
 	}
@@ -127,3 +126,13 @@ type (
 		Err(string) error
 	}
 )
+
+// HandlerFunc is a convenience type like http.HandlerFunc, except
+// ServeHTTP returns a status code and an error. See Handler
+// documentation for more information.
+type HandlerFunc func(http.ResponseWriter, *http.Request) (int, error)
+
+// ServeHTTP implements the Handler interface.
+func (f HandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
+	return f(w, r)
+}
diff --git a/middleware/proxy/proxy.go b/middleware/proxy/proxy.go
index 09c22134f..4e58361ea 100644
--- a/middleware/proxy/proxy.go
+++ b/middleware/proxy/proxy.go
@@ -25,8 +25,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		rules = append(rules, rule)
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) (int, error) {
+	return func(next middleware.Handler) middleware.Handler {
+		return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 
 			for _, rule := range rules {
 				if middleware.Path(r.URL.Path).Matches(rule.from) {
@@ -59,8 +59,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 				}
 			}
 
-			return next(w, r)
-		}
+			return next.ServeHTTP(w, r)
+		})
 	}, nil
 }
 
diff --git a/middleware/redirect/redirect.go b/middleware/redirect/redirect.go
index 6cdbcfa64..77689bc0e 100644
--- a/middleware/redirect/redirect.go
+++ b/middleware/redirect/redirect.go
@@ -38,8 +38,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 		}
 	}
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) (int, error) {
+	return func(next middleware.Handler) middleware.Handler {
+		return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 			for _, rule := range redirects {
 				if middleware.Path(r.URL.Path).Matches(rule.From) {
 					if rule.From == "/" {
@@ -51,8 +51,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 					return 0, nil
 				}
 			}
-			return next(w, r)
-		}
+			return next.ServeHTTP(w, r)
+		})
 	}, nil
 }
 
diff --git a/middleware/rewrite/rewrite.go b/middleware/rewrite/rewrite.go
index f894ce141..cfb391425 100644
--- a/middleware/rewrite/rewrite.go
+++ b/middleware/rewrite/rewrite.go
@@ -29,16 +29,16 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 	}
 
 	// TODO: Why can't we just return an http.Handler here instead?
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return func(w http.ResponseWriter, r *http.Request) (int, error) {
+	return func(next middleware.Handler) middleware.Handler {
+		return middleware.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
 			for _, rule := range rewrites {
 				if r.URL.Path == rule.From {
 					r.URL.Path = rule.To
 					break
 				}
 			}
-			return next(w, r)
-		}
+			return next.ServeHTTP(w, r)
+		})
 	}, nil
 }
 
diff --git a/middleware/websockets/websockets.go b/middleware/websockets/websockets.go
index 8bee6f095..ff70607c7 100644
--- a/middleware/websockets/websockets.go
+++ b/middleware/websockets/websockets.go
@@ -16,7 +16,7 @@ type (
 	// websocket endpoints.
 	WebSockets struct {
 		// Next is the next HTTP handler in the chain for when the path doesn't match
-		Next middleware.HandlerFunc
+		Next middleware.Handler
 
 		// Sockets holds all the web socket endpoint configurations
 		Sockets []WSConfig
@@ -46,7 +46,7 @@ func (ws WebSockets) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, err
 	}
 
 	// Didn't match a websocket path, so pass-thru
-	return ws.Next(w, r)
+	return ws.Next.ServeHTTP(w, r)
 }
 
 // New constructs and configures a new websockets middleware instance.
@@ -115,8 +115,8 @@ func New(c middleware.Controller) (middleware.Middleware, error) {
 	GatewayInterface = envGatewayInterface
 	ServerSoftware = envServerSoftware
 
-	return func(next middleware.HandlerFunc) middleware.HandlerFunc {
-		return WebSockets{Next: next, Sockets: websocks}.ServeHTTP
+	return func(next middleware.Handler) middleware.Handler {
+		return WebSockets{Next: next, Sockets: websocks}
 	}, nil
 }
 
diff --git a/server/server.go b/server/server.go
index 78fef0822..963264b0a 100644
--- a/server/server.go
+++ b/server/server.go
@@ -28,7 +28,7 @@ var servers = make(map[string]*Server)
 type Server struct {
 	config     config.Config
 	fileServer middleware.Handler
-	stack      middleware.HandlerFunc
+	stack      middleware.Handler
 }
 
 // New creates a new Server and registers it with the list
@@ -118,8 +118,9 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	status, _ := s.stack(w, r)
+	status, _ := s.stack.ServeHTTP(w, r)
 
+	// Fallback error response in case error handling wasn't chained in
 	if status >= 400 {
 		w.WriteHeader(status)
 		fmt.Fprintf(w, "%d %s", status, http.StatusText(status))
@@ -144,7 +145,7 @@ func (s *Server) buildStack() error {
 // compile is an elegant alternative to nesting middleware function
 // calls like handler1(handler2(handler3(finalHandler))).
 func (s *Server) compile(layers []middleware.Middleware) {
-	s.stack = s.fileServer.ServeHTTP // core app layer
+	s.stack = s.fileServer // core app layer
 	for i := len(layers) - 1; i >= 0; i-- {
 		s.stack = layers[i](s.stack)
 	}