Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dotnet): fix deep type conversion across the process boundary, intelisense docs, set target to netcoreapp2.1 #772

Merged
merged 12 commits into from
Sep 18, 2019
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netcoreapp2.0</TargetFramework>
<TargetFramework>netcoreapp2.1</TargetFramework>

<IsPackable>false</IsPackable>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions packages/jsii-pacmak/lib/targets/dotnet/filegenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Expand Down