Skip to content

Commit

Permalink
fix(net/ghttp): update response message handling in `MiddlewareHandle…
Browse files Browse the repository at this point in the history
…rResponse` (#4162)
  • Loading branch information
hailaz authored Feb 27, 2025
1 parent a3b3c65 commit 63cb328
Show file tree
Hide file tree
Showing 24 changed files with 274 additions and 101 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
strategy:
matrix:
go-version: [ 'stable' ]

name: golang-ci-lint
runs-on: ubuntu-latest
steps:
Expand All @@ -43,7 +44,7 @@ jobs:
uses: golangci/golangci-lint-action@v6
with:
# Required: specify the golangci-lint version without the patch version to always use the latest patch.
version: v1.62.2
version: v1.64.5
only-new-issues: true
skip-cache: true
github-token: ${{ secrets.GITHUB_TOKEN }}
Expand Down
8 changes: 7 additions & 1 deletion cmd/gf/internal/cmd/genpbentity/genpbentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type (
g.Meta `name:"pbentity" config:"{CGenPbEntityConfig}" brief:"{CGenPbEntityBrief}" eg:"{CGenPbEntityEg}" ad:"{CGenPbEntityAd}"`
Path string `name:"path" short:"p" brief:"{CGenPbEntityBriefPath}" d:"manifest/protobuf/pbentity"`
Package string `name:"package" short:"k" brief:"{CGenPbEntityBriefPackage}"`
GoPackage string `name:"goPackage" short:"g" brief:"{CGenPbEntityBriefGoPackage}"`
Link string `name:"link" short:"l" brief:"{CGenPbEntityBriefLink}"`
Tables string `name:"tables" short:"t" brief:"{CGenPbEntityBriefTables}"`
Prefix string `name:"prefix" short:"f" brief:"{CGenPbEntityBriefPrefix}"`
Expand Down Expand Up @@ -113,6 +114,7 @@ CONFIGURATION SUPPORT
`
CGenPbEntityBriefPath = `directory path for generated files storing`
CGenPbEntityBriefPackage = `package path for all entity proto files`
CGenPbEntityBriefGoPackage = `go package path for all entity proto files`
CGenPbEntityBriefLink = `database configuration, the same as the ORM configuration of GoFrame`
CGenPbEntityBriefTables = `generate models only for given tables, multiple table names separated with ','`
CGenPbEntityBriefPrefix = `add specified prefix for all entity names and entity proto files`
Expand Down Expand Up @@ -239,6 +241,7 @@ func init() {
`CGenPbEntityAd`: CGenPbEntityAd,
`CGenPbEntityBriefPath`: CGenPbEntityBriefPath,
`CGenPbEntityBriefPackage`: CGenPbEntityBriefPackage,
`CGenPbEntityBriefGoPackage`: CGenPbEntityBriefGoPackage,
`CGenPbEntityBriefLink`: CGenPbEntityBriefLink,
`CGenPbEntityBriefTables`: CGenPbEntityBriefTables,
`CGenPbEntityBriefPrefix`: CGenPbEntityBriefPrefix,
Expand Down Expand Up @@ -379,10 +382,13 @@ func generatePbEntityContentFile(ctx context.Context, in CGenPbEntityInternalInp
}
}
}
if in.GoPackage == "" {
in.GoPackage = in.Package
}
entityContent := gstr.ReplaceByMap(getTplPbEntityContent(""), g.MapStrStr{
"{Imports}": packageImportsArray.Join("\n"),
"{PackageName}": gfile.Basename(in.Package),
"{GoPackage}": in.Package,
"{GoPackage}": in.GoPackage,
"{OptionContent}": in.Option,
"{EntityMessage}": entityMessageDefine,
})
Expand Down
2 changes: 1 addition & 1 deletion contrib/config/polaris/polaris.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func (c *Client) doUpdate(ctx context.Context) (err error) {
return gerror.New("config file is empty")
}
var j *gjson.Json
if j, err = gjson.DecodeToJson([]byte(c.client.GetContent())); err != nil {
if j, err = gjson.LoadContent([]byte(c.client.GetContent())); err != nil {
return gerror.Wrap(err, `parse config map item from polaris failed`)
}
c.value.Set(j)
Expand Down
2 changes: 1 addition & 1 deletion net/ghttp/ghttp_middleware_handler_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ func MiddlewareHandlerResponse(r *Request) {
msg = err.Error()
} else {
if r.Response.Status > 0 && r.Response.Status != http.StatusOK {
msg = http.StatusText(r.Response.Status)
switch r.Response.Status {
case http.StatusNotFound:
code = gcode.CodeNotFound
Expand All @@ -77,6 +76,7 @@ func MiddlewareHandlerResponse(r *Request) {
} else {
code = gcode.CodeOK
}
msg = code.Message()
}

r.Response.WriteJson(DefaultHandlerResponse{
Expand Down
20 changes: 10 additions & 10 deletions net/ghttp/ghttp_request_param.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,28 +268,28 @@ func (r *Request) parseForm() {
if r.ContentLength == 0 {
return
}

if contentType := r.Header.Get("Content-Type"); contentType != "" {
var (
err error
repeatableRead = true
)
if gstr.Contains(contentType, "multipart/") {
var isMultiPartRequest = gstr.Contains(contentType, "multipart/")
var isFormRequest = gstr.Contains(contentType, "form")
var err error

if !isMultiPartRequest {
// To avoid big memory consuming.
// The `multipart/` type form always contains binary data, which is not necessary read twice.
repeatableRead = false
r.MakeBodyRepeatableRead(true)
}
if isMultiPartRequest {
// multipart/form-data, multipart/mixed
if err = r.ParseMultipartForm(r.Server.config.FormParsingMemory); err != nil {
panic(gerror.WrapCode(gcode.CodeInvalidRequest, err, "r.ParseMultipartForm failed"))
}
} else if gstr.Contains(contentType, "form") {
} else if isFormRequest {
// application/x-www-form-urlencoded
if err = r.Request.ParseForm(); err != nil {
panic(gerror.WrapCode(gcode.CodeInvalidRequest, err, "r.Request.ParseForm failed"))
}
}
if repeatableRead {
r.MakeBodyRepeatableRead(true)
}
if len(r.PostForm) > 0 {
// Parse the form data using united parsing way.
params := ""
Expand Down
4 changes: 2 additions & 2 deletions net/ghttp/ghttp_request_param_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (r *Request) doGetRequestStruct(pointer interface{}, mapping ...map[string]
return data, nil
}
// `in` Tag Struct values.
if err = r.mergeInTagStructValue(data, pointer); err != nil {
if err = r.mergeInTagStructValue(data); err != nil {
return data, nil
}

Expand Down Expand Up @@ -239,7 +239,7 @@ func (r *Request) mergeDefaultStructValue(data map[string]interface{}, pointer i
}

// mergeInTagStructValue merges the request parameters with header or cookie values from struct `in` tag definition.
func (r *Request) mergeInTagStructValue(data map[string]interface{}, pointer interface{}) error {
func (r *Request) mergeInTagStructValue(data map[string]interface{}) error {
fields := r.serveHandler.Handler.Info.ReqStructFields
if len(fields) > 0 {
var (
Expand Down
17 changes: 0 additions & 17 deletions net/ghttp/ghttp_server_service_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,20 +282,3 @@ func createRouterFunc(funcInfo handlerFuncInfo) func(r *Request) {
}
}
}

// trimGeneric removes type definitions string from response type name if generic
func trimGeneric(structName string) string {
var (
leftBraceIndex = strings.LastIndex(structName, "[") // for generic, it is faster to start at the end than at the beginning
rightBraceIndex = strings.LastIndex(structName, "]")
)
if leftBraceIndex == -1 || rightBraceIndex == -1 {
// not found '[' or ']'
return structName
} else if leftBraceIndex+1 == rightBraceIndex {
// may be a slice, because generic is '[X]', not '[]'
// to be compatible with bad return parameter type: []XxxRes
return structName
}
return structName[:leftBraceIndex]
}
10 changes: 5 additions & 5 deletions net/ghttp/ghttp_z_unit_feature_openapi_swagger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Test_OpenApi_Swagger(t *testing.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

t.Assert(gstr.Contains(c.GetContent(ctx, "/swagger/"), `API Reference`), true)
Expand Down Expand Up @@ -116,9 +116,9 @@ func Test_OpenApi_Multiple_Methods_Swagger(t *testing.T) {
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

// Only works on GET & POST methods.
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

// Not works on other methods.
Expand Down Expand Up @@ -176,9 +176,9 @@ func Test_OpenApi_Method_All_Swagger(t *testing.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(c.PostContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)

t.Assert(gstr.Contains(c.GetContent(ctx, "/swagger/"), `API Reference`), true)
Expand Down
36 changes: 36 additions & 0 deletions net/ghttp/ghttp_z_unit_feature_request_struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,42 @@ func Test_Params_ParseForm(t *testing.T) {
})
}

// https://github.com/gogf/gf/pull/4143
func Test_Params_ParseForm_FixMakeBodyRepeatableRead(t *testing.T) {
type User struct {
Id int
Name string
}
s := g.Server(guid.S())
s.BindHandler("/parse-form", func(r *ghttp.Request) {
var user *User
if err := r.ParseForm(&user); err != nil {
r.Response.WriteExit(err)
}
hasBody := len(r.GetBody()) > 0
r.Response.WriteExit(hasBody)
})
s.SetDumpRouterMap(false)
s.Start()
defer s.Shutdown()

time.Sleep(100 * time.Millisecond)
gtest.C(t, func(t *gtest.T) {
c := g.Client()
c.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))
t.Assert(c.GetContent(ctx, "/parse-form"), `false`)
t.Assert(c.GetContent(ctx, "/parse-form", g.Map{
"id": 1,
"name": "john",
}), false)
t.Assert(c.PostContent(ctx, "/parse-form"), `false`)
t.Assert(c.PostContent(ctx, "/parse-form", g.Map{
"id": 1,
"name": "john",
}), true)
})
}

func Test_Params_ComplexJsonStruct(t *testing.T) {
type ItemEnv struct {
Type string
Expand Down
48 changes: 24 additions & 24 deletions net/ghttp/ghttp_z_unit_feature_router_strict_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func Test_Router_Handler_Strict_WithObject(t *testing.T) {
client := g.Client()
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/test?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/test/error"), `{"code":50,"message":"error","data":{"Id":1,"Age":0,"Name":""}}`)
})
}
Expand Down Expand Up @@ -153,8 +153,8 @@ func Test_Router_Handler_Strict_WithObjectAndMeta(t *testing.T) {
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/"), `{"code":65,"message":"Not Found","data":null}`)
t.Assert(client.GetContent(ctx, "/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.PostContent(ctx, "/custom-test2?age=18&name=john"), `{"code":65,"message":"Not Found","data":null}`)
})
}
Expand Down Expand Up @@ -184,17 +184,17 @@ func Test_Router_Handler_Strict_Group_Bind(t *testing.T) {
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/"), `{"code":65,"message":"Not Found","data":null}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.PostContent(ctx, "/api/v1/custom-test2?age=18&name=john"), `{"code":65,"message":"Not Found","data":null}`)

t.Assert(client.GetContent(ctx, "/api/v1/custom-test3?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test4?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test3?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v1/custom-test4?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)

t.Assert(client.GetContent(ctx, "/api/v2/custom-test1?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test2?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test3?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test4?age=18&name=john"), `{"code":0,"message":"","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test1?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test2?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test3?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Age":18}}`)
t.Assert(client.GetContent(ctx, "/api/v2/custom-test4?age=18&name=john"), `{"code":0,"message":"OK","data":{"Id":1,"Name":"john"}}`)
})
}

