fix:(thrift) api.body shoudn't change field alias by default (for j…

…son-generic)
cloudwego · Jan 7, 2025 · 74bc9f2 · 74bc9f2
1 parent 55704ea
commit 74bc9f2
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 28 deletions.
diff --git a/conv/j2t/conv_test.go b/conv/j2t/conv_test.go
@@ -552,29 +552,51 @@ func TestNullJSON2Thrift(t *testing.T) {
 }
 
 func TestApiBody(t *testing.T) {
-	desc := getExampleDescByName("ApiBodyMethod", true, thrift.Options{})
-	data := []byte(`{"code":1024,"InnerCode":{}}`)
-	cv := NewBinaryConv(conv.Options{
-		EnableHttpMapping:            true,
-		WriteDefaultField:            true,
-		ReadHttpValueFallback:        true,
-		TracebackRequredOrRootFields: true,
+	t.Run("http", func(t *testing.T) {
+		desc := getExampleDescByName("ApiBodyMethod", true, thrift.Options{
+			ApiBodyFastPath: true,
+		})
+		data := []byte(`{"code":1024,"Code":2048,"InnerCode":{}}`)
+		cv := NewBinaryConv(conv.Options{
+			EnableHttpMapping:            true,
+			WriteDefaultField:            true,
+			ReadHttpValueFallback:        true,
+			TracebackRequredOrRootFields: true,
+		})
+		ctx := context.Background()
+		req, err := stdh.NewRequest("POST", "http://localhost:8888/root", bytes.NewBuffer(data))
+		require.Nil(t, err)
+		req.Header.Set("Content-Type", "application/json")
+		r, _ := http.NewHTTPRequestFromStdReq(req)
+		ctx = context.WithValue(ctx, conv.CtxKeyHTTPRequest, r)
+		out, err := cv.Do(ctx, desc, data)
+		require.Nil(t, err)
+		act := example3.NewExampleApiBody()
+		_, err = act.FastRead(out)
+		require.Nil(t, err)
+		require.Equal(t, int64(1024), act.Code)
+		require.Equal(t, int16(1024), act.Code2)
+		require.Equal(t, int64(1024), act.InnerCode.C1)
+		require.Equal(t, int16(0), act.InnerCode.C2)
+	})
+	t.Run("not http", func(t *testing.T) {
+		desc := getExampleDescByName("ApiBodyMethod", true, thrift.Options{
+			ApiBodyFastPath: false,
+		})
+		data := []byte(`{"code":1024,"Code":2048,"InnerCode":{"C1":1,"code":2}}`)
+		cv := NewBinaryConv(conv.Options{
+			WriteDefaultField: true,
+		})
+		out, err := cv.Do(context.Background(), desc, data)
+		require.Nil(t, err)
+		act := example3.NewExampleApiBody()
+		_, err = act.FastRead(out)
+		require.Nil(t, err)
+		require.Equal(t, int64(2048), act.Code)
+		require.Equal(t, int16(1024), act.Code2)
+		require.Equal(t, int64(1), act.InnerCode.C1)
+		require.Equal(t, int16(2), act.InnerCode.C2)
 	})
-	ctx := context.Background()
-	req, err := stdh.NewRequest("POST", "http://localhost:8888/root", bytes.NewBuffer(data))
-	require.Nil(t, err)
-	req.Header.Set("Content-Type", "application/json")
-	r, err := http.NewHTTPRequestFromStdReq(req)
-	ctx = context.WithValue(ctx, conv.CtxKeyHTTPRequest, r)
-	out, err := cv.Do(ctx, desc, data)
-	require.Nil(t, err)
-	act := example3.NewExampleApiBody()
-	_, err = act.FastRead(out)
-	require.Nil(t, err)
-	require.Equal(t, int64(1024), act.Code)
-	require.Equal(t, int16(1024), act.Code2)
-	require.Equal(t, int64(1024), act.InnerCode.C1)
-	require.Equal(t, int16(0), act.InnerCode.C2)
 }
 
 func TestError(t *testing.T) {

diff --git a/thrift/annotation/anno_mapping.go b/thrift/annotation/anno_mapping.go
@@ -71,13 +71,13 @@ func (m apiBodyMapper) Map(ctx context.Context, anns []parser.Annotation, desc i
 		if len(ann.Values) != 1 {
 			return nil, nil, errors.New("api.body must have a value")
 		}
-		ret = append(ret, parser.Annotation{
-			Key:    APIKeyName,
-			Values: []string{ann.Values[0]},
-		})
 		isRoot := ctx.Value(thrift.CtxKeyIsBodyRoot)
-		// special fast-path: if the field is at body root, we don't need to add api.body
-		if isRoot != nil && isRoot.(bool) {
+		// special fast-path: if the field is at body root, we don't need to add api.body to call GetMapBody()
+		if opts.ApiBodyFastPath && isRoot != nil && isRoot.(bool) {
+			ret = append(ret, parser.Annotation{
+				Key:    APIKeyName,
+				Values: []string{ann.Values[0]},
+			})
 			continue
 		} else {
 			ret = append(ret, parser.Annotation{

diff --git a/thrift/idl.go b/thrift/idl.go
@@ -94,6 +94,9 @@ type Options struct {
 	//      `// path := /a/b/c.thrift` will got ["/a/b/c.thrift"]
 	// NOTICE: at present, only StructDescriptor.Annotations() can get this
 	PutThriftFilenameToAnnotation bool
+
+	// ApiBodyFastPath indicates `api.body` will change alias-name of root field, which can avoid search http-body on them
+	ApiBodyFastPath bool
 }
 
 // NewDefaultOptions creates a default Options.