Address more code review concerns

components/core/CMakeLists.txt

@@ -332,7 +332,7 @@ set(SOURCE_FILES_unitTest
         src/clp/GlobalSQLiteMetadataDB.hpp
         src/clp/Grep.cpp
         src/clp/Grep.hpp
-        src/clp/ir/Constant.hpp
+        src/clp/ir/constants.hpp
         src/clp/ir/LogEvent.hpp
         src/clp/ir/LogEventDeserializer.cpp
         src/clp/ir/LogEventDeserializer.hpp

components/core/src/clp/clp/CMakeLists.txt

@@ -36,7 +36,7 @@ set(
         ../GlobalMySQLMetadataDB.hpp
         ../GlobalSQLiteMetadataDB.cpp
         ../GlobalSQLiteMetadataDB.hpp
-        ../ir/Constant.hpp
+        ../ir/constants.hpp
         ../ir/LogEvent.hpp
         ../ir/LogEventDeserializer.cpp
         ../ir/LogEventDeserializer.hpp

components/core/src/clp/clp/FileDecompressor.cpp

@@ -4,7 +4,7 @@
 #include <boost/filesystem/operations.hpp>
 #include <boost/filesystem/path.hpp>
 
-#include "../ir/Constant.hpp"
+#include "../ir/constants.hpp"
 #include "../ir/LogEventSerializer.hpp"
 #include "../ir/utils.hpp"
 
@@ -33,8 +33,10 @@ bool rename_ir_file(
         size_t begin_message_ix,
         size_t end_message_ix
 ) {
-    string ir_file_name = file_orig_id + "_" + std::to_string(begin_message_ix) + "_"
-                               + std::to_string(end_message_ix) + ir::cIrFileExtension;
+    auto ir_file_name = file_orig_id + "_";
+    ir_file_name += std::to_string(begin_message_ix) + "_";
+    ir_file_name += std::to_string(end_message_ix) + "_";
+    ir_file_name += ir::cIrFileExtension;
 
     auto const renamed_ir_path = output_directory / ir_file_name;
     try {
@@ -167,7 +169,9 @@ bool FileDecompressor::decompress_to_ir(
     }
 
     boost::filesystem::path temp_ir_path{temp_output_dir};
-    temp_ir_path /= m_encoded_file.get_id_as_string() + ir::cIrFileExtension;
+    auto temp_ir_file = m_encoded_file.get_id_as_string();
+    temp_ir_file += ir::cIrFileExtension;
+    temp_ir_path /= temp_ir_file;
 
     auto const& file_orig_id = m_encoded_file.get_orig_file_id_as_string();
     auto begin_message_ix = m_encoded_file.get_begin_message_ix();
@@ -214,10 +218,12 @@ bool FileDecompressor::decompress_to_ir(
             }
         }
 
-        if (false == ir_serializer.serialize_log_event(
+        if (false
+            == ir_serializer.serialize_log_event(
                     m_encoded_message.get_ts_in_milli(),
                     m_decompressed_message
-            )) {
+            ))
+        {
             SPDLOG_ERROR(
                     "Failed to serialize log event: {} with ts {}",
                     m_decompressed_message.c_str(),

components/core/src/clp/clp/FileDecompressor.hpp

@@ -1,8 +1,8 @@
 #ifndef CLP_CLP_FILEDECOMPRESSOR_HPP
 #define CLP_CLP_FILEDECOMPRESSOR_HPP
 
-#include <string>
 #include <cstddef>
+#include <string>
 
 #include "../FileWriter.hpp"
 #include "../streaming_archive/MetadataDB.hpp"

components/core/src/clp/ir/LogEventSerializer.cpp

@@ -25,7 +25,7 @@ LogEventSerializer<encoded_variable_t>::~LogEventSerializer() {
 }
 
 template <typename encoded_variable_t>
-auto LogEventSerializer<encoded_variable_t>::open(string const& file_path) -> ErrorCode {
+auto LogEventSerializer<encoded_variable_t>::open(string const& file_path) -> bool {
     if (m_is_open) {
         throw OperationFailed(ErrorCode_NotReady, __FILENAME__, __LINE__);
     }
@@ -41,32 +41,32 @@ auto LogEventSerializer<encoded_variable_t>::open(string const& file_path) -> Er
     if constexpr (std::is_same_v<encoded_variable_t, four_byte_encoded_variable_t>) {
         m_prev_event_timestamp = 0;
         res = ffi::ir_stream::four_byte_encoding::serialize_preamble(
-            cTimestampPattern,
-            cTimestampPatternSyntax,
-            cTimezoneID,
-            m_prev_event_timestamp,
-            m_ir_buf
+                cTimestampPattern,
+                cTimestampPatternSyntax,
+                cTimezoneID,
+                m_prev_event_timestamp,
+                m_ir_buf
         );
     } else {
         res = clp::ffi::ir_stream::eight_byte_encoding::serialize_preamble(
-            cTimestampPattern,
-            cTimestampPatternSyntax,
-            cTimezoneID,
-            m_ir_buf
+                cTimestampPattern,
+                cTimestampPatternSyntax,
+                cTimezoneID,
+                m_ir_buf
         );
     }
 
     if (false == res) {
         close_writer();
-        return ErrorCode_Failure;
+        return true;
     }
 
     m_is_open = true;
 
     // Flush the preamble
     flush();
 
-    return ErrorCode_Success;
+    return false;
 }
 
 template <typename encoded_variable_t>
@@ -75,8 +75,8 @@ auto LogEventSerializer<encoded_variable_t>::flush() -> void {
         throw OperationFailed(ErrorCode_NotInit, __FILENAME__, __LINE__);
     }
     m_zstd_compressor.write(
-        size_checked_pointer_cast<char const>(m_ir_buf.data()),
-        m_ir_buf.size()
+            size_checked_pointer_cast<char const>(m_ir_buf.data()),
+            m_ir_buf.size()
     );
     m_serialized_size += m_ir_buf.size();
     m_ir_buf.clear();
@@ -106,19 +106,19 @@ auto LogEventSerializer<encoded_variable_t>::serialize_log_event(
     bool res{};
     if constexpr (std::is_same_v<encoded_variable_t, eight_byte_encoded_variable_t>) {
         res = clp::ffi::ir_stream::eight_byte_encoding::serialize_log_event(
-            timestamp,
-            message,
-            logtype,
-            m_ir_buf
+                timestamp,
+                message,
+                logtype,
+                m_ir_buf
         );
     } else {
         auto const timestamp_delta = timestamp - m_prev_event_timestamp;
         m_prev_event_timestamp = timestamp;
         res = clp::ffi::ir_stream::four_byte_encoding::serialize_log_event(
-            timestamp_delta,
-            message,
-            logtype,
-            m_ir_buf
+                timestamp_delta,
+                message,
+                logtype,
+                m_ir_buf
         );
     }
     if (false == res) {
@@ -139,9 +139,9 @@ auto LogEventSerializer<encoded_variable_t>::close_writer() -> void {
 template LogEventSerializer<eight_byte_encoded_variable_t>::~LogEventSerializer();
 template LogEventSerializer<four_byte_encoded_variable_t>::~LogEventSerializer();
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::open(string const& file_path
-) -> ErrorCode;
+) -> bool;
 template auto LogEventSerializer<four_byte_encoded_variable_t>::open(string const& file_path
-) -> ErrorCode;
+) -> bool;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::flush() -> void;
 template auto LogEventSerializer<four_byte_encoded_variable_t>::flush() -> void;
 template auto LogEventSerializer<eight_byte_encoded_variable_t>::close() -> void;

components/core/src/clp/ir/LogEventSerializer.hpp

@@ -52,10 +52,14 @@ class LogEventSerializer {
 
     /**
      * Creates a Zstandard-compressed IR file on disk, and writes the IR file's preamble.
-     *
      * @param file_path
+     * @return true on success, false if the preamble fails to serialize
+     * @throw FileWriter::OperationFailed if the FileWriter fails to open the file specified by
+     * file_path
+     * @throw streaming_compression::zstd::Compressor if Zstandard compressor fails to open
+     * @throw ir::LogEventSerializer::OperationFailed on failure
      */
-    [[nodiscard]] auto open(std::string const& file_path) -> ErrorCode;
+    [[nodiscard]] auto open(std::string const& file_path) -> bool;
 
     /**
      * Flushes any buffered data.

components/core/src/clp/ir/Constant.hpp → components/core/src/clp/ir/constants.hpp

@@ -1,8 +1,10 @@
 #ifndef CLP_IR_CONSTANTS_HPP
 #define CLP_IR_CONSTANTS_HPP
 
+#include <string_view>
+
 namespace clp::ir {
-constexpr char cIrFileExtension[] = ".clp.zst";
+constexpr std::string_view cIrFileExtension{".clp.zst"};
 }  // namespace clp::ir
 
 #endif  // CLP_IR_CONSTANTS_HPP