Skip to content

Commit

Permalink
fix(go): map values incorrectly handled though de/serialization (#2587)
Browse files Browse the repository at this point in the history
Map values were not handled in the de/serialization routines, resulting
in incorrect behavior.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller authored Feb 17, 2021
1 parent e4a4d3c commit 0359928
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 23 deletions.
21 changes: 21 additions & 0 deletions packages/@jsii/go-runtime/jsii-calc-test/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,24 @@ func TestReturnsSpecialParam(t *testing.T) {
t.Errorf("Expected type: %s; Actual: %s", expected, actual)
}
}

func TestMaps(t *testing.T) {
allTypes := calc.NewAllTypes()
actual := allTypes.GetMapProperty()
if len(actual) != 0 {
t.Errorf("Expected length of empty map to be 0. Got: %d", len(actual))
}

question := "The answer to the ultimate question of life, the universe, and everything"
answer := calclib.NewNumber(42)
allTypes.SetMapProperty(map[string]calclib.NumberIface{
question: answer,
})
actual = allTypes.GetMapProperty()
if len(actual) != 1 {
t.Errorf("Expected length of empty map to be 1. Got: %d", len(actual))
}
if actual[question].GetValue() != answer.GetValue() {
t.Errorf("Expected to have the value %v in there, got: %v", answer, actual[question])
}
}
16 changes: 10 additions & 6 deletions packages/@jsii/go-runtime/jsii-runtime-go/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ type enumRef struct {
MemberFQN string `json:"$jsii.enum"`
}

type wireMap struct {
MapData map[string]interface{} `json:"$jsii.map"`
}

