fix: rebase skip decoder and add unit test

protocol/thrift/apache/adaptor/adaptor.go

@@ -25,7 +25,7 @@ import (
 )
 
 // AdaptRead receive a kitex binary protocol and read it by given function.
-func AdaptRead(p, iprot interface{}) error {
+func AdaptRead(p, iprot interface{}) (err error) {
 	// for now, we use fastCodec to adapt apache codec.
 	// the struct should have the function 'FastRead'
 	fastStruct, ok := p.(fastReader)
@@ -70,26 +70,24 @@ func AdaptRead(p, iprot interface{}) error {
 			}
 		}
 	}
-	if rd == nil && br == nil {
-		return fmt.Errorf("no available field for reader")
-	}
 
-	var sd *thrift.SkipDecoder
+	var buf []byte
 	if br != nil {
-		sd = thrift.NewSkipDecoder(br)
+		sd := thrift.NewSkipDecoder(br)
+		buf, err = sd.Next(thrift.STRUCT)
+		sd.Release()
+	} else if rd != nil {
+		sd := thrift.NewReaderSkipDecoder(rd)
+		buf, err = sd.Next(thrift.STRUCT)
+		sd.Release()
 	} else {
-		// if there's no bufiox.Reader, do not wrap a new bufiox.Reader, or some data will remain in the buffer
-		// directly read from io.Reader
-		sd = thrift.NewSkipDecoderWithIOReader(rd)
+		return fmt.Errorf("no available field for reader")
 	}
 
-	buf, err := sd.Next(thrift.STRUCT)
 	if err != nil {
 		return err
 	}
 
-	sd.Release()
-
 	// unmarshal the data into struct
 	_, err = fastStruct.FastRead(buf)
 
@@ -148,7 +146,11 @@ func AdaptWrite(p, oprot interface{}) error {
 
 	// use fast codec
 	buf := make([]byte, fastStruct.BLength())
-	buf = buf[:fastStruct.FastWriteNocopy(buf, nil)]
+	n := fastStruct.FastWriteNocopy(buf, nil)
+	if n < 0 {
+		return fmt.Errorf("failed to fast write")
+	}
+	buf = buf[:n]
 	_, err = bw.WriteBinary(buf)
 	if err != nil {
 		return err

protocol/thrift/apache/adaptor/adaptor_apache_test.go

@@ -0,0 +1,72 @@
+// Copyright 2024 CloudWeGo Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package adaptor_test
+
+import (
+	"bytes"
+	"io"
+	"testing"
+)
+
+// TestApacheProtocol
+func TestApacheProtocol(t *testing.T) {
+	// Apache Protocol implement the interface TProtocol (https://github.com/apache/thrift/blob/v0.13.0/lib/go/thrift/protocol.go#L33)
+	// The implementation classes of this interface include many protocols.
+	// Such as TBinaryProtocol, TCompactProtocol, THeaderProtocol, and TJsonProtocol.
+	// However, for users of cloudwego/kitex, only TBinaryProtocol and TCompactProtocol are used
+	// So we only support to adapt these two protocols.
+
+	// apache binary protocol with old kitex struct
+	testAdaptor(t, oldKitexGen, mockTBinaryProtocol())
+
+	// apache binary protocol with new kitex struct
+	testAdaptor(t, newKitexGen, mockTBinaryProtocol())
+
+	// apache compact protocol with old kitex struct
+	testAdaptor(t, oldKitexGen, mockTCompactProtocol())
+
+	// apache compact protocol with new kitex struct
+	testAdaptor(t, newKitexGen, mockTCompactProtocol())
+}
+
+// tBinaryProtocol
+// https://github.com/apache/thrift/blob/v0.13.0/lib/go/thrift/binary_protocol.go#L33
+type tBinaryProtocol struct {
+	trans tRichTransport
+}
+
+func mockTBinaryProtocol() *tBinaryProtocol {
+	return &tBinaryProtocol{
+		trans: bytes.NewBuffer(nil),
+	}
+}
+
+// tCompactProtocol
+// https://github.com/apache/thrift/blob/v0.13.0/lib/go/thrift/compact_protocol.go#L88
+type tCompactProtocol struct {
+	trans tRichTransport
+}
+
+func mockTCompactProtocol() *tCompactProtocol {
+	return &tCompactProtocol{
+		trans: bytes.NewBuffer(nil),
+	}
+}
+
+// https://github.com/apache/thrift/blob/v0.13.0/lib/go/thrift/trans.go
+type tRichTransport interface {
+	io.ReadWriter
+	// another interfaces are not used in apache adaptor, just ignore them.
+}