From 72452c1360624bae4ce12fea1b32709affcfcbee Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Thu, 30 Jan 2025 15:51:36 +0000 Subject: [PATCH] haiqi review --- .../clp/streaming_archive/ArchiveMetadata.cpp | 18 +++++++++------ .../clp/streaming_archive/ArchiveMetadata.hpp | 22 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index fe3b9f20d..eacead7a0 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -4,6 +4,8 @@ #include +#include "../Array.hpp" + namespace clp::streaming_archive { ArchiveMetadata::ArchiveMetadata( archive_format_version_t archive_format_version, @@ -28,15 +30,17 @@ ArchiveMetadata::ArchiveMetadata( auto ArchiveMetadata::create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata { struct stat file_stat{}; - auto clp_rc = file_reader.try_fstat(file_stat); - if (clp::ErrorCode::ErrorCode_Success != clp_rc) { - throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + if (auto const clp_rc = file_reader.try_fstat(file_stat); + clp::ErrorCode::ErrorCode_Success != clp_rc) + { + throw OperationFailed(clp_rc, __FILENAME__, __LINE__); } - std::vector buf(file_stat.st_size); - clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); - if (clp::ErrorCode::ErrorCode_Success != clp_rc) { - throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + clp::Array buf(file_stat.st_size); + if (auto const clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); + clp::ErrorCode::ErrorCode_Success != clp_rc) + { + throw OperationFailed(clp_rc, __FILENAME__, __LINE__); } ArchiveMetadata metadata; diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 1a91e746e..8ae16ebbf 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -2,6 +2,7 @@ #define CLP_STREAMING_ARCHIVE_ARCHIVEMETADATA_HPP #include +#include #include "../Defs.h" #include "../ffi/encoding_methods.hpp" @@ -33,7 +34,10 @@ class ArchiveMetadata { }; // Constructors + // The class must be constructible to convert from msgpack::object. + // https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_adaptor ArchiveMetadata() = default; + /** * Constructs a metadata object with the given parameters * @param archive_format_version @@ -55,8 +59,7 @@ class ArchiveMetadata { * @return The created instance. * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. - * @throw `msgpack::type_error` if MessagePack object can't be converted to - * `ArchiveMetadata`. + * @throw `msgpack::type_error` if MessagePack object can't be converted to `ArchiveMetadata`. */ [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata; @@ -92,15 +95,15 @@ class ArchiveMetadata { [[nodiscard]] auto get_end_timestamp() const { return m_end_timestamp; } - [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string const& { + [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string_view const& { return m_variable_encoding_methods_version; } - [[nodiscard]] auto get_variables_schema_version() const -> std::string const& { + [[nodiscard]] auto get_variables_schema_version() const -> std::string_view const& { return m_variables_schema_version; } - [[nodiscard]] auto get_compression_type() const -> std::string const& { + [[nodiscard]] auto get_compression_type() const -> std::string_view const& { return m_compression_type; } @@ -112,7 +115,7 @@ class ArchiveMetadata { void expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp); /** - * Packs `ArchiveMetadata` to MessagePack and writes to open file. + * Packs `ArchiveMetadata` to MessagePack and writes to the open file. * * @param file_writer Writer for archive metadata file. * @throw FileWriter::OperationFailed if failed to write. @@ -125,6 +128,7 @@ class ArchiveMetadata { MSGPACK_NVP("variables_schema_version", m_variables_schema_version), MSGPACK_NVP("compression_type", m_compression_type), MSGPACK_NVP("creator_id", m_creator_id), + MSGPACK_NVP("creation_idx", m_creation_idx), MSGPACK_NVP("begin_timestamp", m_begin_timestamp), MSGPACK_NVP("end_timestamp", m_end_timestamp), MSGPACK_NVP("uncompressed_size", m_uncompressed_size), @@ -145,9 +149,9 @@ class ArchiveMetadata { // The size of the archive uint64_t m_compressed_size{0}; uint64_t m_dynamic_compressed_size{0}; - std::string m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; - std::string m_variables_schema_version{ffi::cVariablesSchemaVersion}; - std::string m_compression_type{cCompressionTypeZstd}; + std::string_view m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; + std::string_view m_variables_schema_version{ffi::cVariablesSchemaVersion}; + std::string_view m_compression_type{cCompressionTypeZstd}; }; } // namespace clp::streaming_archive