Address review concern

components/core/CMakeLists.txt

@@ -460,7 +460,6 @@ set(SOURCE_FILES_unitTest
         src/clp/streaming_archive/writer/utils.cpp
         src/clp/streaming_archive/writer/utils.hpp
         src/clp/streaming_compression/Compressor.hpp
-        src/clp/streaming_compression/Constants.hpp
         src/clp/streaming_compression/Decompressor.hpp
         src/clp/streaming_compression/passthrough/Compressor.cpp
         src/clp/streaming_compression/passthrough/Compressor.hpp

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

@@ -89,7 +89,6 @@ set(
         ../streaming_archive/writer/File.hpp
         ../streaming_archive/writer/Segment.cpp
         ../streaming_archive/writer/Segment.hpp
-        ../streaming_compression/Constants.hpp
         ../streaming_compression/Decompressor.hpp
         ../streaming_compression/passthrough/Compressor.cpp
         ../streaming_compression/passthrough/Compressor.hpp

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

@@ -89,7 +89,6 @@ set(
         ../streaming_archive/writer/File.hpp
         ../streaming_archive/writer/Segment.cpp
         ../streaming_archive/writer/Segment.hpp
-        ../streaming_compression/Constants.hpp
         ../streaming_compression/Decompressor.hpp
         ../streaming_compression/passthrough/Compressor.cpp
         ../streaming_compression/passthrough/Compressor.hpp

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

@@ -117,7 +117,6 @@ set(
         ../streaming_archive/writer/utils.cpp
         ../streaming_archive/writer/utils.hpp
         ../streaming_compression/Compressor.hpp
-        ../streaming_compression/Constants.hpp
         ../streaming_compression/Decompressor.hpp
         ../streaming_compression/passthrough/Compressor.cpp
         ../streaming_compression/passthrough/Compressor.hpp

components/core/src/clp/streaming_compression/Compressor.hpp

@@ -9,7 +9,6 @@
 #include "../FileWriter.hpp"
 #include "../TraceableException.hpp"
 #include "../WriterInterface.hpp"
-#include "Constants.hpp"
 
 namespace clp::streaming_compression {
 /**
@@ -31,7 +30,7 @@ class Compressor : public WriterInterface {
     };
 
     // Constructor
-    explicit Compressor(CompressorType type) : m_type{type} {}
+    Compressor() = default;
 
     // Destructor
     virtual ~Compressor() = default;
@@ -74,10 +73,6 @@ class Compressor : public WriterInterface {
      * @param file_writer
      */
     virtual auto open(FileWriter& file_writer) -> void = 0;
-
-private:
-    // Variables
-    CompressorType m_type;
 };
 }  // namespace clp::streaming_compression
 

components/core/src/clp/streaming_compression/Decompressor.hpp

@@ -6,7 +6,6 @@
 #include "../FileReader.hpp"
 #include "../ReaderInterface.hpp"
 #include "../TraceableException.hpp"
-#include "Constants.hpp"
 
 namespace clp::streaming_compression {
 class Decompressor : public ReaderInterface {
@@ -25,7 +24,7 @@ class Decompressor : public ReaderInterface {
     };
 
     // Constructor
-    explicit Decompressor(CompressorType type) : m_compression_type(type) {}
+    Decompressor() = default;
 
     // Destructor
     ~Decompressor() = default;
@@ -57,10 +56,6 @@ class Decompressor : public ReaderInterface {
             char* extraction_buf,
             size_t extraction_len
     ) = 0;
-
-protected:
-    // Variables
-    CompressorType m_compression_type;
 };
 }  // namespace clp::streaming_compression
 

components/core/src/clp/streaming_compression/passthrough/Compressor.cpp

@@ -4,14 +4,8 @@
 
 #include "../../ErrorCode.hpp"
 #include "../../TraceableException.hpp"
-#include "../Compressor.hpp"
-#include "../Constants.hpp"
 
 namespace clp::streaming_compression::passthrough {
-Compressor::Compressor()
-        : ::clp::streaming_compression::Compressor{CompressorType::Passthrough},
-          m_compressed_stream_file_writer{nullptr} {}
-
 auto Compressor::write(char const* data, size_t const data_length) -> void {
     if (nullptr == m_compressed_stream_file_writer) {
         throw OperationFailed(ErrorCode_NotInit, __FILENAME__, __LINE__);

components/core/src/clp/streaming_compression/passthrough/Compressor.hpp

@@ -27,8 +27,8 @@ class Compressor : public ::clp::streaming_compression::Compressor {
         }
     };
 
-    // Constructors
-    Compressor();
+    // Constructor
+    Compressor() = default;
 
     // Destructor
     ~Compressor() override = default;
@@ -76,7 +76,7 @@ class Compressor : public ::clp::streaming_compression::Compressor {
 
 private:
     // Variables
-    FileWriter* m_compressed_stream_file_writer;
+    FileWriter* m_compressed_stream_file_writer{nullptr};
 };
 }  // namespace clp::streaming_compression::passthrough
 

components/core/src/clp/streaming_compression/passthrough/Decompressor.hpp

@@ -26,8 +26,7 @@ class Decompressor : public ::clp::streaming_compression::Decompressor {
 
     // Constructors
     Decompressor()
-            : ::clp::streaming_compression::Decompressor(CompressorType::Passthrough),
-              m_input_type(InputType::NotInitialized),
+            : m_input_type(InputType::NotInitialized),
               m_compressed_data_buf(nullptr),
               m_compressed_data_buf_len(0),
               m_decompressed_stream_pos(0) {}

components/core/src/clp/streaming_compression/zstd/Compressor.cpp

@@ -5,47 +5,12 @@
 #include <spdlog/spdlog.h>
 #include <zstd_errors.h>
 
-#include "../../Array.hpp"
 #include "../../ErrorCode.hpp"
 #include "../../FileWriter.hpp"
 #include "../../TraceableException.hpp"
-#include "../Compressor.hpp"
-#include "../Constants.hpp"
-
-namespace {
-/**
- * Checks if a value returned by ZStd function indicates an error code.
- *
- * For most ZStd functions that return `size_t` results, instead of returning a union type that can
- * either be a valid result or an error code, an unanimous `size_t` type is returned.
- * Usually, if the return value exceeds the maximum possible value of valid results, it is treated
- * as an error code. However, the exact behavior is function-dependent, so ZStd provides:
- * 1. A value checking function `ZSTD_isError`
- * 2. A size_t <-> error_code_enum mapping function `ZSTD_getErrorCode`.
- * See also: https://facebook.github.io/zstd/zstd_manual.html
- *
- * @param result A `size_t` type result returned from ZStd APIs
- * @return Whether the result is an error code and indicates an error has occurred
- */
-auto is_error(size_t result) -> bool {
-    return 0 != ZSTD_isError(result) && ZSTD_error_no_error != ZSTD_getErrorCode(result);
-}
-}  // namespace
 
 namespace clp::streaming_compression::zstd {
-Compressor::Compressor()
-        : ::clp::streaming_compression::Compressor{CompressorType::ZSTD},
-          m_compressed_stream_file_writer{nullptr},
-          m_compression_stream{ZSTD_createCStream()},
-          m_compression_stream_contains_data{false},
-          m_compressed_stream_block_size{ZSTD_CStreamOutSize()},
-          m_compressed_stream_block_buffer{Array<char>{m_compressed_stream_block_size}},
-          m_compressed_stream_block{
-                  .dst = m_compressed_stream_block_buffer.data(),
-                  .size = m_compressed_stream_block_size,
-                  .pos = 0
-          },
-          m_uncompressed_stream_pos{0} {
+Compressor::Compressor() {
     if (nullptr == m_compression_stream) {
         SPDLOG_ERROR("streaming_compression::zstd::Compressor: ZSTD_createCStream() error");
         throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__);
@@ -63,7 +28,7 @@ auto Compressor::open(FileWriter& file_writer, int compression_level) -> void {
 
     // Setup compression stream
     auto const init_result{ZSTD_initCStream(m_compression_stream, compression_level)};
-    if (is_error(init_result)) {
+    if (ZSTD_error_no_error != ZSTD_getErrorCode(init_result)) {
         SPDLOG_ERROR(
                 "streaming_compression::zstd::Compressor: ZSTD_initCStream() error: {}",
                 ZSTD_getErrorName(init_result)
@@ -106,7 +71,7 @@ auto Compressor::write(char const* data, size_t data_length) -> void {
                 &m_compressed_stream_block,
                 &uncompressed_stream_block
         )};
-        if (is_error(compress_result)) {
+        if (ZSTD_error_no_error != ZSTD_getErrorCode(compress_result)) {
             SPDLOG_ERROR(
                     "streaming_compression::zstd::Compressor: ZSTD_compressStream() error: {}",
                     ZSTD_getErrorName(compress_result)
@@ -134,7 +99,7 @@ auto Compressor::flush() -> void {
 
     m_compressed_stream_block.pos = 0;
     auto const end_stream_result{ZSTD_endStream(m_compression_stream, &m_compressed_stream_block)};
-    if (is_error(end_stream_result)) {
+    if (ZSTD_error_no_error != ZSTD_getErrorCode(end_stream_result)) {
         // Note: Output buffer is large enough that it is guaranteed to have enough room to be
         // able to flush the entire buffer, so this can only be an error
         SPDLOG_ERROR(
@@ -168,7 +133,7 @@ auto Compressor::flush_without_ending_frame() -> void {
     while (true) {
         m_compressed_stream_block.pos = 0;
         auto const flush_result{ZSTD_flushStream(m_compression_stream, &m_compressed_stream_block)};
-        if (is_error(flush_result)) {
+        if (ZSTD_error_no_error != ZSTD_getErrorCode(flush_result)) {
             SPDLOG_ERROR(
                     "streaming_compression::zstd::Compressor: ZSTD_compressStream2() error: {}",
                     ZSTD_getErrorName(flush_result)

components/core/src/clp/streaming_compression/zstd/Compressor.hpp

@@ -92,17 +92,20 @@ class Compressor : public ::clp::streaming_compression::Compressor {
 
 private:
     // Variables
-    FileWriter* m_compressed_stream_file_writer;
+    FileWriter* m_compressed_stream_file_writer{nullptr};
 
     // Compressed stream variables
-    ZSTD_CStream* m_compression_stream;
-    bool m_compression_stream_contains_data;
-
-    size_t m_compressed_stream_block_size;
-    Array<char> m_compressed_stream_block_buffer;
-    ZSTD_outBuffer m_compressed_stream_block;
+    ZSTD_CStream* m_compression_stream{ZSTD_createCStream()};
+    bool m_compression_stream_contains_data{false};
+
+    Array<char> m_compressed_stream_block_buffer{ZSTD_CStreamOutSize()};
+    ZSTD_outBuffer m_compressed_stream_block{
+            .dst = m_compressed_stream_block_buffer.data(),
+            .size = m_compressed_stream_block_buffer.size(),
+            .pos = 0
+    };
 
-    size_t m_uncompressed_stream_pos;
+    size_t m_uncompressed_stream_pos{0};
 };
 }  // namespace clp::streaming_compression::zstd
 

components/core/src/clp/streaming_compression/zstd/Decompressor.cpp

@@ -8,8 +8,7 @@
 
 namespace clp::streaming_compression::zstd {
 Decompressor::Decompressor()
-        : ::clp::streaming_compression::Decompressor(CompressorType::ZSTD),
-          m_input_type(InputType::NotInitialized),
+        : m_input_type(InputType::NotInitialized),
           m_decompression_stream(nullptr),
           m_file_reader(nullptr),
           m_file_reader_initial_pos(0),

components/core/tests/test-StreamingCompression.cpp

@@ -2,6 +2,7 @@
 #include <array>
 #include <cstring>
 #include <memory>
+#include <numeric>
 #include <string>
 
 #include <boost/filesystem/operations.hpp>
@@ -97,6 +98,16 @@ TEST_CASE("StreamingCompression", "[StreamingCompression]") {
         num_uncompressed_bytes += chunk_size;
     }
 
+    // Sanity check
+    REQUIRE(
+            (std::accumulate(
+                     cCompressionChunkSizes.cbegin(),
+                     cCompressionChunkSizes.cend(),
+                     size_t{0}
+             )
+             == num_uncompressed_bytes)
+    );
+
     // Cleanup
     boost::filesystem::remove(compressed_file_path);
 }