diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index bb86976eddfad6..fdc77d446165d8 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -299,43 +299,45 @@ private static void BindInstance( if (config != null && config.GetChildren().Any()) { - // for arrays, collections, and read-only list-like interfaces, we concatenate on to what is already there + // for arrays, collections, and read-only list-like interfaces, we concatenate on to what is already there, if we can if (type.IsArray || IsArrayCompatibleInterface(type)) { if (!bindingPoint.IsReadOnly) { bindingPoint.SetValue(BindArray(type, (IEnumerable?)bindingPoint.Value, config, options)); + return; + } + + // for getter-only collection properties that we can't add to, nothing more we can do + if (type.IsArray || IsImmutableArrayCompatibleInterface(type)) + { + return; } - return; } - // for sets and read-only set interfaces, we clone what's there into a new collection. - if (TypeIsASetInterface(type)) + // for sets and read-only set interfaces, we clone what's there into a new collection, if we can + if (TypeIsASetInterface(type) && !bindingPoint.IsReadOnly) { - if (!bindingPoint.IsReadOnly) + object? newValue = BindSet(type, (IEnumerable?)bindingPoint.Value, config, options); + if (newValue != null) { - object? newValue = BindSet(type, (IEnumerable?)bindingPoint.Value, config, options); - if (newValue != null) - { - bindingPoint.SetValue(newValue); - } + bindingPoint.SetValue(newValue); } + return; } // For other mutable interfaces like ICollection<>, IDictionary<,> and ISet<>, we prefer copying values and setting them // on a new instance of the interface over populating the existing instance implementing the interface. // This has already been done, so there's not need to check again. - if (TypeIsADictionaryInterface(type)) + if (TypeIsADictionaryInterface(type) && !bindingPoint.IsReadOnly) { - if (!bindingPoint.IsReadOnly) + object? newValue = BindDictionaryInterface(bindingPoint.Value, type, config, options); + if (newValue != null) { - object? newValue = BindDictionaryInterface(bindingPoint.Value, type, config, options); - if (newValue != null) - { - bindingPoint.SetValue(newValue); - } + bindingPoint.SetValue(newValue); } + return; } @@ -848,6 +850,16 @@ private static bool IsArrayCompatibleInterface(Type type) || genericTypeDefinition == typeof(IReadOnlyList<>); } + private static bool IsImmutableArrayCompatibleInterface(Type type) + { + if (!type.IsInterface || !type.IsConstructedGenericType) { return false; } + + Type genericTypeDefinition = type.GetGenericTypeDefinition(); + return genericTypeDefinition == typeof(IEnumerable<>) + || genericTypeDefinition == typeof(IReadOnlyCollection<>) + || genericTypeDefinition == typeof(IReadOnlyList<>); + } + private static bool TypeIsASetInterface(Type type) { if (!type.IsInterface || !type.IsConstructedGenericType) { return false; } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index bfddf6b6400269..909089842d07ee 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -72,6 +72,8 @@ public string ReadOnly public ISet InstantiatedISet { get; set; } = new HashSet(); + public ISet ISetNoSetter { get; } = new HashSet(); + public HashSet InstantiatedHashSetWithSomeValues { get; set; } = new HashSet(new[] {"existing1", "existing2"}); @@ -668,6 +670,27 @@ public void CanBindNonInstantiatedISet() Assert.Equal("Yo2", options.NonInstantiatedISet.ElementAt(1)); } + [Fact] + public void CanBindISetNoSetter() + { + var dic = new Dictionary + { + {"ISetNoSetter:0", "Yo1"}, + {"ISetNoSetter:1", "Yo2"}, + {"ISetNoSetter:2", "Yo2"}, + }; + var configurationBuilder = new ConfigurationBuilder(); + configurationBuilder.AddInMemoryCollection(dic); + + var config = configurationBuilder.Build(); + + var options = config.Get()!; + + Assert.Equal(2, options.ISetNoSetter.Count); + Assert.Equal("Yo1", options.ISetNoSetter.ElementAt(0)); + Assert.Equal("Yo2", options.ISetNoSetter.ElementAt(1)); + } + #if NETCOREAPP [Fact] public void CanBindInstantiatedIReadOnlySet() diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs index 7367d0664cf356..4f2b5911b2b7a9 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs @@ -579,7 +579,10 @@ public void AlreadyInitializedStringDictionaryBinding() { {"AlreadyInitializedStringDictionaryInterface:abc", "val_1"}, {"AlreadyInitializedStringDictionaryInterface:def", "val_2"}, - {"AlreadyInitializedStringDictionaryInterface:ghi", "val_3"} + {"AlreadyInitializedStringDictionaryInterface:ghi", "val_3"}, + + {"IDictionaryNoSetter:Key1", "Value1"}, + {"IDictionaryNoSetter:Key2", "Value2"}, }; var configurationBuilder = new ConfigurationBuilder(); @@ -596,6 +599,10 @@ public void AlreadyInitializedStringDictionaryBinding() Assert.Equal("val_1", options.AlreadyInitializedStringDictionaryInterface["abc"]); Assert.Equal("val_2", options.AlreadyInitializedStringDictionaryInterface["def"]); Assert.Equal("val_3", options.AlreadyInitializedStringDictionaryInterface["ghi"]); + + Assert.Equal(2, options.IDictionaryNoSetter.Count); + Assert.Equal("Value1", options.IDictionaryNoSetter["Key1"]); + Assert.Equal("Value2", options.IDictionaryNoSetter["Key2"]); } [Fact] @@ -1059,7 +1066,10 @@ public void CanBindInitializedIEnumerableAndTheOriginalItemsAreNotMutated() {"AlreadyInitializedIEnumerableInterface:0", "val0"}, {"AlreadyInitializedIEnumerableInterface:1", "val1"}, {"AlreadyInitializedIEnumerableInterface:2", "val2"}, - {"AlreadyInitializedIEnumerableInterface:x", "valx"} + {"AlreadyInitializedIEnumerableInterface:x", "valx"}, + + {"ICollectionNoSetter:0", "val0"}, + {"ICollectionNoSetter:1", "val1"}, }; var configurationBuilder = new ConfigurationBuilder(); @@ -1084,6 +1094,10 @@ public void CanBindInitializedIEnumerableAndTheOriginalItemsAreNotMutated() Assert.Equal(2, options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.Count); Assert.Equal("This was here too", options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.ElementAt(0)); Assert.Equal("Don't touch me!", options.ListUsedInIEnumerableFieldAndShouldNotBeTouched.ElementAt(1)); + + Assert.Equal(2, options.ICollectionNoSetter.Count); + Assert.Equal("val0", options.ICollectionNoSetter.ElementAt(0)); + Assert.Equal("val1", options.ICollectionNoSetter.ElementAt(1)); } [Fact] @@ -1424,6 +1438,8 @@ public InitializedCollectionsOptions() new CustomListIndirectlyDerivedFromIEnumerable(); public IReadOnlyDictionary AlreadyInitializedDictionary { get; set; } + + public ICollection ICollectionNoSetter { get; } = new List(); } private class CustomList : List @@ -1564,6 +1580,8 @@ public OptionsWithDictionary() public Dictionary StringDictionary { get; set; } + public IDictionary IDictionaryNoSetter { get; } = new Dictionary(); + public Dictionary ObjectDictionary { get; set; } public Dictionary> ISetDictionary { get; set; }