Skip to content

Commit

Permalink
fix(go): enums inside structs are not properly serialized (#2636)
Browse files Browse the repository at this point in the history
Serializes struct values according to the jsii protocol, wrapping
the data in a `$jsii.struct` object recording both the struct's
fully qualified type name, and key-value pairs.

This includes recursively transforming field values, so the
behavior is now correct.
  • Loading branch information
Elad Ben-Israel committed Mar 2, 2021
1 parent ebbe1eb commit 19cbd25
Show file tree
Hide file tree
Showing 43 changed files with 970 additions and 28 deletions.
2 changes: 1 addition & 1 deletion packages/@jsii/go-runtime/.gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/jsii-calc/
/jsii-runtime-go/embedded/resources/
/jsii-runtime-go/internal/embedded/resources/

*.generated.*

Expand Down
9 changes: 7 additions & 2 deletions packages/@jsii/go-runtime/build-tools/gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ const EMBEDDED_RUNTIME_SOURCE_ROOT = resolve(

const RUNTIME_ROOT = resolve(__dirname, '..', 'jsii-runtime-go');

const EMBEDDED_RESOURCE_DIR = join(RUNTIME_ROOT, 'embedded', 'resources');
const EMBEDDED_RESOURCE_DIR = join(
RUNTIME_ROOT,
'internal',
'embedded',
'resources',
);

mkdirpSync(EMBEDDED_RESOURCE_DIR);
copySync(EMBEDDED_RUNTIME_SOURCE_ROOT, EMBEDDED_RESOURCE_DIR, {
Expand All @@ -25,7 +30,7 @@ copySync(EMBEDDED_RUNTIME_SOURCE_ROOT, EMBEDDED_RESOURCE_DIR, {
recursive: true,
});

const KERNEL_LIB_DIR = resolve(RUNTIME_ROOT, 'kernel');
const KERNEL_LIB_DIR = resolve(RUNTIME_ROOT, 'internal', 'kernel');
const code = new CodeMaker({ indentationLevel: 1, indentCharacter: '\t' });

const VERSION_FILE = 'version.generated.go';
Expand Down
32 changes: 32 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 @@ -195,6 +195,38 @@ func TestOptionalEnums(t *testing.T) {
}
}

func TestStructWithEnum(t *testing.T) {
obj := calc.NewTestStructWithEnum()
if !obj.IsStringEnumA(calc.StructWithEnum{Foo: calc.StringEnum_A}) {
t.Error("Failed")
}

if !obj.IsStringEnumB(calc.StructWithEnum{
Foo: calc.StringEnum_B,
Bar: calc.AllTypesEnum_THIS_IS_GREAT,
}) {
t.Error("Failed")
}

ret1 := obj.StructWithFoo()
if ret1.Foo != calc.StringEnum_A {
t.Error("Expecting Foo to be A")
}

if ret1.Bar != "" {
t.Error("Expecting Bar to be nil")
}

ret2 := obj.StructWithFooBar()
if ret2.Foo != calc.StringEnum_C {
t.Error("Expecting Foo to be C")
}

if ret2.Bar != calc.AllTypesEnum_MY_ENUM_VALUE {
t.Error("Expecting Foo to be MY_ENUM_VALUE")
}
}

func TestReturnsSpecialParam(t *testing.T) {
retSpecialParam := returnsParam.NewReturnsSpecialParameter()
val := retSpecialParam.ReturnsSpecialParam()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ type WireMap struct {
MapData map[string]interface{} `json:"$jsii.map"`
}

type WireStruct struct {
StructDescriptor `json:"$jsii.struct"`
}

type StructDescriptor struct {
FQN FQN `json:"fqn"`
Fields map[string]interface{} `json:"data"`
}

type Callback struct {
CallbackID *string `json:"cbid"`
Cookie *string `json:"cookie"`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package kernel

import (
"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/internal/api"
)

type BeginProps struct {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package kernel

import (
"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/internal/api"
)

type CallbacksResponse struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
"runtime"
"sync"

"github.com/aws/jsii-runtime-go/kernel/process"
"github.com/aws/jsii-runtime-go/typeregistry"
"github.com/aws/jsii-runtime-go/internal/kernel/process"
"github.com/aws/jsii-runtime-go/internal/typeregistry"
)

var (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package kernel
import (
"reflect"

"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/internal/api"
)

// CastAndSetToPtr accepts a pointer to any type and attempts to cast the value
Expand Down Expand Up @@ -35,7 +35,7 @@ func (c *client) castAndSetToPtr(ptr reflect.Value, data reflect.Value) {

if ref, isRef := castValToRef(data); isRef {
// If return data is a jsii struct passed by reference, de-reference it all.
if fields, isStruct := c.Types().StructFields(ptr.Type()); isStruct {
if fields, _, isStruct := c.Types().StructFields(ptr.Type()); isStruct {
for _, field := range fields {
got, err := c.Get(GetProps{
Property: field.Tag.Get("json"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kernel

import "github.com/aws/jsii-runtime-go/api"
import "github.com/aws/jsii-runtime-go/internal/api"

type CreateProps struct {
FQN api.FQN `json:"fqn"`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kernel

import "github.com/aws/jsii-runtime-go/api"
import "github.com/aws/jsii-runtime-go/internal/api"

type DelProps struct {
ObjRef api.ObjectRef `json:"objref"`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kernel

import "github.com/aws/jsii-runtime-go/api"
import "github.com/aws/jsii-runtime-go/internal/api"

type GetProps struct {
Property string `json:"property"`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kernel

import "github.com/aws/jsii-runtime-go/api"
import "github.com/aws/jsii-runtime-go/internal/api"

type InvokeProps struct {
Method string `json:"method"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package process

import (
"fmt"
"github.com/Masterminds/semver/v3"
"regexp"

"github.com/Masterminds/semver/v3"
)

type handshakeResponse struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"sync"

"github.com/Masterminds/semver/v3"
"github.com/aws/jsii-runtime-go/embedded"
"github.com/aws/jsii-runtime-go/internal/embedded"
)

const JSII_RUNTIME string = "JSII_RUNTIME"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package kernel

import "github.com/aws/jsii-runtime-go/api"
import "github.com/aws/jsii-runtime-go/internal/api"

type SetProps struct {
Property string `json:"property"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import (
"fmt"
"reflect"

"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/internal/api"
)


// RegisterClass maps the given FQN to the provided class interface, and proxy
// maker function. This returns an error if the class type is not a go interface.
func (t *TypeRegistry) RegisterClass(fqn api.FQN, class reflect.Type, maker func() interface{}) error {
Expand Down Expand Up @@ -84,6 +83,10 @@ func (t *TypeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type) error {
return fmt.Errorf("another type was already registered with %s: %v", fqn, existing)
}

if existing, exists := t.typeToFQN[strct]; exists && existing != fqn {
return fmt.Errorf("attempting to register type %s as %s, but it was already registered as: %s", strct.String(), fqn, existing)
}

fields := []reflect.StructField{}
numField := strct.NumField()
for i := 0; i < numField; i++ {
Expand All @@ -101,6 +104,7 @@ func (t *TypeRegistry) RegisterStruct(fqn api.FQN, strct reflect.Type) error {
}

t.fqnToType[fqn] = strct
t.typeToFQN[strct] = fqn
t.structFields[strct] = fields

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"reflect"

"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/internal/api"
)

// typeRegistry is used to record runtime type information about the loaded
Expand All @@ -18,6 +18,10 @@ type TypeRegistry struct {
// enums are not included
fqnToType map[api.FQN]reflect.Type

// typeToFQN is sued to obtain the jsii fully qualified type name for a
// given native go type. Currently only tracks jsii struct types.
typeToFQN map[reflect.Type]api.FQN

// map enum member FQNs (e.g. "jsii-calc.StringEnum/A") to the corresponding
// go const for this member.
fqnToEnumMember map[string]interface{}
Expand All @@ -37,18 +41,24 @@ type TypeRegistry struct {
func NewTypeRegistry() *TypeRegistry {
return &TypeRegistry{
fqnToType: make(map[api.FQN]reflect.Type),
typeToFQN: make(map[reflect.Type]api.FQN),
fqnToEnumMember: make(map[string]interface{}),
typeToEnumFQN: make(map[reflect.Type]api.FQN),
structFields: make(map[reflect.Type][]reflect.StructField),
proxyMakers: make(map[reflect.Type]func() interface{}),
}
}

// IsStruct returns true if the provided type is a registered jsii struct.
func (t *TypeRegistry) StructFields(typ reflect.Type) (fields []reflect.StructField, ok bool) {
// StructFields returns the list of fields associated with a jsii struct type,
// the jsii fully qualified type name, and a boolean telling whether the
// provided type was a registered jsii struct type.
func (t *TypeRegistry) StructFields(typ reflect.Type) (fields []reflect.StructField, fqn api.FQN, ok bool) {
if fqn, ok = t.typeToFQN[typ]; !ok {
return
}

var found []reflect.StructField
found, ok = t.structFields[typ]
if ok {
if found, ok = t.structFields[typ]; ok {
// Returning a copy, to ensure our storage does not get mutated.
fields = append(fields, found...)
}
Expand Down
29 changes: 25 additions & 4 deletions packages/@jsii/go-runtime/jsii-runtime-go/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"reflect"
"regexp"

"github.com/aws/jsii-runtime-go/api"
"github.com/aws/jsii-runtime-go/kernel"
"github.com/aws/jsii-runtime-go/internal/api"
"github.com/aws/jsii-runtime-go/internal/kernel"
)

// FQN represents a fully-qualified type name in the jsii type system.
Expand Down Expand Up @@ -315,12 +315,32 @@ func castPtrToRef(data interface{}) interface{} {

return result

case reflect.Ptr:
valref, valHasRef := client.FindObjectRef(reflect.ValueOf(data))
case reflect.Interface, reflect.Ptr:
valref, valHasRef := client.FindObjectRef(dataVal)
if valHasRef {
return api.ObjectRef{InstanceID: valref}
}

case reflect.Struct:
if fields, fqn, isStruct := client.Types().StructFields(dataVal.Type()); isStruct {
data := make(map[string]interface{})
for _, field := range fields {
fieldVal := dataVal.FieldByIndex(field.Index)
if (fieldVal.Kind() == reflect.Ptr || fieldVal.Kind() == reflect.Interface) && fieldVal.IsNil() {
continue
}
key := field.Tag.Get("json")
data[key] = castPtrToRef(fieldVal.Interface())
}

return api.WireStruct{
StructDescriptor: api.StructDescriptor{
FQN: fqn,
Fields: data,
},
}
}

case reflect.Slice:
refs := make([]interface{}, dataVal.Len())
for i := 0; i < dataVal.Len(); i++ {
Expand All @@ -333,6 +353,7 @@ func castPtrToRef(data interface{}) interface{} {
return enumRef
}
}

return data
}

Expand Down
53 changes: 53 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2872,3 +2872,56 @@ export class StaticHelloChild extends StaticHelloParent {
super();
}
}

// --------------------------------------------------
// enums within structs

export interface StructWithEnum {
/**
* An enum value
*/
readonly foo: StringEnum;

/**
* Optional enum value (of type integer)
* @default AllTypesEnum.YOUR_ENUM_VALUE
*/
readonly bar?: AllTypesEnum;
}

export class TestStructWithEnum {
/**
* Returns true if `foo` is `StringEnum.A`.
*/
public isStringEnumA(input: StructWithEnum) {
return input.foo === StringEnum.A && !input.bar;
}

/**
* Returns true if `foo` is `StringEnum.B` and `bar` is `AllTypesEnum.THIS_IS_GREAT`.
*/
public isStringEnumB(input: StructWithEnum) {
return (
input.foo === StringEnum.B && input.bar === AllTypesEnum.THIS_IS_GREAT
);
}

/**
* Returns `foo: StringEnum.C` and `bar: AllTypesEnum.MY_ENUM_VALUE`.
*/
public get structWithFooBar(): StructWithEnum {
return {
foo: StringEnum.C,
bar: AllTypesEnum.MY_ENUM_VALUE,
};
}

/**
* Returns `foo: StringEnum.A`.
*/
public get structWithFoo(): StructWithEnum {
return {
foo: StringEnum.A,
};
}
}
Loading

0 comments on commit 19cbd25

Please sign in to comment.