fix(go-runtime): enums are not encoded/decoded correctly (#2585)

Enum values are represented in the jsii kernel as `{ "$jsii.enum": "fqn/member" }`, so we need to marshal/unmarshall them based on this encoding. Change the type registry to capture a mapping between FQN to enum member consts and type to enum FQNs. Fixes #2534 Co-authored-by: Romain Marcadier <[email protected]> --- 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
aws · Feb 17, 2021 · 4731aeb · 4731aeb
1 parent 5f28442
commit 4731aeb
Show file tree

Hide file tree

Showing 5 changed files with 170 additions and 40 deletions.
diff --git a/gh-pages/content/specification/2-type-system.md b/gh-pages/content/specification/2-type-system.md
@@ -230,10 +230,10 @@ value:
 
 In **JavaScript**, `enum` entries are represented by their value equivalent. In order to support statically typed
 representations in other languages, these are serialized using a dedicated wrapper object, using a single key
-(`$jsii.enum`) with the fully qualified name of the `enum` entry:
+(`$jsii.enum`) with the fully qualified name of the `enum` entry (formatted as `<enum type fqn>/<entry name>`):
 
 ```json
-{ "$jsii.enum": "@scope/module.EnumType.ENTRY_NAME" }
+{ "$jsii.enum": "@scope/module.EnumType/ENTRY_NAME" }
 ```
 
 ### Identity Serialization

diff --git a/packages/@jsii/go-runtime/jsii-calc-test/main_test.go b/packages/@jsii/go-runtime/jsii-calc-test/main_test.go
@@ -157,6 +157,46 @@ func TestAllTypes(t *testing.T) {
 	})
 }
 
