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.

diff --git a/modules/context/context.go b/modules/context/context.go
index 98c1a9bdb6705717b98762e7623e4cadfe00bd52..47ad310b095a258245d6be002454a1274b2559b2 100644 (file)
--- 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 be50e0a991ee2034b0fe5174f0e39bc6e71ad7f4..c0813fb2da6bad54eb358126d467929537d24d8b 100644 (file)
--- 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 92d7f8b22babcf75e7b6aa747786cafdf191707d..83e6117bcf335b67cf2f0b1070f93901326d7191 100644 (file)
--- 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 99ea7b0738c93dbf79db8f00f788acd0ac99c25c..66a9e1f0023fd79695516284f24130f3cda73b90 100644 (file)
--- 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 b0f4b6f8971da4ade5da1e3bac5f4b958842b940..6284d21463f3fcee9b2859a4b4fb6e8ce78f930a 100644 (file)
--- 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 fb5ada1bdb476ff98250d978b89bc0160dd8d62c..f8cdefdc8ef997bdfa3adaed5260d117624b0f85 100644 (file)
--- 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 226e2e81f4dde276142acb0accdec1926f796ef0..978758e77fd78300380748d8d3ed39f7940ec16e 100644 (file)
--- 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 9a2add14524c1043dc448860c318e0143ea25ffb..b8b5a2dff26d59e343082b7cdf5ad59f4d004c7e 100644 (file)
--- 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 e3854779fe258c9f302ef9de99936565146b84a2..e697a0d5b6b1829f64c2e2288e47db3e424f7eaf 100644 (file)
--- 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 957cf56972ce3b289ddd5cb0ac95f15101934c22..138df45857606f2718e658d33a73ec091aae4596 100644 (file)
--- 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 2f48e03b22bf0d6cdc58b3c3040c6d834e493fe3..d83e10903fdc0f0c90a336d8aad0f3d251065c31 100644 (file)
--- 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))