Skip to content

Fix config source gen binding value types to null configuration values #107654

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.

Sign up for GitHub

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

Merged
tarekgh merged 2 commits into from
Sep 11, 2024
Merged

Fix config source gen binding value types to null configuration values #107654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
859b715
Fix config source gen binding value types to null configuration values
tarekgh Sep 10, 2024
971f649
Address the feedback
tarekgh Sep 10, 2024
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
4 changes: 3 additions & 1 deletion src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Emitter/CoreBindingHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,9 @@ private void EmitBindingLogic(
}
else
{
EmitStartBlock($"if ({sectionValueExpr} is string {nonNull_StringValue_Identifier})");
// In case of calling parsing methods, check the section value first for null or empty before calling parse.
string extraCondition = typeKind == StringParsableTypeKind.AssignFromSectionValue ? "" : $" && !string.IsNullOrEmpty({nonNull_StringValue_Identifier})";
EmitStartBlock($"if ({sectionValueExpr} is string {nonNull_StringValue_Identifier}{extraCondition})");
writeOnSuccess?.Invoke(parsedValueExpr);
EmitEndBlock();
}
Expand Down
20 changes: 20 additions & 0 deletions ...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1111,5 +1111,25 @@ private IEnumerable<KeyValuePair<string, string>> enumerate()
IEnumerator IEnumerable.GetEnumerator() => enumerate().GetEnumerator();
}

public class ParsableValuesClass
{
public int? IntValue { get; set; }
public double? DoubleValue { get; set; }
public bool? BoolValue { get; set; }
public decimal? DecimalValue { get; set; }
public float? FloatValue { get; set; }
public long? LongValue { get; set; }
public short? ShortValue { get; set; }
public byte? ByteValue { get; set; }
public sbyte? SByteValue { get; set; }
public uint? UIntValue { get; set; }
public ushort? UShortValue { get; set; }
public ulong? ULongValue { get; set; }
public DateTime? DateTimeValue { get; set; }
public DateTimeOffset? DateTimeOffsetValue { get; set; }
public TimeSpan? TimeSpanValue { get; set; }
public Guid? GuidValue { get; set; }
public StringComparison? StringComparisonValue { get; set; }
}
}
}
82 changes: 82 additions & 0 deletions ...raries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,88 @@ public void CanBindOnParametersAndProperties_RecordWithArrayConstructorParameter
Assert.Equal(new string[] { "a", "b", "c" }, options.Array);
}


