From bd69dcfc1547ba71ba50f159df00ebbb444f505f Mon Sep 17 00:00:00 2001 From: Nicolas Gomez Palacios Date: Tue, 3 Dec 2024 23:07:45 -0300 Subject: [PATCH] feat(configurationParser): moves the MergeYamlNodes method to an external function, added some UTs for the new MergeYamlNodes function --- src/agent/configuration_parser/CMakeLists.txt | 2 +- .../include/configuration_parser.hpp | 13 -- .../include/configuration_parser_utils.hpp | 16 ++ .../src/configuration_parser.cpp | 110 +------------ .../src/configuration_parser_utils.cpp | 112 +++++++++++++ .../configuration_parser/tests/CMakeLists.txt | 7 + .../tests/configuration_parser_utils_test.cpp | 155 ++++++++++++++++++ 7 files changed, 292 insertions(+), 123 deletions(-) create mode 100644 src/agent/configuration_parser/include/configuration_parser_utils.hpp create mode 100644 src/agent/configuration_parser/src/configuration_parser_utils.cpp create mode 100644 src/agent/configuration_parser/tests/configuration_parser_utils_test.cpp diff --git a/src/agent/configuration_parser/CMakeLists.txt b/src/agent/configuration_parser/CMakeLists.txt index f11bb952df..0659246d8a 100644 --- a/src/agent/configuration_parser/CMakeLists.txt +++ b/src/agent/configuration_parser/CMakeLists.txt @@ -10,7 +10,7 @@ set_common_settings() find_package(yaml-cpp CONFIG REQUIRED) -add_library(ConfigurationParser src/configuration_parser.cpp) +add_library(ConfigurationParser src/configuration_parser.cpp src/configuration_parser_utils.cpp) target_include_directories(ConfigurationParser PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include) target_link_libraries(ConfigurationParser PUBLIC yaml-cpp::yaml-cpp Logger Config) diff --git a/src/agent/configuration_parser/include/configuration_parser.hpp b/src/agent/configuration_parser/include/configuration_parser.hpp index 496f7a9df4..4793e4ccf5 100644 --- a/src/agent/configuration_parser/include/configuration_parser.hpp +++ b/src/agent/configuration_parser/include/configuration_parser.hpp @@ -40,19 +40,6 @@ namespace configuration /// @brief The groups information std::function()> m_getGroups; - /// @brief Merges two YAML nodes, modifying the base node to include or override values from the - /// override node. - /// - /// This function traverses the two YAML nodes. If a key exists in both nodes: - /// - If both values are maps, the function recurses to merge their content. - /// - If both values are sequences, their elements are concatenated. - /// - In all other cases (scalars, aliases, null values), the value from the override node replaces the value in - /// the base node. If a key only exists in the override node, it is added to the base node. - /// - /// @param base Reference to the base YAML::Node that will be modified. - /// @param override Const reference to the YAML::Node containing values to merge into the base. - void MergeYamlNodes(YAML::Node& base, const YAML::Node& override); - /// @brief Method for loading the configuration from local file void LoadLocalConfig(); diff --git a/src/agent/configuration_parser/include/configuration_parser_utils.hpp b/src/agent/configuration_parser/include/configuration_parser_utils.hpp new file mode 100644 index 0000000000..b976176fb1 --- /dev/null +++ b/src/agent/configuration_parser/include/configuration_parser_utils.hpp @@ -0,0 +1,16 @@ +#pragma once + +#include + +/// @brief Merges two YAML nodes, modifying the baseYaml node to include or additionalYaml values from the +/// additionalYaml node. +/// +/// This function traverses the two YAML nodes. If a key exists in both nodes: +/// - If both values are maps, the function recurses to merge their content. +/// - If both values are sequences, their elements are concatenated. +/// - In all other cases (scalars, aliases, null values), the value from the additionalYaml node replaces the value in +/// the baseYaml node. If a key only exists in the additionalYaml node, it is added to the baseYaml node. +/// +/// @param baseYaml Reference to the baseYaml YAML::Node that will be modified. +/// @param additionalYaml Const reference to the YAML::Node containing values to merge into the baseYaml. +void MergeYamlNodes(YAML::Node& baseYaml, const YAML::Node& additionalYaml); diff --git a/src/agent/configuration_parser/src/configuration_parser.cpp b/src/agent/configuration_parser/src/configuration_parser.cpp index f7d8d595a3..a7c48599d3 100644 --- a/src/agent/configuration_parser/src/configuration_parser.cpp +++ b/src/agent/configuration_parser/src/configuration_parser.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -124,115 +125,6 @@ namespace configuration } } - void ConfigurationParser::MergeYamlNodes(YAML::Node& base, const YAML::Node& override) - { - // Queue to manage nodes to be merged. Pairs of nodes are handled directly. - std::queue> nodesToProcess; - nodesToProcess.emplace(base, override); - - while (!nodesToProcess.empty()) - { - auto [baseNode, overrideNode] = nodesToProcess.front(); - nodesToProcess.pop(); - - // Traverse each key-value pair in the override node. - for (auto it = overrideNode.begin(); it != overrideNode.end(); ++it) - { - const auto key = it->first.as(); - YAML::Node value = it->second; - - if (baseNode[key]) - { - // Key exists in the base node. - if (value.IsMap() && baseNode[key].IsMap()) - { - // Both values are maps: enqueue for further merging. - nodesToProcess.emplace(baseNode[key], value); - } - else if (value.IsSequence() && baseNode[key].IsSequence()) - { - // Merge sequences while preserving the order. - YAML::Node mergedSequence = YAML::Node(YAML::NodeType::Sequence); - - // Collect elements from 'override' sequence to preserve insertion order. - std::vector> overrideElements; - for (const YAML::Node& elem : value) - { - if (elem.IsScalar()) - { - overrideElements.emplace_back(elem.as(), elem); - } - else if (elem.IsMap() && elem.begin() != elem.end()) - { - overrideElements.emplace_back(elem.begin()->first.as(), elem); - } - } - - // Track which keys from 'override' sequence are merged. - std::unordered_set mergedKeys; - - for (const YAML::Node& elem : baseNode[key]) - { - std::string elemKey; - - // Extract the key based on the type of element. - if (elem.IsScalar()) - { - elemKey = elem.as(); - } - else if (elem.IsMap() && elem.begin() != elem.end()) - { - elemKey = elem.begin()->first.as(); - } - else - { - // Skip elements that don't fit the expected types. - mergedSequence.push_back(elem); - continue; - } - - // Common logic for merging elements. - auto overrideItem = - std::find_if(overrideElements.begin(), - overrideElements.end(), - [&elemKey](const auto& pair) { return pair.first == elemKey; }); - if (overrideItem != overrideElements.end()) - { - mergedSequence.push_back(overrideItem->second); - mergedKeys.insert(overrideItem->first); - } - else - { - mergedSequence.push_back(elem); - } - } - - // Add remaining elements from 'override' sequence in order. - for (const auto& [itemKey, itemNode] : overrideElements) - { - if (mergedKeys.find(itemKey) == mergedKeys.end()) - { - mergedSequence.push_back(itemNode); - } - } - - baseNode[key] = mergedSequence; - } - else - { - // Other cases (scalar, alias, null): overwrite the value. - baseNode[key] = value; - } - } - else - { - // Key does not exist in the base node: add it directly. - baseNode[key] = value; - } - } - } - } - void ConfigurationParser::LoadSharedConfig() { LogDebug("Loading shared configuration."); diff --git a/src/agent/configuration_parser/src/configuration_parser_utils.cpp b/src/agent/configuration_parser/src/configuration_parser_utils.cpp new file mode 100644 index 0000000000..de89ebe3a3 --- /dev/null +++ b/src/agent/configuration_parser/src/configuration_parser_utils.cpp @@ -0,0 +1,112 @@ +#include +#include +#include + +void MergeYamlNodes(YAML::Node& baseYaml, const YAML::Node& additionalYaml) +{ + // Queue to manage nodes to be merged. Pairs of nodes are handled directly. + std::queue> nodesToProcess; + nodesToProcess.emplace(baseYaml, additionalYaml); + + while (!nodesToProcess.empty()) + { + auto [baseNode, additionalNode] = nodesToProcess.front(); + nodesToProcess.pop(); + + // Traverse each key-value pair in the additionalYaml node. + for (auto it = additionalNode.begin(); it != additionalNode.end(); ++it) + { + const auto key = it->first.as(); + YAML::Node value = it->second; + + if (baseNode[key]) + { + // Key exists in the baseYaml node. + if (value.IsMap() && baseNode[key].IsMap()) + { + // Both values are maps: enqueue for further merging. + nodesToProcess.emplace(baseNode[key], value); + } + else if (value.IsSequence() && baseNode[key].IsSequence()) + { + // Merge sequences while preserving the order. + YAML::Node mergedSequence = YAML::Node(YAML::NodeType::Sequence); + + // Collect elements from 'additionalYaml' sequence to preserve insertion order. + std::vector> additionalElements; + for (const YAML::Node& elem : value) + { + if (elem.IsScalar()) + { + additionalElements.emplace_back(elem.as(), elem); + } + else if (elem.IsMap() && elem.begin() != elem.end()) + { + additionalElements.emplace_back(elem.begin()->first.as(), elem); + } + } + + // Track which keys from 'additionalYaml' sequence are merged. + std::unordered_set mergedKeys; + + for (const YAML::Node& elem : baseNode[key]) + { + std::string elemKey; + + // Extract the key based on the type of element. + if (elem.IsScalar()) + { + elemKey = elem.as(); + } + else if (elem.IsMap() && elem.begin() != elem.end()) + { + elemKey = elem.begin()->first.as(); + } + else + { + // Skip elements that don't fit the expected types. + mergedSequence.push_back(elem); + continue; + } + + // Common logic for merging elements. + auto additionalItem = + std::find_if(additionalElements.begin(), + additionalElements.end(), + [&elemKey](const auto& pair) { return pair.first == elemKey; }); + if (additionalItem != additionalElements.end()) + { + mergedSequence.push_back(additionalItem->second); + mergedKeys.insert(additionalItem->first); + } + else + { + mergedSequence.push_back(elem); + } + } + + // Add remaining elements from 'additionalYaml' sequence in order. + for (const auto& [itemKey, itemNode] : additionalElements) + { + if (mergedKeys.find(itemKey) == mergedKeys.end()) + { + mergedSequence.push_back(itemNode); + } + } + + baseNode[key] = mergedSequence; + } + else + { + // Other cases (scalar, alias, null): overwrite the value. + baseNode[key] = value; + } + } + else + { + // Key does not exist in the baseYaml node: add it directly. + baseNode[key] = value; + } + } + } +} diff --git a/src/agent/configuration_parser/tests/CMakeLists.txt b/src/agent/configuration_parser/tests/CMakeLists.txt index 477498d3d8..3d8e6b3f0b 100644 --- a/src/agent/configuration_parser/tests/CMakeLists.txt +++ b/src/agent/configuration_parser/tests/CMakeLists.txt @@ -1,6 +1,13 @@ find_package(GTest CONFIG REQUIRED) +find_package(yaml-cpp CONFIG REQUIRED) add_executable(ConfigurationParser_test configuration_parser_test.cpp) configure_target(ConfigurationParser_test) target_link_libraries(ConfigurationParser_test PUBLIC ConfigurationParser Config GTest::gtest GTest::gtest_main GTest::gmock GTest::gmock_main Logger) add_test(NAME ConfigParserTest COMMAND ConfigurationParser_test) + +add_executable(ConfigurationParserUtils_test configuration_parser_utils_test.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/configuration_parser_utils.cpp) +configure_target(ConfigurationParserUtils_test) +target_include_directories(ConfigurationParserUtils_test PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../include) +target_link_libraries(ConfigurationParserUtils_test PUBLIC GTest::gtest GTest::gtest_main GTest::gmock GTest::gmock_main yaml-cpp::yaml-cpp) +add_test(NAME ConfigParserUtilsTest COMMAND ConfigurationParserUtils_test) diff --git a/src/agent/configuration_parser/tests/configuration_parser_utils_test.cpp b/src/agent/configuration_parser/tests/configuration_parser_utils_test.cpp new file mode 100644 index 0000000000..7a8154685e --- /dev/null +++ b/src/agent/configuration_parser/tests/configuration_parser_utils_test.cpp @@ -0,0 +1,155 @@ +#include +#include +#include + +/// @brief Test for merging two simple YAML maps with unique keys. +TEST(MergeYamlNodesTest, MergeSimpleMaps) +{ + YAML::Node base = YAML::Load(R"( + key1: value1 + key2: value2 + )"); + YAML::Node additional = YAML::Load(R"( + key3: value3 + key4: value4 + )"); + + MergeYamlNodes(base, additional); + + EXPECT_EQ(base["key1"].as(), "value1"); + EXPECT_EQ(base["key2"].as(), "value2"); + EXPECT_EQ(base["key3"].as(), "value3"); + EXPECT_EQ(base["key4"].as(), "value4"); +} + +/// @brief Test for overriding a key in the base YAML with a value from additional YAML. +TEST(MergeYamlNodesTest, OverrideScalarValue) +{ + YAML::Node base = YAML::Load(R"( + key1: value1 + key2: value2 + )"); + YAML::Node additional = YAML::Load(R"( + key2: new_value + )"); + + MergeYamlNodes(base, additional); + + EXPECT_EQ(base["key1"].as(), "value1"); + EXPECT_EQ(base["key2"].as(), "new_value"); +} + +/// @brief Test for merging nested YAML maps. +TEST(MergeYamlNodesTest, MergeNestedMaps) +{ + YAML::Node base = YAML::Load(R"( + parent: + child1: value1 + child2: value2 + )"); + YAML::Node additional = YAML::Load(R"( + parent: + child2: new_value + child3: value3 + )"); + + MergeYamlNodes(base, additional); + + ASSERT_TRUE(base["parent"].IsMap()); + EXPECT_EQ(base["parent"]["child1"].as(), "value1"); + EXPECT_EQ(base["parent"]["child2"].as(), "new_value"); + EXPECT_EQ(base["parent"]["child3"].as(), "value3"); +} + +/// @brief Test for merging YAML sequences while preserving order and avoiding duplicates. +TEST(MergeYamlNodesTest, MergeSequences) +{ + YAML::Node base = YAML::Load(R"( + key: + - item1 + - item2 + )"); + YAML::Node additional = YAML::Load(R"( + key: + - item2 + - item3 + )"); + + MergeYamlNodes(base, additional); + + ASSERT_TRUE(base["key"].IsSequence()); + EXPECT_EQ(base["key"].size(), 3); + EXPECT_EQ(base["key"][0].as(), "item1"); + EXPECT_EQ(base["key"][1].as(), "item2"); + EXPECT_EQ(base["key"][2].as(), "item3"); +} + +/// @brief Test for adding new keys to a base YAML with existing nested maps. +TEST(MergeYamlNodesTest, AddNewKeysToNestedMap) +{ + YAML::Node base = YAML::Load(R"( + parent: + child1: value1 + )"); + YAML::Node additional = YAML::Load(R"( + parent: + child2: value2 + )"); + + MergeYamlNodes(base, additional); + + ASSERT_TRUE(base["parent"].IsMap()); + EXPECT_EQ(base["parent"]["child1"].as(), "value1"); + EXPECT_EQ(base["parent"]["child2"].as(), "value2"); +} + +/// @brief Test for merging when base YAML is empty. +TEST(MergeYamlNodesTest, BaseYamlEmpty) +{ + YAML::Node base = YAML::Load("{}"); + YAML::Node additional = YAML::Load(R"( + key1: value1 + key2: value2 + )"); + + MergeYamlNodes(base, additional); + + EXPECT_EQ(base["key1"].as(), "value1"); + EXPECT_EQ(base["key2"].as(), "value2"); +} + +/// @brief Test for merging when additional YAML is empty. +TEST(MergeYamlNodesTest, AdditionalYamlEmpty) +{ + YAML::Node base = YAML::Load(R"( + key1: value1 + key2: value2 + )"); + YAML::Node additional = YAML::Load("{}"); + + MergeYamlNodes(base, additional); + + EXPECT_EQ(base["key1"].as(), "value1"); + EXPECT_EQ(base["key2"].as(), "value2"); +} + +/// @brief Test for handling edge cases with scalar values. +TEST(MergeYamlNodesTest, ScalarsOverwriteCorrectly) +{ + YAML::Node base = YAML::Load(R"( + key: old_value + )"); + YAML::Node additional = YAML::Load(R"( + key: new_value + )"); + + MergeYamlNodes(base, additional); + + EXPECT_EQ(base["key"].as(), "new_value"); +} + +int main(int argc, char** argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}