Fixed #457

- Dropped static middleware in favor of Echo#Static & Echo#StaticConfig.
- Enhanced middleware chaining.

Signed-off-by: Vishal Rana <vr@labstack.com>

README.md

@@ -238,7 +238,6 @@ Middleware | Description
 [Gzip](https://labstack.com/echo/guide/middleware/#gzip-middleware:37ab2f15ff048f67959bcac0a6032f32) | Send gzip HTTP response
 [BasicAuth](https://labstack.com/echo/guide/middleware/#basicauth-middleware:37ab2f15ff048f67959bcac0a6032f32) | HTTP basic authentication
 [CORS](https://labstack.com/echo/guide/middleware/#cors-middleware:37ab2f15ff048f67959bcac0a6032f32) | Cross-Origin Resource Sharing
-[Static](https://labstack.com/echo/guide/static-files/#using-static-middleware:123f9d1043075fe4874616541b409e4d) | Serve static files
 [AddTrailingSlash](https://labstack.com/echo/guide/middleware/#addtrailingslash-middleware:37ab2f15ff048f67959bcac0a6032f32) | Add trailing slash to the request URI
 [RemoveTrailingSlash](https://labstack.com/echo/guide/middleware/#removetrailingslash-middleware:37ab2f15ff048f67959bcac0a6032f32) | Remove trailing slash from the request URI
 

context.go

@@ -125,9 +125,6 @@ type (
 		// Error invokes the registered HTTP error handler. Generally used by middleware.
 		Error(err error)
 
-		// Handler implements `Handler` interface.
-		Handle(Context) error
-
 		// Logger returns the `Logger` instance.
 		Logger() *log.Logger
 
@@ -203,10 +200,6 @@ func (c *context) Value(key interface{}) interface{} {
 	return c.netContext.Value(key)
 }
 
-func (c *context) Handle(ctx Context) error {
-	return c.handler(ctx)
-}
-
 func (c *context) Request() engine.Request {
 	return c.request
 }

echo.go

@@ -27,7 +27,7 @@ Example:
 	    e.Use(middleware.Recover())
 
 	    // Routes
-	    e.Get("/", hello())
+	    e.Get("/", hello)
 
 	    // Start server
 	    e.Run(standard.New(":1323"))
@@ -45,7 +45,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"path"
 	"reflect"
 	"runtime"
 	"strings"
@@ -58,10 +57,8 @@ import (
 type (
 	// Echo is the top-level framework instance.
 	Echo struct {
-		prefix           string
+		premiddleware    []MiddlewareFunc
 		middleware       []MiddlewareFunc
-		head             HandlerFunc
-		pristineHead     HandlerFunc
 		maxParam         *int
 		notFoundHandler  HandlerFunc
 		httpErrorHandler HTTPErrorHandler
@@ -219,12 +216,6 @@ func New() (e *Echo) {
 		return NewContext(nil, nil, e)
 	}
 	e.router = NewRouter(e)
-	e.middleware = []MiddlewareFunc{e.router.Process}
-	e.head = func(c Context) error {
-		return c.Handle(c)
-	}
-	e.pristineHead = e.head
-	e.chainMiddleware()
 
 	// Defaults
 	e.SetHTTPErrorHandler(e.DefaultHTTPErrorHandler)
@@ -305,21 +296,12 @@ func (e *Echo) Debug() bool {
 
 // Pre adds middleware to the chain which is run before router.
 func (e *Echo) Pre(middleware ...MiddlewareFunc) {
-	e.middleware = append(middleware, e.middleware...)
+	e.premiddleware = append(e.premiddleware, middleware...)
-	e.chainMiddleware()
 }
 
 // Use adds middleware to the chain which is run after router.
 func (e *Echo) Use(middleware ...MiddlewareFunc) {
 	e.middleware = append(e.middleware, middleware...)
-	e.chainMiddleware()
-}
-
-func (e *Echo) chainMiddleware() {
-	e.head = e.pristineHead
-	for i := len(e.middleware) - 1; i >= 0; i-- {
-		e.head = e.middleware[i](e.head)
-	}
 }
 
 // Connect registers a new CONNECT route for a path with matching handler in the
@@ -392,14 +374,21 @@ func (e *Echo) Match(methods []string, path string, handler HandlerFunc, middlew
 	}
 }
 
-// Static serves files from provided `root` directory for `/<prefix>*` HTTP path.
+// Static serves static files from provided `root` directory for `/<prefix>*` HTTP
+// path.
 func (e *Echo) Static(prefix, root string) {
-	e.Get(prefix+"*", func(c Context) error {
+	e.StaticWithConfig(prefix, StaticConfig{
-		return c.File(path.Join(root, c.P(0))) // Param `_`
+		Root: root,
 	})
 }
 
-// File serves provided file for `/<path>` HTTP path.
+// StaticWithConfig serves static files with provided config for `/<prefix>*` HTTP
+// path.
+func (e *Echo) StaticWithConfig(prefix string, config StaticConfig) {
+	e.Get(prefix+"*", StaticWithConfig(config))
+}
+
+// File serve provided file for `/<path>` HTTP path.
 func (e *Echo) File(path, file string) {
 	e.Get(path, func(c Context) error {
 		return c.File(file)
@@ -428,11 +417,6 @@ func (e *Echo) add(method, path string, handler HandlerFunc, middleware ...Middl
 func (e *Echo) Group(prefix string, m ...MiddlewareFunc) (g *Group) {
 	g = &Group{prefix: prefix, echo: e}
 	g.Use(m...)
-	// Dummy handler to use static middleware with groups
-	// See also issue #446
-	g.Get("/", func(c Context) error {
-		return c.NoContent(http.StatusNotFound)
-	})
 	return
 }
 
@@ -487,8 +471,25 @@ func (e *Echo) ServeHTTP(rq engine.Request, rs engine.Response) {
 	c := e.pool.Get().(*context)
 	c.Reset(rq, rs)
 
+	// Middleware
+	h := func(Context) error {
+		method := rq.Method()
+		path := rq.URL().Path()
+		e.router.Find(method, path, c)
+		h := c.handler
+		for i := len(e.middleware) - 1; i >= 0; i-- {
+			h = e.middleware[i](h)
+		}
+		return h(c)
+	}
+
+	// Premiddleware
+	for i := len(e.premiddleware) - 1; i >= 0; i-- {
+		h = e.premiddleware[i](h)
+	}
+
 	// Execute chain
-	if err := e.head(c); err != nil {
+	if err := h(c); err != nil {
 		e.httpErrorHandler(err, c)
 	}
 

group.go

@@ -81,23 +81,27 @@ func (g *Group) Group(prefix string, m ...MiddlewareFunc) *Group {
 	return g.echo.Group(g.prefix+prefix, m...)
 }
 
-func (g *Group) add(method, path string, handler HandlerFunc, middleware ...MiddlewareFunc) {
+// Static implements `Echo#Static()` for sub-routes within the Group.
-	path = g.prefix + path
+func (g *Group) Static(prefix, root string) {
-	name := handlerName(handler)
+	g.StaticWithConfig(prefix, StaticConfig{
-	middleware = append(g.middleware, middleware...)
+		Root: root,
-
+	})
-	g.echo.router.Add(method, path, func(c Context) error {
+}
-		h := handler
+
-		// Chain middleware
+// StaticWithConfig implements `Echo#StaticWithConfig()` for sub-routes within the
-		for i := len(middleware) - 1; i >= 0; i-- {
+// Group.
-			h = middleware[i](h)
+func (g *Group) StaticWithConfig(prefix string, config StaticConfig) {
-		}
+	g.Get(prefix+"*", StaticWithConfig(config))
-		return h(c)
+}
-	}, g.echo)
+
-	r := Route{
+// File implements `Echo#File()` for sub-routes within the Group.
-		Method:  method,
+func (g *Group) File(path, file string) {
-		Path:    path,
+	g.Get(path, func(c Context) error {
-		Handler: name,
+		return c.File(file)
-	}
+	})
-	g.echo.router.routes = append(g.echo.router.routes, r)
+}
+
+func (g *Group) add(method, path string, handler HandlerFunc, middleware ...MiddlewareFunc) {
+	middleware = append(g.middleware, middleware...)
+	g.echo.add(method, g.prefix+path, handler, middleware...)
 }

group_test.go

@@ -14,4 +14,8 @@ func TestGroup(t *testing.T) {
 	g.Post("/", h)
 	g.Put("/", h)
 	g.Trace("/", h)
+	g.Any("/", h)
+	g.Match([]string{GET, POST}, "/", h)
+	g.Static("/static", "/tmp")
+	g.File("/walle", "_fixture/images//walle.png")
 }

middleware/static.go

@@ -1,118 +0,0 @@
-package middleware
-
-import (
-	"fmt"
-	"net/http"
-	"path"
-
-	"github.com/labstack/echo"
-)
-
-type (
-	// StaticConfig defines the config for static middleware.
-	StaticConfig struct {
-		// Root is the directory from where the static content is served.
-		// Required.
-		Root string `json:"root"`
-
-		// Index is the list of index files to be searched and used when serving
-		// a directory.
-		// Optional with default value as []string{"index.html"}.
-		Index []string `json:"index"`
-
-		// Browse is a flag to enable/disable directory browsing.
-		// Optional with default value as false.
-		Browse bool `json:"browse"`
-	}
-)
-
-var (
-	// DefaultStaticConfig is the default static middleware config.
-	DefaultStaticConfig = StaticConfig{
-		Root:   "",
-		Index:  []string{"index.html"},
-		Browse: false,
-	}
-)
-
-// Static returns a static middleware to serves static content from the provided
-// root directory.
-func Static(root string) echo.MiddlewareFunc {
-	c := DefaultStaticConfig
-	c.Root = root
-	return StaticWithConfig(c)
-}
-
-// StaticWithConfig returns a static middleware from config.
-// See `Static()`.
-func StaticWithConfig(config StaticConfig) echo.MiddlewareFunc {
-	// Defaults
-	if config.Index == nil {
-		config.Index = DefaultStaticConfig.Index
-	}
-
-	return func(next echo.HandlerFunc) echo.HandlerFunc {
-		return func(c echo.Context) error {
-			fs := http.Dir(config.Root)
-			p := c.Request().URL().Path()
-			if c.P(0) != "" { // If serving from `Group`, e.g. `/static*`
-				p = c.P(0)
-			}
-			file := path.Clean(p)
-			f, err := fs.Open(file)
-			if err != nil {
-				return next(c)
-			}
-			defer f.Close()
-
-			fi, err := f.Stat()
-			if err != nil {
-				return err
-			}
-
-			if fi.IsDir() {
-				/* NOTE:
-				Not checking the Last-Modified header as it caches the response `304` when
-				changing different directories for the same path.
-				*/
-				d := f
-
-				// Index file
-				// TODO: search all files
-				file = path.Join(file, config.Index[0])
-				f, err = fs.Open(file)
-				if err != nil {
-					if config.Browse {
-						dirs, err := d.Readdir(-1)
-						if err != nil {
-							return err
-						}
-
-						// Create a directory index
-						rs := c.Response()
-						rs.Header().Set(echo.HeaderContentType, echo.MIMETextHTMLCharsetUTF8)
-						if _, err = fmt.Fprintf(rs, "<pre>\n"); err != nil {
-							return err
-						}
-						for _, d := range dirs {
-							name := d.Name()
-							color := "#212121"
-							if d.IsDir() {
-								color = "#e91e63"
-								name += "/"
-							}
-							if _, err = fmt.Fprintf(rs, "<a href=\"%s\" style=\"color: %s;\">%s</a>\n", name, color, name); err != nil {
-								return err
-							}
-						}
-						_, err = fmt.Fprintf(rs, "</pre>\n")
-						return err
-					}
-					return next(c)
-				}
-				fi, _ = f.Stat() // Index file stat
-			}
-			return c.ServeContent(f, fi.Name(), fi.ModTime())
-		}
-	}
-}

middleware/static_test.go

@@ -1 +0,0 @@
-package middleware

router.go

@@ -50,21 +50,6 @@ func NewRouter(e *Echo) *Router {
 	}
 }
 
-// Process implements `echo.MiddlewareFunc` which makes router a middleware.
-func (r *Router) Process(next HandlerFunc) HandlerFunc {
-	return func(c Context) error {
-		method := c.Request().Method()
-		path := c.Request().URL().Path()
-		r.Find(method, path, c)
-		return next(c)
-	}
-}
-
-// Priority is super secret.
-func (r *Router) Priority() int {
-	return 0
-}
-
 // Add registers a new route for method and path with matching handler.
 func (r *Router) Add(method, path string, h HandlerFunc, e *Echo) {
 	ppath := path        // Pristine path
@@ -406,9 +391,9 @@ func (r *Router) Find(method, path string, context Context) {
 	}
 
 End:
+	ctx.handler = cn.findHandler(method)
 	ctx.path = cn.ppath
 	ctx.pnames = cn.pnames
-	ctx.handler = cn.findHandler(method)
 
 	// NOTE: Slow zone...
 	if ctx.handler == nil {
@@ -419,10 +404,13 @@ End:
 		if cn = cn.findChildByKind(akind); cn == nil {
 			return
 		}
-		ctx.pvalues[len(cn.pnames)-1] = ""
 		if ctx.handler = cn.findHandler(method); ctx.handler == nil {
 			ctx.handler = cn.checkMethodNotAllowed()
 		}
+		ctx.path = cn.ppath
+		ctx.pnames = cn.pnames
+		ctx.pvalues[len(cn.pnames)-1] = ""
 	}
+
 	return
 }

router_test.go

@@ -281,9 +281,9 @@ func TestRouterStatic(t *testing.T) {
 		c.Set("path", path)
 		return nil
 	}, e)
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 	r.Find(GET, path, c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, path, c.Get("path"))
 }
 
@@ -377,10 +377,10 @@ func TestRouterMixParamMatchAny(t *testing.T) {
 	r.Add(GET, "/users/:id/*", func(c Context) error {
 		return nil
 	}, e)
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 
 	r.Find(GET, "/users/joe/comments", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, "joe", c.P(0))
 }
 
@@ -396,11 +396,11 @@ func TestRouterMultiRoute(t *testing.T) {
 	r.Add(GET, "/users/:id", func(c Context) error {
 		return nil
 	}, e)
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 
 	// Route > /users
 	r.Find(GET, "/users", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, "/users", c.Get("path"))
 
 	// Route > /users/:id
@@ -408,9 +408,9 @@ func TestRouterMultiRoute(t *testing.T) {
 	assert.Equal(t, "1", c.P(0))
 
 	// Route > /user
-	c = NewContext(nil, nil, e)
+	c = NewContext(nil, nil, e).Object()
 	r.Find(GET, "/user", c)
-	he := c.Handle(c).(*HTTPError)
+	he := c.handler(c).(*HTTPError)
 	assert.Equal(t, http.StatusNotFound, he.Code)
 }
 
@@ -447,41 +447,41 @@ func TestRouterPriority(t *testing.T) {
 		c.Set("g", 7)
 		return nil
 	}, e)
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 
 	// Route > /users
 	r.Find(GET, "/users", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 1, c.Get("a"))
 
 	// Route > /users/new
 	r.Find(GET, "/users/new", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 2, c.Get("b"))
 
 	// Route > /users/:id
 	r.Find(GET, "/users/1", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 3, c.Get("c"))
 
 	// Route > /users/dew
 	r.Find(GET, "/users/dew", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 4, c.Get("d"))
 
 	// Route > /users/:id/files
 	r.Find(GET, "/users/1/files", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 5, c.Get("e"))
 
 	// Route > /users/:id
 	r.Find(GET, "/users/news", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 3, c.Get("c"))
 
 	// Route > /users/*
 	r.Find(GET, "/users/joe/books", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 7, c.Get("g"))
 	assert.Equal(t, "joe/books", c.Param("_*"))
 }
@@ -490,7 +490,7 @@ func TestRouterPriority(t *testing.T) {
 func TestRouterPriorityNotFound(t *testing.T) {
 	e := New()
 	r := e.router
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 
 	// Add
 	r.Add(GET, "/a/foo", func(c Context) error {
@@ -504,16 +504,16 @@ func TestRouterPriorityNotFound(t *testing.T) {
 
 	// Find
 	r.Find(GET, "/a/foo", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 1, c.Get("a"))
 
 	r.Find(GET, "/a/bar", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, 2, c.Get("b"))
 
-	c = NewContext(nil, nil, e)
+	c = NewContext(nil, nil, e).Object()
 	r.Find(GET, "/abc/def", c)
-	he := c.Handle(c).(*HTTPError)
+	he := c.handler(c).(*HTTPError)
 	assert.Equal(t, http.StatusNotFound, he.Code)
 }
 
@@ -532,11 +532,11 @@ func TestRouterParamNames(t *testing.T) {
 	r.Add(GET, "/users/:uid/files/:fid", func(c Context) error {
 		return nil
 	}, e)
-	c := NewContext(nil, nil, e)
+	c := NewContext(nil, nil, e).Object()
 
 	// Route > /users
 	r.Find(GET, "/users", c)
-	c.Handle(c)
+	c.handler(c)
 	assert.Equal(t, "/users", c.Get("path"))
 
 	// Route > /users/:id

static.go

@@ -0,0 +1,110 @@
+package echo
+
+import (
+	"fmt"
+	"net/http"
+	"path"
+)
+
+type (
+	// StaticConfig defines the config for static handler.
+	StaticConfig struct {
+		// Root is the directory from where the static content is served.
+		// Required.
+		Root string `json:"root"`
+
+		// Index is the list of index files to be searched and used when serving
+		// a directory.
+		// Optional with default value as []string{"index.html"}.
+		Index []string `json:"index"`
+
+		// Browse is a flag to enable/disable directory browsing.
+		// Optional with default value as false.
+		Browse bool `json:"browse"`
+	}
+)
+
+var (
+	// DefaultStaticConfig is the default static handler config.
+	DefaultStaticConfig = StaticConfig{
+		Index:  []string{"index.html"},
+		Browse: false,
+	}
+)
+
+// Static returns a static handler to serves static content from the provided
+// root directory.
+func Static(root string) HandlerFunc {
+	c := DefaultStaticConfig
+	c.Root = root
+	return StaticWithConfig(c)
+}
+
+// StaticWithConfig returns a static handler from config.
+// See `Static()`.
+func StaticWithConfig(config StaticConfig) HandlerFunc {
+	// Defaults
+	if config.Index == nil {
+		config.Index = DefaultStaticConfig.Index
+	}
+
+	return func(c Context) error {
+		fs := http.Dir(config.Root)
+		file := path.Clean(c.P(0))
+		f, err := fs.Open(file)
+		if err != nil {
+			return ErrNotFound
+		}
+		defer f.Close()
+		fi, err := f.Stat()
+		if err != nil {
+			return ErrNotFound
+		}
+
+		if fi.IsDir() {
+			/* NOTE:
+			Not checking the Last-Modified header as it caches the response `304` when
+			changing different directories for the same path.
+			*/
+			d := f
+
+			// Index file
+			// TODO: search all files
+			file = path.Join(file, config.Index[0])
+			f, err = fs.Open(file)
+			if err != nil {
+				return ErrNotFound
+			}
+			if config.Browse {
+				dirs, err := d.Readdir(-1)
+				if err != nil {
+					return err
+				}
+
+				// Create a directory index
+				rs := c.Response()
+				rs.Header().Set(HeaderContentType, MIMETextHTMLCharsetUTF8)
+				if _, err = fmt.Fprintf(rs, "<pre>\n"); err != nil {
+					return err
+				}
+				for _, d := range dirs {
+					name := d.Name()
+					color := "#212121"
+					if d.IsDir() {
+						color = "#e91e63"
+						name += "/"
+					}
+					if _, err = fmt.Fprintf(rs, "<a href=\"%s\" style=\"color: %s;\">%s</a>\n", name, color, name); err != nil {
+						return err
+					}
+				}
+				_, err = fmt.Fprintf(rs, "</pre>\n")
+				return err
+			}
+			if fi, err = f.Stat(); err != nil { // Index file
+				return ErrNotFound
+			}
+		}
+		return c.ServeContent(f, fi.Name(), fi.ModTime())
+	}
+}