Make HTTP middleware easier to use

server/router/handler.go

+// 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)
+        }
+}