Skip to content

Commit abcf5a7

Browse files
authored
Fix install page context, make the install page tests really test (#24858)
Fix #24856 Rename "context.contextKey" to "context.WebContextKey", this context is for web context only. But the Context itself is not renamed, otherwise it would cause a lot of changes (if we really want to rename it, there could be a separate PR). The old test code doesn't really test, the "install page" gets broken not only one time, so use new test code to make sure the "install page" could work.
1 parent 5c0745c commit abcf5a7

File tree

9 files changed

+43
-28
lines changed

9 files changed

+43
-28
lines changed

cmd/web.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,8 @@ func runWeb(ctx *cli.Context) error {
142142
return err
143143
}
144144
}
145-
installCtx, cancel := context.WithCancel(graceful.GetManager().HammerContext())
146-
c := install.Routes(installCtx)
145+
c := install.Routes()
147146
err := listen(c, false)
148-
cancel()
149147
if err != nil {
150148
log.Critical("Unable to open listener for installer. Is Gitea already running?")
151149
graceful.GetManager().DoGracefulShutdown()

modules/context/context.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,12 @@ func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {
6868
return ctx.Locale.Tr(msg, trArgs...)
6969
}
7070

71-
type contextKeyType struct{}
71+
type webContextKeyType struct{}
7272

73-
var contextKey interface{} = contextKeyType{}
73+
var WebContextKey = webContextKeyType{}
7474

75-
func GetContext(req *http.Request) *Context {
76-
ctx, _ := req.Context().Value(contextKey).(*Context)
75+
func GetWebContext(req *http.Request) *Context {
76+
ctx, _ := req.Context().Value(WebContextKey).(*Context)
7777
return ctx
7878
}
7979

@@ -86,7 +86,7 @@ type ValidateContext struct {
8686
func GetValidateContext(req *http.Request) (ctx *ValidateContext) {
8787
if ctxAPI, ok := req.Context().Value(apiContextKey).(*APIContext); ok {
8888
ctx = &ValidateContext{Base: ctxAPI.Base}
89-
} else if ctxWeb, ok := req.Context().Value(contextKey).(*Context); ok {
89+
} else if ctxWeb, ok := req.Context().Value(WebContextKey).(*Context); ok {
9090
ctx = &ValidateContext{Base: ctxWeb.Base}
9191
} else {
9292
panic("invalid context, expect either APIContext or Context")
@@ -135,7 +135,7 @@ func Contexter() func(next http.Handler) http.Handler {
135135
ctx.PageData = map[string]any{}
136136
ctx.Data["PageData"] = ctx.PageData
137137

138-
ctx.Base.AppendContextValue(contextKey, ctx)
138+
ctx.Base.AppendContextValue(WebContextKey, ctx)
139139
ctx.Base.AppendContextValueFunc(git.RepositoryContextKey, func() any { return ctx.Repo.GitRepo })
140140

141141
ctx.Csrf = PrepareCSRFProtector(csrfOpts, ctx)

modules/context/package.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func PackageContexter() func(next http.Handler) http.Handler {
150150
}
151151
defer baseCleanUp()
152152

153-
ctx.Base.AppendContextValue(contextKey, ctx)
153+
ctx.Base.AppendContextValue(WebContextKey, ctx)
154154
next.ServeHTTP(ctx.Resp, ctx.Req)
155155
})
156156
}

modules/web/handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type ResponseStatusProvider interface {
2222
// TODO: decouple this from the context package, let the context package register these providers
2323
var argTypeProvider = map[reflect.Type]func(req *http.Request) ResponseStatusProvider{
2424
reflect.TypeOf(&context.APIContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetAPIContext(req) },
25-
reflect.TypeOf(&context.Context{}): func(req *http.Request) ResponseStatusProvider { return context.GetContext(req) },
25+
reflect.TypeOf(&context.Context{}): func(req *http.Request) ResponseStatusProvider { return context.GetWebContext(req) },
2626
reflect.TypeOf(&context.PrivateContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetPrivateContext(req) },
2727
}
2828

routers/install/install.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,15 @@ func Contexter() func(next http.Handler) http.Handler {
5959
return func(next http.Handler) http.Handler {
6060
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
6161
base, baseCleanUp := context.NewBaseContext(resp, req)
62-
ctx := context.Context{
62+
ctx := &context.Context{
6363
Base: base,
6464
Flash: &middleware.Flash{},
6565
Render: rnd,
6666
Session: session.GetSession(req),
6767
}
6868
defer baseCleanUp()
6969

70+
ctx.AppendContextValue(context.WebContextKey, ctx)
7071
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
7172
ctx.Data.MergeFrom(middleware.ContextData{
7273
"locale": ctx.Locale,

routers/install/routes.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package install
55

66
import (
7-
goctx "context"
87
"fmt"
98
"html"
109
"net/http"
@@ -18,7 +17,7 @@ import (
1817
)
1918

2019
// Routes registers the installation routes
21-
func Routes(ctx goctx.Context) *web.Route {
20+
func Routes() *web.Route {
2221
base := web.NewRoute()
2322
base.Use(common.ProtocolMiddlewares()...)
2423
base.RouteMethods("/assets/*", "GET, HEAD", public.AssetsHandlerFunc("/assets/"))

routers/install/routes_test.go

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,41 @@
1-
// Copyright 2021 The Gitea Authors. All rights reserved.
1+
// Copyright 2023 The Gitea Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

44
package install
55

66
import (
7-
"context"
7+
"net/http/httptest"
8+
"path/filepath"
89
"testing"
910

11+
"code.gitea.io/gitea/models/unittest"
12+
1013
"github.com/stretchr/testify/assert"
1114
)
1215

1316
func TestRoutes(t *testing.T) {
14-
// TODO: this test seems not really testing the handlers
15-
ctx, cancel := context.WithCancel(context.Background())
16-
defer cancel()
17-
base := Routes(ctx)
18-
assert.NotNil(t, base)
19-
r := base.R.Routes()[1]
20-
routes := r.SubRoutes.Routes()[0]
21-
assert.EqualValues(t, "/", routes.Pattern)
22-
assert.Nil(t, routes.SubRoutes)
23-
assert.Len(t, routes.Handlers, 2)
17+
r := Routes()
18+
assert.NotNil(t, r)
19+
20+
w := httptest.NewRecorder()
21+
req := httptest.NewRequest("GET", "/", nil)
22+
r.ServeHTTP(w, req)
23+
assert.EqualValues(t, 200, w.Code)
24+
assert.Contains(t, w.Body.String(), `class="page-content install"`)
25+
26+
w = httptest.NewRecorder()
27+
req = httptest.NewRequest("GET", "/no-such", nil)
28+
r.ServeHTTP(w, req)
29+
assert.EqualValues(t, 404, w.Code)
30+
31+
w = httptest.NewRecorder()
32+
req = httptest.NewRequest("GET", "/assets/img/gitea.svg", nil)
33+
r.ServeHTTP(w, req)
34+
assert.EqualValues(t, 200, w.Code)
35+
}
36+
37+
func TestMain(m *testing.M) {
38+
unittest.MainTest(m, &unittest.TestOptions{
39+
GiteaRootPath: filepath.Join("..", ".."),
40+
})
2441
}

routers/web/web.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,7 @@ func registerRoutes(m *web.Route) {
14051405
}
14061406

14071407
m.NotFound(func(w http.ResponseWriter, req *http.Request) {
1408-
ctx := context.GetContext(req)
1408+
ctx := context.GetWebContext(req)
14091409
ctx.NotFound("", nil)
14101410
})
14111411
}

services/auth/auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
9292
middleware.SetLocaleCookie(resp, user.Language, 0)
9393

9494
// Clear whatever CSRF has right now, force to generate a new one
95-
if ctx := gitea_context.GetContext(req); ctx != nil {
95+
if ctx := gitea_context.GetWebContext(req); ctx != nil {
9696
ctx.Csrf.DeleteCookie(ctx)
9797
}
9898
}

0 commit comments

Comments
 (0)