From 9b9ac1bd043e829e289a455777ec989a624cc23a Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Tue, 1 Nov 2022 07:38:07 +0000 Subject: [PATCH 1/3] First pass --- .../src/BinderOptions.cs | 6 +- .../src/ConfigurationBinder.cs | 29 +++- .../src/Resources/Strings.resx | 3 + .../tests/ConfigurationBinderTests.cs | 124 ++++++++++++++++-- 4 files changed, 147 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/BinderOptions.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/BinderOptions.cs index f5ffd6eb5758f8..b4664675b15dfb 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/BinderOptions.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/BinderOptions.cs @@ -15,10 +15,10 @@ public class BinderOptions public bool BindNonPublicProperties { get; set; } /// - /// When false (the default), no exceptions are thrown when a configuration key is found for which the - /// provided model object does not have an appropriate property which matches the key's name. + /// When false (the default), no exceptions are thrown when trying to convert a value or when a configuration + /// key is found for which the provided model object does not have an appropriate property which matches the key's name. /// When true, an is thrown with a description - /// of the missing properties. + /// of the error. /// public bool ErrorOnUnknownConfiguration { get; set; } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 1d0992781f08e6..332f74ef794b31 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -620,8 +620,13 @@ private static void BindConcreteDictionary( setter.SetValue(dictionary, valueBindingPoint.Value, new object[] { key }); } } - catch + catch(Exception ex) { + if (options.ErrorOnUnknownConfiguration) + { + throw new InvalidOperationException(SR.Format(SR.Error_GeneralErrorWhenBinding, + nameof(options.ErrorOnUnknownConfiguration)), ex); + } } } } @@ -653,8 +658,14 @@ private static void BindCollection( addMethod?.Invoke(collection, new[] { itemBindingPoint.Value }); } } - catch + catch(Exception ex) { + if (options.ErrorOnUnknownConfiguration) + { + throw new InvalidOperationException(SR.Format(SR.Error_GeneralErrorWhenBinding, + nameof(options.ErrorOnUnknownConfiguration)), ex); + } + } } } @@ -702,8 +713,13 @@ private static Array BindArray(Type type, IEnumerable? source, IConfiguration co list.Add(itemBindingPoint.Value); } } - catch + catch (Exception ex) { + if (options.ErrorOnUnknownConfiguration) + { + throw new InvalidOperationException(SR.Format(SR.Error_GeneralErrorWhenBinding, + nameof(options.ErrorOnUnknownConfiguration)), ex); + } } } @@ -761,8 +777,13 @@ private static Array BindArray(Type type, IEnumerable? source, IConfiguration co addMethod.Invoke(instance, arguments); } } - catch + catch (Exception ex) { + if (options.ErrorOnUnknownConfiguration) + { + throw new InvalidOperationException(SR.Format(SR.Error_GeneralErrorWhenBinding, + nameof(options.ErrorOnUnknownConfiguration)), ex); + } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 197b325252ef91..571ea61a9a83be 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -132,6 +132,9 @@ Failed to create instance of type '{0}'. + + '{0}' was set and binding has failed due. The likely cause is an invalid configuration value. + '{0}' was set on the provided {1}, but the following properties were not found on the instance of {2}: {3} diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index a24e7d3b9a3518..13e50f514b583a 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -75,20 +75,20 @@ public string ReadOnly public ISet ISetNoSetter { get; } = new HashSet(); public HashSet InstantiatedHashSetWithSomeValues { get; set; } = - new HashSet(new[] {"existing1", "existing2"}); + new HashSet(new[] { "existing1", "existing2" }); public SortedSet InstantiatedSortedSetWithSomeValues { get; set; } = - new SortedSet(new[] {"existing1", "existing2"}); + new SortedSet(new[] { "existing1", "existing2" }); public SortedSet NonInstantiatedSortedSetWithSomeValues { get; set; } = null!; public ISet InstantiatedISetWithSomeValues { get; set; } = new HashSet(new[] { "existing1", "existing2" }); - + public ISet HashSetWithUnsupportedKey { get; set; } = new HashSet(); - public ISet UninstantiatedHashSetWithUnsupportedKey { get; set; } + public ISet UninstantiatedHashSetWithUnsupportedKey { get; set; } #if NETCOREAPP public IReadOnlySet InstantiatedIReadOnlySet { get; set; } = new HashSet(); @@ -348,7 +348,7 @@ public MutableStructWithConstructor(string randomParameter) } public string Color { get; set; } - public int Length { get; set; } + public int Length { get; set; } } public class ImmutableLengthAndColorClass @@ -505,6 +505,114 @@ public enum TestSettingsEnum Option2, } + public class WhenBindingCollectionsWeCanControlExceptionsWithTheErrorOnUnknownConfigurationProperty + { + public class MyModelContainingArray + { + public TestSettingsEnum[] Enums { get; set; } + } + + public class MyModelContainingADictionary + { + public Dictionary Enums { get; set; } + } + + [Fact] + public void WithFlagUnset_NoExceptionIsThrownWhenFailingToParseEnumsInAnArrayAndValidItemsArePreserved() + { + var dic = new Dictionary + { + {"Section:Enums:0", "Option1"}, + {"Section:Enums:1", "Option3"}, // invalid - ignored + {"Section:Enums:2", "Option4"}, // invalid - ignored + {"Section:Enums:3", "Option2"}, + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + var configSection = config.GetSection("Section"); + + var model = configSection.Get(o => o.ErrorOnUnknownConfiguration = false); + + Assert.Equal(2, model.Enums.Length); + Assert.Equal(TestSettingsEnum.Option1, model.Enums[0]); + Assert.Equal(TestSettingsEnum.Option2, model.Enums[1]); + } + + [Fact] + public void WithFlagUnset_NoExceptionIsThrownWhenFailingToParseEnumsInADictionaryAndValidItemsArePreserved() + { + var dic = new Dictionary + { + {"Section:Enums:First", "Option1"}, + {"Section:Enums:Second", "Option3"}, // invalid - ignored + {"Section:Enums:Third", "Option4"}, // invalid - ignored + {"Section:Enums:Fourth", "Option2"}, + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + var configSection = config.GetSection("Section"); + + var model = configSection.Get(o => + o.ErrorOnUnknownConfiguration = false); + + Assert.Equal(2, model.Enums.Count); + Assert.Equal(TestSettingsEnum.Option1, model.Enums["First"]); + Assert.Equal(TestSettingsEnum.Option2, model.Enums["Fourth"]); + } + + [Fact] + public void WithFlagSet_AnExceptionIsThrownWhenFailingToParseEnumsInAnArray() + { + var dic = new Dictionary + { + {"Section:Enums:0", "Option1"}, + {"Section:Enums:1", "Option3"}, // invalid - exception thrown + {"Section:Enums:2", "Option1"}, + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + var configSection = config.GetSection("Section"); + + var exception = Assert.Throws( + () => configSection.Get(o => o.ErrorOnUnknownConfiguration = true)); + + Assert.Equal( + SR.Format(SR.Error_GeneralErrorWhenBinding, nameof(BinderOptions.ErrorOnUnknownConfiguration)), + exception.Message); + } + + [Fact] + public void WithFlagSet_AnExceptionIsThrownWhenFailingToParseEnumsInADictionary() + { + var dic = new Dictionary + { + {"Section:Enums:First", "Option1"}, + {"Section:Enums:Second", "Option3"}, // invalid - exception thrown + {"Section:Enums:Third", "Option1"}, + }; + + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + var config = configurationBuilder.Build(); + var configSection = config.GetSection("Section"); + + var exception = Assert.Throws( + () => configSection.Get(o => + o.ErrorOnUnknownConfiguration = true)); + + Assert.Equal( + SR.Format(SR.Error_GeneralErrorWhenBinding, nameof(BinderOptions.ErrorOnUnknownConfiguration)), + exception.Message); + } + + } + public record RootConfig(NestedConfig Nested); public record NestedConfig(string MyProp); @@ -804,7 +912,7 @@ public void CanBindInstantiatedDictionaryOfIReadOnlySetWithSomeExistingValues() public class Foo { public IReadOnlyDictionary Items { get; set; } = - new Dictionary {{"existing-item1", 1}, {"existing-item2", 2}}; + new Dictionary { { "existing-item1", 1 }, { "existing-item2", 2 } }; } @@ -829,7 +937,7 @@ public void CanBindInstantiatedReadOnlyDictionary2() Assert.Equal(3, options.Items["item3"]); Assert.Equal(4, options.Items["item4"]); - + } [Fact] @@ -944,7 +1052,7 @@ public void CanBindNonInstantiatedReadOnlyDictionary() Assert.Equal(3, options.NonInstantiatedReadOnlyDictionary["item3"]); Assert.Equal(4, options.NonInstantiatedReadOnlyDictionary["item4"]); } - + [Fact] public void CanBindNonInstantiatedDictionaryOfISet() From d033596d98f75c230740155a7fdac23c0c395965 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Tue, 1 Nov 2022 07:43:23 +0000 Subject: [PATCH 2/3] Update src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx --- .../src/Resources/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx index 571ea61a9a83be..a926fc42386a9b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Resources/Strings.resx @@ -133,7 +133,7 @@ Failed to create instance of type '{0}'. - '{0}' was set and binding has failed due. The likely cause is an invalid configuration value. + '{0}' was set and binding has failed. The likely cause is an invalid configuration value. '{0}' was set on the provided {1}, but the following properties were not found on the instance of {2}: {3} From fb0002d66a61d0ece07109dd4e89451dd1bf2c07 Mon Sep 17 00:00:00 2001 From: Steve Dunn Date: Thu, 3 Nov 2022 21:16:58 +0000 Subject: [PATCH 3/3] Rename test --- .../tests/ConfigurationBinderTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 13e50f514b583a..51a25429f77e83 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -505,7 +505,7 @@ public enum TestSettingsEnum Option2, } - public class WhenBindingCollectionsWeCanControlExceptionsWithTheErrorOnUnknownConfigurationProperty + public class CollectionsBindingWithErrorOnUnknownConfiguration { public class MyModelContainingArray { @@ -610,7 +610,6 @@ public void WithFlagSet_AnExceptionIsThrownWhenFailingToParseEnumsInADictionary( SR.Format(SR.Error_GeneralErrorWhenBinding, nameof(BinderOptions.ErrorOnUnknownConfiguration)), exception.Message); } - } public record RootConfig(NestedConfig Nested);