Skip to content

Commit

Permalink
fix(kernel): make type serialization explicit and recursive
Browse files Browse the repository at this point in the history
Serialize and deserialize types according to their declared static type,
and add validation on the runtime types matching the declared types.

This is in contrast to previously, when we mostly only used the runtime
types to determine what to do, and harly any validation was done. The
runtime types used to be able to freely disagree with the declared
types, and we put a lot of burden on the JSII runtimes at the other
end of the wire.

Fix tests that used to exercise the API with invalid arguments.

Remove Proxy objects since they only existed to prevent accidentally
overwriting certain keys via the jsii interface, and Symbols serve
the same purpose (but simpler).

Fixes aws/aws-cdk#1981.
  • Loading branch information
Rico Huijbers committed Mar 26, 2019
1 parent dd07831 commit 4c0695c
Show file tree
Hide file tree
Showing 33 changed files with 1,891 additions and 512 deletions.
84 changes: 77 additions & 7 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as path from 'path';
import * as os from 'os';
import * as crypto from 'crypto';
import { promisify } from 'util';
import { composition, IFriendlyRandomGenerator, IRandomNumberGenerator, Multiply } from './calculator';
import { IFriendlyRandomGenerator, IRandomNumberGenerator, Multiply } from './calculator';

const bundled = require('jsii-calc-bundled');
import base = require('@scope/jsii-calc-base');
Expand Down Expand Up @@ -163,7 +163,7 @@ export class AllTypes {
// unions

unionProperty: string | number | Number | Multiply = 'foo';
unionArrayProperty: (composition.CompositeOperation | number)[] = [];
unionArrayProperty: (Value | number)[] = [];
unionMapProperty: { [key: string]: (Number | number | string) } = {};

// enum
Expand Down Expand Up @@ -194,6 +194,21 @@ export class AllTypes {
enumMethod(value: StringEnum) {
return value;
}


public anyOut(): any {
const ret = new Number(42);
Object.defineProperty(ret, 'tag', {
value: "you're it"
});
return ret;
}

public anyIn(inp: any) {
if (inp.tag !== "you're it") {
throw new Error(`Not the same object that I gave you, got: ${JSON.stringify(inp)}`);
}
}
}

//
Expand Down Expand Up @@ -1321,18 +1336,73 @@ export class PublicClass {
public hello(): void {}
}
export interface IPublicInterface {
bye(): void;
bye(): string;
}

export interface IPublicInterface2 {
ciao(): string;
}
export class InbetweenClass extends PublicClass implements IPublicInterface2 {
public ciao(): string { return 'ciao'; }
}
export class InbetweenClass extends PublicClass {}
class PrivateClass extends InbetweenClass implements IPublicInterface {
public bye(): void {}
public bye(): string { return 'bye'; }
}

class HiddenClass implements IPublicInterface, IPublicInterface2 {
public bye(): string { return 'bye'; }
public ciao(): string { return 'ciao'; }
}

class HiddenSubclass extends HiddenClass {
}

export class Constructors {
public static makeClass(): PublicClass {
return new PrivateClass();
return new PrivateClass(); // Wire type should be InbetweenClass
}

public static makeInterface(): IPublicInterface {
return new PrivateClass();
return new PrivateClass(); // Wire type should be IPublicInterface
}

public static makeInterface2(): IPublicInterface2 {
return new PrivateClass(); // Wire type should be InbetweenClass
}

public static makeInterfaces(): IPublicInterface[] {
return [new PrivateClass()]; // Wire type should be IPublicInterface[]
}

public static hiddenInterface(): IPublicInterface {
return new HiddenClass(); // Wire type should be IPublicInterface
}

public static hiddenInterfaces(): IPublicInterface[] {
return [new HiddenClass()]; // Wire type should be IPublicInterface[]
}

public static hiddenSubInterfaces(): IPublicInterface[] {
return [new HiddenSubclass()]; // Wire type should be IPublicInterface[]
}
}

/**
* Test that a single instance can be returned under two different FQNs
*
* JSII clients can instantiate 2 different strongly-typed wrappers for the same
* object. Unfortunately, this will break object equality, but if we didn't do
* this it would break runtime type checks in the JVM or CLR.
*/
export class SingleInstanceTwoTypes {
private instance = new PrivateClass();

public interface1(): InbetweenClass {
return this.instance;
}

public interface2(): IPublicInterface {
return this.instance;
}
}

Expand Down
133 changes: 130 additions & 3 deletions packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,23 @@
},
"kind": "class",
"methods": [
{
"name": "anyIn",
"parameters": [
{
"name": "inp",
"type": {
"primitive": "any"
}
}
]
},
{
"name": "anyOut",
"returns": {
"primitive": "any"
}
},
{
"name": "enumMethod",
"parameters": [
Expand Down Expand Up @@ -474,7 +491,7 @@
"primitive": "number"
},
{
"fqn": "jsii-calc.composition.CompositeOperation"
"fqn": "@scope/jsii-calc-lib.Value"
}
]
}
Expand Down Expand Up @@ -1217,6 +1234,37 @@
},
"kind": "class",
"methods": [
{
"name": "hiddenInterface",
"returns": {
"fqn": "jsii-calc.IPublicInterface"
},
"static": true
},
{
"name": "hiddenInterfaces",
"returns": {
"collection": {
"elementtype": {
"fqn": "jsii-calc.IPublicInterface"
},
"kind": "array"
}
},
"static": true
},
{
"name": "hiddenSubInterfaces",
"returns": {
"collection": {
"elementtype": {
"fqn": "jsii-calc.IPublicInterface"
},
"kind": "array"
}
},
"static": true
},
{
"name": "makeClass",
"returns": {
Expand All @@ -1230,6 +1278,25 @@
"fqn": "jsii-calc.IPublicInterface"
},
"static": true
},
{
"name": "makeInterface2",
"returns": {
"fqn": "jsii-calc.IPublicInterface2"
},
"static": true
},
{
"name": "makeInterfaces",
"returns": {
"collection": {
"elementtype": {
"fqn": "jsii-calc.IPublicInterface"
},
"kind": "array"
}
},
"static": true
}
],
"name": "Constructors"
Expand Down Expand Up @@ -2089,11 +2156,29 @@
"methods": [
{
"abstract": true,
"name": "bye"
"name": "bye",
"returns": {
"primitive": "string"
}
}
],
"name": "IPublicInterface"
},
"jsii-calc.IPublicInterface2": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.IPublicInterface2",
"kind": "interface",
"methods": [
{
"abstract": true,
"name": "ciao",
"returns": {
"primitive": "string"
}
}
],
"name": "IPublicInterface2"
},
"jsii-calc.IRandomNumberGenerator": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -2240,7 +2325,23 @@
"initializer": {
"initializer": true
},
"interfaces": [
{
"fqn": "jsii-calc.IPublicInterface2"
}
],
"kind": "class",
"methods": [
{
"name": "ciao",
"overrides": {
"fqn": "jsii-calc.IPublicInterface2"
},
"returns": {
"primitive": "string"
}
}
],
"name": "InbetweenClass"
},
"jsii-calc.InterfaceImplementedByAbstractClass": {
Expand Down Expand Up @@ -3561,6 +3662,32 @@
],
"name": "RuntimeTypeChecking"
},
"jsii-calc.SingleInstanceTwoTypes": {
"assembly": "jsii-calc",
"docs": {
"comment": "Test that a single instance can be returned under two different FQNs\n\nJSII clients can instantiate 2 different strongly-typed wrappers for the same\nobject. Unfortunately, this will break object equality, but if we didn't do\nthis it would break runtime type checks in the JVM or CLR."
},
"fqn": "jsii-calc.SingleInstanceTwoTypes",
"initializer": {
"initializer": true
},
"kind": "class",
"methods": [
{
"name": "interface1",
"returns": {
"fqn": "jsii-calc.InbetweenClass"
}
},
{
"name": "interface2",
"returns": {
"fqn": "jsii-calc.IPublicInterface"
}
}
],
"name": "SingleInstanceTwoTypes"
},
"jsii-calc.Statics": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.Statics",
Expand Down Expand Up @@ -4349,5 +4476,5 @@
}
},
"version": "0.8.0",
"fingerprint": "4LMgT0Rllw3CIJWiDiR/eUfSRPvCEeWyGWxJXMiDvcU="
"fingerprint": "7BN7XlOFufVEMTaAbMH5GDcE4zQmJj4iiDlYoXiRvCs="
}
3 changes: 3 additions & 0 deletions packages/jsii-java-runtime-test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Dive into a single failing test:

