From 69130532239ee7ade82977f456c12826e1adeb1e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 8 Aug 2023 09:22:47 +0800 Subject: [PATCH] Start using template context function (#26254) Before: * `{{.locale.Tr ...}}` * `{{$.locale.Tr ...}}` * `{{$.root.locale.Tr ...}}` * `{{template "sub" .}}` * `{{template "sub" (dict "locale" $.locale)}}` * `{{template "sub" (dict "root" $)}}` * ..... With context function: only need to `{{ctx.Locale.Tr ...}}` The "ctx" could be considered as a super-global variable for all templates including sub-templates. To avoid potential risks (any bug in the template context function package), this PR only starts using "ctx" in "head.tmpl" and "footer.tmpl" and it has a "DataRaceCheck". If there is anything wrong, the code can be fixed or reverted easily. --- modules/context/context.go | 15 +++++++-- modules/context/context_response.go | 4 +-- modules/context/context_template.go | 49 +++++++++++++++++++++++++++++ modules/templates/helper.go | 2 ++ modules/templates/htmlrenderer.go | 12 ++++--- modules/test/context_tests.go | 4 +-- routers/common/errpage.go | 2 +- routers/install/install.go | 4 +++ routers/web/auth/oauth.go | 2 +- routers/web/swagger_json.go | 2 +- templates/base/footer.tmpl | 4 ++- templates/base/head.tmpl | 13 ++++---- 12 files changed, 91 insertions(+), 22 deletions(-) create mode 100644 modules/context/context_template.go diff --git a/modules/context/context.go b/modules/context/context.go index de0518a1d2..b75ba9ab67 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -5,6 +5,7 @@ package context import ( + "context" "html" "html/template" "io" @@ -31,14 +32,16 @@ import ( // Render represents a template render type Render interface { - TemplateLookup(tmpl string) (templates.TemplateExecutor, error) - HTML(w io.Writer, status int, name string, data any) error + TemplateLookup(tmpl string, templateCtx context.Context) (templates.TemplateExecutor, error) + HTML(w io.Writer, status int, name string, data any, templateCtx context.Context) error } // Context represents context of a request. type Context struct { *Base + TemplateContext TemplateContext + Render Render PageData map[string]any // data used by JavaScript modules in one page, it's `window.config.pageData` @@ -60,6 +63,8 @@ type Context struct { Package *Package } +type TemplateContext map[string]any + func init() { web.RegisterResponseStatusProvider[*Context](func(req *http.Request) web_types.ResponseStatusProvider { return req.Context().Value(WebContextKey).(*Context) @@ -133,8 +138,12 @@ func Contexter() func(next http.Handler) http.Handler { } defer baseCleanUp() + // TODO: "install.go" also shares the same logic, which should be refactored to a general function + ctx.TemplateContext = NewTemplateContext(ctx) + ctx.TemplateContext["Locale"] = ctx.Locale + ctx.Data.MergeFrom(middleware.CommonTemplateContextData()) - ctx.Data["Context"] = &ctx + ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI() ctx.Data["Link"] = ctx.Link ctx.Data["locale"] = ctx.Locale diff --git a/modules/context/context_response.go b/modules/context/context_response.go index 9dc6d1fc0e..5729865561 100644 --- a/modules/context/context_response.go +++ b/modules/context/context_response.go @@ -75,7 +75,7 @@ func (ctx *Context) HTML(status int, name base.TplName) { return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms" } - err := ctx.Render.HTML(ctx.Resp, status, string(name), ctx.Data) + err := ctx.Render.HTML(ctx.Resp, status, string(name), ctx.Data, ctx.TemplateContext) if err == nil { return } @@ -93,7 +93,7 @@ func (ctx *Context) HTML(status int, name base.TplName) { // RenderToString renders the template content to a string func (ctx *Context) RenderToString(name base.TplName, data map[string]any) (string, error) { var buf strings.Builder - err := ctx.Render.HTML(&buf, http.StatusOK, string(name), data) + err := ctx.Render.HTML(&buf, http.StatusOK, string(name), data, ctx.TemplateContext) return buf.String(), err } diff --git a/modules/context/context_template.go b/modules/context/context_template.go new file mode 100644 index 0000000000..ba90fc170a --- /dev/null +++ b/modules/context/context_template.go @@ -0,0 +1,49 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package context + +import ( + "context" + "errors" + "time" + + "code.gitea.io/gitea/modules/log" +) + +var _ context.Context = TemplateContext(nil) + +func NewTemplateContext(ctx context.Context) TemplateContext { + return TemplateContext{"_ctx": ctx} +} + +func (c TemplateContext) parentContext() context.Context { + return c["_ctx"].(context.Context) +} + +func (c TemplateContext) Deadline() (deadline time.Time, ok bool) { + return c.parentContext().Deadline() +} + +func (c TemplateContext) Done() <-chan struct{} { + return c.parentContext().Done() +} + +func (c TemplateContext) Err() error { + return c.parentContext().Err() +} + +func (c TemplateContext) Value(key any) any { + return c.parentContext().Value(key) +} + +// DataRaceCheck checks whether the template context function "ctx()" returns the consistent context +// as the current template's rendering context (request context), to help to find data race issues as early as possible. +// When the code is proven to be correct and stable, this function should be removed. +func (c TemplateContext) DataRaceCheck(dataCtx context.Context) (string, error) { + if c.parentContext() != dataCtx { + log.Error("TemplateContext.DataRaceCheck: parent context mismatch\n%s", log.Stack(2)) + return "", errors.New("parent context mismatch") + } + return "", nil +} diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 2b918f42c0..cfcfbbed38 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -28,6 +28,8 @@ import ( // NewFuncMap returns functions for injecting to templates func NewFuncMap() template.FuncMap { return map[string]any{ + "ctx": func() any { return nil }, // template context function + "DumpVar": dumpVar, // ----------------------------------------------------------------- diff --git a/modules/templates/htmlrenderer.go b/modules/templates/htmlrenderer.go index d470435b63..5ab46cb13a 100644 --- a/modules/templates/htmlrenderer.go +++ b/modules/templates/htmlrenderer.go @@ -6,6 +6,7 @@ package templates import ( "bufio" "bytes" + "context" "errors" "fmt" "io" @@ -39,27 +40,28 @@ var ( var ErrTemplateNotInitialized = errors.New("template system is not initialized, check your log for errors") -func (h *HTMLRender) HTML(w io.Writer, status int, name string, data any) error { +func (h *HTMLRender) HTML(w io.Writer, status int, name string, data any, ctx context.Context) error { //nolint:revive if respWriter, ok := w.(http.ResponseWriter); ok { if respWriter.Header().Get("Content-Type") == "" { respWriter.Header().Set("Content-Type", "text/html; charset=utf-8") } respWriter.WriteHeader(status) } - t, err := h.TemplateLookup(name) + t, err := h.TemplateLookup(name, ctx) if err != nil { return texttemplate.ExecError{Name: name, Err: err} } return t.Execute(w, data) } -func (h *HTMLRender) TemplateLookup(name string) (TemplateExecutor, error) { +func (h *HTMLRender) TemplateLookup(name string, ctx context.Context) (TemplateExecutor, error) { //nolint:revive tmpls := h.templates.Load() if tmpls == nil { return nil, ErrTemplateNotInitialized } - - return tmpls.Executor(name, NewFuncMap()) + m := NewFuncMap() + m["ctx"] = func() any { return ctx } + return tmpls.Executor(name, m) } func (h *HTMLRender) CompileTemplates() error { diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go index 9e7095e116..92d7f8b22b 100644 --- a/modules/test/context_tests.go +++ b/modules/test/context_tests.go @@ -150,11 +150,11 @@ func LoadGitRepo(t *testing.T, ctx *context.Context) { type mockRender struct{} -func (tr *mockRender) TemplateLookup(tmpl string) (templates.TemplateExecutor, error) { +func (tr *mockRender) TemplateLookup(tmpl string, _ gocontext.Context) (templates.TemplateExecutor, error) { return nil, nil } -func (tr *mockRender) HTML(w io.Writer, status int, _ string, _ any) error { +func (tr *mockRender) HTML(w io.Writer, status int, _ string, _ any, _ gocontext.Context) error { if resp, ok := w.(http.ResponseWriter); ok { resp.WriteHeader(status) } diff --git a/routers/common/errpage.go b/routers/common/errpage.go index 3d82c96deb..9c8ccc3388 100644 --- a/routers/common/errpage.go +++ b/routers/common/errpage.go @@ -48,7 +48,7 @@ func RenderPanicErrorPage(w http.ResponseWriter, req *http.Request, err any) { data["ErrorMsg"] = "PANIC: " + combinedErr } - err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), data) + err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), data, nil) if err != nil { log.Error("Error occurs again when rendering error page: %v", err) w.WriteHeader(http.StatusInternalServerError) diff --git a/routers/install/install.go b/routers/install/install.go index 6a8f561271..aa9c0f4986 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -68,9 +68,13 @@ func Contexter() func(next http.Handler) http.Handler { } defer baseCleanUp() + ctx.TemplateContext = context.NewTemplateContext(ctx) + ctx.TemplateContext["Locale"] = ctx.Locale + ctx.AppendContextValue(context.WebContextKey, ctx) ctx.Data.MergeFrom(middleware.CommonTemplateContextData()) ctx.Data.MergeFrom(middleware.ContextData{ + "Context": ctx, // TODO: use "ctx" in template and remove this "locale": ctx.Locale, "Title": ctx.Locale.Tr("install.install"), "PageIsInstall": true, diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 3c367b3d27..78dc84472a 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -578,7 +578,7 @@ func GrantApplicationOAuth(ctx *context.Context) { // OIDCWellKnown generates JSON so OIDC clients know Gitea's capabilities func OIDCWellKnown(ctx *context.Context) { - t, err := ctx.Render.TemplateLookup("user/auth/oidc_wellknown") + t, err := ctx.Render.TemplateLookup("user/auth/oidc_wellknown", nil) if err != nil { ctx.ServerError("unable to find template", err) return diff --git a/routers/web/swagger_json.go b/routers/web/swagger_json.go index 1844b90d95..493c97aa67 100644 --- a/routers/web/swagger_json.go +++ b/routers/web/swagger_json.go @@ -13,7 +13,7 @@ const tplSwaggerV1Json base.TplName = "swagger/v1_json" // SwaggerV1Json render swagger v1 json func SwaggerV1Json(ctx *context.Context) { - t, err := ctx.Render.TemplateLookup(string(tplSwaggerV1Json)) + t, err := ctx.Render.TemplateLookup(string(tplSwaggerV1Json), nil) if err != nil { ctx.ServerError("unable to find template", err) return diff --git a/templates/base/footer.tmpl b/templates/base/footer.tmpl index e3cac806a4..31c669a921 100644 --- a/templates/base/footer.tmpl +++ b/templates/base/footer.tmpl @@ -26,6 +26,8 @@ {{end}} {{end}} -{{template "custom/footer" .}} + + {{template "custom/footer" .}} + {{ctx.DataRaceCheck $.Context}} diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index 4858d0b7de..8eebaebd70 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -1,5 +1,5 @@ - + {{if .Title}}{{.Title | RenderEmojiPlain}} - {{end}}{{if .Repository.Name}}{{.Repository.Name}} - {{end}}{{AppName}} @@ -28,7 +28,7 @@ {{if .PageIsUserProfile}} - + {{if .ContextUser.Description}} @@ -48,10 +48,10 @@ {{end}} {{end}} - {{if (.Repository.AvatarLink $.Context)}} - + {{if (.Repository.AvatarLink ctx)}} + {{else}} - + {{end}} {{else}} @@ -65,10 +65,11 @@ {{template "custom/header" .}} + {{ctx.DataRaceCheck $.Context}} {{template "custom/body_outer_pre" .}}
- + {{template "custom/body_inner_pre" .}}