From c731983f2dbf2f0064be9e1f5b01263cc4295208 Mon Sep 17 00:00:00 2001 From: Romain Marcadier-Muller Date: Mon, 11 Nov 2019 23:51:47 +0100 Subject: [PATCH] fix(kernel): correctly de-serialize mappings as JSON (#968) When mapping data (e.g. a Python `dict`) was passed through a JSON value, it would not be deserialized correctly and the `$jsii.map` markers would remain in the JS-visible map. --- .../ComplianceTests.cs | 14 ++++---- .../amazon/jsii/testing/ComplianceTest.java | 4 +-- packages/jsii-kernel/lib/serialization.ts | 32 +++++++++++++++++-- packages/jsii-kernel/test/kernel.test.ts | 18 +++++++++++ .../tests/test_compliance.py | 4 +-- 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs index b6f6849d76..f303b84aa2 100644 --- a/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs +++ b/packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs @@ -60,8 +60,8 @@ public void PrimitiveTypes() Assert.Equal(UnixEpoch.AddMilliseconds(123), types.DateProperty); // json - types.JsonProperty = JObject.Parse(@"{ ""Foo"": 123 }"); - Assert.Equal(123d, types.JsonProperty["Foo"].Value()); + types.JsonProperty = JObject.Parse(@"{ ""Foo"": { ""Bar"": 123 } }"); + Assert.Equal(123d, types.JsonProperty["Foo"]["Bar"].Value()); } [Fact(DisplayName = Prefix + nameof(Dates))] @@ -137,7 +137,7 @@ public void DynamicTypes() new Dictionary { { "World", 123 } - } + } } } }; @@ -1011,12 +1011,12 @@ public void CorrectlyDeserializesStructUnions() var a1 = new StructA { RequiredString = "Present!", OptionalNumber = 1337 }; var b0 = new StructB { RequiredString = "Present!", OptionalBoolean = true }; var b1 = new StructB { RequiredString = "Present!", OptionalStructA = a1 }; - + Assert.True(StructUnionConsumer.IsStructA(a0)); Assert.True(StructUnionConsumer.IsStructA(a1)); Assert.False(StructUnionConsumer.IsStructA(b0)); Assert.False(StructUnionConsumer.IsStructA(b1)); - + Assert.False(StructUnionConsumer.IsStructB(a0)); Assert.False(StructUnionConsumer.IsStructB(a1)); Assert.True(StructUnionConsumer.IsStructB(b0)); @@ -1032,7 +1032,7 @@ public void VariadicCallbacksAreHandledCorrectly() Assert.Equal(new double[]{2d, 3d}, invoker.AsArray(1, 2)); Assert.Equal(new double[]{2d, 3d, 4d}, invoker.AsArray(1, 2, 3)); } - + private sealed class OverrideVariadicMethod : VariadicMethod { public override double[] AsArray(double first, params double[] others) @@ -1047,7 +1047,7 @@ public void OptionalCallbackArgumentsAreHandledCorrectly() var noOption = new InterfaceWithOptionalMethodArguments(); new OptionalArgumentInvoker(noOption).InvokeWithoutOptional(); Assert.True(noOption.Invoked); - + var option = new InterfaceWithOptionalMethodArguments(1337); new OptionalArgumentInvoker(option).InvokeWithOptional(); Assert.True(option.Invoked); diff --git a/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java b/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java index f53c76e6b8..a904d729d5 100644 --- a/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java +++ b/packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java @@ -60,8 +60,8 @@ public void primitiveTypes() throws IOException { assertEquals(Instant.ofEpochMilli(123), types.getDateProperty()); // json - types.setJsonProperty((ObjectNode) new ObjectMapper().readTree("{ \"Foo\": 123 }")); - assertEquals(123, types.getJsonProperty().get("Foo").numberValue()); + types.setJsonProperty((ObjectNode) new ObjectMapper().readTree("{ \"Foo\": { \"Bar\": 123 } }")); + assertEquals(123, types.getJsonProperty().get("Foo").get("Bar").numberValue()); } @Test diff --git a/packages/jsii-kernel/lib/serialization.ts b/packages/jsii-kernel/lib/serialization.ts index 5e5174b6c1..3c87b1bea5 100644 --- a/packages/jsii-kernel/lib/serialization.ts +++ b/packages/jsii-kernel/lib/serialization.ts @@ -168,10 +168,38 @@ export const SERIALIZERS: {[k: string]: Serializer} = { // Just whatever. Dates will automatically serialize themselves to strings. return value; }, - deserialize(value, optionalValue) { + deserialize(value, optionalValue, host) { // /!\ Top-level "null" will turn to undefined, but any null nested in the value is valid JSON, so it'll stay! if (nullAndOk(value, optionalValue)) { return undefined; } - return value; + + // A mapping object can arrive though here. This would be the case if anything that is valid into a Map + // is passed into a JSON transfer point. Indeed, those are also valid JSON! For example, Python "dicts" will be + // serialized (by the Python runtime) as a $jsii.map (the mapping object). We need to de-serialize that as a + // Map in order to obtain the correct output behavior here! + if (isWireMap(value)) { + return SERIALIZERS[SerializationClass.Map].deserialize( + value, + { + optional: false, + type: { collection: { kind: spec.CollectionKind.Map, elementtype: { primitive: spec.PrimitiveType.Json } } }, + }, + host); + } + + if (typeof value !== 'object') { + return value; + } + + if (Array.isArray(value)) { + return value.map(mapJsonValue); + } + + return mapValues(value, mapJsonValue); + + function mapJsonValue(toMap: any) { + if (toMap == null) { return toMap; } + return host.recurse(toMap, { type: { primitive: spec.PrimitiveType.Json } }); + } }, }, diff --git a/packages/jsii-kernel/test/kernel.test.ts b/packages/jsii-kernel/test/kernel.test.ts index c1ab2f4025..44579a73f8 100644 --- a/packages/jsii-kernel/test/kernel.test.ts +++ b/packages/jsii-kernel/test/kernel.test.ts @@ -148,6 +148,24 @@ defineTest('in/out enum values', (sandbox) => { expect(sandbox.get({ objref: alltypes, property: 'enumProperty' }).value).toEqual({ '$jsii.enum': 'jsii-calc.AllTypesEnum/THIS_IS_GREAT' }); }); +describe('in/out json values', () => { + defineTest('with a plain object', (sandbox) => { + const allTypes = sandbox.create({ fqn: 'jsii-calc.AllTypes' }); + sandbox.set({ objref: allTypes, property: 'jsonProperty', value: { foo: 'bar', baz: 1337 } }); + expect(sandbox.get({ objref: allTypes, property: 'jsonProperty' }).value).toEqual({ foo: 'bar', baz: 1337 }); + }); + defineTest('with a simple mapping', (sandbox) => { + const allTypes = sandbox.create({ fqn: 'jsii-calc.AllTypes' }); + sandbox.set({ objref: allTypes, property: 'jsonProperty', value: { [api.TOKEN_MAP]: { foo: 'bar', baz: 1337 } } }); + expect(sandbox.get({ objref: allTypes, property: 'jsonProperty' }).value).toEqual({ foo: 'bar', baz: 1337 }); + }); + defineTest('with a nested mapping', (sandbox) => { + const allTypes = sandbox.create({ fqn: 'jsii-calc.AllTypes' }); + sandbox.set({ objref: allTypes, property: 'jsonProperty', value: { [api.TOKEN_MAP]: { foo: 'bar', baz: { [api.TOKEN_MAP]: { bazinga: [null, 'Pickle Rick'] } } } } }); + expect(sandbox.get({ objref: allTypes, property: 'jsonProperty' }).value).toEqual({ foo: 'bar', baz: { bazinga: [null, 'Pickle Rick'] } }); + }); +}); + defineTest('enum values from @scoped packages awslabs/jsii#138', (sandbox) => { const objref = sandbox.create({ fqn: 'jsii-calc.ReferenceEnumFromScopedPackage' }); diff --git a/packages/jsii-python-runtime/tests/test_compliance.py b/packages/jsii-python-runtime/tests/test_compliance.py index 19fe72d9b6..7895a6cfdd 100644 --- a/packages/jsii-python-runtime/tests/test_compliance.py +++ b/packages/jsii-python-runtime/tests/test_compliance.py @@ -193,8 +193,8 @@ def test_primitiveTypes(): assert types.date_property == datetime.fromtimestamp(123 / 1000.0, tz=timezone.utc) # json - types.json_property = {"Foo": 123} - assert types.json_property.get("Foo") == 123 + types.json_property = { "Foo": { "bar": 123 } } + assert types.json_property.get("Foo") == { "bar": 123 } def test_dates():