Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make web context initialize correctly for different cases #26726

Merged
merged 1 commit into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 25 additions & 16 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions modules/context/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
16 changes: 7 additions & 9 deletions modules/test/context_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
10 changes: 1 addition & 9 deletions routers/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/actions/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 3 additions & 4 deletions routers/web/repo/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions routers/web/repo/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 3 additions & 3 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion services/markup/processorhelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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))
Expand Down