From e6123c7bb3c43ff8edfe51a40a65acd62c9694fb Mon Sep 17 00:00:00 2001 From: Di Weng Date: Mon, 28 Feb 2022 16:35:53 +0800 Subject: [PATCH] Better Dynamic converter (#78) * allow converting dynamic to slice * reorganize dynamic converter * support more types of slices * add test * set values for pointers directly in dynamic converter * add tests for dynamic converter * merge map into slice conversion * add tests for dynamic converter * create pointer if not initialized * Modernized and added negative tests * Add unit test results from forked run * explicit file name * explicit file name * Fixed race Co-authored-by: AsafMah --- .github/workflows/unit_test_results.yml | 2 +- kusto/data/table/from_kusto_test.go | 216 +++++++++++++++++++++--- kusto/data/value/dynamic.go | 80 +++------ kusto/data/value/dynamic_test.go | 212 +++++++++++++++++++++++ 4 files changed, 433 insertions(+), 77 deletions(-) create mode 100644 kusto/data/value/dynamic_test.go diff --git a/.github/workflows/unit_test_results.yml b/.github/workflows/unit_test_results.yml index fb0ea91b..f2b34398 100644 --- a/.github/workflows/unit_test_results.yml +++ b/.github/workflows/unit_test_results.yml @@ -34,4 +34,4 @@ jobs: commit: ${{ github.event.workflow_run.head_sha }} event_file: artifacts/Event File/event.json event_name: ${{ github.event.workflow_run.event }} - files: "artifacts/**/*.xml" \ No newline at end of file + files: "artifacts/report.xml" diff --git a/kusto/data/table/from_kusto_test.go b/kusto/data/table/from_kusto_test.go index 7c003cc5..dcb7988f 100644 --- a/kusto/data/table/from_kusto_test.go +++ b/kusto/data/table/from_kusto_test.go @@ -8,9 +8,9 @@ import ( "github.com/Azure/azure-kusto-go/kusto/data/types" "github.com/Azure/azure-kusto-go/kusto/data/value" + "github.com/stretchr/testify/assert" "github.com/google/uuid" - "github.com/kylelemons/godebug/pretty" ) var now = time.Now() @@ -29,17 +29,43 @@ func TestFieldsConvert(t *testing.T) { ID: 1, } + myArrayOfStruct := []SomeJSON{ + { + Name: "Adam", + ID: 1, + }, + { + Name: "Bob", + ID: 2, + }, + } + myJSON, err := json.Marshal(myStruct) if err != nil { panic(err) } + + myJSONArray, err := json.Marshal(myArrayOfStruct) + if err != nil { + panic(err) + } + myJSONStr := string(myJSON) myJSONStrPtr := &myJSONStr + + myJSONArrayStr := string(myJSONArray) + myJSONArrayStrPtr := &myJSONArrayStr + jsonMap := map[string]interface{}{} if err := json.Unmarshal(myJSON, &jsonMap); err != nil { panic(err) } + jsonList := []interface{}{} + if err := json.Unmarshal(myJSONArray, &jsonList); err != nil { + panic(err) + } + tests := []struct { desc string columns Columns @@ -180,16 +206,166 @@ func TestFieldsConvert(t *testing.T) { myJSONStrPtr, map[string]interface{}{ "Name": "Adam", - "ID": 1, + "ID": float64(1), }, &map[string]interface{}{ "Name": "Adam", - "ID": 1, + "ID": float64(1), }, value.Dynamic{Value: myJSON, Valid: true}, &value.Dynamic{Value: myJSON, Valid: true}, }, }, + { + desc: "valid Dynamic list", + columns: Columns{ + {Type: types.Dynamic, Name: "Struct"}, + {Type: types.Dynamic, Name: "PtrStruct"}, + {Type: types.Dynamic, Name: "String"}, + {Type: types.Dynamic, Name: "PtrString"}, + {Type: types.Dynamic, Name: "Slice"}, + {Type: types.Dynamic, Name: "PtrSlice"}, + {Type: types.Dynamic, Name: "Dynamic"}, + {Type: types.Dynamic, Name: "PtrDynamic"}, + }, + k: value.Dynamic{Value: myJSONArray, Valid: true}, + ptrStruct: &struct { + Struct []SomeJSON + PtrStruct *[]SomeJSON + String string + PtrString *string + Slice []map[string]interface{} + PtrSlice *[]map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{}, + err: false, + want: &struct { + Struct []SomeJSON + PtrStruct *[]SomeJSON + String string + PtrString *string + Slice []map[string]interface{} + PtrSlice *[]map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{ + myArrayOfStruct, + &myArrayOfStruct, + myJSONArrayStr, + myJSONArrayStrPtr, + []map[string]interface{}{ + { + "Name": "Adam", + "ID": float64(1), + }, + { + "Name": "Bob", + "ID": float64(2), + }, + }, + &[]map[string]interface{}{ + { + "Name": "Adam", + "ID": float64(1), + }, + { + "Name": "Bob", + "ID": float64(2), + }, + }, + value.Dynamic{Value: myJSONArray, Valid: true}, + &value.Dynamic{Value: myJSONArray, Valid: true}, + }, + }, + { + desc: "non-valid Dynamic", + columns: Columns{ + {Type: types.Dynamic, Name: "Struct"}, + {Type: types.Dynamic, Name: "PtrStruct"}, + {Type: types.Dynamic, Name: "String"}, + {Type: types.Dynamic, Name: "PtrString"}, + {Type: types.Dynamic, Name: "Map"}, + {Type: types.Dynamic, Name: "PtrMap"}, + {Type: types.Dynamic, Name: "Dynamic"}, + {Type: types.Dynamic, Name: "PtrDynamic"}, + }, + k: value.Dynamic{Value: myJSON, Valid: false}, + ptrStruct: &struct { + Struct SomeJSON + PtrStruct *SomeJSON + String string + PtrString *string + Map map[string]interface{} + PtrMap *map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{}, + err: false, + want: &struct { + Struct SomeJSON + PtrStruct *SomeJSON + String string + PtrString *string + Map map[string]interface{} + PtrMap *map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{ + myStruct, + &myStruct, + myJSONStr, + myJSONStrPtr, + nil, + nil, + value.Dynamic{Value: myJSON, Valid: false}, + &value.Dynamic{Value: myJSON, Valid: false}, + }, + }, + { + desc: "non-valid Dynamic list", + columns: Columns{ + {Type: types.Dynamic, Name: "Struct"}, + {Type: types.Dynamic, Name: "PtrStruct"}, + {Type: types.Dynamic, Name: "String"}, + {Type: types.Dynamic, Name: "PtrString"}, + {Type: types.Dynamic, Name: "Slice"}, + {Type: types.Dynamic, Name: "PtrSlice"}, + {Type: types.Dynamic, Name: "Dynamic"}, + {Type: types.Dynamic, Name: "PtrDynamic"}, + }, + k: value.Dynamic{Value: myJSONArray, Valid: false}, + ptrStruct: &struct { + Struct []SomeJSON + PtrStruct *[]SomeJSON + String string + PtrString *string + Slice []map[string]interface{} + PtrSlice *[]map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{}, + err: false, + want: &struct { + Struct []SomeJSON + PtrStruct *[]SomeJSON + String string + PtrString *string + Slice []map[string]interface{} + PtrSlice *[]map[string]interface{} + Dynamic value.Dynamic + PtrDynamic *value.Dynamic + }{ + nil, + nil, + myJSONArrayStr, + myJSONArrayStrPtr, + nil, + nil, + value.Dynamic{Value: myJSONArray, Valid: false}, + &value.Dynamic{Value: myJSONArray, Valid: false}, + }, + }, { desc: "valid GUID", columns: Columns{ @@ -492,27 +668,25 @@ func TestFieldsConvert(t *testing.T) { } for _, test := range tests { - fields := newFields(test.columns, reflect.TypeOf(test.ptrStruct)) + test := test // Capture + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + fields := newFields(test.columns, reflect.TypeOf(test.ptrStruct)) - ty := reflect.TypeOf(test.ptrStruct) - v := reflect.ValueOf(test.ptrStruct) - for _, column := range test.columns { - err = fields.convert(column, test.k, ty, v) - switch { - case err == nil && test.err: - t.Errorf("TestFieldsConvert(%s): got err == nil, want err != nil", test.desc) - continue - case err != nil && !test.err: - t.Errorf("TestFieldsConvert(%s): got err == %s, want err == nil", test.desc, err) - continue - case err != nil: - continue + ty := reflect.TypeOf(test.ptrStruct) + v := reflect.ValueOf(test.ptrStruct) + for _, column := range test.columns { + err := fields.convert(column, test.k, ty, v) + if test.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } - } - if diff := pretty.Compare(test.want, test.ptrStruct); diff != "" { - t.Errorf("TestFieldsConvert(%s): -want/+got:\n%s", test.desc, diff) - } + assert.EqualValues(t, test.want, test.ptrStruct) + }) + } } diff --git a/kusto/data/value/dynamic.go b/kusto/data/value/dynamic.go index 75b0ac72..e78ea389 100644 --- a/kusto/data/value/dynamic.go +++ b/kusto/data/value/dynamic.go @@ -58,39 +58,32 @@ func (d *Dynamic) Unmarshal(i interface{}) error { // Convert Dynamic into reflect value. func (d Dynamic) Convert(v reflect.Value) error { t := v.Type() + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + + var valueToSet reflect.Value switch { case t.ConvertibleTo(reflect.TypeOf(Dynamic{})): - v.Set(reflect.ValueOf(d)) - return nil - case t.ConvertibleTo(reflect.TypeOf(&Dynamic{})): - v.Set(reflect.ValueOf(&d)) - return nil + valueToSet = reflect.ValueOf(d) case t.ConvertibleTo(reflect.TypeOf([]byte{})): if t.Kind() == reflect.String { s := string(d.Value) - v.Set(reflect.ValueOf(s)) - return nil + valueToSet = reflect.ValueOf(s) + } else { + valueToSet = reflect.ValueOf(d.Value) } - v.Set(reflect.ValueOf(d.Value)) - return nil - case t.Kind() == reflect.Map: + case t.Kind() == reflect.Slice || t.Kind() == reflect.Map: if !d.Valid { return nil } - if t.Key().Kind() != reflect.String { - return fmt.Errorf("Type dymanic and can only be stored in a string, *string, map[string]interface{}, *map[string]interface{}, struct or *struct") - } - if t.Elem().Kind() != reflect.Interface { - return fmt.Errorf("Type dymanic and can only be stored in a string, *string, map[string]interface{}, *map[string]interface{}, struct or *struct") - } - m := map[string]interface{}{} - if err := json.Unmarshal([]byte(d.Value), &m); err != nil { - return fmt.Errorf("Error occurred while trying to marshal type dynamic into a map[string]interface{}: %s", err) + ptr := reflect.New(t) + if err := json.Unmarshal([]byte(d.Value), ptr.Interface()); err != nil { + return fmt.Errorf("Error occurred while trying to unmarshal Dynamic into a %s: %s", t.Kind(), err) } - v.Set(reflect.ValueOf(m)) - return nil + valueToSet = ptr.Elem() case t.Kind() == reflect.Struct: structPtr := reflect.New(t) @@ -98,41 +91,18 @@ func (d Dynamic) Convert(v reflect.Value) error { return fmt.Errorf("Could not unmarshal type dynamic into receiver: %s", err) } - v.Set(structPtr.Elem()) - return nil - case t.Kind() == reflect.Ptr: - if !d.Valid { - return nil - } - - switch { - case t.Elem().Kind() == reflect.String: - str := string(d.Value) - v.Set(reflect.ValueOf(&str)) - return nil - case t.Elem().Kind() == reflect.Struct: - store := reflect.New(t.Elem()) + valueToSet = structPtr.Elem() + default: + return fmt.Errorf("Column was type Kusto.Dynamic, receiver had base Kind %s ", t.Kind()) + } - if err := json.Unmarshal([]byte(d.Value), store.Interface()); err != nil { - return fmt.Errorf("Could not unmarshal type dynamic into receiver: %s", err) - } - v.Set(store) - return nil - case t.Elem().Kind() == reflect.Map: - if t.Elem().Key().Kind() != reflect.String { - return fmt.Errorf("Type dymanic can only be stored in a map of type map[string]interface{}") - } - if t.Elem().Elem().Kind() != reflect.Interface { - return fmt.Errorf("Type dymanic and can only be stored in a of type map[string]interface{}") - } - - m := map[string]interface{}{} - if err := json.Unmarshal([]byte(d.Value), &m); err != nil { - return fmt.Errorf("Error occurred while trying to marshal type dynamic into a map[string]interface{}: %s", err) - } - v.Set(reflect.ValueOf(&m)) - return nil + if v.Type().Kind() != reflect.Ptr { + v.Set(valueToSet) + } else { + if v.IsZero() { + v.Set(reflect.New(valueToSet.Type())) } + v.Elem().Set(valueToSet) } - return fmt.Errorf("Column was type Kusto.Dynamic, receiver had base Kind %s ", t.Kind()) + return nil } diff --git a/kusto/data/value/dynamic_test.go b/kusto/data/value/dynamic_test.go new file mode 100644 index 00000000..31dda752 --- /dev/null +++ b/kusto/data/value/dynamic_test.go @@ -0,0 +1,212 @@ +package value_test + +import ( + "reflect" + "testing" + + "github.com/Azure/azure-kusto-go/kusto/data/value" + "github.com/stretchr/testify/assert" +) + +type DynamicConverterTestCase struct { + Desc string + Value value.Dynamic + Target reflect.Value + Want interface{} +} + +type DynamicConverterNegativeTestCase struct { + Desc string + Value value.Dynamic + Target reflect.Value + Error string +} + +type TestStruct struct { + Name string `json:"name"` + ID int64 `json:"id"` +} + +func TestDynamicConverter(t *testing.T) { + t.Parallel() + + wantByteArray := []byte(`hello`) + emptyStr := "" + wantStr := "hello" + + testCases := []DynamicConverterTestCase{ + { + Desc: "convert to dynamic", + Value: value.Dynamic{Value: []byte(`hello`), Valid: true}, + Target: reflect.ValueOf(&value.Dynamic{}), + Want: &value.Dynamic{Value: []byte(`hello`), Valid: true}, + }, + { + Desc: "convert to []byte", + Value: value.Dynamic{Value: []byte(`hello`), Valid: true}, + Target: reflect.ValueOf(&[]byte{}), + Want: &wantByteArray, + }, + { + Desc: "convert to string", + Value: value.Dynamic{Value: []byte(`hello`), Valid: true}, + Target: reflect.ValueOf(&emptyStr), + Want: &wantStr, + }, + { + Desc: "convert to []string", + Value: value.Dynamic{Value: []byte(`["hello", "world"]`), Valid: true}, + Target: reflect.ValueOf(&[]string{}), + Want: &[]string{"hello", "world"}, + }, + { + Desc: "convert to []int64", + Value: value.Dynamic{Value: []byte(`[1,2,3]`), Valid: true}, + Target: reflect.ValueOf(&[]int64{}), + Want: &[]int64{1, 2, 3}, + }, + { + Desc: "convert to []struct", + Value: value.Dynamic{Value: []byte(`[{"name":"A","id":1},{"name":"B","id":2}]`), Valid: true}, + Target: reflect.ValueOf(&[]TestStruct{}), + Want: &[]TestStruct{ + {Name: "A", ID: 1}, + {Name: "B", ID: 2}, + }, + }, + { + Desc: "convert to []map[string]interface{}", + Value: value.Dynamic{Value: []byte(`[{"name":"A","id":1},{"name":"B","id":2}]`), Valid: true}, + Target: reflect.ValueOf(&[]map[string]interface{}{}), + Want: &[]map[string]interface{}{ + {"name": "A", "id": float64(1)}, + {"name": "B", "id": float64(2)}, + }, + }, + { + Desc: "convert to []map[string]struct", + Value: value.Dynamic{Value: []byte(`[{"group1":{"name":"A","id":1}},{"group2":{"name":"B","id":2}}]`), Valid: true}, + Target: reflect.ValueOf(&[]map[string]TestStruct{}), + Want: &[]map[string]TestStruct{ + {"group1": {Name: "A", ID: 1}}, + {"group2": {Name: "B", ID: 2}}, + }, + }, + { + Desc: "convert to struct", + Value: value.Dynamic{Value: []byte(`{"name":"A","id":1}`), Valid: true}, + Target: reflect.ValueOf(&TestStruct{}), + Want: &TestStruct{ + Name: "A", + ID: 1, + }, + }, + { + Desc: "convert to map[string]interface{}", + Value: value.Dynamic{Value: []byte(`{"name":"A","id":1}`), Valid: true}, + Target: reflect.ValueOf(&map[string]interface{}{}), + Want: &map[string]interface{}{ + "name": "A", + "id": float64(1), + }, + }, + { + Desc: "convert to map[string]struct", + Value: value.Dynamic{Value: []byte(`{"group1": {"name":"A","id":1}}`), Valid: true}, + Target: reflect.ValueOf(&map[string]TestStruct{}), + Want: &map[string]TestStruct{ + "group1": { + Name: "A", + ID: 1, + }, + }, + }, + } + + for _, tc := range testCases { + tc := tc // capture range variable + t.Run(tc.Desc, func(t *testing.T) { + t.Parallel() + + err := tc.Value.Convert(tc.Target.Elem()) + assert.NoError(t, err) + + assert.EqualValues(t, tc.Want, tc.Target.Interface()) + + err = tc.Value.Convert(tc.Target) + assert.NoError(t, err) + + assert.EqualValues(t, tc.Want, tc.Target.Interface()) + }) + + } +} + +func TestDynamicConverterNegative(t *testing.T) { + t.Parallel() + + testCases := []DynamicConverterNegativeTestCase{ + { + Desc: "fail to convert to []string", + Value: value.Dynamic{Value: []byte(`["hello", "world`), Valid: true}, + Target: reflect.ValueOf(&[]string{}), + Error: "Error occurred while trying to unmarshal Dynamic into a slice: unexpected end of JSON input", + }, + { + Desc: "fail to convert to []int64", + Value: value.Dynamic{Value: []byte(`[1,2,"3"]`), Valid: true}, + Target: reflect.ValueOf(&[]int64{}), + Error: "Error occurred while trying to unmarshal Dynamic into a slice: json: cannot unmarshal string into Go value of type int64", + }, + { + Desc: "fail to convert to []struct", + Value: value.Dynamic{Value: []byte(`[{"name":"A","id":1},{"name":"B","id":2}`), Valid: true}, + Target: reflect.ValueOf(&[]TestStruct{}), + Error: "Error occurred while trying to unmarshal Dynamic into a slice: unexpected end of JSON input", + }, + { + Desc: "convert to []map[string]interface{}", + Value: value.Dynamic{Value: []byte(`[{"name":"A","id":1},{"name":"B","id":2}`), Valid: true}, + Target: reflect.ValueOf(&[]map[string]interface{}{}), + Error: "Error occurred while trying to unmarshal Dynamic into a slice: unexpected end of JSON input", + }, + { + Desc: "convert to []map[string]struct", + Value: value.Dynamic{Value: []byte(`[{"group1":{"name":"A","id":1}},{"group2":{"name":"B","id":2}}`), Valid: true}, + Target: reflect.ValueOf(&[]map[string]TestStruct{}), + Error: "Error occurred while trying to unmarshal Dynamic into a slice: unexpected end of JSON input", + }, + { + Desc: "convert to struct", + Value: value.Dynamic{Value: []byte(`{"name":"A","id":1`), Valid: true}, + Target: reflect.ValueOf(&TestStruct{}), + Error: "Could not unmarshal type dynamic into receiver: unexpected end of JSON input", + }, + { + Desc: "convert to map[string]interface{}", + Value: value.Dynamic{Value: []byte(`{"named":"A","id":1`), Valid: true}, + Target: reflect.ValueOf(&map[string]interface{}{}), + Error: "Error occurred while trying to unmarshal Dynamic into a map: unexpected end of JSON input", + }, + { + Desc: "convert to map[string]struct", + Value: value.Dynamic{Value: []byte(`{"group1":{"named":"A","id":1}`), Valid: true}, + Target: reflect.ValueOf(&map[string]TestStruct{}), + Error: "Error occurred while trying to unmarshal Dynamic into a map: unexpected end of JSON input", + }, + } + + for _, tc := range testCases { + tc := tc // capture range variable + t.Run(tc.Desc, func(t *testing.T) { + t.Parallel() + + err := tc.Value.Convert(tc.Target.Elem()) + assert.EqualError(t, err, tc.Error) + + err = tc.Value.Convert(tc.Target) + assert.EqualError(t, err, tc.Error) + }) + + } +}