From 4fcc2ec0768f09244799a655d6a549f6323d3588 Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Thu, 26 Dec 2024 12:59:56 -0600 Subject: [PATCH] unit tests and fixes --- .../include/file_manager/direct_open_file.hpp | 7 +-- .../include/file_manager/i_open_file.hpp | 7 +-- .../include/file_manager/open_file.hpp | 3 -- .../include/file_manager/open_file_base.hpp | 5 -- .../file_manager/ring_buffer_open_file.hpp | 7 +-- .../src/file_manager/file_manager.cpp | 5 +- .../src/file_manager/open_file.cpp | 4 -- .../src/file_manager/open_file_base.cpp | 8 +-- .../include/mocks/mock_open_file.hpp | 54 +++++++++---------- .../repertory_test/src/file_manager_test.cpp | 41 +++++++------- 10 files changed, 55 insertions(+), 86 deletions(-) diff --git a/repertory/librepertory/include/file_manager/direct_open_file.hpp b/repertory/librepertory/include/file_manager/direct_open_file.hpp index 1462860f..9e97b735 100644 --- a/repertory/librepertory/include/file_manager/direct_open_file.hpp +++ b/repertory/librepertory/include/file_manager/direct_open_file.hpp @@ -68,11 +68,6 @@ private: auto download_chunk(std::size_t chunk, bool skip_active) -> api_error; -protected: - [[nodiscard]] auto is_download_complete() const -> bool override { - return false; - } - public: auto close() -> bool override; @@ -86,7 +81,7 @@ public: return total_chunks_; } - [[nodiscard]] auto is_complete() const -> bool override { return true; } + [[nodiscard]] auto is_complete() const -> bool override { return false; } [[nodiscard]] auto is_write_supported() const -> bool override { return false; diff --git a/repertory/librepertory/include/file_manager/i_open_file.hpp b/repertory/librepertory/include/file_manager/i_open_file.hpp index c873bd3e..871e3c6f 100644 --- a/repertory/librepertory/include/file_manager/i_open_file.hpp +++ b/repertory/librepertory/include/file_manager/i_open_file.hpp @@ -62,6 +62,8 @@ public: [[nodiscard]] virtual auto get_source_path() const -> std::string = 0; + [[nodiscard]] virtual auto is_complete() const -> bool = 0; + [[nodiscard]] virtual auto is_directory() const -> bool = 0; [[nodiscard]] virtual auto is_write_supported() const -> bool = 0; @@ -84,9 +86,6 @@ public: virtual void set_api_path(const std::string &api_path) = 0; - virtual void - set_open_data(std::map open_data) = 0; - [[nodiscard]] virtual auto write(std::uint64_t write_offset, const data_buffer &data, std::size_t &bytes_written) -> api_error = 0; @@ -107,8 +106,6 @@ public: [[nodiscard]] virtual auto get_handles() const -> std::vector = 0; - [[nodiscard]] virtual auto is_complete() const -> bool = 0; - [[nodiscard]] virtual auto is_modified() const -> bool = 0; virtual void remove(std::uint64_t handle) = 0; diff --git a/repertory/librepertory/include/file_manager/open_file.hpp b/repertory/librepertory/include/file_manager/open_file.hpp index 382df53a..b24558a4 100644 --- a/repertory/librepertory/include/file_manager/open_file.hpp +++ b/repertory/librepertory/include/file_manager/open_file.hpp @@ -95,9 +95,6 @@ private: void update_background_reader(std::size_t read_chunk); -protected: - [[nodiscard]] auto is_download_complete() const -> bool override; - public: auto close() -> bool override; diff --git a/repertory/librepertory/include/file_manager/open_file_base.hpp b/repertory/librepertory/include/file_manager/open_file_base.hpp index 346e7f4d..4e6105dd 100644 --- a/repertory/librepertory/include/file_manager/open_file_base.hpp +++ b/repertory/librepertory/include/file_manager/open_file_base.hpp @@ -131,8 +131,6 @@ private: protected: [[nodiscard]] auto do_io(std::function action) -> api_error; - [[nodiscard]] virtual auto is_download_complete() const -> bool = 0; - void reset_timeout(); auto set_api_error(const api_error &e) -> api_error; @@ -191,9 +189,6 @@ public: void remove_all() override; void set_api_path(const std::string &api_path) override; - - void - set_open_data(std::map open_data) override; }; } // namespace repertory diff --git a/repertory/librepertory/include/file_manager/ring_buffer_open_file.hpp b/repertory/librepertory/include/file_manager/ring_buffer_open_file.hpp index a4ebb867..38b4f6aa 100644 --- a/repertory/librepertory/include/file_manager/ring_buffer_open_file.hpp +++ b/repertory/librepertory/include/file_manager/ring_buffer_open_file.hpp @@ -69,11 +69,6 @@ private: auto download_chunk(std::size_t chunk, bool skip_active) -> api_error; -protected: - [[nodiscard]] auto is_download_complete() const -> bool override { - return false; - } - public: [[nodiscard]] static auto can_handle_file(std::uint64_t file_size, std::size_t chunk_size, @@ -101,7 +96,7 @@ public: return total_chunks_; } - [[nodiscard]] auto is_complete() const -> bool override { return true; } + [[nodiscard]] auto is_complete() const -> bool override { return false; } [[nodiscard]] auto is_write_supported() const -> bool override { return false; diff --git a/repertory/librepertory/src/file_manager/file_manager.cpp b/repertory/librepertory/src/file_manager/file_manager.cpp index cb67a199..abfa0f2b 100644 --- a/repertory/librepertory/src/file_manager/file_manager.cpp +++ b/repertory/librepertory/src/file_manager/file_manager.cpp @@ -368,7 +368,10 @@ auto file_manager::is_processing(const std::string &api_path) const -> bool { auto closeable_file = file_iter->second; open_lock.unlock(); - return closeable_file->is_modified() || not closeable_file->is_complete(); + return closeable_file->is_write_supported() + ? closeable_file->is_modified() || + not closeable_file->is_complete() + : false; } auto file_manager::open(const std::string &api_path, bool directory, diff --git a/repertory/librepertory/src/file_manager/open_file.cpp b/repertory/librepertory/src/file_manager/open_file.cpp index f1effeef..f553c4bf 100644 --- a/repertory/librepertory/src/file_manager/open_file.cpp +++ b/repertory/librepertory/src/file_manager/open_file.cpp @@ -403,10 +403,6 @@ auto open_file::get_read_state(std::size_t chunk) const -> bool { auto open_file::is_complete() const -> bool { return get_read_state().all(); } -auto open_file::is_download_complete() const -> bool { - return get_read_state().all(); -} - auto open_file::native_operation( i_open_file::native_operation_callback callback) -> api_error { if (stop_requested_) { diff --git a/repertory/librepertory/src/file_manager/open_file_base.cpp b/repertory/librepertory/src/file_manager/open_file_base.cpp index e030d54c..c1bc906c 100644 --- a/repertory/librepertory/src/file_manager/open_file_base.cpp +++ b/repertory/librepertory/src/file_manager/open_file_base.cpp @@ -119,7 +119,7 @@ auto open_file_base::can_close() const -> bool { return true; } - if (is_download_complete()) { + if (is_complete()) { return true; } @@ -303,12 +303,6 @@ void open_file_base::set_api_path(const std::string &api_path) { fsi_.api_parent = utils::path::get_parent_api_path(api_path); } -void open_file_base::set_open_data( - std::map open_data) { - recur_mutex_lock file_lock(file_mtx_); - open_data_ = std::move(open_data); -} - auto open_file_base::close() -> bool { unique_mutex_lock io_lock(io_thread_mtx_); if (io_stop_requested_ || not io_thread_) { diff --git a/repertory/repertory_test/include/mocks/mock_open_file.hpp b/repertory/repertory_test/include/mocks/mock_open_file.hpp index 677416d8..305ed853 100644 --- a/repertory/repertory_test/include/mocks/mock_open_file.hpp +++ b/repertory/repertory_test/include/mocks/mock_open_file.hpp @@ -29,6 +29,13 @@ namespace repertory { class mock_open_file : public virtual i_closeable_open_file { public: + MOCK_METHOD(void, add, (std::uint64_t handle, open_file_data ofd), + (override)); + + MOCK_METHOD(bool, can_close, (), (const, override)); + + MOCK_METHOD(bool, close, (), (override)); + MOCK_METHOD(std::string, get_api_path, (), (const, override)); MOCK_METHOD(std::size_t, get_chunk_size, (), (const, override)); @@ -49,14 +56,28 @@ public: MOCK_METHOD(bool, get_allocated, (), (const, override)); + MOCK_METHOD(std::vector, get_handles, (), (const, override)); + + MOCK_METHOD((std::map &), get_open_data, (), + (override)); + + MOCK_METHOD((const std::map &), get_open_data, + (), (const, override)); + MOCK_METHOD(bool, get_read_state, (std::size_t chunk), (const, override)); MOCK_METHOD(std::string, get_source_path, (), (const, override)); MOCK_METHOD(bool, has_handle, (std::uint64_t handle), (const, override)); + MOCK_METHOD(bool, is_complete, (), (const, override)); + MOCK_METHOD(bool, is_directory, (), (const, override)); + MOCK_METHOD(bool, is_modified, (), (const, override)); + + MOCK_METHOD(bool, is_write_supported, (), (const, override)); + MOCK_METHOD(api_error, native_operation, (native_operation_callback callback), (override)); @@ -69,6 +90,10 @@ public: data_buffer &data), (override)); + MOCK_METHOD(void, remove, (std::uint64_t handle), (override)); + + MOCK_METHOD(void, remove_all, (), (override)); + MOCK_METHOD(api_error, resize, (std::uint64_t new_file_size), (override)); MOCK_METHOD(void, set_api_path, (const std::string &api_path), (override)); @@ -77,35 +102,6 @@ public: (std::uint64_t write_offset, const data_buffer &data, std::size_t &bytes_written), (override)); - - MOCK_METHOD(void, add, (std::uint64_t handle, open_file_data ofd), - (override)); - - MOCK_METHOD(bool, can_close, (), (const, override)); - - MOCK_METHOD(bool, close, (), (override)); - - MOCK_METHOD(std::vector, get_handles, (), (const, override)); - - MOCK_METHOD((std::map &), get_open_data, (), - (override)); - - MOCK_METHOD((const std::map &), get_open_data, - (), (const, override)); - - MOCK_METHOD(bool, is_complete, (), (const, override)); - - MOCK_METHOD(bool, is_modified, (), (const, override)); - - MOCK_METHOD(bool, is_write_supported, (), (const, override)); - - MOCK_METHOD(void, remove, (std::uint64_t handle), (override)); - - MOCK_METHOD(void, remove_all, (), (override)); - - MOCK_METHOD(void, set_open_data, - ((std::map open_data)), - (override)); }; } // namespace repertory diff --git a/repertory/repertory_test/src/file_manager_test.cpp b/repertory/repertory_test/src/file_manager_test.cpp index b904b0c4..e1b58816 100644 --- a/repertory/repertory_test/src/file_manager_test.cpp +++ b/repertory/repertory_test/src/file_manager_test.cpp @@ -565,7 +565,6 @@ TEST_F(file_manager_test, upload_occurs_after_write_if_fully_downloaded) { EXPECT_STREQ(source_path.c_str(), evt2.get_source().get().c_str()); }); - event_capture capture({"download_end"}); auto now = utils::time::get_time_now(); auto meta = create_meta_attributes( @@ -625,25 +624,28 @@ TEST_F(file_manager_test, upload_occurs_after_write_if_fully_downloaded) { EXPECT_TRUE(mgr.get_open_file(handle, true, open_file)); } - std::size_t bytes_written{}; - data_buffer data = {0, 1, 2}; - EXPECT_EQ(api_error::success, open_file->write(0u, data, bytes_written)); - EXPECT_EQ(std::size_t(3u), bytes_written); - open_file.reset(); - - capture.wait_for_empty(); - - EXPECT_CALL(mp, upload_file("/test_write_full_download.txt", source_path, _)) - .WillOnce(Return(api_error::success)); - - event_capture ec2({ + event_capture capture({ "item_timeout", "file_upload_queued", "file_upload_completed", }); + + std::size_t bytes_written{}; + data_buffer data = {0, 1, 2}; + EXPECT_EQ(api_error::success, open_file->write(0u, data, bytes_written)); + EXPECT_EQ(std::size_t(3u), bytes_written); + + while (not open_file->is_complete()) { + std::this_thread::sleep_for(10ms); + } + open_file.reset(); + + EXPECT_CALL(mp, upload_file("/test_write_full_download.txt", source_path, _)) + .WillOnce(Return(api_error::success)); + mgr.close(handle); - ec2.wait_for_empty(); + capture.wait_for_empty(); EXPECT_EQ(std::size_t(0U), mgr.get_open_file_count()); EXPECT_EQ(std::size_t(0U), mgr.get_open_handle_count()); @@ -809,10 +811,9 @@ TEST_F(file_manager_test, evict_file_fails_if_unable_to_get_filesystem_item) { file_manager mgr(*cfg, mp); EXPECT_CALL(mp, get_filesystem_item) - .WillRepeatedly([&meta](const std::string &api_path, bool directory, - filesystem_item &fsi) -> api_error { - return api_error::error; - }); + .WillRepeatedly( + [](const std::string &api_path, bool directory, + filesystem_item &fsi) -> api_error { return api_error::error; }); EXPECT_FALSE(mgr.evict_file("/test_open.txt")); } @@ -822,8 +823,8 @@ TEST_F(file_manager_test, evict_file_fails_if_source_path_is_empty) { file_manager mgr(*cfg, mp); EXPECT_CALL(mp, get_filesystem_item) - .WillRepeatedly([&meta](const std::string &api_path, bool directory, - filesystem_item &fsi) -> api_error { + .WillRepeatedly([](const std::string &api_path, bool directory, + filesystem_item &fsi) -> api_error { fsi.api_path = api_path; fsi.api_parent = utils::path::get_parent_api_path(api_path); fsi.directory = directory;