Skip to content

Commit c87bd25

Browse files
ruborubo
authored
Fix empty value handling of configuration options (#8010)
1 parent a4566f7 commit c87bd25

File tree

3 files changed

+114
-30
lines changed

3 files changed

+114
-30
lines changed

‎src/Nethermind/Nethermind.Config.Test/ConfigProvider_FindIncorrectSettings_Tests.cs

+105-26
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,42 @@
22
// SPDX-License-Identifier: LGPL-3.0-only
33

44
using System;
5+
using System.Collections;
56
using System.Collections.Generic;
67
using NSubstitute;
78
using NUnit.Framework;
89

910
namespace Nethermind.Config.Test;
1011

11-
[TestFixture]
12+
[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
1213
[Parallelizable(ParallelScope.All)]
1314
public class ConfigProvider_FindIncorrectSettings_Tests
1415
{
16+
private IEnvironment _env;
17+
18+
[SetUp]
19+
public void Initialize()
20+
{
21+
_env = Substitute.For<IEnvironment>();
22+
_env.GetEnvironmentVariable(Arg.Any<string>())
23+
.Returns(call =>
24+
{
25+
IDictionary vars = _env.GetEnvironmentVariables();
26+
var key = call.Arg<string>();
27+
28+
return vars.Contains(key) ? vars[key] : null;
29+
});
30+
}
31+
1532
[Test]
1633
public void CorrectSettingNames_CaseInsensitive()
1734
{
1835
JsonConfigSource? jsonSource = new("SampleJson/CorrectSettingNames.json");
1936

20-
IEnvironment? env = Substitute.For<IEnvironment>();
21-
env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() { { "NETHERMIND_NETWORKCONFIG_MAXCANDIDATEPEERCOUNT", "500" } });
22-
EnvConfigSource? envSource = new(env);
37+
Dictionary<string, string> envVars = new() { { "NETHERMIND_NETWORKCONFIG_MAXCANDIDATEPEERCOUNT", "500" } };
38+
39+
_env.GetEnvironmentVariables().Returns(envVars);
40+
EnvConfigSource? envSource = new(_env);
2341

2442
ArgsConfigSource? argsSource = new(new Dictionary<string, string>() {
2543
{ "DiscoveryConfig.BucketSize", "10" },
@@ -33,20 +51,19 @@ public void CorrectSettingNames_CaseInsensitive()
3351
configProvider.Initialize();
3452
(_, IList<(IConfigSource Source, string Category, string Name)> Errors) = configProvider.FindIncorrectSettings();
3553

36-
Assert.That(Errors.Count, Is.EqualTo(0));
54+
Assert.That(Errors, Is.Empty);
3755
}
3856

3957
[Test]
4058
public void NoCategorySettings()
4159
{
42-
IEnvironment? env = Substitute.For<IEnvironment>();
43-
env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
60+
_env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
4461
{ "NETHERMIND_CLI_SWITCH_LOCAL", "http://localhost:80" },
4562
{ "NETHERMIND_CONFIG", "test2.json" },
4663
{ "NETHERMIND_XYZ", "xyz" }, // not existing, should get error
4764
{ "QWER", "qwerty" } // not Nethermind setting, no error
4865
});
49-
EnvConfigSource? envSource = new(env);
66+
EnvConfigSource? envSource = new(_env);
5067

5168
ConfigProvider? configProvider = new();
5269
configProvider.AddSource(envSource);
@@ -55,22 +72,23 @@ public void NoCategorySettings()
5572

5673
(string ErrorMsg, IList<(IConfigSource Source, string Category, string Name)> Errors) = configProvider.FindIncorrectSettings();
5774

58-
Assert.That(Errors.Count, Is.EqualTo(1));
59-
Assert.That(Errors[0].Name, Is.EqualTo("XYZ"));
60-
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:|Name:XYZ"));
61-
75+
Assert.That(Errors, Has.Count.EqualTo(1));
76+
Assert.Multiple(() =>
77+
{
78+
Assert.That(Errors[0].Name, Is.EqualTo("XYZ"));
79+
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:|Name:XYZ"));
80+
});
6281
}
6382

6483
[Test]
6584
public void SettingWithTypos()
6685
{
6786
JsonConfigSource? jsonSource = new("SampleJson/ConfigWithTypos.json");
6887

69-
IEnvironment? env = Substitute.For<IEnvironment>();
70-
env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
88+
_env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
7189
{ "NETHERMIND_NETWORKCONFIG_MAXCANDIDATEPERCOUNT", "500" } // incorrect, should be NETHERMIND_NETWORKCONFIG_MAXCANDIDATEPEERCOUNT
7290
});
73-
EnvConfigSource? envSource = new(env);
91+
EnvConfigSource? envSource = new(_env);
7492

7593
ConfigProvider? configProvider = new();
7694
configProvider.AddSource(jsonSource);
@@ -80,21 +98,23 @@ public void SettingWithTypos()
8098

8199
(string ErrorMsg, IList<(IConfigSource Source, string Category, string Name)> Errors) = configProvider.FindIncorrectSettings();
82100

83-
Assert.That(Errors.Count, Is.EqualTo(3));
84-
Assert.That(Errors[0].Name, Is.EqualTo("Concurrenc"));
85-
Assert.That(Errors[1].Category, Is.EqualTo("BlomConfig"));
86-
Assert.That(Errors[2].Name, Is.EqualTo("MAXCANDIDATEPERCOUNT"));
87-
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:JsonConfigFile|Category:DiscoveRyConfig|Name:Concurrenc{Environment.NewLine}ConfigType:JsonConfigFile|Category:BlomConfig|Name:IndexLevelBucketSizes{Environment.NewLine}ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:NETWORKCONFIG|Name:MAXCANDIDATEPERCOUNT"));
101+
Assert.That(Errors, Has.Count.EqualTo(3));
102+
Assert.Multiple(() =>
103+
{
104+
Assert.That(Errors[0].Name, Is.EqualTo("Concurrenc"));
105+
Assert.That(Errors[1].Category, Is.EqualTo("BlomConfig"));
106+
Assert.That(Errors[2].Name, Is.EqualTo("MAXCANDIDATEPERCOUNT"));
107+
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:JsonConfigFile|Category:DiscoveRyConfig|Name:Concurrenc{Environment.NewLine}ConfigType:JsonConfigFile|Category:BlomConfig|Name:IndexLevelBucketSizes{Environment.NewLine}ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:NETWORKCONFIG|Name:MAXCANDIDATEPERCOUNT"));
108+
});
88109
}
89110

