From 79c3f7db1481958a6aff0a2c36db232d0bf707e8 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 29 Jun 2024 13:35:06 -0400 Subject: [PATCH 01/17] Implement serialization method --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 422 ++++++++++++++++++ .../core/src/clp/ffi/ir_stream/Serializer.hpp | 33 ++ .../clp/ffi/ir_stream/encoding_methods.cpp | 26 +- .../clp/ffi/ir_stream/encoding_methods.hpp | 10 + .../clp/ffi/ir_stream/protocol_constants.hpp | 29 ++ .../core/src/clp/ffi/ir_stream/utils.cpp | 20 + .../core/src/clp/ffi/ir_stream/utils.hpp | 41 ++ 7 files changed, 572 insertions(+), 9 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 6a7128d9f..1a900bcf0 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -1,22 +1,220 @@ #include "Serializer.hpp" +#include +#include #include +#include +#include +#include // Should be removed +#include +#include #include +#include #include #include +#include #include "../../ir/types.hpp" #include "../../time_types.hpp" #include "../encoding_methods.hpp" +#include "../SchemaTree.hpp" +#include "../SchemaTreeNode.hpp" #include "encoding_methods.hpp" #include "protocol_constants.hpp" #include "utils.hpp" +using std::optional; +using std::span; +using std::string; +using std::string_view; +using std::vector; + using clp::ir::eight_byte_encoded_variable_t; using clp::ir::four_byte_encoded_variable_t; namespace clp::ffi::ir_stream { +namespace { +/** + * Class for tracking states when traversing a msgpack map using pre-order traversal. + */ +class MsgpackMapNode { +public: + // Types + using Child = msgpack::object_kv; + + // Constructors + MsgpackMapNode(SchemaTreeNode::id_t parent_id, Child* child_data, size_t child_data_length) + : m_parent_id{parent_id}, + m_children{child_data, child_data_length} {} + + // Methods + /** + * @return The id of the parent in the schema tree. + */ + [[nodiscard]] auto get_parent_id() const -> SchemaTreeNode::id_t { return m_parent_id; } + + /** + * @return Whether it has more child to traverse. + */ + [[nodiscard]] auto has_next_child() const -> bool { + return m_children.size() > m_curr_child_idx; + } + + /** + * Gets the next child and advances the underlying child idx. + * @return The next child to traverse. + */ + [[nodiscard]] auto get_next_child() -> Child const& { return m_children[m_curr_child_idx++]; } + +private: + SchemaTreeNode::id_t m_parent_id; + span m_children; + size_t m_curr_child_idx{0}; +}; + +/** + * Gets the corresponded schema tree node type from the given msgpack value. + * @param val msgpack value. + * @return The corresponded schema tree node type. + * @return std::nullopt if the value doesn't match any of the supported schema tree node type. + */ +[[nodiscard]] auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val +) -> optional; + +/** + * Serializes a value corresponded to an empty object. + * @parma buf Outputs the serialized byte sequence. + */ +auto serialize_empty_object(vector& buf) -> void; + +/** + * Serializes a value of integer type. + * @param val + * @param buf Outputs the serialized byte sequence. + * @return Whether serialization succeeded. + */ +auto serialize_value_int(int64_t val, vector& buf) -> void; + +/** + * Serializes a value of float type. + * @param val + * @param buf Outputs the serialized byte sequence. + */ +auto serialize_value_float(double val, vector& buf) -> void; + +/** + * Serializes a value of bool. + * @param val + * @param buf Outputs the serialized byte sequence. + */ +auto serialize_value_bool(bool val, vector& buf) -> void; + +/** + * Serializes a value of NULL type. + * @param buf Outputs the serialized byte sequence. + */ +auto serialize_value_null(vector& buf) -> void; + +/** + * Serializes a value of string type. + * @tparam encoded_variable_t + * @param val + * @param buf Outputs the serialized byte sequence. + * @return Whether serialization succeeded. + */ +template +[[nodiscard]] auto serialize_value_string(string_view val, vector& buf) -> bool; + +/** + * Serializes a msgpack array as a JSON string, using clp encoding. + * @tparam encoded_variable_t + * @param val + * @param buf Outputs the serialized byte sequence. + * @return Whether serialization succeeded. + */ +template +[[nodiscard]] auto serialize_value_array(msgpack::object const& val, vector& buf) -> bool; + +auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val +) -> optional { + optional ret_val; + switch (val.type) { + case msgpack::type::POSITIVE_INTEGER: + case msgpack::type::NEGATIVE_INTEGER: + ret_val.emplace(SchemaTreeNode::Type::Int); + break; + case msgpack::type::FLOAT32: + case msgpack::type::FLOAT: + ret_val.emplace(SchemaTreeNode::Type::Float); + break; + case msgpack::type::STR: + ret_val.emplace(SchemaTreeNode::Type::Str); + break; + case msgpack::type::BOOLEAN: + ret_val.emplace(SchemaTreeNode::Type::Bool); + break; + case msgpack::type::NIL: + case msgpack::type::MAP: + ret_val.emplace(SchemaTreeNode::Type::Obj); + break; + case msgpack::type::ARRAY: + ret_val.emplace(SchemaTreeNode::Type::UnstructuredArray); + break; + default: + return std::nullopt; + } + return ret_val; +} + +auto serialize_empty_object(vector& buf) -> void { + buf.push_back(cProtocol::Payload::ValueEmpty); +} + +auto serialize_value_int(int64_t val, vector& buf) -> void { + if (INT8_MIN <= val && val <= INT8_MAX) { + buf.push_back(cProtocol::Payload::ValueInt8); + buf.push_back(static_cast(val)); + } else if (INT16_MIN <= val && val <= INT16_MAX) { + buf.push_back(cProtocol::Payload::ValueInt16); + serialize_int(static_cast(val), buf); + } else if (INT32_MIN <= val && val <= INT32_MAX) { + buf.push_back(cProtocol::Payload::ValueInt32); + serialize_int(static_cast(val), buf); + } else { // (INT64_MIN <= val && val <= INT64_MAX) + buf.push_back(cProtocol::Payload::ValueInt64); + serialize_int(val, buf); + } +} + +auto serialize_value_float(double val, vector& buf) -> void { + serialize_int(std::bit_cast(val), buf); +} + +auto serialize_value_bool(bool val, vector& buf) -> void { + buf.push_back(val ? cProtocol::Payload::ValueTrue : cProtocol::Payload::ValueFalse); +} + +auto serialize_value_null(vector& buf) -> void { + buf.push_back(cProtocol::Payload::ValueNull); +} + +template +auto serialize_value_string(string_view val, vector& buf) -> bool { + if (string_view::npos == val.find(' ')) { + return serialize_string_packet(val, buf); + } + return serialize_clp_string(val, buf); +} + +template +auto serialize_value_array(msgpack::object const& val, vector& buf) -> bool { + std::ostringstream oss; + oss << val; + return serialize_clp_string(oss.str(), buf); +} +} // namespace + template auto Serializer::create( ) -> BOOST_OUTCOME_V2_NAMESPACE::std_result> { @@ -59,6 +257,206 @@ auto Serializer::change_utc_offset(UtcOffset utc_offset) -> serialize_utc_offset_change(m_curr_utc_offset, m_ir_buf); } +template +auto Serializer::serialize_log_event(msgpack::object const& log_event) -> bool { + if (msgpack::type::MAP != log_event.type) { + return false; + } + + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) + auto const& map{log_event.via.map}; + if (0 == map.size) { + serialize_empty_object(m_ir_buf); + return true; + } + + m_schema_tree.take_snapshot(); + m_schema_tree_node_buf.clear(); + m_key_group_buf.clear(); + m_value_group_buf.clear(); + + bool failure{false}; + vector working_stack; + working_stack.emplace_back(SchemaTree::cRootId, map.ptr, static_cast(map.size)); + while (false == working_stack.empty()) { + auto& curr{working_stack.back()}; + if (false == curr.has_next_child()) { + working_stack.pop_back(); + continue; + } + auto const& [key, val]{curr.get_next_child()}; + auto const opt_schema_tree_node_type{get_schema_tree_node_type_from_msgpack_val(val)}; + if (false == opt_schema_tree_node_type.has_value()) { + failure = true; + break; + } + + auto const schema_tree_node_type{opt_schema_tree_node_type.value()}; + SchemaTree::NodeLocator const locator{ + curr.get_parent_id(), + key.as(), + schema_tree_node_type + }; + + auto opt_schema_tree_node_id{m_schema_tree.try_get_node_id(locator)}; + if (false == opt_schema_tree_node_id.has_value()) { + opt_schema_tree_node_id.emplace(m_schema_tree.insert_node(locator)); + if (false == serialize_schema_tree_node(locator)) { + failure = true; + break; + } + } + auto const schema_tree_node_id{opt_schema_tree_node_id.value()}; + + if (SchemaTreeNode::Type::Obj == schema_tree_node_type && msgpack::type::MAP == val.type) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) + auto const& inner_map{val.via.map}; + auto const inner_map_size(static_cast(inner_map.size)); + if (0 == inner_map_size) { + if (false == serialize_key(schema_tree_node_id)) { + failure = true; + break; + } + serialize_empty_object(m_value_group_buf); + } else { + working_stack.emplace_back(schema_tree_node_id, inner_map.ptr, inner_map_size); + } + } else { + // Serialize primitive values + if (false + == (serialize_key(schema_tree_node_id) && serialize_val(val, schema_tree_node_type) + )) + { + failure = true; + break; + } + } + } + + if (failure) { + m_schema_tree.revert(); + return false; + } + + m_ir_buf.insert( + m_ir_buf.cend(), + m_schema_tree_node_buf.cbegin(), + m_schema_tree_node_buf.cend() + ); + m_ir_buf.insert(m_ir_buf.cend(), m_key_group_buf.cbegin(), m_key_group_buf.cend()); + m_ir_buf.insert(m_ir_buf.cend(), m_value_group_buf.cbegin(), m_value_group_buf.cend()); + return true; +} + +template +auto Serializer::serialize_schema_tree_node( + SchemaTree::NodeLocator const& locator +) -> bool { + switch (locator.get_type()) { + case SchemaTreeNode::Type::Int: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeInt); + break; + case SchemaTreeNode::Type::Float: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeFloat); + break; + case SchemaTreeNode::Type::Bool: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeBool); + break; + case SchemaTreeNode::Type::Str: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeStr); + break; + case SchemaTreeNode::Type::UnstructuredArray: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeUnstructuredArray); + break; + case SchemaTreeNode::Type::Obj: + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeObj); + break; + default: + // Unknown type + return false; + } + auto const parent_id{locator.get_parent_id()}; + if (parent_id <= UINT8_MAX) { + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUByte); + m_schema_tree_node_buf.push_back(static_cast(static_cast(parent_id))); + } else if (parent_id <= UINT16_MAX) { + m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUShort); + serialize_int(static_cast(parent_id), m_schema_tree_node_buf); + } else { + // Out of range + return false; + } + return serialize_string_packet(locator.get_key_name(), m_schema_tree_node_buf); +} + +template +auto Serializer::serialize_key(SchemaTreeNode::id_t id) -> bool { + if (id <= UINT8_MAX) { + m_key_group_buf.push_back(cProtocol::Payload::KeyIdUByte); + m_key_group_buf.push_back(static_cast(static_cast(id))); + } else if (id <= UINT16_MAX) { + m_key_group_buf.push_back(cProtocol::Payload::KeyIdUShort); + serialize_int(static_cast(id), m_key_group_buf); + } else { + return false; + } + return true; +} + +template +auto Serializer::serialize_val( + msgpack::object const& val, + SchemaTreeNode::Type type +) -> bool { + switch (type) { + case SchemaTreeNode::Type::Int: + if (msgpack::type::POSITIVE_INTEGER == val.type + && static_cast(INT64_MAX) < val.as()) + { + return false; + } + serialize_value_int(val.as(), m_value_group_buf); + break; + + case SchemaTreeNode::Type::Float: + serialize_value_float(val.as(), m_value_group_buf); + break; + + case SchemaTreeNode::Type::Bool: + serialize_value_bool(val.as(), m_value_group_buf); + break; + + case SchemaTreeNode::Type::Str: + if (false + == serialize_value_string( + val.as(), + m_value_group_buf + )) + { + return false; + } + break; + + case SchemaTreeNode::Type::Obj: + if (msgpack::type::NIL != val.type) { + return false; + } + serialize_value_null(m_value_group_buf); + break; + + case SchemaTreeNode::Type::UnstructuredArray: + if (false == serialize_value_array(val, m_value_group_buf)) { + return false; + } + break; + + default: + // Unknown schema tree node type + return false; + } + return true; +} + // Explicitly declare template specializations so that we can define the template methods in this // file template auto Serializer::create( @@ -69,4 +467,28 @@ template auto Serializer::change_utc_offset(UtcOf ) -> void; template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; +template auto Serializer::serialize_log_event( + msgpack::object const& log_event +) -> bool; +template auto Serializer::serialize_log_event( + msgpack::object const& log_event +) -> bool; +template auto Serializer::serialize_schema_tree_node( + SchemaTree::NodeLocator const& locator +) -> bool; +template auto Serializer::serialize_schema_tree_node( + SchemaTree::NodeLocator const& locator +) -> bool; +template auto Serializer::serialize_key(SchemaTreeNode::id_t id +) -> bool; +template auto Serializer::serialize_key(SchemaTreeNode::id_t id +) -> bool; +template auto Serializer::serialize_val( + msgpack::object const& val, + SchemaTreeNode::Type type +) -> bool; +template auto Serializer::serialize_val( + msgpack::object const& val, + SchemaTreeNode::Type type +) -> bool; } // namespace clp::ffi::ir_stream diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index 4c8d48fc6..c76c2dcbf 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -6,9 +6,11 @@ #include #include +#include #include "../../time_types.hpp" #include "../SchemaTree.hpp" +#include "../SchemaTreeNode.hpp" namespace clp::ffi::ir_stream { /** @@ -79,10 +81,41 @@ class Serializer { */ auto change_utc_offset(UtcOffset utc_offset) -> void; + /** + * Serializes the given log event. + * @param log_event Key-value pair log event to serialize, represented as a msgpack object. + * @return Whether the serialization succeeded. + */ + [[nodiscard]] auto serialize_log_event(msgpack::object const& log_event) -> bool; + private: // Constructors Serializer() = default; + // Methods + /** + * Serializes a schema tree node identified by the given locator into `m_schema_tree_node_buf`. + * @param locator + * @return Whether the serialization succeeded. + */ + [[nodiscard]] auto serialize_schema_tree_node(SchemaTree::NodeLocator const& locator) -> bool; + + /** + * Serializes the given key id into `m_key_group_buf`. + * @param id + * @return true on success. + * @return false if the id value exceeds the representable range. + */ + [[nodiscard]] auto serialize_key(SchemaTreeNode::id_t id) -> bool; + + /** + * Serializes the given msgpack value into `m_value_group_buf`. + * @param val + * @param type The corresponded `SchemaTreeNode::Type` of `val`. + * @return Whether the serialization succeeded. + */ + [[nodiscard]] auto serialize_val(msgpack::object const& val, SchemaTreeNode::Type type) -> bool; + UtcOffset m_curr_utc_offset{0}; Buffer m_ir_buf; SchemaTree m_schema_tree; diff --git a/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp index 35363e2d3..128d659c1 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp @@ -135,6 +135,22 @@ bool serialize_log_event( string_view message, string& logtype, vector& ir_buf +) { + if (false == serialize_message(message, logtype, ir_buf)) { + return false; + } + + // Encode timestamp + ir_buf.push_back(cProtocol::Payload::TimestampVal); + serialize_int(timestamp, ir_buf); + + return true; +} + +bool serialize_message( + std::string_view message, + std::string& logtype, + std::vector& ir_buf ) { auto encoded_var_handler = [&ir_buf](eight_byte_encoded_variable_t encoded_var) { ir_buf.push_back(cProtocol::Payload::VarEightByteEncoding); @@ -153,15 +169,7 @@ bool serialize_log_event( return false; } - if (false == serialize_logtype(logtype, ir_buf)) { - return false; - } - - // Encode timestamp - ir_buf.push_back(cProtocol::Payload::TimestampVal); - serialize_int(timestamp, ir_buf); - - return true; + return serialize_logtype(logtype, ir_buf); } } // namespace eight_byte_encoding diff --git a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp index 0129e7550..778b4cd30 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp @@ -39,6 +39,16 @@ bool serialize_log_event( std::string& logtype, std::vector& ir_buf ); + +/** + * Serializes the given message into the eight-byte encoding IR stream + * delta + * @param message + * @param logtype + * @param ir_buf + * @return true on success, false otherwise + */ +bool serialize_message(std::string_view message, std::string& logtype, std::vector& ir_buf); } // namespace eight_byte_encoding namespace four_byte_encoding { diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index f9fd7ed78..f4b25c2ff 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -50,6 +50,35 @@ constexpr int8_t TimestampDeltaInt = 0x33; constexpr int8_t TimestampDeltaLong = 0x34; constexpr int8_t UtcOffsetChange = 0x3F; + +constexpr int8_t StrPacketLenUByte = 0x41; +constexpr int8_t StrPacketLenUShort = 0x41; +constexpr int8_t StrPacketLenUInt = 0x41; + +constexpr int8_t SchemaTreeNodeInt = 0x71; +constexpr int8_t SchemaTreeNodeFloat = 0x72; +constexpr int8_t SchemaTreeNodeBool = 0x73; +constexpr int8_t SchemaTreeNodeStr = 0x74; +constexpr int8_t SchemaTreeNodeUnstructuredArray = 0x75; +constexpr int8_t SchemaTreeNodeObj = 0x76; + +constexpr int8_t SchemaTreeNodeParentIdUByte = 0x60; +constexpr int8_t SchemaTreeNodeParentIdUShort = 0x61; + +constexpr int8_t KeyIdUByte = 0x65; +constexpr int8_t KeyIdUShort = 0x66; + +constexpr int8_t ValueInt8 = 0x51; +constexpr int8_t ValueInt16 = 0x52; +constexpr int8_t ValueInt32 = 0x53; +constexpr int8_t ValueInt64 = 0x54; +constexpr int8_t ValueDouble = 0x55; +constexpr int8_t ValueTrue = 0x56; +constexpr int8_t ValueFalse = 0x57; +constexpr int8_t ValueFourByteEncodingClpStr = 0x58; +constexpr int8_t ValueEightByteEncodingClpStr = 0x59; +constexpr int8_t ValueEmpty = 0x5E; +constexpr int8_t ValueNull = 0x5F; } // namespace Payload constexpr int8_t FourByteEncodingMagicNumber[] diff --git a/components/core/src/clp/ffi/ir_stream/utils.cpp b/components/core/src/clp/ffi/ir_stream/utils.cpp index 0a22536b6..00cac3bcd 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/utils.cpp @@ -1,6 +1,7 @@ #include "utils.hpp" #include +#include #include #include @@ -29,4 +30,23 @@ auto serialize_metadata(nlohmann::json& metadata, std::vector& ir_buf) - return true; } + +auto serialize_string_packet(std::string_view str, std::vector& buf) -> bool { + auto const length{str.length()}; + if (length <= UINT8_MAX) { + buf.push_back(cProtocol::Payload::StrPacketLenUByte); + buf.push_back(static_cast(static_cast(length))); + } else if (length <= UINT16_MAX) { + buf.push_back(cProtocol::Payload::StrPacketLenUShort); + serialize_int(static_cast(length), buf); + } else if (length <= UINT32_MAX) { + buf.push_back(cProtocol::Payload::StrPacketLenUInt); + serialize_int(static_cast(length), buf); + } else { + // Out of range + return false; + } + buf.insert(buf.cend(), str.cbegin(), str.cend()); + return true; +} } // namespace clp::ffi::ir_stream diff --git a/components/core/src/clp/ffi/ir_stream/utils.hpp b/components/core/src/clp/ffi/ir_stream/utils.hpp index 5e1fb293a..d39629a72 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/utils.hpp @@ -3,11 +3,16 @@ #include #include +#include +#include #include #include +#include "../../ir/types.hpp" #include "byteswap.hpp" +#include "encoding_methods.hpp" +#include "protocol_constants.hpp" namespace clp::ffi::ir_stream { /** @@ -28,6 +33,24 @@ serialize_metadata(nlohmann::json& metadata, std::vector& ir_buf) -> boo template auto serialize_int(integer_t value, std::vector& ir_buf) -> void; +/** + * Serializes a string using clp encoding. + * @tparam encoded_variable_t + * @param str + * @param buf Outputs the serialized byte sequence. + * @return Whether serialization succeeded. + */ +template +[[nodiscard]] auto serialize_clp_string(std::string_view str, std::vector& buf) -> bool; + +/** + * Serializes a string packet. + * @param str + * @param buf Outputs the serialized byte sequence. + * @return Whether the serialization succeeded. + */ +[[nodiscard]] auto serialize_string_packet(std::string_view str, std::vector& buf) -> bool; + template auto serialize_int(integer_t value, std::vector& ir_buf) -> void { integer_t value_big_endian{}; @@ -43,5 +66,23 @@ auto serialize_int(integer_t value, std::vector& ir_buf) -> void { std::span const data_view{reinterpret_cast(&value_big_endian), sizeof(value)}; ir_buf.insert(ir_buf.end(), data_view.begin(), data_view.end()); } + +template +[[nodiscard]] auto serialize_clp_string(std::string_view str, std::vector& buf) -> bool { + static_assert( + (std::is_same_v + || std::is_same_v) + ); + std::string logtype; + bool error{}; + if constexpr (std::is_same_v) { + buf.push_back(cProtocol::Payload::ValueFourByteEncodingClpStr); + error = four_byte_encoding::serialize_message(str, logtype, buf); + } else { + buf.push_back(cProtocol::Payload::ValueEightByteEncodingClpStr); + error = eight_byte_encoding::serialize_message(str, logtype, buf); + } + return error; +} } // namespace clp::ffi::ir_stream #endif From c8cc82e9b3b2a467eef08ab28c9781a58eb0ed27 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 29 Jun 2024 13:48:53 -0400 Subject: [PATCH 02/17] Using clp bit cast instead --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 1a900bcf0..b7fd4cb55 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -1,6 +1,5 @@ #include "Serializer.hpp" -#include #include #include #include @@ -17,6 +16,7 @@ #include "../../ir/types.hpp" #include "../../time_types.hpp" +#include "../../type_utils.hpp" #include "../encoding_methods.hpp" #include "../SchemaTree.hpp" #include "../SchemaTreeNode.hpp" @@ -188,7 +188,7 @@ auto serialize_value_int(int64_t val, vector& buf) -> void { } auto serialize_value_float(double val, vector& buf) -> void { - serialize_int(std::bit_cast(val), buf); + serialize_int(bit_cast(val), buf); } auto serialize_value_bool(bool val, vector& buf) -> void { From 61fb0d3ad34849827d599740676fadc7f5bdc8e4 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 29 Jun 2024 17:13:55 -0400 Subject: [PATCH 03/17] Bug fix --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 1 + components/core/src/clp/ffi/ir_stream/protocol_constants.hpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index b7fd4cb55..0c4bcc7de 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -188,6 +188,7 @@ auto serialize_value_int(int64_t val, vector& buf) -> void { } auto serialize_value_float(double val, vector& buf) -> void { + buf.push_back(cProtocol::Payload::ValueFloat); serialize_int(bit_cast(val), buf); } diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index f4b25c2ff..473027470 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -72,7 +72,7 @@ constexpr int8_t ValueInt8 = 0x51; constexpr int8_t ValueInt16 = 0x52; constexpr int8_t ValueInt32 = 0x53; constexpr int8_t ValueInt64 = 0x54; -constexpr int8_t ValueDouble = 0x55; +constexpr int8_t ValueFloat = 0x55; constexpr int8_t ValueTrue = 0x56; constexpr int8_t ValueFalse = 0x57; constexpr int8_t ValueFourByteEncodingClpStr = 0x58; From 2f823ba1427eab0f92ba64adb9ebf306a3a26a1b Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sat, 29 Jun 2024 19:01:40 -0400 Subject: [PATCH 04/17] Update protocol constants --- components/core/src/clp/ffi/ir_stream/protocol_constants.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index 473027470..6455dff31 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -52,8 +52,8 @@ constexpr int8_t TimestampDeltaLong = 0x34; constexpr int8_t UtcOffsetChange = 0x3F; constexpr int8_t StrPacketLenUByte = 0x41; -constexpr int8_t StrPacketLenUShort = 0x41; -constexpr int8_t StrPacketLenUInt = 0x41; +constexpr int8_t StrPacketLenUShort = 0x42; +constexpr int8_t StrPacketLenUInt = 0x43; constexpr int8_t SchemaTreeNodeInt = 0x71; constexpr int8_t SchemaTreeNodeFloat = 0x72; From e9ba6dec7ecaecc7154f1e846a97cde858bd5fa0 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 2 Jul 2024 20:15:04 -0400 Subject: [PATCH 05/17] Add unit tests --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 15 +-- .../core/src/clp/ffi/ir_stream/Serializer.hpp | 6 +- .../core/tests/test-ir_encoding_methods.cpp | 113 ++++++++++++++++++ 3 files changed, 124 insertions(+), 10 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 0c4bcc7de..24d162573 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -259,13 +259,14 @@ auto Serializer::change_utc_offset(UtcOffset utc_offset) -> } template -auto Serializer::serialize_log_event(msgpack::object const& log_event) -> bool { - if (msgpack::type::MAP != log_event.type) { +auto Serializer::serialize_msgpack_map(msgpack::object const& msgpack_map +) -> bool { + if (msgpack::type::MAP != msgpack_map.type) { return false; } // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) - auto const& map{log_event.via.map}; + auto const& map{msgpack_map.via.map}; if (0 == map.size) { serialize_empty_object(m_ir_buf); return true; @@ -468,11 +469,11 @@ template auto Serializer::change_utc_offset(UtcOf ) -> void; template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; -template auto Serializer::serialize_log_event( - msgpack::object const& log_event +template auto Serializer::serialize_msgpack_map( + msgpack::object const& msgpack_map ) -> bool; -template auto Serializer::serialize_log_event( - msgpack::object const& log_event +template auto Serializer::serialize_msgpack_map( + msgpack::object const& msgpack_map ) -> bool; template auto Serializer::serialize_schema_tree_node( SchemaTree::NodeLocator const& locator diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index c76c2dcbf..6b2d41139 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -82,11 +82,11 @@ class Serializer { auto change_utc_offset(UtcOffset utc_offset) -> void; /** - * Serializes the given log event. - * @param log_event Key-value pair log event to serialize, represented as a msgpack object. + * Serializes the given msgpack map as a key-value pair log event. + * @param msgpack_map msgpack key-value pair map to serialize, representing an log event. * @return Whether the serialization succeeded. */ - [[nodiscard]] auto serialize_log_event(msgpack::object const& log_event) -> bool; + [[nodiscard]] auto serialize_msgpack_map(msgpack::object const& msgpack_map) -> bool; private: // Constructors diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 167803e58..4ee92866f 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -1,9 +1,12 @@ #include +#include +#include #include #include #include #include +#include #include "../src/clp/BufferReader.hpp" #include "../src/clp/ErrorCode.hpp" @@ -132,6 +135,19 @@ auto flush_and_clear_serializer_buffer( std::vector& byte_buf ) -> void; +/** + * Unpacks and serializes the given msgpack bytes using kv serializer. + * @tparam encoded_variable_t + * @param serializer + * @param msgpack_bytes + * @return Whether the serialization is succeeded. + */ +template +[[nodiscard]] auto unpack_and_serialize_msgpack_bytes( + Serializer& serializer, + vector const& msgpack_bytes +) -> bool; + template [[nodiscard]] auto serialize_log_events( vector const& log_events, @@ -241,6 +257,20 @@ auto flush_and_clear_serializer_buffer( byte_buf.insert(byte_buf.cend(), view.begin(), view.end()); serializer.clear_ir_buf(); } + +template +auto unpack_and_serialize_msgpack_bytes( + Serializer& serializer, + vector const& msgpack_bytes +) -> bool { + msgpack::object_handle msgpack_oh; + msgpack::unpack( + msgpack_oh, + clp::size_checked_pointer_cast(msgpack_bytes.data()), + msgpack_bytes.size() + ); + return serializer.serialize_msgpack_map(msgpack_oh.get()); +} } // namespace /** @@ -971,3 +1001,86 @@ TEMPLATE_TEST_CASE( && 0 == num_bytes_read) ); } + +TEMPLATE_TEST_CASE( + "ffi_ir_stream_Serializer_serialize_msgpack", + "[clp][ffi][ir_stream][Serializer]", + four_byte_encoded_variable_t, + eight_byte_encoded_variable_t +) { + vector ir_buf; + + auto result{Serializer::create()}; + REQUIRE((false == result.has_error())); + + auto& serializer{result.value()}; + flush_and_clear_serializer_buffer(serializer, ir_buf); + + auto const empty_array = nlohmann::json::parse("[]"); + auto const empty_obj = nlohmann::json::parse("{}"); + REQUIRE(unpack_and_serialize_msgpack_bytes(serializer, nlohmann::json::to_msgpack(empty_obj))); + + // TODO: + // Before kv-pair IR Deserializer is implemented, we cannot test whether the serialized bytes + // can be correctly deserialized. We should improve the test coverage once we have a + // deserializer implementation. + + // Test encoding basic object + constexpr string_view cShortString{"short_string"}; + constexpr string_view cClpString{"uid=0, CPU usage: 99.99%, \"user_name\"=YScope"}; + nlohmann::json const basic_obj + = {{"int8_max", INT8_MAX}, + {"int8_min", INT8_MIN}, + {"int16_max", INT16_MAX}, + {"int16_min", INT16_MIN}, + {"int32_max", INT32_MAX}, + {"int32_min", INT32_MIN}, + {"int64_max", INT64_MAX}, + {"int64_min", INT64_MIN}, + {"float_zero", 0.0}, + {"float_pos", 1.01}, + {"float_neg", -1.01}, + {"true", true}, + {"false", false}, + {"string", cShortString}, + {"clp_string", cClpString}, + {"null", nullptr}, + {"empty_object", empty_obj}, + {"empty_array", empty_array}}; + + REQUIRE(unpack_and_serialize_msgpack_bytes(serializer, nlohmann::json::to_msgpack(basic_obj))); + + auto basic_array = empty_array; + basic_array.emplace_back(1); + basic_array.emplace_back(1.0); + basic_array.emplace_back(true); + basic_array.emplace_back(cShortString); + basic_array.emplace_back(cClpString); + basic_array.emplace_back(nullptr); + basic_array.emplace_back(empty_array); + for (auto const& element : basic_array) { + // Non-map objects should fail the serialization + REQUIRE( + (false + == unpack_and_serialize_msgpack_bytes( + serializer, + nlohmann::json::to_msgpack(element) + )) + ); + } + basic_array.emplace_back(empty_obj); + + // Recursively construct an object containing inner maps and inner arrays. + auto recursive_obj = basic_obj; + auto recursive_array = basic_array; + constexpr size_t cRecursiveDepth{5}; + for (size_t i{0}; i < cRecursiveDepth; ++i) { + recursive_array.emplace_back(recursive_obj); + recursive_obj.emplace("obj_" + std::to_string(i), recursive_obj); + recursive_obj.emplace("array_" + std::to_string(i), recursive_array); + REQUIRE(unpack_and_serialize_msgpack_bytes( + serializer, + nlohmann::json::to_msgpack(recursive_obj) + )); + } +} From 317f07a1249c7db216fd9eded16d4e56d01abff2 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:59:51 -0400 Subject: [PATCH 06/17] Apply suggestions from code review Co-authored-by: Devin Gibson --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 2 +- components/core/tests/test-ir_encoding_methods.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 24d162573..463387472 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -84,7 +84,7 @@ class MsgpackMapNode { /** * Serializes a value corresponded to an empty object. - * @parma buf Outputs the serialized byte sequence. + * @param buf Outputs the serialized byte sequence. */ auto serialize_empty_object(vector& buf) -> void; diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 4ee92866f..5b27724fc 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -140,7 +140,7 @@ auto flush_and_clear_serializer_buffer( * @tparam encoded_variable_t * @param serializer * @param msgpack_bytes - * @return Whether the serialization is succeeded. + * @return Whether the serialization is successful. */ template [[nodiscard]] auto unpack_and_serialize_msgpack_bytes( From a942d8c8c83c894d6d393381167e3076c23b59a4 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Sun, 7 Jul 2024 23:12:36 -0400 Subject: [PATCH 07/17] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 9 ++++++--- .../core/src/clp/ffi/ir_stream/encoding_methods.hpp | 2 +- components/core/src/clp/ffi/ir_stream/utils.cpp | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 463387472..4222ff537 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -145,7 +145,7 @@ auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val ret_val.emplace(SchemaTreeNode::Type::Int); break; case msgpack::type::FLOAT32: - case msgpack::type::FLOAT: + case msgpack::type::FLOAT64: ret_val.emplace(SchemaTreeNode::Type::Float); break; case msgpack::type::STR: @@ -286,14 +286,15 @@ auto Serializer::serialize_msgpack_map(msgpack::object const working_stack.pop_back(); continue; } + auto const& [key, val]{curr.get_next_child()}; auto const opt_schema_tree_node_type{get_schema_tree_node_type_from_msgpack_val(val)}; if (false == opt_schema_tree_node_type.has_value()) { failure = true; break; } - auto const schema_tree_node_type{opt_schema_tree_node_type.value()}; + SchemaTree::NodeLocator const locator{ curr.get_parent_id(), key.as(), @@ -377,6 +378,7 @@ auto Serializer::serialize_schema_tree_node( // Unknown type return false; } + auto const parent_id{locator.get_parent_id()}; if (parent_id <= UINT8_MAX) { m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUByte); @@ -388,6 +390,7 @@ auto Serializer::serialize_schema_tree_node( // Out of range return false; } + return serialize_string_packet(locator.get_key_name(), m_schema_tree_node_buf); } @@ -395,7 +398,7 @@ template auto Serializer::serialize_key(SchemaTreeNode::id_t id) -> bool { if (id <= UINT8_MAX) { m_key_group_buf.push_back(cProtocol::Payload::KeyIdUByte); - m_key_group_buf.push_back(static_cast(static_cast(id))); + m_key_group_buf.push_back(bit_cast(static_cast(id))); } else if (id <= UINT16_MAX) { m_key_group_buf.push_back(cProtocol::Payload::KeyIdUShort); serialize_int(static_cast(id), m_key_group_buf); diff --git a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp index 778b4cd30..a60bf51d9 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp @@ -46,7 +46,7 @@ bool serialize_log_event( * @param message * @param logtype * @param ir_buf - * @return true on success, false otherwise + * @return Whether the message was serialized successfully. */ bool serialize_message(std::string_view message, std::string& logtype, std::vector& ir_buf); } // namespace eight_byte_encoding diff --git a/components/core/src/clp/ffi/ir_stream/utils.cpp b/components/core/src/clp/ffi/ir_stream/utils.cpp index 00cac3bcd..5cfe08a5a 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/utils.cpp @@ -35,7 +35,7 @@ auto serialize_string_packet(std::string_view str, std::vector& buf) -> auto const length{str.length()}; if (length <= UINT8_MAX) { buf.push_back(cProtocol::Payload::StrPacketLenUByte); - buf.push_back(static_cast(static_cast(length))); + buf.push_back(bit_cast(static_cast(length))); } else if (length <= UINT16_MAX) { buf.push_back(cProtocol::Payload::StrPacketLenUShort); serialize_int(static_cast(length), buf); From 156ae44c17e2bfda19afee680a0b6b3f46bb01e0 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 8 Jul 2024 00:19:05 -0400 Subject: [PATCH 08/17] Add comments --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 4222ff537..817460ceb 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -36,15 +36,15 @@ using clp::ir::four_byte_encoded_variable_t; namespace clp::ffi::ir_stream { namespace { /** - * Class for tracking states when traversing a msgpack map using pre-order traversal. + * Class for iterating a msgpack map. */ -class MsgpackMapNode { +class MsgpackMapIterator { public: // Types using Child = msgpack::object_kv; // Constructors - MsgpackMapNode(SchemaTreeNode::id_t parent_id, Child* child_data, size_t child_data_length) + MsgpackMapIterator(SchemaTreeNode::id_t parent_id, Child* child_data, size_t child_data_length) : m_parent_id{parent_id}, m_children{child_data, child_data_length} {} @@ -277,16 +277,19 @@ auto Serializer::serialize_msgpack_map(msgpack::object const m_key_group_buf.clear(); m_value_group_buf.clear(); + // Traverse the map from the root using DFS iteratively. bool failure{false}; - vector working_stack; + vector working_stack; working_stack.emplace_back(SchemaTree::cRootId, map.ptr, static_cast(map.size)); while (false == working_stack.empty()) { auto& curr{working_stack.back()}; if (false == curr.has_next_child()) { + // All child has been visited. Pop it out from the working stack working_stack.pop_back(); continue; } + // Convert the type of the current value to its corresponded schema tree node type auto const& [key, val]{curr.get_next_child()}; auto const opt_schema_tree_node_type{get_schema_tree_node_type_from_msgpack_val(val)}; if (false == opt_schema_tree_node_type.has_value()) { @@ -301,6 +304,7 @@ auto Serializer::serialize_msgpack_map(msgpack::object const schema_tree_node_type }; + // Get the corresponded node in the schema tree. If the node doesn't exist yet, create one auto opt_schema_tree_node_id{m_schema_tree.try_get_node_id(locator)}; if (false == opt_schema_tree_node_id.has_value()) { opt_schema_tree_node_id.emplace(m_schema_tree.insert_node(locator)); @@ -312,6 +316,8 @@ auto Serializer::serialize_msgpack_map(msgpack::object const auto const schema_tree_node_id{opt_schema_tree_node_id.value()}; if (SchemaTreeNode::Type::Obj == schema_tree_node_type && msgpack::type::MAP == val.type) { + // Serialize sub-maps. If the sub-map is not empty, push an iterator of the sub-map into + // the working stack to continue DFS // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) auto const& inner_map{val.via.map}; auto const inner_map_size(static_cast(inner_map.size)); @@ -325,7 +331,7 @@ auto Serializer::serialize_msgpack_map(msgpack::object const working_stack.emplace_back(schema_tree_node_id, inner_map.ptr, inner_map_size); } } else { - // Serialize primitive values + // A primitive value is reached. Directly serialize its key and value into the IR stream if (false == (serialize_key(schema_tree_node_id) && serialize_val(val, schema_tree_node_type) )) From bf4cbc45298980fda5e77c5e6f3274781e6d221f Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 8 Jul 2024 11:28:53 -0400 Subject: [PATCH 09/17] take a map directly --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 20 +++++++++---------- .../core/src/clp/ffi/ir_stream/Serializer.hpp | 2 +- .../core/tests/test-ir_encoding_methods.cpp | 6 +++++- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 817460ceb..962e882e1 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -259,15 +259,9 @@ auto Serializer::change_utc_offset(UtcOffset utc_offset) -> } template -auto Serializer::serialize_msgpack_map(msgpack::object const& msgpack_map +auto Serializer::serialize_msgpack_map(msgpack::object_map const& msgpack_map ) -> bool { - if (msgpack::type::MAP != msgpack_map.type) { - return false; - } - - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) - auto const& map{msgpack_map.via.map}; - if (0 == map.size) { + if (0 == msgpack_map.size) { serialize_empty_object(m_ir_buf); return true; } @@ -280,7 +274,11 @@ auto Serializer::serialize_msgpack_map(msgpack::object const // Traverse the map from the root using DFS iteratively. bool failure{false}; vector working_stack; - working_stack.emplace_back(SchemaTree::cRootId, map.ptr, static_cast(map.size)); + working_stack.emplace_back( + SchemaTree::cRootId, + msgpack_map.ptr, + static_cast(msgpack_map.size) + ); while (false == working_stack.empty()) { auto& curr{working_stack.back()}; if (false == curr.has_next_child()) { @@ -479,10 +477,10 @@ template auto Serializer::change_utc_offset(UtcOf template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; template auto Serializer::serialize_msgpack_map( - msgpack::object const& msgpack_map + msgpack::object_map const& msgpack_map ) -> bool; template auto Serializer::serialize_msgpack_map( - msgpack::object const& msgpack_map + msgpack::object_map const& msgpack_map ) -> bool; template auto Serializer::serialize_schema_tree_node( SchemaTree::NodeLocator const& locator diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index 6b2d41139..feec14440 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -86,7 +86,7 @@ class Serializer { * @param msgpack_map msgpack key-value pair map to serialize, representing an log event. * @return Whether the serialization succeeded. */ - [[nodiscard]] auto serialize_msgpack_map(msgpack::object const& msgpack_map) -> bool; + [[nodiscard]] auto serialize_msgpack_map(msgpack::object_map const& msgpack_map) -> bool; private: // Constructors diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 5b27724fc..48dd00370 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -269,7 +269,11 @@ auto unpack_and_serialize_msgpack_bytes( clp::size_checked_pointer_cast(msgpack_bytes.data()), msgpack_bytes.size() ); - return serializer.serialize_msgpack_map(msgpack_oh.get()); + auto const& msgpack_obj{msgpack_oh.get()}; + if (msgpack::type::MAP != msgpack_obj.type) { + return false; + } + return serializer.serialize_msgpack_map(msgpack_obj.via.map); } } // namespace From 8d74f00aac8352010ad8667122b15ab2213249b0 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 8 Jul 2024 11:31:34 -0400 Subject: [PATCH 10/17] Rename string serialization method --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 4 ++-- components/core/src/clp/ffi/ir_stream/utils.cpp | 2 +- components/core/src/clp/ffi/ir_stream/utils.hpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 962e882e1..02939c165 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -203,7 +203,7 @@ auto serialize_value_null(vector& buf) -> void { template auto serialize_value_string(string_view val, vector& buf) -> bool { if (string_view::npos == val.find(' ')) { - return serialize_string_packet(val, buf); + return serialize_string(val, buf); } return serialize_clp_string(val, buf); } @@ -395,7 +395,7 @@ auto Serializer::serialize_schema_tree_node( return false; } - return serialize_string_packet(locator.get_key_name(), m_schema_tree_node_buf); + return serialize_string(locator.get_key_name(), m_schema_tree_node_buf); } template diff --git a/components/core/src/clp/ffi/ir_stream/utils.cpp b/components/core/src/clp/ffi/ir_stream/utils.cpp index 5cfe08a5a..019efad13 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/utils.cpp @@ -31,7 +31,7 @@ auto serialize_metadata(nlohmann::json& metadata, std::vector& ir_buf) - return true; } -auto serialize_string_packet(std::string_view str, std::vector& buf) -> bool { +auto serialize_string(std::string_view str, std::vector& buf) -> bool { auto const length{str.length()}; if (length <= UINT8_MAX) { buf.push_back(cProtocol::Payload::StrPacketLenUByte); diff --git a/components/core/src/clp/ffi/ir_stream/utils.hpp b/components/core/src/clp/ffi/ir_stream/utils.hpp index d39629a72..f317c9036 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/utils.hpp @@ -49,7 +49,7 @@ template * @param buf Outputs the serialized byte sequence. * @return Whether the serialization succeeded. */ -[[nodiscard]] auto serialize_string_packet(std::string_view str, std::vector& buf) -> bool; +[[nodiscard]] auto serialize_string(std::string_view str, std::vector& buf) -> bool; template auto serialize_int(integer_t value, std::vector& ir_buf) -> void { From dec9f56853c3dddc0d34d5ee4a4915ef72fb01b2 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 8 Jul 2024 11:49:50 -0400 Subject: [PATCH 11/17] Buffer logtypes --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 24 +++++++++++++------ .../core/src/clp/ffi/ir_stream/Serializer.hpp | 2 ++ .../core/src/clp/ffi/ir_stream/utils.hpp | 8 ++++--- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 02939c165..3f2e792e7 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -120,21 +120,25 @@ auto serialize_value_null(vector& buf) -> void; * Serializes a value of string type. * @tparam encoded_variable_t * @param val + * @param logtype_buf * @param buf Outputs the serialized byte sequence. * @return Whether serialization succeeded. */ template -[[nodiscard]] auto serialize_value_string(string_view val, vector& buf) -> bool; +[[nodiscard]] auto +serialize_value_string(string_view val, string& logtype_buf, vector& buf) -> bool; /** * Serializes a msgpack array as a JSON string, using clp encoding. * @tparam encoded_variable_t * @param val + * @param logtype_buf * @param buf Outputs the serialized byte sequence. * @return Whether serialization succeeded. */ template -[[nodiscard]] auto serialize_value_array(msgpack::object const& val, vector& buf) -> bool; +[[nodiscard]] auto +serialize_value_array(msgpack::object const& val, string& logtype_buf, vector& buf) -> bool; auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val ) -> optional { @@ -201,18 +205,21 @@ auto serialize_value_null(vector& buf) -> void { } template -auto serialize_value_string(string_view val, vector& buf) -> bool { +auto serialize_value_string(string_view val, string& logtype_buf, vector& buf) -> bool { if (string_view::npos == val.find(' ')) { return serialize_string(val, buf); } - return serialize_clp_string(val, buf); + logtype_buf.clear(); + return serialize_clp_string(val, logtype_buf, buf); } template -auto serialize_value_array(msgpack::object const& val, vector& buf) -> bool { +auto serialize_value_array(msgpack::object const& val, string& logtype_buf, vector& buf) + -> bool { std::ostringstream oss; oss << val; - return serialize_clp_string(oss.str(), buf); + logtype_buf.clear(); + return serialize_clp_string(oss.str(), logtype_buf, buf); } } // namespace @@ -439,6 +446,7 @@ auto Serializer::serialize_val( if (false == serialize_value_string( val.as(), + m_logtype_buf, m_value_group_buf )) { @@ -454,7 +462,9 @@ auto Serializer::serialize_val( break; case SchemaTreeNode::Type::UnstructuredArray: - if (false == serialize_value_array(val, m_value_group_buf)) { + if (false + == serialize_value_array(val, m_logtype_buf, m_value_group_buf)) + { return false; } break; diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index feec14440..af2b50c6a 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -120,6 +121,7 @@ class Serializer { Buffer m_ir_buf; SchemaTree m_schema_tree; + std::string m_logtype_buf; Buffer m_schema_tree_node_buf; Buffer m_key_group_buf; Buffer m_value_group_buf; diff --git a/components/core/src/clp/ffi/ir_stream/utils.hpp b/components/core/src/clp/ffi/ir_stream/utils.hpp index f317c9036..3b96a9ee5 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/utils.hpp @@ -37,11 +37,13 @@ auto serialize_int(integer_t value, std::vector& ir_buf) -> void; * Serializes a string using clp encoding. * @tparam encoded_variable_t * @param str + * @param logtype Outputs the logtype of the serialized clp string. * @param buf Outputs the serialized byte sequence. * @return Whether serialization succeeded. */ template -[[nodiscard]] auto serialize_clp_string(std::string_view str, std::vector& buf) -> bool; +[[nodiscard]] auto +serialize_clp_string(std::string_view str, std::string& logtype, std::vector& buf) -> bool; /** * Serializes a string packet. @@ -68,12 +70,12 @@ auto serialize_int(integer_t value, std::vector& ir_buf) -> void { } template -[[nodiscard]] auto serialize_clp_string(std::string_view str, std::vector& buf) -> bool { +[[nodiscard]] auto +serialize_clp_string(std::string_view str, std::string& logtype, std::vector& buf) -> bool { static_assert( (std::is_same_v || std::is_same_v) ); - std::string logtype; bool error{}; if constexpr (std::is_same_v) { buf.push_back(cProtocol::Payload::ValueFourByteEncodingClpStr); From ff00b23ca25b565f8655acaf07a788d04351884b Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Tue, 16 Jul 2024 20:51:41 -0400 Subject: [PATCH 12/17] Edit comments and some minor refactoring. --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 58 ++++++++++--------- .../core/src/clp/ffi/ir_stream/Serializer.hpp | 16 ++--- .../core/src/clp/ffi/ir_stream/utils.hpp | 20 +++---- .../core/tests/test-ir_encoding_methods.cpp | 13 ++--- 4 files changed, 53 insertions(+), 54 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 3f2e792e7..c40a89eb5 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -36,7 +36,7 @@ using clp::ir::four_byte_encoded_variable_t; namespace clp::ffi::ir_stream { namespace { /** - * Class for iterating a msgpack map. + * Class for iterating the kv-pairs of a MessagePack map. */ class MsgpackMapIterator { public: @@ -50,12 +50,12 @@ class MsgpackMapIterator { // Methods /** - * @return The id of the parent in the schema tree. + * @return This map's ID in the schema tree. */ [[nodiscard]] auto get_parent_id() const -> SchemaTreeNode::id_t { return m_parent_id; } /** - * @return Whether it has more child to traverse. + * @return Whether there are more children to traverse. */ [[nodiscard]] auto has_next_child() const -> bool { return m_children.size() > m_curr_child_idx; @@ -74,54 +74,54 @@ class MsgpackMapIterator { }; /** - * Gets the corresponded schema tree node type from the given msgpack value. - * @param val msgpack value. - * @return The corresponded schema tree node type. - * @return std::nullopt if the value doesn't match any of the supported schema tree node type. + * Gets the schema-tree node type that corresponds with a given MessagePack value. + * @param val + * @return The corresponding schema-tree node type. + * @return std::nullopt if the value doesn't match any of the supported schema-tree node types. */ [[nodiscard]] auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val ) -> optional; /** - * Serializes a value corresponded to an empty object. - * @param buf Outputs the serialized byte sequence. + * Serializes an empty object. + * @param buf */ auto serialize_empty_object(vector& buf) -> void; /** - * Serializes a value of integer type. + * Serializes an integer. * @param val - * @param buf Outputs the serialized byte sequence. + * @param buf * @return Whether serialization succeeded. */ auto serialize_value_int(int64_t val, vector& buf) -> void; /** - * Serializes a value of float type. + * Serializes a float. * @param val - * @param buf Outputs the serialized byte sequence. + * @param buf */ auto serialize_value_float(double val, vector& buf) -> void; /** - * Serializes a value of bool. + * Serializes a boolean. * @param val - * @param buf Outputs the serialized byte sequence. + * @param buf */ auto serialize_value_bool(bool val, vector& buf) -> void; /** - * Serializes a value of NULL type. - * @param buf Outputs the serialized byte sequence. + * Serializes a null. + * @param buf */ auto serialize_value_null(vector& buf) -> void; /** - * Serializes a value of string type. + * Serializes a string. * @tparam encoded_variable_t * @param val * @param logtype_buf - * @param buf Outputs the serialized byte sequence. + * @param buf * @return Whether serialization succeeded. */ template @@ -129,11 +129,11 @@ template serialize_value_string(string_view val, string& logtype_buf, vector& buf) -> bool; /** - * Serializes a msgpack array as a JSON string, using clp encoding. + * Serializes a MessagePack array as a JSON string, using CLP's encoding for unstructured text. * @tparam encoded_variable_t * @param val * @param logtype_buf - * @param buf Outputs the serialized byte sequence. + * @param buf * @return Whether serialization succeeded. */ template @@ -278,7 +278,7 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c m_key_group_buf.clear(); m_value_group_buf.clear(); - // Traverse the map from the root using DFS iteratively. + // Traverse the map using DFS iteratively bool failure{false}; vector working_stack; working_stack.emplace_back( @@ -289,12 +289,12 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c while (false == working_stack.empty()) { auto& curr{working_stack.back()}; if (false == curr.has_next_child()) { - // All child has been visited. Pop it out from the working stack + // Visited all children, so pop node working_stack.pop_back(); continue; } - // Convert the type of the current value to its corresponded schema tree node type + // Convert the current value's type to its corresponding schema-tree node type auto const& [key, val]{curr.get_next_child()}; auto const opt_schema_tree_node_type{get_schema_tree_node_type_from_msgpack_val(val)}; if (false == opt_schema_tree_node_type.has_value()) { @@ -309,7 +309,8 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c schema_tree_node_type }; - // Get the corresponded node in the schema tree. If the node doesn't exist yet, create one + // Get the schema-tree node that corresponds with the current kv-pair, or add it if it + // doesn't exist. auto opt_schema_tree_node_id{m_schema_tree.try_get_node_id(locator)}; if (false == opt_schema_tree_node_id.has_value()) { opt_schema_tree_node_id.emplace(m_schema_tree.insert_node(locator)); @@ -321,22 +322,23 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c auto const schema_tree_node_id{opt_schema_tree_node_id.value()}; if (SchemaTreeNode::Type::Obj == schema_tree_node_type && msgpack::type::MAP == val.type) { - // Serialize sub-maps. If the sub-map is not empty, push an iterator of the sub-map into - // the working stack to continue DFS + // Serialize map // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) auto const& inner_map{val.via.map}; auto const inner_map_size(static_cast(inner_map.size)); if (0 == inner_map_size) { + // Value is an empty map, so we can serialize it immediately if (false == serialize_key(schema_tree_node_id)) { failure = true; break; } serialize_empty_object(m_value_group_buf); } else { + // Add map for DFS iteration working_stack.emplace_back(schema_tree_node_id, inner_map.ptr, inner_map_size); } } else { - // A primitive value is reached. Directly serialize its key and value into the IR stream + // Serialize primitive if (false == (serialize_key(schema_tree_node_id) && serialize_val(val, schema_tree_node_type) )) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index af2b50c6a..5b85fcc2f 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -84,8 +84,8 @@ class Serializer { /** * Serializes the given msgpack map as a key-value pair log event. - * @param msgpack_map msgpack key-value pair map to serialize, representing an log event. - * @return Whether the serialization succeeded. + * @param msgpack_map + * @return Whether serialization succeeded. */ [[nodiscard]] auto serialize_msgpack_map(msgpack::object_map const& msgpack_map) -> bool; @@ -97,23 +97,23 @@ class Serializer { /** * Serializes a schema tree node identified by the given locator into `m_schema_tree_node_buf`. * @param locator - * @return Whether the serialization succeeded. + * @return Whether serialization succeeded. */ [[nodiscard]] auto serialize_schema_tree_node(SchemaTree::NodeLocator const& locator) -> bool; /** - * Serializes the given key id into `m_key_group_buf`. + * Serializes the given key ID into `m_key_group_buf`. * @param id * @return true on success. - * @return false if the id value exceeds the representable range. + * @return false if the ID exceeds the representable range. */ [[nodiscard]] auto serialize_key(SchemaTreeNode::id_t id) -> bool; /** - * Serializes the given msgpack value into `m_value_group_buf`. + * Serializes the given MessagePack value into `m_value_group_buf`. * @param val - * @param type The corresponded `SchemaTreeNode::Type` of `val`. - * @return Whether the serialization succeeded. + * @param type The schema-node type that corresponds to `val`. + * @return Whether serialization succeeded. */ [[nodiscard]] auto serialize_val(msgpack::object const& val, SchemaTreeNode::Type type) -> bool; diff --git a/components/core/src/clp/ffi/ir_stream/utils.hpp b/components/core/src/clp/ffi/ir_stream/utils.hpp index 3b96a9ee5..9f12313d5 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/utils.hpp @@ -34,11 +34,11 @@ template auto serialize_int(integer_t value, std::vector& ir_buf) -> void; /** - * Serializes a string using clp encoding. + * Serializes a string using CLP's encoding for unstructured text. * @tparam encoded_variable_t * @param str - * @param logtype Outputs the logtype of the serialized clp string. - * @param buf Outputs the serialized byte sequence. + * @param logtype Returns the corresponding logtype. + * @param buf * @return Whether serialization succeeded. */ template @@ -46,10 +46,10 @@ template serialize_clp_string(std::string_view str, std::string& logtype, std::vector& buf) -> bool; /** - * Serializes a string packet. + * Serializes a string. * @param str - * @param buf Outputs the serialized byte sequence. - * @return Whether the serialization succeeded. + * @param buf + * @return Whether serialization succeeded. */ [[nodiscard]] auto serialize_string(std::string_view str, std::vector& buf) -> bool; @@ -76,15 +76,15 @@ serialize_clp_string(std::string_view str, std::string& logtype, std::vector || std::is_same_v) ); - bool error{}; + bool succeeded{}; if constexpr (std::is_same_v) { buf.push_back(cProtocol::Payload::ValueFourByteEncodingClpStr); - error = four_byte_encoding::serialize_message(str, logtype, buf); + succeeded = four_byte_encoding::serialize_message(str, logtype, buf); } else { buf.push_back(cProtocol::Payload::ValueEightByteEncodingClpStr); - error = eight_byte_encoding::serialize_message(str, logtype, buf); + succeeded = eight_byte_encoding::serialize_message(str, logtype, buf); } - return error; + return succeeded; } } // namespace clp::ffi::ir_stream #endif diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 48dd00370..5e5b86454 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -140,7 +140,7 @@ auto flush_and_clear_serializer_buffer( * @tparam encoded_variable_t * @param serializer * @param msgpack_bytes - * @return Whether the serialization is successful. + * @return Whether serialization succeeded. */ template [[nodiscard]] auto unpack_and_serialize_msgpack_bytes( @@ -1012,6 +1012,8 @@ TEMPLATE_TEST_CASE( four_byte_encoded_variable_t, eight_byte_encoded_variable_t ) { + // TODO: Test deserializing the serialized bytes once a KV-pair IR deserializer is implemented. + vector ir_buf; auto result{Serializer::create()}; @@ -1020,18 +1022,13 @@ TEMPLATE_TEST_CASE( auto& serializer{result.value()}; flush_and_clear_serializer_buffer(serializer, ir_buf); - auto const empty_array = nlohmann::json::parse("[]"); auto const empty_obj = nlohmann::json::parse("{}"); REQUIRE(unpack_and_serialize_msgpack_bytes(serializer, nlohmann::json::to_msgpack(empty_obj))); - // TODO: - // Before kv-pair IR Deserializer is implemented, we cannot test whether the serialized bytes - // can be correctly deserialized. We should improve the test coverage once we have a - // deserializer implementation. - // Test encoding basic object constexpr string_view cShortString{"short_string"}; constexpr string_view cClpString{"uid=0, CPU usage: 99.99%, \"user_name\"=YScope"}; + auto const empty_array = nlohmann::json::parse("[]"); nlohmann::json const basic_obj = {{"int8_max", INT8_MAX}, {"int8_min", INT8_MIN}, @@ -1063,7 +1060,7 @@ TEMPLATE_TEST_CASE( basic_array.emplace_back(nullptr); basic_array.emplace_back(empty_array); for (auto const& element : basic_array) { - // Non-map objects should fail the serialization + // Non-map objects should not be serializable REQUIRE( (false == unpack_and_serialize_msgpack_bytes( From f9f64104120e2663b4ae42e7573906817b80f605 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Tue, 16 Jul 2024 21:24:20 -0400 Subject: [PATCH 13/17] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 4 ++-- components/core/src/clp/ffi/ir_stream/encoding_methods.hpp | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index c40a89eb5..01abdc012 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -4,7 +4,7 @@ #include #include #include -#include // Should be removed +#include #include #include #include @@ -395,7 +395,7 @@ auto Serializer::serialize_schema_tree_node( auto const parent_id{locator.get_parent_id()}; if (parent_id <= UINT8_MAX) { m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUByte); - m_schema_tree_node_buf.push_back(static_cast(static_cast(parent_id))); + m_schema_tree_node_buf.push_back(static_cast(bit_cast(parent_id))); } else if (parent_id <= UINT16_MAX) { m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUShort); serialize_int(static_cast(parent_id), m_schema_tree_node_buf); diff --git a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp index a60bf51d9..be042e870 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.hpp @@ -41,10 +41,9 @@ bool serialize_log_event( ); /** - * Serializes the given message into the eight-byte encoding IR stream - * delta + * Serializes the given message into the eight-byte encoding IR stream. * @param message - * @param logtype + * @param logtype Returns the message's logtype. * @param ir_buf * @return Whether the message was serialized successfully. */ From dac044a833ff157bc19c6d11262712a3ac3eaa21 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 16 Jul 2024 22:53:56 -0400 Subject: [PATCH 14/17] Apply code review comments for serializer implementation --- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 161 ++++++++++-------- .../core/src/clp/ffi/ir_stream/Serializer.hpp | 5 +- .../core/src/clp/ffi/ir_stream/utils.cpp | 30 ++-- .../core/src/clp/ffi/ir_stream/utils.hpp | 40 +++-- 4 files changed, 130 insertions(+), 106 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 01abdc012..fae9116b5 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -44,33 +44,36 @@ class MsgpackMapIterator { using Child = msgpack::object_kv; // Constructors - MsgpackMapIterator(SchemaTreeNode::id_t parent_id, Child* child_data, size_t child_data_length) - : m_parent_id{parent_id}, - m_children{child_data, child_data_length} {} + MsgpackMapIterator(SchemaTreeNode::id_t schema_tree_node_id, span children) + : m_schema_tree_node_id{schema_tree_node_id}, + m_children{children}, + m_curr_child_it{m_children.begin()} {} // Methods /** * @return This map's ID in the schema tree. */ - [[nodiscard]] auto get_parent_id() const -> SchemaTreeNode::id_t { return m_parent_id; } + [[nodiscard]] auto get_schema_tree_node_id() const -> SchemaTreeNode::id_t { + return m_schema_tree_node_id; + } /** * @return Whether there are more children to traverse. */ [[nodiscard]] auto has_next_child() const -> bool { - return m_children.size() > m_curr_child_idx; + return m_curr_child_it != m_children.end(); } /** * Gets the next child and advances the underlying child idx. * @return The next child to traverse. */ - [[nodiscard]] auto get_next_child() -> Child const& { return m_children[m_curr_child_idx++]; } + [[nodiscard]] auto get_next_child() -> Child const& { return *(m_curr_child_it++); } private: - SchemaTreeNode::id_t m_parent_id; + SchemaTreeNode::id_t m_schema_tree_node_id; span m_children; - size_t m_curr_child_idx{0}; + span::iterator m_curr_child_it; }; /** @@ -84,61 +87,64 @@ class MsgpackMapIterator { /** * Serializes an empty object. - * @param buf + * @param output_buf */ -auto serialize_empty_object(vector& buf) -> void; +auto serialize_value_empty_object(vector& output_buf) -> void; /** * Serializes an integer. * @param val - * @param buf + * @param output_buf * @return Whether serialization succeeded. */ -auto serialize_value_int(int64_t val, vector& buf) -> void; +auto serialize_value_int(int64_t val, vector& output_buf) -> void; /** * Serializes a float. * @param val - * @param buf + * @param output_buf */ -auto serialize_value_float(double val, vector& buf) -> void; +auto serialize_value_float(double val, vector& output_buf) -> void; /** * Serializes a boolean. * @param val - * @param buf + * @param output_buf */ -auto serialize_value_bool(bool val, vector& buf) -> void; +auto serialize_value_bool(bool val, vector& output_buf) -> void; /** * Serializes a null. - * @param buf + * @param output_buf */ -auto serialize_value_null(vector& buf) -> void; +auto serialize_value_null(vector& output_buf) -> void; /** * Serializes a string. * @tparam encoded_variable_t * @param val * @param logtype_buf - * @param buf + * @param output_buf * @return Whether serialization succeeded. */ template [[nodiscard]] auto -serialize_value_string(string_view val, string& logtype_buf, vector& buf) -> bool; +serialize_value_string(string_view val, string& logtype_buf, vector& output_buf) -> bool; /** * Serializes a MessagePack array as a JSON string, using CLP's encoding for unstructured text. * @tparam encoded_variable_t * @param val * @param logtype_buf - * @param buf + * @param output_buf * @return Whether serialization succeeded. */ template -[[nodiscard]] auto -serialize_value_array(msgpack::object const& val, string& logtype_buf, vector& buf) -> bool; +[[nodiscard]] auto serialize_value_array( + msgpack::object const& val, + string& logtype_buf, + vector& output_buf +) -> bool; auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val ) -> optional { @@ -171,55 +177,59 @@ auto get_schema_tree_node_type_from_msgpack_val(msgpack::object const& val return ret_val; } -auto serialize_empty_object(vector& buf) -> void { - buf.push_back(cProtocol::Payload::ValueEmpty); +auto serialize_value_empty_object(vector& output_buf) -> void { + output_buf.push_back(cProtocol::Payload::ValueEmpty); } -auto serialize_value_int(int64_t val, vector& buf) -> void { +auto serialize_value_int(int64_t val, vector& output_buf) -> void { if (INT8_MIN <= val && val <= INT8_MAX) { - buf.push_back(cProtocol::Payload::ValueInt8); - buf.push_back(static_cast(val)); + output_buf.push_back(cProtocol::Payload::ValueInt8); + output_buf.push_back(static_cast(val)); } else if (INT16_MIN <= val && val <= INT16_MAX) { - buf.push_back(cProtocol::Payload::ValueInt16); - serialize_int(static_cast(val), buf); + output_buf.push_back(cProtocol::Payload::ValueInt16); + serialize_int(static_cast(val), output_buf); } else if (INT32_MIN <= val && val <= INT32_MAX) { - buf.push_back(cProtocol::Payload::ValueInt32); - serialize_int(static_cast(val), buf); + output_buf.push_back(cProtocol::Payload::ValueInt32); + serialize_int(static_cast(val), output_buf); } else { // (INT64_MIN <= val && val <= INT64_MAX) - buf.push_back(cProtocol::Payload::ValueInt64); - serialize_int(val, buf); + output_buf.push_back(cProtocol::Payload::ValueInt64); + serialize_int(val, output_buf); } } -auto serialize_value_float(double val, vector& buf) -> void { - buf.push_back(cProtocol::Payload::ValueFloat); - serialize_int(bit_cast(val), buf); +auto serialize_value_float(double val, vector& output_buf) -> void { + output_buf.push_back(cProtocol::Payload::ValueFloat); + serialize_int(bit_cast(val), output_buf); } -auto serialize_value_bool(bool val, vector& buf) -> void { - buf.push_back(val ? cProtocol::Payload::ValueTrue : cProtocol::Payload::ValueFalse); +auto serialize_value_bool(bool val, vector& output_buf) -> void { + output_buf.push_back(val ? cProtocol::Payload::ValueTrue : cProtocol::Payload::ValueFalse); } -auto serialize_value_null(vector& buf) -> void { - buf.push_back(cProtocol::Payload::ValueNull); +auto serialize_value_null(vector& output_buf) -> void { + output_buf.push_back(cProtocol::Payload::ValueNull); } template -auto serialize_value_string(string_view val, string& logtype_buf, vector& buf) -> bool { +auto serialize_value_string(string_view val, string& logtype_buf, vector& output_buf) + -> bool { if (string_view::npos == val.find(' ')) { - return serialize_string(val, buf); + return serialize_string(val, output_buf); } logtype_buf.clear(); - return serialize_clp_string(val, logtype_buf, buf); + return serialize_clp_string(val, logtype_buf, output_buf); } template -auto serialize_value_array(msgpack::object const& val, string& logtype_buf, vector& buf) - -> bool { +auto serialize_value_array( + msgpack::object const& val, + string& logtype_buf, + vector& output_buf +) -> bool { std::ostringstream oss; oss << val; logtype_buf.clear(); - return serialize_clp_string(oss.str(), logtype_buf, buf); + return serialize_clp_string(oss.str(), logtype_buf, output_buf); } } // namespace @@ -269,7 +279,7 @@ template auto Serializer::serialize_msgpack_map(msgpack::object_map const& msgpack_map ) -> bool { if (0 == msgpack_map.size) { - serialize_empty_object(m_ir_buf); + serialize_value_empty_object(m_ir_buf); return true; } @@ -280,17 +290,16 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c // Traverse the map using DFS iteratively bool failure{false}; - vector working_stack; - working_stack.emplace_back( + vector dfs_stack; + dfs_stack.emplace_back( SchemaTree::cRootId, - msgpack_map.ptr, - static_cast(msgpack_map.size) + span{msgpack_map.ptr, msgpack_map.size} ); - while (false == working_stack.empty()) { - auto& curr{working_stack.back()}; + while (false == dfs_stack.empty()) { + auto& curr{dfs_stack.back()}; if (false == curr.has_next_child()) { // Visited all children, so pop node - working_stack.pop_back(); + dfs_stack.pop_back(); continue; } @@ -304,7 +313,7 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c auto const schema_tree_node_type{opt_schema_tree_node_type.value()}; SchemaTree::NodeLocator const locator{ - curr.get_parent_id(), + curr.get_schema_tree_node_id(), key.as(), schema_tree_node_type }; @@ -321,21 +330,24 @@ auto Serializer::serialize_msgpack_map(msgpack::object_map c } auto const schema_tree_node_id{opt_schema_tree_node_id.value()}; - if (SchemaTreeNode::Type::Obj == schema_tree_node_type && msgpack::type::MAP == val.type) { + if (msgpack::type::MAP == val.type) { // Serialize map // NOLINTNEXTLINE(cppcoreguidelines-pro-type-union-access) auto const& inner_map{val.via.map}; - auto const inner_map_size(static_cast(inner_map.size)); + auto const inner_map_size(inner_map.size); if (0 == inner_map_size) { // Value is an empty map, so we can serialize it immediately if (false == serialize_key(schema_tree_node_id)) { failure = true; break; } - serialize_empty_object(m_value_group_buf); + serialize_value_empty_object(m_value_group_buf); } else { // Add map for DFS iteration - working_stack.emplace_back(schema_tree_node_id, inner_map.ptr, inner_map_size); + dfs_stack.emplace_back( + schema_tree_node_id, + span{inner_map.ptr, inner_map_size} + ); } } else { // Serialize primitive @@ -395,7 +407,7 @@ auto Serializer::serialize_schema_tree_node( auto const parent_id{locator.get_parent_id()}; if (parent_id <= UINT8_MAX) { m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUByte); - m_schema_tree_node_buf.push_back(static_cast(bit_cast(parent_id))); + m_schema_tree_node_buf.push_back(bit_cast(static_cast(parent_id))); } else if (parent_id <= UINT16_MAX) { m_schema_tree_node_buf.push_back(cProtocol::Payload::SchemaTreeNodeParentIdUShort); serialize_int(static_cast(parent_id), m_schema_tree_node_buf); @@ -424,9 +436,9 @@ auto Serializer::serialize_key(SchemaTreeNode::id_t id) -> b template auto Serializer::serialize_val( msgpack::object const& val, - SchemaTreeNode::Type type + SchemaTreeNode::Type schema_tree_node_type ) -> bool { - switch (type) { + switch (schema_tree_node_type) { case SchemaTreeNode::Type::Int: if (msgpack::type::POSITIVE_INTEGER == val.type && static_cast(INT64_MAX) < val.as()) @@ -472,7 +484,7 @@ auto Serializer::serialize_val( break; default: - // Unknown schema tree node type + // Unknown schema tree node schema_tree_node_type return false; } return true; @@ -484,32 +496,37 @@ template auto Serializer::create( ) -> BOOST_OUTCOME_V2_NAMESPACE::std_result>; template auto Serializer::create( ) -> BOOST_OUTCOME_V2_NAMESPACE::std_result>; + template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; + template auto Serializer::serialize_msgpack_map( msgpack::object_map const& msgpack_map ) -> bool; template auto Serializer::serialize_msgpack_map( msgpack::object_map const& msgpack_map ) -> bool; -template auto Serializer::serialize_schema_tree_node( - SchemaTree::NodeLocator const& locator -) -> bool; + template auto Serializer::serialize_schema_tree_node( SchemaTree::NodeLocator const& locator ) -> bool; -template auto Serializer::serialize_key(SchemaTreeNode::id_t id +template auto Serializer::serialize_schema_tree_node( + SchemaTree::NodeLocator const& locator ) -> bool; + template auto Serializer::serialize_key(SchemaTreeNode::id_t id ) -> bool; -template auto Serializer::serialize_val( - msgpack::object const& val, - SchemaTreeNode::Type type +template auto Serializer::serialize_key(SchemaTreeNode::id_t id ) -> bool; + template auto Serializer::serialize_val( msgpack::object const& val, - SchemaTreeNode::Type type + SchemaTreeNode::Type schema_tree_node_type +) -> bool; +template auto Serializer::serialize_val( + msgpack::object const& val, + SchemaTreeNode::Type schema_tree_node_type ) -> bool; } // namespace clp::ffi::ir_stream diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.hpp b/components/core/src/clp/ffi/ir_stream/Serializer.hpp index 5b85fcc2f..82863bc7e 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.hpp @@ -112,10 +112,11 @@ class Serializer { /** * Serializes the given MessagePack value into `m_value_group_buf`. * @param val - * @param type The schema-node type that corresponds to `val`. + * @param schema_tree_node_type The type of the schema tree node that corresponds to `val`. * @return Whether serialization succeeded. */ - [[nodiscard]] auto serialize_val(msgpack::object const& val, SchemaTreeNode::Type type) -> bool; + [[nodiscard]] auto + serialize_val(msgpack::object const& val, SchemaTreeNode::Type schema_tree_node_type) -> bool; UtcOffset m_curr_utc_offset{0}; Buffer m_ir_buf; diff --git a/components/core/src/clp/ffi/ir_stream/utils.cpp b/components/core/src/clp/ffi/ir_stream/utils.cpp index 019efad13..3b6c961ee 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/utils.cpp @@ -10,43 +10,43 @@ #include "protocol_constants.hpp" namespace clp::ffi::ir_stream { -auto serialize_metadata(nlohmann::json& metadata, std::vector& ir_buf) -> bool { - ir_buf.push_back(cProtocol::Metadata::EncodingJson); +auto serialize_metadata(nlohmann::json& metadata, std::vector& output_buf) -> bool { + output_buf.push_back(cProtocol::Metadata::EncodingJson); auto const metadata_serialized = metadata.dump(-1, ' ', false, nlohmann::json::error_handler_t::ignore); auto const metadata_serialized_length = metadata_serialized.length(); if (metadata_serialized_length <= UINT8_MAX) { - ir_buf.push_back(cProtocol::Metadata::LengthUByte); - ir_buf.push_back(bit_cast(static_cast(metadata_serialized_length))); + output_buf.push_back(cProtocol::Metadata::LengthUByte); + output_buf.push_back(bit_cast(static_cast(metadata_serialized_length))); } else if (metadata_serialized_length <= UINT16_MAX) { - ir_buf.push_back(cProtocol::Metadata::LengthUShort); - serialize_int(static_cast(metadata_serialized_length), ir_buf); + output_buf.push_back(cProtocol::Metadata::LengthUShort); + serialize_int(static_cast(metadata_serialized_length), output_buf); } else { // Can't encode metadata longer than 64 KiB return false; } - ir_buf.insert(ir_buf.cend(), metadata_serialized.cbegin(), metadata_serialized.cend()); + output_buf.insert(output_buf.cend(), metadata_serialized.cbegin(), metadata_serialized.cend()); return true; } -auto serialize_string(std::string_view str, std::vector& buf) -> bool { +auto serialize_string(std::string_view str, std::vector& output_buf) -> bool { auto const length{str.length()}; if (length <= UINT8_MAX) { - buf.push_back(cProtocol::Payload::StrPacketLenUByte); - buf.push_back(bit_cast(static_cast(length))); + output_buf.push_back(cProtocol::Payload::StrPacketLenUByte); + output_buf.push_back(bit_cast(static_cast(length))); } else if (length <= UINT16_MAX) { - buf.push_back(cProtocol::Payload::StrPacketLenUShort); - serialize_int(static_cast(length), buf); + output_buf.push_back(cProtocol::Payload::StrPacketLenUShort); + serialize_int(static_cast(length), output_buf); } else if (length <= UINT32_MAX) { - buf.push_back(cProtocol::Payload::StrPacketLenUInt); - serialize_int(static_cast(length), buf); + output_buf.push_back(cProtocol::Payload::StrPacketLenUInt); + serialize_int(static_cast(length), output_buf); } else { // Out of range return false; } - buf.insert(buf.cend(), str.cbegin(), str.cend()); + output_buf.insert(output_buf.cend(), str.cbegin(), str.cend()); return true; } } // namespace clp::ffi::ir_stream diff --git a/components/core/src/clp/ffi/ir_stream/utils.hpp b/components/core/src/clp/ffi/ir_stream/utils.hpp index 9f12313d5..7879c3f74 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/utils.hpp @@ -18,43 +18,46 @@ namespace clp::ffi::ir_stream { /** * Serializes the given metadata into the IR stream. * @param metadata - * @param ir_buf + * @param output_buf * @return Whether serialization succeeded. */ [[nodiscard]] auto -serialize_metadata(nlohmann::json& metadata, std::vector& ir_buf) -> bool; +serialize_metadata(nlohmann::json& metadata, std::vector& output_buf) -> bool; /** * Serializes the given integer into the IR stream. * @tparam integer_t * @param value - * @param ir_buf + * @param output_buf */ template -auto serialize_int(integer_t value, std::vector& ir_buf) -> void; +auto serialize_int(integer_t value, std::vector& output_buf) -> void; /** * Serializes a string using CLP's encoding for unstructured text. * @tparam encoded_variable_t * @param str * @param logtype Returns the corresponding logtype. - * @param buf + * @param output_buf * @return Whether serialization succeeded. */ template -[[nodiscard]] auto -serialize_clp_string(std::string_view str, std::string& logtype, std::vector& buf) -> bool; +[[nodiscard]] auto serialize_clp_string( + std::string_view str, + std::string& logtype, + std::vector& output_buf +) -> bool; /** * Serializes a string. * @param str - * @param buf + * @param output_buf * @return Whether serialization succeeded. */ -[[nodiscard]] auto serialize_string(std::string_view str, std::vector& buf) -> bool; +[[nodiscard]] auto serialize_string(std::string_view str, std::vector& output_buf) -> bool; template -auto serialize_int(integer_t value, std::vector& ir_buf) -> void { +auto serialize_int(integer_t value, std::vector& output_buf) -> void { integer_t value_big_endian{}; static_assert(sizeof(integer_t) == 2 || sizeof(integer_t) == 4 || sizeof(integer_t) == 8); if constexpr (sizeof(value) == 2) { @@ -66,23 +69,26 @@ auto serialize_int(integer_t value, std::vector& ir_buf) -> void { } // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) std::span const data_view{reinterpret_cast(&value_big_endian), sizeof(value)}; - ir_buf.insert(ir_buf.end(), data_view.begin(), data_view.end()); + output_buf.insert(output_buf.end(), data_view.begin(), data_view.end()); } template -[[nodiscard]] auto -serialize_clp_string(std::string_view str, std::string& logtype, std::vector& buf) -> bool { +[[nodiscard]] auto serialize_clp_string( + std::string_view str, + std::string& logtype, + std::vector& output_buf +) -> bool { static_assert( (std::is_same_v || std::is_same_v) ); bool succeeded{}; if constexpr (std::is_same_v) { - buf.push_back(cProtocol::Payload::ValueFourByteEncodingClpStr); - succeeded = four_byte_encoding::serialize_message(str, logtype, buf); + output_buf.push_back(cProtocol::Payload::ValueFourByteEncodingClpStr); + succeeded = four_byte_encoding::serialize_message(str, logtype, output_buf); } else { - buf.push_back(cProtocol::Payload::ValueEightByteEncodingClpStr); - succeeded = eight_byte_encoding::serialize_message(str, logtype, buf); + output_buf.push_back(cProtocol::Payload::ValueEightByteEncodingClpStr); + succeeded = eight_byte_encoding::serialize_message(str, logtype, output_buf); } return succeeded; } From ddca2480a7d3275ae05e23e00f1632a4cd378cbe Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 16 Jul 2024 23:20:06 -0400 Subject: [PATCH 15/17] Apply the rest of code review comments --- .../clp/ffi/ir_stream/protocol_constants.hpp | 42 ++++++++++--------- .../core/src/clp/ffi/ir_stream/utils.cpp | 6 +-- .../core/tests/test-ir_encoding_methods.cpp | 14 +++---- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index 6455dff31..e5d0e2bfc 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -51,16 +51,22 @@ constexpr int8_t TimestampDeltaLong = 0x34; constexpr int8_t UtcOffsetChange = 0x3F; -constexpr int8_t StrPacketLenUByte = 0x41; -constexpr int8_t StrPacketLenUShort = 0x42; -constexpr int8_t StrPacketLenUInt = 0x43; +constexpr int8_t StrLenUByte = 0x41; +constexpr int8_t StrLenUShort = 0x42; +constexpr int8_t StrLenUInt = 0x43; -constexpr int8_t SchemaTreeNodeInt = 0x71; -constexpr int8_t SchemaTreeNodeFloat = 0x72; -constexpr int8_t SchemaTreeNodeBool = 0x73; -constexpr int8_t SchemaTreeNodeStr = 0x74; -constexpr int8_t SchemaTreeNodeUnstructuredArray = 0x75; -constexpr int8_t SchemaTreeNodeObj = 0x76; +constexpr int8_t ValueInt8 = 0x51; +constexpr int8_t ValueInt16 = 0x52; +constexpr int8_t ValueInt32 = 0x53; +constexpr int8_t ValueInt64 = 0x54; +constexpr int8_t ValueUnsignedInt64 = 0x55; +constexpr int8_t ValueFloat = 0x56; +constexpr int8_t ValueTrue = 0x57; +constexpr int8_t ValueFalse = 0x58; +constexpr int8_t ValueFourByteEncodingClpStr = 0x59; +constexpr int8_t ValueEightByteEncodingClpStr = 0x5A; +constexpr int8_t ValueEmpty = 0x5E; +constexpr int8_t ValueNull = 0x5F; constexpr int8_t SchemaTreeNodeParentIdUByte = 0x60; constexpr int8_t SchemaTreeNodeParentIdUShort = 0x61; @@ -68,17 +74,13 @@ constexpr int8_t SchemaTreeNodeParentIdUShort = 0x61; constexpr int8_t KeyIdUByte = 0x65; constexpr int8_t KeyIdUShort = 0x66; -constexpr int8_t ValueInt8 = 0x51; -constexpr int8_t ValueInt16 = 0x52; -constexpr int8_t ValueInt32 = 0x53; -constexpr int8_t ValueInt64 = 0x54; -constexpr int8_t ValueFloat = 0x55; -constexpr int8_t ValueTrue = 0x56; -constexpr int8_t ValueFalse = 0x57; -constexpr int8_t ValueFourByteEncodingClpStr = 0x58; -constexpr int8_t ValueEightByteEncodingClpStr = 0x59; -constexpr int8_t ValueEmpty = 0x5E; -constexpr int8_t ValueNull = 0x5F; +constexpr int8_t SchemaTreeNodeInt = 0x71; +constexpr int8_t SchemaTreeNodeFloat = 0x72; +constexpr int8_t SchemaTreeNodeBool = 0x73; +constexpr int8_t SchemaTreeNodeStr = 0x74; +constexpr int8_t SchemaTreeNodeUnstructuredArray = 0x75; +constexpr int8_t SchemaTreeNodeObj = 0x76; + } // namespace Payload constexpr int8_t FourByteEncodingMagicNumber[] diff --git a/components/core/src/clp/ffi/ir_stream/utils.cpp b/components/core/src/clp/ffi/ir_stream/utils.cpp index 3b6c961ee..66277720c 100644 --- a/components/core/src/clp/ffi/ir_stream/utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/utils.cpp @@ -34,13 +34,13 @@ auto serialize_metadata(nlohmann::json& metadata, std::vector& output_bu auto serialize_string(std::string_view str, std::vector& output_buf) -> bool { auto const length{str.length()}; if (length <= UINT8_MAX) { - output_buf.push_back(cProtocol::Payload::StrPacketLenUByte); + output_buf.push_back(cProtocol::Payload::StrLenUByte); output_buf.push_back(bit_cast(static_cast(length))); } else if (length <= UINT16_MAX) { - output_buf.push_back(cProtocol::Payload::StrPacketLenUShort); + output_buf.push_back(cProtocol::Payload::StrLenUShort); serialize_int(static_cast(length), output_buf); } else if (length <= UINT32_MAX) { - output_buf.push_back(cProtocol::Payload::StrPacketLenUInt); + output_buf.push_back(cProtocol::Payload::StrLenUInt); serialize_int(static_cast(length), output_buf); } else { // Out of range diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 5e5b86454..694fef16f 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -145,7 +145,7 @@ auto flush_and_clear_serializer_buffer( template [[nodiscard]] auto unpack_and_serialize_msgpack_bytes( Serializer& serializer, - vector const& msgpack_bytes + vector const& msgpack_bytes ) -> bool; template @@ -261,15 +261,13 @@ auto flush_and_clear_serializer_buffer( template auto unpack_and_serialize_msgpack_bytes( Serializer& serializer, - vector const& msgpack_bytes + vector const& msgpack_bytes ) -> bool { - msgpack::object_handle msgpack_oh; - msgpack::unpack( - msgpack_oh, + auto const msgpack_obj_handle{msgpack::unpack( clp::size_checked_pointer_cast(msgpack_bytes.data()), msgpack_bytes.size() - ); - auto const& msgpack_obj{msgpack_oh.get()}; + )}; + auto const msgpack_obj{msgpack_obj_handle.get()}; if (msgpack::type::MAP != msgpack_obj.type) { return false; } @@ -1074,7 +1072,7 @@ TEMPLATE_TEST_CASE( // Recursively construct an object containing inner maps and inner arrays. auto recursive_obj = basic_obj; auto recursive_array = basic_array; - constexpr size_t cRecursiveDepth{5}; + constexpr size_t cRecursiveDepth{6}; for (size_t i{0}; i < cRecursiveDepth; ++i) { recursive_array.emplace_back(recursive_obj); recursive_obj.emplace("obj_" + std::to_string(i), recursive_obj); From 9d2f537739b0ad603607c824e1fb2bb5a7486cf4 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 17 Jul 2024 00:31:17 -0400 Subject: [PATCH 16/17] Apply code review comments --- .../clp/ffi/ir_stream/protocol_constants.hpp | 1 - .../core/tests/test-ir_encoding_methods.cpp | 24 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index e5d0e2bfc..f22457f1e 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -59,7 +59,6 @@ constexpr int8_t ValueInt8 = 0x51; constexpr int8_t ValueInt16 = 0x52; constexpr int8_t ValueInt32 = 0x53; constexpr int8_t ValueInt64 = 0x54; -constexpr int8_t ValueUnsignedInt64 = 0x55; constexpr int8_t ValueFloat = 0x56; constexpr int8_t ValueTrue = 0x57; constexpr int8_t ValueFalse = 0x58; diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 694fef16f..4199428ff 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include #include @@ -138,14 +140,14 @@ auto flush_and_clear_serializer_buffer( /** * Unpacks and serializes the given msgpack bytes using kv serializer. * @tparam encoded_variable_t - * @param serializer * @param msgpack_bytes + * @param serializer * @return Whether serialization succeeded. */ template [[nodiscard]] auto unpack_and_serialize_msgpack_bytes( - Serializer& serializer, - vector const& msgpack_bytes + vector const& msgpack_bytes, + Serializer& serializer ) -> bool; template @@ -260,8 +262,8 @@ auto flush_and_clear_serializer_buffer( template auto unpack_and_serialize_msgpack_bytes( - Serializer& serializer, - vector const& msgpack_bytes + vector const& msgpack_bytes, + Serializer& serializer ) -> bool { auto const msgpack_obj_handle{msgpack::unpack( clp::size_checked_pointer_cast(msgpack_bytes.data()), @@ -1021,7 +1023,7 @@ TEMPLATE_TEST_CASE( flush_and_clear_serializer_buffer(serializer, ir_buf); auto const empty_obj = nlohmann::json::parse("{}"); - REQUIRE(unpack_and_serialize_msgpack_bytes(serializer, nlohmann::json::to_msgpack(empty_obj))); + REQUIRE(unpack_and_serialize_msgpack_bytes(nlohmann::json::to_msgpack(empty_obj), serializer)); // Test encoding basic object constexpr string_view cShortString{"short_string"}; @@ -1047,7 +1049,7 @@ TEMPLATE_TEST_CASE( {"empty_object", empty_obj}, {"empty_array", empty_array}}; - REQUIRE(unpack_and_serialize_msgpack_bytes(serializer, nlohmann::json::to_msgpack(basic_obj))); + REQUIRE(unpack_and_serialize_msgpack_bytes(nlohmann::json::to_msgpack(basic_obj), serializer)); auto basic_array = empty_array; basic_array.emplace_back(1); @@ -1062,8 +1064,8 @@ TEMPLATE_TEST_CASE( REQUIRE( (false == unpack_and_serialize_msgpack_bytes( - serializer, - nlohmann::json::to_msgpack(element) + nlohmann::json::to_msgpack(element), + serializer )) ); } @@ -1078,8 +1080,8 @@ TEMPLATE_TEST_CASE( recursive_obj.emplace("obj_" + std::to_string(i), recursive_obj); recursive_obj.emplace("array_" + std::to_string(i), recursive_array); REQUIRE(unpack_and_serialize_msgpack_bytes( - serializer, - nlohmann::json::to_msgpack(recursive_obj) + nlohmann::json::to_msgpack(recursive_obj), + serializer )); } } From 1f98ef783b399637dfe478745f67c8983f97ba13 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Wed, 17 Jul 2024 00:36:55 -0400 Subject: [PATCH 17/17] Minor fixes. --- components/core/src/clp/ffi/ir_stream/Serializer.cpp | 6 +++--- .../core/src/clp/ffi/ir_stream/protocol_constants.hpp | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index fae9116b5..770a6a78c 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -484,7 +484,7 @@ auto Serializer::serialize_val( break; default: - // Unknown schema tree node schema_tree_node_type + // Unknown schema tree node type return false; } return true; @@ -502,10 +502,10 @@ template auto Serializer::change_utc_offset(UtcOf template auto Serializer::change_utc_offset(UtcOffset utc_offset ) -> void; -template auto Serializer::serialize_msgpack_map( +template auto Serializer::serialize_msgpack_map( msgpack::object_map const& msgpack_map ) -> bool; -template auto Serializer::serialize_msgpack_map( +template auto Serializer::serialize_msgpack_map( msgpack::object_map const& msgpack_map ) -> bool; diff --git a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp index f22457f1e..9f84cc4e1 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -79,7 +79,6 @@ constexpr int8_t SchemaTreeNodeBool = 0x73; constexpr int8_t SchemaTreeNodeStr = 0x74; constexpr int8_t SchemaTreeNodeUnstructuredArray = 0x75; constexpr int8_t SchemaTreeNodeObj = 0x76; - } // namespace Payload constexpr int8_t FourByteEncodingMagicNumber[]