+func TestEnumUnmarshal(t *testing.T) {
+	actual := calc.EnumDispenser_RandomStringLikeEnum()
+	if actual != calc.StringEnumB {
+		t.Errorf("Expected StringEnum.B. Actual: %s", actual)
+	}
+}
+
+func TestEnumRoundtrip(t *testing.T) {
+	allTypes := calc.NewAllTypes()
+	actual := allTypes.EnumMethod(calc.StringEnumA)
+	if actual != calc.StringEnumA {
+		t.Errorf("Expected StringEnum.A. Actual: %s", actual)
+	}
+
+	actual = allTypes.EnumMethod(calc.StringEnumC)
+	if actual != calc.StringEnumC {
+		t.Errorf("Expected StringEnum.C. Actual: %s", actual)
+	}
+}
+
+func TestOptionalEnums(t *testing.T) {
+	allTypes := calc.NewAllTypes()
+	actual := allTypes.GetOptionalEnumValue()
+	if actual != "" {
+		t.Error("Expected value to be nil")
+	}
+
+	allTypes.SetOptionalEnumValue(calc.StringEnumB)
+	actual = allTypes.GetOptionalEnumValue()
+	if actual != calc.StringEnumB {
+		t.Errorf("Expected StringEnum.B. Actual: %s", actual)
+	}
+
+	allTypes.SetOptionalEnumValue("")
+	actual = allTypes.GetOptionalEnumValue()
+	if actual != "" {
+		t.Error("Expected value to be nil")
+	}
+}
+
 func TestReturnsSpecialParam(t *testing.T) {
 	retSpecialParam := returnsParam.NewReturnsSpecialParameter()
 	val := retSpecialParam.ReturnsSpecialParam()

diff --git a/packages/@jsii/go-runtime/jsii-runtime-go/api.go b/packages/@jsii/go-runtime/jsii-runtime-go/api.go
@@ -85,10 +85,14 @@ func (r kernelResponder) isResponse() {
 	return
 }
 
-type objref struct {
+type objectRef struct {
 	InstanceID string `json:"$jsii.byref"`
 }
 
+type enumRef struct {
+	MemberFQN string `json:"$jsii.enum"`
+}
+
 type loadRequest struct {
 	kernelRequester
 
@@ -126,7 +130,7 @@ type delRequest struct {
 	kernelRequester
 
 	API    string `json:"api"`
-	ObjRef objref `json:"objref"`
+	ObjRef objectRef `json:"objref"`
 }
 
 type delResponse struct {
@@ -138,7 +142,7 @@ type getRequest struct {
 
 	API      string `json:"api"`
 	Property string `json:"property"`
-	ObjRef   objref `json:"objref"`
+	ObjRef   objectRef `json:"objref"`
 }
 
 type staticGetRequest struct {
@@ -161,7 +165,7 @@ type setRequest struct {
 	API      string      `json:"api"`
 	Property string      `json:"property"`
 	Value    interface{} `json:"value"`
-	ObjRef   objref      `json:"objref"`
+	ObjRef   objectRef      `json:"objref"`
 }
 
 type staticSetRequest struct {
@@ -192,7 +196,7 @@ type invokeRequest struct {
 	API       string        `json:"api"`
 	Method    string        `json:"method"`
 	Arguments []interface{} `json:"args"`
-	ObjRef    objref        `json:"objref"`
+	ObjRef    objectRef        `json:"objref"`
 }
 
 type invokeResponse struct {
@@ -207,7 +211,7 @@ type beginRequest struct {
 	API       string        `json:"api"`
 	Method    *string       `json:"method"`
 	Arguments []interface{} `json:"args"`
-	ObjRef    objref        `json:"objref"`
+	ObjRef    objectRef        `json:"objref"`
 }
 
 type beginResponse struct {

diff --git a/packages/@jsii/go-runtime/jsii-runtime-go/runtime.go b/packages/@jsii/go-runtime/jsii-runtime-go/runtime.go
@@ -117,7 +117,7 @@ func Invoke(obj interface{}, method string, args []interface{}, hasReturn bool,
 		API:       "invoke",
 		Method:    method,
 		Arguments: castPtrsToRef(args),
-		ObjRef: objref{
+		ObjRef: objectRef{
 			InstanceID: refid,
 		},
 	})
@@ -167,7 +167,7 @@ func Get(obj interface{}, property string, ret interface{}) {
 	res, err := client.get(getRequest{
 		API:      "get",
 		Property: property,
-		ObjRef: objref{
+		ObjRef: objectRef{
 			InstanceID: refid,
 		},
 	})
@@ -213,7 +213,7 @@ func Set(obj interface{}, property string, value interface{}) {
 		API:      "set",
 		Property: property,
 		Value:    castPtrToRef(value),
-		ObjRef: objref{
+		ObjRef: objectRef{
 			InstanceID: refid,
 		},
 	})
@@ -240,8 +240,8 @@ func StaticSet(fqn FQN, property string, value interface{}) {
 	}
 }
 
-func castValToRef(data interface{}) (objref, bool) {
-	ref := objref{}
+func castValToRef(data interface{}) (objectRef, bool) {
+	ref := objectRef{}
 	ok := false
 	dataVal := reflect.ValueOf(data)
 
@@ -262,6 +262,27 @@ func castValToRef(data interface{}) (objref, bool) {
 	return ref, ok
 }
 
+func castValToEnumRef(data interface{}) (enum enumRef, ok bool) {
+	dataVal := reflect.ValueOf(data)
+	ok = false
+
+	if dataVal.Kind() == reflect.Map {
+		for _, k := range dataVal.MapKeys() {
+			// Finding values type requires extracting from reflect.Value
+			// otherwise .Kind() returns `interface{}`
+			v := reflect.ValueOf(dataVal.MapIndex(k).Interface())
+
+			if k.Kind() == reflect.String && k.String() == "$jsii.enum" && v.Kind() == reflect.String {
+				enum.MemberFQN = v.String()
+				ok = true
+				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
@@ -273,7 +294,11 @@ func castPtrToRef(data interface{}) interface{} {
 	if dataVal.Kind() == reflect.Ptr {
 		valref, valHasRef := client.findObjectRef(data)
 		if valHasRef {
-			return objref{InstanceID: valref}
+			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 {
 		refs := make([]interface{}, dataVal.Len())
@@ -308,7 +333,28 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
 	dataVal := reflect.ValueOf(data)
 
 	ref, isRef := castValToRef(data)
+	if 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))
+			c.objects[ptrVal.Interface()] = ref.InstanceID
+		} else {
+			panic(err)
+		}
+		return
+	}
+
+	if enumref, isEnum := castValToEnumRef(data); isEnum {
+		member, err := c.types.enumMemberForEnumRef(enumref)
+		if err != nil {
+			panic(err)
+		}
+
+		ptrVal.Set(reflect.ValueOf(member))
+		return
+	}
 
+	// arrays
 	if ptrVal.Kind() == reflect.Slice && dataVal.Kind() == reflect.Slice {
 		// If return type is a slice, recursively cast elements
 		for i := 0; i < dataVal.Len(); i++ {
@@ -318,15 +364,13 @@ func (c *client) castAndSetToPtr(ptr interface{}, data interface{}) {
 			c.castAndSetToPtr(inner.Interface(), dataVal.Index(i).Interface())
 			ptrVal.Set(reflect.Append(ptrVal, inner.Elem()))
 		}
-	} else if isRef {
-		// If return data is JSII object references, add to objects table.
-		concreteType, err := c.types.concreteTypeFor(ptrVal.Type())
-		if err != nil {
-			panic(err)
-		}
-		ptrVal.Set(reflect.New(concreteType))
-		c.objects[ptrVal.Interface()] = ref.InstanceID
-	} else {
+
+		return
+	}
+
+	// TODO: maps
+
+	if data != nil {
 		val := reflect.ValueOf(data)
 		ptrVal.Set(val)
 	}

diff --git a/packages/@jsii/go-runtime/jsii-runtime-go/type-registry.go b/packages/@jsii/go-runtime/jsii-runtime-go/type-registry.go
@@ -13,24 +13,30 @@ type typeRegistry struct {
 	// qualified type name. The kind of type being returned depends on what the
 	// FQN represents... This will be the second argument of provided to a
 	// register* function.
+	// enums are not included
 	fqnToType map[FQN]reflect.Type
 
 	// ifaceToStruct provides the go struct that should be used to build a proxy
 	// for interface types, so the correct dynamic type can be returned from a
 	// conversion.
 	ifaceToStruct map[reflect.Type]reflect.Type
 
-	// enumMembers has enum constants for each registered enum type, supporting
-	// correct conversion of enum values received from JavaScript.
-	enumMembers map[reflect.Type]map[string]interface{}
+	// map enum member FQNs (e.g. "jsii-calc.StringEnum/A") to the corresponding
+	// go const for this member.
+	fqnToEnumMember map[string]interface{}
+
+	// maps Go enum type ("StringEnum") to the corresponding jsii enum FQN (e.g.
+	// "jsii-calc.StringEnum")
+	typeToEnumFQN map[reflect.Type]FQN
 }
 
 // newTypeRegistry creates a new type registry.
 func newTypeRegistry() *typeRegistry {
 	return &typeRegistry{
-		fqnToType:     make(map[FQN]reflect.Type),
-		ifaceToStruct: make(map[reflect.Type]reflect.Type),
-		enumMembers:   make(map[reflect.Type]map[string]interface{}),
+		fqnToType:       make(map[FQN]reflect.Type),
+		ifaceToStruct:   make(map[reflect.Type]reflect.Type),
+		fqnToEnumMember: make(map[string]interface{}),
+		typeToEnumFQN:   make(map[reflect.Type]FQN),
 	}
 }
 
@@ -66,21 +72,24 @@ func (t *typeRegistry) registerEnum(fqn FQN, enm reflect.Type, members map[strin
 	if enm.Kind() != reflect.String {
 		return fmt.Errorf("the provided enum is not a string derivative: %v", enm)
 	}
-
-	for k, v := range members {
-		vt := reflect.ValueOf(v).Type()
+	if existing, exists := t.fqnToType[fqn]; exists && existing != enm {
+		return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
+	}
+	for memberName, memberVal := range members {
+		vt := reflect.ValueOf(memberVal).Type()
 		if vt != enm {
-			return fmt.Errorf("the enum entry for key %s has incorrect type %v", k, vt)
+			return fmt.Errorf("the enum entry for key %s has incorrect type %v", memberName, vt)
 		}
+		// Not setting in t.fqnToEnumMember here so we don't cause any side-effects
+		// if the pre-condition fails at any point. This is done in a second loop.
 	}
 
-	if existing, exists := t.fqnToType[fqn]; exists && existing != enm {
-		return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
+	t.typeToEnumFQN[enm] = fqn
+	for memberName, memberVal := range members {
+		memberFQN := fmt.Sprintf("%s/%s", fqn, memberName)
+		t.fqnToEnumMember[memberFQN] = memberVal
 	}
 
-	t.fqnToType[fqn] = enm
-	t.enumMembers[enm] = members
-
 	return nil
 }
 
@@ -143,12 +152,45 @@ func (t *typeRegistry) concreteTypeFor(typ reflect.Type) (structType reflect.Typ
 		err = nil
 	} else if typ.Kind() == reflect.Interface {
 		var ok bool
-		structType, ok = t.ifaceToStruct[typ]
-		if !ok {
+		if structType, ok = t.ifaceToStruct[typ]; !ok {
 			err = fmt.Errorf("no concrete type was registered for interface: %v", typ)
 		}
 	} else {
 		err = fmt.Errorf("invalid argument: expected a struct or interface type, but got %v", typ)
 	}
 	return
 }
+
+// enumMemberForEnumRef returns the go enum member corresponding to a jsii fully
+// qualified enum member name (e.g: "jsii-calc.StringEnum/A"). If no enum member
+// was registered (via registerEnum) for the provided enumref, an error is
+// returned.
+func (t *typeRegistry) enumMemberForEnumRef(ref enumRef) (interface{}, error) {
+	if member, ok := t.fqnToEnumMember[ref.MemberFQN]; ok {
+		return member, nil
+	}
+	return nil, fmt.Errorf("no enum member registered for %s", ref.MemberFQN)
+}
+
+// tryRenderEnumRef returns an enumref if the provided value corresponds to a
+// registered enum type. The returned enumref is nil if the provided enum value
+// is a zero-value (i.e: "").
+func (t *typeRegistry) tryRenderEnumRef(value reflect.Value) (ref *enumRef, isEnumRef bool) {
+	if value.Kind() != reflect.String {
+		isEnumRef = false
+		return
+	}
+
+	if enumFQN, ok := t.typeToEnumFQN[value.Type()]; ok {
+		isEnumRef = true
+		if memberName := value.String(); memberName != "" {
+			ref = &enumRef{MemberFQN: fmt.Sprintf("%s/%s", enumFQN, memberName)}
+		} else {
+			ref = nil
+		}
+	} else {
+		isEnumRef = false
+	}
+
+	return
+}