Skip to content

Commit 7f40528

Browse files
ryanofskyryanofsky
committed
Deduplicate settings merge code
Get rid of settings merging code in util/system.cpp repeated 5 places, inconsistently: - ArgsManagerHelper::GetArg - ArgsManagerHelper::GetNetBoolArg - ArgsManager::GetArgs - ArgsManager::IsArgNegated - ArgsManager::GetUnsuitableSectionOnlyArgs Having settings merging code separated from parsing simplifies parsing somewhat (for example negated values can simply be represented as false values instead of partially cleared or emply placeholder lists). Having settings merge happen one place instead of 5 makes it easier to add new settings sources and harder to introduce new inconsistencies in the way settings are merged. This commit does not change behavior in any way.
1 parent 9dcb952 commit 7f40528

File tree

3 files changed

+172
-270
lines changed

3 files changed

+172
-270
lines changed

‎src/test/util_tests.cpp‎

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include <stdint.h>
1919
#include <thread>
20+
#include <univalue.h>
2021
#include <utility>
2122
#include <vector>
2223
#ifndef WIN32
@@ -166,14 +167,12 @@ BOOST_AUTO_TEST_CASE(util_FormatISO8601Date)
166167
struct TestArgsManager : public ArgsManager
167168
{
168169
TestArgsManager() { m_network_only_args.clear(); }
169-
std::map<std::string, std::vector<std::string> >& GetOverrideArgs() { return m_override_args; }
170-
std::map<std::string, std::vector<std::string> >& GetConfigArgs() { return m_config_args; }
171170
void ReadConfigString(const std::string str_config)
172171
{
173172
std::istringstream streamConfig(str_config);
174173
{
175174
LOCK(cs_args);
176-
m_config_args.clear();
175+
m_settings.ro_config.clear();
177176
m_config_sections.clear();
178177
}
179178
std::string error;
@@ -193,6 +192,7 @@ struct TestArgsManager : public ArgsManager
193192
using ArgsManager::ReadConfigStream;
194193
using ArgsManager::cs_args;
195194
using ArgsManager::m_network;
195+
using ArgsManager::m_settings;
196196
};
197197

198198
BOOST_AUTO_TEST_CASE(util_ParseParameters)
@@ -206,28 +206,29 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
206206
const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"};
207207

208208
std::string error;
209+
LOCK(testArgs.cs_args);
209210
testArgs.SetupArgs({a, b, ccc, d});
210211
BOOST_CHECK(testArgs.ParseParameters(0, (char**)argv_test, error));
211-
BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
212+
BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty());
212213

213214
BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error));
214-
BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
215+
BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty());
215216

216217
BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error));
217218
// expectation: -ignored is ignored (program name argument),
218219
// -a, -b and -ccc end up in map, -d ignored because it is after
219220
// a non-option argument (non-GNU option parsing)
220-
BOOST_CHECK(testArgs.GetOverrideArgs().size() == 3 && testArgs.GetConfigArgs().empty());
221+
BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 3 && testArgs.m_settings.ro_config.empty());
221222
BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc")
222223
&& !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d"));
223-
BOOST_CHECK(testArgs.GetOverrideArgs().count("-a") && testArgs.GetOverrideArgs().count("-b") && testArgs.GetOverrideArgs().count("-ccc")
224-
&& !testArgs.GetOverrideArgs().count("f") && !testArgs.GetOverrideArgs().count("-d"));
225-
226-
BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].size() == 1);
227-
BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].front() == "");
228-
BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].size() == 2);
229-
BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].front() == "argument");
230-
BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].back() == "multiple");
224+
BOOST_CHECK(testArgs.m_settings.command_line_options.count("a") && testArgs.m_settings.command_line_options.count("b") && testArgs.m_settings.command_line_options.count("ccc")
225+
&& !testArgs.m_settings.command_line_options.count("f") && !testArgs.m_settings.command_line_options.count("d"));
226+
227+
BOOST_CHECK(testArgs.m_settings.command_line_options["a"].size() == 1);
228+
BOOST_CHECK(testArgs.m_settings.command_line_options["a"].front().get_str() == "");
229+
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].size() == 2);
230+
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].front().get_str() == "argument");
231+
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].back().get_str() == "multiple");
231232
BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2);
232233
}
233234

@@ -298,6 +299,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
298299
const char *argv_test[] = {
299300
"ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
300301
std::string error;
302+
LOCK(testArgs.cs_args);
301303
testArgs.SetupArgs({a, b, c, d, e, f});
302304
BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error));
303305

@@ -306,8 +308,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
306308
BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt);
307309

308310
// Nothing else should be in the map
309-
BOOST_CHECK(testArgs.GetOverrideArgs().size() == 6 &&
310-
testArgs.GetConfigArgs().empty());
311+
BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 6 &&
312+
testArgs.m_settings.ro_config.empty());
311313

312314
// The -no prefix should get stripped on the way in.
313315
BOOST_CHECK(!testArgs.IsArgSet("-nob"));
@@ -403,6 +405,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
403405
"iii=2\n";
404406

