Make HTTP middleware easier to use

server/router/router_test.go

Index: server/router/router_test.go
diff --git a/server/router/router_test.go b/server/router/router_test.go
new file mode 100644
index 0000000000000000000000000000000000000000..8d6f0d630a3ae14dc236f12ae31de4e4a188e788
--- /dev/null
+++ b/server/router/router_test.go
@@ -0,0 +1,273 @@
+// Copyright 2016 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package router
+
+import (
+	"io"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"golang.org/x/net/context"
+
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func TestRouter(t *testing.T) {
+	t.Parallel()
+
+	outputKey := "output"
+	appendValue := func(c context.Context, key string, val string) context.Context {
+		var current []string
+		if v := c.Value(key); v != nil {
+			current = v.([]string)
+		}
+		return context.WithValue(c, key, append(current, val))
+	}
+	a := func(c *Context, next Handler) {
+		c.Context = appendValue(c, outputKey, "a:before")
+		next(c)
+		c.Context = appendValue(c, outputKey, "a:after")
+	}
+	b := func(c *Context, next Handler) {
+		c.Context = appendValue(c, outputKey, "b:before")
+		next(c)
+		c.Context = appendValue(c, outputKey, "b:after")
+	}
+	c := func(c *Context, next Handler) {
+		c.Context = appendValue(c, outputKey, "c")
+		next(c)
+	}
+	d := func(c *Context, next Handler) {
+		next(c)
+		c.Context = appendValue(c, outputKey, "d")
+	}
+	stop := func(_ *Context, _ Handler) {}
+	handler := func(c *Context) {
+		c.Context = appendValue(c, outputKey, "handler")
+	}
+
+	Convey("Router", t, func() {
+		r := New()
+
+		Convey("New", func() {
+			Convey("Should initialize non-nil httprouter.Router", func() {
+				So(r.hrouter, ShouldNotBeNil)
+			})
+		})
+
+		Convey("Use", func() {
+			Convey("Should append middleware", func() {
+				So(len(r.middleware), ShouldEqual, 0)
+				r.Use(MiddlewareChain{a, b})
+				So(len(r.middleware), ShouldEqual, 2)
+			})
+		})
+
+		Convey("Subrouter", func() {
+			Convey("Should create new router with values from original router", func() {
+				r.BasePath = "/foo/"
+				r.Use(MiddlewareChain{a, b})
+				r2 := r.Subrouter("/bar")
+				So(r.hrouter, ShouldPointTo, r2.hrouter)
+				So(r2.BasePath, ShouldEqual, "/foo/bar/")
+				So(&r.middleware[0], ShouldNotEqual, &r2.middleware[0])
+				So(&r.middleware[1], ShouldNotEqual, &r2.middleware[1])
+				So(len(r.middleware), ShouldEqual, len(r2.middleware))
+				So(r.middleware[0], ShouldEqual, r2.middleware[0])
+				So(r.middleware[1], ShouldEqual, r2.middleware[1])
+			})
+		})
+
+		Convey("Handle", func() {
+			Convey("Should not modify existing empty r.middleware slice", func() {
+				So(len(r.middleware), ShouldEqual, 0)
+				r.Handle("GET", "/bar", MiddlewareChain{b, c}, handler)
+				So(len(r.middleware), ShouldEqual, 0)
+			})
+
+			Convey("Should not modify existing r.middleware slice", func() {
+				r.Use(MiddlewareChain{a})
+				So(len(r.middleware), ShouldEqual, 1)
+				r.Handle("GET", "/bar", MiddlewareChain{b, c}, handler)
+				So(len(r.middleware), ShouldEqual, 1)
+			})
+		})
+
+		Convey("run", func() {
+			ctx := &Context{Context: context.Background()}
+
+			Convey("Should execute handler when using nil middlewares", func() {
+				run(ctx, nil, nil, handler)
+				So(ctx.Value(outputKey).([]string), ShouldResemble, []string{"handler"})

[nodir, 2016/06/16 22:19:00]

ctx.Value(outputKey).([]string) casts the value from interface{} to []string,
but first parameter of So is interface{}, so conversion is noop, thus
unnecessary

[nishanths, 2016/06/16 22:24:21]

Thanks. Done.

+			})
+
+			Convey("Should execute middlewares and handler in order", func() {
+				m := MiddlewareChain{a, b, c}
+				n := MiddlewareChain{d}
+				run(ctx, m, n, handler)
+				So(ctx.Value(outputKey).([]string), ShouldResemble,
+					[]string{"a:before", "b:before", "c", "handler", "d", "b:after", "a:after"},
+				)
+			})
+
+			Convey("Should not execute upcoming middleware/handlers if next is not called", func() {
+				mc := MiddlewareChain{a, stop, b}
+				run(ctx, mc, nil, handler)
+				So(ctx.Value(outputKey).([]string), ShouldResemble, []string{"a:before", "a:after"})
+			})
+		})
+
+		Convey("ServeHTTP", func() {
+			ts := httptest.NewServer(r)
+			client := &http.Client{}
+			a := func(c *Context, next Handler) {
+				c.Context = appendValue(c, outputKey, "a:before")
+				next(c)
+				c.Context = appendValue(c, outputKey, "a:after")
+				io.WriteString(c, strings.Join(c.Value(outputKey).([]string), ","))
+			}
+
+			Convey("Should execute middleware registered in Use and Handle in order", func() {
+				r.Use(MiddlewareChain{a})
+				r.GET("/ab", MiddlewareChain{b}, handler)
+				res, err := client.Get(ts.URL + "/ab")
+				So(err, ShouldBeNil)
+				defer res.Body.Close()
+				So(res.StatusCode, ShouldEqual, http.StatusOK)
+				p, err := ioutil.ReadAll(res.Body)
+				So(err, ShouldBeNil)
+				So(string(p), ShouldEqual, strings.Join(
+					[]string{"a:before", "b:before", "handler", "b:after", "a:after"},
+					",",
+				))
+			})
+
+			Convey("Should return method not allowed for existing path but wrong method in request", func() {
+				r.POST("/comment", nil, handler)
+				res, err := client.Get(ts.URL + "/comment")
+				So(err, ShouldBeNil)
+				defer res.Body.Close()
+				So(res.StatusCode, ShouldEqual, http.StatusMethodNotAllowed)
+			})
+
+			Convey("Should return expected response from handler", func() {
+				handler := func(c *Context) {
+					c.Write([]byte("Hello, " + c.Params[0].Value))
+				}
+				r.GET("/hello/:name", MiddlewareChain{c, d}, handler)
+				res, err := client.Get(ts.URL + "/hello/世界")
+				So(err, ShouldBeNil)
+				defer res.Body.Close()
+				So(res.StatusCode, ShouldEqual, http.StatusOK)
+				p, err := ioutil.ReadAll(res.Body)
+				So(err, ShouldBeNil)
+				So(string(p), ShouldEqual, "Hello, 世界")
+			})
+
+			Reset(func() {
+				ts.Close()
+			})
+		})
+
+		Convey("makeBasePath", func() {
+			cases := []struct{ base, relative, result string }{
+				{"/", "", "/"},
+				{"", "", "/"},
+				{"/foo/", "bar/", "/foo/bar/"},
+				{"/foo/", "bar//", "/foo/bar/"},
+				{"/foo/", "bar", "/foo/bar/"},
+				{"/foo", "//bar", "/foo/bar/"},
+				{"/foo//", "//bar///baz/qux", "/foo/bar/baz/qux/"},
+			}
+			for _, c := range cases {
+				So(makeBasePath(c.base, c.relative), ShouldEqual, c.result)
+			}
+		})
+
+		Convey("StatusCode() and Written()", func() {
+			ts := httptest.NewServer(r)
+			client := &http.Client{}
+
+			Convey("Should change after calls to Write", func(g C) {
+				a := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeFalse)
+					g.So(c.StatusCode(), ShouldEqual, 0)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+				}
+				b := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeFalse)
+					g.So(c.StatusCode(), ShouldEqual, 0)
+					io.WriteString(c, "foo")
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+				}
+				c := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+				}
+				handler := func(c *Context) {
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusOK)
+				}
+				r.Use(MiddlewareChain{a, b, c})
+				r.GET("/test/write", nil, handler)
+				res, err := client.Get(ts.URL + "/test/write")
+				g.So(err, ShouldBeNil)
+				res.Body.Close()
+			})
+
+			Convey("Should change after calls to WriteHeader", func(g C) {
+				a := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeFalse)
+					g.So(c.StatusCode(), ShouldEqual, 0)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+				}
+				b := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeFalse)
+					g.So(c.StatusCode(), ShouldEqual, 0)
+					c.WriteHeader(http.StatusTeapot)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+				}
+				c := func(c *Context, next Handler) {
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+					next(c)
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+				}
+				handler := func(c *Context) {
+					g.So(c.Written(), ShouldBeTrue)
+					g.So(c.StatusCode(), ShouldEqual, http.StatusTeapot)
+				}
+				r.Use(MiddlewareChain{a, b, c})
+				r.GET("/test/writeheader", nil, handler)
+				res, err := client.Get(ts.URL + "/test/writeheader")
+				g.So(err, ShouldBeNil)
+				res.Body.Close()
+			})
+
+			Reset(func() {
+				ts.Close()
+			})
+		})
+	})
+}