From f569658532ddacc58be34da8f77f9702e10e0ed9 Mon Sep 17 00:00:00 2001 From: Nathan Hughes Date: Tue, 17 Sep 2024 10:40:44 -0400 Subject: [PATCH] Fix/collection defaults (#33) * (wip) fix overriding vector defaults * revert default changes and fix map * fix printing issue with newer gtest --- .../internal/visitor_impl.hpp | 25 +++++++++----- config_utilities/test/tests/config_arrays.cpp | 34 +++++++++++++++++++ config_utilities/test/tests/config_maps.cpp | 30 ++++++++++++++++ 3 files changed, 80 insertions(+), 9 deletions(-) diff --git a/config_utilities/include/config_utilities/internal/visitor_impl.hpp b/config_utilities/include/config_utilities/internal/visitor_impl.hpp index 6ce93f3..2e5fe9d 100644 --- a/config_utilities/include/config_utilities/internal/visitor_impl.hpp +++ b/config_utilities/include/config_utilities/internal/visitor_impl.hpp @@ -231,10 +231,15 @@ void Visitor::visitField(std::vector& config, const std::string& field_ } if (visitor.mode == Visitor::Mode::kSet) { + const auto array_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name; + const auto subnode = lookupNamespace(visitor.data.data, array_ns); + if (!subnode) { + return; // don't override the field if not present + } + // When setting the values first allocate the correct amount of configs. config.clear(); - const auto array_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name; - const std::vector nodes = getNodeArray(lookupNamespace(visitor.data.data, array_ns)); + const std::vector nodes = getNodeArray(subnode); size_t index = 0; for (const auto& node : nodes) { ConfigT& sub_config = config.emplace_back(); @@ -274,11 +279,8 @@ void Visitor::visitField(std::map& config, const std::string& field_ return; } - OrderedMap intermediate; - // copy current config state if doing something else other than settings the config - if (visitor.mode != Visitor::Mode::kSet) { - intermediate.insert(intermediate.begin(), config.begin(), config.end()); - } + // copy current config state + OrderedMap intermediate(config.begin(), config.end()); // make use of more general named parsing visitField(intermediate, field_name, unit); @@ -301,10 +303,15 @@ void Visitor::visitField(OrderedMap& config, const std::string& fiel } if (visitor.mode == Visitor::Mode::kSet) { + const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name; + const auto subnode = lookupNamespace(visitor.data.data, map_ns); + if (!subnode) { + return; // don't override the field if not present + } + // When setting the values first allocate the correct amount of configs. config.clear(); - const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name; - const auto nodes = getNodeMap(lookupNamespace(visitor.data.data, map_ns)); + const auto nodes = getNodeMap(subnode); for (auto&& [key, node] : nodes) { auto& entry = config.emplace_back(); entry.first = key.template as(); diff --git a/config_utilities/test/tests/config_arrays.cpp b/config_utilities/test/tests/config_arrays.cpp index 1b23362..55fca0e 100644 --- a/config_utilities/test/tests/config_arrays.cpp +++ b/config_utilities/test/tests/config_arrays.cpp @@ -105,6 +105,8 @@ class AddString : public ProcessorBase { config::RegistrationWithConfig("AddString"); }; +bool operator==(const AddString::Config& lhs, const AddString::Config& rhs) { return lhs.s == rhs.s; } + void declare_config(AddString::Config& config) { name("AddString"); field(config.s, "s"); @@ -128,6 +130,21 @@ void declare_config(ConfigWithNestedArray& config) { field(config.nested, "nested"); } +struct ConfigWithDefaultArray { + std::vector configs{{"world!"}, {"hello"}}; +}; + +void declare_config(ConfigWithDefaultArray& config) { + name("ConfigWithDefaultArray"); + field(config.configs, "configs"); +} + +bool operator==(const ConfigWithDefaultArray& lhs, const ConfigWithDefaultArray& rhs) { + return lhs.configs == rhs.configs; +} + +void PrintTo(const ConfigWithDefaultArray& config, std::ostream* os) { *os << toString(config); } + TEST(ConfigArrays, FromYamlSeq) { const std::string yaml_seq = R"( - s: "a" @@ -407,4 +424,21 @@ config_array[2] [ArrConfig]: EXPECT_EQ(formatted, expected); } +TEST(ConfigArrays, ArrayWithDefaults) { + { // make sure not specifying anything results in default + const auto node = YAML::Load(""); + auto config = fromYaml(node); + EXPECT_EQ(config.configs.size(), 2); + EXPECT_EQ(config, ConfigWithDefaultArray()); + } + + { // specifying empty list clears default + const auto node = YAML::Load("configs: []"); + auto config = fromYaml(node); + EXPECT_EQ(config.configs.size(), 0); + ConfigWithDefaultArray expected{{}}; + EXPECT_EQ(config, expected); + } +} + } // namespace config::test diff --git a/config_utilities/test/tests/config_maps.cpp b/config_utilities/test/tests/config_maps.cpp index 8f753c5..eeedd7c 100644 --- a/config_utilities/test/tests/config_maps.cpp +++ b/config_utilities/test/tests/config_maps.cpp @@ -99,6 +99,19 @@ void declare_config(ConfigWithNestedMaps& config) { field(config.nested, "nested"); } +struct ConfigWithDefaultMap { + std::map configs{{"name1", {"world!", 1}}, {"name2", {"hello", 3}}}; +}; + +void declare_config(ConfigWithDefaultMap& config) { + name("ConfigWithDefaultMap"); + field(config.configs, "configs"); +} + +bool operator==(const ConfigWithDefaultMap& lhs, const ConfigWithDefaultMap& rhs) { return lhs.configs == rhs.configs; } + +void PrintTo(const ConfigWithDefaultMap& config, std::ostream* os) { *os << toString(config); } + TEST(ConfigMaps, FromYamlMap) { const std::string yaml_map = R"( x: @@ -278,4 +291,21 @@ config_map[4] [MapConfig]: EXPECT_EQ(formatted, expected); } +TEST(ConfigMaps, MapWithDefault) { + { // make sure not specifying anything results in default + const auto node = YAML::Load(""); + auto config = fromYaml(node); + EXPECT_EQ(config.configs.size(), 2); + EXPECT_EQ(config, ConfigWithDefaultMap()); + } + + { // specifying empty list clears default + const auto node = YAML::Load("configs: {}"); + auto config = fromYaml(node); + EXPECT_EQ(config.configs.size(), 0); + ConfigWithDefaultMap expected{{}}; + EXPECT_EQ(config, expected); + } +} + } // namespace config::test