fix(python): correctly handle structs out of callbacks (#1009)

* fix(python): correctly handle structs out of callbacks

When moving out of callbacks, the result was not turned into a reference
before being passed out to the jsii kernel, causing TypeErrors when
attempting to turn structs (and probably other non-primitive types) to
JSON.

Discovered while attempting to reproduce #1003, which can no longer
trigger as it depended on returning a Struct through and `any` and being
hit with the bug fixed in #997... the fix also removed the conditions
that make leveraging the cause possible (or at the very least I could
not come up with a reproduction, and have verified that the original bug
report no longer reproduces once #997 is in).

That being said, still introduced the necessary changes to address the
cause by properly discovering interfaces and adding the relevant
overrides filtering, as this may improve the performance and reliability
of the jsii runtime for Java.

Fixes #1003

* improve documentation & test coverage

* fixup

* inline-fy doc

* remove spurious 'final'

* Add missing fget

* add type annotation to make MyPy happy

packages/jsii-calc/lib/compliance.ts

@@ -2396,3 +2396,17 @@ export class ConfusingToJackson {
 export interface ConfusingToJacksonStruct {
     readonly unionProperty?: Array<IFriendly | AbstractClass> | IFriendly;
 }
+
+/**
+ * Verifies that a "pure" implementation of an interface works correctly
+ */
+export interface IStructReturningDelegate {
+    returnStruct(): StructB;
+}
+export class ConsumePureInterface {
+    constructor(private readonly delegate: IStructReturningDelegate) { }
+
+    public workItBaby() {
+        return this.delegate.returnStruct();
+    }
+}

packages/jsii-calc/test/assembly.jsii

@@ -2533,6 +2533,49 @@
       ],
       "name": "Constructors"
     },
+    "jsii-calc.ConsumePureInterface": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "experimental"
+      },
+      "fqn": "jsii-calc.ConsumePureInterface",
+      "initializer": {
+        "docs": {
+          "stability": "experimental"
+        },
+        "parameters": [
+          {
+            "name": "delegate",
+            "type": {
+              "fqn": "jsii-calc.IStructReturningDelegate"
+            }
+          }
+        ]
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/compliance.ts",
+        "line": 2406
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2409
+          },
+          "name": "workItBaby",
+          "returns": {
+            "type": {
+              "fqn": "jsii-calc.StructB"
+            }
+          }
+        }
+      ],
+      "name": "ConsumePureInterface"
+    },
     "jsii-calc.ConsumerCanRingBell": {
       "assembly": "jsii-calc",
       "docs": {
@@ -5624,6 +5667,38 @@
         }
       ]
     },
+    "jsii-calc.IStructReturningDelegate": {
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "experimental",
+        "summary": "Verifies that a \"pure\" implementation of an interface works correctly."
+      },
+      "fqn": "jsii-calc.IStructReturningDelegate",
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/compliance.ts",
+        "line": 2403
+      },
+      "methods": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 2404
+          },
+          "name": "returnStruct",
+          "returns": {
+            "type": {
+              "fqn": "jsii-calc.StructB"
+            }
+          }
+        }
+      ],
+      "name": "IStructReturningDelegate"
+    },
     "jsii-calc.ImplementInternalInterface": {
       "assembly": "jsii-calc",
       "docs": {
@@ -11668,5 +11743,5 @@
     }
   },
   "version": "0.20.8",
-  "fingerprint": "s++IK3929zHRbg2MXmnn2A+PdvZnMVvPwaarJuUeBNM="
+  "fingerprint": "R5DFMiv/0riWRnreyHs899X53BbbVd5NTXmswDeF4/8="
 }

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

@@ -1310,5 +1310,82 @@ public void CanObtainStructReferenceWithOverloadedSetters()
         {
             Assert.NotNull(ConfusingToJackson.MakeStructInstance());
         }
