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): Return object literals as references #249

Merged
merged 5 commits into from
Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
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