90111
[Test]
91112
public void IncorrectFormat()
92113
{
93-
IEnvironment? env = Substitute.For<IEnvironment>();
94-
env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
114+
_env.GetEnvironmentVariables().Returns(new Dictionary<string, string>() {
95115
{ "NETHERMIND_NETWORKCONFIGMAXCANDIDATEPEERCOUNT", "500" } // incorrect, should be NETHERMIND_NETWORKCONFIG_MAXCANDIDATEPEERCOUNT
96116
});
97-
EnvConfigSource? envSource = new(env);
117+
EnvConfigSource? envSource = new(_env);
98118

99119
ConfigProvider? configProvider = new();
100120
configProvider.AddSource(envSource);
@@ -103,9 +123,68 @@ public void IncorrectFormat()
103123

104124
(string ErrorMsg, IList<(IConfigSource Source, string Category, string Name)> Errors) = configProvider.FindIncorrectSettings();
105125

106-
Assert.That(Errors.Count, Is.EqualTo(1));
107-
Assert.That(Errors[0].Name, Is.EqualTo("NETWORKCONFIGMAXCANDIDATEPEERCOUNT"));
108-
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:|Name:NETWORKCONFIGMAXCANDIDATEPEERCOUNT"));
126+
Assert.That(Errors, Has.Count.EqualTo(1));
127+
Assert.Multiple(() =>
128+
{
129+
Assert.That(Errors[0].Name, Is.EqualTo("NETWORKCONFIGMAXCANDIDATEPEERCOUNT"));
130+
Assert.That(ErrorMsg, Is.EqualTo($"ConfigType:EnvironmentVariable(NETHERMIND_*)|Category:|Name:NETWORKCONFIGMAXCANDIDATEPEERCOUNT"));
131+
});
132+
}
133+
134+
[Test]
135+
public void Should_keep_blank_string_values()
136+
{
137+
Dictionary<string, string> envVars = new()
138+
{
139+
{ "NETHERMIND_BLOCKSCONFIG_EXTRADATA", "" }
140+
};
141+
142+
_env.GetEnvironmentVariables().Returns(envVars);
143+
EnvConfigSource? envSource = new(_env);
144+
145+
ConfigProvider? configProvider = new();
146+
configProvider.AddSource(envSource);
147+
148+
(bool isSet, object value) = envSource.GetValue(typeof(string), "BlocksConfig", "ExtraData");
149+
150+
Assert.Multiple(() =>
151+
{
152+
Assert.That(isSet, Is.True);
153+
Assert.That(value, Is.Empty);
154+
});
109155
}
110156