JSII_DEBUG=1 mvn test -Dtest=software.amazon.jsii.testing.ComplianceTest#unionTypes
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,12 @@ public void unionTypes() {

// map
Map<String, Object> map = new HashMap<>();
map.put("Foo", new Multiply(new Number(2), new Number(99)));
map.put("Foo", new Number(99));
types.setUnionMapProperty(map);

// array
types.setUnionArrayProperty(Arrays.asList("Hello", 123, new Number(33)));
assertEquals(33, ((Number)((List<?>)types.getUnionArrayProperty()).get(2)).getValue());
types.setUnionArrayProperty(Arrays.asList(123, new Number(33)));
assertEquals(33, ((Number)((List<?>)types.getUnionArrayProperty()).get(1)).getValue());
}


Expand Down
44 changes: 39 additions & 5 deletions packages/jsii-kernel/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,50 @@ export const TOKEN_REF = '$jsii.byref';
export const TOKEN_DATE = '$jsii.date';
export const TOKEN_ENUM = '$jsii.enum';

export class ObjRef {
[token: string]: string; // token = TOKEN_REF
export interface ObjRef {
[TOKEN_REF]: string;
}

export interface Override {
method?: string;
property?: string;
export function isObjRef(value: any): value is ObjRef {
return typeof value === 'object' && value !== null && TOKEN_REF in value;
}

export interface WireDate {
[TOKEN_DATE]: string;
}

export function isWireDate(value: any): value is WireDate {
return typeof value === 'object' && value !== null && TOKEN_DATE in value;
}

export interface WireEnum {
[TOKEN_ENUM]: string;
}

export function isWireEnum(value: any): value is WireEnum {
return typeof value === 'object' && value !== null && TOKEN_ENUM in value;
}

export type Override = MethodOverride | PropertyOverride;

export interface MethodOverride {
method: string;
cookie?: string;
}

export function isMethodOverride(value: Override): value is MethodOverride {
return (value as any).method != null; // Python passes "null"
}

export interface PropertyOverride {
property: string;
cookie?: string;
}

export function isPropertyOverride(value: Override): value is PropertyOverride {
return (value as any).property != null; // Python passes "null"
}

export interface Callback {
cbid: string;
cookie: string | undefined;
Expand Down
Loading

0 comments on commit 4c0695c

Please sign in to comment.