Clean up template locale usage (#27856)

After many refactoring PRs for the "locale" and "template context
function", now the ".locale" is not needed for web templates any more.

This PR does a clean up for:

1. Remove `ctx.Data["locale"]` for web context.
2. Use `ctx.Locale` in `500.tmpl`, for consistency.
3. Add a test check for `500 page` locale usage.
4. Remove the `Str2html` and `DotEscape` from mail template context
data, they are copy&paste errors introduced by #19169 and #16200 . These
functions are template functions (provided by the common renderer), but
not template data variables.
5. Make email `SendAsync` function mockable (I was planning to add more
tests but it would make this PR much too complex, so the tests could be
done in another PR)
This commit is contained in:
wxiaoguang 2023-10-31 22:11:48 +08:00 committed by GitHub
parent 16d15ce087
commit a4b242ae7a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 33 additions and 65 deletions

View File

@ -157,7 +157,6 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this
ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI() ctx.Data["CurrentURL"] = setting.AppSubURL + req.URL.RequestURI()
ctx.Data["Link"] = ctx.Link ctx.Data["Link"] = ctx.Link
ctx.Data["locale"] = ctx.Locale
// PageData is passed by reference, and it will be rendered to `window.config.pageData` in `head.tmpl` for JavaScript modules // PageData is passed by reference, and it will be rendered to `window.config.pageData` in `head.tmpl` for JavaScript modules
ctx.PageData = map[string]any{} ctx.PageData = map[string]any{}

View File

@ -9,6 +9,7 @@ import (
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/httpcache"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
@ -35,20 +36,18 @@ func RenderPanicErrorPage(w http.ResponseWriter, req *http.Request, err any) {
httpcache.SetCacheControlInHeader(w.Header(), 0, "no-transform") httpcache.SetCacheControlInHeader(w.Header(), 0, "no-transform")
w.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions) w.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
data := middleware.GetContextData(req.Context()) tmplCtx := context.TemplateContext{}
if data["locale"] == nil { tmplCtx["Locale"] = middleware.Locale(w, req)
data = middleware.CommonTemplateContextData() ctxData := middleware.GetContextData(req.Context())
data["locale"] = middleware.Locale(w, req)
}
// This recovery handler could be called without Gitea's web context, so we shouldn't touch that context too much. // This recovery handler could be called without Gitea's web context, so we shouldn't touch that context too much.
// Otherwise, the 500-page may cause new panics, eg: cache.GetContextWithData, it makes the developer&users couldn't find the original panic. // Otherwise, the 500-page may cause new panics, eg: cache.GetContextWithData, it makes the developer&users couldn't find the original panic.
user, _ := data[middleware.ContextDataKeySignedUser].(*user_model.User) user, _ := ctxData[middleware.ContextDataKeySignedUser].(*user_model.User)
if !setting.IsProd || (user != nil && user.IsAdmin) { if !setting.IsProd || (user != nil && user.IsAdmin) {
data["ErrorMsg"] = "PANIC: " + combinedErr ctxData["ErrorMsg"] = "PANIC: " + combinedErr
} }
err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), data, nil) err = templates.HTMLRenderer().HTML(w, http.StatusInternalServerError, string(tplStatus500), ctxData, tmplCtx)
if err != nil { if err != nil {
log.Error("Error occurs again when rendering error page: %v", err) log.Error("Error occurs again when rendering error page: %v", err)
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)

View File

@ -26,6 +26,7 @@ func TestRenderPanicErrorPage(t *testing.T) {
respContent := w.Body.String() respContent := w.Body.String()
assert.Contains(t, respContent, `class="page-content status-page-500"`) assert.Contains(t, respContent, `class="page-content status-page-500"`)
assert.Contains(t, respContent, `</html>`) assert.Contains(t, respContent, `</html>`)
assert.Contains(t, respContent, `lang="en-US"`) // make sure the locale work
// the 500 page doesn't have normal pages footer, it makes it easier to distinguish a normal page and a failed page. // the 500 page doesn't have normal pages footer, it makes it easier to distinguish a normal page and a failed page.
// especially when a sub-template causes page error, the HTTP response code is still 200, // especially when a sub-template causes page error, the HTTP response code is still 200,

View File

@ -26,7 +26,6 @@ import (
"code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation"
incoming_payload "code.gitea.io/gitea/services/mailer/incoming/payload" incoming_payload "code.gitea.io/gitea/services/mailer/incoming/payload"
@ -68,15 +67,12 @@ func SendTestMail(email string) error {
func sendUserMail(language string, u *user_model.User, tpl base.TplName, code, subject, info string) { func sendUserMail(language string, u *user_model.User, tpl base.TplName, code, subject, info string) {
locale := translation.NewLocale(language) locale := translation.NewLocale(language)
data := map[string]any{ data := map[string]any{
"locale": locale,
"DisplayName": u.DisplayName(), "DisplayName": u.DisplayName(),
"ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale),
"ResetPwdCodeLives": timeutil.MinutesToFriendly(setting.Service.ResetPwdCodeLives, locale), "ResetPwdCodeLives": timeutil.MinutesToFriendly(setting.Service.ResetPwdCodeLives, locale),
"Code": code, "Code": code,
"Language": locale.Language(), "Language": locale.Language(),
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var content bytes.Buffer var content bytes.Buffer
@ -119,15 +115,12 @@ func SendActivateEmailMail(u *user_model.User, email *user_model.EmailAddress) {
} }
locale := translation.NewLocale(u.Language) locale := translation.NewLocale(u.Language)
data := map[string]any{ data := map[string]any{
"locale": locale,
"DisplayName": u.DisplayName(), "DisplayName": u.DisplayName(),
"ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale), "ActiveCodeLives": timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, locale),
"Code": u.GenerateEmailActivateCode(email.Email), "Code": u.GenerateEmailActivateCode(email.Email),
"Email": email.Email, "Email": email.Email,
"Language": locale.Language(), "Language": locale.Language(),
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var content bytes.Buffer var content bytes.Buffer
@ -152,13 +145,10 @@ func SendRegisterNotifyMail(u *user_model.User) {
locale := translation.NewLocale(u.Language) locale := translation.NewLocale(u.Language)
data := map[string]any{ data := map[string]any{
"locale": locale,
"DisplayName": u.DisplayName(), "DisplayName": u.DisplayName(),
"Username": u.Name, "Username": u.Name,
"Language": locale.Language(), "Language": locale.Language(),
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var content bytes.Buffer var content bytes.Buffer
@ -185,14 +175,11 @@ func SendCollaboratorMail(u, doer *user_model.User, repo *repo_model.Repository)
subject := locale.Tr("mail.repo.collaborator.added.subject", doer.DisplayName(), repoName) subject := locale.Tr("mail.repo.collaborator.added.subject", doer.DisplayName(), repoName)
data := map[string]any{ data := map[string]any{
"locale": locale,
"Subject": subject, "Subject": subject,
"RepoName": repoName, "RepoName": repoName,
"Link": repo.HTMLURL(), "Link": repo.HTMLURL(),
"Language": locale.Language(), "Language": locale.Language(),
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var content bytes.Buffer var content bytes.Buffer
@ -259,6 +246,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
locale := translation.NewLocale(lang) locale := translation.NewLocale(lang)
mailMeta := map[string]any{ mailMeta := map[string]any{
"locale": locale,
"FallbackSubject": fallback, "FallbackSubject": fallback,
"Body": body, "Body": body,
"Link": link, "Link": link,
@ -275,10 +263,6 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
"ReviewComments": reviewComments, "ReviewComments": reviewComments,
"Language": locale.Language(), "Language": locale.Language(),
"CanReply": setting.IncomingEmail.Enabled && commentType != issues_model.CommentTypePullRequestPush, "CanReply": setting.IncomingEmail.Enabled && commentType != issues_model.CommentTypePullRequestPush,
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var mailSubject bytes.Buffer var mailSubject bytes.Buffer
@ -469,7 +453,7 @@ func SendIssueAssignedMail(ctx context.Context, issue *issues_model.Issue, doer
if err != nil { if err != nil {
return err return err
} }
SendAsyncs(msgs) SendAsync(msgs...)
} }
return nil return nil
} }

View File

@ -162,7 +162,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*user_model.User, vi
if err != nil { if err != nil {
return err return err
} }
SendAsyncs(msgs) SendAsync(msgs...)
receivers = receivers[:i] receivers = receivers[:i]
} }
} }

View File

@ -14,7 +14,6 @@ import (
"code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation"
) )
@ -69,13 +68,10 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo
subject := locale.Tr("mail.release.new.subject", rel.TagName, rel.Repo.FullName()) subject := locale.Tr("mail.release.new.subject", rel.TagName, rel.Repo.FullName())
mailMeta := map[string]any{ mailMeta := map[string]any{
"locale": locale,
"Release": rel, "Release": rel,
"Subject": subject, "Subject": subject,
"Language": locale.Language(), "Language": locale.Language(),
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var mailBody bytes.Buffer var mailBody bytes.Buffer
@ -95,5 +91,5 @@ func mailNewRelease(ctx context.Context, lang string, tos []string, rel *repo_mo
msgs = append(msgs, msg) msgs = append(msgs, msg)
} }
SendAsyncs(msgs) SendAsync(msgs...)
} }

View File

@ -12,7 +12,6 @@ import (
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation"
) )
@ -65,6 +64,7 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U
} }
data := map[string]any{ data := map[string]any{
"locale": locale,
"Doer": doer, "Doer": doer,
"User": repo.Owner, "User": repo.Owner,
"Repo": repo.FullName(), "Repo": repo.FullName(),
@ -72,10 +72,6 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U
"Subject": subject, "Subject": subject,
"Language": locale.Language(), "Language": locale.Language(),
"Destination": destination, "Destination": destination,
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
if err := bodyTemplates.ExecuteTemplate(&content, string(mailRepoTransferNotify), data); err != nil { if err := bodyTemplates.ExecuteTemplate(&content, string(mailRepoTransferNotify), data); err != nil {

View File

@ -14,7 +14,6 @@ import (
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/translation"
) )
@ -53,16 +52,13 @@ func MailTeamInvite(ctx context.Context, inviter *user_model.User, team *org_mod
subject := locale.Tr("mail.team_invite.subject", inviter.DisplayName(), org.DisplayName()) subject := locale.Tr("mail.team_invite.subject", inviter.DisplayName(), org.DisplayName())
mailMeta := map[string]any{ mailMeta := map[string]any{
"locale": locale,
"Inviter": inviter, "Inviter": inviter,
"Organization": org, "Organization": org,
"Team": team, "Team": team,
"Invite": invite, "Invite": invite,
"Subject": subject, "Subject": subject,
"InviteURL": inviteURL, "InviteURL": inviteURL,
// helper
"locale": locale,
"Str2html": templates.Str2html,
"DotEscape": templates.DotEscape,
} }
var mailBody bytes.Buffer var mailBody bytes.Buffer

View File

@ -425,15 +425,12 @@ func NewContext(ctx context.Context) {
go graceful.GetManager().RunWithCancel(mailQueue) go graceful.GetManager().RunWithCancel(mailQueue)
} }
// SendAsync send mail asynchronously // SendAsync send emails asynchronously (make it mockable)
func SendAsync(msg *Message) { var SendAsync = sendAsync
SendAsyncs([]*Message{msg})
}
// SendAsyncs send mails asynchronously func sendAsync(msgs ...*Message) {
func SendAsyncs(msgs []*Message) {
if setting.MailService == nil { if setting.MailService == nil {
log.Error("Mailer: SendAsyncs is being invoked but mail service hasn't been initialized") log.Error("Mailer: SendAsync is being invoked but mail service hasn't been initialized")
return return
} }

View File

@ -1,12 +1,12 @@
{{/* This page should only depend the minimal template functions/variables, to avoid triggering new panics. {{/* This page should only depend the minimal template functions/variables, to avoid triggering new panics.
* base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, ThemeName, Str2html * base template functions: AppName, AssetUrlPrefix, AssetVersion, AppSubUrl, ThemeName, Str2html
* locale * ctx.Locale
* Flash * .Flash
* ErrorMsg * .ErrorMsg
* SignedUser (optional) * .SignedUser (optional)
*/}} */}}
<!DOCTYPE html> <!DOCTYPE html>
<html lang="{{.locale.Lang}}" data-theme="{{ThemeName .SignedUser}}"> <html lang="{{ctx.Locale.Lang}}" data-theme="{{ThemeName .SignedUser}}">
<head> <head>
<meta name="viewport" content="width=device-width, initial-scale=1"> <meta name="viewport" content="width=device-width, initial-scale=1">
<title>Internal Server Error - {{AppName}}</title> <title>Internal Server Error - {{AppName}}</title>
@ -19,8 +19,8 @@
<nav class="ui secondary menu gt-border-secondary-bottom"> <nav class="ui secondary menu gt-border-secondary-bottom">
<div class="ui container gt-df"> <div class="ui container gt-df">
<div class="item gt-f1"> <div class="item gt-f1">
<a href="{{AppSubUrl}}/" aria-label="{{.locale.Tr "home"}}"> <a href="{{AppSubUrl}}/" aria-label="{{ctx.Locale.Tr "home"}}">
<img width="30" height="30" src="{{AssetUrlPrefix}}/img/logo.svg" alt="{{.locale.Tr "logo"}}" aria-hidden="true"> <img width="30" height="30" src="{{AssetUrlPrefix}}/img/logo.svg" alt="{{ctx.Locale.Tr "logo"}}" aria-hidden="true">
</a> </a>
</div> </div>
<div class="item"> <div class="item">
@ -37,12 +37,12 @@
<div class="divider"></div> <div class="divider"></div>
<div class="ui container gt-my-5"> <div class="ui container gt-my-5">
{{if .ErrorMsg}} {{if .ErrorMsg}}
<p>{{.locale.Tr "error.occurred"}}:</p> <p>{{ctx.Locale.Tr "error.occurred"}}:</p>
<pre class="gt-whitespace-pre-wrap gt-break-all">{{.ErrorMsg}}</pre> <pre class="gt-whitespace-pre-wrap gt-break-all">{{.ErrorMsg}}</pre>
{{end}} {{end}}
<div class="center gt-mt-5"> <div class="center gt-mt-5">
{{if or .SignedUser.IsAdmin .ShowFooterVersion}}<p>{{.locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}} {{if or .SignedUser.IsAdmin .ShowFooterVersion}}<p>{{ctx.Locale.Tr "admin.config.app_ver"}}: {{AppVer}}</p>{{end}}
{{if .SignedUser.IsAdmin}}<p>{{.locale.Tr "error.report_message" | Str2html}}</p>{{end}} {{if .SignedUser.IsAdmin}}<p>{{ctx.Locale.Tr "error.report_message" | Str2html}}</p>{{end}}
</div> </div>
</div> </div>
</div> </div>