Milo Refactor: Remove theme support

milo/appengine/settings/middleware.go

Index: milo/appengine/settings/middleware.go
diff --git a/milo/appengine/settings/middleware.go b/milo/appengine/settings/middleware.go
new file mode 100644
index 0000000000000000000000000000000000000000..cfb6bbd0475d4e95a6e55baaeb899c0e3fd69ac3
--- /dev/null
+++ b/milo/appengine/settings/middleware.go
@@ -0,0 +1,121 @@
+// Copyright 2015 The LUCI Authors. All rights reserved.

[hinoka, 2017/03/15 18:51:16]

This was settings.go, but renamed and trimmed down.

[nodir, 2017/03/16 17:42:51]

why is this generic file in settings package? i'd expect common functions to be
a in a common package

[hinoka, 2017/03/16 21:04:06]

ack, splitting settings into config and common

[hinoka, 2017/03/17 20:00:21]

Actually theres an import loop this way, just renaming this to common

+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package settings
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+
+	"github.com/julienschmidt/httprouter"
+	"github.com/luci/gae/service/info"
+	"github.com/luci/luci-go/appengine/gaeauth/server"
+	"github.com/luci/luci-go/appengine/gaemiddleware"
+	"github.com/luci/luci-go/common/clock"
+	"github.com/luci/luci-go/milo/common/miloerror"
+	"github.com/luci/luci-go/server/analytics"
+	"github.com/luci/luci-go/server/auth"
+	"github.com/luci/luci-go/server/router"
+	"github.com/luci/luci-go/server/templates"
+	"golang.org/x/net/context"

[nodir, 2017/03/16 17:42:51]

put 3p imports before 1p, in a separate section
also applies to httprouter

[hinoka, 2017/03/17 20:00:21]

Done.

+)
+
+type Handler func(c context.Context, r *http.Request, p httprouter.Params) (*templates.Args, error)
+
+// getTemplateBundles is used to render HTML templates. It provides a base args
+// passed to all templates.
+func GetTemplateBundle() *templates.Bundle {
+	return &templates.Bundle{
+		Loader:          templates.FileSystemLoader("templates"),
+		DebugMode:       info.IsDevAppServer,
+		DefaultTemplate: "base",
+		DefaultArgs: func(c context.Context) (templates.Args, error) {
+			r := getRequest(c)
+			path := r.URL.Path
+			loginURL, err := auth.LoginURL(c, path)
+			if err != nil {
+				return nil, err
+			}
+			logoutURL, err := auth.LogoutURL(c, path)
+			if err != nil {
+				return nil, err
+			}
+			if err != nil {
+				return nil, err
+			}
+			return templates.Args{
+				"AppVersion":  strings.Split(info.VersionID(c), ".")[0],
+				"IsAnonymous": auth.CurrentIdentity(c) == "anonymous:anonymous",
+				"User":        auth.CurrentUser(c),
+				"LoginURL":    loginURL,
+				"LogoutURL":   logoutURL,
+				"CurrentTime": clock.Now(c),
+				"Analytics":   analytics.Snippet(c),
+				"RequestID":   info.RequestID(c),
+			}, nil
+		},
+		FuncMap: funcMap,
+	}
+}
+
+// Base returns the basic luci appengine middlewares.
+func Base() router.MiddlewareChain {
+	methods := auth.Authenticator{
+		&server.OAuth2Method{Scopes: []string{server.EmailScope}},
+		server.CookieAuth,
+		&server.InboundAppIDAuthMethod{},
+	}
+	m := gaemiddleware.BaseProd().Extend(auth.Use(methods), auth.Authenticate)
+	m = m.Extend(templates.WithTemplates(GetTemplateBundle()))

[nodir, 2017/03/16 17:42:51]

i think it is more natural to inject http request into the context before this
line. In this case you don't need WithRequest function

[hinoka, 2017/03/16 21:04:06]

You can't though... you don't get the request object until the handler is
called, which is the only reason it's in the wrap() function.

[hinoka, 2017/03/17 20:00:21]

Turns out you can, changed.

+	return m
+}
+
+// Wrap wraps a Milo Handler and returns router.Handler that writes into
+// c.Writer with a template named templateName.
+func Wrap(h Handler, templateName string) router.Handler {
+	return func(c *router.Context) {
+		// Inject the request into the context.
+		c.Context = WithRequest(c.Context, c.Request)
+
+		// Do the things.
+		args, err := h(c.Context, c.Request, c.Params)

[nodir, 2017/03/16 17:42:51]

why not pass c itself?

[hinoka, 2017/03/16 21:04:06]

It's a different type of context.  I think estaab's intern did this in a
refactor.

[hinoka, 2017/03/17 20:00:21]

Done.

+
+		// Throw errors
+		if err != nil {
+			msg := err.Error()
+			code := http.StatusInternalServerError
+			if merr, ok := err.(miloerror.Error); ok {

[nodir, 2017/03/16 17:42:51]

without templates, I don't see why do we need miloerror. Why not write http
responses directly.

[hinoka, 2017/03/16 21:04:06]

To do default error templates? Otherwise this error handling code would need to
be replicated everywhere.

[hinoka, 2017/03/17 20:00:21]

Changed error handling code to replicate everywhere

+				msg = merr.Message
+				code = merr.Code
+			}
+			c.Writer.WriteHeader(code)
+			name := "pages/error.html"
+			templates.MustRender(c.Context, c.Writer, name, templates.Args{
+				"Code":    code,
+				"Message": msg,
+			})
+			return
+		}
+
+		// Render the stuff.
+		name := fmt.Sprintf("pages/%s", templateName)
+		templates.MustRender(c.Context, c.Writer, name, *args)
+	}
+}
+
+// The context key, so that we can embed the http.Request object into
+// the context.
+var requestKey = "http.request"
+
+func WithRequest(c context.Context, r *http.Request) context.Context {
+	return context.WithValue(c, &requestKey, r)
+}
+
+func getRequest(c context.Context) *http.Request {
+	if req, ok := c.Value(&requestKey).(*http.Request); ok {
+		return req
+	}
+	panic("No http.request found in context")
+}