fix(dotnet): fix deep type conversion across the process boundary, in…

…telisense docs, set target to netcoreapp2.1 (#772) This PR fixes various issues due to how we convert collection elements, and how the type reference is passed when converting. These two issues are similar and are due to nested Arrays of object not being casted properly: Fixes aws/aws-cdk#3244 Fixes aws/aws-cdk#3672 Added a unit test to cover the case This issue is due to Maps of Arrays of Anys not being casted properly: Fixes aws/aws-cdk#3813 Added a unit test to cover the case Also added support for xml documentation which is now added to the NuGet packages and should allow intellisense to provide in IDE help: Fixes #749 Fixes aws/aws-cdk#1846 Migrated the target framework to netcoreapp2.1 instead of netstandard2.0: Fixes #714
aws · Sep 18, 2019 · ecf8d3b · ecf8d3b
1 parent 2e10060
commit ecf8d3b
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 18 deletions.
diff --git a/...est/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj b/...est/test/Amazon.JSII.Runtime.IntegrationTests/Amazon.JSII.Runtime.IntegrationTests.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
     <PropertyGroup>
-        <TargetFramework>netcoreapp2.0</TargetFramework>
+        <TargetFramework>netcoreapp2.1</TargetFramework>
 
         <IsPackable>false</IsPackable>
     </PropertyGroup>

diff --git a/...time/src/Amazon.JSII.Runtime.UnitTests/Deputy/Converters/FrameworkToJsiiConverterTests.cs b/...time/src/Amazon.JSII.Runtime.UnitTests/Deputy/Converters/FrameworkToJsiiConverterTests.cs
@@ -450,6 +450,118 @@ public void RecursivelyConvertsArrayElements()
                     }
                 );
             }
+
+            [Fact(DisplayName = _Prefix + nameof(RecursivelyConvertsMapElementsWithMapOfAny))]
+            public void RecursivelyConvertsMapElementsWithMapOfAny()
+            {
+                var instance = new OptionalValue(new TypeReference(
+                    collection: new CollectionTypeReference(CollectionKind.Map,
+                        new TypeReference(
+                            collection: new CollectionTypeReference(CollectionKind.Map,
+                                new TypeReference(primitive: PrimitiveType.Any)
+                            )
+                        )
+                    )
+                ));
+
+                var frameworkMap = new Dictionary<string, IDictionary<string, object>>
+                {
+                    { "myKey1", new Dictionary<string, object> { { "mySubKey1", "myValue1" } } },
+                    { "myKey2", new Dictionary<string, object> { { "mySubKey2", "myValue2" } } },
+                };
+
+                // This will test the call to FrameworkToJsiiConverter.TryConvertCollectionElement()
+                // In the case of a of a Map of Map of Any
+                bool success = _converter.TryConvert(instance, _referenceMap, frameworkMap, out object actual);
+
+                Assert.True(success);
+                Assert.IsType<JObject>(actual);
+                Assert.Collection
+                (
+                    ((IEnumerable<KeyValuePair<string, JToken>>)actual).OrderBy(kvp => kvp.Key),
+                    kvp =>
+                    {
+                        Assert.Equal("myKey1", kvp.Key);
+                        Assert.IsType<JObject>(kvp.Value);
+                        Assert.Collection
+                        (
+                            ((IEnumerable<KeyValuePair<string, JToken>>)kvp.Value),
+                            subKvp =>
+                            {
+                                Assert.Equal("mySubKey1", subKvp.Key, ignoreLineEndingDifferences: true);
+                                Assert.Equal("myValue1", subKvp.Value);
+                            }
+                        );
+                    },
+                    kvp =>
+                    {
+                        Assert.Equal("myKey2", kvp.Key, ignoreLineEndingDifferences: true);
+                        Assert.IsType<JObject>(kvp.Value);
+                        Assert.Collection
+                        (
+                            ((IEnumerable<KeyValuePair<string, JToken>>)kvp.Value),
+                            subKvp =>
+                            {
+                                Assert.Equal("mySubKey2", subKvp.Key, ignoreLineEndingDifferences: true);
+                                Assert.Equal("myValue2", subKvp.Value);
+                            }
+                        );
+                    }
+                );
+            }
+
+            [Fact(DisplayName = _Prefix + nameof(RecursivelyConvertsMapElementsWithArrayOfAny))]
+            public void RecursivelyConvertsMapElementsWithArrayOfAny()
+            {
+                var instance = new OptionalValue(new TypeReference
+                (
+                    collection: new CollectionTypeReference(CollectionKind.Map,
+                        new TypeReference
+                        (
+                            collection: new CollectionTypeReference(CollectionKind.Array,
+                                new TypeReference(primitive: PrimitiveType.Any)
+                            )
+                        )
+                    )
+                ));
+
+                var frameworkArray = new Dictionary<string, object>()
+                {
+                    {"key", new [] { "true" }},
+                    {"key2", new [] { false }},
+                };
+
+                // This will test the call to FrameworkToJsiiConverter.TryConvertCollectionElement()
+                // In the case of a of a Map of Array of Any
+                bool success = _converter.TryConvert(instance, _referenceMap, frameworkArray, out object actual);
+
+                Assert.True(success);
+                Assert.IsType<JObject>(actual);
+                Assert.Collection
+                (
+                    ((IEnumerable<KeyValuePair<string, JToken>>)actual).OrderBy(kvp => kvp.Key),
+                    kvp =>
+                    {
+                        Assert.Equal("key", kvp.Key);
+                        Assert.IsType<JArray>(kvp.Value);
+                        Assert.Collection
+                        (
+                            (JArray)kvp.Value,
+                            subValue => Assert.Equal("true", subValue)
+                        );
+                    },
+                    kvp =>
+                    {
+                        Assert.Equal("key2", kvp.Key, ignoreLineEndingDifferences: true);
+                        Assert.IsType<JArray>(kvp.Value);
+                        Assert.Collection
+                        (
+                            (JArray)kvp.Value,
+                            subValue => Assert.Equal(false, subValue)
+                        );
+                    }
+                );
+            }
 
             [Fact(DisplayName = _Prefix + nameof(ConvertsNullMap))]
             public void ConvertsNullMap()

