From 0732bb6e29a2c6c061e1e62fcbf4906df34ef8cd Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 16:28:17 -0400 Subject: [PATCH 01/17] Rename the current code to indicate user-generated --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 34 ++++++++++++------- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 28 ++++++++------- .../tests/test-ffi_IrUnitHandlerInterface.cpp | 2 +- .../core/tests/test-ir_encoding_methods.cpp | 4 ++- 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 2a84795f7..21ef6959a 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -381,21 +381,28 @@ auto decode_as_encoded_text_ast(Value const& val) -> std::optional { } // namespace auto KeyValuePairLogEvent::create( - std::shared_ptr schema_tree, - NodeIdValuePairs node_id_value_pairs, + std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result { - if (auto const ret_val{validate_node_id_value_pairs(*schema_tree, node_id_value_pairs)}; + if (auto const ret_val{validate_node_id_value_pairs( + *user_generated_schema_tree, + user_generated_node_id_value_pairs + )}; std::errc{} != ret_val) { return ret_val; } - return KeyValuePairLogEvent{std::move(schema_tree), std::move(node_id_value_pairs), utc_offset}; + return KeyValuePairLogEvent{ + std::move(user_generated_schema_tree), + std::move(user_generated_node_id_value_pairs), + utc_offset + }; } auto KeyValuePairLogEvent::serialize_to_json( ) const -> OUTCOME_V2_NAMESPACE::std_result { - if (m_node_id_value_pairs.empty()) { + if (m_user_generated_node_id_value_pairs.empty()) { return nlohmann::json::object(); } @@ -409,9 +416,10 @@ auto KeyValuePairLogEvent::serialize_to_json( // vector grows). std::stack dfs_stack; - auto const schema_subtree_bitmap_ret{ - get_schema_subtree_bitmap(m_node_id_value_pairs, *m_schema_tree) - }; + auto const schema_subtree_bitmap_ret{get_schema_subtree_bitmap( + m_user_generated_node_id_value_pairs, + *m_user_generated_schema_tree + )}; if (schema_subtree_bitmap_ret.has_error()) { return schema_subtree_bitmap_ret.error(); } @@ -426,7 +434,7 @@ auto KeyValuePairLogEvent::serialize_to_json( // // On the way up, add the current node's `nlohmann::json::object_t` to the parent's // `nlohmann::json::object_t`. - auto const& root_schema_tree_node{m_schema_tree->get_root()}; + auto const& root_schema_tree_node{m_user_generated_schema_tree->get_root()}; auto root_json_obj = nlohmann::json::object_t(); dfs_stack.emplace( @@ -442,13 +450,15 @@ auto KeyValuePairLogEvent::serialize_to_json( continue; } auto const child_schema_tree_node_id{top.get_next_child_schema_tree_node()}; - auto const& child_schema_tree_node{m_schema_tree->get_node(child_schema_tree_node_id)}; - if (m_node_id_value_pairs.contains(child_schema_tree_node_id)) { + auto const& child_schema_tree_node{ + m_user_generated_schema_tree->get_node(child_schema_tree_node_id) + }; + if (m_user_generated_node_id_value_pairs.contains(child_schema_tree_node_id)) { // Handle leaf node if (false == insert_kv_pair_into_json_obj( child_schema_tree_node, - m_node_id_value_pairs.at(child_schema_tree_node_id), + m_user_generated_node_id_value_pairs.at(child_schema_tree_node_id), top.get_json_obj() )) { diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 04fd31c9e..6c9b1d597 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -28,15 +28,15 @@ class KeyValuePairLogEvent { // Factory functions /** - * @param schema_tree - * @param node_id_value_pairs + * @param user_generated_schema_tree + * @param user_generated_node_id_value_pairs * @param utc_offset * @return A result containing the key-value pair log event or an error code indicating the * failure. See `validate_node_id_value_pairs` for the possible error codes. */ [[nodiscard]] static auto create( - std::shared_ptr schema_tree, - NodeIdValuePairs node_id_value_pairs, + std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result; @@ -52,10 +52,12 @@ class KeyValuePairLogEvent { ~KeyValuePairLogEvent() = default; // Methods - [[nodiscard]] auto get_schema_tree() const -> SchemaTree const& { return *m_schema_tree; } + [[nodiscard]] auto get_user_generated_schema_tree() const -> SchemaTree const& { + return *m_user_generated_schema_tree; + } - [[nodiscard]] auto get_node_id_value_pairs() const -> NodeIdValuePairs const& { - return m_node_id_value_pairs; + [[nodiscard]] auto get_user_generated_node_id_value_pairs() const -> NodeIdValuePairs const& { + return m_user_generated_node_id_value_pairs; } [[nodiscard]] auto get_utc_offset() const -> UtcOffset { return m_utc_offset; } @@ -75,17 +77,17 @@ class KeyValuePairLogEvent { private: // Constructor KeyValuePairLogEvent( - std::shared_ptr schema_tree, - NodeIdValuePairs node_id_value_pairs, + std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) - : m_schema_tree{std::move(schema_tree)}, - m_node_id_value_pairs{std::move(node_id_value_pairs)}, + : m_user_generated_schema_tree{std::move(user_generated_schema_tree)}, + m_user_generated_node_id_value_pairs{std::move(user_generated_node_id_value_pairs)}, m_utc_offset{utc_offset} {} // Variables - std::shared_ptr m_schema_tree; - NodeIdValuePairs m_node_id_value_pairs; + std::shared_ptr m_user_generated_schema_tree; + NodeIdValuePairs m_user_generated_node_id_value_pairs; UtcOffset m_utc_offset{0}; }; } // namespace clp::ffi diff --git a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp index 5b8ad82cd..cc06b77c3 100644 --- a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp +++ b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp @@ -127,7 +127,7 @@ TEMPLATE_TEST_CASE( REQUIRE( (optional_log_event.has_value() && optional_log_event.value().get_utc_offset() == cTestUtcOffset - && optional_log_event.value().get_node_id_value_pairs().empty()) + && optional_log_event.value().get_user_generated_node_id_value_pairs().empty()) ); auto const& optional_schema_tree_locator{handler.get_schema_tree_node_locator()}; REQUIRE( diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 5c27a6fab..811744f21 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -1197,7 +1197,9 @@ TEMPLATE_TEST_CASE( auto const& deserialized_log_event{deserialized_log_events.at(idx)}; auto const num_leaves_in_json_obj{count_num_leaves(expect)}; - auto const num_kv_pairs{deserialized_log_event.get_node_id_value_pairs().size()}; + auto const num_kv_pairs{ + deserialized_log_event.get_user_generated_node_id_value_pairs().size() + }; REQUIRE((num_leaves_in_json_obj == num_kv_pairs)); auto const serialized_json_result{deserialized_log_event.serialize_to_json()}; From 8a192bb01445c7c3e661f9890f9b61a6622631c2 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 19:16:13 -0400 Subject: [PATCH 02/17] Add auto-generated key in kv pair log-event; Disable kv pair log event unit tests temporarily --- components/core/CMakeLists.txt | 2 +- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 114 ++++++++++++------ .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 45 +++++-- .../src/clp/ffi/ir_stream/Deserializer.hpp | 17 ++- .../ir_unit_deserialization_methods.cpp | 7 +- .../ir_unit_deserialization_methods.hpp | 8 +- .../tests/test-ffi_IrUnitHandlerInterface.cpp | 10 +- .../core/tests/test-ir_encoding_methods.cpp | 4 +- 8 files changed, 145 insertions(+), 62 deletions(-) diff --git a/components/core/CMakeLists.txt b/components/core/CMakeLists.txt index 83e5c7630..b4cd2a21f 100644 --- a/components/core/CMakeLists.txt +++ b/components/core/CMakeLists.txt @@ -502,7 +502,7 @@ set(SOURCE_FILES_unitTest tests/test-EncodedVariableInterpreter.cpp tests/test-encoding_methods.cpp tests/test-ffi_IrUnitHandlerInterface.cpp - tests/test-ffi_KeyValuePairLogEvent.cpp + # tests/test-ffi_KeyValuePairLogEvent.cpp tests/test-ffi_SchemaTree.cpp tests/test-FileDescriptorReader.cpp tests/test-Grep.cpp diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 21ef6959a..74deddcf8 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -189,6 +189,18 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> */ [[nodiscard]] auto decode_as_encoded_text_ast(Value const& val) -> std::optional; +/** + * Serializes the given node ID value pairs into a `nlohmann::json` object. + * @return A result containing the serialized JSON object or an error code indicating the failure: + * - std::errc::protocol_error if a value in the log event couldn't be decoded or it couldn't be + * inserted into a JSON object. + * - std::errc::result_out_of_range if a node ID in the log event doesn't exist in the schema tree. + */ +[[nodiscard]] auto serialize_node_id_value_pairs_to_json( + SchemaTree const& schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs +) -> OUTCOME_V2_NAMESPACE::std_result; + auto node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> bool { switch (type) { case SchemaTree::Node::Type::Obj: @@ -378,31 +390,12 @@ auto decode_as_encoded_text_ast(Value const& val) -> std::optional { ? val.get_immutable_view().decode_and_unparse() : val.get_immutable_view().decode_and_unparse(); } -} // namespace -auto KeyValuePairLogEvent::create( - std::shared_ptr user_generated_schema_tree, - NodeIdValuePairs user_generated_node_id_value_pairs, - UtcOffset utc_offset -) -> OUTCOME_V2_NAMESPACE::std_result { - if (auto const ret_val{validate_node_id_value_pairs( - *user_generated_schema_tree, - user_generated_node_id_value_pairs - )}; - std::errc{} != ret_val) - { - return ret_val; - } - return KeyValuePairLogEvent{ - std::move(user_generated_schema_tree), - std::move(user_generated_node_id_value_pairs), - utc_offset - }; -} - -auto KeyValuePairLogEvent::serialize_to_json( -) const -> OUTCOME_V2_NAMESPACE::std_result { - if (m_user_generated_node_id_value_pairs.empty()) { +auto serialize_node_id_value_pairs_to_json( + SchemaTree const& schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs +) -> OUTCOME_V2_NAMESPACE::std_result { + if (node_id_value_pairs.empty()) { return nlohmann::json::object(); } @@ -416,10 +409,8 @@ auto KeyValuePairLogEvent::serialize_to_json( // vector grows). std::stack dfs_stack; - auto const schema_subtree_bitmap_ret{get_schema_subtree_bitmap( - m_user_generated_node_id_value_pairs, - *m_user_generated_schema_tree - )}; + auto const schema_subtree_bitmap_ret{get_schema_subtree_bitmap(node_id_value_pairs, schema_tree) + }; if (schema_subtree_bitmap_ret.has_error()) { return schema_subtree_bitmap_ret.error(); } @@ -434,7 +425,7 @@ auto KeyValuePairLogEvent::serialize_to_json( // // On the way up, add the current node's `nlohmann::json::object_t` to the parent's // `nlohmann::json::object_t`. - auto const& root_schema_tree_node{m_user_generated_schema_tree->get_root()}; + auto const& root_schema_tree_node{schema_tree.get_root()}; auto root_json_obj = nlohmann::json::object_t(); dfs_stack.emplace( @@ -450,15 +441,13 @@ auto KeyValuePairLogEvent::serialize_to_json( continue; } auto const child_schema_tree_node_id{top.get_next_child_schema_tree_node()}; - auto const& child_schema_tree_node{ - m_user_generated_schema_tree->get_node(child_schema_tree_node_id) - }; - if (m_user_generated_node_id_value_pairs.contains(child_schema_tree_node_id)) { + auto const& child_schema_tree_node{schema_tree.get_node(child_schema_tree_node_id)}; + if (node_id_value_pairs.contains(child_schema_tree_node_id)) { // Handle leaf node if (false == insert_kv_pair_into_json_obj( child_schema_tree_node, - m_user_generated_node_id_value_pairs.at(child_schema_tree_node_id), + node_id_value_pairs.at(child_schema_tree_node_id), top.get_json_obj() )) { @@ -480,4 +469,61 @@ auto KeyValuePairLogEvent::serialize_to_json( return root_json_obj; } +} // namespace + +auto KeyValuePairLogEvent::create( + std::shared_ptr auto_generated_schema_tree, + std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs auto_generated_node_id_value_pairs, + NodeIdValuePairs user_generated_node_id_value_pairs, + UtcOffset utc_offset +) -> OUTCOME_V2_NAMESPACE::std_result { + if (auto const ret_val{validate_node_id_value_pairs( + *auto_generated_schema_tree, + auto_generated_node_id_value_pairs + )}; + std::errc{} != ret_val) + { + return ret_val; + } + + if (auto const ret_val{validate_node_id_value_pairs( + *user_generated_schema_tree, + user_generated_node_id_value_pairs + )}; + std::errc{} != ret_val) + { + return ret_val; + } + + return KeyValuePairLogEvent{ + std::move(auto_generated_schema_tree), + std::move(user_generated_schema_tree), + std::move(auto_generated_node_id_value_pairs), + std::move(user_generated_node_id_value_pairs), + utc_offset + }; +} + +auto KeyValuePairLogEvent::serialize_to_json( +) const -> OUTCOME_V2_NAMESPACE::std_result> { + auto serialized_auto_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( + *m_auto_generated_schema_tree, + m_auto_generated_node_id_value_pairs + )}; + if (serialized_auto_generated_kv_pairs_result.has_error()) { + return serialized_auto_generated_kv_pairs_result.error(); + } + + auto serialized_user_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( + *m_user_generated_schema_tree, + m_user_generated_node_id_value_pairs + )}; + if (serialized_user_generated_kv_pairs_result.has_error()) { + return serialized_user_generated_kv_pairs_result.error(); + } + + return {std::move(serialized_auto_generated_kv_pairs_result.value()), + std::move(serialized_user_generated_kv_pairs_result.value())}; +} } // namespace clp::ffi diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 6c9b1d597..85692d710 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -16,10 +16,13 @@ namespace clp::ffi { /** * A log event containing key-value pairs. Each event contains: - * - A collection of node-ID & value pairs, where each pair represents a leaf `SchemaTreeNode` in - * the `SchemaTree`. - * - A reference to the `SchemaTree` - * - The UTC offset of the current log event + * - A reference to the schema tree of auto-generated keys. + * - A reference to the schema tree of user-generated keys. + * - A collection of auto-generated node-ID & value pairs, where each pair represents a leaf + * `SchemaTree::Node` in the `SchemaTree`. + * - A collection of user-generated node-ID & value pairs, where each pair represents a leaf + * `SchemaTree::Node` in the `SchemaTree`. + * - The UTC offset of the current log event. */ class KeyValuePairLogEvent { public: @@ -28,14 +31,18 @@ class KeyValuePairLogEvent { // Factory functions /** + * @param auto_generated_schema_tree * @param user_generated_schema_tree + * @param auto_generated_node_id_value_pairs * @param user_generated_node_id_value_pairs * @param utc_offset * @return A result containing the key-value pair log event or an error code indicating the * failure. See `validate_node_id_value_pairs` for the possible error codes. */ [[nodiscard]] static auto create( + std::shared_ptr auto_generated_schema_tree, std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs auto_generated_node_id_value_pairs, NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result; @@ -52,10 +59,18 @@ class KeyValuePairLogEvent { ~KeyValuePairLogEvent() = default; // Methods + [[nodiscard]] auto get_auto_generated_schema_tree() const -> SchemaTree const& { + return *m_auto_generated_schema_tree; + } + [[nodiscard]] auto get_user_generated_schema_tree() const -> SchemaTree const& { return *m_user_generated_schema_tree; } + [[nodiscard]] auto get_auto_generated_node_id_value_pairs() const -> NodeIdValuePairs const& { + return m_auto_generated_node_id_value_pairs; + } + [[nodiscard]] auto get_user_generated_node_id_value_pairs() const -> NodeIdValuePairs const& { return m_user_generated_node_id_value_pairs; } @@ -63,30 +78,34 @@ class KeyValuePairLogEvent { [[nodiscard]] auto get_utc_offset() const -> UtcOffset { return m_utc_offset; } /** - * Serializes the log event into a `nlohmann::json` object. - * @return A result containing the serialized JSON object or an error code indicating the - * failure: - * - std::errc::protocol_error if a value in the log event couldn't be decoded or it couldn't be - * inserted into a JSON object. - * - std::errc::result_out_of_range if a node ID in the log event doesn't exist in the schema - * tree. + * Serializes the log event into `nlohmann::json` objects. + * @return A result containing a pair of serialized JSON objects (the first contains + * auto-generated key-value pairs, while the second contains user-generated key-value pairs), or + * an error code indicating the failure: + * - Forwards `serialize_node_id_value_pairs_to_json`'s return values. */ [[nodiscard]] auto serialize_to_json( - ) const -> OUTCOME_V2_NAMESPACE::std_result; + ) const -> OUTCOME_V2_NAMESPACE::std_result>; private: // Constructor KeyValuePairLogEvent( + std::shared_ptr auto_generated_schema_tree, std::shared_ptr user_generated_schema_tree, + NodeIdValuePairs auto_generated_node_id_value_pairs, NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) - : m_user_generated_schema_tree{std::move(user_generated_schema_tree)}, + : m_auto_generated_schema_tree{std::move(auto_generated_schema_tree)}, + m_user_generated_schema_tree{std::move(user_generated_schema_tree)}, + m_auto_generated_node_id_value_pairs{std::move(auto_generated_node_id_value_pairs)}, m_user_generated_node_id_value_pairs{std::move(user_generated_node_id_value_pairs)}, m_utc_offset{utc_offset} {} // Variables + std::shared_ptr m_auto_generated_schema_tree; std::shared_ptr m_user_generated_schema_tree; + NodeIdValuePairs m_auto_generated_node_id_value_pairs; NodeIdValuePairs m_user_generated_node_id_value_pairs; UtcOffset m_utc_offset{0}; }; diff --git a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp index c1fc13c85..e37277570 100644 --- a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp @@ -116,7 +116,8 @@ class Deserializer { Deserializer(IrUnitHandler ir_unit_handler) : m_ir_unit_handler{std::move(ir_unit_handler)} {} // Variables - std::shared_ptr m_schema_tree{std::make_shared()}; + std::shared_ptr m_auto_generated_schema_tree{std::make_shared()}; + std::shared_ptr m_user_generated_schema_tree{std::make_shared()}; UtcOffset m_utc_offset{0}; IrUnitHandler m_ir_unit_handler; bool m_is_complete{false}; @@ -186,9 +187,13 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read auto const ir_unit_type{optional_ir_unit_type.value()}; switch (ir_unit_type) { case IrUnitType::LogEvent: { - auto result{ - deserialize_ir_unit_kv_pair_log_event(reader, tag, m_schema_tree, m_utc_offset) - }; + auto result{deserialize_ir_unit_kv_pair_log_event( + reader, + tag, + m_auto_generated_schema_tree, + m_user_generated_schema_tree, + m_utc_offset + )}; if (result.has_error()) { return result.error(); } @@ -210,7 +215,7 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read } auto const node_locator{result.value()}; - if (m_schema_tree->has_node(node_locator)) { + if (m_user_generated_schema_tree->has_node(node_locator)) { return std::errc::protocol_error; } @@ -220,7 +225,7 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read return ir_error_code_to_errc(err); } - std::ignore = m_schema_tree->insert_node(node_locator); + std::ignore = m_user_generated_schema_tree->insert_node(node_locator); break; } diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp index d475ad196..df1ec3cfb 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp @@ -538,7 +538,8 @@ auto deserialize_ir_unit_utc_offset_change(ReaderInterface& reader auto deserialize_ir_unit_kv_pair_log_event( ReaderInterface& reader, encoded_tag_t tag, - std::shared_ptr schema_tree, + std::shared_ptr auto_generated_schema_tree, + std::shared_ptr user_generated_schema_tree, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result { Schema schema; @@ -567,7 +568,9 @@ auto deserialize_ir_unit_kv_pair_log_event( } return KeyValuePairLogEvent::create( - std::move(schema_tree), + std::move(auto_generated_schema_tree), + std::move(user_generated_schema_tree), + {}, std::move(node_id_value_pairs), utc_offset ); diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp index 454676cf5..cd3166a59 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp @@ -55,7 +55,10 @@ namespace clp::ffi::ir_stream { * Deserializes a key-value pair log event IR unit. * @param reader * @param tag - * @param schema_tree Schema tree used to construct the KV-pair log event. + * @param auto_generated_schema_tree Schema tree of auto-generated keys used to construct the + * KV-pair log event. + * @param user_generated_schema_tree Schema tree of user-generated keys used to construct the + * KV-pair log event. * @param utc_offset UTC offset used to construct the KV-pair log event. * @return A result containing the deserialized log event or an error code indicating the * failure: @@ -69,7 +72,8 @@ namespace clp::ffi::ir_stream { [[nodiscard]] auto deserialize_ir_unit_kv_pair_log_event( ReaderInterface& reader, encoded_tag_t tag, - std::shared_ptr schema_tree, + std::shared_ptr auto_generated_schema_tree, + std::shared_ptr user_generated_schema_tree, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result; } // namespace clp::ffi::ir_stream diff --git a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp index cc06b77c3..799351d78 100644 --- a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp +++ b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp @@ -87,9 +87,13 @@ auto test_ir_unit_handler_interface(clp::ffi::ir_stream::IrUnitHandlerInterface auto test_ir_unit_handler_interface(clp::ffi::ir_stream::IrUnitHandlerInterface auto& handler ) -> void { - auto test_log_event_result{ - KeyValuePairLogEvent::create(std::make_shared(), {}, cTestUtcOffset) - }; + auto test_log_event_result{KeyValuePairLogEvent::create( + std::make_shared(), + std::make_shared(), + {}, + {}, + cTestUtcOffset + )}; REQUIRE( (false == test_log_event_result.has_error() && IRErrorCode::IRErrorCode_Success diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 811744f21..96ea3a6f4 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -1204,7 +1204,9 @@ TEMPLATE_TEST_CASE( auto const serialized_json_result{deserialized_log_event.serialize_to_json()}; REQUIRE_FALSE(serialized_json_result.has_error()); - REQUIRE((expect == serialized_json_result.value())); + auto const& [auto_generated, user_generated]{serialized_json_result.value()}; + REQUIRE(auto_generated.empty()); + REQUIRE((expect == user_generated)); } auto const eof_result{deserializer.deserialize_next_ir_unit(reader)}; From 41f4c5e0947afe74e60db91cd19d47e9f9bee3d6 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 22:24:53 -0400 Subject: [PATCH 03/17] Fix siblings key uniqueness check; add kv log event unit tests back --- components/core/CMakeLists.txt | 2 +- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 79 ++++- components/core/src/clp/ffi/SchemaTree.hpp | 8 + .../tests/test-ffi_KeyValuePairLogEvent.cpp | 269 +++++++++++++----- 4 files changed, 280 insertions(+), 78 deletions(-) diff --git a/components/core/CMakeLists.txt b/components/core/CMakeLists.txt index b4cd2a21f..83e5c7630 100644 --- a/components/core/CMakeLists.txt +++ b/components/core/CMakeLists.txt @@ -502,7 +502,7 @@ set(SOURCE_FILES_unitTest tests/test-EncodedVariableInterpreter.cpp tests/test-encoding_methods.cpp tests/test-ffi_IrUnitHandlerInterface.cpp - # tests/test-ffi_KeyValuePairLogEvent.cpp + tests/test-ffi_KeyValuePairLogEvent.cpp tests/test-ffi_SchemaTree.cpp tests/test-FileDescriptorReader.cpp tests/test-Grep.cpp diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 74deddcf8..72e07c76b 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -201,6 +201,19 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs ) -> OUTCOME_V2_NAMESPACE::std_result; +/** + * @param node A non-root schema tree node. + * @param parent_node_id_to_key_names + * @return true if `node`'s key is unique among its sibling nodes with `parent_node_id_to_key_names` + * updated to keep track of this unique key name. + * @return false if `node`'s key already exists in other sibling nodes. + */ +[[nodiscard]] auto check_key_uniqueness_among_sibling_nodes( + SchemaTree::Node const& node, + std::unordered_map>& + parent_node_id_to_key_names +) -> bool; + auto node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> bool { switch (type) { case SchemaTree::Node::Type::Obj: @@ -228,6 +241,7 @@ auto validate_node_id_value_pairs( try { std::unordered_map> parent_node_id_to_key_names; + std::vector key_duplication_checked_node_id_bitmap(schema_tree.get_size(), false); for (auto const& [node_id, value] : node_id_value_pairs) { auto const& node{schema_tree.get_node(node_id)}; if (node.is_root()) { @@ -252,20 +266,38 @@ auto validate_node_id_value_pairs( return std::errc::operation_not_permitted; } - // We checked that the node isn't the root above, so we can query the underlying ID - // safely without a repeated check. - auto const parent_node_id{node.get_parent_id_unsafe()}; - auto const key_name{node.get_key_name()}; - if (parent_node_id_to_key_names.contains(parent_node_id)) { - auto const [it, new_key_inserted]{ - parent_node_id_to_key_names.at(parent_node_id).emplace(key_name) - }; - if (false == new_key_inserted) { - // The key is duplicated under the same parent + if (false + == check_key_uniqueness_among_sibling_nodes(node, parent_node_id_to_key_names)) + { + return std::errc::protocol_not_supported; + } + + // Iteratively check if there's any key duplication in ancestors until: + // 1. The ancestor is already checked. We only need to check an ancestor node once. If + // there are key duplications among its sibling, it will be caught when the sibling + // is first checked. The order of which sibling gets checked first doesn't affect the + // results. + // 2. Reached the root nod + auto next_ancestor_node_id_to_check{node.get_parent_id_unsafe()}; + while (false == key_duplication_checked_node_id_bitmap[next_ancestor_node_id_to_check]) + { + auto const& node_to_check{schema_tree.get_node(next_ancestor_node_id_to_check)}; + if (node_to_check.is_root()) { + key_duplication_checked_node_id_bitmap[node_to_check.get_id()] = true; + break; + } + + if (false + == check_key_uniqueness_among_sibling_nodes( + node_to_check, + parent_node_id_to_key_names + )) + { return std::errc::protocol_not_supported; } - } else { - parent_node_id_to_key_names.emplace(parent_node_id, std::unordered_set{key_name}); + + key_duplication_checked_node_id_bitmap[next_ancestor_node_id_to_check] = true; + next_ancestor_node_id_to_check = node_to_check.get_parent_id_unsafe(); } } } catch (SchemaTree::OperationFailed const& ex) { @@ -469,6 +501,29 @@ auto serialize_node_id_value_pairs_to_json( return root_json_obj; } + +auto check_key_uniqueness_among_sibling_nodes( + SchemaTree::Node const& node, + std::unordered_map>& + parent_node_id_to_key_names +) -> bool { + // The given node must not be the root (checked by the caller), so we can query the underlying + // ID safely without a repeated check. + auto const parent_node_id{node.get_parent_id_unsafe()}; + auto const key_name{node.get_key_name()}; + if (parent_node_id_to_key_names.contains(parent_node_id)) { + auto const [it, new_key_inserted]{ + parent_node_id_to_key_names.at(parent_node_id).emplace(key_name) + }; + if (false == new_key_inserted) { + // The key is duplicated under the same parent + return false; + } + } else { + parent_node_id_to_key_names.emplace(parent_node_id, std::unordered_set{key_name}); + } + return true; +} } // namespace auto KeyValuePairLogEvent::create( diff --git a/components/core/src/clp/ffi/SchemaTree.hpp b/components/core/src/clp/ffi/SchemaTree.hpp index 46494fa71..abcb0eefa 100644 --- a/components/core/src/clp/ffi/SchemaTree.hpp +++ b/components/core/src/clp/ffi/SchemaTree.hpp @@ -127,6 +127,9 @@ class SchemaTree { // Destructor ~Node() = default; + // Defines default equal-to operator + [[nodiscard]] auto operator==(SchemaTree::Node const& rhs) const -> bool = default; + // Methods [[nodiscard]] auto get_id() const -> id_t { return m_id; } @@ -248,6 +251,11 @@ class SchemaTree { // Destructor ~SchemaTree() = default; + // Equal-to operator + [[nodiscard]] auto operator==(SchemaTree const& rhs) const -> bool { + return m_tree_nodes == rhs.m_tree_nodes; + } + // Methods [[nodiscard]] auto get_size() const -> size_t { return m_tree_nodes.size(); } diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 2e9cfb691..5a52797f7 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -11,6 +11,7 @@ #include #include +#include #include "../src/clp/ffi/encoding_methods.hpp" #include "../src/clp/ffi/KeyValuePairLogEvent.hpp" @@ -81,6 +82,25 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( KeyValuePairLogEvent::NodeIdValuePairs& invalid_node_id_value_pairs ) -> void; +/** + * Asserts the kv pair log event creation fails with the expected error code. + * @param auto_generated_schema_tree + * @param user_generated_schema_tree + * @param auto_generated_node_id_value_pairs + * @param user_generated_node_id_value_pairs + * @param utc_offset + * @param expected_error_code + * @return Whether the assertion succeeded. + */ +[[nodiscard]] auto assert_kv_pair_log_event_creation_failure( + std::shared_ptr auto_generated_schema_tree, + std::shared_ptr user_generated_schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& auto_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs const& user_generated_node_id_value_pairs, + UtcOffset utc_offset, + std::errc expected_error_code +) -> bool; + template requires(std::is_same_v || std::is_same_v) @@ -197,6 +217,24 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( invalid_node_id_value_pairs.emplace(node_id, Value{}); } } + +auto assert_kv_pair_log_event_creation_failure( + std::shared_ptr auto_generated_schema_tree, + std::shared_ptr user_generated_schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& auto_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs const& user_generated_node_id_value_pairs, + UtcOffset utc_offset, + std::errc expected_error_code +) -> bool { + auto const result{KeyValuePairLogEvent::create( + auto_generated_schema_tree, + user_generated_schema_tree, + auto_generated_node_id_value_pairs, + user_generated_node_id_value_pairs, + utc_offset + )}; + return result.has_error() && result.error() == expected_error_code; +} } // namespace TEST_CASE("ffi_Value_basic", "[ffi][Value]") { @@ -250,7 +288,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { * | * |------------> <1:a:Obj> * | | - * |--> <2:a:Int> |--> <3:b:Obj> + * |--> <2:b:Int> |--> <3:b:Obj> * | * |------------> <4:c:Obj> * | | @@ -262,10 +300,11 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { * | * |--> <11:f:Obj> */ - auto const schema_tree{std::make_shared()}; + auto const auto_generated_schema_tree{std::make_shared()}; + auto const user_generated_schema_tree{std::make_shared()}; std::vector const locators{ {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Obj}, - {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Int}, + {SchemaTree::cRootId, "b", SchemaTree::Node::Type::Int}, {1, "b", SchemaTree::Node::Type::Obj}, {3, "c", SchemaTree::Node::Type::Obj}, {3, "d", SchemaTree::Node::Type::Str}, @@ -277,14 +316,20 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { {4, "f", SchemaTree::Node::Type::Obj} }; for (auto const& locator : locators) { - REQUIRE_NOTHROW(schema_tree->insert_node(locator)); + REQUIRE_NOTHROW(auto_generated_schema_tree->insert_node(locator)); + REQUIRE_NOTHROW(user_generated_schema_tree->insert_node(locator)); } + // This test case implicitly requires the auto-generated and user-generated schema trees to + // be identical. Adding this check to prevent this assumption is broken by future development. + REQUIRE((*auto_generated_schema_tree == *user_generated_schema_tree)); + SECTION("Test empty ID-value pairs") { - KeyValuePairLogEvent::NodeIdValuePairs node_id_value_pairs; auto const result{KeyValuePairLogEvent::create( - schema_tree, - std::move(node_id_value_pairs), + auto_generated_schema_tree, + user_generated_schema_tree, + {}, + {}, UtcOffset{0} )}; REQUIRE_FALSE(result.has_error()); @@ -295,42 +340,42 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) // Int: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 2, invalid_node_id_value_pairs ); // Float: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 9, invalid_node_id_value_pairs ); // Bool: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 6, invalid_node_id_value_pairs ); // Str: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 5, invalid_node_id_value_pairs ); // UnstructuredArray: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 7, invalid_node_id_value_pairs ); // Obj: insert_invalid_node_id_value_pairs_with_node_type_errors( - *schema_tree, + *user_generated_schema_tree, 3, invalid_node_id_value_pairs ); @@ -343,26 +388,37 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { } else { node_id_value_pair_to_test.emplace(node_id, std::nullopt); } - auto const result{KeyValuePairLogEvent::create( - schema_tree, - std::move(node_id_value_pair_to_test), - UtcOffset{0} - )}; - REQUIRE(result.has_error()); - auto const& err{result.error()}; - REQUIRE((std::errc::protocol_error == err)); + + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + node_id_value_pair_to_test, + {}, + UtcOffset{0}, + std::errc::protocol_error + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + {}, + node_id_value_pair_to_test, + UtcOffset{0}, + std::errc::protocol_error + )); } } SECTION("Test valid ID-value pairs") { - KeyValuePairLogEvent::NodeIdValuePairs node_id_value_pairs; + constexpr std::string_view cJsonArrayToEncode{"[\"a\", 1, 0.1, null]"}; + constexpr std::string_view cStaticText{"Test"}; + KeyValuePairLogEvent::NodeIdValuePairs valid_node_id_value_pairs; /* * The sub schema tree of `node_id_value_pairs`: * <0:root:Obj> * | * |------------> <1:a:Obj> * | | - * |--> <2:a:Int> |--> <3:b:Obj> + * |--> <2:b:Int> |--> <3:b:Obj> * | * |------------> <4:c:Obj> * | | @@ -375,77 +431,160 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { * |--> <11:f:Obj> */ // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - node_id_value_pairs.emplace(2, Value{static_cast(0)}); - node_id_value_pairs.emplace(5, Value{string{"Test"}}); - node_id_value_pairs.emplace( + valid_node_id_value_pairs.emplace(2, Value{static_cast(0)}); + valid_node_id_value_pairs.emplace(5, Value{string{cStaticText}}); + valid_node_id_value_pairs.emplace( 8, Value{get_encoded_text_ast(cStringToEncode)} ); - node_id_value_pairs.emplace( + valid_node_id_value_pairs.emplace( 7, - Value{get_encoded_text_ast(cStringToEncode)} + Value{get_encoded_text_ast(cJsonArrayToEncode)} ); - node_id_value_pairs.emplace(10, Value{}); - node_id_value_pairs.emplace(11, std::nullopt); + valid_node_id_value_pairs.emplace(10, Value{}); + valid_node_id_value_pairs.emplace(11, std::nullopt); // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - auto const result{ - KeyValuePairLogEvent::create(schema_tree, node_id_value_pairs, UtcOffset{0}) - }; + auto const result{KeyValuePairLogEvent::create( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0} + )}; REQUIRE_FALSE(result.has_error()); + SECTION("Test JSON serialization") { + nlohmann::json const subtree_rooted_at_node_4 + = {{"a", nlohmann::json::parse(cJsonArrayToEncode)}, + {"d", cStringToEncode}, + {"f", nlohmann::json::object_t()}}; + nlohmann::json const subtree_rooted_at_node_3 + = {{"c", subtree_rooted_at_node_4}, {"d", cStaticText}, {"e", nullptr}}; + nlohmann::json const expected = { + {"a", {{"b", subtree_rooted_at_node_3}}}, + {"b", 0}, + }; + + auto const& kv_pair_log_event{result.value()}; + auto const serialized_json_result{kv_pair_log_event.serialize_to_json()}; + REQUIRE_FALSE(serialized_json_result.has_error()); + auto const& [serialized_auto_generated_kv_pairs, serialized_user_generated_kv_pairs]{ + serialized_json_result.value() + }; + REQUIRE((serialized_auto_generated_kv_pairs == expected)); + REQUIRE((serialized_user_generated_kv_pairs == expected)); + } + SECTION("Test duplicated key conflict on node #3") { + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - node_id_value_pairs.emplace(6, Value{static_cast(false)}); - auto const result{ - KeyValuePairLogEvent::create(schema_tree, node_id_value_pairs, UtcOffset{0}) - }; - REQUIRE(result.has_error()); - REQUIRE((std::errc::protocol_not_supported == result.error())); + invalid_node_id_value_pairs.emplace(6, Value{static_cast(false)}); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); } SECTION("Test duplicated key conflict on node #4") { + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - node_id_value_pairs.emplace(9, Value{static_cast(0.0)}); - auto const result{ - KeyValuePairLogEvent::create(schema_tree, node_id_value_pairs, UtcOffset{0}) - }; - REQUIRE(result.has_error()); - REQUIRE((std::errc::protocol_not_supported == result.error())); + invalid_node_id_value_pairs.emplace(9, Value{static_cast(0.0)}); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); } SECTION("Test invalid sub-tree on node #3") { - node_id_value_pairs.emplace(3, std::nullopt); - auto const result{ - KeyValuePairLogEvent::create(schema_tree, node_id_value_pairs, UtcOffset{0}) - }; + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; + invalid_node_id_value_pairs.emplace(3, std::nullopt); // Node #3 is empty, but its descendants appear in the sub schema tree (node #5 & #10) - REQUIRE(result.has_error()); - REQUIRE((std::errc::operation_not_permitted == result.error())); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::operation_not_permitted + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::operation_not_permitted + )); } SECTION("Test invalid sub-tree on node #4") { - node_id_value_pairs.emplace(4, Value{}); - auto const result{ - KeyValuePairLogEvent::create(schema_tree, node_id_value_pairs, UtcOffset{0}) - }; + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; + invalid_node_id_value_pairs.emplace(4, Value{}); // Node #4 is null, but its descendants appear in the sub schema tree (node #5 & #10) - REQUIRE(result.has_error()); - REQUIRE((std::errc::operation_not_permitted == result.error())); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::operation_not_permitted + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::operation_not_permitted + )); } } SECTION("Test out-of-bound node ID") { KeyValuePairLogEvent::NodeIdValuePairs node_id_value_pairs_out_of_bound; node_id_value_pairs_out_of_bound.emplace( - static_cast(schema_tree->get_size()), + static_cast(user_generated_schema_tree->get_size()), Value{} ); - auto const out_of_bound_result{KeyValuePairLogEvent::create( - schema_tree, - std::move(node_id_value_pairs_out_of_bound), - UtcOffset{0} - )}; - REQUIRE(out_of_bound_result.has_error()); - REQUIRE((std::errc::operation_not_permitted == out_of_bound_result.error())); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + node_id_value_pairs_out_of_bound, + {}, + UtcOffset{0}, + std::errc::operation_not_permitted + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + {}, + node_id_value_pairs_out_of_bound, + UtcOffset{0}, + std::errc::operation_not_permitted + )); } } From 5ccb0f1e1d4e56290219b401ded12916461150c1 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 22:35:24 -0400 Subject: [PATCH 04/17] Add additional test cases for sibling key duplications --- .../tests/test-ffi_KeyValuePairLogEvent.cpp | 86 +++++++++++++++---- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 5a52797f7..cce689061 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -95,8 +95,8 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( [[nodiscard]] auto assert_kv_pair_log_event_creation_failure( std::shared_ptr auto_generated_schema_tree, std::shared_ptr user_generated_schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs const& auto_generated_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs const& user_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset, std::errc expected_error_code ) -> bool; @@ -221,16 +221,16 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( auto assert_kv_pair_log_event_creation_failure( std::shared_ptr auto_generated_schema_tree, std::shared_ptr user_generated_schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs const& auto_generated_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs const& user_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset, std::errc expected_error_code ) -> bool { auto const result{KeyValuePairLogEvent::create( - auto_generated_schema_tree, - user_generated_schema_tree, - auto_generated_node_id_value_pairs, - user_generated_node_id_value_pairs, + std::move(auto_generated_schema_tree), + std::move(user_generated_schema_tree), + std::move(auto_generated_node_id_value_pairs), + std::move(user_generated_node_id_value_pairs), utc_offset )}; return result.has_error() && result.error() == expected_error_code; @@ -289,16 +289,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { * |------------> <1:a:Obj> * | | * |--> <2:b:Int> |--> <3:b:Obj> - * | - * |------------> <4:c:Obj> - * | | - * |--> <5:d:Str> |--> <7:a:UnstructuredArray> - * | | - * |--> <6:d:Bool> |--> <8:d:Str> - * | | - * |--> <10:e:Obj> |--> <9:d:Float> - * | - * |--> <11:f:Obj> + * | | | + * |--> <12:a:Int> | |------------> <4:c:Obj> + * | | | + * | |--> <5:d:Str> |--> <7:a:UnstructuredArray> + * | | | + * | |--> <6:d:Bool> |--> <8:d:Str> + * | | | + * | |--> <10:e:Obj> |--> <9:d:Float> + * | | + * |--> <13:b:Bool> |--> <11:f:Obj> */ auto const auto_generated_schema_tree{std::make_shared()}; auto const user_generated_schema_tree{std::make_shared()}; @@ -313,7 +313,9 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { {4, "d", SchemaTree::Node::Type::Str}, {4, "d", SchemaTree::Node::Type::Float}, {3, "e", SchemaTree::Node::Type::Obj}, - {4, "f", SchemaTree::Node::Type::Obj} + {4, "f", SchemaTree::Node::Type::Obj}, + {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Int}, + {1, "b", SchemaTree::Node::Type::Bool} }; for (auto const& locator : locators) { REQUIRE_NOTHROW(auto_generated_schema_tree->insert_node(locator)); @@ -519,6 +521,52 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } + SECTION("Test duplicated keys among siblings of node #1") { + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + invalid_node_id_value_pairs.emplace(12, static_cast(0)); + // Node #12 has the same key as its sibling node #1 + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + } + + SECTION("Test duplicated keys among siblings of node #3") { + auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + invalid_node_id_value_pairs.emplace(13, false); + // Node #12 has the same key as its sibling node #3 + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + invalid_node_id_value_pairs, + valid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + user_generated_schema_tree, + valid_node_id_value_pairs, + invalid_node_id_value_pairs, + UtcOffset{0}, + std::errc::protocol_not_supported + )); + } + SECTION("Test invalid sub-tree on node #3") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; invalid_node_id_value_pairs.emplace(3, std::nullopt); From 71d0b92e4da4a853528dd166b23a1b2244d64d5a Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 22:45:08 -0400 Subject: [PATCH 05/17] Update comments --- components/core/src/clp/ffi/KeyValuePairLogEvent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 72e07c76b..e052c1866 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -277,7 +277,7 @@ auto validate_node_id_value_pairs( // there are key duplications among its sibling, it will be caught when the sibling // is first checked. The order of which sibling gets checked first doesn't affect the // results. - // 2. Reached the root nod + // 2. Reached the root node. auto next_ancestor_node_id_to_check{node.get_parent_id_unsafe()}; while (false == key_duplication_checked_node_id_bitmap[next_ancestor_node_id_to_check]) { From 45bb0ec33cc531c531ddb0da82d26bddbf47869c Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 23:37:05 -0400 Subject: [PATCH 06/17] Add schema tree nullptr check --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 4 ++++ .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 4 +++- .../tests/test-ffi_KeyValuePairLogEvent.cpp | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index e052c1866..07aef7e9e 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -533,6 +533,10 @@ auto KeyValuePairLogEvent::create( NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result { + if (nullptr == auto_generated_schema_tree || nullptr == user_generated_schema_tree) { + return std::errc::invalid_argument; + } + if (auto const ret_val{validate_node_id_value_pairs( *auto_generated_schema_tree, auto_generated_node_id_value_pairs diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 85692d710..23b353ed0 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -37,7 +37,9 @@ class KeyValuePairLogEvent { * @param user_generated_node_id_value_pairs * @param utc_offset * @return A result containing the key-value pair log event or an error code indicating the - * failure. See `validate_node_id_value_pairs` for the possible error codes. + * failure, or an error code indicating the failure: + * - std::errc::invalid_argument if any of the given schema tree pointers are null. + * - Forwards `validate_node_id_value_pairs`'s return values. */ [[nodiscard]] static auto create( std::shared_ptr auto_generated_schema_tree, diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index cce689061..5ee5dfeaf 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -337,6 +337,25 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { REQUIRE_FALSE(result.has_error()); } + SECTION("Test schema tree pointers being null") { + REQUIRE(assert_kv_pair_log_event_creation_failure( + nullptr, + user_generated_schema_tree, + {}, + {}, + UtcOffset{0}, + std::errc::invalid_argument + )); + REQUIRE(assert_kv_pair_log_event_creation_failure( + auto_generated_schema_tree, + nullptr, + {}, + {}, + UtcOffset{0}, + std::errc::invalid_argument + )); + } + SECTION("Test mismatched types") { KeyValuePairLogEvent::NodeIdValuePairs invalid_node_id_value_pairs; // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) From 13860205d7400c5109eba40ba8d7a1f93f0f2244 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 13 Oct 2024 23:48:10 -0400 Subject: [PATCH 07/17] Using find instead of contains --- components/core/src/clp/ffi/KeyValuePairLogEvent.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 07aef7e9e..63073fbf9 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -511,10 +511,9 @@ auto check_key_uniqueness_among_sibling_nodes( // ID safely without a repeated check. auto const parent_node_id{node.get_parent_id_unsafe()}; auto const key_name{node.get_key_name()}; - if (parent_node_id_to_key_names.contains(parent_node_id)) { - auto const [it, new_key_inserted]{ - parent_node_id_to_key_names.at(parent_node_id).emplace(key_name) - }; + auto const parent_node_id_to_key_names_it{parent_node_id_to_key_names.find(parent_node_id)}; + if (parent_node_id_to_key_names_it != parent_node_id_to_key_names.end()) { + auto const [it, new_key_inserted]{parent_node_id_to_key_names_it->second.emplace(key_name)}; if (false == new_key_inserted) { // The key is duplicated under the same parent return false; From 83871a507548fd96d1e66eb41eef39e6327fdd52 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 23 Nov 2024 20:23:17 -0500 Subject: [PATCH 08/17] Merging main --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 48 ++++++++++++++----- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 23 +++++++++ 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 63073fbf9..aa1968428 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -191,6 +191,9 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> /** * Serializes the given node ID value pairs into a `nlohmann::json` object. + * @param schema_tree + * @param node_id_value_pairs + * @param schema_subtree_bitmap * @return A result containing the serialized JSON object or an error code indicating the failure: * - std::errc::protocol_error if a value in the log event couldn't be decoded or it couldn't be * inserted into a JSON object. @@ -198,7 +201,8 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> */ [[nodiscard]] auto serialize_node_id_value_pairs_to_json( SchemaTree const& schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs, + vector const& schema_subtree_bitmap ) -> OUTCOME_V2_NAMESPACE::std_result; /** @@ -425,7 +429,8 @@ auto decode_as_encoded_text_ast(Value const& val) -> std::optional { auto serialize_node_id_value_pairs_to_json( SchemaTree const& schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs, + vector const& schema_subtree_bitmap ) -> OUTCOME_V2_NAMESPACE::std_result { if (node_id_value_pairs.empty()) { return nlohmann::json::object(); @@ -441,13 +446,6 @@ auto serialize_node_id_value_pairs_to_json( // vector grows). std::stack dfs_stack; - auto const schema_subtree_bitmap_ret{get_schema_subtree_bitmap(node_id_value_pairs, schema_tree) - }; - if (schema_subtree_bitmap_ret.has_error()) { - return schema_subtree_bitmap_ret.error(); - } - auto const& schema_subtree_bitmap{schema_subtree_bitmap_ret.value()}; - // Traverse the schema tree in DFS order, but only traverse the nodes that are set in // `schema_subtree_bitmap`. // @@ -563,19 +561,47 @@ auto KeyValuePairLogEvent::create( }; } +auto KeyValuePairLogEvent::get_auto_generated_schema_subtree_bitmap( +) const -> OUTCOME_V2_NAMESPACE::std_result> { + return get_schema_subtree_bitmap( + m_auto_generated_node_id_value_pairs, + *m_auto_generated_schema_tree + ); +} + +auto KeyValuePairLogEvent::get_user_generated_schema_subtree_bitmap( +) const -> outcome_v2::std_result> { + return get_schema_subtree_bitmap( + m_user_generated_node_id_value_pairs, + *m_user_generated_schema_tree + ); +} + auto KeyValuePairLogEvent::serialize_to_json( ) const -> OUTCOME_V2_NAMESPACE::std_result> { + auto const auto_generated_schema_subtree_bitmap_result{get_auto_generated_schema_subtree_bitmap( + )}; + if (auto_generated_schema_subtree_bitmap_result.has_error()) { + return auto_generated_schema_subtree_bitmap_result.error(); + } auto serialized_auto_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_auto_generated_schema_tree, - m_auto_generated_node_id_value_pairs + m_auto_generated_node_id_value_pairs, + auto_generated_schema_subtree_bitmap_result.value() )}; if (serialized_auto_generated_kv_pairs_result.has_error()) { return serialized_auto_generated_kv_pairs_result.error(); } + auto const user_generated_schema_subtree_bitmap_result{get_user_generated_schema_subtree_bitmap( + )}; + if (user_generated_schema_subtree_bitmap_result.has_error()) { + return user_generated_schema_subtree_bitmap_result.error(); + } auto serialized_user_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_user_generated_schema_tree, - m_user_generated_node_id_value_pairs + m_user_generated_node_id_value_pairs, + user_generated_schema_subtree_bitmap_result.value() )}; if (serialized_user_generated_kv_pairs_result.has_error()) { return serialized_user_generated_kv_pairs_result.error(); diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 23b353ed0..007f155bb 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -77,6 +78,28 @@ class KeyValuePairLogEvent { return m_user_generated_node_id_value_pairs; } + /** + * @return A result containing a bitmap where every bit corresponds to the ID of a node in the + * auto-generated schema tree, and the set bits correspond to the nodes in the subtree defined + * by all paths from the root node to the nodes in `m_auto_generated_node_id_value_pairs`; or an + * error code indicating a failure: + * - std::errc::result_out_of_range if a node ID in `m_auto_generated_node_id_value_pairs` + * doesn't exist in the schema tree. + */ + [[nodiscard]] auto get_auto_generated_schema_subtree_bitmap( + ) const -> OUTCOME_V2_NAMESPACE::std_result>; + + /** + * @return A result containing a bitmap where every bit corresponds to the ID of a node in the + * user-generated schema tree, and the set bits correspond to the nodes in the subtree defined + * by all paths from the root node to the nodes in `m_user_generated_node_id_value_pairs`; or an + * error code indicating a failure: + * - std::errc::result_out_of_range if a node ID in `m_user_generated_node_id_value_pairs` + * doesn't exist in the schema tree. + */ + [[nodiscard]] auto get_user_generated_schema_subtree_bitmap( + ) const -> OUTCOME_V2_NAMESPACE::std_result>; + [[nodiscard]] auto get_utc_offset() const -> UtcOffset { return m_utc_offset; } /** From 09d2db0a8baf8d28cc8a6ca06ff091e60a4fabaa Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 23 Nov 2024 20:26:12 -0500 Subject: [PATCH 09/17] Fix comment --- components/core/src/clp/ffi/KeyValuePairLogEvent.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 007f155bb..754750f05 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -104,10 +104,12 @@ class KeyValuePairLogEvent { /** * Serializes the log event into `nlohmann::json` objects. - * @return A result containing a pair of serialized JSON objects (the first contains - * auto-generated key-value pairs, while the second contains user-generated key-value pairs), or - * an error code indicating the failure: - * - Forwards `serialize_node_id_value_pairs_to_json`'s return values. + * @return A result containing a pair or an error code indicating the failure: + * - The pair: + * - Serialized auto-generated key-value pairs as a JSON object + * - Serialized user-generated key-value pairs as a JSON object + * - The possible error codes: + * - Forwards `serialize_node_id_value_pairs_to_json`'s return values on failure. */ [[nodiscard]] auto serialize_to_json( ) const -> OUTCOME_V2_NAMESPACE::std_result>; From 4281df6765f807236171c897bb33282114e8175c Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Mon, 9 Dec 2024 10:47:11 -0500 Subject: [PATCH 10/17] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 25 +++++++++---------- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 23 +++++++++-------- components/core/src/clp/ffi/SchemaTree.hpp | 4 +-- .../ir_unit_deserialization_methods.hpp | 4 +-- .../tests/test-ffi_KeyValuePairLogEvent.cpp | 12 ++++----- 5 files changed, 33 insertions(+), 35 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index aa1968428..4e4ba57c9 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -190,14 +190,13 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> [[nodiscard]] auto decode_as_encoded_text_ast(Value const& val) -> std::optional; /** - * Serializes the given node ID value pairs into a `nlohmann::json` object. + * Serializes the given node-ID-value pairs into a `nlohmann::json` object. * @param schema_tree * @param node_id_value_pairs * @param schema_subtree_bitmap * @return A result containing the serialized JSON object or an error code indicating the failure: - * - std::errc::protocol_error if a value in the log event couldn't be decoded or it couldn't be + * - std::errc::protocol_error if a value in the log event couldn't be decoded, or it couldn't be * inserted into a JSON object. - * - std::errc::result_out_of_range if a node ID in the log event doesn't exist in the schema tree. */ [[nodiscard]] auto serialize_node_id_value_pairs_to_json( SchemaTree const& schema_tree, @@ -210,7 +209,7 @@ node_type_matches_value_type(SchemaTree::Node::Type type, Value const& value) -> * @param parent_node_id_to_key_names * @return true if `node`'s key is unique among its sibling nodes with `parent_node_id_to_key_names` * updated to keep track of this unique key name. - * @return false if `node`'s key already exists in other sibling nodes. + * @return false if a sibling of `node` has the same key. */ [[nodiscard]] auto check_key_uniqueness_among_sibling_nodes( SchemaTree::Node const& node, @@ -276,12 +275,12 @@ auto validate_node_id_value_pairs( return std::errc::protocol_not_supported; } - // Iteratively check if there's any key duplication in ancestors until: - // 1. The ancestor is already checked. We only need to check an ancestor node once. If - // there are key duplications among its sibling, it will be caught when the sibling - // is first checked. The order of which sibling gets checked first doesn't affect the - // results. - // 2. Reached the root node. + // Iteratively check if there's any key duplication in the node's ancestors until: + // 1. The ancestor has already been checked. We only need to check an ancestor node + // once since if there are key duplications among its siblings, it would've been + // caught when the sibling was first checked (the order in which siblings get checked + // doesn't affect the results). + // 2. We reach the root node. auto next_ancestor_node_id_to_check{node.get_parent_id_unsafe()}; while (false == key_duplication_checked_node_id_bitmap[next_ancestor_node_id_to_check]) { @@ -335,7 +334,7 @@ auto get_schema_subtree_bitmap( KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs, SchemaTree const& schema_tree ) -> OUTCOME_V2_NAMESPACE::std_result> { - auto schema_subtree_bitmap{vector(schema_tree.get_size(), false)}; + vector schema_subtree_bitmap(schema_tree.get_size(), false); for (auto const& [node_id, val] : node_id_value_pairs) { if (node_id >= schema_subtree_bitmap.size()) { return std::errc::result_out_of_range; @@ -505,8 +504,8 @@ auto check_key_uniqueness_among_sibling_nodes( std::unordered_map>& parent_node_id_to_key_names ) -> bool { - // The given node must not be the root (checked by the caller), so we can query the underlying - // ID safely without a repeated check. + // The caller checks that the given node is not the root, so we can query the underlying + // parent ID safely without a check. auto const parent_node_id{node.get_parent_id_unsafe()}; auto const key_name{node.get_key_name()}; auto const parent_node_id_to_key_names_it{parent_node_id_to_key_names.find(parent_node_id)}; diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 754750f05..dde80ae31 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -17,12 +17,12 @@ namespace clp::ffi { /** * A log event containing key-value pairs. Each event contains: - * - A reference to the schema tree of auto-generated keys. - * - A reference to the schema tree of user-generated keys. + * - A reference to the schema tree for auto-generated keys. + * - A reference to the schema tree for user-generated keys. * - A collection of auto-generated node-ID & value pairs, where each pair represents a leaf - * `SchemaTree::Node` in the `SchemaTree`. + * `SchemaTree::Node` in the schema tree for auto-generated keys. * - A collection of user-generated node-ID & value pairs, where each pair represents a leaf - * `SchemaTree::Node` in the `SchemaTree`. + * `SchemaTree::Node` in the schema tree for user-generated keys. * - The UTC offset of the current log event. */ class KeyValuePairLogEvent { @@ -38,7 +38,7 @@ class KeyValuePairLogEvent { * @param user_generated_node_id_value_pairs * @param utc_offset * @return A result containing the key-value pair log event or an error code indicating the - * failure, or an error code indicating the failure: + * failure: * - std::errc::invalid_argument if any of the given schema tree pointers are null. * - Forwards `validate_node_id_value_pairs`'s return values. */ @@ -80,9 +80,9 @@ class KeyValuePairLogEvent { /** * @return A result containing a bitmap where every bit corresponds to the ID of a node in the - * auto-generated schema tree, and the set bits correspond to the nodes in the subtree defined - * by all paths from the root node to the nodes in `m_auto_generated_node_id_value_pairs`; or an - * error code indicating a failure: + * schema tree for auto-generated keys, and the set bits correspond to the nodes in the subtree + * defined by all paths from the root node to the nodes in + * `m_auto_generated_node_id_value_pairs`; or an error code indicating a failure: * - std::errc::result_out_of_range if a node ID in `m_auto_generated_node_id_value_pairs` * doesn't exist in the schema tree. */ @@ -91,9 +91,9 @@ class KeyValuePairLogEvent { /** * @return A result containing a bitmap where every bit corresponds to the ID of a node in the - * user-generated schema tree, and the set bits correspond to the nodes in the subtree defined - * by all paths from the root node to the nodes in `m_user_generated_node_id_value_pairs`; or an - * error code indicating a failure: + * schema tree for user-generated keys, and the set bits correspond to the nodes in the subtree + * defined by all paths from the root node to the nodes in + * `m_user_generated_node_id_value_pairs`; or an error code indicating a failure: * - std::errc::result_out_of_range if a node ID in `m_user_generated_node_id_value_pairs` * doesn't exist in the schema tree. */ @@ -109,6 +109,7 @@ class KeyValuePairLogEvent { * - Serialized auto-generated key-value pairs as a JSON object * - Serialized user-generated key-value pairs as a JSON object * - The possible error codes: + * - Forwards `get_auto_generated_schema_subtree_bitmap`'s return values on failure. * - Forwards `serialize_node_id_value_pairs_to_json`'s return values on failure. */ [[nodiscard]] auto serialize_to_json( diff --git a/components/core/src/clp/ffi/SchemaTree.hpp b/components/core/src/clp/ffi/SchemaTree.hpp index abcb0eefa..9c0c66aca 100644 --- a/components/core/src/clp/ffi/SchemaTree.hpp +++ b/components/core/src/clp/ffi/SchemaTree.hpp @@ -127,8 +127,7 @@ class SchemaTree { // Destructor ~Node() = default; - // Defines default equal-to operator - [[nodiscard]] auto operator==(SchemaTree::Node const& rhs) const -> bool = default; + [[nodiscard]] auto operator==(Node const& rhs) const -> bool = default; // Methods [[nodiscard]] auto get_id() const -> id_t { return m_id; } @@ -251,7 +250,6 @@ class SchemaTree { // Destructor ~SchemaTree() = default; - // Equal-to operator [[nodiscard]] auto operator==(SchemaTree const& rhs) const -> bool { return m_tree_nodes == rhs.m_tree_nodes; } diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp index c70481fcf..a816ce5b5 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp @@ -57,9 +57,9 @@ namespace clp::ffi::ir_stream { * Deserializes a key-value pair log event IR unit. * @param reader * @param tag - * @param auto_generated_schema_tree Schema tree of auto-generated keys used to construct the + * @param auto_generated_schema_tree Schema tree for auto-generated keys, used to construct the * KV-pair log event. - * @param user_generated_schema_tree Schema tree of user-generated keys used to construct the + * @param user_generated_schema_tree Schema tree for user-generated keys, used to construct the * KV-pair log event. * @param utc_offset UTC offset used to construct the KV-pair log event. * @return A result containing the deserialized log event or an error code indicating the diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 5ee5dfeaf..180424e0b 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -83,7 +83,7 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( ) -> void; /** - * Asserts the kv pair log event creation fails with the expected error code. + * Asserts that `KeyValuePairLogEvent` creation fails with the expected error code. * @param auto_generated_schema_tree * @param user_generated_schema_tree * @param auto_generated_node_id_value_pairs @@ -496,7 +496,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { REQUIRE((serialized_user_generated_kv_pairs == expected)); } - SECTION("Test duplicated key conflict on node #3") { + SECTION("Test duplicated key conflict under node #3") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(6, Value{static_cast(false)}); @@ -518,7 +518,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } - SECTION("Test duplicated key conflict on node #4") { + SECTION("Test duplicated key conflict under node #4") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(9, Value{static_cast(0.0)}); @@ -540,7 +540,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } - SECTION("Test duplicated keys among siblings of node #1") { + SECTION("Test duplicated keys amongst siblings of node #1") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(12, static_cast(0)); @@ -563,11 +563,11 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } - SECTION("Test duplicated keys among siblings of node #3") { + SECTION("Test duplicated keys amongst siblings of node #3") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(13, false); - // Node #12 has the same key as its sibling node #3 + // Node #13 has the same key as its sibling node #3 REQUIRE(assert_kv_pair_log_event_creation_failure( auto_generated_schema_tree, user_generated_schema_tree, From b10ad4b42f594d7effdc9e4cbdf5cf88ce59e4af Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 10:54:39 -0500 Subject: [PATCH 11/17] Move == operator to method sectiong --- components/core/src/clp/ffi/SchemaTree.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/core/src/clp/ffi/SchemaTree.hpp b/components/core/src/clp/ffi/SchemaTree.hpp index 9c0c66aca..4efbbf81e 100644 --- a/components/core/src/clp/ffi/SchemaTree.hpp +++ b/components/core/src/clp/ffi/SchemaTree.hpp @@ -127,9 +127,9 @@ class SchemaTree { // Destructor ~Node() = default; + // Methods [[nodiscard]] auto operator==(Node const& rhs) const -> bool = default; - // Methods [[nodiscard]] auto get_id() const -> id_t { return m_id; } [[nodiscard]] auto is_root() const -> bool { return false == m_parent_id.has_value(); } @@ -250,11 +250,11 @@ class SchemaTree { // Destructor ~SchemaTree() = default; + // Methods [[nodiscard]] auto operator==(SchemaTree const& rhs) const -> bool { return m_tree_nodes == rhs.m_tree_nodes; } - // Methods [[nodiscard]] auto get_size() const -> size_t { return m_tree_nodes.size(); } [[nodiscard]] auto get_root() const -> Node const& { return m_tree_nodes[cRootId]; } From 733c896268aac33afe4fb9ef64064fc974673955 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 11:13:13 -0500 Subject: [PATCH 12/17] Fix naming --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 44 +++---- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 64 +++++----- .../src/clp/ffi/ir_stream/Deserializer.hpp | 12 +- .../ir_unit_deserialization_methods.cpp | 8 +- .../ir_unit_deserialization_methods.hpp | 8 +- .../tests/test-ffi_IrUnitHandlerInterface.cpp | 2 +- .../tests/test-ffi_KeyValuePairLogEvent.cpp | 116 +++++++++--------- .../core/tests/test-ir_encoding_methods.cpp | 4 +- 8 files changed, 125 insertions(+), 133 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 4e4ba57c9..53fee24f5 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -523,19 +523,19 @@ auto check_key_uniqueness_among_sibling_nodes( } // namespace auto KeyValuePairLogEvent::create( - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, - NodeIdValuePairs auto_generated_node_id_value_pairs, - NodeIdValuePairs user_generated_node_id_value_pairs, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, + NodeIdValuePairs auto_gen_node_id_value_pairs, + NodeIdValuePairs user_gen_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result { - if (nullptr == auto_generated_schema_tree || nullptr == user_generated_schema_tree) { + if (nullptr == auto_gen_keys_schema_tree || nullptr == user_gen_keys_schema_tree) { return std::errc::invalid_argument; } if (auto const ret_val{validate_node_id_value_pairs( - *auto_generated_schema_tree, - auto_generated_node_id_value_pairs + *auto_gen_keys_schema_tree, + auto_gen_node_id_value_pairs )}; std::errc{} != ret_val) { @@ -543,8 +543,8 @@ auto KeyValuePairLogEvent::create( } if (auto const ret_val{validate_node_id_value_pairs( - *user_generated_schema_tree, - user_generated_node_id_value_pairs + *user_gen_keys_schema_tree, + user_gen_node_id_value_pairs )}; std::errc{} != ret_val) { @@ -552,28 +552,22 @@ auto KeyValuePairLogEvent::create( } return KeyValuePairLogEvent{ - std::move(auto_generated_schema_tree), - std::move(user_generated_schema_tree), - std::move(auto_generated_node_id_value_pairs), - std::move(user_generated_node_id_value_pairs), + std::move(auto_gen_keys_schema_tree), + std::move(user_gen_keys_schema_tree), + std::move(auto_gen_node_id_value_pairs), + std::move(user_gen_node_id_value_pairs), utc_offset }; } auto KeyValuePairLogEvent::get_auto_generated_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result> { - return get_schema_subtree_bitmap( - m_auto_generated_node_id_value_pairs, - *m_auto_generated_schema_tree - ); + return get_schema_subtree_bitmap(m_auto_gen_node_id_value_pairs, *m_auto_gen_keys_schema_tree); } auto KeyValuePairLogEvent::get_user_generated_schema_subtree_bitmap( ) const -> outcome_v2::std_result> { - return get_schema_subtree_bitmap( - m_user_generated_node_id_value_pairs, - *m_user_generated_schema_tree - ); + return get_schema_subtree_bitmap(m_user_gen_node_id_value_pairs, *m_user_gen_keys_schema_tree); } auto KeyValuePairLogEvent::serialize_to_json( @@ -584,8 +578,8 @@ auto KeyValuePairLogEvent::serialize_to_json( return auto_generated_schema_subtree_bitmap_result.error(); } auto serialized_auto_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( - *m_auto_generated_schema_tree, - m_auto_generated_node_id_value_pairs, + *m_auto_gen_keys_schema_tree, + m_auto_gen_node_id_value_pairs, auto_generated_schema_subtree_bitmap_result.value() )}; if (serialized_auto_generated_kv_pairs_result.has_error()) { @@ -598,8 +592,8 @@ auto KeyValuePairLogEvent::serialize_to_json( return user_generated_schema_subtree_bitmap_result.error(); } auto serialized_user_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( - *m_user_generated_schema_tree, - m_user_generated_node_id_value_pairs, + *m_user_gen_keys_schema_tree, + m_user_gen_node_id_value_pairs, user_generated_schema_subtree_bitmap_result.value() )}; if (serialized_user_generated_kv_pairs_result.has_error()) { diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index dde80ae31..187e85bc9 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -32,10 +32,10 @@ class KeyValuePairLogEvent { // Factory functions /** - * @param auto_generated_schema_tree - * @param user_generated_schema_tree - * @param auto_generated_node_id_value_pairs - * @param user_generated_node_id_value_pairs + * @param auto_gen_keys_schema_tree + * @param user_gen_keys_schema_tree + * @param auto_gen_node_id_value_pairs + * @param user_gen_node_id_value_pairs * @param utc_offset * @return A result containing the key-value pair log event or an error code indicating the * failure: @@ -43,10 +43,10 @@ class KeyValuePairLogEvent { * - Forwards `validate_node_id_value_pairs`'s return values. */ [[nodiscard]] static auto create( - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, - NodeIdValuePairs auto_generated_node_id_value_pairs, - NodeIdValuePairs user_generated_node_id_value_pairs, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, + NodeIdValuePairs auto_gen_node_id_value_pairs, + NodeIdValuePairs user_gen_node_id_value_pairs, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result; @@ -62,28 +62,28 @@ class KeyValuePairLogEvent { ~KeyValuePairLogEvent() = default; // Methods - [[nodiscard]] auto get_auto_generated_schema_tree() const -> SchemaTree const& { - return *m_auto_generated_schema_tree; + [[nodiscard]] auto get_auto_gen_keys_schema_tree() const -> SchemaTree const& { + return *m_auto_gen_keys_schema_tree; } - [[nodiscard]] auto get_user_generated_schema_tree() const -> SchemaTree const& { - return *m_user_generated_schema_tree; + [[nodiscard]] auto get_user_gen_keys_schema_tree() const -> SchemaTree const& { + return *m_user_gen_keys_schema_tree; } - [[nodiscard]] auto get_auto_generated_node_id_value_pairs() const -> NodeIdValuePairs const& { - return m_auto_generated_node_id_value_pairs; + [[nodiscard]] auto get_auto_gen_node_id_value_pairs() const -> NodeIdValuePairs const& { + return m_auto_gen_node_id_value_pairs; } - [[nodiscard]] auto get_user_generated_node_id_value_pairs() const -> NodeIdValuePairs const& { - return m_user_generated_node_id_value_pairs; + [[nodiscard]] auto get_user_gen_node_id_value_pairs() const -> NodeIdValuePairs const& { + return m_user_gen_node_id_value_pairs; } /** * @return A result containing a bitmap where every bit corresponds to the ID of a node in the * schema tree for auto-generated keys, and the set bits correspond to the nodes in the subtree * defined by all paths from the root node to the nodes in - * `m_auto_generated_node_id_value_pairs`; or an error code indicating a failure: - * - std::errc::result_out_of_range if a node ID in `m_auto_generated_node_id_value_pairs` + * `m_auto_gen_node_id_value_pairs`; or an error code indicating a failure: + * - std::errc::result_out_of_range if a node ID in `m_auto_gen_node_id_value_pairs` * doesn't exist in the schema tree. */ [[nodiscard]] auto get_auto_generated_schema_subtree_bitmap( @@ -93,8 +93,8 @@ class KeyValuePairLogEvent { * @return A result containing a bitmap where every bit corresponds to the ID of a node in the * schema tree for user-generated keys, and the set bits correspond to the nodes in the subtree * defined by all paths from the root node to the nodes in - * `m_user_generated_node_id_value_pairs`; or an error code indicating a failure: - * - std::errc::result_out_of_range if a node ID in `m_user_generated_node_id_value_pairs` + * `m_user_gen_node_id_value_pairs`; or an error code indicating a failure: + * - std::errc::result_out_of_range if a node ID in `m_user_gen_node_id_value_pairs` * doesn't exist in the schema tree. */ [[nodiscard]] auto get_user_generated_schema_subtree_bitmap( @@ -118,23 +118,23 @@ class KeyValuePairLogEvent { private: // Constructor KeyValuePairLogEvent( - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, - NodeIdValuePairs auto_generated_node_id_value_pairs, - NodeIdValuePairs user_generated_node_id_value_pairs, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, + NodeIdValuePairs auto_gen_node_id_value_pairs, + NodeIdValuePairs user_gen_node_id_value_pairs, UtcOffset utc_offset ) - : m_auto_generated_schema_tree{std::move(auto_generated_schema_tree)}, - m_user_generated_schema_tree{std::move(user_generated_schema_tree)}, - m_auto_generated_node_id_value_pairs{std::move(auto_generated_node_id_value_pairs)}, - m_user_generated_node_id_value_pairs{std::move(user_generated_node_id_value_pairs)}, + : m_auto_gen_keys_schema_tree{std::move(auto_gen_keys_schema_tree)}, + m_user_gen_keys_schema_tree{std::move(user_gen_keys_schema_tree)}, + m_auto_gen_node_id_value_pairs{std::move(auto_gen_node_id_value_pairs)}, + m_user_gen_node_id_value_pairs{std::move(user_gen_node_id_value_pairs)}, m_utc_offset{utc_offset} {} // Variables - std::shared_ptr m_auto_generated_schema_tree; - std::shared_ptr m_user_generated_schema_tree; - NodeIdValuePairs m_auto_generated_node_id_value_pairs; - NodeIdValuePairs m_user_generated_node_id_value_pairs; + std::shared_ptr m_auto_gen_keys_schema_tree; + std::shared_ptr m_user_gen_keys_schema_tree; + NodeIdValuePairs m_auto_gen_node_id_value_pairs; + NodeIdValuePairs m_user_gen_node_id_value_pairs; UtcOffset m_utc_offset{0}; }; } // namespace clp::ffi diff --git a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp index 042801871..d31699cd2 100644 --- a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp @@ -115,8 +115,8 @@ class Deserializer { Deserializer(IrUnitHandler ir_unit_handler) : m_ir_unit_handler{std::move(ir_unit_handler)} {} // Variables - std::shared_ptr m_auto_generated_schema_tree{std::make_shared()}; - std::shared_ptr m_user_generated_schema_tree{std::make_shared()}; + std::shared_ptr m_auto_gen_keys_schema_tree{std::make_shared()}; + std::shared_ptr m_user_gen_keys_schema_tree{std::make_shared()}; UtcOffset m_utc_offset{0}; IrUnitHandler m_ir_unit_handler; bool m_is_complete{false}; @@ -187,8 +187,8 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read auto result{deserialize_ir_unit_kv_pair_log_event( reader, tag, - m_auto_generated_schema_tree, - m_user_generated_schema_tree, + m_auto_gen_keys_schema_tree, + m_user_gen_keys_schema_tree, m_utc_offset )}; if (result.has_error()) { @@ -212,7 +212,7 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read } auto const node_locator{result.value()}; - if (m_user_generated_schema_tree->has_node(node_locator)) { + if (m_user_gen_keys_schema_tree->has_node(node_locator)) { return std::errc::protocol_error; } @@ -222,7 +222,7 @@ auto Deserializer::deserialize_next_ir_unit(ReaderInterface& read return ir_error_code_to_errc(err); } - std::ignore = m_user_generated_schema_tree->insert_node(node_locator); + std::ignore = m_user_gen_keys_schema_tree->insert_node(node_locator); break; } diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp index c115d9767..cea4a1b84 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.cpp @@ -551,8 +551,8 @@ auto deserialize_ir_unit_utc_offset_change(ReaderInterface& reader auto deserialize_ir_unit_kv_pair_log_event( ReaderInterface& reader, encoded_tag_t tag, - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result { auto const schema_result{deserialize_schema(reader, tag)}; @@ -580,8 +580,8 @@ auto deserialize_ir_unit_kv_pair_log_event( } return KeyValuePairLogEvent::create( - std::move(auto_generated_schema_tree), - std::move(user_generated_schema_tree), + std::move(auto_gen_keys_schema_tree), + std::move(user_gen_keys_schema_tree), {}, std::move(node_id_value_pairs), utc_offset diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp index a816ce5b5..abe40bab7 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp @@ -57,9 +57,9 @@ namespace clp::ffi::ir_stream { * Deserializes a key-value pair log event IR unit. * @param reader * @param tag - * @param auto_generated_schema_tree Schema tree for auto-generated keys, used to construct the + * @param auto_gen_keys_schema_tree Schema tree for auto-generated keys, used to construct the * KV-pair log event. - * @param user_generated_schema_tree Schema tree for user-generated keys, used to construct the + * @param user_gen_keys_schema_tree Schema tree for user-generated keys, used to construct the * KV-pair log event. * @param utc_offset UTC offset used to construct the KV-pair log event. * @return A result containing the deserialized log event or an error code indicating the @@ -75,8 +75,8 @@ namespace clp::ffi::ir_stream { [[nodiscard]] auto deserialize_ir_unit_kv_pair_log_event( ReaderInterface& reader, encoded_tag_t tag, - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, UtcOffset utc_offset ) -> OUTCOME_V2_NAMESPACE::std_result; } // namespace clp::ffi::ir_stream diff --git a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp index 799351d78..8f76a2f1a 100644 --- a/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp +++ b/components/core/tests/test-ffi_IrUnitHandlerInterface.cpp @@ -131,7 +131,7 @@ TEMPLATE_TEST_CASE( REQUIRE( (optional_log_event.has_value() && optional_log_event.value().get_utc_offset() == cTestUtcOffset - && optional_log_event.value().get_user_generated_node_id_value_pairs().empty()) + && optional_log_event.value().get_user_gen_node_id_value_pairs().empty()) ); auto const& optional_schema_tree_locator{handler.get_schema_tree_node_locator()}; REQUIRE( diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 180424e0b..3fb396a25 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -84,8 +84,8 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( /** * Asserts that `KeyValuePairLogEvent` creation fails with the expected error code. - * @param auto_generated_schema_tree - * @param user_generated_schema_tree + * @param auto_gen_keys_schema_tree + * @param user_gen_keys_schema_tree * @param auto_generated_node_id_value_pairs * @param user_generated_node_id_value_pairs * @param utc_offset @@ -93,8 +93,8 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( * @return Whether the assertion succeeded. */ [[nodiscard]] auto assert_kv_pair_log_event_creation_failure( - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset, @@ -219,16 +219,16 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( } auto assert_kv_pair_log_event_creation_failure( - std::shared_ptr auto_generated_schema_tree, - std::shared_ptr user_generated_schema_tree, + std::shared_ptr auto_gen_keys_schema_tree, + std::shared_ptr user_gen_keys_schema_tree, KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, UtcOffset utc_offset, std::errc expected_error_code ) -> bool { auto const result{KeyValuePairLogEvent::create( - std::move(auto_generated_schema_tree), - std::move(user_generated_schema_tree), + std::move(auto_gen_keys_schema_tree), + std::move(user_gen_keys_schema_tree), std::move(auto_generated_node_id_value_pairs), std::move(user_generated_node_id_value_pairs), utc_offset @@ -300,8 +300,8 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { * | | * |--> <13:b:Bool> |--> <11:f:Obj> */ - auto const auto_generated_schema_tree{std::make_shared()}; - auto const user_generated_schema_tree{std::make_shared()}; + auto const auto_gen_keys_schema_tree{std::make_shared()}; + auto const user_gen_keys_schema_tree{std::make_shared()}; std::vector const locators{ {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Obj}, {SchemaTree::cRootId, "b", SchemaTree::Node::Type::Int}, @@ -318,18 +318,18 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { {1, "b", SchemaTree::Node::Type::Bool} }; for (auto const& locator : locators) { - REQUIRE_NOTHROW(auto_generated_schema_tree->insert_node(locator)); - REQUIRE_NOTHROW(user_generated_schema_tree->insert_node(locator)); + REQUIRE_NOTHROW(auto_gen_keys_schema_tree->insert_node(locator)); + REQUIRE_NOTHROW(user_gen_keys_schema_tree->insert_node(locator)); } // This test case implicitly requires the auto-generated and user-generated schema trees to // be identical. Adding this check to prevent this assumption is broken by future development. - REQUIRE((*auto_generated_schema_tree == *user_generated_schema_tree)); + REQUIRE((*auto_gen_keys_schema_tree == *user_gen_keys_schema_tree)); SECTION("Test empty ID-value pairs") { auto const result{KeyValuePairLogEvent::create( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, {}, {}, UtcOffset{0} @@ -340,14 +340,14 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { SECTION("Test schema tree pointers being null") { REQUIRE(assert_kv_pair_log_event_creation_failure( nullptr, - user_generated_schema_tree, + user_gen_keys_schema_tree, {}, {}, UtcOffset{0}, std::errc::invalid_argument )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, + auto_gen_keys_schema_tree, nullptr, {}, {}, @@ -361,42 +361,42 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) // Int: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 2, invalid_node_id_value_pairs ); // Float: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 9, invalid_node_id_value_pairs ); // Bool: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 6, invalid_node_id_value_pairs ); // Str: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 5, invalid_node_id_value_pairs ); // UnstructuredArray: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 7, invalid_node_id_value_pairs ); // Obj: insert_invalid_node_id_value_pairs_with_node_type_errors( - *user_generated_schema_tree, + *user_gen_keys_schema_tree, 3, invalid_node_id_value_pairs ); @@ -411,16 +411,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { } REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, node_id_value_pair_to_test, {}, UtcOffset{0}, std::errc::protocol_error )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, {}, node_id_value_pair_to_test, UtcOffset{0}, @@ -466,8 +466,8 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { valid_node_id_value_pairs.emplace(11, std::nullopt); // NOLINTEND(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) auto const result{KeyValuePairLogEvent::create( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0} @@ -501,16 +501,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(6, Value{static_cast(false)}); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::protocol_not_supported )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -523,16 +523,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(9, Value{static_cast(0.0)}); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::protocol_not_supported )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -546,16 +546,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { invalid_node_id_value_pairs.emplace(12, static_cast(0)); // Node #12 has the same key as its sibling node #1 REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::protocol_not_supported )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -569,16 +569,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { invalid_node_id_value_pairs.emplace(13, false); // Node #13 has the same key as its sibling node #3 REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::protocol_not_supported )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -591,16 +591,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { invalid_node_id_value_pairs.emplace(3, std::nullopt); // Node #3 is empty, but its descendants appear in the sub schema tree (node #5 & #10) REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::operation_not_permitted )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -613,16 +613,16 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { invalid_node_id_value_pairs.emplace(4, Value{}); // Node #4 is null, but its descendants appear in the sub schema tree (node #5 & #10) REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, invalid_node_id_value_pairs, valid_node_id_value_pairs, UtcOffset{0}, std::errc::operation_not_permitted )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, valid_node_id_value_pairs, invalid_node_id_value_pairs, UtcOffset{0}, @@ -634,20 +634,20 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { SECTION("Test out-of-bound node ID") { KeyValuePairLogEvent::NodeIdValuePairs node_id_value_pairs_out_of_bound; node_id_value_pairs_out_of_bound.emplace( - static_cast(user_generated_schema_tree->get_size()), + static_cast(user_gen_keys_schema_tree->get_size()), Value{} ); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, node_id_value_pairs_out_of_bound, {}, UtcOffset{0}, std::errc::operation_not_permitted )); REQUIRE(assert_kv_pair_log_event_creation_failure( - auto_generated_schema_tree, - user_generated_schema_tree, + auto_gen_keys_schema_tree, + user_gen_keys_schema_tree, {}, node_id_value_pairs_out_of_bound, UtcOffset{0}, diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 059b4eeff..347dadb7a 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -1246,9 +1246,7 @@ TEMPLATE_TEST_CASE( auto const& deserialized_log_event{deserialized_log_events.at(idx)}; auto const num_leaves_in_json_obj{count_num_leaves(expect)}; - auto const num_kv_pairs{ - deserialized_log_event.get_user_generated_node_id_value_pairs().size() - }; + auto const num_kv_pairs{deserialized_log_event.get_user_gen_node_id_value_pairs().size()}; REQUIRE((num_leaves_in_json_obj == num_kv_pairs)); auto const serialized_json_result{deserialized_log_event.serialize_to_json()}; From 7338c34eecef282ff395ae7aa7992fcf424dd93b Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 11:24:43 -0500 Subject: [PATCH 13/17] Fix subtree bitmap methods --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 24 +++++++++---------- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 6 ++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index 53fee24f5..e55860aba 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -560,41 +560,41 @@ auto KeyValuePairLogEvent::create( }; } -auto KeyValuePairLogEvent::get_auto_generated_schema_subtree_bitmap( +auto KeyValuePairLogEvent::get_auto_gen_keys_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result> { return get_schema_subtree_bitmap(m_auto_gen_node_id_value_pairs, *m_auto_gen_keys_schema_tree); } -auto KeyValuePairLogEvent::get_user_generated_schema_subtree_bitmap( +auto KeyValuePairLogEvent::get_user_gen_keys_schema_subtree_bitmap( ) const -> outcome_v2::std_result> { return get_schema_subtree_bitmap(m_user_gen_node_id_value_pairs, *m_user_gen_keys_schema_tree); } auto KeyValuePairLogEvent::serialize_to_json( ) const -> OUTCOME_V2_NAMESPACE::std_result> { - auto const auto_generated_schema_subtree_bitmap_result{get_auto_generated_schema_subtree_bitmap( - )}; - if (auto_generated_schema_subtree_bitmap_result.has_error()) { - return auto_generated_schema_subtree_bitmap_result.error(); + auto const auto_gen_keys_schema_subtree_bitmap_result{get_auto_gen_keys_schema_subtree_bitmap() + }; + if (auto_gen_keys_schema_subtree_bitmap_result.has_error()) { + return auto_gen_keys_schema_subtree_bitmap_result.error(); } auto serialized_auto_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_auto_gen_keys_schema_tree, m_auto_gen_node_id_value_pairs, - auto_generated_schema_subtree_bitmap_result.value() + auto_gen_keys_schema_subtree_bitmap_result.value() )}; if (serialized_auto_generated_kv_pairs_result.has_error()) { return serialized_auto_generated_kv_pairs_result.error(); } - auto const user_generated_schema_subtree_bitmap_result{get_user_generated_schema_subtree_bitmap( - )}; - if (user_generated_schema_subtree_bitmap_result.has_error()) { - return user_generated_schema_subtree_bitmap_result.error(); + auto const user_gen_keys_schema_subtree_bitmap_result{get_user_gen_keys_schema_subtree_bitmap() + }; + if (user_gen_keys_schema_subtree_bitmap_result.has_error()) { + return user_gen_keys_schema_subtree_bitmap_result.error(); } auto serialized_user_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_user_gen_keys_schema_tree, m_user_gen_node_id_value_pairs, - user_generated_schema_subtree_bitmap_result.value() + user_gen_keys_schema_subtree_bitmap_result.value() )}; if (serialized_user_generated_kv_pairs_result.has_error()) { return serialized_user_generated_kv_pairs_result.error(); diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 187e85bc9..1620978bd 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -86,7 +86,7 @@ class KeyValuePairLogEvent { * - std::errc::result_out_of_range if a node ID in `m_auto_gen_node_id_value_pairs` * doesn't exist in the schema tree. */ - [[nodiscard]] auto get_auto_generated_schema_subtree_bitmap( + [[nodiscard]] auto get_auto_gen_keys_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result>; /** @@ -97,7 +97,7 @@ class KeyValuePairLogEvent { * - std::errc::result_out_of_range if a node ID in `m_user_gen_node_id_value_pairs` * doesn't exist in the schema tree. */ - [[nodiscard]] auto get_user_generated_schema_subtree_bitmap( + [[nodiscard]] auto get_user_gen_keys_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result>; [[nodiscard]] auto get_utc_offset() const -> UtcOffset { return m_utc_offset; } @@ -109,7 +109,7 @@ class KeyValuePairLogEvent { * - Serialized auto-generated key-value pairs as a JSON object * - Serialized user-generated key-value pairs as a JSON object * - The possible error codes: - * - Forwards `get_auto_generated_schema_subtree_bitmap`'s return values on failure. + * - Forwards `get_auto_gen_keys_schema_subtree_bitmap`'s return values on failure. * - Forwards `serialize_node_id_value_pairs_to_json`'s return values on failure. */ [[nodiscard]] auto serialize_to_json( From 091791dfc5258c4df3f267d0bebed096d35d10c6 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 11:31:11 -0500 Subject: [PATCH 14/17] Update comment to use forwarded value --- .../core/src/clp/ffi/KeyValuePairLogEvent.hpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp index 1620978bd..2929c7498 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.hpp @@ -81,10 +81,9 @@ class KeyValuePairLogEvent { /** * @return A result containing a bitmap where every bit corresponds to the ID of a node in the * schema tree for auto-generated keys, and the set bits correspond to the nodes in the subtree - * defined by all paths from the root node to the nodes in - * `m_auto_gen_node_id_value_pairs`; or an error code indicating a failure: - * - std::errc::result_out_of_range if a node ID in `m_auto_gen_node_id_value_pairs` - * doesn't exist in the schema tree. + * defined by all paths from the root node to the nodes in `m_auto_gen_node_id_value_pairs`; or + * an error code indicating a failure: + * - Forwards `get_schema_subtree_bitmap`'s return values. */ [[nodiscard]] auto get_auto_gen_keys_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result>; @@ -92,10 +91,9 @@ class KeyValuePairLogEvent { /** * @return A result containing a bitmap where every bit corresponds to the ID of a node in the * schema tree for user-generated keys, and the set bits correspond to the nodes in the subtree - * defined by all paths from the root node to the nodes in - * `m_user_gen_node_id_value_pairs`; or an error code indicating a failure: - * - std::errc::result_out_of_range if a node ID in `m_user_gen_node_id_value_pairs` - * doesn't exist in the schema tree. + * defined by all paths from the root node to the nodes in `m_user_gen_node_id_value_pairs`; or + * an error code indicating a failure: + * - Forwards `get_schema_subtree_bitmap`'s return values. */ [[nodiscard]] auto get_user_gen_keys_schema_subtree_bitmap( ) const -> OUTCOME_V2_NAMESPACE::std_result>; From bef61cd0e667c3d79146d0a6d0767ac00b44e70a Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 14:21:34 -0500 Subject: [PATCH 15/17] Done --- .../src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp | 3 +-- components/core/tests/test-ffi_KeyValuePairLogEvent.cpp | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp index abe40bab7..451f627db 100644 --- a/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/ir_unit_deserialization_methods.hpp @@ -62,8 +62,7 @@ namespace clp::ffi::ir_stream { * @param user_gen_keys_schema_tree Schema tree for user-generated keys, used to construct the * KV-pair log event. * @param utc_offset UTC offset used to construct the KV-pair log event. - * @return A result containing the deserialized log event or an error code indicating the - * failure: + * @return A result containing the deserialized log event or an error code indicating the failure: * - std::errc::result_out_of_range if the IR stream is truncated. * - std::errc::protocol_error if the IR stream is corrupted. * - std::errc::protocol_not_supported if the IR stream contains an unsupported metadata format diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 3fb396a25..18c7c4c18 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -322,8 +322,6 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { REQUIRE_NOTHROW(user_gen_keys_schema_tree->insert_node(locator)); } - // This test case implicitly requires the auto-generated and user-generated schema trees to - // be identical. Adding this check to prevent this assumption is broken by future development. REQUIRE((*auto_gen_keys_schema_tree == *user_gen_keys_schema_tree)); SECTION("Test empty ID-value pairs") { From de335ad207a8410ccc155cd9d7253db11aa6e0d5 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 18:04:41 -0500 Subject: [PATCH 16/17] Apply code review comments --- .../tests/test-ffi_KeyValuePairLogEvent.cpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp index 18c7c4c18..9ffee4f68 100644 --- a/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp +++ b/components/core/tests/test-ffi_KeyValuePairLogEvent.cpp @@ -86,8 +86,8 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( * Asserts that `KeyValuePairLogEvent` creation fails with the expected error code. * @param auto_gen_keys_schema_tree * @param user_gen_keys_schema_tree - * @param auto_generated_node_id_value_pairs - * @param user_generated_node_id_value_pairs + * @param auto_gen_node_id_value_pairs + * @param user_gen_node_id_value_pairs * @param utc_offset * @param expected_error_code * @return Whether the assertion succeeded. @@ -95,8 +95,8 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( [[nodiscard]] auto assert_kv_pair_log_event_creation_failure( std::shared_ptr auto_gen_keys_schema_tree, std::shared_ptr user_gen_keys_schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs auto_gen_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs user_gen_node_id_value_pairs, UtcOffset utc_offset, std::errc expected_error_code ) -> bool; @@ -221,16 +221,16 @@ auto insert_invalid_node_id_value_pairs_with_node_type_errors( auto assert_kv_pair_log_event_creation_failure( std::shared_ptr auto_gen_keys_schema_tree, std::shared_ptr user_gen_keys_schema_tree, - KeyValuePairLogEvent::NodeIdValuePairs auto_generated_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs user_generated_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs auto_gen_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs user_gen_node_id_value_pairs, UtcOffset utc_offset, std::errc expected_error_code ) -> bool { auto const result{KeyValuePairLogEvent::create( std::move(auto_gen_keys_schema_tree), std::move(user_gen_keys_schema_tree), - std::move(auto_generated_node_id_value_pairs), - std::move(user_generated_node_id_value_pairs), + std::move(auto_gen_node_id_value_pairs), + std::move(user_gen_node_id_value_pairs), utc_offset )}; return result.has_error() && result.error() == expected_error_code; @@ -487,11 +487,11 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { auto const& kv_pair_log_event{result.value()}; auto const serialized_json_result{kv_pair_log_event.serialize_to_json()}; REQUIRE_FALSE(serialized_json_result.has_error()); - auto const& [serialized_auto_generated_kv_pairs, serialized_user_generated_kv_pairs]{ + auto const& [serialized_auto_gen_kv_pairs, serialized_user_gen_kv_pairs]{ serialized_json_result.value() }; - REQUIRE((serialized_auto_generated_kv_pairs == expected)); - REQUIRE((serialized_user_generated_kv_pairs == expected)); + REQUIRE((serialized_auto_gen_kv_pairs == expected)); + REQUIRE((serialized_user_gen_kv_pairs == expected)); } SECTION("Test duplicated key conflict under node #3") { @@ -538,7 +538,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } - SECTION("Test duplicated keys amongst siblings of node #1") { + SECTION("Test duplicated keys among siblings of node #1") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(12, static_cast(0)); @@ -561,7 +561,7 @@ TEST_CASE("ffi_KeyValuePairLogEvent_create", "[ffi]") { )); } - SECTION("Test duplicated keys amongst siblings of node #3") { + SECTION("Test duplicated keys among siblings of node #3") { auto invalid_node_id_value_pairs{valid_node_id_value_pairs}; // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) invalid_node_id_value_pairs.emplace(13, false); From 8403be95374507256f078adf4c54bfdbf70a3b46 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 9 Dec 2024 18:09:24 -0500 Subject: [PATCH 17/17] Use to replace --- .../core/src/clp/ffi/KeyValuePairLogEvent.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp index e55860aba..8e8bb15f5 100644 --- a/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp +++ b/components/core/src/clp/ffi/KeyValuePairLogEvent.cpp @@ -577,13 +577,13 @@ auto KeyValuePairLogEvent::serialize_to_json( if (auto_gen_keys_schema_subtree_bitmap_result.has_error()) { return auto_gen_keys_schema_subtree_bitmap_result.error(); } - auto serialized_auto_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( + auto serialized_auto_gen_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_auto_gen_keys_schema_tree, m_auto_gen_node_id_value_pairs, auto_gen_keys_schema_subtree_bitmap_result.value() )}; - if (serialized_auto_generated_kv_pairs_result.has_error()) { - return serialized_auto_generated_kv_pairs_result.error(); + if (serialized_auto_gen_kv_pairs_result.has_error()) { + return serialized_auto_gen_kv_pairs_result.error(); } auto const user_gen_keys_schema_subtree_bitmap_result{get_user_gen_keys_schema_subtree_bitmap() @@ -591,16 +591,16 @@ auto KeyValuePairLogEvent::serialize_to_json( if (user_gen_keys_schema_subtree_bitmap_result.has_error()) { return user_gen_keys_schema_subtree_bitmap_result.error(); } - auto serialized_user_generated_kv_pairs_result{serialize_node_id_value_pairs_to_json( + auto serialized_user_gen_kv_pairs_result{serialize_node_id_value_pairs_to_json( *m_user_gen_keys_schema_tree, m_user_gen_node_id_value_pairs, user_gen_keys_schema_subtree_bitmap_result.value() )}; - if (serialized_user_generated_kv_pairs_result.has_error()) { - return serialized_user_generated_kv_pairs_result.error(); + if (serialized_user_gen_kv_pairs_result.has_error()) { + return serialized_user_gen_kv_pairs_result.error(); } - return {std::move(serialized_auto_generated_kv_pairs_result.value()), - std::move(serialized_user_generated_kv_pairs_result.value())}; + return {std::move(serialized_auto_gen_kv_pairs_result.value()), + std::move(serialized_user_gen_kv_pairs_result.value())}; } } // namespace clp::ffi