Make HTTP middleware easier to use

appengine/cmd/cron/frontend/handler.go

Index: appengine/cmd/cron/frontend/handler.go
diff --git a/appengine/cmd/cron/frontend/handler.go b/appengine/cmd/cron/frontend/handler.go
index 9846aaf48fbabcb7b12bbfefeaaf1c41080255ef..b61e7872a46b646d5c9b6a72aa72e0a64b8a5889 100644
--- a/appengine/cmd/cron/frontend/handler.go
+++ b/appengine/cmd/cron/frontend/handler.go
@@ -32,7 +32,7 @@ import (
 	"github.com/luci/gae/service/taskqueue"
 
 	"github.com/luci/luci-go/server/auth"
-	"github.com/luci/luci-go/server/middleware"
+	"github.com/luci/luci-go/server/router"
 
 	"github.com/luci/luci-go/appengine/gaeauth/server"
 	"github.com/luci/luci-go/appengine/gaeconfig"
@@ -132,38 +132,46 @@ func getConfigImpl(c context.Context) (config.Interface, error) {
 	return gaeconfig.New(c)
 }
 
-// wrap converts the handler to format accepted by middleware lib. It also adds
-// context initialization code.
-func wrap(h handler) middleware.Handler {
-	return func(c context.Context, w http.ResponseWriter, r *http.Request, p httprouter.Params) {
-		h(&requestContext{c, w, r, p})
+// wrap converts the handler to handler format accepted by router lib.
+func wrap(h handler) router.Handler {
+	return func(c *router.Context) {
+		h(&requestContext{c.Context, c.Writer, c.Request, c.Params})

[iannucci, 2016/06/13 19:23:28]

hm... at this point, can't we just use router.Handler directly instead of having
cron's `handler` type? cron's *requestContext looks almost exactly like
*router.Context now.

 	}
 }
 
 // base starts middleware chain. It initializes prod context and sets up
 // authentication config.
-func base(h middleware.Handler) httprouter.Handle {
+func base() []router.Handler {
 	methods := auth.Authenticator{
 		&server.OAuth2Method{Scopes: []string{server.EmailScope}},
 		server.CookieAuth,
 		&server.InboundAppIDAuthMethod{},
 	}
-	wrapper := func(c context.Context, w http.ResponseWriter, r *http.Request, p httprouter.Params) {
-		globalInit.Do(func() { initializeGlobalState(c) })
-		c = auth.SetAuthenticator(c, methods)
-		h(c, w, r, p)
-	}
-	return gaemiddleware.BaseProd(wrapper)
+	return append(
+		gaemiddleware.BaseProd(),
+		func(c *router.Context) {
+			globalInit.Do(func() { initializeGlobalState(c.Context) })
+			c.Context = auth.SetAuthenticator(c.Context, methods)
+		},
+	)
 }
 
 // cronHandler returns handler intended for cron jobs.
-func cronHandler(h handler) httprouter.Handle {
-	return base(gaemiddleware.RequireCron(wrap(h)))
+func cronHandler(h handler) []router.Handler {
+	return append(
+		base(),
+		gaemiddleware.RequireCron(),
+		wrap(h),
+	)
 }
 
 // taskQueueHandler returns handler intended for task queue calls.
-func taskQueueHandler(name string, h handler) httprouter.Handle {
-	return base(gaemiddleware.RequireTaskQueue(name, wrap(h)))
+func taskQueueHandler(name string, h handler) []router.Handler {
+	return append(
+		base(),
+		gaemiddleware.RequireTaskQueue(name),
+		wrap(h),
+	)
 }
 
 //// Routes.
@@ -195,26 +203,26 @@ func init() {
 	})
 
 	// Setup HTTP routes.
-	router := httprouter.New()
+	router := router.New()
 
-	gaemiddleware.InstallHandlers(router, base)
-	ui.InstallHandlers(router, base, ui.Config{
+	gaemiddleware.InstallHandlers(router, base())
+	ui.InstallHandlers(router, base(), ui.Config{
 		Engine:        globalEngine,
 		TemplatesPath: "templates",
 	})
 
-	router.GET("/_ah/warmup", base(wrap(warmupHandler)))
-	router.GET("/_ah/start", base(wrap(warmupHandler)))
-	router.POST("/pubsub", base(wrap(pubsubPushHandler)))
-	router.GET("/internal/cron/read-config", cronHandler(readConfigCron))
-	router.POST("/internal/tasks/read-project-config", taskQueueHandler("read-project-config", readProjectConfigTask))
-	router.POST("/internal/tasks/timers", taskQueueHandler("timers", actionTask))
-	router.POST("/internal/tasks/invocations", taskQueueHandler("invocations", actionTask))
+	router.GET("/_ah/warmup", append(base(), wrap(warmupHandler))...)

[iannucci, 2016/06/13 19:23:28]

er... can't this be

  router.GET("/_ah/warmup", base(), wrap(warmupHandler))

? It looks like you're doing an append and then unwrapping the result
immediately.

edit: Oh, it looks like router.GET and friends take ...router.Handler. Why don't
we make this take ...[]router.Handler instead?

+	router.GET("/_ah/start", append(base(), wrap(warmupHandler))...)
+	router.POST("/pubsub", append(base(), wrap(pubsubPushHandler))...)
+	router.GET("/internal/cron/read-config", cronHandler(readConfigCron)...)
+	router.POST("/internal/tasks/read-project-config", taskQueueHandler("read-project-config", readProjectConfigTask)...)
+	router.POST("/internal/tasks/timers", taskQueueHandler("timers", actionTask)...)
+	router.POST("/internal/tasks/invocations", taskQueueHandler("invocations", actionTask)...)
 
 	// Devserver can't accept PubSub pushes, so allow manual pulls instead to
 	// simplify local development.
 	if appengine.IsDevAppServer() {
-		router.GET("/pubsub/pull/:ManagerName/:Publisher", base(wrap(pubsubPullHandler)))
+		router.GET("/pubsub/pull/:ManagerName/:Publisher", append(base(), wrap(pubsubPullHandler))...)
 	}
 
 	http.DefaultServeMux.Handle("/", router)