Skip to content

Commit

Permalink
fix(kernel): Correctly return instances of un-exported types (#321)
Browse files Browse the repository at this point in the history
When an un-exported type that extends an exported type is returned with
an interface as the declared type, the JSII kernel used to return a ref
with the FQN of the exported supertype, instead of correctly wrapping
the instance in a proxy of the interface type as it should have.

---

Adds a test that covers the behavior of the JSII runtimes when a method
is declared to return an interface type, and returns an instance of a
private (un-exported) type that implements the interface while extending
an exported type. This has been seen to cause issues in the Java
runtime, for example, as the JSII kernel will return an ObjID with a
type fragment that refers to the exported super-class, and not the
interface type.

---

Fixes #302
  • Loading branch information
RomainMuller committed Nov 26, 2018
1 parent 44c3b9b commit 9c59acc
Show file tree
Hide file tree
Showing 20 changed files with 458 additions and 12 deletions.
4 changes: 3 additions & 1 deletion packages/jsii-build-tools/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1196,3 +1196,28 @@ export interface LoadBalancedFargateServiceProps {
*/
publicTasks?: boolean;
}

/**
* Helps ensure the JSII kernel & runtime cooperate correctly when an un-exported instance of a class is returned with
* a declared type that is an exported interface, and the instance inherits from an exported class.
*
* @returns an instance of an un-exported class that extends ``ExportedBaseClass``, declared as ``IPrivatelyImplemented``.
*
* @see https://github.com/awslabs/jsii/issues/320
*/
export class ReturnsPrivateImplementationOfInterface {
public get privateImplementation(): IPrivatelyImplemented {
return new PrivateImplementation();
}
}
export interface IPrivatelyImplemented {
readonly success: boolean;
}
export class ExportedBaseClass {
constructor(public readonly success: boolean) {}
}
class PrivateImplementation extends ExportedBaseClass implements IPrivatelyImplemented {
constructor() {
super(true);
}
}
67 changes: 66 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -1410,6 +1410,32 @@
],
"name": "DoubleTrouble"
},
"jsii-calc.ExportedBaseClass": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.ExportedBaseClass",
"initializer": {
"initializer": true,
"parameters": [
{
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"kind": "class",
"name": "ExportedBaseClass",
"properties": [
{
"immutable": true,
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.GiveMeStructs": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.GiveMeStructs",
Expand Down Expand Up @@ -1633,6 +1659,22 @@
],
"name": "IInterfaceWithOptionalMethodArguments"
},
"jsii-calc.IPrivatelyImplemented": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.IPrivatelyImplemented",
"kind": "interface",
"name": "IPrivatelyImplemented",
"properties": [
{
"abstract": true,
"immutable": true,
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.IRandomNumberGenerator": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -2825,6 +2867,29 @@
}
]
},
"jsii-calc.ReturnsPrivateImplementationOfInterface": {
"assembly": "jsii-calc",
"docs": {
"comment": "Helps ensure the JSII kernel & runtime cooperate correctly when an un-exported instance of a class is returned with\na declared type that is an exported interface, and the instance inherits from an exported class.",
"return": "an instance of an un-exported class that extends ``ExportedBaseClass``, declared as ``IPrivatelyImplemented``.",
"see": "https://github.com/awslabs/jsii/issues/320"
},
"fqn": "jsii-calc.ReturnsPrivateImplementationOfInterface",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "ReturnsPrivateImplementationOfInterface",
"properties": [
{
"immutable": true,
"name": "privateImplementation",
"type": {
"fqn": "jsii-calc.IPrivatelyImplemented"
}
}
]
},
"jsii-calc.RuntimeTypeChecking": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.RuntimeTypeChecking",
Expand Down Expand Up @@ -3671,5 +3736,5 @@
}
},
"version": "0.7.11",
"fingerprint": "2o7FtEirv0LpuaJ1G+wAxoHTW7FHr4U1taZ5GcVYt2o="
"fingerprint": "Nw3vyHfzec4IFe674buqsgL3x8QgvKP0lUC3csRIW7g="
}
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,12 @@ public void NullShouldBeTreatedAsUndefined()
obj.VerifyPropertyIsUndefined();
}

[Fact(DisplayName = Prefix + nameof(ReceiveInstanceOfPrivateClass))]
public void ReceiveInstanceOfPrivateClass()
{
Assert.True(new ReturnsPrivateImplementationOfInterface().PrivateImplementation.Success);
}

