From 53475f4cd964b81dfa0f983514d68e4914ba46d2 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 12 Jul 2024 16:51:35 +0300 Subject: [PATCH 1/4] feat: Add file-based persistence handler --- src/common/filesettingspersistence.cpp | 83 +++++++++++++ .../displaydevice/filesettingspersistence.h | 48 ++++++++ .../settingspersistenceinterface.h | 1 + .../general/test_filesettingspersistence.cpp | 116 ++++++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 src/common/filesettingspersistence.cpp create mode 100644 src/common/include/displaydevice/filesettingspersistence.h create mode 100644 tests/unit/general/test_filesettingspersistence.cpp diff --git a/src/common/filesettingspersistence.cpp b/src/common/filesettingspersistence.cpp new file mode 100644 index 0000000..43a82de --- /dev/null +++ b/src/common/filesettingspersistence.cpp @@ -0,0 +1,83 @@ +// class header include +#include "displaydevice/filesettingspersistence.h" + +// system includes +#include +#include +#include + +// local includes +#include "displaydevice/logging.h" + +namespace display_device { + FileSettingsPersistence:: + FileSettingsPersistence(std::filesystem::path filepath): + m_filepath { std::move(filepath) } { + if (m_filepath.empty()) { + throw std::runtime_error { "Empty filename provided for FileSettingsPersistence!" }; + } + } + + bool + FileSettingsPersistence::store(const std::vector &data) { + try { + std::ofstream stream { m_filepath, std::ios::binary | std::ios::trunc }; + if (!stream) { + DD_LOG(error) << "Failed to open " << m_filepath << " for writing!"; + return false; + } + + std::ranges::copy(data, std::ostream_iterator { stream }); + return true; + } + catch (const std::exception &error) { + DD_LOG(error) << "Failed to write to " << m_filepath << "! Error:\n" + << error.what(); + return false; + } + } + + std::optional> + FileSettingsPersistence::load() const { + if (std::error_code error_code; !std::filesystem::exists(m_filepath, error_code)) { + if (error_code) { + DD_LOG(error) << "Failed to load " << m_filepath << "! Error:\n" + << "[" << error_code.value() << "] " << error_code.message(); + return std::nullopt; + } + + return std::vector {}; + } + + try { + std::ifstream stream { m_filepath, std::ios::binary }; + if (!stream) { + DD_LOG(error) << "Failed to open " << m_filepath << " for reading!"; + return std::nullopt; + } + + return std::vector { std::istreambuf_iterator { stream }, + std::istreambuf_iterator {} }; + } + catch (const std::exception &error) { + DD_LOG(error) << "Failed to read " << m_filepath << "! Error:\n" + << error.what(); + return std::nullopt; + } + } + + bool + FileSettingsPersistence::clear() { + // Return valud does not matter since we check the error code in case the file could NOT be removed. + std::error_code error_code; + std::filesystem::remove(m_filepath, error_code); + + if (error_code) { + DD_LOG(error) << "Failed to remove " << m_filepath << "! Error:\n" + << "[" << error_code.value() << "] " << error_code.message(); + return false; + } + + return true; + } +} // namespace display_device diff --git a/src/common/include/displaydevice/filesettingspersistence.h b/src/common/include/displaydevice/filesettingspersistence.h new file mode 100644 index 0000000..f78b0a9 --- /dev/null +++ b/src/common/include/displaydevice/filesettingspersistence.h @@ -0,0 +1,48 @@ +#pragma once + +// system includes +#include + +// local includes +#include "settingspersistenceinterface.h" + +namespace display_device { + /** + * @brief Implementation of the SettingsPersistenceInterface, + * that saves/loads the persistent settings to/from the file. + */ + class FileSettingsPersistence: public SettingsPersistenceInterface { + public: + /** + * Default constructor. Does not perform any operations on the file yet. + * @param filepath A non-empty filepath. Throws on empty. + */ + explicit FileSettingsPersistence(std::filesystem::path filepath); + + /** + * Store the data in the file specified in constructor. + * @warning The method does not create missing directories! + * @see SettingsPersistenceInterface::store for more details. + */ + [[nodiscard]] bool + store(const std::vector &data) override; + + /** + * Read the data from the file specified in constructor. + * @note If file does not exist, an empty data list will be returned instead of null optional. + * @see SettingsPersistenceInterface::load for more details. + */ + [[nodiscard]] std::optional> + load() const override; + + /** + * Remove the file specified in constructor (if it exists). + * @see SettingsPersistenceInterface::clear for more details. + */ + [[nodiscard]] bool + clear() override; + + private: + std::filesystem::path m_filepath; + }; +} // namespace display_device diff --git a/src/common/include/displaydevice/settingspersistenceinterface.h b/src/common/include/displaydevice/settingspersistenceinterface.h index 496bc21..8770916 100644 --- a/src/common/include/displaydevice/settingspersistenceinterface.h +++ b/src/common/include/displaydevice/settingspersistenceinterface.h @@ -48,6 +48,7 @@ namespace display_device { /** * @brief Clear the persistent settings data. + * @returns True if data was cleared, false otherwise. * * EXAMPLES: * ```cpp diff --git a/tests/unit/general/test_filesettingspersistence.cpp b/tests/unit/general/test_filesettingspersistence.cpp new file mode 100644 index 0000000..0097119 --- /dev/null +++ b/tests/unit/general/test_filesettingspersistence.cpp @@ -0,0 +1,116 @@ +// system includes +#include +#include + +// local includes +#include "displaydevice/filesettingspersistence.h" +#include "fixtures/fixtures.h" + +namespace { + // Convenience keywords for GMock + using ::testing::HasSubstr; + + // Test fixture(s) for this file + class FileSettingsPersistenceTest: public BaseTest { + public: + ~ + FileSettingsPersistenceTest() override { + std::filesystem::remove(m_filepath); + } + + display_device::FileSettingsPersistence & + getImpl(const std::filesystem::path &filepath = "testfile.ext") { + if (!m_impl) { + m_filepath = filepath; + m_impl = std::make_unique(m_filepath); + } + + return *m_impl; + } + + private: + std::filesystem::path m_filepath; + std::unique_ptr m_impl; + }; + + // Specialized TEST macro(s) for this test file +#define TEST_F_S(...) DD_MAKE_TEST(TEST_F, FileSettingsPersistenceTest, __VA_ARGS__) +} // namespace + +TEST_F_S(EmptyFilenameProvided) { + EXPECT_THAT([]() { const display_device::FileSettingsPersistence persistence { {} }; }, + ThrowsMessage(HasSubstr("Empty filename provided for FileSettingsPersistence!"))); +} + +TEST_F_S(Store, NewFileCreated) { + const std::filesystem::path filepath { "myfile.ext" }; + const std::vector data { 0x00, 0x01, 0x02, 0x04, 'S', 'O', 'M', 'E', ' ', 'D', 'A', 'T', 'A' }; + + EXPECT_FALSE(std::filesystem::exists(filepath)); + EXPECT_TRUE(getImpl(filepath).store(data)); + EXPECT_TRUE(std::filesystem::exists(filepath)); + + std::ifstream stream { filepath, std::ios::binary }; + std::vector file_data { std::istreambuf_iterator { stream }, std::istreambuf_iterator {} }; + EXPECT_EQ(file_data, data); +} + +TEST_F_S(Store, FileOverwritten) { + const std::filesystem::path filepath { "myfile.ext" }; + const std::vector data1 { 0x00, 0x01, 0x02, 0x04, 'S', 'O', 'M', 'E', ' ', 'D', 'A', 'T', 'A', ' ', '1' }; + const std::vector data2 { 0x00, 0x01, 0x02, 0x04, 'S', 'O', 'M', 'E', ' ', 'D', 'A', 'T', 'A', ' ', '2' }; + + { + std::ofstream file { filepath, std::ios_base::binary }; + std::ranges::copy(data1, std::ostream_iterator { file }); + } + + EXPECT_TRUE(std::filesystem::exists(filepath)); + EXPECT_TRUE(getImpl(filepath).store(data2)); + EXPECT_TRUE(std::filesystem::exists(filepath)); + + std::ifstream stream { filepath, std::ios::binary }; + std::vector file_data { std::istreambuf_iterator { stream }, std::istreambuf_iterator {} }; + EXPECT_EQ(file_data, data2); +} + +TEST_F_S(Store, FilepathWithDirectory) { + const std::filesystem::path filepath { "somedir/myfile.ext" }; + const std::vector data { 0x00, 0x01, 0x02, 0x04, 'S', 'O', 'M', 'E', ' ', 'D', 'A', 'T', 'A' }; + + EXPECT_FALSE(std::filesystem::exists(filepath)); + EXPECT_FALSE(getImpl(filepath).store(data)); + EXPECT_FALSE(std::filesystem::exists(filepath)); +} + +TEST_F_S(Load, NoFileAvailable) { + EXPECT_EQ(getImpl().load(), std::vector {}); +} + +TEST_F_S(Load, FileRead) { + const std::filesystem::path filepath { "myfile.ext" }; + const std::vector data { 0x00, 0x01, 0x02, 0x04, 'S', 'O', 'M', 'E', ' ', 'D', 'A', 'T', 'A' }; + + { + std::ofstream file { filepath, std::ios_base::binary }; + std::ranges::copy(data, std::ostream_iterator { file }); + } + + EXPECT_EQ(getImpl(filepath).load(), data); +} + +TEST_F_S(Clear, NoFileAvailable) { + EXPECT_TRUE(getImpl().clear()); +} + +TEST_F_S(Clear, FileRemoved) { + const std::filesystem::path filepath { "myfile.ext" }; + { + std::ofstream file { filepath }; + file << "some data"; + } + + EXPECT_TRUE(std::filesystem::exists(filepath)); + EXPECT_TRUE(getImpl(filepath).clear()); + EXPECT_FALSE(std::filesystem::exists(filepath)); +} From 6a6d86f45825da0a66d5bc2d6be62c51ec1cccc6 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 12 Jul 2024 16:53:38 +0300 Subject: [PATCH 2/4] formatting --- src/common/filesettingspersistence.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/filesettingspersistence.cpp b/src/common/filesettingspersistence.cpp index 43a82de..77300a4 100644 --- a/src/common/filesettingspersistence.cpp +++ b/src/common/filesettingspersistence.cpp @@ -10,8 +10,7 @@ #include "displaydevice/logging.h" namespace display_device { - FileSettingsPersistence:: - FileSettingsPersistence(std::filesystem::path filepath): + FileSettingsPersistence::FileSettingsPersistence(std::filesystem::path filepath): m_filepath { std::move(filepath) } { if (m_filepath.empty()) { throw std::runtime_error { "Empty filename provided for FileSettingsPersistence!" }; From e802ecf9e67b5859936d86f09c97eacfbd47f55b Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 12 Jul 2024 16:55:45 +0300 Subject: [PATCH 3/4] ffs --- tests/unit/general/test_filesettingspersistence.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/general/test_filesettingspersistence.cpp b/tests/unit/general/test_filesettingspersistence.cpp index 0097119..e9dab66 100644 --- a/tests/unit/general/test_filesettingspersistence.cpp +++ b/tests/unit/general/test_filesettingspersistence.cpp @@ -13,8 +13,7 @@ namespace { // Test fixture(s) for this file class FileSettingsPersistenceTest: public BaseTest { public: - ~ - FileSettingsPersistenceTest() override { + ~FileSettingsPersistenceTest() override { std::filesystem::remove(m_filepath); } From ecdabad0fdeb22b6e018edc8c0dc1997804753d3 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Fri, 12 Jul 2024 17:18:15 +0300 Subject: [PATCH 4/4] Use better iterator --- src/common/filesettingspersistence.cpp | 2 +- tests/unit/general/test_filesettingspersistence.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/filesettingspersistence.cpp b/src/common/filesettingspersistence.cpp index 77300a4..d9fb40a 100644 --- a/src/common/filesettingspersistence.cpp +++ b/src/common/filesettingspersistence.cpp @@ -26,7 +26,7 @@ namespace display_device { return false; } - std::ranges::copy(data, std::ostream_iterator { stream }); + std::ranges::copy(data, std::ostreambuf_iterator { stream }); return true; } catch (const std::exception &error) { diff --git a/tests/unit/general/test_filesettingspersistence.cpp b/tests/unit/general/test_filesettingspersistence.cpp index e9dab66..581bbbf 100644 --- a/tests/unit/general/test_filesettingspersistence.cpp +++ b/tests/unit/general/test_filesettingspersistence.cpp @@ -61,7 +61,7 @@ TEST_F_S(Store, FileOverwritten) { { std::ofstream file { filepath, std::ios_base::binary }; - std::ranges::copy(data1, std::ostream_iterator { file }); + std::ranges::copy(data1, std::ostreambuf_iterator { file }); } EXPECT_TRUE(std::filesystem::exists(filepath)); @@ -92,7 +92,7 @@ TEST_F_S(Load, FileRead) { { std::ofstream file { filepath, std::ios_base::binary }; - std::ranges::copy(data, std::ostream_iterator { file }); + std::ranges::copy(data, std::ostreambuf_iterator { file }); } EXPECT_EQ(getImpl(filepath).load(), data);