From ccc60b42459b29fed266759bd668b697636d5253 Mon Sep 17 00:00:00 2001 From: Aleksei Bavshin Date: Tue, 14 Sep 2021 12:16:37 +0700 Subject: [PATCH] refactor(config): more sensible multi-bar include behavior --- include/config.hpp | 2 +- src/config.cpp | 20 +++++++------------- test/config.cpp | 3 ++- test/config/include-multi-0.json | 13 ++++--------- test/config/include-multi-1.json | 11 +++-------- test/config/include-multi-2.json | 13 ++++--------- test/config/include-multi-3-0.json | 11 +++-------- test/config/include-multi-3.json | 13 ++++--------- 8 files changed, 28 insertions(+), 58 deletions(-) diff --git a/include/config.hpp b/include/config.hpp index 25b78ab2..82d55995 100644 --- a/include/config.hpp +++ b/include/config.hpp @@ -28,7 +28,7 @@ class Config { std::vector getOutputConfigs(const std::string &name, const std::string &identifier); private: - void setupConfig(const std::string &config_file, int depth); + void setupConfig(Json::Value &dst, const std::string &config_file, int depth); void resolveConfigIncludes(Json::Value &config, int depth); void mergeConfig(Json::Value &a_config_, Json::Value &b_config_); diff --git a/src/config.cpp b/src/config.cpp index 207e1bd1..63f18c26 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -46,7 +46,7 @@ std::optional Config::findConfigPath(const std::vector return std::nullopt; } -void Config::setupConfig(const std::string &config_file, int depth) { +void Config::setupConfig(Json::Value &dst, const std::string &config_file, int depth) { if (depth > 100) { throw std::runtime_error("Aborting due to likely recursive include in config files"); } @@ -64,7 +64,7 @@ void Config::setupConfig(const std::string &config_file, int depth) { } else { resolveConfigIncludes(tmp_config, depth); } - mergeConfig(config_, tmp_config); + mergeConfig(dst, tmp_config); } void Config::resolveConfigIncludes(Json::Value &config, int depth) { @@ -72,11 +72,11 @@ void Config::resolveConfigIncludes(Json::Value &config, int depth) { if (includes.isArray()) { for (const auto &include : includes) { spdlog::info("Including resource file: {}", include.asString()); - setupConfig(tryExpandPath(include.asString()).value_or(""), ++depth); + setupConfig(config, tryExpandPath(include.asString()).value_or(""), ++depth); } } else if (includes.isString()) { spdlog::info("Including resource file: {}", includes.asString()); - setupConfig(tryExpandPath(includes.asString()).value_or(""), ++depth); + setupConfig(config, tryExpandPath(includes.asString()).value_or(""), ++depth); } } @@ -88,17 +88,11 @@ void Config::mergeConfig(Json::Value &a_config_, Json::Value &b_config_) { for (const auto &key : b_config_.getMemberNames()) { if (a_config_[key].isObject() && b_config_[key].isObject()) { mergeConfig(a_config_[key], b_config_[key]); - } else { + } else if (a_config_[key].isNull()) { + // do not allow overriding value set by top or previously included config a_config_[key] = b_config_[key]; } } - } else if (a_config_.isArray() && b_config_.isArray()) { - // This can happen only on the top-level array of a multi-bar config - for (Json::Value::ArrayIndex i = 0; i < b_config_.size(); i++) { - if (a_config_[i].isObject() && b_config_[i].isObject()) { - mergeConfig(a_config_[i], b_config_[i]); - } - } } else { spdlog::error("Cannot merge config, conflicting or invalid JSON types"); } @@ -133,7 +127,7 @@ void Config::load(const std::string &config) { } config_file_ = file.value(); spdlog::info("Using configuration file {}", config_file_); - setupConfig(config_file_, 0); + setupConfig(config_, config_file_, 0); } std::vector Config::getOutputConfigs(const std::string &name, diff --git a/test/config.cpp b/test/config.cpp index 0dc0b42a..343a1c11 100644 --- a/test/config.cpp +++ b/test/config.cpp @@ -62,7 +62,8 @@ TEST_CASE("Load simple config with include", "[config]") { SECTION("validate the config data") { auto& data = conf.getConfig(); - REQUIRE(data["layer"].asString() == "bottom"); + // config override behavior: preserve first included value + REQUIRE(data["layer"].asString() == "top"); REQUIRE(data["height"].asInt() == 30); // config override behavior: preserve value from the top config REQUIRE(data["position"].asString() == "top"); diff --git a/test/config/include-multi-0.json b/test/config/include-multi-0.json index 87b6cabe..a4c3fc17 100644 --- a/test/config/include-multi-0.json +++ b/test/config/include-multi-0.json @@ -1,9 +1,4 @@ -[ - { - "output": "OUT-0", - "height": 20 - }, - {}, - {}, - {} -] +{ + "output": "OUT-0", + "height": 20 +} diff --git a/test/config/include-multi-1.json b/test/config/include-multi-1.json index d816a0fd..2b28d6c5 100644 --- a/test/config/include-multi-1.json +++ b/test/config/include-multi-1.json @@ -1,8 +1,3 @@ -[ - {}, - { - "height": 21 - }, - {}, - {} -] +{ + "height": 21 +} diff --git a/test/config/include-multi-2.json b/test/config/include-multi-2.json index 47616ef1..f74c2b4e 100644 --- a/test/config/include-multi-2.json +++ b/test/config/include-multi-2.json @@ -1,9 +1,4 @@ -[ - {}, - {}, - { - "output": "OUT-1", - "height": 22 - }, - {} -] +{ + "output": "OUT-1", + "height": 22 +} diff --git a/test/config/include-multi-3-0.json b/test/config/include-multi-3-0.json index 3f4da0c9..11cdd3f9 100644 --- a/test/config/include-multi-3-0.json +++ b/test/config/include-multi-3-0.json @@ -1,8 +1,3 @@ -[ - {}, - {}, - {}, - { - "height": 23 - } -] +{ + "height": 23 +} diff --git a/test/config/include-multi-3.json b/test/config/include-multi-3.json index d0951894..309fe15e 100644 --- a/test/config/include-multi-3.json +++ b/test/config/include-multi-3.json @@ -1,9 +1,4 @@ -[ - {}, - {}, - {}, - { - "output": "OUT-3", - "include": "test/config/include-multi-3-0.json" - } -] +{ + "output": "OUT-3", + "include": "test/config/include-multi-3-0.json" +}