From 0f0003cb20bdc6b3bc79b940436189c931ab1972 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 18:53:22 -0500 Subject: [PATCH 1/8] Add version validation for IRv2 --- .../src/clp/ffi/ir_stream/Deserializer.hpp | 6 +- .../core/src/clp/ffi/ir_stream/Serializer.cpp | 2 +- .../clp/ffi/ir_stream/decoding_methods.cpp | 64 ++++++++++++---- .../clp/ffi/ir_stream/decoding_methods.hpp | 27 ++++--- .../clp/ffi/ir_stream/encoding_methods.cpp | 3 +- .../clp/ffi/ir_stream/protocol_constants.hpp | 6 +- .../core/src/clp/ir/LogEventDeserializer.cpp | 2 +- .../core/tests/test-ir_encoding_methods.cpp | 75 +++++++++++++++---- 8 files changed, 136 insertions(+), 49 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp index c1fc13c85..45cca8f26 100644 --- a/components/core/src/clp/ffi/ir_stream/Deserializer.hpp +++ b/components/core/src/clp/ffi/ir_stream/Deserializer.hpp @@ -154,10 +154,8 @@ auto Deserializer::create(ReaderInterface& reader, IrUnitHandler return std::errc::protocol_error; } auto const version = version_iter->get_ref(); - // TODO: Just before the KV-pair IR format is formally released, we should replace this - // hard-coded version check with `ffi::ir_stream::validate_protocol_version`. - if (std::string_view{static_cast(cProtocol::Metadata::BetaVersionValue)} - != version) + if (ffi::ir_stream::IRProtocolErrorCode::Supported + != ffi::ir_stream::validate_protocol_version(version)) { return std::errc::protocol_not_supported; } diff --git a/components/core/src/clp/ffi/ir_stream/Serializer.cpp b/components/core/src/clp/ffi/ir_stream/Serializer.cpp index 01215eb9d..b29cf0492 100644 --- a/components/core/src/clp/ffi/ir_stream/Serializer.cpp +++ b/components/core/src/clp/ffi/ir_stream/Serializer.cpp @@ -253,7 +253,7 @@ auto Serializer::create( ir_buf.insert(ir_buf.cend(), cMagicNumber.begin(), cMagicNumber.end()); nlohmann::json metadata; - metadata.emplace(cProtocol::Metadata::VersionKey, cProtocol::Metadata::BetaVersionValue); + metadata.emplace(cProtocol::Metadata::VersionKey, cProtocol::Metadata::VersionValue); metadata.emplace(cProtocol::Metadata::VariablesSchemaIdKey, cVariablesSchemaVersion); metadata.emplace( cProtocol::Metadata::VariableEncodingMethodsIdKey, diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp index 9388470e4..595d1dc8b 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp @@ -1,6 +1,8 @@ #include "decoding_methods.hpp" +#include #include +#include #include "../../ir/types.hpp" #include "byteswap.hpp" @@ -468,13 +470,23 @@ IRErrorCode deserialize_preamble( return IRErrorCode_Success; } -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; +auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolErrorCode { + // These versions are hardcoded to support the IR protocol version that predates the key-value + // pair IR format. + constexpr std::array cBackwardCompatibleVersions{ + "v0.0.0", + "0.0.1", + "0.0.2" + }; + for (auto const backward_compatible_version : cBackwardCompatibleVersions) { + if (backward_compatible_version == protocol_version) { + return IRProtocolErrorCode::Backward_Compatible; + } } - std::regex const protocol_version_regex{cProtocol::Metadata::VersionRegex}; + + std::regex const protocol_version_regex{ + static_cast(cProtocol::Metadata::VersionRegex) + }; if (false == std::regex_match( protocol_version.begin(), @@ -482,19 +494,45 @@ IRProtocolErrorCode validate_protocol_version(std::string_view protocol_version) protocol_version_regex )) { - return IRProtocolErrorCode_Invalid; + return IRProtocolErrorCode::Invalid; } - std::string_view current_build_protocol_version{cProtocol::Metadata::VersionValue}; + + std::string_view const current_build_protocol_version{ + static_cast(cProtocol::Metadata::VersionValue) + }; + auto get_version_core = [](std::string_view version) -> std::string_view { + // Strip any pre-release version or build info from the version string based on the spec: + // https://semver.org/ + auto const pos_pre_release{version.find('-')}; + if (std::string_view::npos == pos_pre_release) { + return version.substr(0, version.find('+')); + } + return version.substr(0, pos_pre_release); + }; + auto const curr_build_protocol_version_core{get_version_core(current_build_protocol_version)}; + auto const protocol_version_core{get_version_core(protocol_version)}; + if (curr_build_protocol_version_core < protocol_version_core) { + return IRProtocolErrorCode::Too_New; + } + + // Check major version 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; + if (get_major_version(curr_build_protocol_version_core) + > get_major_version(protocol_version_core)) + { + return IRProtocolErrorCode::Too_Old; } - if (get_major_version(current_build_protocol_version) > get_major_version(protocol_version)) { - return IRProtocolErrorCode_Too_Old; + + if (protocol_version_core < get_version_core( + static_cast(cProtocol::Metadata::MinimumSupportedVersionValue) + )) + { + return IRProtocolErrorCode::Too_Old; } - return IRProtocolErrorCode_Supported; + + return IRProtocolErrorCode::Supported; } IRErrorCode deserialize_utc_offset_change(ReaderInterface& reader, UtcOffset& utc_offset) { diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp index fb6f6a3c0..86fa9748b 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp @@ -1,6 +1,7 @@ #ifndef CLP_FFI_IR_STREAM_DECODING_METHODS_HPP #define CLP_FFI_IR_STREAM_DECODING_METHODS_HPP +#include #include #include @@ -20,12 +21,13 @@ typedef enum { IRErrorCode_Incomplete_IR, } IRErrorCode; -typedef enum { - IRProtocolErrorCode_Supported, - IRProtocolErrorCode_Too_Old, - IRProtocolErrorCode_Too_New, - IRProtocolErrorCode_Invalid, -} IRProtocolErrorCode; +enum class IRProtocolErrorCode : uint8_t { + Supported, + Backward_Compatible, + Too_Old, + Too_New, + Invalid, +}; class DecodingException : public TraceableException { public: @@ -193,15 +195,18 @@ IRErrorCode deserialize_utc_offset_change(ReaderInterface& reader, UtcOffset& ut /** * Validates whether the given protocol version can be supported by the current build. * @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 this + * @return IRProtocolErrorCode::Supported if the protocol version is supported. + * @return IRProtocolErrorCode::BackwardCompatible if the protocol version indicates a stream format + * that predates the key-value pair IR format. + * @return IRProtocolErrorCode::Too_Old if the protocol version is no longer supported by this * build's protocol version. - * @return IRProtocolErrorCode_Too_New if the protocol version is newer than this build's protocol + * @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 + * @return IRProtocolErrorCode::Invalid if the protocol version does not follow the SemVer * specification. */ -IRProtocolErrorCode validate_protocol_version(std::string_view protocol_version); +[[nodiscard]] auto validate_protocol_version(std::string_view protocol_version +) -> IRProtocolErrorCode; namespace eight_byte_encoding { /** 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 128d659c1..b50b0ad29 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp @@ -98,7 +98,8 @@ static void add_base_metadata_fields( string_view time_zone_id, nlohmann::json& metadata ) { - metadata[cProtocol::Metadata::VersionKey] = cProtocol::Metadata::VersionValue; + metadata[cProtocol::Metadata::VersionKey] + = cProtocol::Metadata::LatestBackwardCompatibleVersionValue; metadata[cProtocol::Metadata::VariablesSchemaIdKey] = cVariablesSchemaVersion; metadata[cProtocol::Metadata::VariableEncodingMethodsIdKey] = cVariableEncodingMethodsVersion; metadata[cProtocol::Metadata::TimestampPatternKey] = timestamp_pattern; 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 c6fd92397..125993668 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -12,8 +12,10 @@ constexpr int8_t LengthUByte = 0x11; constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; -constexpr char VersionValue[] = "0.0.2"; -constexpr char BetaVersionValue[] = "0.1.0-beta.1"; +constexpr char VersionValue[] = "0.1.0"; +// This is used for IR stream format that predates the key-value pair IR format. +constexpr char LatestBackwardCompatibleVersionValue[] = "0.0.2"; +constexpr char MinimumSupportedVersionValue[] = "0.1.0"; // The following regex can be used to validate a Semantic Versioning string. The source of the // regex can be found here: https://semver.org/ diff --git a/components/core/src/clp/ir/LogEventDeserializer.cpp b/components/core/src/clp/ir/LogEventDeserializer.cpp index 6106568dd..65cfa41ef 100644 --- a/components/core/src/clp/ir/LogEventDeserializer.cpp +++ b/components/core/src/clp/ir/LogEventDeserializer.cpp @@ -42,7 +42,7 @@ auto LogEventDeserializer::create(ReaderInterface& reader return std::errc::protocol_error; } auto metadata_version = version_iter->get_ref(); - if (ffi::ir_stream::IRProtocolErrorCode_Supported + if (ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible != 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 b4f0257c1..7e05133d3 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -630,8 +630,8 @@ TEMPLATE_TEST_CASE( auto metadata_json = nlohmann::json::parse(json_metadata); std::string const version = metadata_json.at(clp::ffi::ir_stream::cProtocol::Metadata::VersionKey); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Supported == validate_protocol_version(version) - ); + REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + == validate_protocol_version(version)); REQUIRE(clp::ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); @@ -844,17 +844,60 @@ TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][deserialize_lo } TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("v0.0.1") - ); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("0.1")); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Invalid == validate_protocol_version("0.a.1")); - - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Too_New - == validate_protocol_version("1000.0.0")); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Supported - == validate_protocol_version(clp::ffi::ir_stream::cProtocol::Metadata::VersionValue)); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Supported - == validate_protocol_version("v0.0.0")); + SECTION("Test invalid versions") { + auto const invalid_versions{GENERATE( + std::string_view{"v0.0.1"}, + std::string_view{"0.1"}, + std::string_view{"0.1.a"}, + std::string_view{"0.a.1"} + )}; + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Invalid + == validate_protocol_version(invalid_versions)) + ); + } + + SECTION("Test backward compatible versions") { + auto const backward_compatible_versions{GENERATE( + std::string_view{"v0.0.0"}, + std::string_view{"0.0.1"}, + std::string_view{"0.0.2"} + )}; + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + == validate_protocol_version(backward_compatible_versions)) + ); + } + + SECTION("Test supported versions") { + auto const supported_versions{GENERATE( + std::string_view{"0.1.0-beta.1"}, + std::string_view{static_cast( + clp::ffi::ir_stream::cProtocol::Metadata::VersionValue + )} + )}; + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Supported + == validate_protocol_version(supported_versions)) + ); + } + + SECTION("Test version too new") { + auto const old_versions{ + GENERATE(std::string_view{"0.0.3"}, std::string_view{"0.0.3-beta.1"}) + }; + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Too_Old + == validate_protocol_version(old_versions)) + ); + } + + SECTION("Test version too new") { + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Too_New + == validate_protocol_version("1000.0.0")) + ); + } } TEMPLATE_TEST_CASE( @@ -905,8 +948,8 @@ TEMPLATE_TEST_CASE( string_view json_metadata{json_metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); string const version = metadata_json.at(clp::ffi::ir_stream::cProtocol::Metadata::VersionKey); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode_Supported == validate_protocol_version(version) - ); + REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + == validate_protocol_version(version)); REQUIRE(clp::ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); REQUIRE(timestamp_pattern_syntax == ts_info.timestamp_pattern_syntax); @@ -1055,7 +1098,7 @@ TEMPLATE_TEST_CASE( nlohmann::json expected_metadata; expected_metadata.emplace( clp::ffi::ir_stream::cProtocol::Metadata::VersionKey, - clp::ffi::ir_stream::cProtocol::Metadata::BetaVersionValue + clp::ffi::ir_stream::cProtocol::Metadata::VersionValue ); expected_metadata.emplace( clp::ffi::ir_stream::cProtocol::Metadata::VariablesSchemaIdKey, From 2d85153ffd4941fc8547163cf20e3f0b0245edb3 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 18:56:05 -0500 Subject: [PATCH 2/8] Reorder... --- components/core/src/clp/ffi/ir_stream/protocol_constants.hpp | 2 +- components/core/tests/test-ir_encoding_methods.cpp | 5 ++++- 2 files changed, 5 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 125993668..744d23c8d 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -13,9 +13,9 @@ constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; constexpr char VersionValue[] = "0.1.0"; +constexpr char MinimumSupportedVersionValue[] = "0.1.0"; // This is used for IR stream format that predates the key-value pair IR format. constexpr char LatestBackwardCompatibleVersionValue[] = "0.0.2"; -constexpr char MinimumSupportedVersionValue[] = "0.1.0"; // The following regex can be used to validate a Semantic Versioning string. The source of the // regex can be found here: https://semver.org/ diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 7e05133d3..e3a99e7d5 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -893,9 +893,12 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { } SECTION("Test version too new") { + auto const new_versions{ + GENERATE(std::string_view{"10000.0.0"}, std::string_view{"0.10000.0"}) + }; REQUIRE( (clp::ffi::ir_stream::IRProtocolErrorCode::Too_New - == validate_protocol_version("1000.0.0")) + == validate_protocol_version(new_versions)) ); } } From 4d859ad6683ff6b7713a5d8231cd72e33cb12876 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 19:48:31 -0500 Subject: [PATCH 3/8] Add version validation on pre release --- .../clp/ffi/ir_stream/decoding_methods.cpp | 23 +++++++++++++++---- .../core/tests/test-ir_encoding_methods.cpp | 8 ++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp index 595d1dc8b..8ca223645 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp @@ -525,13 +525,28 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE return IRProtocolErrorCode::Too_Old; } - if (protocol_version_core < get_version_core( - static_cast(cProtocol::Metadata::MinimumSupportedVersionValue) - )) - { + auto const minimum_supported_version_core{ + get_version_core(cProtocol::Metadata::MinimumSupportedVersionValue) + }; + if (protocol_version_core < minimum_supported_version_core) { return IRProtocolErrorCode::Too_Old; } + if (protocol_version_core == minimum_supported_version_core + && protocol_version.size() != protocol_version_core.size()) + { + // The given protocol core has pre-release version + if (minimum_supported_version_core.size() + == cProtocol::Metadata::MinimumSupportedVersionValue.size()) + { + // The minimum supported version is a formal release + return IRProtocolErrorCode::Too_Old; + } + if (protocol_version < cProtocol::Metadata::MinimumSupportedVersionValue) { + return IRProtocolErrorCode::Too_Old; + } + } + return IRProtocolErrorCode::Supported; } diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index e3a99e7d5..432750950 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -883,9 +883,11 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { } SECTION("Test version too new") { - auto const old_versions{ - GENERATE(std::string_view{"0.0.3"}, std::string_view{"0.0.3-beta.1"}) - }; + auto const old_versions{GENERATE( + std::string_view{"0.0.3"}, + std::string_view{"0.0.3-beta.1"}, + std::string_view{"0.1.0-beta"} + )}; REQUIRE( (clp::ffi::ir_stream::IRProtocolErrorCode::Too_Old == validate_protocol_version(old_versions)) From 005790bcc8df90666ca4d6bb7796543f6e1d2b92 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 19:48:59 -0500 Subject: [PATCH 4/8] Forgot to add protocol --- components/core/src/clp/ffi/ir_stream/protocol_constants.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 744d23c8d..cef813924 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -3,6 +3,7 @@ #include #include +#include #include namespace clp::ffi::ir_stream::cProtocol { @@ -13,10 +14,11 @@ constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; constexpr char VersionValue[] = "0.1.0"; -constexpr char MinimumSupportedVersionValue[] = "0.1.0"; // This is used for IR stream format that predates the key-value pair IR format. constexpr char LatestBackwardCompatibleVersionValue[] = "0.0.2"; +constexpr std::string_view MinimumSupportedVersionValue{"0.1.0-beta.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*)" From db08c503deb239a9e760ddc284a11b8d6240fa3a Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 20:23:26 -0500 Subject: [PATCH 5/8] Use hardcoded versions instead --- .../clp/ffi/ir_stream/decoding_methods.cpp | 60 ++++--------------- .../clp/ffi/ir_stream/decoding_methods.hpp | 8 +-- .../clp/ffi/ir_stream/encoding_methods.cpp | 2 +- .../clp/ffi/ir_stream/protocol_constants.hpp | 7 +-- .../core/tests/test-ir_encoding_methods.cpp | 34 ++++++----- 5 files changed, 37 insertions(+), 74 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp index 8ca223645..a3316000f 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp @@ -1,7 +1,9 @@ #include "decoding_methods.hpp" #include +#include #include +#include #include #include "../../ir/types.hpp" @@ -476,7 +478,7 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE constexpr std::array cBackwardCompatibleVersions{ "v0.0.0", "0.0.1", - "0.0.2" + cProtocol::Metadata::LatestBackwardCompatibleVersion }; for (auto const backward_compatible_version : cBackwardCompatibleVersions) { if (backward_compatible_version == protocol_version) { @@ -497,57 +499,19 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE return IRProtocolErrorCode::Invalid; } - std::string_view const current_build_protocol_version{ - static_cast(cProtocol::Metadata::VersionValue) + // TODO: currently, we hardcode all supported versions. This should be removed once we + // implement a proper version parser + constexpr std::array cSupportedVersions{ + cProtocol::Metadata::VersionValue, + cProtocol::Metadata::MinimumSupportedVersion }; - auto get_version_core = [](std::string_view version) -> std::string_view { - // Strip any pre-release version or build info from the version string based on the spec: - // https://semver.org/ - auto const pos_pre_release{version.find('-')}; - if (std::string_view::npos == pos_pre_release) { - return version.substr(0, version.find('+')); - } - return version.substr(0, pos_pre_release); - }; - auto const curr_build_protocol_version_core{get_version_core(current_build_protocol_version)}; - auto const protocol_version_core{get_version_core(protocol_version)}; - if (curr_build_protocol_version_core < protocol_version_core) { - return IRProtocolErrorCode::Too_New; - } - - // Check major version - auto get_major_version{[](std::string_view version) { - return version.substr(0, version.find('.')); - }}; - if (get_major_version(curr_build_protocol_version_core) - > get_major_version(protocol_version_core)) - { - return IRProtocolErrorCode::Too_Old; - } - - auto const minimum_supported_version_core{ - get_version_core(cProtocol::Metadata::MinimumSupportedVersionValue) - }; - if (protocol_version_core < minimum_supported_version_core) { - return IRProtocolErrorCode::Too_Old; - } - - if (protocol_version_core == minimum_supported_version_core - && protocol_version.size() != protocol_version_core.size()) - { - // The given protocol core has pre-release version - if (minimum_supported_version_core.size() - == cProtocol::Metadata::MinimumSupportedVersionValue.size()) - { - // The minimum supported version is a formal release - return IRProtocolErrorCode::Too_Old; - } - if (protocol_version < cProtocol::Metadata::MinimumSupportedVersionValue) { - return IRProtocolErrorCode::Too_Old; + for (auto const supported_version : cSupportedVersions) { + if (supported_version == protocol_version) { + return IRProtocolErrorCode::Supported; } } - return IRProtocolErrorCode::Supported; + return IRProtocolErrorCode::Unsupported; } IRErrorCode deserialize_utc_offset_change(ReaderInterface& reader, UtcOffset& utc_offset) { diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp index 86fa9748b..e63b6f978 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp @@ -24,8 +24,7 @@ typedef enum { enum class IRProtocolErrorCode : uint8_t { Supported, Backward_Compatible, - Too_Old, - Too_New, + Unsupported, Invalid, }; @@ -198,10 +197,7 @@ IRErrorCode deserialize_utc_offset_change(ReaderInterface& reader, UtcOffset& ut * @return IRProtocolErrorCode::Supported if the protocol version is supported. * @return IRProtocolErrorCode::BackwardCompatible if the protocol version indicates a stream format * that predates the key-value pair IR format. - * @return IRProtocolErrorCode::Too_Old if the protocol version is no longer 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::Unsupported if the protocol version is not supported by this build. * @return IRProtocolErrorCode::Invalid if the protocol version does not follow the SemVer * specification. */ 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 b50b0ad29..7c036de0d 100644 --- a/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/encoding_methods.cpp @@ -99,7 +99,7 @@ static void add_base_metadata_fields( nlohmann::json& metadata ) { metadata[cProtocol::Metadata::VersionKey] - = cProtocol::Metadata::LatestBackwardCompatibleVersionValue; + = cProtocol::Metadata::LatestBackwardCompatibleVersion; metadata[cProtocol::Metadata::VariablesSchemaIdKey] = cVariablesSchemaVersion; metadata[cProtocol::Metadata::VariableEncodingMethodsIdKey] = cVariableEncodingMethodsVersion; metadata[cProtocol::Metadata::TimestampPatternKey] = timestamp_pattern; 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 cef813924..7c6429eca 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -13,11 +13,10 @@ constexpr int8_t LengthUByte = 0x11; constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; -constexpr char VersionValue[] = "0.1.0"; +constexpr std::string_view VersionValue{"0.1.0"}; +constexpr std::string_view MinimumSupportedVersion{"0.1.0-beta.1"}; // This is used for IR stream format that predates the key-value pair IR format. -constexpr char LatestBackwardCompatibleVersionValue[] = "0.0.2"; - -constexpr std::string_view MinimumSupportedVersionValue{"0.1.0-beta.1"}; +constexpr std::string_view LatestBackwardCompatibleVersion{"0.0.2"}; // The following regex can be used to validate a Semantic Versioning string. The source of the // regex can be found here: https://semver.org/ diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 432750950..676cdc289 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -844,6 +844,23 @@ TEST_CASE("decode_next_message_four_byte_timestamp_delta", "[ffi][deserialize_lo } TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Supported + == validate_protocol_version(clp::ffi::ir_stream::cProtocol::Metadata::VersionValue)) + ); + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Supported + == validate_protocol_version( + clp::ffi::ir_stream::cProtocol::Metadata::MinimumSupportedVersion + )) + ); + REQUIRE( + (clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + == validate_protocol_version( + clp::ffi::ir_stream::cProtocol::Metadata::LatestBackwardCompatibleVersion + )) + ); + SECTION("Test invalid versions") { auto const invalid_versions{GENERATE( std::string_view{"v0.0.1"}, @@ -869,19 +886,6 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { ); } - SECTION("Test supported versions") { - auto const supported_versions{GENERATE( - std::string_view{"0.1.0-beta.1"}, - std::string_view{static_cast( - clp::ffi::ir_stream::cProtocol::Metadata::VersionValue - )} - )}; - REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Supported - == validate_protocol_version(supported_versions)) - ); - } - SECTION("Test version too new") { auto const old_versions{GENERATE( std::string_view{"0.0.3"}, @@ -889,7 +893,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { std::string_view{"0.1.0-beta"} )}; REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Too_Old + (clp::ffi::ir_stream::IRProtocolErrorCode::Unsupported == validate_protocol_version(old_versions)) ); } @@ -899,7 +903,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { GENERATE(std::string_view{"10000.0.0"}, std::string_view{"0.10000.0"}) }; REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Too_New + (clp::ffi::ir_stream::IRProtocolErrorCode::Unsupported == validate_protocol_version(new_versions)) ); } From a013d51ecb284388c1700a63248ddb974e8c717d Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 20:24:35 -0500 Subject: [PATCH 6/8] Fix description doc --- components/core/tests/test-ir_encoding_methods.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 676cdc289..22cf87b2a 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -886,7 +886,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { ); } - SECTION("Test version too new") { + SECTION("Test version too old") { auto const old_versions{GENERATE( std::string_view{"0.0.3"}, std::string_view{"0.0.3-beta.1"}, From 0cf884a597a0c0589738bea0ffae7c8c18b4cbbf Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Wed, 6 Nov 2024 21:54:36 -0500 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../core/src/clp/ffi/ir_stream/decoding_methods.cpp | 4 ++-- .../core/src/clp/ffi/ir_stream/decoding_methods.hpp | 10 +++++++--- .../core/src/clp/ffi/ir_stream/protocol_constants.hpp | 3 ++- components/core/tests/test-ir_encoding_methods.cpp | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp index a3316000f..2a696cab2 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp @@ -499,8 +499,8 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE return IRProtocolErrorCode::Invalid; } - // TODO: currently, we hardcode all supported versions. This should be removed once we - // implement a proper version parser + // TODO: Currently, we hardcode all supported versions. This should be removed once we + // implement a proper version parser. constexpr std::array cSupportedVersions{ cProtocol::Metadata::VersionValue, cProtocol::Metadata::MinimumSupportedVersion diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp index e63b6f978..31d766e94 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp @@ -194,9 +194,13 @@ IRErrorCode deserialize_utc_offset_change(ReaderInterface& reader, UtcOffset& ut /** * Validates whether the given protocol version can be supported by the current build. * @param protocol_version - * @return IRProtocolErrorCode::Supported if the protocol version is supported. - * @return IRProtocolErrorCode::BackwardCompatible if the protocol version indicates a stream format - * that predates the key-value pair IR format. + * @return IRProtocolErrorCode::Supported if the protocol version is supported by the key-value + * pair IR stream serializer and deserializer. TODO: Update this once we integrate backwards + * compatibility into the deserializer. + * @return IRProtocolErrorCode::BackwardCompatible if the protocol version is supported by the + * serializer and deserializer for the IR stream format that predates the key-value pair IR stream + * format. TODO: Update this once we integrate backwards compatibility into the key-value pair IR + * stream format. * @return IRProtocolErrorCode::Unsupported if the protocol version is not supported by this build. * @return IRProtocolErrorCode::Invalid if the protocol version does not follow the SemVer * specification. 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 7c6429eca..d589d2a75 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -15,7 +15,8 @@ constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; constexpr std::string_view VersionValue{"0.1.0"}; constexpr std::string_view MinimumSupportedVersion{"0.1.0-beta.1"}; -// This is used for IR stream format that predates the key-value pair IR format. + +// This is used for the IR stream format that predates the key-value pair IR format. constexpr std::string_view LatestBackwardCompatibleVersion{"0.0.2"}; // The following regex can be used to validate a Semantic Versioning string. The source of the diff --git a/components/core/tests/test-ir_encoding_methods.cpp b/components/core/tests/test-ir_encoding_methods.cpp index 22cf87b2a..6b390fb00 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -886,7 +886,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { ); } - SECTION("Test version too old") { + SECTION("Test versions that're too old") { auto const old_versions{GENERATE( std::string_view{"0.0.3"}, std::string_view{"0.0.3-beta.1"}, @@ -898,7 +898,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { ); } - SECTION("Test version too new") { + SECTION("Test versions that're too new") { auto const new_versions{ GENERATE(std::string_view{"10000.0.0"}, std::string_view{"0.10000.0"}) }; From 1a001015eb87c96dda29e3c7436e1791883ccd6c Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 6 Nov 2024 22:06:58 -0500 Subject: [PATCH 8/8] Apply code review comments --- .../clp/ffi/ir_stream/decoding_methods.cpp | 22 +++++++------------ .../clp/ffi/ir_stream/decoding_methods.hpp | 2 +- .../clp/ffi/ir_stream/protocol_constants.hpp | 1 - .../core/src/clp/ir/LogEventDeserializer.cpp | 2 +- .../core/tests/test-ir_encoding_methods.cpp | 14 ++++-------- 5 files changed, 14 insertions(+), 27 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp index 2a696cab2..f61efc1df 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.cpp @@ -1,7 +1,7 @@ #include "decoding_methods.hpp" +#include #include -#include #include #include #include @@ -480,10 +480,10 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE "0.0.1", cProtocol::Metadata::LatestBackwardCompatibleVersion }; - for (auto const backward_compatible_version : cBackwardCompatibleVersions) { - if (backward_compatible_version == protocol_version) { - return IRProtocolErrorCode::Backward_Compatible; - } + if (cBackwardCompatibleVersions.cend() + != std::ranges::find(cBackwardCompatibleVersions, protocol_version)) + { + return IRProtocolErrorCode::BackwardCompatible; } std::regex const protocol_version_regex{ @@ -499,16 +499,10 @@ auto validate_protocol_version(std::string_view protocol_version) -> IRProtocolE return IRProtocolErrorCode::Invalid; } - // TODO: Currently, we hardcode all supported versions. This should be removed once we + // TODO: Currently, we hardcode the supported versions. This should be removed once we // implement a proper version parser. - constexpr std::array cSupportedVersions{ - cProtocol::Metadata::VersionValue, - cProtocol::Metadata::MinimumSupportedVersion - }; - for (auto const supported_version : cSupportedVersions) { - if (supported_version == protocol_version) { - return IRProtocolErrorCode::Supported; - } + if (cProtocol::Metadata::VersionValue == protocol_version) { + return IRProtocolErrorCode::Supported; } return IRProtocolErrorCode::Unsupported; diff --git a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp index 31d766e94..a9bc5c4fd 100644 --- a/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp +++ b/components/core/src/clp/ffi/ir_stream/decoding_methods.hpp @@ -23,7 +23,7 @@ typedef enum { enum class IRProtocolErrorCode : uint8_t { Supported, - Backward_Compatible, + BackwardCompatible, Unsupported, Invalid, }; 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 d589d2a75..d89b99cf5 100644 --- a/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp +++ b/components/core/src/clp/ffi/ir_stream/protocol_constants.hpp @@ -14,7 +14,6 @@ constexpr int8_t LengthUShort = 0x12; constexpr char VersionKey[] = "VERSION"; constexpr std::string_view VersionValue{"0.1.0"}; -constexpr std::string_view MinimumSupportedVersion{"0.1.0-beta.1"}; // This is used for the IR stream format that predates the key-value pair IR format. constexpr std::string_view LatestBackwardCompatibleVersion{"0.0.2"}; diff --git a/components/core/src/clp/ir/LogEventDeserializer.cpp b/components/core/src/clp/ir/LogEventDeserializer.cpp index 65cfa41ef..8a1064a78 100644 --- a/components/core/src/clp/ir/LogEventDeserializer.cpp +++ b/components/core/src/clp/ir/LogEventDeserializer.cpp @@ -42,7 +42,7 @@ auto LogEventDeserializer::create(ReaderInterface& reader return std::errc::protocol_error; } auto metadata_version = version_iter->get_ref(); - if (ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + if (ffi::ir_stream::IRProtocolErrorCode::BackwardCompatible != 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 6b390fb00..1ee1e3542 100644 --- a/components/core/tests/test-ir_encoding_methods.cpp +++ b/components/core/tests/test-ir_encoding_methods.cpp @@ -630,7 +630,7 @@ TEMPLATE_TEST_CASE( auto metadata_json = nlohmann::json::parse(json_metadata); std::string const version = metadata_json.at(clp::ffi::ir_stream::cProtocol::Metadata::VersionKey); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::BackwardCompatible == validate_protocol_version(version)); REQUIRE(clp::ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info); @@ -849,13 +849,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { == validate_protocol_version(clp::ffi::ir_stream::cProtocol::Metadata::VersionValue)) ); REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Supported - == validate_protocol_version( - clp::ffi::ir_stream::cProtocol::Metadata::MinimumSupportedVersion - )) - ); - REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + (clp::ffi::ir_stream::IRProtocolErrorCode::BackwardCompatible == validate_protocol_version( clp::ffi::ir_stream::cProtocol::Metadata::LatestBackwardCompatibleVersion )) @@ -881,7 +875,7 @@ TEST_CASE("validate_protocol_version", "[ffi][validate_version_protocol]") { std::string_view{"0.0.2"} )}; REQUIRE( - (clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + (clp::ffi::ir_stream::IRProtocolErrorCode::BackwardCompatible == validate_protocol_version(backward_compatible_versions)) ); } @@ -957,7 +951,7 @@ TEMPLATE_TEST_CASE( string_view json_metadata{json_metadata_ptr, metadata_size}; auto metadata_json = nlohmann::json::parse(json_metadata); string const version = metadata_json.at(clp::ffi::ir_stream::cProtocol::Metadata::VersionKey); - REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::Backward_Compatible + REQUIRE(clp::ffi::ir_stream::IRProtocolErrorCode::BackwardCompatible == validate_protocol_version(version)); REQUIRE(clp::ffi::ir_stream::cProtocol::Metadata::EncodingJson == metadata_type); set_timestamp_info(metadata_json, ts_info);