From 9510998bb8faefe179f7c97cbf3abc0595afecc4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 12 Jan 2023 20:39:11 -0800 Subject: [PATCH] [release/7.0] Fix Configuration to ensure calling the property setters. (#80562) * Fix Configuration to ensure calling the property setters. * Address the feedback * Package authoring Co-authored-by: Tarek Mahmoud Sayed --- .../src/ConfigurationBinder.cs | 5 +- ...oft.Extensions.Configuration.Binder.csproj | 4 +- .../tests/ConfigurationBinderTests.cs | 57 +++++++++++++++++++ ...tensions.Configuration.Binder.Tests.csproj | 3 +- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs index 9266f9325ad69..0d2fa560a4e03 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs @@ -262,7 +262,10 @@ private static void BindProperty(PropertyInfo property, object instance, IConfig config.GetSection(GetPropertyName(property)), options); - if (propertyBindingPoint.HasNewValue) + // For property binding, there are some cases when HasNewValue is not set in BindingPoint while a non-null Value inside that object can be retrieved from the property getter. + // As example, when binding a property which not having a configuration entry matching this property and the getter can initialize the Value. + // It is important to call the property setter as the setters can have a logic adjusting the Value. + if (!propertyBindingPoint.IsReadOnly && propertyBindingPoint.Value is not null) { property.SetValue(instance, propertyBindingPoint.Value); } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj index 76207aa9c3498..c8834b6b4b3d5 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/src/Microsoft.Extensions.Configuration.Binder.csproj @@ -5,8 +5,8 @@ true true true - 2 - false + 3 + true Functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration. diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs index 521501d5938de..7c2792780ea26 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/ConfigurationBinderTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.Test; using System; using System.Collections; using System.Collections.Generic; @@ -2578,6 +2580,61 @@ public void CanBindPrivatePropertiesFromBaseClass() Assert.Equal("a", test.ExposePrivatePropertyValue()); } + [Fact] + public void EnsureCallingThePropertySetter() + { + var json = @"{ + ""IPFiltering"": { + ""HttpStatusCode"": 401, + ""Blacklist"": [ ""192.168.0.10-192.168.10.20"", ""fe80::/10"" ] + } + }"; + + var configuration = new ConfigurationBuilder() + .AddJsonStream(TestStreamHelpers.StringToStream(json)) + .Build(); + + OptionWithCollectionProperties options = configuration.GetSection("IPFiltering").Get(); + + Assert.NotNull(options); + Assert.Equal(2, options.Blacklist.Count); + Assert.Equal("192.168.0.10-192.168.10.20", options.Blacklist.ElementAt(0)); + Assert.Equal("fe80::/10", options.Blacklist.ElementAt(1)); + + Assert.Equal(2, options.ParsedBlacklist.Count); // should be initialized when calling the options.Blacklist setter. + + Assert.Equal(401, options.HttpStatusCode); // exists in configuration and properly sets the property + Assert.Equal(2, options.OtherCode); // doesn't exist in configuration. the setter sets default value '2' + } + + public class OptionWithCollectionProperties + { + private int _otherCode; + private ICollection blacklist = new HashSet(); + + public ICollection Blacklist + { + get => this.blacklist; + set + { + this.blacklist = value ?? new HashSet(); + this.ParsedBlacklist = this.blacklist.Select(b => b).ToList(); + } + } + + public int HttpStatusCode { get; set; } = 0; + + // ParsedBlacklist initialized using the setter of Blacklist. + public ICollection ParsedBlacklist { get; private set; } = new HashSet(); + + // This property not having any match in the configuration. Still the setter need to be called during the binding. + public int OtherCode + { + get => _otherCode; + set => _otherCode = value == 0 ? 2 : value; + } + } + private interface ISomeInterface { } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj index a408e921ec89e..198d6cbfae86b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Microsoft.Extensions.Configuration.Binder.Tests.csproj @@ -10,13 +10,14 @@ Link="Common\ConfigurationProviderExtensions.cs" /> - + +