public static IEnumerable<object[]> Configuration_TestData()
{
yield return new object[]
{
new ConfigurationBuilder()
.AddJsonStream(new System.IO.MemoryStream(System.Text.Encoding.UTF8.GetBytes(@"{
""IntValue"" : """",
""DoubleValue"" : """",
""BoolValue"" : """",
""DecimalValue"" : """",
""FloatValue"" : """",
""LongValue"" : """",
""ShortValue"" : """",
""ByteValue"" : """",
""SByteValue"" : """",
""UIntValue"" : """",
""UShortValue"" : """",
""ULongValue"" : """",
""DateTimeValue"" : """",
""DateTimeOffsetValue"" : """",
""TimeSpanValue"" : """",
""GuidValue"" : """",
""StringComparisonValue"" : """"
}"
))).Build()
};

yield return new object[]
{
new ConfigurationBuilder().AddInMemoryCollection(
new Dictionary<string, string>
{
{ "IntValue", null },
{ "DoubleValue", null },
{ "BoolValue", null },
{ "DecimalValue", null },
{ "FloatValue", null },
{ "LongValue", null },
{ "ShortValue", null },
{ "ByteValue", null },
{ "SByteValue", null },
{ "UIntValue", null },
{ "UShortValue", null },
{ "ULongValue", null },
{ "DateTimeValue", null },
{ "DateTimeOffsetValue", null },
{ "TimeSpanValue", null },
{ "GuidValue", null },
{ "StringComparisonValue", null },
}).Build()
};
}

/// <summary>
/// Test binding a value parsable types to null configuration values.
/// The test ensure the binding will succeed without exceptions and the value will be null (not set).
/// </summary>
[Theory]
[MemberData(nameof(Configuration_TestData))]
public void BindToKnownParsableTypesWithNullValueTest(IConfiguration configuration)
{
ParsableValuesClass instance = configuration.Get<ParsableValuesClass>();
Assert.Null(instance.IntValue);
Assert.Null(instance.DoubleValue);
Assert.Null(instance.BoolValue);
Assert.Null(instance.DecimalValue);
Assert.Null(instance.FloatValue);
Assert.Null(instance.LongValue);
Assert.Null(instance.ShortValue);
Assert.Null(instance.ByteValue);
Assert.Null(instance.SByteValue);
Assert.Null(instance.UIntValue);
Assert.Null(instance.UShortValue);
Assert.Null(instance.ULongValue);
Assert.Null(instance.DateTimeValue);
Assert.Null(instance.DateTimeOffsetValue);
Assert.Null(instance.TimeSpanValue);
Assert.Null(instance.GuidValue);
Assert.Null(instance.StringComparisonValue);
}

/// <summary>
/// Test binding to recursive types using Dictionary or Collections.
/// This ensure no stack overflow will occur during the compilation through the source gen or at runtime.
Expand Down
4 changes: 2 additions & 2 deletions ...ts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -143,7 +143,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...GenerationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...s/Baselines/net462/ConfigurationBinder/Version1/Bind_Instance_BinderOptions.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...rationTests/Baselines/net462/ConfigurationBinder/Version1/Bind_Key_Instance.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
8 changes: 4 additions & 4 deletions ...sts/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -102,7 +102,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration

foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
temp2.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -141,7 +141,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value4)
if (configuration["MyInt"] is string value4 && !string.IsNullOrEmpty(value4))
{
instance.MyInt = ParseInt(value4, configuration.GetSection("MyInt").Path);
}
Expand Down Expand Up @@ -191,7 +191,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);

if (configuration["MyInt"] is string value14)
if (configuration["MyInt"] is string value14 && !string.IsNullOrEmpty(value14))
{
instance.MyInt = ParseInt(value14, configuration.GetSection("MyInt").Path);
}
Expand Down
6 changes: 3 additions & 3 deletions ...ationTests/Baselines/net462/ConfigurationBinder/Version1/Get_PrimitivesOnly.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
throw new InvalidOperationException();
}
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
return ParseInt(value, section.Path);
}
Expand All @@ -90,7 +90,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
throw new InvalidOperationException();
}
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
return ParseFloat(value, section.Path);
}
Expand All @@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
throw new InvalidOperationException();
}
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
return ParseDouble(value, section.Path);
}
Expand Down
6 changes: 3 additions & 3 deletions ...s/SourceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_T.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -83,7 +83,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration

foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
temp1.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -122,7 +122,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value3)
if (configuration["MyInt"] is string value3 && !string.IsNullOrEmpty(value3))
{
instance.MyInt = ParseInt(value3, configuration.GetSection("MyInt").Path);
}
Expand Down
6 changes: 3 additions & 3 deletions ...tionTests/Baselines/net462/ConfigurationBinder/Version1/Get_T_BinderOptions.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -83,7 +83,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration

foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
temp1.Add(ParseInt(value, section.Path));
}
Expand Down Expand Up @@ -122,7 +122,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value3)
if (configuration["MyInt"] is string value3 && !string.IsNullOrEmpty(value3))
{
instance.MyInt = ParseInt(value3, configuration.GetSection("MyInt").Path);
}
Expand Down
2 changes: 1 addition & 1 deletion ...rceGenerationTests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
2 changes: 1 addition & 1 deletion ...ests/Baselines/net462/ConfigurationBinder/Version1/Get_TypeOf_BinderOptions.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
ValidateConfigurationKeys(typeof(global::Program.MyClass2), s_configKeys_ProgramMyClass2, configuration, binderOptions);

if (configuration["MyInt"] is string value1)
if (configuration["MyInt"] is string value1 && !string.IsNullOrEmpty(value1))
{
instance.MyInt = ParseInt(value1, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...eGenerationTests/Baselines/net462/OptionsBuilder/Version1/BindConfiguration.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -125,7 +125,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value2)
if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
{
instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...elines/net462/OptionsBuilder/Version1/BindConfigurationWithConfigureActions.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -125,7 +125,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value2)
if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
{
instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...tests/SourceGenerationTests/Baselines/net462/OptionsBuilder/Version1/Bind_T.generated.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
{
foreach (IConfigurationSection section in configuration.GetChildren())
{
if (section.Value is string value)
if (section.Value is string value && !string.IsNullOrEmpty(value))
{
instance.Add(ParseInt(value, section.Path));
}
Expand All @@ -131,7 +131,7 @@ namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration
}
}

if (configuration["MyInt"] is string value2)
if (configuration["MyInt"] is string value2 && !string.IsNullOrEmpty(value2))
{
instance.MyInt = ParseInt(value2, configuration.GetSection("MyInt").Path);
}
Expand Down
Loading