157+
[Test]
158+
public void Should_ignore_blank_nonstring_values()
159+
{
160+
Dictionary<string, string> envVars = new()
161+
{
162+
{ "NETHERMIND_BLOOMCONFIG_INDEX", " " },
163+
{ "NETHERMIND_BLOOMCONFIG_MIGRATION", "" }
164+
};
165+
166+
_env.GetEnvironmentVariables().Returns(envVars);
167+
EnvConfigSource? envSource = new(_env);
168+
169+
ConfigProvider? configProvider = new();
170+
configProvider.AddSource(envSource);
171+
172+
Assert.DoesNotThrow(configProvider.Initialize);
173+
174+
(bool isSet, object value) = envSource.GetValue(typeof(bool), "BloomConfig", "Index");
175+
176+
Assert.Multiple(() =>
177+
{
178+
Assert.That(isSet, Is.False);
179+
Assert.That(((ValueTuple<bool, object>)value).Item2, Is.False);
180+
});
181+
182+
(isSet, value) = envSource.GetValue(typeof(bool), "BloomConfig", "Migration");
183+
184+
Assert.Multiple(() =>
185+
{
186+
Assert.That(isSet, Is.False);
187+
Assert.That(((ValueTuple<bool, object>)value).Item2, Is.False);
188+
});
189+
}
111190
}

‎src/Nethermind/Nethermind.Config/ConfigSourceHelper.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public static object ParseValue(Type valueType, string valueString, string categ
3535
//supports Arrays, e.g int[] and generic IEnumerable<T>, IList<T>
3636
var itemType = valueType.IsGenericType ? valueType.GetGenericArguments()[0] : valueType.GetElementType();
3737

38-
if (itemType == typeof(byte) && !valueString.AsSpan().TrimStart().StartsWith("["))
38+
if (itemType == typeof(byte) && !valueString.AsSpan().TrimStart().StartsWith('['))
3939
{
4040
// hex encoded byte array
4141
value = Bytes.FromHexString(valueString.Trim());
@@ -99,13 +99,13 @@ public static object ParseValue(Type valueType, string valueString, string categ
9999
}
100100

101101
private static bool IsNullString(string valueString) =>
102-
string.IsNullOrEmpty(valueString) || valueString.Equals("null", StringComparison.OrdinalIgnoreCase);
102+
valueString?.Equals("null", StringComparison.OrdinalIgnoreCase) ?? true;
103103

104104
public static object GetDefault(Type type) => type.IsValueType ? (false, Activator.CreateInstance(type)) : (false, null);
105105

106106
private static bool TryFromHex(Type type, string itemValue, out object value)
107107
{
108-
if (!itemValue.StartsWith("0x"))
108+
if (!itemValue.StartsWith("0x", StringComparison.Ordinal))
109109
{
110110
value = null;
111111
return false;

‎src/Nethermind/Nethermind.Config/EnvConfigSource.cs

+6-1
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,19 @@ public EnvConfigSource(IEnvironment environmentWrapper)
2323
public (bool IsSet, object Value) GetValue(Type type, string category, string name)
2424
{
2525
(bool isSet, string value) = GetRawValue(category, name);
26+
27+
// Unset blank values for non-string types
28+
if (type != typeof(string) && string.IsNullOrWhiteSpace(value))
29+
isSet = false;
30+
2631
return (isSet, isSet ? ConfigSourceHelper.ParseValue(type, value, category, name) : ConfigSourceHelper.GetDefault(type));
2732
}
2833

2934
public (bool IsSet, string Value) GetRawValue(string category, string name)
3035
{
3136
var variableName = string.IsNullOrEmpty(category) ? $"NETHERMIND_{name.ToUpperInvariant()}" : $"NETHERMIND_{category.ToUpperInvariant()}_{name.ToUpperInvariant()}";
3237
var variableValueString = _environmentWrapper.GetEnvironmentVariable(variableName);
33-
return string.IsNullOrWhiteSpace(variableValueString) ? (false, null) : (true, variableValueString);
38+
return (variableValueString is not null, variableValueString);
3439
}
3540

3641
public IEnumerable<(string Category, string Name)> GetConfigKeys()

0 commit comments

Comments
 (0)