From 9cac42ef5b01540cda13ece346d1ebd08da75170 Mon Sep 17 00:00:00 2001 From: Matteo Nicoli Date: Thu, 30 Jan 2025 16:41:46 +0100 Subject: [PATCH] refactor(clp_s): delete clp_s::FileReader (#664) --- components/core/CMakeLists.txt | 2 - components/core/src/clp_s/ArchiveWriter.cpp | 12 +- components/core/src/clp_s/CMakeLists.txt | 2 - .../core/src/clp_s/CommandLineArguments.cpp | 48 +++-- components/core/src/clp_s/Decompressor.hpp | 4 +- components/core/src/clp_s/FileReader.cpp | 150 ---------------- components/core/src/clp_s/FileReader.hpp | 166 ------------------ components/core/src/clp_s/JsonParser.hpp | 1 - components/core/src/clp_s/SchemaReader.hpp | 1 - .../src/clp_s/TimestampDictionaryReader.hpp | 1 - .../core/src/clp_s/ZstdDecompressor.cpp | 12 +- .../core/src/clp_s/ZstdDecompressor.hpp | 5 +- .../core/src/clp_s/indexer/CMakeLists.txt | 2 - 13 files changed, 39 insertions(+), 367 deletions(-) delete mode 100644 components/core/src/clp_s/FileReader.cpp delete mode 100644 components/core/src/clp_s/FileReader.hpp diff --git a/components/core/CMakeLists.txt b/components/core/CMakeLists.txt index 7c48a3d39..a0265fae4 100644 --- a/components/core/CMakeLists.txt +++ b/components/core/CMakeLists.txt @@ -273,8 +273,6 @@ set(SOURCE_FILES_clp_s_unitTest src/clp_s/DictionaryEntry.hpp src/clp_s/DictionaryWriter.cpp src/clp_s/DictionaryWriter.hpp - src/clp_s/FileReader.cpp - src/clp_s/FileReader.hpp src/clp_s/FileWriter.cpp src/clp_s/FileWriter.hpp src/clp_s/InputConfig.cpp diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index ec5a2c5f1..01cbdc517 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -177,23 +177,21 @@ void ArchiveWriter::write_archive_files( FileWriter& archive_writer, std::vector const& files ) { - FileReader reader; for (auto const& file : files) { std::string file_path = m_archive_path + file.n; - reader.open(file_path); + clp::FileReader reader(file_path); char read_buffer[cReadBlockSize]; while (true) { size_t num_bytes_read{0}; - ErrorCode const error_code + clp::ErrorCode const error_code = reader.try_read(read_buffer, cReadBlockSize, num_bytes_read); - if (ErrorCodeEndOfFile == error_code) { + if (clp::ErrorCode::ErrorCode_EndOfFile == error_code) { break; - } else if (ErrorCodeSuccess != error_code) { - throw OperationFailed(error_code, __FILENAME__, __LINE__); + } else if (clp::ErrorCode::ErrorCode_Success != error_code) { + throw OperationFailed(static_cast(error_code), __FILENAME__, __LINE__); } archive_writer.write(read_buffer, num_bytes_read); } - reader.close(); if (false == std::filesystem::remove(file_path)) { throw OperationFailed(ErrorCodeFileExists, __FILENAME__, __LINE__); } diff --git a/components/core/src/clp_s/CMakeLists.txt b/components/core/src/clp_s/CMakeLists.txt index ef2eee40e..4806d481a 100644 --- a/components/core/src/clp_s/CMakeLists.txt +++ b/components/core/src/clp_s/CMakeLists.txt @@ -108,8 +108,6 @@ set( DictionaryWriter.cpp DictionaryWriter.hpp ErrorCode.hpp - FileReader.cpp - FileReader.hpp FileWriter.cpp FileWriter.hpp InputConfig.cpp diff --git a/components/core/src/clp_s/CommandLineArguments.cpp b/components/core/src/clp_s/CommandLineArguments.cpp index 7fcee9a70..ccbdec11f 100644 --- a/components/core/src/clp_s/CommandLineArguments.cpp +++ b/components/core/src/clp_s/CommandLineArguments.cpp @@ -9,9 +9,9 @@ #include #include "../clp/cli_utils.hpp" +#include "../clp/FileReader.hpp" #include "../clp/type_utils.hpp" #include "../reducer/types.hpp" -#include "FileReader.hpp" namespace po = boost::program_options; @@ -33,34 +33,32 @@ bool read_paths_from_file( std::string const& input_path_list_file_path, std::vector& path_destination ) { - FileReader reader; - auto error_code = reader.try_open(input_path_list_file_path); - if (ErrorCodeFileNotFound == error_code) { - SPDLOG_ERROR( - "Failed to open input path list file {} - file not found", - input_path_list_file_path - ); - return false; - } else if (ErrorCodeSuccess != error_code) { - SPDLOG_ERROR("Error opening input path list file {}", input_path_list_file_path); - return false; - } - - std::string line; - while (true) { - error_code = reader.try_read_to_delimiter('\n', false, false, line); - if (ErrorCodeSuccess != error_code) { - break; - } - if (false == line.empty()) { - path_destination.push_back(line); + try { + clp::FileReader reader(input_path_list_file_path); + std::string line; + clp::ErrorCode error_code; + while (true) { + error_code = reader.try_read_to_delimiter('\n', false, false, line); + if (clp::ErrorCode::ErrorCode_Success != error_code) { + break; + } + if (false == line.empty()) { + path_destination.push_back(line); + } } - } - if (ErrorCodeEndOfFile != error_code) { + if (clp::ErrorCode::ErrorCode_EndOfFile != error_code) { + return false; + } + return true; + } catch (clp::FileReader::OperationFailed const& e) { + SPDLOG_ERROR( + "Failed to open file for reading - {} - {}", + input_path_list_file_path, + e.what() + ); return false; } - return true; } /** diff --git a/components/core/src/clp_s/Decompressor.hpp b/components/core/src/clp_s/Decompressor.hpp index 0a15930ec..bf1ed16a7 100644 --- a/components/core/src/clp_s/Decompressor.hpp +++ b/components/core/src/clp_s/Decompressor.hpp @@ -5,8 +5,8 @@ #include +#include "../clp/FileReader.hpp" #include "../clp/ReaderInterface.hpp" -#include "FileReader.hpp" #include "TraceableException.hpp" namespace clp_s { @@ -49,7 +49,7 @@ class Decompressor { * @param file_reader * @param file_read_buffer_capacity The maximum amount of data to read from a file at a time */ - virtual void open(FileReader& file_reader, size_t file_read_buffer_capacity) = 0; + virtual void open(clp::FileReader& file_reader, size_t file_read_buffer_capacity) = 0; /** * Initializes the decompressor to decompress from an open clp reader diff --git a/components/core/src/clp_s/FileReader.cpp b/components/core/src/clp_s/FileReader.cpp deleted file mode 100644 index 91bafed0a..000000000 --- a/components/core/src/clp_s/FileReader.cpp +++ /dev/null @@ -1,150 +0,0 @@ -// Code from CLP - -#include "FileReader.hpp" - -#include -#include - -#include -#include - -using std::string; - -namespace clp_s { -FileReader::~FileReader() { - close(); - free(m_getdelim_buf); -} - -ErrorCode FileReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) { - if (nullptr == m_file) { - return ErrorCodeNotInit; - } - if (nullptr == buf) { - return ErrorCodeBadParam; - } - - num_bytes_read = fread(buf, sizeof(*buf), num_bytes_to_read, m_file); - if (num_bytes_read < num_bytes_to_read) { - if (ferror(m_file)) { - return ErrorCodeErrno; - } else if (feof(m_file)) { - if (0 == num_bytes_read) { - return ErrorCodeEndOfFile; - } - } - } - - return ErrorCodeSuccess; -} - -ErrorCode FileReader::try_seek_from_begin(size_t pos) { - if (nullptr == m_file) { - return ErrorCodeNotInit; - } - - int retval = fseeko(m_file, pos, SEEK_SET); - if (0 != retval) { - return ErrorCodeErrno; - } - - return ErrorCodeSuccess; -} - -ErrorCode FileReader::try_get_pos(size_t& pos) { - if (nullptr == m_file) { - return ErrorCodeNotInit; - } - - pos = ftello(m_file); - if ((off_t)-1 == pos) { - return ErrorCodeErrno; - } - - return ErrorCodeSuccess; -} - -ErrorCode FileReader::try_open(string const& path) { - // Cleanup in case caller forgot to call close before calling this function - close(); - - m_file = fopen(path.c_str(), "rb"); - if (nullptr == m_file) { - if (ENOENT == errno) { - return ErrorCodeFileNotFound; - } - return ErrorCodeErrno; - } - - return ErrorCodeSuccess; -} - -void FileReader::open(string const& path) { - ErrorCode error_code = try_open(path); - if (ErrorCodeSuccess != error_code) { - throw OperationFailed(error_code, __FILENAME__, __LINE__); - } -} - -void FileReader::close() { - if (m_file != nullptr) { - // NOTE: We don't check errors for fclose since it seems the only reason it could fail is if - // it was interrupted by a signal - fclose(m_file); - m_file = nullptr; - } -} - -ErrorCode -FileReader::try_read_to_delimiter(char delim, bool keep_delimiter, bool append, string& str) { - assert(nullptr != m_file); - - if (false == append) { - str.clear(); - } - ssize_t num_bytes_read = getdelim(&m_getdelim_buf, &m_getdelim_buf_len, delim, m_file); - if (num_bytes_read < 1) { - if (ferror(m_file)) { - return ErrorCodeErrno; - } else if (feof(m_file)) { - return ErrorCodeEndOfFile; - } - } - if (false == keep_delimiter && delim == m_getdelim_buf[num_bytes_read - 1]) { - --num_bytes_read; - } - str.append(m_getdelim_buf, num_bytes_read); - - return ErrorCodeSuccess; -} - -ErrorCode FileReader::try_read_exact_length(char* buf, size_t num_bytes) { - size_t num_bytes_read; - auto error_code = try_read(buf, num_bytes, num_bytes_read); - if (ErrorCodeSuccess != error_code) { - return error_code; - } - if (num_bytes_read < num_bytes) { - return ErrorCodeTruncated; - } - - return ErrorCodeSuccess; -} - -size_t FileReader::get_pos() { - size_t pos; - ErrorCode error_code = try_get_pos(pos); - if (ErrorCodeSuccess != error_code) { - throw OperationFailed(error_code, __FILENAME__, __LINE__); - } - - return pos; -} - -void FileReader::seek_from_begin(size_t pos) { - ErrorCode error_code = try_seek_from_begin(pos); - if (ErrorCodeSuccess != error_code) { - throw OperationFailed(error_code, __FILENAME__, __LINE__); - } -} -} // namespace clp_s diff --git a/components/core/src/clp_s/FileReader.hpp b/components/core/src/clp_s/FileReader.hpp deleted file mode 100644 index 59e88eaec..000000000 --- a/components/core/src/clp_s/FileReader.hpp +++ /dev/null @@ -1,166 +0,0 @@ -// Code from CLP - -#ifndef CLP_S_FILEREADER_HPP -#define CLP_S_FILEREADER_HPP - -#include -#include - -#include "ErrorCode.hpp" -#include "TraceableException.hpp" - -namespace clp_s { -class FileReader { -public: - // Types - class OperationFailed : public TraceableException { - public: - // Constructors - OperationFailed(ErrorCode error_code, char const* const filename, int line_number) - : TraceableException(error_code, filename, line_number) {} - }; - - // Constructor - FileReader() : m_file(nullptr), m_getdelim_buf_len(0), m_getdelim_buf(nullptr) {} - - // Destructor - ~FileReader(); - - // Methods implementing the ReaderInterface - /** - * Tries to get the current position of the read head in the file - * @param pos Position of the read head in the file - * @return ErrorCodeNotInit if the file is not open - * @return ErrorCodeErrno on error - * @return ErrorCodeSuccess on success - */ - ErrorCode try_get_pos(size_t& pos); - - /** - * Tries to seek from the beginning of the file to the given position - * @param pos The position to seek to - * @return ErrorCodeNotInit if the file is not open - * @return ErrorCodeErrno on error - * @return ErrorCodeSuccess on success - */ - ErrorCode try_seek_from_begin(size_t pos); - - /** - * Tries to read up to a given number of bytes from the file - * @param buf The buffer to read into - * @param num_bytes_to_read The number of bytes to try and read - * @param num_bytes_read The actual number of bytes read - * @return ErrorCodeNotInit if the file is not open - * @return ErrorCodeBadParam if buf is invalid - * @return ErrorCodeErrno on error - * @return ErrorCodeEndOfFile on EOF - * @return ErrorCodeSuccess on success - */ - ErrorCode try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read); - - /** - * Tries to read a string from the file until it reaches the specified delimiter - * @param delim The delimiter to stop at - * @param keep_delimiter Whether to include the delimiter in the output string or not - * @param append Whether to append to the given string or replace its contents - * @param str The string read - * @return ErrorCodeSuccess on success - * @return ErrorCodeEndOfFile on EOF - * @return ErrorCodeErrno otherwise - */ - ErrorCode try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str); - - /** - * Tries to read a number of bytes - * @param buf The buffer to read into - * @param num_bytes Number of bytes to read - * @return Same as the underlying medium's try_read method - * @return ErrorCodeTruncated if 0 < # bytes read < num_bytes - */ - ErrorCode try_read_exact_length(char* buf, size_t num_bytes); - - /** - * Tries to read a numeric value - * @tparam ValueType The type of the value to read - * @param value The value read - * @return Same as the underlying medium's try_read_exact_length method - */ - template - ErrorCode try_read_numeric_value(ValueType& value) { - ErrorCode error_code - = try_read_exact_length(reinterpret_cast(&value), sizeof(value)); - if (ErrorCodeSuccess != error_code) { - return error_code; - } - return ErrorCodeSuccess; - } - - /** - * Reads a numeric value - * @tparam ValueType The type of the value to read - * @param value The value read - * @param eof_possible Whether EOF is possible or not - * @return true on success - * @return false on EOF if eof_possible is true - * @throw FileReader::OperationFailed on failure - */ - template - bool read_numeric_value(ValueType& value, bool eof_possible) { - ErrorCode error_code = try_read_numeric_value(value); - if (ErrorCodeEndOfFile == error_code && eof_possible) { - return false; - } - if (ErrorCodeSuccess != error_code) { - throw OperationFailed(error_code, __FILENAME__, __LINE__); - } - return true; - } - - // Methods - /** - * Checks if the file is open - * @return true if the file is open, false otherwise - */ - bool is_open() const { return m_file != nullptr; } - - /** - * Tries to open a file - * @param path - * @return ErrorCodeSuccess on success - * @return ErrorCodeFileNotFound if the file was not found - * @return ErrorCodeErrno otherwise - */ - ErrorCode try_open(std::string const& path); - - /** - * Opens a file - * @param path - * @throw FileReader::OperationFailed on failure - */ - void open(std::string const& path); - - /** - * Closes the file if it's open - */ - void close(); - - /** - * Gets the current position of the read head - * @return Position of the read head - */ - size_t get_pos(); - - /** - * Seeks from the beginning to the given position - * @param pos - */ - void seek_from_begin(size_t pos); - -private: - FILE* m_file; - size_t m_getdelim_buf_len; - char* m_getdelim_buf; -}; -} // namespace clp_s - -#endif // CLP_S_FILEREADER_HPP diff --git a/components/core/src/clp_s/JsonParser.hpp b/components/core/src/clp_s/JsonParser.hpp index 12199df6c..7e5b036f7 100644 --- a/components/core/src/clp_s/JsonParser.hpp +++ b/components/core/src/clp_s/JsonParser.hpp @@ -21,7 +21,6 @@ #include "ArchiveWriter.hpp" #include "CommandLineArguments.hpp" #include "DictionaryWriter.hpp" -#include "FileReader.hpp" #include "FileWriter.hpp" #include "InputConfig.hpp" #include "ParsedMessage.hpp" diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 75f9ff117..af77f1155 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -9,7 +9,6 @@ #include #include "ColumnReader.hpp" -#include "FileReader.hpp" #include "JsonSerializer.hpp" #include "SchemaTree.hpp" #include "search/Projection.hpp" diff --git a/components/core/src/clp_s/TimestampDictionaryReader.hpp b/components/core/src/clp_s/TimestampDictionaryReader.hpp index c2108e587..249dc6ff6 100644 --- a/components/core/src/clp_s/TimestampDictionaryReader.hpp +++ b/components/core/src/clp_s/TimestampDictionaryReader.hpp @@ -4,7 +4,6 @@ #include #include -#include "FileReader.hpp" #include "search/FilterOperation.hpp" #include "TimestampEntry.hpp" #include "TimestampPattern.hpp" diff --git a/components/core/src/clp_s/ZstdDecompressor.cpp b/components/core/src/clp_s/ZstdDecompressor.cpp index aaa101356..1d6fc48b7 100644 --- a/components/core/src/clp_s/ZstdDecompressor.cpp +++ b/components/core/src/clp_s/ZstdDecompressor.cpp @@ -66,8 +66,8 @@ ZstdDecompressor::try_read(char const* buf, size_t num_bytes_to_read, size_t& nu m_file_read_buffer_capacity, m_file_read_buffer_length ); - if (ErrorCodeSuccess != error_code) { - if (ErrorCodeEndOfFile == error_code) { + if (clp::ErrorCode::ErrorCode_Success != error_code) { + if (clp::ErrorCode::ErrorCode_EndOfFile == error_code) { num_bytes_read = decompressed_stream_block.pos; if (0 == decompressed_stream_block.pos) { return ErrorCodeEndOfFile; @@ -75,7 +75,7 @@ ZstdDecompressor::try_read(char const* buf, size_t num_bytes_to_read, size_t& nu return ErrorCodeSuccess; } } else { - return error_code; + return static_cast(error_code); } } @@ -164,7 +164,7 @@ void ZstdDecompressor::open(char const* compressed_data_buf, size_t compressed_d reset_stream(); } -void ZstdDecompressor::open(FileReader& file_reader, size_t file_read_buffer_capacity) { +void ZstdDecompressor::open(clp::FileReader& file_reader, size_t file_read_buffer_capacity) { if (InputType::NotInitialized != m_input_type) { throw OperationFailed(ErrorCodeNotReady, __FILENAME__, __LINE__); } @@ -288,9 +288,9 @@ ErrorCode ZstdDecompressor::open(std::string const& compressed_file_path) { void ZstdDecompressor::reset_stream() { if (InputType::File == m_input_type) { if (auto rc = m_file_reader->try_seek_from_begin(m_file_reader_initial_pos); - ErrorCodeSuccess != rc && ErrorCodeEndOfFile != rc) + clp::ErrorCode::ErrorCode_Success != rc && clp::ErrorCode::ErrorCode_EndOfFile != rc) { - throw OperationFailed(rc, __FILENAME__, __LINE__); + throw OperationFailed(static_cast(rc), __FILENAME__, __LINE__); } m_file_read_buffer_length = 0; m_compressed_stream_block.size = m_file_read_buffer_length; diff --git a/components/core/src/clp_s/ZstdDecompressor.hpp b/components/core/src/clp_s/ZstdDecompressor.hpp index 5d3c7d1ee..12a4701ff 100644 --- a/components/core/src/clp_s/ZstdDecompressor.hpp +++ b/components/core/src/clp_s/ZstdDecompressor.hpp @@ -9,6 +9,7 @@ #include #include +#include "../clp/FileReader.hpp" #include "../clp/ReaderInterface.hpp" #include "Decompressor.hpp" #include "TraceableException.hpp" @@ -41,7 +42,7 @@ class ZstdDecompressor : public Decompressor { // Methods implementing the Decompressor interface void open(char const* compressed_data_buf, size_t compressed_data_buf_size) override; - void open(FileReader& file_reader, size_t file_read_buffer_capacity) override; + void open(clp::FileReader& file_reader, size_t file_read_buffer_capacity) override; void open(clp::ReaderInterface& reader, size_t file_read_buffer_capacity) override; @@ -130,7 +131,7 @@ class ZstdDecompressor : public Decompressor { ZSTD_DStream* m_decompression_stream; boost::iostreams::mapped_file_source m_memory_mapped_compressed_file; - FileReader* m_file_reader; + clp::FileReader* m_file_reader; clp::ReaderInterface* m_reader; size_t m_file_reader_initial_pos; std::unique_ptr m_file_read_buffer; diff --git a/components/core/src/clp_s/indexer/CMakeLists.txt b/components/core/src/clp_s/indexer/CMakeLists.txt index 8c2738d99..eae6823f7 100644 --- a/components/core/src/clp_s/indexer/CMakeLists.txt +++ b/components/core/src/clp_s/indexer/CMakeLists.txt @@ -41,8 +41,6 @@ set( ../DictionaryReader.hpp ../DictionaryEntry.cpp ../DictionaryEntry.hpp - ../FileReader.cpp - ../FileReader.hpp ../FileWriter.cpp ../FileWriter.hpp ../InputConfig.cpp