+
+        [Fact(DisplayName = Prefix + nameof(PureInterfacesCanBeUsedTransparently))]
+        public void PureInterfacesCanBeUsedTransparently()
+        {
+            var expected = new StructB { RequiredString = "It's Britney b**ch!" };
+            var del = new StructReturningDelegate(expected);
+            var consumer = new ConsumePureInterface(del);
+            Assert.Equal(expected.RequiredString, consumer.WorkItBaby().RequiredString);
+        }
+
+        private sealed class StructReturningDelegate: DeputyBase, IStructReturningDelegate
+        {
+            internal StructReturningDelegate(StructB expected)
+            {
+                Expected = expected;
+            }
+
+            private IStructB Expected { get; }
+
+            public IStructB ReturnStruct()
+            {
+                return Expected;
+            }
+        }
+
+        [Fact(DisplayName = Prefix + nameof(PureInterfacesCanBeUsedTransparently_WhenTransitivelyImplemented))]
+        public void PureInterfacesCanBeUsedTransparently_WhenTransitivelyImplemented()
+        {
+            var expected = new StructB { RequiredString = "It's Britney b**ch!" };
+            var del = new IndirectlyImplementsStructReturningDelegate(expected);
+            var consumer = new ConsumePureInterface(del);
+            Assert.Equal(expected.RequiredString, consumer.WorkItBaby().RequiredString);
+        }
+
+        private sealed class IndirectlyImplementsStructReturningDelegate : ImplementsStructReturningDelegate
+        {
+            internal IndirectlyImplementsStructReturningDelegate(StructB @struct) : base(@struct) {}
+        }
+
+        private class ImplementsStructReturningDelegate : DeputyBase, IStructReturningDelegate
+        {
+            private StructB Struct;
+
+            protected ImplementsStructReturningDelegate(StructB @struct)
+            {
+                this.Struct = @struct;
+            }
+
+            public IStructB ReturnStruct()
+            {
+                return Struct;
+            }
+        }
+
+        [Fact(DisplayName = Prefix + nameof(PureInterfacesCanBeUsedTransparently_WhenAddedToJsiiType))]
+        public void PureInterfacesCanBeUsedTransparently_WhenAddedToJsiiType()
+        {
+            var expected = new StructB { RequiredString = "It's Britney b**ch!" };
+            var del = new ImplementsAdditionalInterface(expected);
+            var consumer = new ConsumePureInterface(del);
+            Assert.Equal(expected.RequiredString, consumer.WorkItBaby().RequiredString);
+        }
+
+        private sealed class ImplementsAdditionalInterface : AllTypes, IStructReturningDelegate
+        {
+            private StructB Struct;
+
+            internal ImplementsAdditionalInterface(StructB @struct)
+            {
+                this.Struct = @struct;
+            }
+
+            public IStructB ReturnStruct()
+            {
+                return Struct;
+            }
+        }
     }
 }

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

@@ -1594,4 +1594,68 @@ public void canObtainReferenceWithOverloadedSetter() {
     public void canObtainStructReferenceWithOverloadedSetter() {
         assertNotNull(ConfusingToJackson.makeStructInstance());
     }
+
+    @Test
+    public void pureInterfacesCanBeUsedTransparently() {
+        final StructB expected = StructB.builder()
+            .requiredString("It's Britney b**ch!")
+            .build();
+        final IStructReturningDelegate delegate = new IStructReturningDelegate() {
+            public StructB returnStruct() {
+                return expected;
+            }
+        };
+        final ConsumePureInterface consumer = new ConsumePureInterface(delegate);
+        assertEquals(expected, consumer.workItBaby());
+    }
+
+    @Test
+    public void pureInterfacesCanBeUsedTransparently_WhenTransitivelyImplementing() {
+        final StructB expected = StructB.builder()
+            .requiredString("It's Britney b**ch!")
+            .build();
+        final IStructReturningDelegate delegate = new IndirectlyImplementsStructReturningDelegate(expected);
+        final ConsumePureInterface consumer = new ConsumePureInterface(delegate);
+        assertEquals(expected, consumer.workItBaby());
+    }
+
+    private static final class IndirectlyImplementsStructReturningDelegate extends ImplementsStructReturningDelegate {
+        public IndirectlyImplementsStructReturningDelegate(final StructB struct) {
+            super(struct);
+        }
+    }
+
+    private static class ImplementsStructReturningDelegate implements IStructReturningDelegate {
+        private final StructB struct;
+
+        protected ImplementsStructReturningDelegate(final StructB struct) {
+            this.struct = struct;
+        }
+
+        public StructB returnStruct() {
+            return this.struct;
+        }
+    }
+
+    @Test
+    public void interfacesCanBeUsedTransparently_WhenAddedToJsiiType() {
+        final StructB expected = StructB.builder()
+            .requiredString("It's Britney b**ch!")
+            .build();
+        final IStructReturningDelegate delegate = new ImplementsAdditionalInterface(expected);
+        final ConsumePureInterface consumer = new ConsumePureInterface(delegate);
+        assertEquals(expected, consumer.workItBaby());
+    }
+
+    private static final class ImplementsAdditionalInterface extends AllTypes implements IStructReturningDelegate {
+        private final StructB struct;
+
+        public ImplementsAdditionalInterface(final StructB struct) {
+            this.struct = struct;
+        }
+
+        public StructB returnStruct() {
+            return this.struct;
+        }
+    }
 }

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/JsiiClientTest.java

