Milo Refactor: Remove theme support

milo/appengine/settings/middleware.go

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