Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kernel): cannot pass decorated structs to kernel as "any" #997

Merged
merged 5 commits into from
Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2331,3 +2331,16 @@ export class ObjectWithPropertyProvider {

private constructor() { }
}

/**
* Make sure structs are un-decorated on the way in.
*
* @see https://github.com/aws/aws-cdk/issues/5066
*/
export class JsonFormatter {
public static stringify(value: any): string {
return JSON.stringify(value, null, 2);
}

private constructor() { }
}
43 changes: 42 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -6826,6 +6826,47 @@
}
]
},
"jsii-calc.JsonFormatter": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/aws-cdk/issues/5066",
"stability": "experimental",
"summary": "Make sure structs are un-decorated on the way in."
},
"fqn": "jsii-calc.JsonFormatter",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2340
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2341
},
"name": "stringify",
"parameters": [
{
"name": "value",
"type": {
"primitive": "any"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
},
"static": true
}
],
"name": "JsonFormatter"
},
"jsii-calc.LoadBalancedFargateServiceProps": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -11413,5 +11454,5 @@
}
},
"version": "0.20.6",
"fingerprint": "veHd1Q/376CoMR3O/DjjAiH9aVD/jcwnPEg88barg9I="
"fingerprint": "xXC7/htdFP0Ns+aNf856G/K6PBPRJjPlsg5KtO3bEXY="
}
Original file line number Diff line number Diff line change
Expand Up @@ -1297,5 +1297,18 @@ public void CanUseInterfaceSetters()
obj.Property = "New Value";
Assert.True(obj.WasSet());
}

[Fact(DisplayName = Prefix + nameof(StructsAreUndecoratedOntheWayToKernel))]
public void StructsAreUndecoratedOntheWayToKernel()
{
var json = JsonFormatter.Stringify(new StructB {RequiredString = "Bazinga!", OptionalBoolean = false});
var actual = JObject.Parse(json);

var expected = new JObject();
expected.Add("RequiredString", "Bazinga!");
expected.Add("OptionalBoolean", false);

Assert.Equal(expected, actual);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ public sealed class JsiiByValueAttribute : Attribute
{
public JsiiByValueAttribute(string fqn)
{
Fqn = fqn ?? throw new ArgumentNullException(nameof(fqn));
FullyQualifiedName = fqn ?? throw new ArgumentNullException(nameof(fqn));
}

public string Fqn { get; }
public string FullyQualifiedName { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ protected override bool TryConvertClass(Type type, IReferenceMap referenceMap, o
}

var structInfo = new JObject();
structInfo.Add(new JProperty("fqn", byValueAttribute.Fqn));
structInfo.Add(new JProperty("fqn", byValueAttribute.FullyQualifiedName));
structInfo.Add(new JProperty("data", data));

var resultObject = new JObject();
Expand Down Expand Up @@ -353,6 +353,12 @@ TypeReference InferType(IReferenceMap referenceMap, Type type)
return new TypeReference(enumAttribute.FullyQualifiedName);
}

JsiiByValueAttribute structAttribute = type.GetCustomAttribute<JsiiByValueAttribute>();
if (structAttribute != null)
{
return new TypeReference(structAttribute.FullyQualifiedName);
}

if (typeof(string).IsAssignableFrom(type))
{
return new TypeReference(primitive: PrimitiveType.String);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package software.amazon.jsii.testing;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.Test;
Expand Down Expand Up @@ -1580,4 +1581,17 @@ public void canUseInterfaceSetters() {
obj.setProperty("New Value");
assertTrue(obj.wasSet());
}

@Test
public void structsAreUndecoratedOntheWayToKernel() throws IOException {
final ObjectMapper om = new ObjectMapper();
final String json = JsonFormatter.stringify(StructB.builder().requiredString("Bazinga!").optionalBoolean(false).build());
final JsonNode actual = om.readTree(json);

final ObjectNode expected = om.createObjectNode();
expected.put("requiredString", "Bazinga!");
expected.put("optionalBoolean", Boolean.FALSE);

assertEquals(expected, actual);
}
}
11 changes: 10 additions & 1 deletion packages/jsii-kernel/lib/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
*/

import * as spec from 'jsii-spec';
import { isObjRef, isWireDate, isWireEnum, isWireMap, ObjRef, TOKEN_DATE, TOKEN_ENUM, TOKEN_MAP, WireDate, WireEnum } from './api';
import { isObjRef, isWireDate, isWireEnum, isWireMap, ObjRef, TOKEN_DATE, TOKEN_ENUM, TOKEN_MAP, WireDate, WireEnum, isWireStruct, TOKEN_STRUCT } from './api';
import { jsiiTypeFqn, objectReference, ObjectTable } from './objects';
import { api } from '.';

Expand Down Expand Up @@ -520,6 +520,15 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
host.debug('ANY is a Ref');
return host.objects.findObject(value).instance;
}

// if the value has a struct token, it was serialized by a typed jsii
// struct, but since the deserialization target is ANY, all we can do is
// strip the data from $jsii.struct and continue to deserialize as ANY.
if (isWireStruct(value)) {
const { fqn, data } = value[TOKEN_STRUCT];
host.debug(`ANY is a struct of type ${fqn}`);
return SERIALIZERS[SerializationClass.Any].deserialize(data, _type, host);
eladb marked this conversation as resolved.
Show resolved Hide resolved
}

// At this point again, deserialize by-value.
host.debug('ANY is a Map');
Expand Down
7 changes: 7 additions & 0 deletions packages/jsii-kernel/test/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,13 @@ defineTest('correctly deserializes struct unions', (sandbox) => {
}
});

