fix(kernel): Correctly return instances of un-exported types (#321)

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
aws · Nov 26, 2018 · 9c59acc · 9c59acc · RomainMuller · Nov 26, 2018
1 parent 44c3b9b
commit 9c59acc
Show file tree

Hide file tree

Showing 20 changed files with 458 additions and 12 deletions.
diff --git a/packages/jsii-build-tools/package-lock.json b/packages/jsii-build-tools/package-lock.json
diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts
@@ -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);
+    }
+}
diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -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",
@@ -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": {
@@ -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",
@@ -3671,5 +3736,5 @@
     }
   },
   "version": "0.7.11",
-  "fingerprint": "2o7FtEirv0LpuaJ1G+wAxoHTW7FHr4U1taZ5GcVYt2o="
+  "fingerprint": "Nw3vyHfzec4IFe674buqsgL3x8QgvKP0lUC3csRIW7g="
 }
diff --git a/...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs b/...ges/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs
@@ -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)

diff --git a/packages/jsii-java-runtime-test/package-lock.json b/packages/jsii-java-runtime-test/package-lock.json
diff --git a/...-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java b/...-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java
@@ -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;
@@ -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));

diff --git a/packages/jsii-java-runtime/pom.xml.t.js b/packages/jsii-java-runtime/pom.xml.t.js
@@ -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>

diff --git a/packages/jsii-kernel/lib/kernel.ts b/packages/jsii-kernel/lib/kernel.ts
@@ -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 }
+                    });
             }
         }
     }
@@ -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

diff --git a/packages/jsii-pacmak/lib/targets/java.ts b/packages/jsii-pacmak/lib/targets/java.ts
@@ -389,7 +389,7 @@ class JavaGenerator extends Generator {
 
                     'properties': { 'project.build.sourceEncoding': 'UTF-8' },
 
-                    'dependencies': { dependency: mavenDependencies() },
+                    'dependencies': { dependency: mavenDependencies() },
 
                     'build': {
                         plugins: {

diff --git a/...es/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii b/...es/jsii-pacmak/test/expected.jsii-calc/dotnet/Amazon.JSII.Tests.CalculatorPackageId/.jsii
@@ -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",
@@ -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": {
@@ -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",
@@ -3671,5 +3736,5 @@
     }
   },
   "version": "0.7.11",
-  "fingerprint": "2o7FtEirv0LpuaJ1G+wAxoHTW7FHr4U1taZ5GcVYt2o="
+  "fingerprint": "Nw3vyHfzec4IFe674buqsgL3x8QgvKP0lUC3csRIW7g="
 }
diff --git a/...JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/ExportedBaseClass.cs b/...JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/ExportedBaseClass.cs
@@ -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>();
+        }
+    }
+}
diff --git a/...Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IIPrivatelyImplemented.cs b/...Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IIPrivatelyImplemented.cs
@@ -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;
+        }
+    }
+}
diff --git a/...s.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IPrivatelyImplementedProxy.cs b/...s.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/IPrivatelyImplementedProxy.cs
@@ -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>();
+        }
+    }
+}