Expand Down Expand Up @@ -251,7 +251,7 @@ func Test_Issue1708(t *testing.T) {
`
t.Assert(
client.PostContent(ctx, "/test", content),
`{"code":0,"message":"","data":{"page":0,"size":0,"targetType":"topic","targetId":10785,"test":[[{"name":"123"}]]}}`,
`{"code":0,"message":"OK","data":{"page":0,"size":0,"targetType":"topic","targetId":10785,"test":[[{"name":"123"}]]}}`,
)
})
}
Expand Down Expand Up @@ -295,7 +295,7 @@ func Test_Custom_Slice_Type_Attribute(t *testing.T) {
`
t.Assert(
client.PostContent(ctx, "/test", content),
`{"code":0,"message":"","data":{"Content":"{\"Id\":1,\"List\":{\"key\":[\"a\",\"b\",\"c\"]}}"}}`,
`{"code":0,"message":"OK","data":{"Content":"{\"Id\":1,\"List\":{\"key\":[\"a\",\"b\",\"c\"]}}"}}`,
)
})
}
Expand Down Expand Up @@ -370,12 +370,12 @@ func Test_Router_Handler_Strict_WithGeneric(t *testing.T) {
client := g.Client()
client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort()))

t.Assert(client.GetContent(ctx, "/test1?age=1"), `{"code":0,"message":"","data":{"Age":{"Test":1}}}`)
t.Assert(client.GetContent(ctx, "/test1_slice?age=1"), `{"code":0,"message":"","data":[{"Age":{"Test":1}}]}`)
t.Assert(client.GetContent(ctx, "/test2?age=2"), `{"code":0,"message":"","data":{"Test":2}}`)
t.Assert(client.GetContent(ctx, "/test2_slice?age=2"), `{"code":0,"message":"","data":[{"Test":2}]}`)
t.Assert(client.GetContent(ctx, "/test3?age=3"), `{"code":0,"message":"","data":{"Test":3}}`)
t.Assert(client.GetContent(ctx, "/test3_slice?age=3"), `{"code":0,"message":"","data":[{"Test":3}]}`)
t.Assert(client.GetContent(ctx, "/test1?age=1"), `{"code":0,"message":"OK","data":{"Age":{"Test":1}}}`)
t.Assert(client.GetContent(ctx, "/test1_slice?age=1"), `{"code":0,"message":"OK","data":[{"Age":{"Test":1}}]}`)
t.Assert(client.GetContent(ctx, "/test2?age=2"), `{"code":0,"message":"OK","data":{"Test":2}}`)
t.Assert(client.GetContent(ctx, "/test2_slice?age=2"), `{"code":0,"message":"OK","data":[{"Test":2}]}`)
t.Assert(client.GetContent(ctx, "/test3?age=3"), `{"code":0,"message":"OK","data":{"Test":3}}`)
t.Assert(client.GetContent(ctx, "/test3_slice?age=3"), `{"code":0,"message":"OK","data":[{"Test":3}]}`)
})
}

