git724/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Refactor web route (#24080)

Browse source 
The old code is unnecessarily complex, and has many misuses.

Old code "wraps" a lot, wrap wrap wrap, it's difficult to understand
which kind of handler is used.

The new code uses a general approach, we do not need to write all kinds
of handlers into the "wrapper", do not need to wrap them again and
again.

New code, there are only 2 concepts:

1. HandlerProvider: `func (h any) (handlerProvider func (next)
http.Handler)`, it can be used as middleware
2. Use HandlerProvider to get the final HandlerFunc, and use it for
`r.Get()`


And we can decouple the route package from context package (see the
TODO).

# FAQ

## Is `reflect` safe?

Yes, all handlers are checked during startup, see the `preCheckHandler`
comment. If any handler is wrong, developers could know it in the first
time.

## Does `reflect` affect performance?

No. https://github.com/go-gitea/gitea/pull/24080#discussion_r1164825901

1. This reflect code only runs for each web handler call, handler is far
more slower: 10ms-50ms
2. The reflect is pretty fast (comparing to other code): 0.000265ms
3. XORM has more reflect operations already
This commit is contained in:
wxiaoguang 2023-04-21 02:49:06 +08:00 • committed by GitHub
parent 70fc47a22a
commit b9a97ccd0e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 297 additions and 332 deletions
Download patch file Download diff file Expand all files Collapse all files

48
modules/web/route_test.go
View file

@ -7,6 +7,7 @@ import (
"bytes"
"net/http"
"net/http/httptest"
"strconv"
"testing"
chi "github.com/go-chi/chi/v5"
@ -39,7 +40,7 @@ func TestRoute2(t *testing.T) {
recorder := httptest.NewRecorder()
recorder.Body = buff
var route int
hit := -1
r := NewRoute()
r.Group("/{username}/{reponame}", func() {
@ -51,7 +52,7 @@ func TestRoute2(t *testing.T) {
assert.EqualValues(t, "gitea", reponame)
tp := chi.URLParam(req, "type")
assert.EqualValues(t, "issues", tp)
route = 0
hit = 0
})
r.Get("/{type:issues|pulls}/{index}", func(resp http.ResponseWriter, req *http.Request) {
@ -63,10 +64,13 @@ func TestRoute2(t *testing.T) {
assert.EqualValues(t, "issues", tp)
index := chi.URLParam(req, "index")
assert.EqualValues(t, "1", index)
route = 1
hit = 1
})
}, func(resp http.ResponseWriter, req *http.Request) {
resp.WriteHeader(http.StatusOK)
if stop, err := strconv.Atoi(req.FormValue("stop")); err == nil {
hit = stop
resp.WriteHeader(http.StatusOK)
}
})
r.Group("/issues/{index}", func() {
@ -77,7 +81,7 @@ func TestRoute2(t *testing.T) {
assert.EqualValues(t, "gitea", reponame)
index := chi.URLParam(req, "index")
assert.EqualValues(t, "1", index)
route = 2
hit = 2
})
})
})
@ -86,19 +90,25 @@ func TestRoute2(t *testing.T) {
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 0, route)
assert.EqualValues(t, 0, hit)
req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 1, route)
assert.EqualValues(t, 1, hit)
req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1?stop=100", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 100, hit)
req, err = http.NewRequest("GET", "http://localhost:8000/gitea/gitea/issues/1/view", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 2, route)
assert.EqualValues(t, 2, hit)
}
func TestRoute3(t *testing.T) {
@ -106,7 +116,7 @@ func TestRoute3(t *testing.T) {
recorder := httptest.NewRecorder()
recorder.Body = buff
var route int
hit := -1
m := NewRoute()
r := NewRoute()
@ -116,20 +126,20 @@ func TestRoute3(t *testing.T) {
m.Group("/{username}/{reponame}", func() {
m.Group("/branch_protections", func() {
m.Get("", func(resp http.ResponseWriter, req *http.Request) {
route = 0
hit = 0
})
m.Post("", func(resp http.ResponseWriter, req *http.Request) {
route = 1
hit = 1
})
m.Group("/{name}", func() {
m.Get("", func(resp http.ResponseWriter, req *http.Request) {
route = 2
hit = 2
})
m.Patch("", func(resp http.ResponseWriter, req *http.Request) {
route = 3
hit = 3
})
m.Delete("", func(resp http.ResponseWriter, req *http.Request) {
route = 4
hit = 4
})
})
})
@ -140,29 +150,29 @@ func TestRoute3(t *testing.T) {
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 0, route)
assert.EqualValues(t, 0, hit)
req, err = http.NewRequest("POST", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code, http.StatusOK)
assert.EqualValues(t, 1, route)
assert.EqualValues(t, 1, hit)
req, err = http.NewRequest("GET", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 2, route)
assert.EqualValues(t, 2, hit)
req, err = http.NewRequest("PATCH", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 3, route)
assert.EqualValues(t, 3, hit)
req, err = http.NewRequest("DELETE", "http://localhost:8000/api/v1/repos/gitea/gitea/branch_protections/master", nil)
assert.NoError(t, err)
r.ServeHTTP(recorder, req)
assert.EqualValues(t, http.StatusOK, recorder.Code)
assert.EqualValues(t, 4, route)
assert.EqualValues(t, 4, hit)
}