Skip to content

Commit

Permalink
fix(kernel): Return object literals as references (#249)
Browse files Browse the repository at this point in the history
Use the javascript `Proxy` class to coalesce object literals to an
interface type, allowing them to be returned by reference instead of by
value.

Introduces a test in the `jsii-kernel` to demonstrate the feature works
as intended.

Fixes #248
Fixes aws/aws-cdk#774
  • Loading branch information
RomainMuller committed Sep 28, 2018
1 parent 96ac5d6 commit 61cb3a4
Show file tree
Hide file tree
Showing 17 changed files with 517 additions and 11 deletions.
8 changes: 8 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -887,3 +887,11 @@ export class AbstractClassReturner {
}
}
}

export interface MutableObjectLiteral {
value: string;
}

export class ClassWithMutableObjectLiteralProperty {
public mutableObject: MutableObjectLiteral = { value: 'default' };
}
35 changes: 34 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,23 @@
}
]
},
"jsii-calc.ClassWithMutableObjectLiteralProperty": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.ClassWithMutableObjectLiteralProperty",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "ClassWithMutableObjectLiteralProperty",
"properties": [
{
"name": "mutableObject",
"type": {
"fqn": "jsii-calc.MutableObjectLiteral"
}
}
]
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.DefaultedConstructorArgument",
Expand Down Expand Up @@ -1883,6 +1900,22 @@
}
]
},
"jsii-calc.MutableObjectLiteral": {
"assembly": "jsii-calc",
"datatype": true,
"fqn": "jsii-calc.MutableObjectLiteral",
"kind": "interface",
"name": "MutableObjectLiteral",
"properties": [
{
"abstract": true,
"name": "value",
"type": {
"primitive": "string"
}
}
]
},
"jsii-calc.Negate": {
"assembly": "jsii-calc",
"base": {
Expand Down Expand Up @@ -3230,5 +3263,5 @@
}
},
"version": "0.7.6",
"fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY="
"fingerprint": "DFShLmIJQmPjTv5Dmz4JoBKqoCtAyifD1RpCuHo+sEc="
}
135 changes: 129 additions & 6 deletions packages/jsii-kernel/lib/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { TOKEN_DATE, TOKEN_ENUM, TOKEN_REF } from './api';
*/
const OBJID_PROP = '$__jsii__objid__$';
const FQN_PROP = '$__jsii__fqn__$';
const PROXIES_PROP = '$__jsii__proxies__$';
const PROXY_REFERENT_PROP = '$__jsii__proxy_referent__$';

/**
* A special FQN that can be used to create empty javascript objects.
Expand Down Expand Up @@ -149,9 +151,14 @@ export class Kernel {
const { objref } = req;

this._debug('del', objref);
this._findObject(objref); // make sure object exists
const obj = this._findObject(objref); // make sure object exists
delete this.objects[objref[TOKEN_REF]];

if (obj[PROXY_REFERENT_PROP]) {
// De-register the proxy if this was a proxy...
delete obj[PROXY_REFERENT_PROP][PROXIES_PROP][obj[FQN_PROP]];
}

return { };
}

Expand Down Expand Up @@ -923,12 +930,20 @@ export class Kernel {
// so the client receives a real object.
if (typeof(v) === 'object' && targetType && spec.isNamedTypeReference(targetType)) {
this._debug('coalescing to', targetType);
const newObjRef = this._create({ fqn: targetType.fqn });
const newObj = this._findObject(newObjRef);
for (const k of Object.keys(v)) {
newObj[k] = v[k];
/*
* We "cache" proxy instances in [PROXIES_PROP] so we can return an
* identical object reference upon multiple accesses of the same
* object literal under the same exposed type. This results in a
* behavior that is more consistent with class instances.
*/
const proxies: Proxies = v[PROXIES_PROP] = v[PROXIES_PROP] || {};
if (!proxies[targetType.fqn]) {
const handler = new KernelProxyHandler(v);
const proxy = new Proxy(v, handler);
// _createObjref will set the FQN_PROP & OBJID_PROP on the proxy.
proxies[targetType.fqn] = { objRef: this._createObjref(proxy, targetType.fqn), handler };
}
return newObjRef;
return proxies[targetType.fqn].objRef;
}

// date (https://stackoverflow.com/a/643827/737957)
Expand Down Expand Up @@ -1135,3 +1150,111 @@ function mapSource(err: Error, sourceMaps: { [assm: string]: SourceMapConsumer }
return frame;
}
}

