Make HTTP middleware easier to use

appengine/cmd/cron/frontend/handler.go

 // Copyright 2015 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 frontend implements GAE web server for luci-cron service.
 //
 // Due to the way classic GAE imports work, this package can not have
 // subpackages (or at least subpackages referenced via absolute import path).
 // We can't use relative imports because luci-go will then become unbuildable
 // by regular (non GAE) toolset.
@@ skipping 14 matching lines @@
         "time"
 
         "github.com/julienschmidt/httprouter"
         "golang.org/x/net/context"
         "google.golang.org/appengine"
 
         "github.com/luci/gae/service/info"
         "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"
         "github.com/luci/luci-go/appengine/gaemiddleware"
 
         "github.com/luci/luci-go/common/config"
         "github.com/luci/luci-go/common/config/impl/memory"
         "github.com/luci/luci-go/common/errors"
         "github.com/luci/luci-go/common/logging"
 
@@ skipping 79 matching lines @@
 // getConfigImpl returns config.Interface implementation to use from the
 // catalog.
 func getConfigImpl(c context.Context) (config.Interface, error) {
         // Use fake config data on dev server for simplicity.
         if info.Get(c).IsDevAppServer() {
                 return memory.New(devServerConfigs()), nil
         }
         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.
 
 func init() {
         // Dev server likes to restart a lot, and upon a restart math/rand seed is
         // always set to 1, resulting in lots of presumably "random" IDs not being
         // very random. Seed it with real randomness.
         var seed int64
         if err := binary.Read(cryptorand.Reader, binary.LittleEndian, &seed); err != nil {
@@ skipping 11 matching lines @@
         globalEngine = engine.NewEngine(engine.Config{
                 Catalog:              globalCatalog,
                 TimersQueuePath:      "/internal/tasks/timers",
                 TimersQueueName:      "timers",
                 InvocationsQueuePath: "/internal/tasks/invocations",
                 InvocationsQueueName: "invocations",
                 PubSubPushPath:       "/pubsub",
         })
 
         // 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)
 }
 
 // warmupHandler warms in-memory caches.
 func warmupHandler(rc *requestContext) {
         if err := server.Warmup(rc); err != nil {
                 rc.fail(500, "Failed to warmup OpenID: %s", err)
                 return
@@ skipping 105 matching lines @@
                 return
         }
         count, _ := strconv.Atoi(c.r.Header.Get("X-AppEngine-TaskExecutionCount"))
         err = globalEngine.ExecuteSerializedAction(c.Context, body, count)
         if err != nil {
                 c.err(err, "Error when executing the action")
                 return
         }
         c.ok()
 }