Skip to content

Commit

Permalink
Apply code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
LinZhihao-723 committed Nov 7, 2024
1 parent 0cf884a commit 1a00101
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 27 deletions.
22 changes: 8 additions & 14 deletions components/core/src/clp/ffi/ir_stream/decoding_methods.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "decoding_methods.hpp"

#include <algorithm>
#include <array>
#include <exception>
#include <regex>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -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{
Expand All @@ -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<std::string_view, 2> 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;
Expand Down
2 changes: 1 addition & 1 deletion components/core/src/clp/ffi/ir_stream/decoding_methods.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ typedef enum {

enum class IRProtocolErrorCode : uint8_t {
Supported,
Backward_Compatible,
BackwardCompatible,
Unsupported,
Invalid,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"};
Expand Down
2 changes: 1 addition & 1 deletion components/core/src/clp/ir/LogEventDeserializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ auto LogEventDeserializer<encoded_variable_t>::create(ReaderInterface& reader
return std::errc::protocol_error;
}
auto metadata_version = version_iter->get_ref<nlohmann::json::string_t&>();
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;
Expand Down
14 changes: 4 additions & 10 deletions components/core/tests/test-ir_encoding_methods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
))
Expand All @@ -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))
);
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 1a00101

Please sign in to comment.