defineTest('structs are undecorated on the way to kernel', sandbox => {
const input = { '$jsii.struct': { 'fqn': 'jsii-calc.StructB', 'data': { 'requiredString': 'Bazinga!', 'optionalBoolean': false } } };
const ret = sandbox.sinvoke({ fqn: 'jsii-calc.JsonFormatter', method: 'stringify', args: [input] });
const json = JSON.parse(ret.result);
expect(json).toStrictEqual({ 'requiredString': 'Bazinga!', 'optionalBoolean': false });
});

// =================================================================================================

const testNames: { [name: string]: boolean } = { };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6826,6 +6826,47 @@
}
]
},
"jsii-calc.JsonFormatter": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/aws-cdk/issues/5066",
"stability": "experimental",
"summary": "Make sure structs are un-decorated on the way in."
},
"fqn": "jsii-calc.JsonFormatter",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2340
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2341
},
"name": "stringify",
"parameters": [
{
"name": "value",
"type": {
"primitive": "any"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
},
"static": true
}
],
"name": "JsonFormatter"
},
"jsii-calc.LoadBalancedFargateServiceProps": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -11413,5 +11454,5 @@
}
},
"version": "0.20.6",
"fingerprint": "veHd1Q/376CoMR3O/DjjAiH9aVD/jcwnPEg88barg9I="
"fingerprint": "xXC7/htdFP0Ns+aNf856G/K6PBPRJjPlsg5KtO3bEXY="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Make sure structs are un-decorated on the way in.</summary>
/// <remarks>
/// stability: Experimental
/// see:
/// https://github.com/aws/aws-cdk/issues/5066
/// </remarks>
[JsiiClass(nativeType: typeof(Amazon.JSII.Tests.CalculatorNamespace.JsonFormatter), fullyQualifiedName: "jsii-calc.JsonFormatter")]
public class JsonFormatter : DeputyBase
{
protected JsonFormatter(ByRefValue reference): base(reference)
{
}

protected JsonFormatter(DeputyProps props): base(props)
{
}

/// <remarks>
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "stringify", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"value\",\"type\":{\"primitive\":\"any\"}}]")]
public static string Stringify(object @value)
{
return InvokeStaticMethod<string>(typeof(Amazon.JSII.Tests.CalculatorNamespace.JsonFormatter), new System.Type[]{typeof(object)}, new object[]{@value});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
case "jsii-calc.Jsii487Derived": return software.amazon.jsii.tests.calculator.Jsii487Derived.class;
case "jsii-calc.Jsii496Derived": return software.amazon.jsii.tests.calculator.Jsii496Derived.class;
case "jsii-calc.JsiiAgent": return software.amazon.jsii.tests.calculator.JsiiAgent.class;
case "jsii-calc.JsonFormatter": return software.amazon.jsii.tests.calculator.JsonFormatter.class;
case "jsii-calc.LoadBalancedFargateServiceProps": return software.amazon.jsii.tests.calculator.LoadBalancedFargateServiceProps.class;
case "jsii-calc.Multiply": return software.amazon.jsii.tests.calculator.Multiply.class;
case "jsii-calc.Negate": return software.amazon.jsii.tests.calculator.Negate.class;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package software.amazon.jsii.tests.calculator;

/**
* Make sure structs are un-decorated on the way in.
*
* EXPERIMENTAL
*
* @see https://github.com/aws/aws-cdk/issues/5066
*/
@javax.annotation.Generated(value = "jsii-pacmak")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.$Module.class, fqn = "jsii-calc.JsonFormatter")
public class JsonFormatter extends software.amazon.jsii.JsiiObject {

protected JsonFormatter(final software.amazon.jsii.JsiiObjectRef objRef) {
super(objRef);
}

protected JsonFormatter(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) {
super(initializationMode);
}

/**
* EXPERIMENTAL
*
* @param value This parameter is required.
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public static java.lang.String stringify(final java.lang.Object value) {
return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.jsii.tests.calculator.JsonFormatter.class, "stringify", java.lang.String.class, new Object[] { value });
}
}
Loading