diff --git a/...ii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/Converters/FrameworkToJsiiConverter.cs b/...ii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/Converters/FrameworkToJsiiConverter.cs
@@ -157,7 +157,7 @@ protected override bool TryConvertJson(object value, out object result)
                 return true;
             }
 
-            if (value.GetType().IsAssignableFrom(typeof(JObject)))
+            if (value.GetType().IsAssignableFrom(typeof(JObject)) || value.GetType().IsAssignableFrom(typeof(JArray)))
             {
                 result = value;
                 return true;
@@ -223,7 +223,7 @@ protected override bool TryConvertArray(IReferenceMap referenceMap, TypeReferenc
             JArray resultArray = new JArray();
             foreach (object element in array)
             {
-                if (!TryConvert(elementType, referenceMap, element, out object convertedElement))
+                if (!TryConvertCollectionElement(element, referenceMap, elementType, out object convertedElement))
                 {
                     result = null;
                     return false;
@@ -264,15 +264,7 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
             {
                 object element = indexer.GetValue(value, new object[] {key});
 
-                TypeReference childElementType = InferType(referenceMap, element);
-
-                // We should not pass the parent element type as we are in a map
-                // A map<string, object> could be a map<string, map<string, object> etc
-                // If we pass the parent referenceMap then it will try to convert it as Any
-                // So by inferring the child element type we are always converting the correct type.
-                // See https://github.com/aws/aws-cdk/issues/2496
-
-                if (!TryConvert(childElementType, referenceMap, element, out object convertedElement))
+                if (!TryConvertCollectionElement(element, referenceMap, elementType, out object convertedElement))
                 {
                     result = null;
                     return false;
@@ -285,6 +277,47 @@ protected override bool TryConvertMap(IReferenceMap referenceMap, TypeReference
             return true;
         }
 
+        /// <summary>
+        /// Converts a collection element
+        /// </summary>
+        /// <param name="element">The element to convert in the collection</param>
+        /// <param name="referenceMap">The known references map</param>
+        /// <param name="elementType">The TypeReference of the element, as seen by Jsii</param>
+        /// <param name="convertedElement">out: the converted element</param>
+        /// <returns>True if the conversion was successful, false otherwise</returns>
+        private bool TryConvertCollectionElement(object element, IReferenceMap referenceMap, TypeReference elementType,
+            out object convertedElement)
+        {
+            if (element is IDictionary<string, object> || element is object[])
+            {
+                var objectType = InferType(referenceMap, element);
+                var nestedType = elementType.Primitive == PrimitiveType.Any ? elementType : objectType.Collection.ElementType;
+                switch (objectType.Collection?.Kind)
+                {
+                    case CollectionKind.Map:
+                        // We should not pass the parent element type as we are
+                        // in a map<string, object> containing another map.
+                        // If we pass the parent elementType then it will try to convert it as Any
+                        // So we can directly convert to another map here, and forgo the type hierarchy
+                        // induced by elementType
+                        // See https://github.com/aws/aws-cdk/issues/2496
+                        return TryConvertMap(referenceMap, nestedType, element,
+                                out convertedElement);
+                    case CollectionKind.Array:
+                        // The [object] could be another array. (ie Tags)
+                        // https://github.com/aws/aws-cdk/issues/3244
+                        return TryConvertArray(referenceMap, nestedType, element,
+                                out convertedElement);
+                    default:
+                        return TryConvert(elementType, referenceMap, element, out convertedElement);
+                }
+            }
+            else
+            {
+                return TryConvert(elementType, referenceMap, element, out convertedElement);
+            }
+        }
+
         protected override TypeReference InferType(IReferenceMap referenceMap, object value)
         {
             value = value ?? throw new ArgumentNullException(nameof(value));
@@ -328,7 +361,7 @@ TypeReference InferType(IReferenceMap referenceMap, Type type)
                 return new TypeReference(primitive: PrimitiveType.Date);
             }
 
-            if (type.IsAssignableFrom(typeof(JObject)))
+            if (type.IsAssignableFrom(typeof(JObject)) || type.IsAssignableFrom(typeof(JArray)))
             {
                 return new TypeReference(primitive: PrimitiveType.Json);
             }

diff --git a/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts
@@ -50,8 +50,9 @@ export class FileGenerator {
         const rootNode = xmlbuilder.create('Project', {encoding: 'UTF-8', headless: true});
         rootNode.att("Sdk", "Microsoft.NET.Sdk");
         const propertyGroup = rootNode.ele("PropertyGroup");
-        propertyGroup.ele("TargetFramework", "netstandard2.0");
+        propertyGroup.ele("TargetFramework", "netcoreapp2.1");
         propertyGroup.ele("GeneratePackageOnBuild", "true");
+        propertyGroup.ele("GenerateDocumentationFile", "true");
         propertyGroup.ele("IncludeSymbols", "true");
         propertyGroup.ele("IncludeSource", "true");
         propertyGroup.ele("PackageVersion", this.getDecoratedVersion(assembly));
@@ -77,7 +78,7 @@ export class FileGenerator {
         }
 
         if (dotnetInfo!.iconUrl != null) {
-            propertyGroup.ele("IconUrl", dotnetInfo!.iconUrl);
+            propertyGroup.ele("PackageIconUrl", dotnetInfo!.iconUrl);
         }
 
         const itemGroup1 = rootNode.ele("ItemGroup");

diff --git a/...culatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj b/...culatorPackageId.BasePackageId/Amazon.JSII.Tests.CalculatorPackageId.BasePackageId.csproj
@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0</PackageVersion>

diff --git a/...alculatorPackageId.LibPackageId/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId.csproj b/...alculatorPackageId.LibPackageId/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId.csproj
@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0-devpreview</PackageVersion>

diff --git a/...dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj b/...dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon.JSII.Tests.CalculatorPackageId.csproj
@@ -1,7 +1,8 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <IncludeSymbols>true</IncludeSymbols>
     <IncludeSource>true</IncludeSource>
     <PackageVersion>0.16.0</PackageVersion>