finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Refactor web package and context package (#25298)

Browse source 
1. The "web" package shouldn't depends on "modules/context" package,
instead, let each "web context" register themselves to the "web"
package.
2. The old Init/Free doesn't make sense, so simplify it
* The ctx in "Init(ctx)" is never used, and shouldn't be used that way
* The "Free" is never called and shouldn't be called because the SSPI
instance is shared

---------

Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
wxiaoguang 2023-06-18 15:59:09 +08:00 • committed by GitHub
parent fc2115b494
commit 4e2f1ee58d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
45 changed files with 218 additions and 292 deletions
Download patch file Download diff file Expand all files Collapse all files

8
modules/context/api.go
View file

@ -20,6 +20,8 @@ import (
"code.gitea.io/gitea/modules/httpcache"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/web"
web_types "code.gitea.io/gitea/modules/web/types"
"gitea.com/go-chi/cache"
)
@ -41,6 +43,12 @@ type APIContext struct {
Package *Package
}
func init() {
web.RegisterResponseStatusProvider[*APIContext](func(req *http.Request) web_types.ResponseStatusProvider {
return req.Context().Value(apiContextKey).(*APIContext)
})
}
// Currently, we have the following common fields in error response:
// * message: the message for end users (it shouldn't be used for error type detection)
// if we need to indicate some errors, we should introduce some new fields like ErrorCode or ErrorType

6
modules/context/base.go
View file

@ -96,7 +96,11 @@ func (b *Base) SetTotalCountHeader(total int64) {
// Written returns true if there are something sent to web browser
func (b *Base) Written() bool {
return b.Resp.Status() > 0
return b.Resp.WrittenStatus() != 0
}
func (b *Base) WrittenStatus() int {
return b.Resp.WrittenStatus()
}
// Status writes status code

8
modules/context/context.go
View file

@ -21,7 +21,9 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/modules/web/middleware"
web_types "code.gitea.io/gitea/modules/web/types"
"gitea.com/go-chi/cache"
"gitea.com/go-chi/session"
@ -58,6 +60,12 @@ type Context struct {
Package *Package
}
func init() {
web.RegisterResponseStatusProvider[*Context](func(req *http.Request) web_types.ResponseStatusProvider {
return req.Context().Value(WebContextKey).(*Context)
})
}
// TrHTMLEscapeArgs runs ".Locale.Tr()" but pre-escapes all arguments with html.EscapeString.
// This is useful if the locale message is intended to only produce HTML content.
func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {

8
modules/context/private.go
View file

@ -11,6 +11,8 @@ import (
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/web"
web_types "code.gitea.io/gitea/modules/web/types"
)
// PrivateContext represents a context for private routes
@ -21,6 +23,12 @@ type PrivateContext struct {
Repo *Repository
}
func init() {
web.RegisterResponseStatusProvider[*PrivateContext](func(req *http.Request) web_types.ResponseStatusProvider {
return req.Context().Value(privateContextKey).(*PrivateContext)
})
}
// Deadline is part of the interface for context.Context and we pass this to the request context
func (ctx *PrivateContext) Deadline() (deadline time.Time, ok bool) {
if ctx.Override != nil {

17
modules/context/response.go
View file

@ -5,15 +5,20 @@ package context
import (
"net/http"
web_types "code.gitea.io/gitea/modules/web/types"
)
// ResponseWriter represents a response writer for HTTP
type ResponseWriter interface {
http.ResponseWriter
http.Flusher
Status() int
web_types.ResponseStatusProvider
Before(func(ResponseWriter))
Size() int // used by access logger template
Status() int // used by access logger template
Size() int // used by access logger template
}
var _ ResponseWriter = &Response{}
@ -46,6 +51,10 @@ func (r *Response) Write(bs []byte) (int, error) {
return size, nil
}
func (r *Response) Status() int {
return r.status
}
func (r *Response) Size() int {
return r.written
}
@ -71,8 +80,8 @@ func (r *Response) Flush() {
}
}
// Status returned status code written
func (r *Response) Status() int {
// WrittenStatus returned status code written
func (r *Response) WrittenStatus() int {
return r.status
}

42
modules/test/context_tests.go
View file

@ -9,6 +9,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
access_model "code.gitea.io/gitea/models/perm/access"
@ -25,19 +26,26 @@ import (
"github.com/stretchr/testify/assert"
)
// MockContext mock context for unit tests
// TODO: move this function to other packages, because it depends on "models" package
func MockContext(t *testing.T, path string) *context.Context {
resp := httptest.NewRecorder()
func mockRequest(t *testing.T, reqPath string) *http.Request {
method, path, found := strings.Cut(reqPath, " ")
if !found {
method = "GET"
path = reqPath
}
requestURL, err := url.Parse(path)
assert.NoError(t, err)
req := &http.Request{
URL: requestURL,
Form: url.Values{},
}
req := &http.Request{Method: method, URL: requestURL, Form: url.Values{}}
req = req.WithContext(middleware.WithContextData(req.Context()))
return req
}
// MockContext mock context for unit tests
// TODO: move this function to other packages, because it depends on "models" package
func MockContext(t *testing.T, reqPath string) (*context.Context, *httptest.ResponseRecorder) {
resp := httptest.NewRecorder()
req := mockRequest(t, reqPath)
base, baseCleanUp := context.NewBaseContext(resp, req)
base.Data = middleware.ContextData{}
base.Data = middleware.GetContextData(req.Context())
base.Locale = &translation.MockLocale{}
ctx := &context.Context{
Base: base,
@ -48,29 +56,23 @@ func MockContext(t *testing.T, path string) *context.Context {
chiCtx := chi.NewRouteContext()
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
return ctx
return ctx, resp
}
// MockAPIContext mock context for unit tests
// TODO: move this function to other packages, because it depends on "models" package
func MockAPIContext(t *testing.T, path string) *context.APIContext {
func MockAPIContext(t *testing.T, reqPath string) (*context.APIContext, *httptest.ResponseRecorder) {
resp := httptest.NewRecorder()
requestURL, err := url.Parse(path)
assert.NoError(t, err)
req := &http.Request{
URL: requestURL,
Form: url.Values{},
}
req := mockRequest(t, reqPath)
base, baseCleanUp := context.NewBaseContext(resp, req)
base.Data = middleware.ContextData{}
base.Data = middleware.GetContextData(req.Context())
base.Locale = &translation.MockLocale{}
ctx := &context.APIContext{Base: base}
_ = baseCleanUp // during test, it doesn't need to do clean up. TODO: this can be improved later
chiCtx := chi.NewRouteContext()
ctx.Base.AppendContextValue(chi.RouteCtxKey, chiCtx)
return ctx
return ctx, resp
}
// LoadRepo load a repo into a test context.

34
modules/web/handler.go
View file

@ -9,25 +9,15 @@ import (
"net/http"
"reflect"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/web/routing"
"code.gitea.io/gitea/modules/web/types"
)
// ResponseStatusProvider is an interface to check whether the response has been written by the handler
type ResponseStatusProvider interface {
Written() bool
}
var responseStatusProviders = map[reflect.Type]func(req *http.Request) types.ResponseStatusProvider{}
// 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.GetWebContext(req) },
reflect.TypeOf(&context.PrivateContext{}): func(req *http.Request) ResponseStatusProvider { return context.GetPrivateContext(req) },
}
func RegisterHandleTypeProvider[T any](fn func(req *http.Request) ResponseStatusProvider) {
argTypeProvider[reflect.TypeOf((*T)(nil)).Elem()] = fn
func RegisterResponseStatusProvider[T any](fn func(req *http.Request) types.ResponseStatusProvider) {
responseStatusProviders[reflect.TypeOf((*T)(nil)).Elem()] = fn
}
// responseWriter is a wrapper of http.ResponseWriter, to check whether the response has been written
@ -36,10 +26,10 @@ type responseWriter struct {
status int
}
var _ ResponseStatusProvider = (*responseWriter)(nil)
var _ types.ResponseStatusProvider = (*responseWriter)(nil)
func (r *responseWriter) Written() bool {
return r.status > 0
func (r *responseWriter) WrittenStatus() int {
return r.status
}
func (r *responseWriter) Header() http.Header {
@ -68,7 +58,7 @@ var (
func preCheckHandler(fn reflect.Value, argsIn []reflect.Value) {
hasStatusProvider := false
for _, argIn := range argsIn {
if _, hasStatusProvider = argIn.Interface().(ResponseStatusProvider); hasStatusProvider {
if _, hasStatusProvider = argIn.Interface().(types.ResponseStatusProvider); hasStatusProvider {
break
}
}
@ -101,7 +91,7 @@ func prepareHandleArgsIn(resp http.ResponseWriter, req *http.Request, fn reflect
case httpReqType:
argsIn[i] = reflect.ValueOf(req)
default:
if argFn, ok := argTypeProvider[argTyp]; ok {
if argFn, ok := responseStatusProviders[argTyp]; ok {
if isPreCheck {
argsIn[i] = reflect.ValueOf(&responseWriter{})
} else {
@ -129,8 +119,8 @@ func handleResponse(fn reflect.Value, ret []reflect.Value) goctx.CancelFunc {
func hasResponseBeenWritten(argsIn []reflect.Value) bool {
for _, argIn := range argsIn {
if statusProvider, ok := argIn.Interface().(ResponseStatusProvider); ok {
if statusProvider.Written() {
if statusProvider, ok := argIn.Interface().(types.ResponseStatusProvider); ok {
if statusProvider.WrittenStatus() != 0 {
return true
}
}
@ -161,7 +151,7 @@ func toHandlerProvider(handler any) func(next http.Handler) http.Handler {
return http.HandlerFunc(func(respOrig http.ResponseWriter, req *http.Request) {
// wrap the response writer to check whether the response has been written
resp := respOrig
if _, ok := resp.(ResponseStatusProvider); !ok {
if _, ok := resp.(types.ResponseStatusProvider); !ok {
resp = &responseWriter{respWriter: resp}
}

2
modules/web/middleware/data.go
View file

@ -17,7 +17,7 @@ type ContextDataStore interface {
type ContextData map[string]any
func (ds ContextData) GetData() map[string]any {
func (ds ContextData) GetData() ContextData {
return ds
}

24
modules/web/route.go
View file

@ -7,31 +7,31 @@ import (
"net/http"
"strings"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/web/middleware"
"gitea.com/go-chi/binding"
chi "github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5"
)
// Bind binding an obj to a handler
func Bind[T any](_ T) any {
return func(ctx *context.Context) {
// Bind binding an obj to a handler's context data
func Bind[T any](_ T) http.HandlerFunc {
return func(resp http.ResponseWriter, req *http.Request) {
theObj := new(T) // create a new form obj for every request but not use obj directly
binding.Bind(ctx.Req, theObj)
SetForm(ctx, theObj)
middleware.AssignForm(theObj, ctx.Data)
data := middleware.GetContextData(req.Context())
binding.Bind(req, theObj)
SetForm(data, theObj)
middleware.AssignForm(theObj, data)
}
}
// SetForm set the form object
func SetForm(data middleware.ContextDataStore, obj interface{}) {
data.GetData()["__form"] = obj
func SetForm(dataStore middleware.ContextDataStore, obj interface{}) {
dataStore.GetData()["__form"] = obj
}
// GetForm returns the validate form information
func GetForm(data middleware.ContextDataStore) interface{} {
return data.GetData()["__form"]
func GetForm(dataStore middleware.ContextDataStore) interface{} {
return dataStore.GetData()["__form"]
}
// Route defines a route based on chi's router

6
modules/web/routing/logger.go
View file

@ -8,8 +8,8 @@ import (
"strings"
"time"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/web/types"
)
// NewLoggerHandler is a handler that will log routing to the router log taking account of
@ -86,8 +86,8 @@ func logPrinter(logger log.Logger) func(trigger Event, record *requestRecord) {
}
var status int
if v, ok := record.responseWriter.(context.ResponseWriter); ok {
status = v.Status()
if v, ok := record.responseWriter.(types.ResponseStatusProvider); ok {
status = v.WrittenStatus()
}
logf := log.Info
if strings.HasPrefix(req.RequestURI, "/assets/") {

10
modules/web/types/response.go Normal file
View file

@ -0,0 +1,10 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package types
// ResponseStatusProvider is an interface to get the written status in the response
// Many packages need this interface, so put it in the separate package to avoid import cycle
type ResponseStatusProvider interface {
WrittenStatus() int
}