From 412e5c0946fc5b83456685bc2fb1aed682228f57 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Fri, 25 Aug 2023 19:07:42 +0800
Subject: [PATCH] Make web context initialize correctly for different cases
(#26726)
The web context (modules/context.Context) is quite complex, it's
difficult for the callers to initialize correctly.
This PR introduces a `NewWebContext` function, to make sure the web
context have the same behavior for different cases.
---
modules/context/context.go | 41 +++++++++++++++----------
modules/context/package.go | 6 ++--
modules/test/context_tests.go | 16 +++++-----
routers/install/install.go | 10 +-----
routers/web/repo/actions/actions.go | 2 +-
routers/web/repo/helper.go | 7 ++---
routers/web/repo/helper_test.go | 7 ++---
routers/web/repo/issue.go | 6 ++--
routers/web/repo/pull.go | 2 +-
routers/web/repo/release.go | 4 +--
services/markup/processorhelper_test.go | 3 +-
11 files changed, 50 insertions(+), 54 deletions(-)
diff --git a/modules/context/context.go b/modules/context/context.go
index 98c1a9bdb6..47ad310b09 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -107,6 +107,29 @@ func GetValidateContext(req *http.Request) (ctx *ValidateContext) {
return ctx
}
+func NewTemplateContextForWeb(ctx *Context) TemplateContext {
+ tmplCtx := NewTemplateContext(ctx)
+ tmplCtx["Locale"] = ctx.Base.Locale
+ tmplCtx["AvatarUtils"] = templates.NewAvatarUtils(ctx)
+ return tmplCtx
+}
+
+func NewWebContext(base *Base, render Render, session session.Store) *Context {
+ ctx := &Context{
+ Base: base,
+ Render: render,
+ Session: session,
+
+ Cache: mc.GetCache(),
+ Link: setting.AppSubURL + strings.TrimSuffix(base.Req.URL.EscapedPath(), "/"),
+ Repo: &Repository{PullRequest: &PullRequest{}},
+ Org: &Organization{},
+ }
+ ctx.TemplateContext = NewTemplateContextForWeb(ctx)
+ ctx.Flash = &middleware.Flash{DataStore: ctx, Values: url.Values{}}
+ return ctx
+}
+
// Contexter initializes a classic context for a request.
func Contexter() func(next http.Handler) http.Handler {
rnd := templates.HTMLRenderer()
@@ -127,21 +150,8 @@ func Contexter() func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
base, baseCleanUp := NewBaseContext(resp, req)
- ctx := &Context{
- Base: base,
- Cache: mc.GetCache(),
- Link: setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/"),
- Render: rnd,
- Session: session.GetSession(req),
- Repo: &Repository{PullRequest: &PullRequest{}},
- Org: &Organization{},
- }
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.TemplateContext["AvatarUtils"] = templates.NewAvatarUtils(ctx)
+ ctx := NewWebContext(base, rnd, session.GetSession(req))
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this
@@ -172,8 +182,7 @@ func Contexter() func(next http.Handler) http.Handler {
}
}
- // prepare an empty Flash message for current request
- ctx.Flash = &middleware.Flash{DataStore: ctx, Values: url.Values{}}
+ // if there are new messages in the ctx.Flash, write them into cookie
ctx.Resp.Before(func(resp ResponseWriter) {
if val := ctx.Flash.Encode(); val != "" {
middleware.SetSiteCookie(ctx.Resp, CookieNameFlash, val, 0)
diff --git a/modules/context/package.go b/modules/context/package.go
index be50e0a991..c0813fb2da 100644
--- a/modules/context/package.go
+++ b/modules/context/package.go
@@ -154,12 +154,10 @@ func PackageContexter() func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
base, baseCleanUp := NewBaseContext(resp, req)
- ctx := &Context{
- Base: base,
- Render: renderer, // it is still needed when rendering 500 page in a package handler
- }
defer baseCleanUp()
+ // it is still needed when rendering 500 page in a package handler
+ ctx := NewWebContext(base, renderer, nil)
ctx.Base.AppendContextValue(WebContextKey, ctx)
next.ServeHTTP(ctx.Resp, ctx.Req)
})
diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go
index 92d7f8b22b..83e6117bcf 100644
--- a/modules/test/context_tests.go
+++ b/modules/test/context_tests.go
@@ -45,14 +45,12 @@ func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.Resp
resp := httptest.NewRecorder()
req := mockRequest(t, reqPath)
base, baseCleanUp := context.NewBaseContext(resp, req)
+ _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
base.Data = middleware.GetContextData(req.Context())
base.Locale = &translation.MockLocale{}
- ctx := &context.Context{
- Base: base,
- Render: &mockRender{},
- Flash: &middleware.Flash{Values: url.Values{}},
- }
- _ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
+
+ ctx := context.NewWebContext(base, &MockRender{}, nil)
+ ctx.Flash = &middleware.Flash{Values: url.Values{}}
chiCtx := chi.NewRouteContext()
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
@@ -148,13 +146,13 @@ func LoadGitRepo(t *testing.T, ctx *context.Context) {
assert.NoError(t, err)
}
-type mockRender struct{}
+type MockRender struct{}
-func (tr *mockRender) TemplateLookup(tmpl string, _ gocontext.Context) (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, _ gocontext.Context) 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/install/install.go b/routers/install/install.go
index 99ea7b0738..66a9e1f002 100644
--- a/routers/install/install.go
+++ b/routers/install/install.go
@@ -60,17 +60,9 @@ func Contexter() func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
base, baseCleanUp := context.NewBaseContext(resp, req)
- ctx := &context.Context{
- Base: base,
- Flash: &middleware.Flash{},
- Render: rnd,
- Session: session.GetSession(req),
- }
defer baseCleanUp()
- ctx.TemplateContext = context.NewTemplateContext(ctx)
- ctx.TemplateContext["Locale"] = ctx.Locale
-
+ ctx := context.NewWebContext(base, rnd, session.GetSession(req))
ctx.AppendContextValue(context.WebContextKey, ctx)
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
ctx.Data.MergeFrom(middleware.ContextData{
diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go
index b0f4b6f897..6284d21463 100644
--- a/routers/web/repo/actions/actions.go
+++ b/routers/web/repo/actions/actions.go
@@ -191,7 +191,7 @@ func List(ctx *context.Context) {
ctx.Error(http.StatusInternalServerError, err.Error())
return
}
- ctx.Data["Actors"] = repo.MakeSelfOnTop(ctx, actors)
+ ctx.Data["Actors"] = repo.MakeSelfOnTop(ctx.Doer, actors)
ctx.Data["StatusInfoList"] = actions_model.GetStatusInfoList(ctx)
diff --git a/routers/web/repo/helper.go b/routers/web/repo/helper.go
index fb5ada1bdb..f8cdefdc8e 100644
--- a/routers/web/repo/helper.go
+++ b/routers/web/repo/helper.go
@@ -7,16 +7,15 @@ import (
"sort"
"code.gitea.io/gitea/models/user"
- "code.gitea.io/gitea/modules/context"
)
-func MakeSelfOnTop(ctx *context.Context, users []*user.User) []*user.User {
- if ctx.Doer != nil {
+func MakeSelfOnTop(doer *user.User, users []*user.User) []*user.User {
+ if doer != nil {
sort.Slice(users, func(i, j int) bool {
if users[i].ID == users[j].ID {
return false
}
- return users[i].ID == ctx.Doer.ID // if users[i] is self, put it before others, so less=true
+ return users[i].ID == doer.ID // if users[i] is self, put it before others, so less=true
})
}
return users
diff --git a/routers/web/repo/helper_test.go b/routers/web/repo/helper_test.go
index 226e2e81f4..978758e77f 100644
--- a/routers/web/repo/helper_test.go
+++ b/routers/web/repo/helper_test.go
@@ -7,21 +7,20 @@ import (
"testing"
"code.gitea.io/gitea/models/user"
- "code.gitea.io/gitea/modules/context"
"github.com/stretchr/testify/assert"
)
func TestMakeSelfOnTop(t *testing.T) {
- users := MakeSelfOnTop(&context.Context{}, []*user.User{{ID: 2}, {ID: 1}})
+ users := MakeSelfOnTop(nil, []*user.User{{ID: 2}, {ID: 1}})
assert.Len(t, users, 2)
assert.EqualValues(t, 2, users[0].ID)
- users = MakeSelfOnTop(&context.Context{Doer: &user.User{ID: 1}}, []*user.User{{ID: 2}, {ID: 1}})
+ users = MakeSelfOnTop(&user.User{ID: 1}, []*user.User{{ID: 2}, {ID: 1}})
assert.Len(t, users, 2)
assert.EqualValues(t, 1, users[0].ID)
- users = MakeSelfOnTop(&context.Context{Doer: &user.User{ID: 2}}, []*user.User{{ID: 2}, {ID: 1}})
+ users = MakeSelfOnTop(&user.User{ID: 2}, []*user.User{{ID: 2}, {ID: 1}})
assert.Len(t, users, 2)
assert.EqualValues(t, 2, users[0].ID)
}
diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go
index 9a2add1452..b8b5a2dff2 100644
--- a/routers/web/repo/issue.go
+++ b/routers/web/repo/issue.go
@@ -331,7 +331,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
ctx.ServerError("GetRepoAssignees", err)
return
}
- ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
+ ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
handleTeamMentions(ctx)
if ctx.Written() {
@@ -535,7 +535,7 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.R
ctx.ServerError("GetRepoAssignees", err)
return
}
- ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
+ ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
handleTeamMentions(ctx)
}
@@ -3625,7 +3625,7 @@ func issuePosters(ctx *context.Context, isPullList bool) {
}
}
- posters = MakeSelfOnTop(ctx, posters)
+ posters = MakeSelfOnTop(ctx.Doer, posters)
resp := &userSearchResponse{}
resp.Results = make([]*userSearchInfo, len(posters))
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index e3854779fe..e697a0d5b6 100644
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -956,7 +956,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
ctx.ServerError("GetRepoAssignees", err)
return
}
- ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
+ ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
handleTeamMentions(ctx)
if ctx.Written() {
diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 957cf56972..138df45857 100644
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -349,7 +349,7 @@ func NewRelease(ctx *context.Context) {
ctx.ServerError("GetRepoAssignees", err)
return
}
- ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
+ ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
upload.AddUploadContext(ctx, "release")
@@ -538,7 +538,7 @@ func EditRelease(ctx *context.Context) {
ctx.ServerError("GetRepoAssignees", err)
return
}
- ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
+ ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
ctx.HTML(http.StatusOK, tplReleaseNew)
}
diff --git a/services/markup/processorhelper_test.go b/services/markup/processorhelper_test.go
index 2f48e03b22..d83e10903f 100644
--- a/services/markup/processorhelper_test.go
+++ b/services/markup/processorhelper_test.go
@@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/models/user"
gitea_context "code.gitea.io/gitea/modules/context"
+ "code.gitea.io/gitea/modules/test"
"github.com/stretchr/testify/assert"
)
@@ -41,7 +42,7 @@ func TestProcessorHelper(t *testing.T) {
assert.NoError(t, err)
base, baseCleanUp := gitea_context.NewBaseContext(httptest.NewRecorder(), req)
defer baseCleanUp()
- giteaCtx := &gitea_context.Context{Base: base}
+ giteaCtx := gitea_context.NewWebContext(base, &test.MockRender{}, nil)
assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic))
assert.False(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate))