refactor(config): more sensible multi-bar include behavior

include/config.hpp

@@ -28,7 +28,7 @@ class Config {
   std::vector<Json::Value> 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_);
 

src/config.cpp

@@ -46,7 +46,7 @@ std::optional<std::string> Config::findConfigPath(const std::vector<std::string>
   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<Json::Value> Config::getOutputConfigs(const std::string &name,

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");

test/config/include-multi-0.json

@@ -1,9 +1,4 @@
-[
-  {
-    "output": "OUT-0",
-    "height": 20
-  },
-  {},
-  {},
-  {}
-]
+{
+  "output": "OUT-0",
+  "height": 20
+}

test/config/include-multi-1.json

@@ -1,8 +1,3 @@
-[
-  {},
-  {
-    "height": 21
-  },
-  {},
-  {}
-]
+{
+  "height": 21
+}

test/config/include-multi-2.json

@@ -1,9 +1,4 @@
-[
-  {},
-  {},
-  {
-    "output": "OUT-1",
-    "height": 22
-  },
-  {}
-]
+{
+  "output": "OUT-1",
+  "height": 22
+}

test/config/include-multi-3-0.json

@@ -1,8 +1,3 @@
-[
-  {},
-  {},
-  {},
-  {
-    "height": 23
-  }
-]
+{
+  "height": 23
+}

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"
+}