Skip to content

Commit

Permalink
fix(go): unable to set an array of interfaces/enums (#2754)
Browse files Browse the repository at this point in the history
Contents of array objects were not correctly de-serialized upon receiving
them from the node process: the re-hydratation was not recursive, and
also hit a bug in array re-hydratation when the receiving pointer does
not point to a slice type (i.e: it is an `interface{}`).

The runtime is now correctly re-hydrating recursively through arrays, and
they can be correctly re-hydrated into `interface{}` values.

Fixes #2686



---

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 Mar 29, 2021
1 parent 903b9f9 commit 0cd514e
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
6 changes: 3 additions & 3 deletions gh-pages/content/specification/6-compliance-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
This section details the current state of each language binding with respect to our standard compliance suite.


| number | test | java (100.00%) | golang (76.07%) | Dotnet | Python |
| number | test | java (100.00%) | golang (77.78%) | Dotnet | Python |
| ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------- | -------------------------------------------- | ------ | ------ |
| 1 | asyncOverrides_overrideCallsSuper | 🟢 | [🔴](https://github.com/aws/jsii/issues/2670) |||
| 2 | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements") | 🟢 | 🟢 |||
Expand All @@ -32,8 +32,8 @@ This section details the current state of each language binding with respect to
| 23 | [reservedKeywordsAreSlugifiedInClassProperties]("TS code that uses reserved words as class property names get slugified so it is usable in the target language") | 🟢 ||||
| 24 | [objectIdDoesNotGetReallocatedWhenTheConstructorPassesThisOut]("Ensure the JSII kernel can pass 'this' out to JSII remotes from within the constructor") | 🟢 | 🟢 |||
| 25 | [interfaceBuilder]("Seems to be a duplicate of 'propertyOverrides_interfaces'?") | 🟢 | 🟢 |||
| 26 | unionTypes | 🟢 | [🔴](https://github.com/aws/jsii/issues/2686) |||
| 27 | arrays | 🟢 | [🔴](https://github.com/aws/jsii/issues/2686) |||
| 26 | unionTypes | 🟢 | 🟢 |||
| 27 | arrays | 🟢 | 🟢 |||
| 28 | staticMapInClassCannotBeModified | 🟢 ||||
| 29 | consts | 🟢 | 🟢 |||
| 30 | pureInterfacesCanBeUsedTransparently_WhenTransitivelyImplementing | 🟢 | 🟢 |||
Expand Down
10 changes: 4 additions & 6 deletions packages/@jsii/go-runtime-test/project/compliance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1402,25 +1402,23 @@ func (suite *ComplianceSuite) TestUnionTypes() {
require.Equal(float64(99), *number.Value())

// array
suite.FailTest("Unable to set an array of interfaces", "https://github.com/aws/jsii/issues/2686")
a := []interface{}{123, calclib.NewNumber(jsii.Number(33))}
types.SetUnionArrayProperty(&a)

unionArrayProp := *types.UnionArrayProperty()
number, ok = unionArrayProp[1].(calclib.Number)
require.True(ok)
require.Equal(33, *number.Value())
require.Equal(float64(33), *number.Value())
}

func (suite *ComplianceSuite) TestArrays() {
require := suite.Require()
sum := calc.NewSum()

suite.FailTest("Unable to set an array of interfaces", "https://github.com/aws/jsii/issues/2686")
sum.SetParts(&[]calclib.NumericValue{calclib.NewNumber(jsii.Number(5)), calclib.NewNumber(jsii.Number(10)), calc.NewMultiply(calclib.NewNumber(jsii.Number(2)), calclib.NewNumber(jsii.Number(3)))})
require.Equal(10+5+(2*3), sum.Value())
require.Equal(5, *(*sum.Parts())[0].Value())
require.Equal(6, *(*sum.Parts())[2].Value())
require.Equal(float64(10+5+(2*3)), *sum.Value())
require.Equal(float64(5), *(*sum.Parts())[0].Value())
require.Equal(float64(6), *(*sum.Parts())[2].Value())
require.Equal("(((0 + 5) + 10) + (2 * 3))", *sum.ToString())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
"github.com/aws/jsii-runtime-go/internal/api"
)

var (
anyType = reflect.TypeOf((*interface{})(nil)).Elem()
)

// CastAndSetToPtr accepts a pointer to any type and attempts to cast the value
// argument to be the same type. Then it sets the value of the pointer element
// to be the newly cast data. This is used to cast payloads from JSII to
Expand Down Expand Up @@ -98,15 +102,21 @@ func (c *Client) castAndSetToPtr(ptr reflect.Value, data reflect.Value) {
}

// arrays
if ptr.Kind() == reflect.Slice && data.Kind() == reflect.Slice {
if data.Kind() == reflect.Slice {
len := data.Len()
ptr.Set(reflect.MakeSlice(ptr.Type(), len, len))
var slice reflect.Value
if ptr.Kind() == reflect.Slice {
slice = reflect.MakeSlice(ptr.Type(), len, len)
} else {
slice = reflect.MakeSlice(reflect.SliceOf(anyType), len, len)
}

// If return type is a slice, recursively cast elements
for i := 0; i < len; i++ {
c.castAndSetToPtr(ptr.Index(i), data.Index(i))
c.castAndSetToPtr(slice.Index(i), data.Index(i))
}

ptr.Set(slice)
return
}

Expand Down Expand Up @@ -186,7 +196,7 @@ func (c *Client) CastPtrToRef(dataVal reflect.Value) interface{} {
case reflect.Slice:
refs := make([]interface{}, dataVal.Len())
for i := 0; i < dataVal.Len(); i++ {
refs[i] = dataVal.Index(i).Interface()
refs[i] = c.CastPtrToRef(dataVal.Index(i))
}
return refs

Expand Down Expand Up @@ -307,7 +317,6 @@ func (c *Client) castValToMap(data reflect.Value, mapType reflect.Type) (m refle
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))
}
Expand Down

0 comments on commit 0cd514e

Please sign in to comment.