fix(dotnet): stop mutating Dictionary when iterating on it

When bringding callback results into dotnet types, we used to modify the Dictionary item within the loop, which is illegal in C#. Any mutation on the collection will cause the iterator to abort. Insead, create a new Dictionary and add the resolved values to that. Fixes #690
aws · Aug 8, 2019 · 36c6395 · 36c6395
1 parent 3defca3
commit 36c6395
Show file tree

Hide file tree

Showing 11 changed files with 135 additions and 24 deletions.
diff --git a/packages/jsii-calc/lib/compliance.ts b/packages/jsii-calc/lib/compliance.ts
@@ -1737,6 +1737,10 @@ export class DataRenderer {
         return this.renderMap(data);
     }
 
+    public renderArbitrary(data: { [key: string]: any }): string {
+        return this.renderMap(data);
+    }
+
     public renderMap(map: { [key: string]: any }): string {
         return JSON.stringify(map, null, 2);
     }

diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii
@@ -2132,6 +2132,34 @@
             "filename": "lib/compliance.ts",
             "line": 1740
           },
+          "name": "renderArbitrary",
+          "parameters": [
+            {
+              "name": "data",
+              "type": {
+                "collection": {
+                  "elementtype": {
+                    "primitive": "any"
+                  },
+                  "kind": "map"
+                }
+              }
+            }
+          ],
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        },
+        {
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 1744
+          },
           "name": "renderMap",
           "parameters": [
             {
@@ -7160,7 +7188,7 @@
       "kind": "interface",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1762
+        "line": 1766
       },
       "name": "SecondLevelStruct",
       "properties": [
@@ -7173,7 +7201,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1766
+            "line": 1770
           },
           "name": "deeperRequiredProp",
           "type": {
@@ -7189,7 +7217,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1771
+            "line": 1775
           },
           "name": "deeperOptionalProp",
           "optional": true,
@@ -7816,7 +7844,7 @@
       "kind": "class",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1779
+        "line": 1783
       },
       "methods": [
         {
@@ -7825,7 +7853,7 @@
           },
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1788
+            "line": 1792
           },
           "name": "howManyVarArgsDidIPass",
           "parameters": [
@@ -7857,7 +7885,7 @@
           },
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1780
+            "line": 1784
           },
           "name": "roundTrip",
           "parameters": [
@@ -8242,7 +8270,7 @@
       "kind": "interface",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1745
+        "line": 1749
       },
       "name": "TopLevelStruct",
       "properties": [
@@ -8255,7 +8283,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1749
+            "line": 1753
           },
           "name": "required",
           "type": {
@@ -8271,7 +8299,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1759
+            "line": 1763
           },
           "name": "secondLevel",
           "type": {
@@ -8296,7 +8324,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1754
+            "line": 1758
           },
           "name": "optional",
           "optional": true,
@@ -9053,5 +9081,5 @@
     }
   },
   "version": "0.14.3",
-  "fingerprint": "HflAzjllHZLqkcqeSKW9kn6gvwv+uDmBpezvDUDp9RY="
+  "fingerprint": "Ld7vqOD5LvQ5a/I1LdRkj08XkdS+7syV580DELBDa+Y="
 }
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
@@ -920,6 +920,12 @@ public void CallbacksCorrectlyDeserializeArguments()
         {
             var obj = new DataRendererSubclass();
             Assert.Equal("{\n  \"anumber\": 42,\n  \"astring\": \"bazinga!\"\n}", obj.Render(null));
+
+            Assert.Equal("{\n  \"Key\": {},\n  \"Baz\": \"Zinga\"\n}", obj.RenderArbitrary(new Dictionary<string, object>()
+            {
+                { "Key", obj },
+                { "Baz", "Zinga" }
+            }));
         }
 
         class DataRendererSubclass : DataRenderer

diff --git a/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs b/packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs
@@ -144,15 +144,20 @@ private static object FromKernel(object obj, IReferenceMap referenceMap)
                  * result in an ArgumentError for not being able to convert JObject to IDictionary.
                  */
                 var dict = ((JObject)obj).ToObject<Dictionary<string, object>>();
+                var mapped = new Dictionary<string, object>(dict.Count);
                 foreach (var key in dict.Keys)
                 {
                     var value = dict[key];
                     if (value != null && value.GetType() == typeof(JObject))
                     {
-                        dict[key] = FromKernel(value, referenceMap);
+                        mapped[key] = FromKernel(value, referenceMap);
+                    }
+                    else
+                    {
+                        mapped[key] = value;
                     }
                 }
-                return dict;
+                return mapped;
             }
             return obj;
         }

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
@@ -2132,6 +2132,34 @@
             "filename": "lib/compliance.ts",
             "line": 1740
           },