type ObjectKey = string | number | symbol;
/**
* A Proxy handler class to support mutation of the returned object literals, as
* they may "embody" several different interfaces. The handler is in particular
* responsible to make sure the ``FQN_PROP`` and ``OBJID_PROP`` do not get set
* on the ``referent`` object, for this would cause subsequent accesses to
* possibly return incorrect object references.
*/
class KernelProxyHandler implements ProxyHandler<any> {
private readonly ownProperties: { [key: string]: any } = {};

/**
* @param referent the "real" value that will be returned.
*/
constructor(public readonly referent: any) {
/*
* Proxy-properties must exist as non-configurable & writable on the
* referent, otherwise the Proxy will not allow returning ``true`` in
* response to ``defineProperty``.
*/
for (const prop of [FQN_PROP, OBJID_PROP]) {
Object.defineProperty(referent, prop, {
configurable: false,
enumerable: false,
writable: true,
value: undefined
});
}
}

public defineProperty(target: any, property: ObjectKey, attributes: PropertyDescriptor): boolean {
switch (property) {
case FQN_PROP:
case OBJID_PROP:
return Object.defineProperty(this.ownProperties, property, attributes);
default:
return Object.defineProperty(target, property, attributes);
}
}

public deleteProperty(target: any, property: ObjectKey): boolean {
switch (property) {
case FQN_PROP:
case OBJID_PROP:
delete this.ownProperties[property];
break;
default:
delete target[property];
}
return true;
}

public getOwnPropertyDescriptor(target: any, property: ObjectKey): PropertyDescriptor | undefined {
switch (property) {
case FQN_PROP:
case OBJID_PROP:
return Object.getOwnPropertyDescriptor(this.ownProperties, property);
default:
return Object.getOwnPropertyDescriptor(target, property);
}
}

public get(target: any, property: ObjectKey): any {
switch (property) {
// Magical property for the proxy, so we can tell it's one...
case PROXY_REFERENT_PROP:
return this.referent;
case FQN_PROP:
case OBJID_PROP:
return this.ownProperties[property];
default:
return target[property];
}
}

public set(target: any, property: ObjectKey, value: any): boolean {
switch (property) {
case FQN_PROP:
case OBJID_PROP:
this.ownProperties[property] = value;
break;
default:
target[property] = value;
}
return true;
}

public has(target: any, property: ObjectKey): boolean {
switch (property) {
case FQN_PROP:
case OBJID_PROP:
return property in this.ownProperties;
default:
return property in target;
}
}

public ownKeys(target: any): ObjectKey[] {
return Reflect.ownKeys(target).concat(Reflect.ownKeys(this.ownProperties));
}
}

type Proxies = { [fqn: string]: ProxyReference };
interface ProxyReference {
objRef: api.ObjRef;
handler: KernelProxyHandler;
}
17 changes: 17 additions & 0 deletions packages/jsii-kernel/test/test.kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,23 @@ defineTest('node.js standard library', async (test, sandbox) => {
{ result: "6a2da20943931e9834fc12cfe5bb47bbd9ae43489a30726962b576f4e3993e50" });
});

// @see awslabs/jsii#248
defineTest('object literals are returned by reference', async (test, sandbox) => {
const objref = sandbox.create({ fqn: 'jsii-calc.ClassWithMutableObjectLiteralProperty' });
const property = sandbox.get({ objref, property: 'mutableObject' }).value;

const newValue = 'Bazinga!1!';
sandbox.set({ objref: property, property: 'value', value: newValue });

test.equal(newValue,
sandbox.get({
objref: sandbox.get({ objref, property: 'mutableObject' }).value,
property: 'value'
}).value);

sandbox.del({ objref: property });
});

const testNames: { [name: string]: boolean } = { };

async function createCalculatorSandbox(name: string) {
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-kernel/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
/* Source Map Options */
// "sourceRoot": "./", /* Specify the location where debugger should locate TypeScript files instead of source locations. */
// "mapRoot": "./", /* Specify the location where debugger should locate map files instead of generated locations. */
// "inlineSourceMap": false, /* Emit a single file with source maps instead of having a separate file. */
// "inlineSources": false, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */
"inlineSourceMap": true, /* Emit a single file with source maps instead of having a separate file. */
"inlineSources": true, /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

/* Experimental Options */
"experimentalDecorators": true /* Enables experimental support for ES7 decorators. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,23 @@
}
]
},
"jsii-calc.ClassWithMutableObjectLiteralProperty": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.ClassWithMutableObjectLiteralProperty",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "ClassWithMutableObjectLiteralProperty",
"properties": [
{
"name": "mutableObject",
"type": {
"fqn": "jsii-calc.MutableObjectLiteral"
}
}
]
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.DefaultedConstructorArgument",
Expand Down Expand Up @@ -1883,6 +1900,22 @@
}
]
},
"jsii-calc.MutableObjectLiteral": {
"assembly": "jsii-calc",
"datatype": true,
"fqn": "jsii-calc.MutableObjectLiteral",
"kind": "interface",
"name": "MutableObjectLiteral",
"properties": [
{
"abstract": true,
"name": "value",
"type": {
"primitive": "string"
}
}
]
},
"jsii-calc.Negate": {
"assembly": "jsii-calc",
"base": {
Expand Down Expand Up @@ -3230,5 +3263,5 @@
}
},
"version": "0.7.6",
"fingerprint": "ecDtx3DHVZi7UjyCDhGncg4jbSRaD536bUyh6YzAxlY="
"fingerprint": "DFShLmIJQmPjTv5Dmz4JoBKqoCtAyifD1RpCuHo+sEc="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
[JsiiClass(typeof(ClassWithMutableObjectLiteralProperty), "jsii-calc.ClassWithMutableObjectLiteralProperty", "[]")]
public class ClassWithMutableObjectLiteralProperty : DeputyBase
{
public ClassWithMutableObjectLiteralProperty(): base(new DeputyProps(new object[]{}))
{
}

protected ClassWithMutableObjectLiteralProperty(ByRefValue reference): base(reference)
{
}

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

[JsiiProperty("mutableObject", "{\"fqn\":\"jsii-calc.MutableObjectLiteral\"}")]
public virtual IMutableObjectLiteral MutableObject
{
get => GetInstanceProperty<IMutableObjectLiteral>();
set => SetInstanceProperty(value);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
[JsiiInterface(typeof(IMutableObjectLiteral), "jsii-calc.MutableObjectLiteral")]
public interface IMutableObjectLiteral
{
[JsiiProperty("value", "{\"primitive\":\"string\"}")]
string Value
{
get;
set;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
public class MutableObjectLiteral : DeputyBase, IMutableObjectLiteral
{
[JsiiProperty("value", "{\"primitive\":\"string\"}", true)]
public string Value
{
get;
set;
}
}
}
Loading

0 comments on commit 61cb3a4

Please sign in to comment.