Skip to content

Fix install page context, make the install page tests really test #24858

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

Merged
merged 2 commits into from
May 23, 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
4 changes: 1 addition & 3 deletions cmd/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,8 @@ func runWeb(ctx *cli.Context) error {
return err
}
}
installCtx, cancel := context.WithCancel(graceful.GetManager().HammerContext())
c := install.Routes(installCtx)
c := install.Routes()
err := listen(c, false)
cancel()
if err != nil {
log.Critical("Unable to open listener for installer. Is Gitea already running?")
graceful.GetManager().DoGracefulShutdown()
Expand Down
12 changes: 6 additions & 6 deletions modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@ func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {
return ctx.Locale.Tr(msg, trArgs...)
}

type contextKeyType struct{}
type webContextKeyType struct{}

var contextKey interface{} = contextKeyType{}
var WebContextKey = webContextKeyType{}

func GetContext(req *http.Request) *Context {
ctx, _ := req.Context().Value(contextKey).(*Context)
func GetWebContext(req *http.Request) *Context {
ctx, _ := req.Context().Value(WebContextKey).(*Context)
return ctx
}

Expand All @@ -86,7 +86,7 @@ type ValidateContext struct {
func GetValidateContext(req *http.Request) (ctx *ValidateContext) {
if ctxAPI, ok := req.Context().Value(apiContextKey).(*APIContext); ok {
ctx = &ValidateContext{Base: ctxAPI.Base}
} else if ctxWeb, ok := req.Context().Value(contextKey).(*Context); ok {
} else if ctxWeb, ok := req.Context().Value(WebContextKey).(*Context); ok {
ctx = &ValidateContext{Base: ctxWeb.Base}
} else {
panic("invalid context, expect either APIContext or Context")
Expand Down Expand Up @@ -135,7 +135,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.PageData = map[string]any{}
ctx.Data["PageData"] = ctx.PageData

ctx.Base.AppendContextValue(contextKey, ctx)
ctx.Base.AppendContextValue(WebContextKey, ctx)
ctx.Base.AppendContextValueFunc(git.RepositoryContextKey, func() any { return ctx.Repo.GitRepo })

ctx.Csrf = PrepareCSRFProtector(csrfOpts, ctx)
Expand Down
2 changes: 1 addition & 1 deletion modules/context/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func PackageContexter() func(next http.Handler) http.Handler {
}
defer baseCleanUp()

ctx.Base.AppendContextValue(contextKey, ctx)
ctx.Base.AppendContextValue(WebContextKey, ctx)
next.ServeHTTP(ctx.Resp, ctx.Req)
})
}
Expand Down
2 changes: 1 addition & 1 deletion modules/web/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type ResponseStatusProvider interface {
// TODO: decouple this from the context package, let the context package register these providers
var argTypeProvider = map[reflect.Type]func(req *http.Request) ResponseStatusProvider{
reflect.TypeOf(&context.APIContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetAPIContext(req) },
reflect.TypeOf(&context.Context{}): func(req *http.Request) ResponseStatusProvider { return context.GetContext(req) },
reflect.TypeOf(&context.Context{}): func(req *http.Request) ResponseStatusProvider { return context.GetWebContext(req) },
reflect.TypeOf(&context.PrivateContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetPrivateContext(req) },
}

Expand Down
3 changes: 2 additions & 1 deletion routers/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,15 @@ 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{
ctx := &context.Context{
Base: base,
Flash: &middleware.Flash{},
Render: rnd,
Session: session.GetSession(req),
}
defer baseCleanUp()

ctx.AppendContextValue(context.WebContextKey, ctx)
ctx.Data.MergeFrom(middleware.CommonTemplateContextData())
ctx.Data.MergeFrom(middleware.ContextData{
"locale": ctx.Locale,
Expand Down
3 changes: 1 addition & 2 deletions routers/install/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package install

import (
goctx "context"
"fmt"
"html"
"net/http"
Expand All @@ -18,7 +17,7 @@ import (
)

// Routes registers the installation routes
func Routes(ctx goctx.Context) *web.Route {
func Routes() *web.Route {
base := web.NewRoute()
base.Use(common.ProtocolMiddlewares()...)
base.RouteMethods("/assets/*", "GET, HEAD", public.AssetsHandlerFunc("/assets/"))
Expand Down
41 changes: 29 additions & 12 deletions routers/install/routes_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,41 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package install

import (
"context"
"net/http/httptest"
"path/filepath"
"testing"

"code.gitea.io/gitea/models/unittest"

"github.com/stretchr/testify/assert"
)

func TestRoutes(t *testing.T) {
// TODO: this test seems not really testing the handlers
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
base := Routes(ctx)
assert.NotNil(t, base)
r := base.R.Routes()[1]
routes := r.SubRoutes.Routes()[0]
assert.EqualValues(t, "/", routes.Pattern)
assert.Nil(t, routes.SubRoutes)
assert.Len(t, routes.Handlers, 2)
r := Routes()
assert.NotNil(t, r)

w := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/", nil)
r.ServeHTTP(w, req)
assert.EqualValues(t, 200, w.Code)
assert.Contains(t, w.Body.String(), `class="page-content install"`)

w = httptest.NewRecorder()
req = httptest.NewRequest("GET", "/no-such", nil)
r.ServeHTTP(w, req)
assert.EqualValues(t, 404, w.Code)

w = httptest.NewRecorder()
req = httptest.NewRequest("GET", "/assets/img/gitea.svg", nil)
r.ServeHTTP(w, req)
assert.EqualValues(t, 200, w.Code)
}

func TestMain(m *testing.M) {
unittest.MainTest(m, &unittest.TestOptions{
GiteaRootPath: filepath.Join("..", ".."),
})
}
2 changes: 1 addition & 1 deletion routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,7 @@ func registerRoutes(m *web.Route) {
}

m.NotFound(func(w http.ResponseWriter, req *http.Request) {
ctx := context.GetContext(req)
ctx := context.GetWebContext(req)
ctx.NotFound("", nil)
})
}
2 changes: 1 addition & 1 deletion services/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
middleware.SetLocaleCookie(resp, user.Language, 0)

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