From 3721ec8a5cb4e7b132801712e821a8910607d868 Mon Sep 17 00:00:00 2001 From: LittleStar <59785146+LinZhihao-723@users.noreply.github.com> Date: Sat, 23 Sep 2023 22:17:44 -0700 Subject: [PATCH 01/10] Add 64-bit timestamp delta support --- components/core/src/ffi/ir_stream/decoding_methods.cpp | 6 ++++++ components/core/src/ffi/ir_stream/encoding_methods.cpp | 5 ++++- components/core/src/ffi/ir_stream/protocol_constants.hpp | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/components/core/src/ffi/ir_stream/decoding_methods.cpp b/components/core/src/ffi/ir_stream/decoding_methods.cpp index 167fc77cd..b9a065e87 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.cpp @@ -251,6 +251,12 @@ parse_timestamp(ReaderInterface& reader, encoded_tag_t encoded_tag, epoch_time_m return IRErrorCode_Incomplete_IR; } ts = ts_delta; + } else if (cProtocol::Payload::TimestampDeltaLong == encoded_tag) { + int64_t ts_delta; + if (false == decode_int(reader, ts_delta)) { + return IRErrorCode_Incomplete_IR; + } + ts = ts_delta; } else { return IRErrorCode_Corrupted_IR; } diff --git a/components/core/src/ffi/ir_stream/encoding_methods.cpp b/components/core/src/ffi/ir_stream/encoding_methods.cpp index ec427ab76..d7e949d96 100644 --- a/components/core/src/ffi/ir_stream/encoding_methods.cpp +++ b/components/core/src/ffi/ir_stream/encoding_methods.cpp @@ -308,8 +308,11 @@ namespace four_byte_encoding { } else if (INT32_MIN <= timestamp_delta && timestamp_delta <= INT32_MAX) { ir_buf.push_back(cProtocol::Payload::TimestampDeltaInt); encode_int(static_cast(timestamp_delta), ir_buf); + } else if (INT64_MIN <= timestamp_delta && timestamp_delta <= INT64_MAX) { + ir_buf.push_back(cProtocol::Payload::TimestampDeltaLong); + encode_int(static_cast(timestamp_delta), ir_buf); } else { - // Delta exceeds maximum representable by an int (24.86 days) + // Delta exceeds maximum representable by a 64-bit int return false; } diff --git a/components/core/src/ffi/ir_stream/protocol_constants.hpp b/components/core/src/ffi/ir_stream/protocol_constants.hpp index 652c2a16a..3f413a1c0 100644 --- a/components/core/src/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/ffi/ir_stream/protocol_constants.hpp @@ -39,6 +39,7 @@ namespace Payload { constexpr int8_t TimestampDeltaByte = 0x31; constexpr int8_t TimestampDeltaShort = 0x32; constexpr int8_t TimestampDeltaInt = 0x33; + constexpr int8_t TimestampDeltaLong = 0x34; } // namespace Payload constexpr int8_t FourByteEncodingMagicNumber[] From 17e0a8a7b39f35e12ef603eac9b57748f2fe0ea1 Mon Sep 17 00:00:00 2001 From: LittleStar <59785146+LinZhihao-723@users.noreply.github.com> Date: Sat, 23 Sep 2023 23:33:51 -0700 Subject: [PATCH 02/10] Add unit tests for timestamp delta encoding --- .../core/tests/test-ir_encoding_methods.cpp | 70 ++++++++++++------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index c4d586d3e..5e8b6811c 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -469,32 +469,54 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { )); } -TEST_CASE("decode_next_message_four_byte_negative_delta", "[ffi][decode_next_message]") { - string message = "Static <\text>, dictVar1, 123, 456345232.7234223, " +TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][decode_next_message]") { + string const message = "Static <\text>, dictVar1, 123, 456345232.7234223, " "dictVar2, 987, 654.3, end of static text"; - vector ir_buf; - string logtype; - - epoch_time_ms_t reference_delta_ts_negative = -5; - REQUIRE(true - == encode_message( - reference_delta_ts_negative, - message, - logtype, - ir_buf - )); + auto test_timestamp_delta = [&](epoch_time_ms_t ref_ts_delta) { + vector ir_buf; + string logtype; + REQUIRE(true + == encode_message( + ref_ts_delta, + message, + logtype, + ir_buf + )); - BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; - string decoded_message; - epoch_time_ms_t delta_ts; - REQUIRE(IRErrorCode::IRErrorCode_Success - == decode_next_message( - ir_buffer, - decoded_message, - delta_ts - )); - REQUIRE(message == decoded_message); - REQUIRE(delta_ts == reference_delta_ts_negative); + BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; + string decoded_message; + epoch_time_ms_t delta_ts; + REQUIRE(IRErrorCode::IRErrorCode_Success + == decode_next_message( + ir_buffer, + decoded_message, + delta_ts + )); + REQUIRE(message == decoded_message); + REQUIRE(delta_ts == ref_ts_delta); + }; + + test_timestamp_delta(0); + + test_timestamp_delta(INT8_MIN); + test_timestamp_delta(INT8_MIN + 1); + test_timestamp_delta(INT8_MAX - 1); + test_timestamp_delta(INT8_MAX); + + test_timestamp_delta(INT16_MIN); + test_timestamp_delta(INT16_MIN + 1); + test_timestamp_delta(INT16_MAX - 1); + test_timestamp_delta(INT16_MAX); + + test_timestamp_delta(INT32_MIN); + test_timestamp_delta(INT32_MIN + 1); + test_timestamp_delta(INT32_MAX - 1); + test_timestamp_delta(INT32_MAX); + + test_timestamp_delta(INT64_MIN); + test_timestamp_delta(INT64_MIN + 1); + test_timestamp_delta(INT64_MAX - 1); + test_timestamp_delta(INT64_MAX); } TEMPLATE_TEST_CASE( From b0525196b33917d5f34ca34c879de3d5bcef5030 Mon Sep 17 00:00:00 2001 From: LittleStar <59785146+LinZhihao-723@users.noreply.github.com> Date: Sat, 23 Sep 2023 23:36:44 -0700 Subject: [PATCH 03/10] Increment ir_stream protocol version --- components/core/src/ffi/ir_stream/protocol_constants.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/ffi/ir_stream/protocol_constants.hpp b/components/core/src/ffi/ir_stream/protocol_constants.hpp index 3f413a1c0..0128db54b 100644 --- a/components/core/src/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/ffi/ir_stream/protocol_constants.hpp @@ -12,7 +12,7 @@ namespace Metadata { constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; - constexpr char VersionValue[] = "v0.0.0"; + constexpr char VersionValue[] = "v0.0.1"; constexpr char TimestampPatternKey[] = "TIMESTAMP_PATTERN"; constexpr char TimestampPatternSyntaxKey[] = "TIMESTAMP_PATTERN_SYNTAX"; From 33e57b7932bd23f50d66c3df299c52089e94f6b5 Mon Sep 17 00:00:00 2001 From: LittleStar <59785146+LinZhihao-723@users.noreply.github.com> Date: Sun, 24 Sep 2023 01:32:27 -0700 Subject: [PATCH 04/10] Add proper version checking --- .../src/ffi/ir_stream/decoding_methods.cpp | 19 ++++++++++++++++--- .../src/ffi/ir_stream/decoding_methods.hpp | 18 ++++++++++++++++++ .../src/ffi/ir_stream/protocol_constants.hpp | 6 ++++++ .../core/src/ir/LogEventDeserializer.cpp | 4 ++-- .../core/tests/test-ir_encoding_methods.cpp | 15 ++++++++++----- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/components/core/src/ffi/ir_stream/decoding_methods.cpp b/components/core/src/ffi/ir_stream/decoding_methods.cpp index b9a065e87..c2b8f3e51 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.cpp @@ -1,5 +1,7 @@ #include "decoding_methods.hpp" +#include + #include "byteswap.hpp" #include "protocol_constants.hpp" @@ -283,9 +285,8 @@ generic_decode_next_message(ReaderInterface& reader, string& message, epoch_time message.append(value, begin_pos, length); }; - auto encoded_int_handler = [&](encoded_variable_t value) { - message.append(decode_integer_var(value)); - }; + auto encoded_int_handler + = [&](encoded_variable_t value) { message.append(decode_integer_var(value)); }; auto encoded_float_handler = [&](encoded_variable_t encoded_float) { message.append(decode_float_var(encoded_float)); @@ -464,6 +465,18 @@ IRErrorCode decode_preamble( return IRErrorCode_Success; } +IRProtocolErrorCode validate_protocol_version(std::string const& protocol_version) { + std::regex const protocol_version_regex{cProtocol::Metadata::VersionRegex}; + if (false == std::regex_match(protocol_version, protocol_version_regex)) { + return IRProtocolErrorCode_Unknown; + } + std::string_view current_protocol_version{cProtocol::Metadata::VersionValue}; + if (current_protocol_version < protocol_version) { + return IRProtocolErrorCode_Unsupported; + } + return IRProtocolErrorCode_Supported; +} + namespace four_byte_encoding { IRErrorCode decode_next_message( ReaderInterface& reader, diff --git a/components/core/src/ffi/ir_stream/decoding_methods.hpp b/components/core/src/ffi/ir_stream/decoding_methods.hpp index dcf5c207d..36a671b6f 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.hpp @@ -142,6 +142,24 @@ IRErrorCode decode_preamble( std::vector& metadata ); +typedef enum { + IRProtocolErrorCode_Supported, + IRProtocolErrorCode_Unsupported, + IRProtocolErrorCode_Unknown, +} IRProtocolErrorCode; + +/** + * Validates whether the given protocol version can be supported by the current + * decoding method implementations. + * @param protocol_version Protocol version to be validate. + * @return IRProtocolErrorCode_Supported if the protocol version is supported. + * @return IRProtocolErrorCode_Unsupported if the protocol version cannot be + * supported. + * @return IRProtocolErrorCode_Unknow if the protocol version does not follow + * the form specified by SemVer. + */ +IRProtocolErrorCode validate_protocol_version(std::string const& protocol_version); + namespace eight_byte_encoding { /** * Decodes the next message for the eight-byte encoding IR stream. diff --git a/components/core/src/ffi/ir_stream/protocol_constants.hpp b/components/core/src/ffi/ir_stream/protocol_constants.hpp index 0128db54b..087497020 100644 --- a/components/core/src/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/ffi/ir_stream/protocol_constants.hpp @@ -3,6 +3,7 @@ #include #include +#include #include namespace ffi::ir_stream::cProtocol { @@ -13,6 +14,11 @@ namespace Metadata { constexpr char VersionKey[] = "VERSION"; constexpr char VersionValue[] = "v0.0.1"; + constexpr char VersionRegex[] = + "^v(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)" + "(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)" + "(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?" + "(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$"; constexpr char TimestampPatternKey[] = "TIMESTAMP_PATTERN"; constexpr char TimestampPatternSyntaxKey[] = "TIMESTAMP_PATTERN_SYNTAX"; diff --git a/components/core/src/ir/LogEventDeserializer.cpp b/components/core/src/ir/LogEventDeserializer.cpp index 16ba71ba2..2f3adb517 100644 --- a/components/core/src/ir/LogEventDeserializer.cpp +++ b/components/core/src/ir/LogEventDeserializer.cpp @@ -37,8 +37,8 @@ auto LogEventDeserializer::create(ReaderInterface& reader) return std::errc::protocol_error; } auto metadata_version = version_iter->get_ref(); - if (static_cast(ffi::ir_stream::cProtocol::Metadata::VersionValue) - != metadata_version) + if (ffi::ir_stream::IRProtocolErrorCode_Supported + != ffi::ir_stream::validate_protocol_version(metadata_version)) { return std::errc::protocol_not_supported; } diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 5e8b6811c..6962fc2e7 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -28,6 +28,7 @@ using ffi::ir_stream::decode_preamble; using ffi::ir_stream::encoded_tag_t; using ffi::ir_stream::get_encoding_type; using ffi::ir_stream::IRErrorCode; +using ffi::ir_stream::validate_protocol_version; using ffi::wildcard_query_matches_any_encoded_var; using ir::VariablePlaceholder; using std::chrono::duration_cast; @@ -329,8 +330,10 @@ TEMPLATE_TEST_CASE( string_view json_metadata{metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); - REQUIRE(ffi::ir_stream::cProtocol::Metadata::VersionValue - == metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey)); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported + == validate_protocol_version( + metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey) + )); REQUIRE(ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); @@ -471,7 +474,7 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][decode_next_message]") { string const message = "Static <\text>, dictVar1, 123, 456345232.7234223, " - "dictVar2, 987, 654.3, end of static text"; + "dictVar2, 987, 654.3, end of static text"; auto test_timestamp_delta = [&](epoch_time_ms_t ref_ts_delta) { vector ir_buf; string logtype; @@ -582,8 +585,10 @@ TEMPLATE_TEST_CASE( auto* json_metadata_ptr{size_checked_pointer_cast(ir_buf.data() + metadata_pos)}; string_view json_metadata{json_metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); - REQUIRE(ffi::ir_stream::cProtocol::Metadata::VersionValue - == metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey)); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported + == validate_protocol_version( + metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey) + )); REQUIRE(ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); From 4b7dc50893bb40a0f1eb7769adb23165eef75006 Mon Sep 17 00:00:00 2001 From: LittleStar <59785146+LinZhihao-723@users.noreply.github.com> Date: Sun, 24 Sep 2023 02:11:20 -0700 Subject: [PATCH 05/10] Update comments --- components/core/src/ffi/ir_stream/decoding_methods.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/ffi/ir_stream/decoding_methods.hpp b/components/core/src/ffi/ir_stream/decoding_methods.hpp index 36a671b6f..faf59ab44 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.hpp @@ -150,7 +150,7 @@ typedef enum { /** * Validates whether the given protocol version can be supported by the current - * decoding method implementations. + * build. * @param protocol_version Protocol version to be validate. * @return IRProtocolErrorCode_Supported if the protocol version is supported. * @return IRProtocolErrorCode_Unsupported if the protocol version cannot be From cb2859b5f5dce6baecdf5d4e9fb9f861036d9560 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Thu, 28 Sep 2023 15:35:17 -0400 Subject: [PATCH 06/10] Fix code review comments --- .../src/ffi/ir_stream/decoding_methods.cpp | 30 +++- .../src/ffi/ir_stream/decoding_methods.hpp | 27 ++-- .../src/ffi/ir_stream/encoding_methods.cpp | 5 +- .../src/ffi/ir_stream/protocol_constants.hpp | 15 +- .../core/tests/test-ir_encoding_methods.cpp | 129 ++++++++++-------- 5 files changed, 117 insertions(+), 89 deletions(-) diff --git a/components/core/src/ffi/ir_stream/decoding_methods.cpp b/components/core/src/ffi/ir_stream/decoding_methods.cpp index c2b8f3e51..3ceb05ecc 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.cpp @@ -465,14 +465,32 @@ IRErrorCode decode_preamble( return IRErrorCode_Success; } -IRProtocolErrorCode validate_protocol_version(std::string const& protocol_version) { +IRProtocolErrorCode validate_protocol_version(std::string_view protocol_version) { + if ("v0.0.0" == protocol_version) { + // This version is hardcoded to support the oldest IR protocol version. + // When this version is no longer supported, this branch should be + // removed. + return IRProtocolErrorCode_Supported; + } std::regex const protocol_version_regex{cProtocol::Metadata::VersionRegex}; - if (false == std::regex_match(protocol_version, protocol_version_regex)) { - return IRProtocolErrorCode_Unknown; + if (false + == std::regex_match( + protocol_version.begin(), + protocol_version.end(), + protocol_version_regex + )) + { + return IRProtocolErrorCode_Invalid; + } + std::string_view current_build_protocol_version{cProtocol::Metadata::VersionValue}; + auto get_major_version{[](std::string_view version) { + return version.substr(0, version.find('.')); + }}; + if (current_build_protocol_version < protocol_version) { + return IRProtocolErrorCode_Too_New; } - std::string_view current_protocol_version{cProtocol::Metadata::VersionValue}; - if (current_protocol_version < protocol_version) { - return IRProtocolErrorCode_Unsupported; + if (get_major_version(current_build_protocol_version) > get_major_version(protocol_version)) { + return IRProtocolErrorCode_Too_Old; } return IRProtocolErrorCode_Supported; } diff --git a/components/core/src/ffi/ir_stream/decoding_methods.hpp b/components/core/src/ffi/ir_stream/decoding_methods.hpp index faf59ab44..78f66c417 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.hpp @@ -18,6 +18,13 @@ typedef enum { IRErrorCode_Incomplete_IR, } IRErrorCode; +typedef enum { + IRProtocolErrorCode_Supported, + IRProtocolErrorCode_Too_Old, + IRProtocolErrorCode_Too_New, + IRProtocolErrorCode_Invalid, +} IRProtocolErrorCode; + class DecodingException : public TraceableException { public: // Constructors @@ -142,23 +149,19 @@ IRErrorCode decode_preamble( std::vector& metadata ); -typedef enum { - IRProtocolErrorCode_Supported, - IRProtocolErrorCode_Unsupported, - IRProtocolErrorCode_Unknown, -} IRProtocolErrorCode; - /** * Validates whether the given protocol version can be supported by the current * build. - * @param protocol_version Protocol version to be validate. + * @param protocol_version * @return IRProtocolErrorCode_Supported if the protocol version is supported. - * @return IRProtocolErrorCode_Unsupported if the protocol version cannot be - * supported. - * @return IRProtocolErrorCode_Unknow if the protocol version does not follow - * the form specified by SemVer. + * @return IRProtocolErrorCode_Too_Old if the protocol version is no longer + * supported by the current build version. + * @return IRProtocolErrorCode_Too_New if the protocol version is newer than the + * build version. + * @return IRProtocolErrorCode_Invalid if the protocol version does not follow + * the SemVer specification. */ -IRProtocolErrorCode validate_protocol_version(std::string const& protocol_version); +IRProtocolErrorCode validate_protocol_version(std::string_view protocol_version); namespace eight_byte_encoding { /** diff --git a/components/core/src/ffi/ir_stream/encoding_methods.cpp b/components/core/src/ffi/ir_stream/encoding_methods.cpp index d7e949d96..35c766aef 100644 --- a/components/core/src/ffi/ir_stream/encoding_methods.cpp +++ b/components/core/src/ffi/ir_stream/encoding_methods.cpp @@ -57,10 +57,7 @@ static void add_base_metadata_fields( * @param logtype * @return true */ -static bool append_constant_to_logtype( - string_view constant, - string& logtype -); +static bool append_constant_to_logtype(string_view constant, string& logtype); /** * A functor for encoding dictionary variables in a message diff --git a/components/core/src/ffi/ir_stream/protocol_constants.hpp b/components/core/src/ffi/ir_stream/protocol_constants.hpp index 087497020..418e32a44 100644 --- a/components/core/src/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/ffi/ir_stream/protocol_constants.hpp @@ -3,7 +3,6 @@ #include #include -#include #include namespace ffi::ir_stream::cProtocol { @@ -13,12 +12,14 @@ namespace Metadata { constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; - constexpr char VersionValue[] = "v0.0.1"; - constexpr char VersionRegex[] = - "^v(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)" - "(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)" - "(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?" - "(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$"; + constexpr char VersionValue[] = "0.0.1"; + + // The following regex can be used to validate a Semantic Versioning string. + // The source of the regex can be found here: https://semver.org/ + constexpr char VersionRegex[] = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)" + "(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)" + "(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?" + "(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$"; constexpr char TimestampPatternKey[] = "TIMESTAMP_PATTERN"; constexpr char TimestampPatternSyntaxKey[] = "TIMESTAMP_PATTERN_SYNTAX"; diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 6962fc2e7..f387fbb5c 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -130,8 +130,7 @@ static epoch_time_ms_t get_current_ts() { template bool match_encoding_type(bool is_four_bytes_encoding) { - static_assert(is_same_v || - is_same_v); + static_assert(is_same_v || is_same_v); if constexpr (is_same_v) { return false == is_four_bytes_encoding; @@ -142,8 +141,7 @@ bool match_encoding_type(bool is_four_bytes_encoding) { template epoch_time_ms_t get_next_timestamp_for_test() { - static_assert(is_same_v || - is_same_v); + static_assert(is_same_v || is_same_v); // We return an absolute timestamp for the eight-byte encoding and a mocked // timestamp delta for the four-byte encoding @@ -166,8 +164,7 @@ bool encode_preamble( epoch_time_ms_t reference_timestamp, vector& ir_buf ) { - static_assert(is_same_v || - is_same_v); + static_assert(is_same_v || is_same_v); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::encode_preamble( @@ -194,8 +191,7 @@ bool encode_message( string& logtype, vector& ir_buf ) { - static_assert(is_same_v || - is_same_v); + static_assert(is_same_v || is_same_v); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::encode_message( @@ -217,8 +213,7 @@ bool encode_message( template IRErrorCode decode_next_message(BufferReader& reader, string& message, epoch_time_ms_t& decoded_ts) { - static_assert(is_same_v || - is_same_v); + static_assert(is_same_v || is_same_v); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::decode_next_message( @@ -245,11 +240,13 @@ TEST_CASE("get_encoding_type", "[ffi][get_encoding_type]") { // Test eight-byte encoding vector eight_byte_encoding_vec{ EightByteEncodingMagicNumber, - EightByteEncodingMagicNumber + MagicNumberLength}; + EightByteEncodingMagicNumber + MagicNumberLength + }; BufferReader eight_byte_ir_buffer{ size_checked_pointer_cast(eight_byte_encoding_vec.data()), - eight_byte_encoding_vec.size()}; + eight_byte_encoding_vec.size() + }; REQUIRE(get_encoding_type(eight_byte_ir_buffer, is_four_bytes_encoding) == IRErrorCode::IRErrorCode_Success); REQUIRE(match_encoding_type(is_four_bytes_encoding)); @@ -257,11 +254,13 @@ TEST_CASE("get_encoding_type", "[ffi][get_encoding_type]") { // Test four-byte encoding vector four_byte_encoding_vec{ FourByteEncodingMagicNumber, - FourByteEncodingMagicNumber + MagicNumberLength}; + FourByteEncodingMagicNumber + MagicNumberLength + }; BufferReader four_byte_ir_buffer{ size_checked_pointer_cast(four_byte_encoding_vec.data()), - four_byte_encoding_vec.size()}; + four_byte_encoding_vec.size() + }; REQUIRE(get_encoding_type(four_byte_ir_buffer, is_four_bytes_encoding) == IRErrorCode::IRErrorCode_Success); REQUIRE(match_encoding_type(is_four_bytes_encoding)); @@ -276,7 +275,8 @@ TEST_CASE("get_encoding_type", "[ffi][get_encoding_type]") { BufferReader incomplete_buffer{ size_checked_pointer_cast(four_byte_encoding_vec.data()), - four_byte_encoding_vec.size() - 1}; + four_byte_encoding_vec.size() - 1 + }; REQUIRE(get_encoding_type(incomplete_buffer, is_four_bytes_encoding) == IRErrorCode::IRErrorCode_Incomplete_IR); @@ -284,7 +284,8 @@ TEST_CASE("get_encoding_type", "[ffi][get_encoding_type]") { vector const invalid_ir_vec{0x02, 0x43, 0x24, 0x34}; BufferReader invalid_ir_buffer{ size_checked_pointer_cast(invalid_ir_vec.data()), - invalid_ir_vec.size()}; + invalid_ir_vec.size() + }; REQUIRE(get_encoding_type(invalid_ir_buffer, is_four_bytes_encoding) == IRErrorCode::IRErrorCode_Corrupted_IR); } @@ -330,10 +331,8 @@ TEMPLATE_TEST_CASE( string_view json_metadata{metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); - REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported - == validate_protocol_version( - metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey) - )); + std::string const version = metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported == validate_protocol_version(version)); REQUIRE(ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); @@ -356,7 +355,8 @@ TEMPLATE_TEST_CASE( == IRErrorCode::IRErrorCode_Success); string_view json_metadata_copied{ size_checked_pointer_cast(json_metadata_vec.data()), - json_metadata_vec.size()}; + json_metadata_vec.size() + }; // Crosscheck with the json_metadata decoded previously REQUIRE(json_metadata_copied == json_metadata); @@ -364,7 +364,8 @@ TEMPLATE_TEST_CASE( ir_buf.resize(encoded_preamble_end_pos - 1); BufferReader incomplete_preamble_buffer{ size_checked_pointer_cast(ir_buf.data()), - ir_buf.size()}; + ir_buf.size() + }; incomplete_preamble_buffer.seek_from_begin(MagicNumberLength); REQUIRE(decode_preamble(incomplete_preamble_buffer, metadata_type, metadata_pos, metadata_size) == IRErrorCode::IRErrorCode_Incomplete_IR); @@ -373,7 +374,8 @@ TEMPLATE_TEST_CASE( ir_buf[MagicNumberLength] = 0x23; BufferReader corrupted_preamble_buffer{ size_checked_pointer_cast(ir_buf.data()), - ir_buf.size()}; + ir_buf.size() + }; REQUIRE(decode_preamble(corrupted_preamble_buffer, metadata_type, metadata_pos, metadata_size) == IRErrorCode::IRErrorCode_Corrupted_IR); } @@ -415,7 +417,8 @@ TEMPLATE_TEST_CASE( ir_buf.resize(encoded_message_end_pos - 4); BufferReader incomplete_preamble_buffer{ size_checked_pointer_cast(ir_buf.data()), - ir_buf.size()}; + ir_buf.size() + }; REQUIRE(IRErrorCode::IRErrorCode_Incomplete_IR == decode_next_message(incomplete_preamble_buffer, message, timestamp)); } @@ -449,7 +452,8 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { ir_with_extra_escape.at(logtype_end_pos - 1) = ir::cVariablePlaceholderEscapeCharacter; BufferReader ir_with_extra_escape_buffer{ size_checked_pointer_cast(ir_with_extra_escape.data()), - ir_with_extra_escape.size()}; + ir_with_extra_escape.size() + }; REQUIRE(IRErrorCode::IRErrorCode_Decode_Error == decode_next_message( ir_with_extra_escape_buffer, @@ -463,7 +467,8 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { = enum_to_underlying_type(VariablePlaceholder::Dictionary); BufferReader ir_with_extra_placeholder_buffer{ size_checked_pointer_cast(ir_with_extra_placeholder.data()), - ir_with_extra_placeholder.size()}; + ir_with_extra_placeholder.size() + }; REQUIRE(IRErrorCode::IRErrorCode_Decode_Error == decode_next_message( ir_with_extra_placeholder_buffer, @@ -475,51 +480,56 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][decode_next_message]") { string const message = "Static <\text>, dictVar1, 123, 456345232.7234223, " "dictVar2, 987, 654.3, end of static text"; - auto test_timestamp_delta = [&](epoch_time_ms_t ref_ts_delta) { + auto test_timestamp_delta = [&](epoch_time_ms_t ts_delta) { vector ir_buf; string logtype; REQUIRE(true - == encode_message( - ref_ts_delta, - message, - logtype, - ir_buf - )); + == encode_message(ts_delta, message, logtype, ir_buf) + ); BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; string decoded_message; - epoch_time_ms_t delta_ts; + epoch_time_ms_t decoded_delta_ts{}; REQUIRE(IRErrorCode::IRErrorCode_Success == decode_next_message( ir_buffer, decoded_message, - delta_ts + decoded_delta_ts )); REQUIRE(message == decoded_message); - REQUIRE(delta_ts == ref_ts_delta); + REQUIRE(decoded_delta_ts == ts_delta); + return true; }; - test_timestamp_delta(0); - - test_timestamp_delta(INT8_MIN); - test_timestamp_delta(INT8_MIN + 1); - test_timestamp_delta(INT8_MAX - 1); - test_timestamp_delta(INT8_MAX); - - test_timestamp_delta(INT16_MIN); - test_timestamp_delta(INT16_MIN + 1); - test_timestamp_delta(INT16_MAX - 1); - test_timestamp_delta(INT16_MAX); + auto timestamp_deltas = GENERATE( + 0, + INT8_MIN, + INT8_MIN + 1, + INT8_MAX - 1, + INT8_MAX, + INT16_MIN, + INT16_MIN + 1, + INT16_MAX - 1, + INT16_MAX, + INT32_MIN, + INT32_MIN + 1, + INT32_MAX - 1, + INT32_MAX, + INT64_MIN, + INT64_MAX + ); + REQUIRE(test_timestamp_delta(timestamp_deltas)); +} - test_timestamp_delta(INT32_MIN); - test_timestamp_delta(INT32_MIN + 1); - test_timestamp_delta(INT32_MAX - 1); - test_timestamp_delta(INT32_MAX); +TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("v0.0.1")); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("0.1")); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("0.a.1")); - test_timestamp_delta(INT64_MIN); - test_timestamp_delta(INT64_MIN + 1); - test_timestamp_delta(INT64_MAX - 1); - test_timestamp_delta(INT64_MAX); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Too_New == validate_protocol_version("1000.0.0")); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported + == validate_protocol_version(ffi::ir_stream::cProtocol::Metadata::VersionValue)); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported == validate_protocol_version("v0.0.0")); } TEMPLATE_TEST_CASE( @@ -566,7 +576,8 @@ TEMPLATE_TEST_CASE( BufferReader complete_ir_buffer{ size_checked_pointer_cast(ir_buf.data()), - ir_buf.size()}; + ir_buf.size() + }; bool is_four_bytes_encoding; REQUIRE(get_encoding_type(complete_ir_buffer, is_four_bytes_encoding) @@ -585,10 +596,8 @@ TEMPLATE_TEST_CASE( auto* json_metadata_ptr{size_checked_pointer_cast(ir_buf.data() + metadata_pos)}; string_view json_metadata{json_metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); - REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported - == validate_protocol_version( - metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey) - )); + string const version = metadata_json.at(ffi::ir_stream::cProtocol::Metadata::VersionKey); + REQUIRE(ffi::ir_stream::IRProtocolErrorCode_Supported == validate_protocol_version(version)); REQUIRE(ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); From 2c7572082b751929d984c927bcd4810561484383 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Thu, 28 Sep 2023 15:48:14 -0400 Subject: [PATCH 07/10] Fix clang-format --- .../core/tests/test-ir_encoding_methods.cpp | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index f387fbb5c..f7571afdd 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -130,7 +130,10 @@ static epoch_time_ms_t get_current_ts() { template bool match_encoding_type(bool is_four_bytes_encoding) { - static_assert(is_same_v || is_same_v); + static_assert( + (is_same_v) + || (is_same_v) + ); if constexpr (is_same_v) { return false == is_four_bytes_encoding; @@ -141,7 +144,10 @@ bool match_encoding_type(bool is_four_bytes_encoding) { template epoch_time_ms_t get_next_timestamp_for_test() { - static_assert(is_same_v || is_same_v); + static_assert( + (is_same_v) + || (is_same_v) + ); // We return an absolute timestamp for the eight-byte encoding and a mocked // timestamp delta for the four-byte encoding @@ -164,7 +170,10 @@ bool encode_preamble( epoch_time_ms_t reference_timestamp, vector& ir_buf ) { - static_assert(is_same_v || is_same_v); + static_assert( + (is_same_v) + || (is_same_v) + ); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::encode_preamble( @@ -191,7 +200,10 @@ bool encode_message( string& logtype, vector& ir_buf ) { - static_assert(is_same_v || is_same_v); + static_assert( + (is_same_v) + || (is_same_v) + ); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::encode_message( @@ -213,7 +225,10 @@ bool encode_message( template IRErrorCode decode_next_message(BufferReader& reader, string& message, epoch_time_ms_t& decoded_ts) { - static_assert(is_same_v || is_same_v); + static_assert( + (is_same_v) + || (is_same_v) + ); if constexpr (is_same_v) { return ffi::ir_stream::eight_byte_encoding::decode_next_message( From 4d2fc5d1e26e21d40799e7fffc119ba7623e22e7 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Fri, 29 Sep 2023 07:57:59 -0400 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- components/core/src/ffi/ir_stream/decoding_methods.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/core/src/ffi/ir_stream/decoding_methods.hpp b/components/core/src/ffi/ir_stream/decoding_methods.hpp index 78f66c417..000dee70f 100644 --- a/components/core/src/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/ffi/ir_stream/decoding_methods.hpp @@ -155,9 +155,9 @@ IRErrorCode decode_preamble( * @param protocol_version * @return IRProtocolErrorCode_Supported if the protocol version is supported. * @return IRProtocolErrorCode_Too_Old if the protocol version is no longer - * supported by the current build version. - * @return IRProtocolErrorCode_Too_New if the protocol version is newer than the - * build version. + * supported by this build's protocol version. + * @return IRProtocolErrorCode_Too_New if the protocol version is newer than this + * build's protocol version. * @return IRProtocolErrorCode_Invalid if the protocol version does not follow * the SemVer specification. */ From 180a20fae28c9a52dc650b52aea2beffd824c681 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Fri, 29 Sep 2023 08:30:01 -0400 Subject: [PATCH 09/10] Remove lambda --- .../core/tests/test-ir_encoding_methods.cpp | 70 +++++++++---------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index f7571afdd..a2419819f 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -495,45 +495,39 @@ TEST_CASE("message_decode_error", "[ffi][decode_next_message]") { TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][decode_next_message]") { string const message = "Static <\text>, dictVar1, 123, 456345232.7234223, " "dictVar2, 987, 654.3, end of static text"; - auto test_timestamp_delta = [&](epoch_time_ms_t ts_delta) { - vector ir_buf; - string logtype; - REQUIRE(true - == encode_message(ts_delta, message, logtype, ir_buf) - ); - - BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; - string decoded_message; - epoch_time_ms_t decoded_delta_ts{}; - REQUIRE(IRErrorCode::IRErrorCode_Success - == decode_next_message( - ir_buffer, - decoded_message, - decoded_delta_ts - )); - REQUIRE(message == decoded_message); - REQUIRE(decoded_delta_ts == ts_delta); - return true; - }; - - auto timestamp_deltas = GENERATE( - 0, - INT8_MIN, - INT8_MIN + 1, - INT8_MAX - 1, - INT8_MAX, - INT16_MIN, - INT16_MIN + 1, - INT16_MAX - 1, - INT16_MAX, - INT32_MIN, - INT32_MIN + 1, - INT32_MAX - 1, - INT32_MAX, - INT64_MIN, - INT64_MAX + auto ts_delta = GENERATE( + static_cast(0), + static_cast(INT8_MIN), + static_cast(INT8_MIN + 1), + static_cast(INT8_MAX - 1), + static_cast(INT8_MAX), + static_cast(INT16_MIN), + static_cast(INT16_MIN + 1), + static_cast(INT16_MAX - 1), + static_cast(INT16_MAX), + static_cast(INT32_MIN), + static_cast(INT32_MIN + 1), + static_cast(INT32_MAX - 1), + static_cast(INT32_MAX), + static_cast(INT64_MIN), + static_cast(INT64_MAX) + ); + vector ir_buf; + string logtype; + REQUIRE(true == encode_message(ts_delta, message, logtype, ir_buf) ); - REQUIRE(test_timestamp_delta(timestamp_deltas)); + + BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; + string decoded_message; + epoch_time_ms_t decoded_delta_ts{}; + REQUIRE(IRErrorCode::IRErrorCode_Success + == decode_next_message( + ir_buffer, + decoded_message, + decoded_delta_ts + )); + REQUIRE(message == decoded_message); + REQUIRE(decoded_delta_ts == ts_delta); } TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { From ab20e28e5941919bc919eda80d47d9df968c59ea Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Fri, 29 Sep 2023 08:35:15 -0400 Subject: [PATCH 10/10] Fix Kirk's comments --- components/core/tests/test-ir_encoding_methods.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index a2419819f..53dc345c4 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -514,8 +514,7 @@ TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][decode_next_me ); vector ir_buf; string logtype; - REQUIRE(true == encode_message(ts_delta, message, logtype, ir_buf) - ); + REQUIRE(encode_message(ts_delta, message, logtype, ir_buf)); BufferReader ir_buffer{size_checked_pointer_cast(ir_buf.data()), ir_buf.size()}; string decoded_message;