From 49064e72326becb7e8b389610975d6d9189f5ad8 Mon Sep 17 00:00:00 2001 From: Cluas Date: Fri, 4 Jun 2021 16:35:55 +0800 Subject: [PATCH] encoding: remove reflect on yaml and xml (#1005) --- encoding/encoding_test.go | 92 +++++++++++++++++++++++++++++ encoding/xml/xml.go | 8 --- encoding/xml/xml_test.go | 117 +++++++++++++++++++++++++++++++++++++ encoding/yaml/yaml.go | 9 --- encoding/yaml/yaml_test.go | 92 +++++++++++++++++++++++++++++ 5 files changed, 301 insertions(+), 17 deletions(-) create mode 100644 encoding/encoding_test.go create mode 100644 encoding/xml/xml_test.go create mode 100644 encoding/yaml/yaml_test.go diff --git a/encoding/encoding_test.go b/encoding/encoding_test.go new file mode 100644 index 000000000..f39cef12f --- /dev/null +++ b/encoding/encoding_test.go @@ -0,0 +1,92 @@ +package encoding + +import ( + "encoding/xml" + "fmt" + "runtime/debug" + "testing" +) + +type codec struct{} + +func (c codec) Marshal(v interface{}) ([]byte, error) { + panic("implement me") +} + +func (c codec) Unmarshal(data []byte, v interface{}) error { + panic("implement me") +} + +func (c codec) Name() string { + return "" +} + +// codec2 is a Codec implementation with xml. +type codec2 struct{} + +func (codec2) Marshal(v interface{}) ([]byte, error) { + return xml.Marshal(v) +} + +func (codec2) Unmarshal(data []byte, v interface{}) error { + return xml.Unmarshal(data, v) +} + +func (codec2) Name() string { + return "xml" +} + +func TestRegisterCodec(t *testing.T) { + f := func() { RegisterCodec(nil) } + funcDidPanic, panicValue, _ := didPanic(f) + if !funcDidPanic { + t.Fatalf(fmt.Sprintf("func should panic\n\tPanic value:\t%#v", panicValue)) + } + if panicValue != "cannot register a nil Codec" { + t.Fatalf("panic error got %s want cannot register a nil Codec", panicValue) + } + f = func() { + RegisterCodec(codec{}) + } + funcDidPanic, panicValue, _ = didPanic(f) + if !funcDidPanic { + t.Fatalf(fmt.Sprintf("func should panic\n\tPanic value:\t%#v", panicValue)) + } + if panicValue != "cannot register Codec with empty string result for Name()" { + t.Fatalf("panic error got %s want cannot register Codec with empty string result for Name()", panicValue) + } + codec := codec2{} + RegisterCodec(codec) + got := GetCodec("xml") + if got != codec { + t.Fatalf("RegisterCodec(%v) want %v got %v", codec, codec, got) + } +} + +// PanicTestFunc defines a func that should be passed to the assert.Panics and assert.NotPanics +// methods, and represents a simple func that takes no arguments, and returns nothing. +type PanicTestFunc func() + +// didPanic returns true if the function passed to it panics. Otherwise, it returns false. +func didPanic(f PanicTestFunc) (bool, interface{}, string) { + + didPanic := false + var message interface{} + var stack string + func() { + + defer func() { + if message = recover(); message != nil { + didPanic = true + stack = string(debug.Stack()) + } + }() + + // call the target function + f() + + }() + + return didPanic, message, stack + +} diff --git a/encoding/xml/xml.go b/encoding/xml/xml.go index 1876039bf..3352fdfaf 100644 --- a/encoding/xml/xml.go +++ b/encoding/xml/xml.go @@ -2,7 +2,6 @@ package xml import ( "encoding/xml" - "reflect" "github.com/go-kratos/kratos/v2/encoding" ) @@ -22,13 +21,6 @@ func (codec) Marshal(v interface{}) ([]byte, error) { } func (codec) Unmarshal(data []byte, v interface{}) error { - rv := reflect.ValueOf(v) - for rv.Kind() == reflect.Ptr { - if rv.IsNil() { - rv.Set(reflect.New(rv.Type().Elem())) - } - rv = rv.Elem() - } return xml.Unmarshal(data, v) } diff --git a/encoding/xml/xml_test.go b/encoding/xml/xml_test.go new file mode 100644 index 000000000..685d7bc07 --- /dev/null +++ b/encoding/xml/xml_test.go @@ -0,0 +1,117 @@ +package xml + +import ( + "reflect" + "strings" + "testing" +) + +type Plain struct { + V interface{} +} + +type NestedOrder struct { + XMLName struct{} `xml:"result"` + Field1 string `xml:"parent>c"` + Field2 string `xml:"parent>b"` + Field3 string `xml:"parent>a"` +} + +func TestCodec_Marshal(t *testing.T) { + tests := []struct { + Value interface{} + ExpectXML string + }{ + // Test value types + {Value: &Plain{true}, ExpectXML: `true`}, + {Value: &Plain{false}, ExpectXML: `false`}, + {Value: &Plain{int(42)}, ExpectXML: `42`}, + { + Value: &NestedOrder{Field1: "C", Field2: "B", Field3: "A"}, + ExpectXML: `` + + `` + + `C` + + `B` + + `A` + + `` + + ``, + }, + } + for _, tt := range tests { + data, err := (codec{}).Marshal(tt.Value) + if err != nil { + t.Errorf("marshal(%#v): %s", tt.Value, err) + } + if got, want := string(data), tt.ExpectXML; got != want { + if strings.Contains(want, "\n") { + t.Errorf("marshal(%#v):\nHAVE:\n%s\nWANT:\n%s", tt.Value, got, want) + } else { + t.Errorf("marshal(%#v):\nhave %#q\nwant %#q", tt.Value, got, want) + } + } + } +} + +func TestCodec_Unmarshal(t *testing.T) { + tests := []struct { + want interface{} + InputXML string + }{ + { + want: &NestedOrder{Field1: "C", Field2: "B", Field3: "A"}, + InputXML: `` + + `` + + `C` + + `B` + + `A` + + `` + + ``}, + } + + for _, tt := range tests { + vt := reflect.TypeOf(tt.want) + dest := reflect.New(vt.Elem()).Interface() + data := []byte(tt.InputXML) + codec := codec{} + err := codec.Unmarshal(data, dest) + if err != nil { + t.Errorf("unmarshal(%#v, %#v): %s", tt.InputXML, dest, err) + } + if got, want := dest, tt.want; !reflect.DeepEqual(got, want) { + t.Errorf("unmarshal(%q):\nhave %#v\nwant %#v", tt.InputXML, got, want) + } + } +} + +func TestCodec_NilUnmarshal(t *testing.T) { + + tests := []struct { + want interface{} + InputXML string + }{ + { + want: &NestedOrder{Field1: "C", Field2: "B", Field3: "A"}, + InputXML: `` + + `` + + `C` + + `B` + + `A` + + `` + + ``}, + } + + for _, tt := range tests { + s := struct { + A string `xml:"a"` + B *NestedOrder + }{A: "a"} + data := []byte(tt.InputXML) + err := (codec{}).Unmarshal(data, &s.B) + if err != nil { + t.Errorf("unmarshal(%#v, %#v): %s", tt.InputXML, s.B, err) + } + if got, want := s.B, tt.want; !reflect.DeepEqual(got, want) { + t.Errorf("unmarshal(%q):\nhave %#v\nwant %#v", tt.InputXML, got, want) + } + } +} diff --git a/encoding/yaml/yaml.go b/encoding/yaml/yaml.go index 4e49284b2..974e88131 100644 --- a/encoding/yaml/yaml.go +++ b/encoding/yaml/yaml.go @@ -1,8 +1,6 @@ package yaml import ( - "reflect" - "github.com/go-kratos/kratos/v2/encoding" "gopkg.in/yaml.v2" ) @@ -22,13 +20,6 @@ func (codec) Marshal(v interface{}) ([]byte, error) { } func (codec) Unmarshal(data []byte, v interface{}) error { - rv := reflect.ValueOf(v) - for rv.Kind() == reflect.Ptr { - if rv.IsNil() { - rv.Set(reflect.New(rv.Type().Elem())) - } - rv = rv.Elem() - } return yaml.Unmarshal(data, v) } diff --git a/encoding/yaml/yaml_test.go b/encoding/yaml/yaml_test.go new file mode 100644 index 000000000..6459e6569 --- /dev/null +++ b/encoding/yaml/yaml_test.go @@ -0,0 +1,92 @@ +package yaml + +import ( + "math" + "reflect" + "testing" +) + +func TestCodec_Unmarshal(t *testing.T) { + + tests := []struct { + data string + value interface{} + }{ + { + "", + (*struct{})(nil), + }, + { + "{}", &struct{}{}, + }, { + "v: hi", + map[string]string{"v": "hi"}, + }, { + "v: hi", map[string]interface{}{"v": "hi"}, + }, { + "v: true", + map[string]string{"v": "true"}, + }, { + "v: true", + map[string]interface{}{"v": true}, + }, { + "v: 10", + map[string]interface{}{"v": 10}, + }, { + "v: 0b10", + map[string]interface{}{"v": 2}, + }, { + "v: 0xA", + map[string]interface{}{"v": 10}, + }, { + "v: 4294967296", + map[string]int64{"v": 4294967296}, + }, { + "v: 0.1", + map[string]interface{}{"v": 0.1}, + }, { + "v: .1", + map[string]interface{}{"v": 0.1}, + }, { + "v: .Inf", + map[string]interface{}{"v": math.Inf(+1)}, + }, { + "v: -.Inf", + map[string]interface{}{"v": math.Inf(-1)}, + }, { + "v: -10", + map[string]interface{}{"v": -10}, + }, { + "v: -.1", + map[string]interface{}{"v": -0.1}, + }, + } + for _, tt := range tests { + v := reflect.ValueOf(tt.value).Type() + value := reflect.New(v) + err := (codec{}).Unmarshal([]byte(tt.data), value.Interface()) + if err != nil { + t.Fatalf("(codec{}).Unmarshal should not return err") + } + } + spec := struct { + A string + B map[string]interface{} + }{A: "a"} + err := (codec{}).Unmarshal([]byte("v: hi"), &spec.B) + if err != nil { + t.Fatalf("(codec{}).Unmarshal should not return err") + } + +} + +func TestCodec_Marshal(t *testing.T) { + value := map[string]string{"v": "hi"} + got, err := (codec{}).Marshal(value) + if err != nil { + t.Fatalf("should not return err") + } + if string(got) != "v: hi\n" { + t.Fatalf("want \"v: hi\n\" return \"%s\"", string(got)) + } +}