Make HTTP middleware easier to use

server/router/handler.go

Index: server/router/handler.go
diff --git a/server/router/handler.go b/server/router/handler.go
new file mode 100644
index 0000000000000000000000000000000000000000..ea2bd6458c4736375dc94b781ac2ad7691b020cb
--- /dev/null
+++ b/server/router/handler.go
@@ -0,0 +1,76 @@
+// 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 "golang.org/x/net/context"
+
+type (
+	// Handler is the type for all request handlers.
+	Handler func(*Context)
+
+	// Middleware is a function that accepts a shared context and the next
+	// function. Since Middleware is typically part of a chain of functions
+	// that handles an HTTP request, it must obey the following rules.
+	//
+	// Middleware must call next if it has not written to the Context
+	// by the end of the function. Middleware must not call next
+	// if it has written to the Context. Middleware must not write
+	// to the Context after next is called and c.Written()==true. Middleware
+	// may modify the embedded context before calling next.

[iannucci, 2016/06/16 00:54:24]

let's make these bullets:

   //   - Middleware must call next if it has not written to the Context
   //     by the end of the function.
   //   - Middleware must not call next if it has written to the Context.
   //   - Middleware must not write to the Context after next is called 
   //     and c.Written()==true. 
   //   - Middleware may modify the embedded context before calling next.

Check with the godoc command-line tool to make sure this is properly formatted
in the ui (e.g. `godoc -http=:6060`, http://localhost:6060/...)

[nishanths, 2016/06/16 03:43:27]

+1 Done. With aligned dashes inside a <pre> block. (This is how the standard lib
does it too: https://golang.org/pkg/encoding/xml/#Marshal)

[nodir, 2016/06/16 04:20:25]

+1

+	Middleware func(c *Context, next Handler)
+
+	// MiddlewareChain is a list of Middleware.
+	MiddlewareChain []Middleware
+)
+
+// Run executes the middleware chain and final handler with the given context as
+// the initial context.
+func (mc MiddlewareChain) Run(c *Context, h Handler) {
+	run(c, mc, nil, h)
+}
+
+// run executes the middleware chains m and n, and the handler h using the
+// Context c as the initial context.
+func run(c *Context, m, n MiddlewareChain, h Handler) {
+	switch {
+	case len(m) > 0:
+		m[0](c, func(ctx *Context) {
+			run(ctx, m[1:], n, h)
+		})
+	case len(n) > 0:
+		n[0](c, func(ctx *Context) {
+			run(ctx, m, n[1:], h)
+		})
+	default:
+		h(c)
+	}
+}
+
+// MergeMiddlewareChains allocates a new array and copies the given middleware
+// chains into it.
+func MergeMiddlewareChains(mchains ...MiddlewareChain) (final MiddlewareChain) {

[iannucci, 2016/06/16 00:54:24]

Maybe JoinMiddlewareChains? Or ConcatMiddlewareChains? It's not really a merge
(e.g. not the 'merge' interleaving action ala
https://en.wikipedia.org/wiki/Merge_sort)

[nishanths, 2016/06/16 03:43:27]

Good point. But, removing this func as it is unused, since a few commits ago
when we added the run method. Previously it was used in adapt.

+	finalLen := 0
+	for _, mc := range mchains {
+		finalLen += len(mc)
+	}
+	if finalLen == 0 {
+		return
+	}
+
+	final = make(MiddlewareChain, 0, finalLen)
+	for _, mc := range mchains {
+		final = append(final, mc...)
+	}
+	return
+}
+
+// MiddlewareWithContext returns a Middleware that embeds the given
+// context.Context into the shared context and calls the next handler.
+func MiddlewareWithContext(c context.Context) Middleware {

[iannucci, 2016/06/16 00:54:24]

hm... this seems a bit dangerous: this /replaces/ the context with the provided
one and calls the next handler. This means if this is done in the middle of a
chain, all the previous context is lost. What about

func SetContext(c context.Context) Middleware {
  ...
}

Then when you see:

Handle("PUT", "/something", MiddlewareChain{SetContext(c), Something,
Something}, h)

it makes sense, and if you saw it in the middle, it would immediately look
suspicious

Handle("PUT", "/something", MiddlewareChain{Something, SetContext(c),
Something}, h)

wdyt?

[nishanths, 2016/06/16 03:43:27]

Yep, SetContext is a far better name. 
Nevertheless, I am removing this function because, as Nodir noted, there are not
many places it will be used.

+	return func(ctx *Context, next Handler) {
+		ctx.Context = c
+		next(ctx)
+	}
+}