From c277f58f2a215ef7464f7f930fb98559dabd29e9 Mon Sep 17 00:00:00 2001 From: GuangyuFan <97507466+FGYFFFF@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:23:29 +0800 Subject: [PATCH 1/4] feat(hz): optional filed parse for client (#1177) --- cmd/hz/app/app.go | 4 ++++ cmd/hz/config/argument.go | 3 +++ cmd/hz/generator/package_tpl.go | 6 ++++++ cmd/hz/thrift/ast.go | 20 ++++++++++++++++---- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cmd/hz/app/app.go b/cmd/hz/app/app.go index 2885a7e56..bf2b620bb 100644 --- a/cmd/hz/app/app.go +++ b/cmd/hz/app/app.go @@ -194,6 +194,9 @@ func Init() *cli.App { handlerByMethod := cli.BoolFlag{Name: "handler_by_method", Usage: "Generate a separate handler file for each method.", Destination: &globalArgs.HandlerByMethod} trimGoPackage := cli.StringFlag{Name: "trim_gopackage", Aliases: []string{"trim_pkg"}, Usage: "Trim the prefix of go_package for protobuf.", Destination: &globalArgs.TrimGoPackage} + // client flag + enableClientOptionalFlag := cli.BoolFlag{Name: "enable_optional", Usage: "Optional field do not transfer for thrift if not set.(Only works for query tag)", Destination: &globalArgs.EnableClientOptional} + // app app := cli.NewApp() app.Name = "hz" @@ -327,6 +330,7 @@ func Init() *cli.App { &trimGoPackage, &jsonEnumStrFlag, + &enableClientOptionalFlag, &queryEnumIntFlag, &unsetOmitemptyFlag, &protoCamelJSONTag, diff --git a/cmd/hz/config/argument.go b/cmd/hz/config/argument.go index d9dcca441..2f5f5f7f4 100644 --- a/cmd/hz/config/argument.go +++ b/cmd/hz/config/argument.go @@ -76,6 +76,9 @@ type Argument struct { EnableExtends bool SortRouter bool + // client flag + EnableClientOptional bool + CustomizeLayout string CustomizeLayoutData string CustomizePackage string diff --git a/cmd/hz/generator/package_tpl.go b/cmd/hz/generator/package_tpl.go index 0efa0ccfe..7b8e2f46d 100644 --- a/cmd/hz/generator/package_tpl.go +++ b/cmd/hz/generator/package_tpl.go @@ -589,7 +589,13 @@ func (r *request) addHeaders(params map[string]string) *request { func (r *request) setQueryParam(param string, value interface{}) *request { + if value == nil { + return r + } v := reflect.ValueOf(value) + if v.Kind() == reflect.Pointer && v.IsNil() { + return r + } switch v.Kind() { case reflect.Slice, reflect.Array: for index := 0; index < v.Len(); index++ { diff --git a/cmd/hz/thrift/ast.go b/cmd/hz/thrift/ast.go index 83b27fd7f..7388ecc40 100644 --- a/cmd/hz/thrift/ast.go +++ b/cmd/hz/thrift/ast.go @@ -216,7 +216,7 @@ func astToService(ast *parser.Thrift, resolver *Resolver, args *config.Argument) if err != nil { return nil, err } - err = parseAnnotationToClient(clientMethod, m.Arguments[0].GetType(), rt) + err = parseAnnotationToClient(clientMethod, m.Arguments[0].GetType(), rt, args.EnableClientOptional) if err != nil { return nil, err } @@ -244,7 +244,7 @@ func newHTTPMethod(s *parser.Service, m *parser.Function, method *generator.Http return &newMethod, nil } -func parseAnnotationToClient(clientMethod *generator.ClientMethod, p *parser.Type, symbol ResolvedSymbol) error { +func parseAnnotationToClient(clientMethod *generator.ClientMethod, p *parser.Type, symbol ResolvedSymbol, enableOptional bool) error { if p == nil { return fmt.Errorf("get type failed for parse annotatoon to client") } @@ -270,13 +270,21 @@ func parseAnnotationToClient(clientMethod *generator.ClientMethod, p *parser.Typ for _, field := range st.Fields() { hasAnnotation := false isStringFieldType := false + isOptional := false if field.GetType().String() == "string" { isStringFieldType = true } + if field.GetRequiredness() == parser.FieldType_Optional { + isOptional = true + } if anno := getAnnotation(field.Annotations, AnnotationQuery); len(anno) > 0 { hasAnnotation = true query := checkSnakeName(anno[0]) - clientMethod.QueryParamsCode += fmt.Sprintf("%q: req.Get%s(),\n", query, field.GoName().String()) + if isOptional && enableOptional { + clientMethod.QueryParamsCode += fmt.Sprintf("%q: func() interface{} {\n\t\t\t\tif req.IsSet%s() {\n\t\t\t\t\treturn req.Get%s()\n\t\t\t\t} else {\n\t\t\t\t\treturn nil\n\t\t\t\t}}(),\n", query, field.GoName().String(), field.GoName().String()) + } else { + clientMethod.QueryParamsCode += fmt.Sprintf("%q: req.Get%s(),\n", query, field.GoName().String()) + } } if anno := getAnnotation(field.Annotations, AnnotationPath); len(anno) > 0 { @@ -326,7 +334,11 @@ func parseAnnotationToClient(clientMethod *generator.ClientMethod, p *parser.Typ // cookie do nothing } if !hasAnnotation && strings.EqualFold(clientMethod.HTTPMethod, "get") { - clientMethod.QueryParamsCode += fmt.Sprintf("%q: req.Get%s(),\n", checkSnakeName(field.GetName()), field.GoName().String()) + if isOptional && enableOptional { + clientMethod.QueryParamsCode += fmt.Sprintf("%q: func() interface{} {\n\t\t\t\tif req.IsSet%s() {\n\t\t\t\t\treturn req.Get%s()\n\t\t\t\t} else {\n\t\t\t\t\treturn nil\n\t\t\t\t}}(),\n", checkSnakeName(field.GetName()), field.GoName().String(), field.GoName().String()) + } else { + clientMethod.QueryParamsCode += fmt.Sprintf("%q: req.Get%s(),\n", checkSnakeName(field.GetName()), field.GoName().String()) + } } } clientMethod.BodyParamsCode = meta.SetBodyParam From a859259445e32cb4e597fce071045a70aa0e3115 Mon Sep 17 00:00:00 2001 From: Aliothmoon <107878625+Aliothmoon@users.noreply.github.com> Date: Thu, 2 Jan 2025 13:58:00 +0800 Subject: [PATCH 2/4] chore: update comments for ClientIP function (#1253) --- pkg/app/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/context.go b/pkg/app/context.go index 1d925b7a5..738c12334 100644 --- a/pkg/app/context.go +++ b/pkg/app/context.go @@ -1305,7 +1305,7 @@ func (ctx *RequestContext) Body() ([]byte, error) { return ctx.Request.BodyE() } -// ClientIP tries to parse the headers in [X-Real-Ip, X-Forwarded-For]. +// ClientIP attempts to parse the headers in the order of [X-Forwarded-For, X-Real-IP]. // It calls RemoteIP() under the hood. If it cannot satisfy the requirements, // use engine.SetClientIPFunc to inject your own implementation. func (ctx *RequestContext) ClientIP() string { From 572963549f7d0ad00382458fce36b2b23b8be400 Mon Sep 17 00:00:00 2001 From: Xuran <37136584+Duslia@users.noreply.github.com> Date: Thu, 2 Jan 2025 15:02:33 +0800 Subject: [PATCH 3/4] fix: call conn.release method in ext.ReleaseBodystream (#1252) --- pkg/common/test/mock/network.go | 6 ++++-- pkg/common/test/mock/network_test.go | 6 +++--- pkg/protocol/client/client_test.go | 1 - pkg/protocol/http1/ext/stream.go | 16 +++++++++++++++- pkg/protocol/http1/req/request_test.go | 1 + 5 files changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/common/test/mock/network.go b/pkg/common/test/mock/network.go index fe671ae56..b3ae54bb8 100644 --- a/pkg/common/test/mock/network.go +++ b/pkg/common/test/mock/network.go @@ -325,7 +325,8 @@ func (m *Conn) AddCloseCallback(callback netpoll.CloseCallback) error { } type StreamConn struct { - Data []byte + HasReleased bool + Data []byte } func NewStreamConn() *StreamConn { @@ -354,7 +355,8 @@ func (m *StreamConn) Skip(n int) error { } func (m *StreamConn) Release() error { - panic("implement me") + m.HasReleased = true + return nil } func (m *StreamConn) Len() int { diff --git a/pkg/common/test/mock/network_test.go b/pkg/common/test/mock/network_test.go index 4c9c4cf5b..34b0769db 100644 --- a/pkg/common/test/mock/network_test.go +++ b/pkg/common/test/mock/network_test.go @@ -174,13 +174,13 @@ func TestStreamConn(t *testing.T) { assert.DeepEqual(t, cap(conn.Data), conn.Len()) err = conn.Skip(conn.Len() + 1) assert.DeepEqual(t, "not enough data", err.Error()) + err = conn.Release() + assert.DeepEqual(t, nil, err) + assert.DeepEqual(t, true, conn.HasReleased) }) t.Run("TestNotImplement", func(t *testing.T) { conn := NewStreamConn() - assert.Panic(t, func() { - conn.Release() - }) assert.Panic(t, func() { conn.ReadByte() }) diff --git a/pkg/protocol/client/client_test.go b/pkg/protocol/client/client_test.go index 49e7e7df6..2babab4ab 100644 --- a/pkg/protocol/client/client_test.go +++ b/pkg/protocol/client/client_test.go @@ -33,7 +33,6 @@ type MockDoer struct { } func (m *MockDoer) Do(ctx context.Context, req *protocol.Request, resp *protocol.Response) error { - // this is the real logic in (c *HostClient) doNonNilReqResp method if len(req.Header.Host()) == 0 { req.Header.SetHostBytes(req.URI().Host()) diff --git a/pkg/protocol/http1/ext/stream.go b/pkg/protocol/http1/ext/stream.go index ae81b560e..c771cb82b 100644 --- a/pkg/protocol/http1/ext/stream.go +++ b/pkg/protocol/http1/ext/stream.go @@ -272,6 +272,12 @@ func (rs *bodyStream) skipRest() error { if err != nil { return err } + // After Skip, the buffer needs to be released to prevent OOM if there are too much data on conn. + err = rs.reader.Release() + if err != nil { + return err + } + } } // max value of pSize is 8193, it's safe. @@ -300,7 +306,15 @@ func (rs *bodyStream) skipRest() error { if skip > needSkipLen { skip = needSkipLen } - rs.reader.Skip(skip) + err := rs.reader.Skip(skip) + if err != nil { + return err + } + // After Skip, the buffer needs to be released to prevent OOM if there are too much data on conn. + err = rs.reader.Release() + if err != nil { + return err + } needSkipLen -= skip if needSkipLen == 0 { return nil diff --git a/pkg/protocol/http1/req/request_test.go b/pkg/protocol/http1/req/request_test.go index 0411187a5..e9451e776 100644 --- a/pkg/protocol/http1/req/request_test.go +++ b/pkg/protocol/http1/req/request_test.go @@ -1425,6 +1425,7 @@ func TestStreamNotEnoughData(t *testing.T) { err = ext.ReleaseBodyStream(req.BodyStream()) assert.Nil(t, err) assert.DeepEqual(t, 0, len(conn.Data)) + assert.DeepEqual(t, true, conn.HasReleased) } func TestRequestBodyStreamWithTrailer(t *testing.T) { From 2a22913d7b14fe5cda06d92266d8c0edd401138d Mon Sep 17 00:00:00 2001 From: alice <90381261+alice-yyds@users.noreply.github.com> Date: Fri, 3 Jan 2025 14:59:21 +0800 Subject: [PATCH 4/4] chore: update version v0.9.5 --- version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.go b/version.go index 027817a09..25a5bdcf3 100644 --- a/version.go +++ b/version.go @@ -19,5 +19,5 @@ package hertz // Name and Version info of this framework, used for statistics and debug const ( Name = "Hertz" - Version = "v0.9.4" + Version = "v0.9.5" )