[dubbo-go-hessian2] branch master updated: Fix hessian codec bugs (#167)

[wo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wongoo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/dubbo-go-hessian2.git


The following commit(s) were added to refs/heads/master by this push:
     new 0da2291  Fix hessian codec bugs (#167)
0da2291 is described below

commit 0da22914f92b7826a0c76d3ccca3ac8887110b48
Author: 诣极 yì jí <yi...@apache.org>
AuthorDate: Wed Mar 18 14:54:44 2020 +0800

    Fix hessian codec bugs (#167)
    
    * fix skip class fields.
    
    * fix skip class fields & decode response attachments.
    
    * remove unit tests.
    
    * fix code review problems.
    
    * Fix when the return value is nil
    
    * refactor code.
    
    * add UnRegisterPOJO for tests.
    
    * refactor codes.
    
    * remove construct structInfo.
    
    * cancel export unRegisterPOJOs method
---
 hessian_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 object.go       | 14 +++++++++++---
 pojo.go         | 52 ++++++++++++++++++++++++++++++++++++++++++----------
 response.go     |  9 +++++++--
 4 files changed, 107 insertions(+), 16 deletions(-)

diff --git a/hessian_test.go b/hessian_test.go
index fa74abe..e466cf2 100644
--- a/hessian_test.go
+++ b/hessian_test.go
@@ -34,8 +34,27 @@ type Case struct {
 	B int
 }
 
+type CaseA struct {
+	A string
+	B int
+	C Case
+}
+
+type CaseB struct {
+	A string
+	B CaseA
+}
+
+func (c *CaseB) JavaClassName() string {
+	return "com.test.caseb"
+}
+
+func (c CaseA) JavaClassName() string {
+	return "com.test.casea"
+}
+
 //JavaClassName  java fully qualified path
-func (c *Case) JavaClassName() string {
+func (c Case) JavaClassName() string {
 	return "com.test.case"
 }
 
@@ -171,3 +190,30 @@ func TestRequest(t *testing.T) {
 	doTestRequest(t, PackageRequest, Zero, []interface{}{"a", 3, true, []*Case{{A: "a", B: 3}}})
 	doTestRequest(t, PackageRequest, Zero, []interface{}{map[string][]*Case{"key": {{A: "a", B: 3}}}})
 }
+
+func TestHessianCodec_ReadAttachments(t *testing.T) {
+	body := &Response{
+		RspObj:      &CaseB{A: "A", B: CaseA{A: "a", B: 1, C: Case{A: "c", B: 2}}},
+		Exception:   nil,
+		Attachments: map[string]string{DUBBO_VERSION_KEY: "2.6.4"},
+	}
+	resp, err := doTestHessianEncodeHeader(t, PackageResponse, Response_OK, body)
+	assert.NoError(t, err)
+
+	unRegisterPOJOs(&CaseB{}, &CaseA{})
+	codecR1 := NewHessianCodec(bufio.NewReader(bytes.NewReader(resp)))
+	codecR2 := NewHessianCodec(bufio.NewReader(bytes.NewReader(resp)))
+	h := &DubboHeader{}
+	assert.NoError(t, codecR1.ReadHeader(h))
+	t.Log(h)
+	assert.NoError(t, codecR2.ReadHeader(h))
+	t.Log(h)
+
+	err = codecR1.ReadBody(body)
+	assert.Equal(t, "can not find go type name com.test.caseb in registry", err.Error())
+	attrs, err := codecR2.ReadAttachments()
+	assert.NoError(t, err)
+	assert.Equal(t, "2.6.4", attrs[DUBBO_VERSION_KEY])
+
+	t.Log(attrs)
+}
diff --git a/object.go b/object.go
index 857ae05..4025dac 100644
--- a/object.go
+++ b/object.go
@@ -541,11 +541,19 @@ func (d *Decoder) decEnum(javaName string, flag int32) (JavaEnum, error) {
 
 // skip this object
 func (d *Decoder) skip(cls classInfo) error {
-	if len(cls.fieldNameList) < 1 {
+	len := len(cls.fieldNameList)
+	if len < 1 {
 		return nil
 	}
-	_, err := d.DecodeValue()
-	return err
+
+	for i := 0; i < len; i++ {
+		// skip class fields.
+		if _, err := d.DecodeValue(); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 func (d *Decoder) decObject(flag int32) (interface{}, error) {
diff --git a/pojo.go b/pojo.go
index ed5ab8b..0c9f9aa 100644
--- a/pojo.go
+++ b/pojo.go
@@ -130,18 +130,9 @@ func RegisterPOJO(o POJO) int {
 		fieldList  []string
 		structInfo structInfo
 		clsDef     classInfo
-		v          reflect.Value
 	)
 
-	v = reflect.ValueOf(o)
-	switch v.Kind() {
-	case reflect.Struct:
-		structInfo.typ = v.Type()
-	case reflect.Ptr:
-		structInfo.typ = v.Elem().Type()
-	default:
-		structInfo.typ = reflect.TypeOf(o)
-	}
+	structInfo.typ = obtainValueType(o)
 
 	structInfo.goName = structInfo.typ.String()
 	structInfo.javaName = o.JavaClassName()
@@ -210,6 +201,47 @@ func RegisterPOJO(o POJO) int {
 	return structInfo.index
 }
 
+// easy for test
+func unRegisterPOJOs(os ...POJO) []int {
+	arr := make([]int, len(os))
+	for i := range os {
+		arr[i] = unRegisterPOJO(os[i])
+	}
+
+	return arr
+}
+
+func unRegisterPOJO(o POJO) int {
+	pojoRegistry.Lock()
+	defer pojoRegistry.Unlock()
+
+	goName := obtainValueType(o).String()
+
+	if structInfo, ok := pojoRegistry.registry[goName]; ok {
+		delete(pojoRegistry.j2g, structInfo.javaName)
+		listTypeNameMapper.Delete(structInfo.goName)
+		// remove registry cache.
+		delete(pojoRegistry.registry, structInfo.goName)
+		// don't remove registry classInfoList,
+		// indexes of registered pojo may be affected.
+		return structInfo.index
+	}
+
+	return -1
+}
+
+func obtainValueType(o POJO) reflect.Type {
+	v := reflect.ValueOf(o)
+	switch v.Kind() {
+	case reflect.Struct:
+		return v.Type()
+	case reflect.Ptr:
+		return v.Elem().Type()
+	}
+
+	return reflect.TypeOf(o)
+}
+
 // RegisterPOJOs register a POJO instance arr @os. The return value is @os's
 // mathching index array, in which "-1" means its matching POJO has been registered.
 func RegisterPOJOs(os ...POJO) []int {
diff --git a/response.go b/response.go
index 4ba9e39..1fede8e 100644
--- a/response.go
+++ b/response.go
@@ -199,13 +199,18 @@ func unpackResponseBody(decoder *Decoder, resp interface{}) error {
 				return perrors.WithStack(err)
 			}
 			if v, ok := attachments.(map[interface{}]interface{}); ok {
-				atta := ToMapStringString(v)
-				response.Attachments = atta
+				response.Attachments = ToMapStringString(v)
 			} else {
 				return perrors.Errorf("get wrong attachments: %+v", attachments)
 			}
 		}
 
+		// If the return value is nil,
+		// we should consider it normal
+		if rsp == nil {
+			return nil
+		}
+
 		return perrors.WithStack(ReflectResponse(rsp, response.RspObj))
 
 	case RESPONSE_NULL_VALUE, RESPONSE_NULL_VALUE_WITH_ATTACHMENTS: