Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0] Fix Configuration to ensure calling the property setters. #80562

Merged
merged 3 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
<EnableDefaultItems>true</EnableDefaultItems>
<IsPackable>true</IsPackable>
<EnableAOTAnalyzer>true</EnableAOTAnalyzer>
<ServicingVersion>2</ServicingVersion>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<ServicingVersion>3</ServicingVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ViktorHofer @carlossanlop @ericstj could you please help review this package authoring change in this file? Just to ensure I am doing the right thing here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<PackageDescription>Functionality to bind an object to data in configuration providers for Microsoft.Extensions.Configuration.</PackageDescription>
</PropertyGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<OptionWithCollectionProperties>();

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<string> blacklist = new HashSet<string>();

public ICollection<string> Blacklist
{
get => this.blacklist;
set
{
this.blacklist = value ?? new HashSet<string>();
this.ParsedBlacklist = this.blacklist.Select(b => b).ToList();
}
}

public int HttpStatusCode { get; set; } = 0;

// ParsedBlacklist initialized using the setter of Blacklist.
public ICollection<string> ParsedBlacklist { get; private set; } = new HashSet<string>();

// 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
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@
Link="Common\ConfigurationProviderExtensions.cs" />
<Compile Include="$(LibrariesProjectRoot)Microsoft.Extensions.Configuration\tests\Common\TestStreamHelpers.cs"
Link="Common\TestStreamHelpers.cs" />
<Compile Include="$(CoreLibSharedDir)System\Runtime\CompilerServices\IsExternalInit.cs" Link="Common\System\Runtime\CompilerServices\IsExternalInit.cs" />
<Compile Include="$(CoreLibSharedDir)System\Runtime\CompilerServices\IsExternalInit.cs" Link="Common\System\Runtime\CompilerServices\IsExternalInit.cs" />

<TrimmerRootDescriptor Include="$(MSBuildThisFileDirectory)ILLink.Descriptors.xml" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Configuration\src\Microsoft.Extensions.Configuration.csproj" SkipUseReferenceAssembly="true" />
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.Configuration.Json\src\Microsoft.Extensions.Configuration.Json.csproj" SkipUseReferenceAssembly="true" />
<ProjectReference Include="$(LibrariesProjectRoot)Microsoft.Extensions.FileProviders.Abstractions\src\Microsoft.Extensions.FileProviders.Abstractions.csproj" SkipUseReferenceAssembly="true" />
<ProjectReference Include="..\src\Microsoft.Extensions.Configuration.Binder.csproj" SkipUseReferenceAssembly="true" />
</ItemGroup>
Expand Down