type loadRequest struct {
kernelRequester

Expand Down Expand Up @@ -129,7 +133,7 @@ type createResponse struct {
type delRequest struct {
kernelRequester

API string `json:"api"`
API string `json:"api"`
ObjRef objectRef `json:"objref"`
}

Expand All @@ -140,8 +144,8 @@ type delResponse struct {
type getRequest struct {
kernelRequester

API string `json:"api"`
Property string `json:"property"`
API string `json:"api"`
Property string `json:"property"`
ObjRef objectRef `json:"objref"`
}

Expand All @@ -165,7 +169,7 @@ type setRequest struct {
API string `json:"api"`
Property string `json:"property"`
Value interface{} `json:"value"`
ObjRef objectRef `json:"objref"`
ObjRef objectRef `json:"objref"`
}

type staticSetRequest struct {
Expand Down Expand Up @@ -196,7 +200,7 @@ type invokeRequest struct {
API string `json:"api"`
Method string `json:"method"`
Arguments []interface{} `json:"args"`
ObjRef objectRef `json:"objref"`
ObjRef objectRef `json:"objref"`
}

type invokeResponse struct {
Expand All @@ -211,7 +215,7 @@ type beginRequest struct {
API string `json:"api"`
Method *string `json:"method"`
Arguments []interface{} `json:"args"`
ObjRef objectRef `json:"objref"`
ObjRef objectRef `json:"objref"`
}

type beginResponse struct {
Expand Down
106 changes: 89 additions & 17 deletions packages/@jsii/go-runtime/jsii-runtime-go/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,14 @@ func castValToRef(data interface{}) (objectRef, bool) {
return ref, ok
}

func castValToEnumRef(data interface{}) (enum enumRef, ok bool) {
dataVal := reflect.ValueOf(data)
func castValToEnumRef(data reflect.Value) (enum enumRef, ok bool) {
ok = false

if dataVal.Kind() == reflect.Map {
for _, k := range dataVal.MapKeys() {
if data.Kind() == reflect.Map {
for _, k := range data.MapKeys() {
// Finding values type requires extracting from reflect.Value
// otherwise .Kind() returns `interface{}`
v := reflect.ValueOf(dataVal.MapIndex(k).Interface())
v := reflect.ValueOf(data.MapIndex(k).Interface())

if k.Kind() == reflect.String && k.String() == "$jsii.enum" && v.Kind() == reflect.String {
enum.MemberFQN = v.String()
Expand All @@ -283,31 +282,99 @@ func castValToEnumRef(data interface{}) (enum enumRef, ok bool) {
return
}

// castValToMap attempts converting the provided jsii wire value to a
// go map. This recognizes the "$jsii.map" object and does the necessary
// recursive value conversion.
func (c *client) castValToMap(data reflect.Value, mapType reflect.Type) (m reflect.Value, ok bool) {
ok = false

if data.Kind() != reflect.Map || data.Type().Key().Kind() != reflect.String {
return
}

if mapType.Kind() == reflect.Map && mapType.Key().Kind() != reflect.String {
return
}
anyType := reflect.TypeOf((*interface{})(nil)).Elem()
if mapType == anyType {
mapType = reflect.TypeOf((map[string]interface{})(nil))
}

dataIter := data.MapRange()
for dataIter.Next() {
key := dataIter.Key().String()
if key != "$jsii.map" {
continue
}

// Finding value type requries extracting from reflect.Value
// otherwise .Kind() returns `interface{}`
val := reflect.ValueOf(dataIter.Value().Interface())
if val.Kind() != reflect.Map {
return
}

ok = true

m = reflect.MakeMap(mapType)

iter := val.MapRange()
for iter.Next() {
val := iter.Value().Interface()
// Note: reflect.New(t) returns a pointer to a newly allocated t
convertedVal := reflect.New(mapType.Elem())
c.castAndSetToPtr(convertedVal.Interface(), val)

m.SetMapIndex(iter.Key(), convertedVal.Elem())
}
return
}
return
}

// Accepts pointers to structs that implement interfaces and searches for an
// existing object reference in the client. If it exists, it casts it to an
// objref for the runtime. Recursively casts types that may contain nested
// object references.
func castPtrToRef(data interface{}) interface{} {
if data == nil {
return data
}

client := getClient()
dataVal := reflect.ValueOf(data)

if dataVal.Kind() == reflect.Ptr {
switch dataVal.Kind() {
case reflect.Map:
result := wireMap{MapData: make(map[string]interface{})}

iter := dataVal.MapRange()
for iter.Next() {
key := iter.Key().String()
val := iter.Value().Interface()
result.MapData[key] = castPtrToRef(val)
}

return result

case reflect.Ptr:
valref, valHasRef := client.findObjectRef(data)
if valHasRef {
return objectRef{InstanceID: valref}
}
} else if dataVal.Kind() == reflect.String {
if enumRef, isEnumRef := client.types.tryRenderEnumRef(dataVal); isEnumRef {
return enumRef
}
} else if dataVal.Kind() == reflect.Slice {

case reflect.Slice:
refs := make([]interface{}, dataVal.Len())
for i := 0; i < dataVal.Len(); i++ {
refs[i] = dataVal.Index(i).Interface()
}
return refs
}

case reflect.String:
if enumRef, isEnumRef := client.types.tryRenderEnumRef(dataVal); isEnumRef {
return enumRef
}
}
return data
}

Expand All @@ -332,8 +399,8 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
ptrVal := reflect.ValueOf(ptr).Elem()
dataVal := reflect.ValueOf(data)

ref, isRef := castValToRef(data)
if isRef {
// object refs
if ref, isRef := castValToRef(data); isRef {
// If return data is JSII object references, add to objects table.
if concreteType, err := c.types.concreteTypeFor(ptrVal.Type()); err == nil {
ptrVal.Set(reflect.New(concreteType))
Expand All @@ -344,7 +411,8 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
return
}

if enumref, isEnum := castValToEnumRef(data); isEnum {
// enums
if enumref, isEnum := castValToEnumRef(dataVal); isEnum {
member, err := c.types.enumMemberForEnumRef(enumref)
if err != nil {
panic(err)
Expand All @@ -354,6 +422,12 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
return
}

// maps
if m, isMap := c.castValToMap(dataVal, ptrVal.Type()); isMap {
ptrVal.Set(m)
return
}

// arrays
if ptrVal.Kind() == reflect.Slice && dataVal.Kind() == reflect.Slice {
// If return type is a slice, recursively cast elements
Expand All @@ -368,8 +442,6 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
return
}

// TODO: maps

if data != nil {
val := reflect.ValueOf(data)
ptrVal.Set(val)
Expand Down

0 comments on commit 0359928

Please sign in to comment.