From 4b9b095104275b2a25a090137218cc6129a540b9 Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Mon, 2 Dec 2024 19:16:52 -0600 Subject: [PATCH] refactor stop --- .../include/providers/base_provider.hpp | 3 +- .../providers/encrypt/encrypt_provider.hpp | 150 +++++++++--------- .../librepertory/include/utils/polling.hpp | 6 +- .../src/file_manager/file_manager.cpp | 25 +-- .../src/providers/base_provider.cpp | 21 ++- .../providers/encrypt/encrypt_provider.cpp | 133 +++++++++------- repertory/librepertory/src/utils/polling.cpp | 2 +- 7 files changed, 173 insertions(+), 167 deletions(-) diff --git a/repertory/librepertory/include/providers/base_provider.hpp b/repertory/librepertory/include/providers/base_provider.hpp index 4f25734c..874b3812 100644 --- a/repertory/librepertory/include/providers/base_provider.hpp +++ b/repertory/librepertory/include/providers/base_provider.hpp @@ -44,10 +44,9 @@ private: api_item_added_callback api_item_added_; std::unique_ptr db3_; i_file_manager *fm_{}; - std::atomic stop_requested_{false}; private: - void remove_deleted_files(); + void remove_deleted_files(const stop_type &stop_requested); protected: [[nodiscard]] static auto diff --git a/repertory/librepertory/include/providers/encrypt/encrypt_provider.hpp b/repertory/librepertory/include/providers/encrypt/encrypt_provider.hpp index cf878933..e69b4f62 100644 --- a/repertory/librepertory/include/providers/encrypt/encrypt_provider.hpp +++ b/repertory/librepertory/include/providers/encrypt/encrypt_provider.hpp @@ -77,74 +77,68 @@ private: const encrypt_config &cfg, std::string &api_path) const -> bool; - void remove_deleted_files(); + void remove_deleted_files(const stop_type &stop_requested); public: [[nodiscard]] auto create_directory(const std::string &api_path, api_meta_map &meta) -> api_error override; - [[nodiscard]] auto - create_directory_clone_source_meta(const std::string & /*source_api_path*/, - const std::string & /*api_path*/) - -> api_error override { - return api_error::not_implemented; - } - - [[nodiscard]] auto create_file(const std::string & /*api_path*/, - api_meta_map & /*meta*/) - -> api_error override { + [[nodiscard]] auto create_directory_clone_source_meta( + const std::string & /*source_api_path*/, + const std::string & /*api_path*/) -> api_error override { return api_error::not_implemented; } [[nodiscard]] auto - get_api_path_from_source(const std::string & /*source_path*/, - std::string & /*api_path*/) const - -> api_error override; + create_file(const std::string & /*api_path*/, + api_meta_map & /*meta*/) -> api_error override { + return api_error::not_implemented; + } + + [[nodiscard]] auto get_api_path_from_source( + const std::string & /*source_path*/, + std::string & /*api_path*/) const -> api_error override; [[nodiscard]] auto get_directory_item_count(const std::string &api_path) const -> std::uint64_t override; - [[nodiscard]] auto get_directory_items(const std::string &api_path, - directory_item_list &list) const - -> api_error override; + [[nodiscard]] auto + get_directory_items(const std::string &api_path, + directory_item_list &list) const -> api_error override; - [[nodiscard]] auto get_file(const std::string &api_path, api_file &file) const - -> api_error override; - - [[nodiscard]] auto get_file_list(api_file_list &list, - std::string &marker) const - -> api_error override; - - [[nodiscard]] auto get_file_size(const std::string &api_path, - std::uint64_t &file_size) const - -> api_error override; - - [[nodiscard]] auto get_filesystem_item(const std::string &api_path, - bool directory, - filesystem_item &fsi) const - -> api_error override; - - [[nodiscard]] auto get_filesystem_item_and_file(const std::string &api_path, - api_file &file, - filesystem_item &fsi) const - -> api_error override; + [[nodiscard]] auto get_file(const std::string &api_path, + api_file &file) const -> api_error override; [[nodiscard]] auto - get_filesystem_item_from_source_path(const std::string &source_path, - filesystem_item &fsi) const - -> api_error override; + get_file_list(api_file_list &list, + std::string &marker) const -> api_error override; - [[nodiscard]] auto get_pinned_files() const - -> std::vector override; + [[nodiscard]] auto + get_file_size(const std::string &api_path, + std::uint64_t &file_size) const -> api_error override; - [[nodiscard]] auto get_item_meta(const std::string &api_path, - api_meta_map &meta) const - -> api_error override; + [[nodiscard]] auto + get_filesystem_item(const std::string &api_path, bool directory, + filesystem_item &fsi) const -> api_error override; - [[nodiscard]] auto get_item_meta(const std::string &api_path, - const std::string &key, - std::string &value) const - -> api_error override; + [[nodiscard]] auto get_filesystem_item_and_file( + const std::string &api_path, api_file &file, + filesystem_item &fsi) const -> api_error override; + + [[nodiscard]] auto get_filesystem_item_from_source_path( + const std::string &source_path, + filesystem_item &fsi) const -> api_error override; + + [[nodiscard]] auto + get_pinned_files() const -> std::vector override; + + [[nodiscard]] auto + get_item_meta(const std::string &api_path, + api_meta_map &meta) const -> api_error override; + + [[nodiscard]] auto + get_item_meta(const std::string &api_path, const std::string &key, + std::string &value) const -> api_error override; [[nodiscard]] auto get_total_drive_space() const -> std::uint64_t override; @@ -159,11 +153,11 @@ public: [[nodiscard]] auto is_directory(const std::string &api_path, bool &exists) const -> api_error override; - [[nodiscard]] auto is_file(const std::string &api_path, bool &exists) const - -> api_error override; + [[nodiscard]] auto is_file(const std::string &api_path, + bool &exists) const -> api_error override; - [[nodiscard]] auto is_file_writeable(const std::string &api_path) const - -> bool override; + [[nodiscard]] auto + is_file_writeable(const std::string &api_path) const -> bool override; [[nodiscard]] auto is_online() const -> bool override; @@ -173,44 +167,42 @@ public: return false; } - [[nodiscard]] auto read_file_bytes(const std::string &api_path, - std::size_t size, std::uint64_t offset, - data_buffer &data, - stop_type &stop_requested) - -> api_error override; + [[nodiscard]] auto + read_file_bytes(const std::string &api_path, std::size_t size, + std::uint64_t offset, data_buffer &data, + stop_type &stop_requested) -> api_error override; - [[nodiscard]] auto remove_directory(const std::string & /*api_path*/) - -> api_error override { + [[nodiscard]] auto + remove_directory(const std::string & /*api_path*/) -> api_error override { return api_error::not_implemented; } - [[nodiscard]] auto remove_file(const std::string & /*api_path*/) - -> api_error override { + [[nodiscard]] auto + remove_file(const std::string & /*api_path*/) -> api_error override { return api_error::not_implemented; } - [[nodiscard]] auto remove_item_meta(const std::string & /*api_path*/, - const std::string & /*key*/) - -> api_error override { + [[nodiscard]] auto + remove_item_meta(const std::string & /*api_path*/, + const std::string & /*key*/) -> api_error override { return api_error::success; } - [[nodiscard]] auto rename_file(const std::string & /*from_api_path*/, - const std::string & /*to_api_path*/) - -> api_error override { + [[nodiscard]] auto + rename_file(const std::string & /*from_api_path*/, + const std::string & /*to_api_path*/) -> api_error override { return api_error::not_implemented; } - [[nodiscard]] auto set_item_meta(const std::string & /*api_path*/, - const std::string & /*key*/, - const std::string & /*value*/) - -> api_error override { + [[nodiscard]] auto + set_item_meta(const std::string & /*api_path*/, const std::string & /*key*/, + const std::string & /*value*/) -> api_error override { return api_error::success; } - [[nodiscard]] auto set_item_meta(const std::string & /*api_path*/, - const api_meta_map & /*meta*/) - -> api_error override { + [[nodiscard]] auto + set_item_meta(const std::string & /*api_path*/, + const api_meta_map & /*meta*/) -> api_error override { return api_error::success; } @@ -219,10 +211,10 @@ public: void stop() override; - [[nodiscard]] auto upload_file(const std::string & /*api_path*/, - const std::string & /*source_path*/, - stop_type & /*stop_requested*/) - -> api_error override { + [[nodiscard]] auto + upload_file(const std::string & /*api_path*/, + const std::string & /*source_path*/, + stop_type & /*stop_requested*/) -> api_error override { return api_error::not_implemented; } }; diff --git a/repertory/librepertory/include/utils/polling.hpp b/repertory/librepertory/include/utils/polling.hpp index 83127d63..a14b970c 100644 --- a/repertory/librepertory/include/utils/polling.hpp +++ b/repertory/librepertory/include/utils/polling.hpp @@ -37,7 +37,7 @@ public: struct polling_item { std::string name; frequency freq; - std::function action; + std::function action; }; public: @@ -58,7 +58,7 @@ public: static auto instance() -> polling & { return instance_; } private: - app_config *config_ = nullptr; + app_config *config_{nullptr}; std::unique_ptr high_frequency_thread_; std::unordered_map items_; std::unique_ptr low_frequency_thread_; @@ -66,7 +66,7 @@ private: std::condition_variable notify_; std::unique_ptr second_frequency_thread_; std::mutex start_stop_mutex_; - stop_type stop_requested_ = false; + stop_type stop_requested_{false}; private: void frequency_thread(std::function get_frequency_seconds, diff --git a/repertory/librepertory/src/file_manager/file_manager.cpp b/repertory/librepertory/src/file_manager/file_manager.cpp index dc6d1c5b..a6de367a 100644 --- a/repertory/librepertory/src/file_manager/file_manager.cpp +++ b/repertory/librepertory/src/file_manager/file_manager.cpp @@ -43,8 +43,8 @@ #include "utils/time.hpp" namespace { -[[nodiscard]] auto create_resume_entry(const repertory::i_open_file &file) - -> json { +[[nodiscard]] auto +create_resume_entry(const repertory::i_open_file &file) -> json { return { {"chunk_size", file.get_chunk_size()}, {"path", file.get_api_path()}, @@ -447,11 +447,10 @@ auto file_manager::open(const std::string &api_path, bool directory, return open(api_path, directory, ofd, handle, file, nullptr); } -auto file_manager::open(const std::string &api_path, bool directory, - const open_file_data &ofd, std::uint64_t &handle, - std::shared_ptr &file, - std::shared_ptr closeable_file) - -> api_error { +auto file_manager::open( + const std::string &api_path, bool directory, const open_file_data &ofd, + std::uint64_t &handle, std::shared_ptr &file, + std::shared_ptr closeable_file) -> api_error { const auto create_and_add_handle = [&](std::shared_ptr cur_file) { handle = get_next_handle(); @@ -699,8 +698,8 @@ auto file_manager::rename_directory(const std::string &from_api_path, } auto file_manager::rename_file(const std::string &from_api_path, - const std::string &to_api_path, bool overwrite) - -> api_error { + const std::string &to_api_path, + bool overwrite) -> api_error { if (not provider_.is_rename_supported()) { return api_error::not_implemented; } @@ -780,9 +779,11 @@ void file_manager::start() { stop_requested_ = false; - polling::instance().set_callback( - {"timed_out_close", polling::frequency::second, - [this]() { this->close_timed_out_files(); }}); + polling::instance().set_callback({"timed_out_close", + polling::frequency::second, + [this](auto && /* stop_requested */) { + this->close_timed_out_files(); + }}); if (provider_.is_read_only()) { stop_requested_ = false; diff --git a/repertory/librepertory/src/providers/base_provider.cpp b/repertory/librepertory/src/providers/base_provider.cpp index f28e29e7..5e01d052 100644 --- a/repertory/librepertory/src/providers/base_provider.cpp +++ b/repertory/librepertory/src/providers/base_provider.cpp @@ -413,14 +413,14 @@ auto base_provider::is_file_writeable(const std::string &api_path) const return not exists; } -void base_provider::remove_deleted_files() { +void base_provider::remove_deleted_files(const stop_type &stop_requested) { REPERTORY_USES_FUNCTION_NAME(); if (not is_read_only()) { auto source_list = utils::file::directory{config_.get_cache_directory()}.get_files(); for (auto &&source_file : source_list) { - if (stop_requested_) { + if (stop_requested) { return; } @@ -466,7 +466,7 @@ void base_provider::remove_deleted_files() { api_file_list list{}; std::string marker; auto res{api_error::more_data}; - while (not stop_requested_ && res == api_error::more_data) { + while (not stop_requested && res == api_error::more_data) { res = get_file_list(list, marker); if (res != api_error::success && res != api_error::more_data) { utils::error::raise_error(function_name, res, @@ -478,7 +478,7 @@ void base_provider::remove_deleted_files() { std::vector removed_list{}; for (auto &&api_path : db3_->get_api_path_list()) { - if (stop_requested_) { + if (stop_requested) { return; } @@ -516,7 +516,7 @@ void base_provider::remove_deleted_files() { utils::path::combine(config_.get_data_directory(), {"orphaned"}); for (auto &&item : removed_list) { - if (stop_requested_) { + if (stop_requested) { return; } @@ -569,7 +569,7 @@ void base_provider::remove_deleted_files() { } for (auto &&item : removed_list) { - if (stop_requested_) { + if (stop_requested) { return; } @@ -678,8 +678,6 @@ auto base_provider::set_item_meta(const std::string &api_path, auto base_provider::start(api_item_added_callback api_item_added, i_file_manager *mgr) -> bool { - stop_requested_ = false; - api_item_added_ = api_item_added; fm_ = mgr; @@ -714,13 +712,14 @@ auto base_provider::start(api_item_added_callback api_item_added, return false; } - polling::instance().set_callback({"check_deleted", polling::frequency::low, - [this]() { remove_deleted_files(); }}); + polling::instance().set_callback( + {"check_deleted", polling::frequency::low, [this](auto &&stop_requested) { + remove_deleted_files(stop_requested); + }}); return true; } void base_provider::stop() { - stop_requested_ = true; polling::instance().remove_callback("check_deleted"); db3_.reset(); } diff --git a/repertory/librepertory/src/providers/encrypt/encrypt_provider.cpp b/repertory/librepertory/src/providers/encrypt/encrypt_provider.cpp index d6271fa2..9de5cefd 100644 --- a/repertory/librepertory/src/providers/encrypt/encrypt_provider.cpp +++ b/repertory/librepertory/src/providers/encrypt/encrypt_provider.cpp @@ -69,10 +69,9 @@ const std::map sql_create_tables = { namespace repertory { encrypt_provider::encrypt_provider(app_config &config) : config_(config) {} -auto encrypt_provider::create_api_file(const std::string &api_path, - bool directory, - const std::string &source_path) - -> api_file { +auto encrypt_provider::create_api_file( + const std::string &api_path, bool directory, + const std::string &source_path) -> api_file { auto times = utils::file::get_times(source_path); if (not times.has_value()) { throw std::runtime_error("failed to get file times"); @@ -98,10 +97,10 @@ auto encrypt_provider::create_api_file(const std::string &api_path, void encrypt_provider::create_item_meta(api_meta_map &meta, bool directory, const api_file &file) { #if defined(_WIN32) - struct _stat64 buf{}; + struct _stat64 buf {}; _stat64(file.source_path.c_str(), &buf); #else // !defined(_WIN32) - struct stat buf{}; + struct stat buf {}; stat(file.source_path.c_str(), &buf); #endif // defined(_WIN32) @@ -188,9 +187,8 @@ auto encrypt_provider::do_fs_operation( return callback(cfg, source_path); } -auto encrypt_provider::get_api_path_from_source(const std::string &source_path, - std::string &api_path) const - -> api_error { +auto encrypt_provider::get_api_path_from_source( + const std::string &source_path, std::string &api_path) const -> api_error { REPERTORY_USES_FUNCTION_NAME(); try { @@ -255,9 +253,8 @@ auto encrypt_provider::get_directory_item_count( return count; } -auto encrypt_provider::get_directory_items(const std::string &api_path, - directory_item_list &list) const - -> api_error { +auto encrypt_provider::get_directory_items( + const std::string &api_path, directory_item_list &list) const -> api_error { REPERTORY_USES_FUNCTION_NAME(); return do_fs_operation( @@ -405,9 +402,8 @@ auto encrypt_provider::get_file(const std::string &api_path, return api_error::error; } -auto encrypt_provider::get_file_list(api_file_list &list, - std::string & /* marker */) const - -> api_error { +auto encrypt_provider::get_file_list( + api_file_list &list, std::string & /* marker */) const -> api_error { REPERTORY_USES_FUNCTION_NAME(); const auto cfg = config_.get_encrypt_config(); @@ -430,9 +426,8 @@ auto encrypt_provider::get_file_list(api_file_list &list, return api_error::error; } -auto encrypt_provider::get_file_size(const std::string &api_path, - std::uint64_t &file_size) const - -> api_error { +auto encrypt_provider::get_file_size( + const std::string &api_path, std::uint64_t &file_size) const -> api_error { REPERTORY_USES_FUNCTION_NAME(); try { @@ -458,10 +453,9 @@ auto encrypt_provider::get_file_size(const std::string &api_path, return api_error::error; } -auto encrypt_provider::get_filesystem_item(const std::string &api_path, - bool directory, - filesystem_item &fsi) const - -> api_error { +auto encrypt_provider::get_filesystem_item( + const std::string &api_path, bool directory, + filesystem_item &fsi) const -> api_error { auto result = utils::db::sqlite::db_select{*db_, source_table} .column("source_path") .where("api_path") @@ -537,10 +531,9 @@ auto encrypt_provider::get_filesystem_item_from_source_path( return get_filesystem_item(api_path, false, fsi); } -auto encrypt_provider::get_filesystem_item_and_file(const std::string &api_path, - api_file &file, - filesystem_item &fsi) const - -> api_error { +auto encrypt_provider::get_filesystem_item_and_file( + const std::string &api_path, api_file &file, + filesystem_item &fsi) const -> api_error { REPERTORY_USES_FUNCTION_NAME(); try { @@ -667,8 +660,8 @@ auto encrypt_provider::is_directory(const std::string &api_path, return api_error::success; } -auto encrypt_provider::is_file(const std::string &api_path, bool &exists) const - -> api_error { +auto encrypt_provider::is_file(const std::string &api_path, + bool &exists) const -> api_error { auto result = utils::db::sqlite::db_select{*db_, source_table} .column("source_path") .where("api_path") @@ -954,11 +947,11 @@ auto encrypt_provider::read_file_bytes(const std::string &api_path, return api_error::success; } -void encrypt_provider::remove_deleted_files() { +void encrypt_provider::remove_deleted_files(const stop_type &stop_requested) { struct removed_item { - std::string api_path{}; + std::string api_path; bool directory{}; - std::string source_path{}; + std::string source_path; }; std::vector removed_list{}; @@ -966,6 +959,10 @@ void encrypt_provider::remove_deleted_files() { auto result = utils::db::sqlite::db_select{*db_, source_table}.go(); while (result.has_row()) { + if (stop_requested) { + return; + } + std::optional row; if (result.get_row(row) && row.has_value()) { row_list.push_back(row.value()); @@ -973,6 +970,10 @@ void encrypt_provider::remove_deleted_files() { } for (auto &&row : row_list) { + if (stop_requested) { + return; + } + auto source_path = row.get_column("source_path").get_value(); if (not utils::path::exists(source_path)) { auto api_path = row.get_column("api_path").get_value(); @@ -987,37 +988,49 @@ void encrypt_provider::remove_deleted_files() { } for (auto &&item : removed_list) { - if (not item.directory) { - auto del_res = utils::db::sqlite::db_delete{*db_, source_table} - .where("api_path") - .equals(item.api_path) - .go(); - // TODO handle error - del_res = utils::db::sqlite::db_delete{*db_, file_table} - .where("source_path") - .equals(item.source_path) - .go(); - // TODO handle error - event_system::instance().raise(item.api_path, - item.source_path); + if (stop_requested) { + return; } + + if (item.directory) { + continue; + } + + auto del_res = utils::db::sqlite::db_delete{*db_, source_table} + .where("api_path") + .equals(item.api_path) + .go(); + // TODO handle error + del_res = utils::db::sqlite::db_delete{*db_, file_table} + .where("source_path") + .equals(item.source_path) + .go(); + // TODO handle error + event_system::instance().raise(item.api_path, + item.source_path); } for (auto &&item : removed_list) { - if (item.directory) { - auto del_res = utils::db::sqlite::db_delete{*db_, source_table} - .where("api_path") - .equals(item.api_path) - .go(); - // TODO handle error - del_res = utils::db::sqlite::db_delete{*db_, directory_table} - .where("source_path") - .equals(item.source_path) - .go(); - // TODO handle error - event_system::instance().raise( - item.api_path, item.source_path); + if (stop_requested) { + return; } + + if (not item.directory) { + continue; + } + auto del_res = utils::db::sqlite::db_delete{*db_, source_table} + + .where("api_path") + .equals(item.api_path) + .go(); + // TODO handle error + del_res = utils::db::sqlite::db_delete{*db_, directory_table} + .where("source_path") + .equals(item.source_path) + .go(); + // TODO handle error + event_system::instance().raise( + item.api_path, item.source_path); } } @@ -1066,8 +1079,10 @@ auto encrypt_provider::start(api_item_added_callback /*api_item_added*/, // TODO error handling } - polling::instance().set_callback({"check_deleted", polling::frequency::low, - [this]() { remove_deleted_files(); }}); + polling::instance().set_callback( + {"check_deleted", polling::frequency::low, [this](auto &&stop_requested) { + remove_deleted_files(stop_requested); + }}); event_system::instance().raise("encrypt_provider"); return true; diff --git a/repertory/librepertory/src/utils/polling.cpp b/repertory/librepertory/src/utils/polling.cpp index 08afa131..a7bb73d5 100644 --- a/repertory/librepertory/src/utils/polling.cpp +++ b/repertory/librepertory/src/utils/polling.cpp @@ -39,7 +39,7 @@ void polling::frequency_thread( freq != frequency::second) { event_system::instance().raise(item.first); } - item.second.action(); + item.second.action(stop_requested_); if (config_->get_event_level() == event_level::trace || freq != frequency::second) { event_system::instance().raise(item.first);