Change ConfigurationBinder to avoid reading properties unnecessarily. (#66723)

Co-authored-by: NStephen Halter <halter73@gmail.com>

src/libraries/Microsoft.Extensions.Configuration.Binder/src/BindingPoint.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Diagnostics;
+
+namespace Microsoft.Extensions.Configuration
+{
+    internal sealed class BindingPoint
+    {
+        private readonly Func<object?>? _initialValueProvider;
+        private object? _initialValue;
+        private object? _setValue;
+        private bool _valueSet;
+
+        public BindingPoint(object? initialValue = null, bool isReadOnly = false)
+        {
+            _initialValue = initialValue;
+            IsReadOnly = isReadOnly;
+        }
+
+        public BindingPoint(Func<object?> initialValueProvider, bool isReadOnly)
+        {
+            _initialValueProvider = initialValueProvider;
+            IsReadOnly = isReadOnly;
+        }
+
+        public bool IsReadOnly { get; }
+
+        public bool HasNewValue
+        {
+            get
+            {
+                if (IsReadOnly)
+                {
+                    return false;
+                }
+
+                if (_valueSet)
+                {
+                    return true;
+                }
+
+                // When binding mutable value types, even if we didn't explicitly set a new value
+                // We still end up editing a copy of that value and therefore should treat it as
+                // a new value that needs to be written back to the parent object.
+                return _initialValue?.GetType() is { } initialValueType
+                    && initialValueType.IsValueType
+                    // Skipping primitive value types isn't strictly necessary but avoids us needing
+                    // to update the parent object in a very common case (certainly more common than
+                    // mutable structs). We'll still do a "wasted" update for non-primitive immutable structs.
+                    && !initialValueType.IsPrimitive;
+            }
+        }
+
+        public object? Value => _valueSet ? _setValue : _initialValue ??= _initialValueProvider?.Invoke();
+
+        public void SetValue(object? newValue)
+        {
+            Debug.Assert(!IsReadOnly);
+            Debug.Assert(!_valueSet);
+            _setValue = newValue;
+            _valueSet = true;
+        }
+
+        public void TrySetValue(object? newValue)
+        {
+            if (!IsReadOnly)
+            {
+                SetValue(newValue);
+            }
+        }
+    }
+}

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs

@@ -1043,6 +1043,82 @@ public void ExceptionWhenTryingToBindToByteArray()
                 exception.Message);
         }
 
+        [Fact]
+        public void DoesNotReadPropertiesUnnecessarily()
+        {
+            ConfigurationBuilder configurationBuilder = new();
+            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
+            {
+                { nameof(ClassWithReadOnlyPropertyThatThrows.Safe), "value" },
+                { nameof(ClassWithReadOnlyPropertyThatThrows.StringThrows), "value" },
+                { $"{nameof(ClassWithReadOnlyPropertyThatThrows.EnumerableThrows)}:0", "0" },
+            });
+            IConfiguration config = configurationBuilder.Build();
+
+            ClassWithReadOnlyPropertyThatThrows bound = config.Get<ClassWithReadOnlyPropertyThatThrows>();
+            Assert.Equal("value", bound.Safe);
+        }
+
+        /// <summary>
+        /// Binding to mutable structs is important to support properties
+        /// like JsonConsoleFormatterOptions.JsonWriterOptions.
+        /// </summary>
+        [Fact]
+        public void CanBindNestedStructProperties()
+        {
+            ConfigurationBuilder configurationBuilder = new();
+            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
+            {
+                { "ReadWriteNestedStruct:String", "s" },
+                { "ReadWriteNestedStruct:DeeplyNested:Int32", "100" },
+                { "ReadWriteNestedStruct:DeeplyNested:Boolean", "true" },
+            });
+            IConfiguration config = configurationBuilder.Build();
+
+            StructWithNestedStructs bound = config.Get<StructWithNestedStructs>();
+            Assert.Equal("s", bound.ReadWriteNestedStruct.String);
+            Assert.Equal(100, bound.ReadWriteNestedStruct.DeeplyNested.Int32);
+            Assert.True(bound.ReadWriteNestedStruct.DeeplyNested.Boolean);
+        }
+
+        [Fact]
+        public void IgnoresReadOnlyNestedStructProperties()
+        {
+            ConfigurationBuilder configurationBuilder = new();
+            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
+            {
+                { "ReadOnlyNestedStruct:String", "s" },
+                { "ReadOnlyNestedStruct:DeeplyNested:Int32", "100" },
+                { "ReadOnlyNestedStruct:DeeplyNested:Boolean", "true" },
+            });
+            IConfiguration config = configurationBuilder.Build();
+
+            StructWithNestedStructs bound = config.Get<StructWithNestedStructs>();
+            Assert.Null(bound.ReadOnlyNestedStruct.String);
+            Assert.Equal(0, bound.ReadWriteNestedStruct.DeeplyNested.Int32);
+            Assert.False(bound.ReadWriteNestedStruct.DeeplyNested.Boolean);
+        }
+
+        [Fact]
+        public void CanBindNullableNestedStructProperties()
+        {
+            ConfigurationBuilder configurationBuilder = new();
+            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
+            {
+                { "NullableNestedStruct:String", "s" },
+                { "NullableNestedStruct:DeeplyNested:Int32", "100" },
+                { "NullableNestedStruct:DeeplyNested:Boolean", "true" },
+            });
+            IConfiguration config = configurationBuilder.Build();
+
+            StructWithNestedStructs bound = config.Get<StructWithNestedStructs>();
+            Assert.NotNull(bound.NullableNestedStruct);
+            Assert.Equal("s", bound.NullableNestedStruct.Value.String);
+            Assert.Equal(100, bound.NullableNestedStruct.Value.DeeplyNested.Int32);
+            Assert.True(bound.NullableNestedStruct.Value.DeeplyNested.Boolean);
+        }
+
+
         private interface ISomeInterface
         {
         }
