Merge #15935: Add <datadir>/settings.json persistent settings storage

9c69cfe4c5 Add <datadir>/settings.json persistent settings storage. (Russell Yanofsky)
eb682c5700 util: Add ReadSettings and WriteSettings functions (Russell Yanofsky)

Pull request description:

  Persistent settings are used in followup PRs #15936 to unify gui settings between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to the loadwallet RPC and maintain a dynamic list of wallets that should be loaded on startup that also can be shared between bitcoind and bitcoin-qt.

ACKs for top commit:
  MarcoFalke:
    Approach re-ACK 9c69cfe4c5 🌾
  jnewbery:
    utACK 9c69cfe4c5

Tree-SHA512: 39fcc6051717117c9141e934de1d0d3f739484be4685cdf97d54de967c8c816502b4fd0de12114433beaa5c5b7060c810fd8ae4e2b3ce7c371eb729ac01ba2e1

src/test/settings_tests.cpp

@@ -12,10 +12,90 @@
 #include <univalue.h>
 #include <util/strencodings.h>
 #include <util/string.h>
+#include <util/system.h>
 #include <vector>
 
+inline bool operator==(const util::SettingsValue& a, const util::SettingsValue& b)
+{
+    return a.write() == b.write();
+}
+
+inline std::ostream& operator<<(std::ostream& os, const util::SettingsValue& value)
+{
+    os << value.write();
+    return os;
+}
+
+inline std::ostream& operator<<(std::ostream& os, const std::pair<std::string, util::SettingsValue>& kv)
+{
+    util::SettingsValue out(util::SettingsValue::VOBJ);
+    out.__pushKV(kv.first, kv.second);
+    os << out.write();
+    return os;
+}
+
+inline void WriteText(const fs::path& path, const std::string& text)
+{
+    fsbridge::ofstream file;
+    file.open(path);
+    file << text;
+}
+
 BOOST_FIXTURE_TEST_SUITE(settings_tests, BasicTestingSetup)
 
+BOOST_AUTO_TEST_CASE(ReadWrite)
+{
+    fs::path path = GetDataDir() / "settings.json";
+
+    WriteText(path, R"({
+        "string": "string",
+        "num": 5,
+        "bool": true,
+        "null": null
+    })");
+
+    std::map<std::string, util::SettingsValue> expected{
+        {"string", "string"},
+        {"num", 5},
+        {"bool", true},
+        {"null", {}},
+    };
+
+    // Check file read.
+    std::map<std::string, util::SettingsValue> values;
+    std::vector<std::string> errors;
+    BOOST_CHECK(util::ReadSettings(path, values, errors));
+    BOOST_CHECK_EQUAL_COLLECTIONS(values.begin(), values.end(), expected.begin(), expected.end());
+    BOOST_CHECK(errors.empty());
+
+    // Check no errors if file doesn't exist.
+    fs::remove(path);
+    BOOST_CHECK(util::ReadSettings(path, values, errors));
+    BOOST_CHECK(values.empty());
+    BOOST_CHECK(errors.empty());
+
+    // Check duplicate keys not allowed
+    WriteText(path, R"({
+        "dupe": "string",
+        "dupe": "dupe"
+    })");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> dup_keys = {strprintf("Found duplicate key dupe in settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), dup_keys.begin(), dup_keys.end());
+
+    // Check non-kv json files not allowed
+    WriteText(path, R"("non-kv")");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> non_kv = {strprintf("Found non-object value \"non-kv\" in settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), non_kv.begin(), non_kv.end());
+
+    // Check invalid json not allowed
+    WriteText(path, R"(invalid json)");
+    BOOST_CHECK(!util::ReadSettings(path, values, errors));
+    std::vector<std::string> fail_parse = {strprintf("Unable to parse settings file %s", path.string())};
+    BOOST_CHECK_EQUAL_COLLECTIONS(errors.begin(), errors.end(), fail_parse.begin(), fail_parse.end());
+}
+
 //! Check settings struct contents against expected json strings.
 static void CheckValues(const util::Settings& settings, const std::string& single_val, const std::string& list_val)
 {

src/test/util_tests.cpp

@@ -9,6 +9,7 @@
 #include <key.h>  // For CKey
 #include <optional.h>
 #include <sync.h>
+#include <test/util/logging.h>
 #include <test/util/setup_common.h>
 #include <test/util/str.h>
 #include <uint256.h>
@@ -1138,6 +1139,28 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup)
     BOOST_CHECK_EQUAL(out_sha_hex, "f0b3a3c29869edc765d579c928f7f1690a71fbb673b49ccf39cbc4de18156a0d");
 }
 
+BOOST_AUTO_TEST_CASE(util_ReadWriteSettings)
+{
+    // Test writing setting.
+    TestArgsManager args1;
+    args1.LockSettings([&](util::Settings& settings) { settings.rw_settings["name"] = "value"; });
+    args1.WriteSettingsFile();
+
+    // Test reading setting.
+    TestArgsManager args2;
+    args2.ReadSettingsFile();
+    args2.LockSettings([&](util::Settings& settings) { BOOST_CHECK_EQUAL(settings.rw_settings["name"].get_str(), "value"); });
+
+    // Test error logging, and remove previously written setting.
+    {
+        ASSERT_DEBUG_LOG("Failed renaming settings file");
+        fs::remove(GetDataDir() / "settings.json");
+        fs::create_directory(GetDataDir() / "settings.json");
+        args2.WriteSettingsFile();
+        fs::remove(GetDataDir() / "settings.json");
+    }
+}
+
 BOOST_AUTO_TEST_CASE(util_FormatMoney)
 {
     BOOST_CHECK_EQUAL(FormatMoney(0), "0.00");