405407
TestArgsManager test_args;
408+
LOCK(test_args.cs_args);
406409
const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL);
407410
const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL);
408411
const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_STRING);
@@ -419,22 +422,25 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
419422
// expectation: a, b, ccc, d, fff, ggg, h, i end up in map
420423
// so do sec1.ccc, sec1.d, sec1.h, sec2.ccc, sec2.iii
421424

422-
BOOST_CHECK(test_args.GetOverrideArgs().empty());
423-
BOOST_CHECK(test_args.GetConfigArgs().size() == 13);
424-
425-
BOOST_CHECK(test_args.GetConfigArgs().count("-a")
426-
&& test_args.GetConfigArgs().count("-b")
427-
&& test_args.GetConfigArgs().count("-ccc")
428-
&& test_args.GetConfigArgs().count("-d")
429-
&& test_args.GetConfigArgs().count("-fff")
430-
&& test_args.GetConfigArgs().count("-ggg")
431-
&& test_args.GetConfigArgs().count("-h")
432-
&& test_args.GetConfigArgs().count("-i")
425+
BOOST_CHECK(test_args.m_settings.command_line_options.empty());
426+
BOOST_CHECK(test_args.m_settings.ro_config.size() == 3);
427+
BOOST_CHECK(test_args.m_settings.ro_config[""].size() == 8);
428+
BOOST_CHECK(test_args.m_settings.ro_config["sec1"].size() == 3);
429+
BOOST_CHECK(test_args.m_settings.ro_config["sec2"].size() == 2);
430+
431+
BOOST_CHECK(test_args.m_settings.ro_config[""].count("a")
432+
&& test_args.m_settings.ro_config[""].count("b")
433+
&& test_args.m_settings.ro_config[""].count("ccc")
434+
&& test_args.m_settings.ro_config[""].count("d")
435+
&& test_args.m_settings.ro_config[""].count("fff")
436+
&& test_args.m_settings.ro_config[""].count("ggg")
437+
&& test_args.m_settings.ro_config[""].count("h")
438+
&& test_args.m_settings.ro_config[""].count("i")
433439
);
434-
BOOST_CHECK(test_args.GetConfigArgs().count("-sec1.ccc")
435-
&& test_args.GetConfigArgs().count("-sec1.h")
436-
&& test_args.GetConfigArgs().count("-sec2.ccc")
437-
&& test_args.GetConfigArgs().count("-sec2.iii")
440+
BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("ccc")
441+
&& test_args.m_settings.ro_config["sec1"].count("h")
442+
&& test_args.m_settings.ro_config["sec2"].count("ccc")
443+
&& test_args.m_settings.ro_config["sec2"].count("iii")
438444
);
439445

440446
BOOST_CHECK(test_args.IsArgSet("-a")
@@ -573,24 +579,25 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
573579
BOOST_AUTO_TEST_CASE(util_GetArg)
574580
{
575581
TestArgsManager testArgs;
576-
testArgs.GetOverrideArgs().clear();
577-
testArgs.GetOverrideArgs()["strtest1"] = {"string..."};
582+
LOCK(testArgs.cs_args);
583+
testArgs.m_settings.command_line_options.clear();
584+
testArgs.m_settings.command_line_options["strtest1"] = {"string..."};
578585
// strtest2 undefined on purpose
579-
testArgs.GetOverrideArgs()["inttest1"] = {"12345"};
580-
testArgs.GetOverrideArgs()["inttest2"] = {"81985529216486895"};
586+
testArgs.m_settings.command_line_options["inttest1"] = {"12345"};
587+
testArgs.m_settings.command_line_options["inttest2"] = {"81985529216486895"};
581588
// inttest3 undefined on purpose
582-
testArgs.GetOverrideArgs()["booltest1"] = {""};
589+
testArgs.m_settings.command_line_options["booltest1"] = {""};
583590
// booltest2 undefined on purpose
584-
testArgs.GetOverrideArgs()["booltest3"] = {"0"};
585-
testArgs.GetOverrideArgs()["booltest4"] = {"1"};
591+
testArgs.m_settings.command_line_options["booltest3"] = {"0"};
592+
testArgs.m_settings.command_line_options["booltest4"] = {"1"};
586593

587594
// priorities
588-
testArgs.GetOverrideArgs()["pritest1"] = {"a", "b"};
589-
testArgs.GetConfigArgs()["pritest2"] = {"a", "b"};
590-
testArgs.GetOverrideArgs()["pritest3"] = {"a"};
591-
testArgs.GetConfigArgs()["pritest3"] = {"b"};
592-
testArgs.GetOverrideArgs()["pritest4"] = {"a","b"};
593-
testArgs.GetConfigArgs()["pritest4"] = {"c","d"};
595+
testArgs.m_settings.command_line_options["pritest1"] = {"a", "b"};
596+
testArgs.m_settings.ro_config[""]["pritest2"] = {"a", "b"};
597+
testArgs.m_settings.command_line_options["pritest3"] = {"a"};
598+
testArgs.m_settings.ro_config[""]["pritest3"] = {"b"};
599+
testArgs.m_settings.command_line_options["pritest4"] = {"a","b"};
600+
testArgs.m_settings.ro_config[""]["pritest4"] = {"c","d"};
594601

595602
BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string...");
596603
BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default");

0 commit comments

Comments
 (0)