Expand Down Expand Up @@ -415,7 +415,7 @@ func Test_Router_Handler_Strict_ParameterCaseSensitive(t *testing.T) {
for i := 0; i < 1000; i++ {
t.Assert(
client.PostContent(ctx, "/api/111", `{"Path":"222"}`),
`{"code":0,"message":"","data":{"Path":"222"}}`,
`{"code":0,"message":"OK","data":{"Path":"222"}}`,
)
}
})
Expand Down Expand Up @@ -474,10 +474,10 @@ func Test_JsonRawMessage_Issue3449(t *testing.T) {
},
}

expect1 := `{"code":0,"message":"","data":{"name":"test","jsonRaw":[{"jkey1":"11","jkey2":"12"},{"jkey1":"21","jkey2":"22"}]}}`
expect1 := `{"code":0,"message":"OK","data":{"name":"test","jsonRaw":[{"jkey1":"11","jkey2":"12"},{"jkey1":"21","jkey2":"22"}]}}`
t.Assert(client.PostContent(ctx, "/test", data), expect1)

expect2 := `{"code":0,"message":"","data":{"name":"test","jsonRaw":{"jkey1":"11","jkey2":"12"}}}`
expect2 := `{"code":0,"message":"OK","data":{"name":"test","jsonRaw":{"jkey1":"11","jkey2":"12"}}}`
t.Assert(client.PostContent(ctx, "/test", map[string]any{
"Name": "test",
"jsonRaw": v1,
Expand Down Expand Up @@ -524,13 +524,13 @@ func Test_NullString_Issue3465(t *testing.T) {
"name": "null",
}

expect1 := `{"code":0,"message":"","data":{"name":["null"]}}`
expect1 := `{"code":0,"message":"OK","data":{"name":["null"]}}`
t.Assert(client.GetContent(ctx, "/test", data1), expect1)

data2 := map[string]any{
"name": []string{"null", "null"},
}
expect2 := `{"code":0,"message":"","data":{"name":["null","null"]}}`
expect2 := `{"code":0,"message":"OK","data":{"name":["null","null"]}}`
t.Assert(client.GetContent(ctx, "/test", data2), expect2)

})
Expand Down
Loading

0 comments on commit 63cb328

Please sign in to comment.