@@ -1084,5 +1160,35 @@ private class TestOptions
 
             public NestedOptions1 NestedOptionsProperty { get; set; }
         }
+
+        private class ClassWithReadOnlyPropertyThatThrows
+        {
+            public string StringThrows => throw new InvalidOperationException(nameof(StringThrows));
+
+            public IEnumerable<int> EnumerableThrows => throw new InvalidOperationException(nameof(EnumerableThrows));
+
+            public string Safe { get; set; }
+        }
+
+        private struct StructWithNestedStructs
+        {
+            public Nested ReadWriteNestedStruct { get; set; }
+
+            public Nested ReadOnlyNestedStruct { get; }
+
+            public Nested? NullableNestedStruct { get; set; }
+
+            public struct Nested
+            {
+                public string String { get; set; }
+                public DeeplyNested DeeplyNested { get; set; }
+            }
+
+            public struct DeeplyNested
+            {
+                public int Int32 { get; set; }
+                public bool Boolean { get; set; }
+            }
+        }
     }
 }

src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationCollectionBindingTests.cs

@@ -931,6 +931,25 @@ public void JaggedArrayBinding()
             Assert.Equal("12", options.JaggedArray[1][2]);
         }
 
+        [Fact]
+        public void ReadOnlyArrayIsIgnored()
+        {
+            var input = new Dictionary<string, string>
+            {
+                {"ReadOnlyArray:0", "10"},
+                {"ReadOnlyArray:1", "20"},
+                {"ReadOnlyArray:2", "30"},
+            };
+
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(input);
+            var config = configurationBuilder.Build();
+            var options = new OptionsWithArrays();
+            config.Bind(options);
+
+            Assert.Equal(new OptionsWithArrays().ReadOnlyArray, options.ReadOnlyArray);
+        }
+
         [Fact]
         public void CanBindUninitializedIEnumerable()
         {
@@ -1186,6 +1205,51 @@ public void CanBindUninitializedIReadOnlyDictionary()
             Assert.Equal("val_3", options.IReadOnlyDictionary["ghi"]);
         }
 
+        /// <summary>
+        /// Replicates scenario from https://github.com/dotnet/runtime/issues/65710
+        /// </summary>
+        [Fact]
+        public void CanBindWithInterdependentProperties()
+        {
+            var input = new Dictionary<string, string>
+            {
+                {"ConfigValues:0", "5"},
+                {"ConfigValues:1", "50"},
+            };
+
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(input);
+            var config = configurationBuilder.Build();
+
+            var options = new OptionsWithInterdependentProperties();
+            config.Bind(options);
+
+            Assert.Equal(new[] { 5, 50 }, options.ConfigValues);
+            Assert.Equal(new[] { 50 }, options.FilteredConfigValues);
+        }
+
+        /// <summary>
+        /// Replicates scenario from https://github.com/dotnet/runtime/issues/63479
+        /// </summary>
+        [Fact]
+        public void TestCanBindListPropertyWithoutSetter()
+        {
+            var input = new Dictionary<string, string>
+            {
+                {"ListPropertyWithoutSetter:0", "a"},
+                {"ListPropertyWithoutSetter:1", "b"},
+            };
+
+            var configurationBuilder = new ConfigurationBuilder();
+            configurationBuilder.AddInMemoryCollection(input);
+            var config = configurationBuilder.Build();
+
+            var options = new OptionsWithLists();
+            config.Bind(options);
+
+            Assert.Equal(new[] { "a", "b" }, options.ListPropertyWithoutSetter);
+        }
+
         private class UnintializedCollectionsOptions
         {
             public IEnumerable<string> IEnumerable { get; set; }
@@ -1293,12 +1357,14 @@ public OptionsWithArrays()
 
             public string[] StringArray { get; set; }
 
-            // this should throw becase we do not support multidimensional arrays
+            // this should throw because we do not support multidimensional arrays
             public string[,] DimensionalArray { get; set; }
 
             public string[][] JaggedArray { get; set; }
 
             public NestedOptions[] ObjectArray { get; set; }
+
+            public int[] ReadOnlyArray { get; } = new[] { 1, 2 };
         }
 
         private class OptionsWithLists
@@ -1332,6 +1398,8 @@ public OptionsWithLists()
             public List<NestedOptions> ObjectList { get; set; }
 
             public IList<string> AlreadyInitializedListInterface { get; set; }
+
+            public List<string> ListPropertyWithoutSetter { get; } = new();
         }
 
         private class OptionsWithDictionary
@@ -1360,5 +1428,11 @@ public OptionsWithDictionary()
 
             public IDictionary<string, string> AlreadyInitializedStringDictionaryInterface { get; set; }
         }
+
+        private class OptionsWithInterdependentProperties
+        {
+            public IEnumerable<int> FilteredConfigValues => ConfigValues.Where(p => p > 10);
+            public IEnumerable<int> ConfigValues { get; set; }
+        }
     }
 }