class NumberReturner : DeputyBase, IIReturnsNumber
{
public NumberReturner(double number)
Expand Down
7 changes: 5 additions & 2 deletions packages/jsii-java-runtime-test/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import software.amazon.jsii.tests.calculator.Polymorphism;
import software.amazon.jsii.tests.calculator.Power;
import software.amazon.jsii.tests.calculator.ReferenceEnumFromScopedPackage;
import software.amazon.jsii.tests.calculator.ReturnsPrivateImplementationOfInterface;
import software.amazon.jsii.tests.calculator.Statics;
import software.amazon.jsii.tests.calculator.Sum;
import software.amazon.jsii.tests.calculator.SyncVirtualMethods;
Expand Down Expand Up @@ -953,6 +954,14 @@ public void nullShouldBeTreatedAsUndefined() {
obj.verifyPropertyIsUndefined();
}

/**
* @see https://github.com/awslabs/jsii/issues/320
*/
@Test
public void receiveInstanceOfPrivateClass() {
assertTrue(new ReturnsPrivateImplementationOfInterface().getPrivateImplementation().getSuccess());
}

static class MulTen extends Multiply {
public MulTen(final int value) {
super(new Number(value), new Number(10));
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-java-runtime/pom.xml.t.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ process.stdout.write(`<?xml version="1.0" encoding="UTF-8"?>
<dependency>
<groupId>javax.annotation</groupId>
<artifactId>javax.annotation-api</artifactId>
<version>[1.2,)</version>
<version>[1.3.2,)</version>
<scope>provided</scope>
</dependency>
</dependencies>
Expand Down
11 changes: 9 additions & 2 deletions packages/jsii-kernel/lib/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,12 @@ export class Kernel {
case spec.TypeKind.Class:
case spec.TypeKind.Enum:
const constructor = this._findSymbol(fqn);
constructor.__jsii__ = { fqn };
Object.defineProperty(constructor, '__jsii__', {
configurable: false,
enumerable: false,
writable: false,
value: { fqn }
});
}
}
}
Expand Down Expand Up @@ -956,7 +961,9 @@ export class Kernel {
// have an object id, so we need to allocate one for it.
this._debug('creating objref for', v);
const fqn = this._fqnForObject(v);
return this._createObjref(v, fqn);
if (!targetType || !spec.isNamedTypeReference(targetType) || fqn === targetType.fqn) {
return this._createObjref(v, fqn);
}
}

// if the method/property returns an object literal and the return type
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-pacmak/lib/targets/java.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ class JavaGenerator extends Generator {

'properties': { 'project.build.sourceEncoding': 'UTF-8' },

'dependencies': { dependency: mavenDependencies() },
'dependencies': { dependency: mavenDependencies() },

'build': {
plugins: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1410,6 +1410,32 @@
],
"name": "DoubleTrouble"
},
"jsii-calc.ExportedBaseClass": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.ExportedBaseClass",
"initializer": {
"initializer": true,
"parameters": [
{
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"kind": "class",
"name": "ExportedBaseClass",
"properties": [
{
"immutable": true,
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.GiveMeStructs": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.GiveMeStructs",
Expand Down Expand Up @@ -1633,6 +1659,22 @@
],
"name": "IInterfaceWithOptionalMethodArguments"
},
"jsii-calc.IPrivatelyImplemented": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.IPrivatelyImplemented",
"kind": "interface",
"name": "IPrivatelyImplemented",
"properties": [
{
"abstract": true,
"immutable": true,
"name": "success",
"type": {
"primitive": "boolean"
}
}
]
},
"jsii-calc.IRandomNumberGenerator": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -2825,6 +2867,29 @@
}
]
},
"jsii-calc.ReturnsPrivateImplementationOfInterface": {
"assembly": "jsii-calc",
"docs": {
"comment": "Helps ensure the JSII kernel & runtime cooperate correctly when an un-exported instance of a class is returned with\na declared type that is an exported interface, and the instance inherits from an exported class.",
"return": "an instance of an un-exported class that extends ``ExportedBaseClass``, declared as ``IPrivatelyImplemented``.",
"see": "https://github.com/awslabs/jsii/issues/320"
},
"fqn": "jsii-calc.ReturnsPrivateImplementationOfInterface",
"initializer": {
"initializer": true
},
"kind": "class",
"name": "ReturnsPrivateImplementationOfInterface",
"properties": [
{
"immutable": true,
"name": "privateImplementation",
"type": {
"fqn": "jsii-calc.IPrivatelyImplemented"
}
}
]
},
"jsii-calc.RuntimeTypeChecking": {
"assembly": "jsii-calc",
"fqn": "jsii-calc.RuntimeTypeChecking",
Expand Down Expand Up @@ -3671,5 +3736,5 @@
}
},
"version": "0.7.11",
"fingerprint": "2o7FtEirv0LpuaJ1G+wAxoHTW7FHr4U1taZ5GcVYt2o="
"fingerprint": "Nw3vyHfzec4IFe674buqsgL3x8QgvKP0lUC3csRIW7g="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
[JsiiClass(typeof(ExportedBaseClass), "jsii-calc.ExportedBaseClass", "[{\"name\":\"success\",\"type\":{\"primitive\":\"boolean\"}}]")]
public class ExportedBaseClass : DeputyBase
{
public ExportedBaseClass(bool success): base(new DeputyProps(new object[]{success}))
{
}

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

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

[JsiiProperty("success", "{\"primitive\":\"boolean\"}")]
public virtual bool Success
{
get => GetInstanceProperty<bool>();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
[JsiiInterface(typeof(IIPrivatelyImplemented), "jsii-calc.IPrivatelyImplemented")]
public interface IIPrivatelyImplemented
{
[JsiiProperty("success", "{\"primitive\":\"boolean\"}")]
bool Success
{
get;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
[JsiiTypeProxy(typeof(IIPrivatelyImplemented), "jsii-calc.IPrivatelyImplemented")]
internal sealed class IPrivatelyImplementedProxy : DeputyBase, IIPrivatelyImplemented
{
private IPrivatelyImplementedProxy(ByRefValue reference): base(reference)
{
}

[JsiiProperty("success", "{\"primitive\":\"boolean\"}")]
public bool Success
{
get => GetInstanceProperty<bool>();
}
}
}
Loading

1 comment on commit 9c59acc

@RomainMuller
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes #320, not #302.

Please sign in to comment.