Skip to content

Commit 412e5c0

Browse files
wxiaoguangwxiaoguang
authored
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.
1 parent ee9e83b commit 412e5c0

File tree

11 files changed

+50
-54
lines changed

11 files changed

+50
-54
lines changed

‎modules/context/context.go

+25-16
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,29 @@ func GetValidateContext(req *http.Request) (ctx *ValidateContext) {
107107
return ctx
108108
}
109109

110+
func NewTemplateContextForWeb(ctx *Context) TemplateContext {
111+
tmplCtx := NewTemplateContext(ctx)
112+
tmplCtx["Locale"] = ctx.Base.Locale
113+
tmplCtx["AvatarUtils"] = templates.NewAvatarUtils(ctx)
114+
return tmplCtx
115+
}
116+
117+
func NewWebContext(base *Base, render Render, session session.Store) *Context {
118+
ctx := &Context{
119+
Base: base,
120+
Render: render,
121+
Session: session,
122+
123+
Cache: mc.GetCache(),
124+
Link: setting.AppSubURL + strings.TrimSuffix(base.Req.URL.EscapedPath(), "/"),
125+
Repo: &Repository{PullRequest: &PullRequest{}},
126+
Org: &Organization{},
127+
}
128+
ctx.TemplateContext = NewTemplateContextForWeb(ctx)
129+
ctx.Flash = &middleware.Flash{DataStore: ctx, Values: url.Values{}}
130+
return ctx
131+
}
132+
110133
// Contexter initializes a classic context for a request.
111134
func Contexter() func(next http.Handler) http.Handler {
112135
rnd := templates.HTMLRenderer()
@@ -127,21 +150,8 @@ func Contexter() func(next http.Handler) http.Handler {
127150
return func(next http.Handler) http.Handler {
128151
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
129152
base, baseCleanUp := NewBaseContext(resp, req)
130-
ctx := &Context{
131-
Base: base,
132-
Cache: mc.GetCache(),
133-
Link: setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/"),
134-
Render: rnd,
135-
Session: session.GetSession(req),
136-
Repo: &Repository{PullRequest: &PullRequest{}},
137-
Org: &Organization{},
138-
}
139153
defer baseCleanUp()
140-
141-
// TODO: "install.go" also shares the same logic, which should be refactored to a general function
142-
ctx.TemplateContext = NewTemplateContext(ctx)
143-
ctx.TemplateContext["Locale"] = ctx.Locale
144-
ctx.TemplateContext["AvatarUtils"] = templates.NewAvatarUtils(ctx)
154+
ctx := NewWebContext(base, rnd, session.GetSession(req))
145155

146156
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
147157
ctx.Data["Context"] = ctx // TODO: use "ctx" in template and remove this
@@ -172,8 +182,7 @@ func Contexter() func(next http.Handler) http.Handler {
172182
}
173183
}
174184

175-
// prepare an empty Flash message for current request
176-
ctx.Flash = &middleware.Flash{DataStore: ctx, Values: url.Values{}}
185+
// if there are new messages in the ctx.Flash, write them into cookie
177186
ctx.Resp.Before(func(resp ResponseWriter) {
178187
if val := ctx.Flash.Encode(); val != "" {
179188
middleware.SetSiteCookie(ctx.Resp, CookieNameFlash, val, 0)

‎modules/context/package.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,10 @@ func PackageContexter() func(next http.Handler) http.Handler {
154154
return func(next http.Handler) http.Handler {
155155
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
156156
base, baseCleanUp := NewBaseContext(resp, req)
157-
ctx := &Context{
158-
Base: base,
159-
Render: renderer, // it is still needed when rendering 500 page in a package handler
160-
}
161157
defer baseCleanUp()
162158

159+
// it is still needed when rendering 500 page in a package handler
160+
ctx := NewWebContext(base, renderer, nil)
163161
ctx.Base.AppendContextValue(WebContextKey, ctx)
164162
next.ServeHTTP(ctx.Resp, ctx.Req)
165163
})

‎modules/test/context_tests.go

+7-9
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,12 @@ func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.Resp
4545
resp := httptest.NewRecorder()
4646
req := mockRequest(t, reqPath)
4747
base, baseCleanUp := context.NewBaseContext(resp, req)
48+
_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
4849
base.Data = middleware.GetContextData(req.Context())
4950
base.Locale = &translation.MockLocale{}
50-
ctx := &context.Context{
51-
Base: base,
52-
Render: &mockRender{},
53-
Flash: &middleware.Flash{Values: url.Values{}},
54-
}
55-
_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
51+
52+
ctx := context.NewWebContext(base, &MockRender{}, nil)
53+
ctx.Flash = &middleware.Flash{Values: url.Values{}}
5654

5755
chiCtx := chi.NewRouteContext()
5856
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
@@ -148,13 +146,13 @@ func LoadGitRepo(t *testing.T, ctx *context.Context) {
148146
assert.NoError(t, err)
149147
}
150148

151-
type mockRender struct{}
149+
type MockRender struct{}
152150

153-
func (tr *mockRender) TemplateLookup(tmpl string, _ gocontext.Context) (templates.TemplateExecutor, error) {
151+
func (tr *MockRender) TemplateLookup(tmpl string, _ gocontext.Context) (templates.TemplateExecutor, error) {
154152
return nil, nil
155153
}
156154

157-
func (tr *mockRender) HTML(w io.Writer, status int, _ string, _ any, _ gocontext.Context) error {
155+
func (tr *MockRender) HTML(w io.Writer, status int, _ string, _ any, _ gocontext.Context) error {
158156
if resp, ok := w.(http.ResponseWriter); ok {
159157
resp.WriteHeader(status)
160158
}

‎routers/install/install.go

+1-9
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,9 @@ func Contexter() func(next http.Handler) http.Handler {
6060
return func(next http.Handler) http.Handler {
6161
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
6262
base, baseCleanUp := context.NewBaseContext(resp, req)
63-
ctx := &context.Context{
64-
Base: base,
65-
Flash: &middleware.Flash{},
66-
Render: rnd,
67-
Session: session.GetSession(req),
68-
}
6963
defer baseCleanUp()
7064

71-
ctx.TemplateContext = context.NewTemplateContext(ctx)
72-
ctx.TemplateContext["Locale"] = ctx.Locale
73-
65+
ctx := context.NewWebContext(base, rnd, session.GetSession(req))
7466
ctx.AppendContextValue(context.WebContextKey, ctx)
7567
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
7668
ctx.Data.MergeFrom(middleware.ContextData{

‎routers/web/repo/actions/actions.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func List(ctx *context.Context) {
191191
ctx.Error(http.StatusInternalServerError, err.Error())
192192
return
193193
}
194-
ctx.Data["Actors"] = repo.MakeSelfOnTop(ctx, actors)
194+
ctx.Data["Actors"] = repo.MakeSelfOnTop(ctx.Doer, actors)
195195

196196
ctx.Data["StatusInfoList"] = actions_model.GetStatusInfoList(ctx)
197197

‎routers/web/repo/helper.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,15 @@ import (
77
"sort"
88

99
"code.gitea.io/gitea/models/user"
10-
"code.gitea.io/gitea/modules/context"
1110
)
1211

13-
func MakeSelfOnTop(ctx *context.Context, users []*user.User) []*user.User {
14-
if ctx.Doer != nil {
12+
func MakeSelfOnTop(doer *user.User, users []*user.User) []*user.User {
13+
if doer != nil {
1514
sort.Slice(users, func(i, j int) bool {
1615
if users[i].ID == users[j].ID {
1716
return false
1817
}
19-
return users[i].ID == ctx.Doer.ID // if users[i] is self, put it before others, so less=true
18+
return users[i].ID == doer.ID // if users[i] is self, put it before others, so less=true
2019
})
2120
}
2221
return users

‎routers/web/repo/helper_test.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,20 @@ import (
77
"testing"
88

99
"code.gitea.io/gitea/models/user"
10-
"code.gitea.io/gitea/modules/context"
1110

1211
"github.com/stretchr/testify/assert"
1312
)
1413

1514
func TestMakeSelfOnTop(t *testing.T) {
16-
users := MakeSelfOnTop(&context.Context{}, []*user.User{{ID: 2}, {ID: 1}})
15+
users := MakeSelfOnTop(nil, []*user.User{{ID: 2}, {ID: 1}})
1716
assert.Len(t, users, 2)
1817
assert.EqualValues(t, 2, users[0].ID)
1918

20-
users = MakeSelfOnTop(&context.Context{Doer: &user.User{ID: 1}}, []*user.User{{ID: 2}, {ID: 1}})
19+
users = MakeSelfOnTop(&user.User{ID: 1}, []*user.User{{ID: 2}, {ID: 1}})
2120
assert.Len(t, users, 2)
2221
assert.EqualValues(t, 1, users[0].ID)
2322

24-
users = MakeSelfOnTop(&context.Context{Doer: &user.User{ID: 2}}, []*user.User{{ID: 2}, {ID: 1}})
23+
users = MakeSelfOnTop(&user.User{ID: 2}, []*user.User{{ID: 2}, {ID: 1}})
2524
assert.Len(t, users, 2)
2625
assert.EqualValues(t, 2, users[0].ID)
2726
}

‎routers/web/repo/issue.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti
331331
ctx.ServerError("GetRepoAssignees", err)
332332
return
333333
}
334-
ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
334+
ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
335335

336336
handleTeamMentions(ctx)
337337
if ctx.Written() {
@@ -535,7 +535,7 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *repo_model.R
535535
ctx.ServerError("GetRepoAssignees", err)
536536
return
537537
}
538-
ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
538+
ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
539539

540540
handleTeamMentions(ctx)
541541
}
@@ -3625,7 +3625,7 @@ func issuePosters(ctx *context.Context, isPullList bool) {
36253625
}
36263626
}
36273627

3628-
posters = MakeSelfOnTop(ctx, posters)
3628+
posters = MakeSelfOnTop(ctx.Doer, posters)
36293629

36303630
resp := &userSearchResponse{}
36313631
resp.Results = make([]*userSearchInfo, len(posters))

‎routers/web/repo/pull.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
956956
ctx.ServerError("GetRepoAssignees", err)
957957
return
958958
}
959-
ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
959+
ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
960960

961961
handleTeamMentions(ctx)
962962
if ctx.Written() {

‎routers/web/repo/release.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ func NewRelease(ctx *context.Context) {
349349
ctx.ServerError("GetRepoAssignees", err)
350350
return
351351
}
352-
ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
352+
ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
353353

354354
upload.AddUploadContext(ctx, "release")
355355

@@ -538,7 +538,7 @@ func EditRelease(ctx *context.Context) {
538538
ctx.ServerError("GetRepoAssignees", err)
539539
return
540540
}
541-
ctx.Data["Assignees"] = MakeSelfOnTop(ctx, assigneeUsers)
541+
ctx.Data["Assignees"] = MakeSelfOnTop(ctx.Doer, assigneeUsers)
542542

543543
ctx.HTML(http.StatusOK, tplReleaseNew)
544544
}

‎services/markup/processorhelper_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"code.gitea.io/gitea/models/unittest"
1414
"code.gitea.io/gitea/models/user"
1515
gitea_context "code.gitea.io/gitea/modules/context"
16+
"code.gitea.io/gitea/modules/test"
1617

1718
"github.com/stretchr/testify/assert"
1819
)
@@ -41,7 +42,7 @@ func TestProcessorHelper(t *testing.T) {
4142
assert.NoError(t, err)
4243
base, baseCleanUp := gitea_context.NewBaseContext(httptest.NewRecorder(), req)
4344
defer baseCleanUp()
44-
giteaCtx := &gitea_context.Context{Base: base}
45+
giteaCtx := gitea_context.NewWebContext(base, &test.MockRender{}, nil)
4546

4647
assert.True(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPublic))
4748
assert.False(t, ProcessorHelper().IsUsernameMentionable(giteaCtx, userPrivate))

0 commit comments

Comments
 (0)