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

@@ -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)

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)
 		})

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)
 	}

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{

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)
 

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

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)
 }

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))

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() {

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)
 }

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))