@@ -46,14 +46,14 @@ public void setUp() {
 
     @Test
     public void initialTest() {
-        JsiiObjectRef obj = client.createObject("@scope/jsii-calc-lib.Number", Collections.singletonList(42));
+        JsiiObjectRef obj = client.createObject("@scope/jsii-calc-lib.Number", Collections.singletonList(42), Arrays.asList(), Arrays.asList());
         assertEquals(84, fromSandbox(client.getPropertyValue(obj, "doubleValue")));
         assertEquals("Number", fromSandbox(client.callMethod(obj, "typeName", toSandboxArray())));
 
         ObjectNode calculatorProps = JSON.objectNode();
         calculatorProps.set("initialValue", JSON.numberNode(100));
 
-        JsiiObjectRef calculator = client.createObject("jsii-calc.Calculator", Collections.singletonList(calculatorProps));
+        JsiiObjectRef calculator = client.createObject("jsii-calc.Calculator", Collections.singletonList(calculatorProps), Arrays.asList(), Arrays.asList());
         assertNull(fromSandbox(client.callMethod(calculator, "add", toSandboxArray(50))));
 
         JsiiObjectRef add = JsiiObjectRef.parse(client.getPropertyValue(calculator, "curr"));
@@ -79,7 +79,7 @@ public void initialTest() {
 
     @Test
     public void asyncMethods() {
-        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods", Arrays.asList());
+        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods", Arrays.asList(), Arrays.asList(), Arrays.asList());
 
         // begin will return a promise
         JsiiPromise promise = client.beginAsyncMethod(obj, "callMe", toSandboxArray());
@@ -99,8 +99,7 @@ private Collection<JsiiOverride> methodOverride(final String methodName, final S
 
     @Test
     public void asyncMethodOverrides() {
-        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods",
-                Arrays.asList(), methodOverride("overrideMe", "myCookie"));
+        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods", Arrays.asList(), methodOverride("overrideMe", "myCookie"), Arrays.asList());
 
         // begin will return a promise
         JsiiPromise promise = client.beginAsyncMethod(obj, "callMe", toSandboxArray());
@@ -129,8 +128,7 @@ public void asyncMethodOverrides() {
 
     @Test
     public void asyncMethodOverridesThrow() {
-        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods",
-                Arrays.asList(), methodOverride("overrideMe", "myCookie"));
+        JsiiObjectRef obj = client.createObject("jsii-calc.AsyncVirtualMethods", Arrays.asList(), methodOverride("overrideMe", "myCookie"), Arrays.asList());
 
         // begin will return a promise
         JsiiPromise promise = client.beginAsyncMethod(obj, "callMe", toSandboxArray());
@@ -165,8 +163,7 @@ public void asyncMethodOverridesThrow() {
 
     @Test
     public void syncVirtualMethods() {
-        JsiiObjectRef obj = client.createObject("jsii-calc.SyncVirtualMethods",
-                Arrays.asList(), methodOverride("virtualMethod","myCookie"));
+        JsiiObjectRef obj = client.createObject("jsii-calc.SyncVirtualMethods", Arrays.asList(), methodOverride("virtualMethod","myCookie"), Arrays.asList());
 
         jsiiRuntime.setCallbackHandler(callback -> {
             assertEquals(obj.getObjId(), JsiiObjectRef.parse(callback.getInvoke().getObjref()).getObjId());
@@ -175,7 +172,7 @@ public void syncVirtualMethods() {
             assertEquals("myCookie", callback.getCookie());
 
             // interact with jsii from inside the callback
-            JsiiObjectRef num = client.createObject("@scope/jsii-calc-lib.Number", Arrays.asList(42));
+            JsiiObjectRef num = client.createObject("@scope/jsii-calc-lib.Number", Arrays.asList(42), Arrays.asList(), Arrays.asList());
             assertEquals(84, fromSandbox(client.getPropertyValue(num, "doubleValue")));
 
             return JSON.numberNode(898);
@@ -206,7 +203,7 @@ public void staticProperties() {
         JsonNode defaultInstance = client.getStaticPropertyValue(fqn, "instance");
         assertEquals("default", client.getPropertyValue(JsiiObjectRef.parse(defaultInstance), "value").textValue());
 
-        JsiiObjectRef newValue = client.createObject(fqn, Arrays.asList("NewValue"));
+        JsiiObjectRef newValue = client.createObject(fqn, Arrays.asList("NewValue"), Arrays.asList(), Arrays.asList());
         client.setStaticPropertyValue(fqn, "instance", newValue.toJson());
 
         JsonNode newInstance = client.getStaticPropertyValue(fqn, "instance");

packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiClient.java

@@ -56,16 +56,6 @@ public void loadModule(final JsiiModule module) {
         }
     }
 
-    /**
-     * Creates a remote jsii object.
-     * @param fqn The fully-qualified-name of the class.
-     * @param initializerArgs Constructor arguments.
-     * @return A jsii object reference.
-     */
-    public JsiiObjectRef createObject(final String fqn, final List<Object> initializerArgs) {
-        return createObject(fqn, initializerArgs, Collections.emptyList());
-    }
-
     /**
      * Creates a remote jsii object.
      * @param fqn The fully-qualified-name of the class.
@@ -77,12 +67,14 @@ public JsiiObjectRef createObject(final String fqn, final List<Object> initializ
      */
     public JsiiObjectRef createObject(final String fqn,
                                       final Collection<Object> initializerArgs,
-                                      final Collection<JsiiOverride> overrides) {
+                                      final Collection<JsiiOverride> overrides,
+                                      final Collection<String> interfaces) {
 
         CreateRequest request = new CreateRequest();
         request.setFqn(fqn);
         request.setArgs(initializerArgs);
         request.setOverrides(overrides);
+        request.setInterfaces(interfaces);
 
         ObjectNode req = JsiiObjectMapper.valueToTree(request);
         req.put("api", "create");