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 <asafmahlev@microsoft.com>

.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"
+          files: "artifacts/report.xml"

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)
+		})
+
 	}
 }
 

kusto/data/value/dynamic.go

@@ -58,81 +58,51 @@ 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)
 
 		if err := json.Unmarshal([]byte(d.Value), structPtr.Interface()); err != nil {
 			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
 }