From 0769bb8e18a51858fd69a4af2da053a38cba8c3c Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Mon, 17 Jul 2023 13:36:06 -0700 Subject: [PATCH 1/2] Fix binding logic for dictionaries with complex elements --- .../gen/Helpers/Emitter/CoreBindingHelper.cs | 2 +- .../ConfigurationBinderTests.TestClasses.cs | 59 +++++++++++++++++ .../tests/Common/ConfigurationBinderTests.cs | 65 +++++++++++++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Emitter/CoreBindingHelper.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Emitter/CoreBindingHelper.cs index bdf56060b82dbe..dd13ee8e2a3582 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Emitter/CoreBindingHelper.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/Emitter/CoreBindingHelper.cs @@ -731,7 +731,7 @@ void Emit_BindAndAddLogic_ForElement(string parsedKeyExpr) """); } - EmitBindCoreCall(elementType, $"{Identifier.element}!", Identifier.section, InitializationKind.None); + EmitBindCoreCall(elementType, Identifier.element, Identifier.section, InitializationKind.None); _writer.WriteLine($"{objIdentifier}[{parsedKeyExpr}] = {Identifier.element};"); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 5e4855a55d2ff3..9b65a0e479e42f 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -3,6 +3,8 @@ using System; using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using Microsoft.Extensions.Configuration; @@ -664,5 +666,62 @@ public struct StructWithParameterlessAndParameterizedCtor public int MyInt { get; } } + + [TypeConverter(typeof(GeolocationTypeConverter))] + public struct Geolocation : IEquatable +#if NETCOREAPP + , IParsable +#endif + { + public static readonly Geolocation Zero = new(0, 0); + + public Geolocation(double latitude, double longitude) + { + Latitude = latitude; + Longitude = longitude; + } + + public double Latitude { get; set; } + + public double Longitude { get; set; } + + private sealed class GeolocationTypeConverter : TypeConverter + { + public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) + { + if (sourceType == typeof(string) || sourceType == typeof(Geolocation)) + { + return true; + } + + return base.CanConvertFrom(context, sourceType); + } + + public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) + { + if (value is string s) + { + return Parse(s, culture); + } + else if (value is Geolocation geolocation) + { + return geolocation; + } + + return base.ConvertFrom(context, culture, value); + } + } + + public bool Equals(Geolocation other) => Latitude == other.Latitude && Longitude == other.Longitude; + + public static Geolocation Parse(string s, IFormatProvider? provider) => throw new NotImplementedException(); + + public static bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, [MaybeNullWhen(false)] out Geolocation result) => throw new NotImplementedException(); + } + + public class GeolocationWrapper + { + public Geolocation Location { get; set; } + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index d9dfefecce0c76..5c00a2756545c9 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1900,5 +1900,70 @@ public void AllowsCaseInsensitiveMatch() GenericOptionsWithParamCtor obj2 = configuration.Get>(); Assert.Equal("MyString", obj2.Value); } + + [Fact] + public void ObjWith_IParsableT_And_TypeConverter() + { + var configuration = TestHelpers.GetConfigurationFromJsonString(""" + { + "Location": + { + "Latitude": 3, + "Longitude": 4, + } + } + """); + + // Neither IParsableT nor TypeConverter impl are honored (https://github.com/dotnet/runtime/issues/83599). + + GeolocationWrapper obj = configuration.Get(); + ValidateGeolocation(obj.Location); + + configuration = TestHelpers.GetConfigurationFromJsonString(""" { "Geolocation": "3, 4", } """); + obj = configuration.Get(); + Assert.Equal(Geolocation.Zero, obj.Location); + } + + [Fact] + public void ComplexObj_As_Dictionary_Element() + { + var configuration = TestHelpers.GetConfigurationFromJsonString(""" + { + "First": + { + "Latitude": 3, + "Longitude": 4, + } + } + """); + + Geolocation obj = configuration.Get>()["First"]; + ValidateGeolocation(obj); + + obj = configuration.Get>()["First"]; + ValidateGeolocation(obj); + } + + [Fact] + public void ComplexObj_As_Enumerable_Element() + { + var configuration = TestHelpers.GetConfigurationFromJsonString("""{ "Enumerable": [{ "Latitude": 3, "Longitude": 4 }] }""") + .GetSection("Enumerable"); + + Geolocation obj = configuration.Get>()[0]; + ValidateGeolocation(obj); + + obj = configuration.Get()[0]; + ValidateGeolocation(obj); + + obj = configuration.Get>()[0]; + ValidateGeolocation(obj); + } + + private void ValidateGeolocation(Geolocation location) + { + Assert.Equal(3, location.Latitude); + Assert.Equal(4, location.Longitude); + } } } From 6db8a9cda1dfac6a7371d8a84ce659ed7d1546da Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Tue, 18 Jul 2023 12:45:43 -0700 Subject: [PATCH 2/2] Remove interace impls not relevant to test and causing issues --- .../ConfigurationBinderTests.TestClasses.cs | 37 +++---------------- .../tests/Common/ConfigurationBinderTests.cs | 4 +- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 9b65a0e479e42f..df894d626fa611 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -668,10 +668,7 @@ public struct StructWithParameterlessAndParameterizedCtor } [TypeConverter(typeof(GeolocationTypeConverter))] - public struct Geolocation : IEquatable -#if NETCOREAPP - , IParsable -#endif + public struct Geolocation { public static readonly Geolocation Zero = new(0, 0); @@ -687,36 +684,12 @@ public Geolocation(double latitude, double longitude) private sealed class GeolocationTypeConverter : TypeConverter { - public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) - { - if (sourceType == typeof(string) || sourceType == typeof(Geolocation)) - { - return true; - } - - return base.CanConvertFrom(context, sourceType); - } + public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) => + throw new NotImplementedException(); - public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) - { - if (value is string s) - { - return Parse(s, culture); - } - else if (value is Geolocation geolocation) - { - return geolocation; - } - - return base.ConvertFrom(context, culture, value); - } + public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) => + throw new NotImplementedException(); } - - public bool Equals(Geolocation other) => Latitude == other.Latitude && Longitude == other.Longitude; - - public static Geolocation Parse(string s, IFormatProvider? provider) => throw new NotImplementedException(); - - public static bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, [MaybeNullWhen(false)] out Geolocation result) => throw new NotImplementedException(); } public class GeolocationWrapper diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs index 5c00a2756545c9..b1af371dc23eef 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs @@ -1902,7 +1902,7 @@ public void AllowsCaseInsensitiveMatch() } [Fact] - public void ObjWith_IParsableT_And_TypeConverter() + public void ObjWith_TypeConverter() { var configuration = TestHelpers.GetConfigurationFromJsonString(""" { @@ -1914,7 +1914,7 @@ public void ObjWith_IParsableT_And_TypeConverter() } """); - // Neither IParsableT nor TypeConverter impl are honored (https://github.com/dotnet/runtime/issues/83599). + // TypeConverter impl is not honored (https://github.com/dotnet/runtime/issues/83599). GeolocationWrapper obj = configuration.Get(); ValidateGeolocation(obj.Location);