+          "name": "renderArbitrary",
+          "parameters": [
+            {
+              "name": "data",
+              "type": {
+                "collection": {
+                  "elementtype": {
+                    "primitive": "any"
+                  },
+                  "kind": "map"
+                }
+              }
+            }
+          ],
+          "returns": {
+            "type": {
+              "primitive": "string"
+            }
+          }
+        },
+        {
+          "docs": {
+            "stability": "experimental"
+          },
+          "locationInModule": {
+            "filename": "lib/compliance.ts",
+            "line": 1744
+          },
           "name": "renderMap",
           "parameters": [
             {
@@ -7160,7 +7188,7 @@
       "kind": "interface",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1762
+        "line": 1766
       },
       "name": "SecondLevelStruct",
       "properties": [
@@ -7173,7 +7201,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1766
+            "line": 1770
           },
           "name": "deeperRequiredProp",
           "type": {
@@ -7189,7 +7217,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1771
+            "line": 1775
           },
           "name": "deeperOptionalProp",
           "optional": true,
@@ -7816,7 +7844,7 @@
       "kind": "class",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1779
+        "line": 1783
       },
       "methods": [
         {
@@ -7825,7 +7853,7 @@
           },
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1788
+            "line": 1792
           },
           "name": "howManyVarArgsDidIPass",
           "parameters": [
@@ -7857,7 +7885,7 @@
           },
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1780
+            "line": 1784
           },
           "name": "roundTrip",
           "parameters": [
@@ -8242,7 +8270,7 @@
       "kind": "interface",
       "locationInModule": {
         "filename": "lib/compliance.ts",
-        "line": 1745
+        "line": 1749
       },
       "name": "TopLevelStruct",
       "properties": [
@@ -8255,7 +8283,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1749
+            "line": 1753
           },
           "name": "required",
           "type": {
@@ -8271,7 +8299,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1759
+            "line": 1763
           },
           "name": "secondLevel",
           "type": {
@@ -8296,7 +8324,7 @@
           "immutable": true,
           "locationInModule": {
             "filename": "lib/compliance.ts",
-            "line": 1754
+            "line": 1758
           },
           "name": "optional",
           "optional": true,
@@ -9053,5 +9081,5 @@
     }
   },
   "version": "0.14.3",
-  "fingerprint": "HflAzjllHZLqkcqeSKW9kn6gvwv+uDmBpezvDUDp9RY="
+  "fingerprint": "Ld7vqOD5LvQ5a/I1LdRkj08XkdS+7syV580DELBDa+Y="
 }
diff --git a/...azon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DataRenderer.cs b/...azon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/DataRenderer.cs
@@ -33,6 +33,15 @@ public virtual string Render(Amazon.JSII.Tests.CalculatorNamespace.LibNamespace.
             return InvokeInstanceMethod<string>(new object[]{data});
         }
 
+        /// <remarks>
+        /// stability: Experimental
+        /// </remarks>
+        [JsiiMethod(name: "renderArbitrary", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"data\",\"type\":{\"collection\":{\"elementtype\":{\"primitive\":\"any\"},\"kind\":\"map\"}}}]")]
+        public virtual string RenderArbitrary(System.Collections.Generic.IDictionary<string, object> data)
+        {
+            return InvokeInstanceMethod<string>(new object[]{data});
+        }
+
         /// <remarks>
         /// stability: Experimental
         /// </remarks>

diff --git a/...cted.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DataRenderer.java b/...cted.jsii-calc/java/src/main/java/software/amazon/jsii/tests/calculator/DataRenderer.java
@@ -37,6 +37,14 @@ public java.lang.String render() {
         return this.jsiiCall("render", java.lang.String.class);
     }
 
+    /**
+     * EXPERIMENTAL
+     */
+    @software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
+    public java.lang.String renderArbitrary(final java.util.Map<java.lang.String, java.lang.Object> data) {
+        return this.jsiiCall("renderArbitrary", java.lang.String.class, new Object[] { java.util.Objects.requireNonNull(data, "data is required") });
+    }
+
     /**
      * EXPERIMENTAL
      */

diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py b/packages/jsii-pacmak/test/expected.jsii-calc/python/src/jsii_calc/__init__.py
@@ -850,6 +850,16 @@ def render(self, *, anumber: jsii.Number, astring: str, first_optional: typing.O
 
         return jsii.invoke(self, "render", [data])
 
+    @jsii.member(jsii_name="renderArbitrary")
+    def render_arbitrary(self, data: typing.Mapping[str,typing.Any]) -> str:
+        """
+        :param data: -
+
+        stability
+        :stability: experimental
+        """
+        return jsii.invoke(self, "renderArbitrary", [data])
+
     @jsii.member(jsii_name="renderMap")
     def render_map(self, map: typing.Mapping[str,typing.Any]) -> str:
         """

diff --git a/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst b/packages/jsii-pacmak/test/expected.jsii-calc/sphinx/jsii-calc.rst
@@ -1479,6 +1479,13 @@ DataRenderer
       :rtype: string
 
 
+   .. py:method:: renderArbitrary(data) -> string
+
+      :param data: 
+      :type data: string => any
+      :rtype: string
+
+
    .. py:method:: renderMap(map) -> string
 
       :param map: 

diff --git a/packages/jsii-reflect/test/jsii-tree.test.all.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.all.expected.txt
@@ -320,6 +320,11 @@ assemblies
  │   │   │ │ └─┬ data
  │   │   │ │   └── type: Optional<@scope/jsii-calc-lib.MyFirstStruct>
  │   │   │ └── returns: string
+ │   │   ├─┬ renderArbitrary(data) method (experimental)
+ │   │   │ ├─┬ parameters
+ │   │   │ │ └─┬ data
+ │   │   │ │   └── type: Map<string => any>
+ │   │   │ └── returns: string
  │   │   └─┬ renderMap(map) method (experimental)
  │   │     ├─┬ parameters
  │   │     │ └─┬ map

diff --git a/packages/jsii-reflect/test/jsii-tree.test.members.expected.txt b/packages/jsii-reflect/test/jsii-tree.test.members.expected.txt
@@ -136,6 +136,7 @@ assemblies
  │   │ └─┬ members
  │   │   ├── <initializer>() initializer
  │   │   ├── render(data) method
+ │   │   ├── renderArbitrary(data) method
  │   │   └── renderMap(map) method
  │   ├─┬ class DefaultedConstructorArgument
  │   │ └─┬ members