winfsp unit tests and fixes

repertory/librepertory/include/file_manager/i_open_file.hpp

@@ -106,7 +106,9 @@ public:
 
   [[nodiscard]] virtual auto is_write_supported() const -> bool = 0;
 
-  virtual void remove(std::uint64_t handle, bool noQueue = false) = 0;
+  virtual void remove(std::uint64_t handle) = 0;
+
+  virtual void remove_all() = 0;
 };
 } // namespace repertory
 

repertory/librepertory/include/file_manager/open_file.hpp

@@ -107,7 +107,9 @@ public:
                                       native_operation_callback callback)
       -> api_error override;
 
-  void remove(std::uint64_t handle, bool noQueue = false) override;
+  void remove(std::uint64_t handle) override;
+
+  void remove_all() override;
 
   [[nodiscard]] auto read(std::size_t read_size, std::uint64_t read_offset,
                           data_buffer &data) -> api_error override;

repertory/librepertory/include/file_manager/open_file_base.hpp

@@ -185,7 +185,9 @@ public:
 
   [[nodiscard]] auto is_modified() const -> bool override;
 
-  void remove(std::uint64_t handle, bool noQueue = false) override;
+  void remove(std::uint64_t handle) override;
+
+  void remove_all() override;
 
   void set_api_path(const std::string &api_path) override;
 };

repertory/librepertory/src/file_manager/file_manager.cpp

@@ -128,9 +128,11 @@ file_manager::~file_manager() {
 void file_manager::close(std::uint64_t handle) {
   recur_mutex_lock file_lock(open_file_mtx_);
   auto closeable_file = get_open_file_by_handle(handle);
-  if (closeable_file) {
+  if (not closeable_file) {
-    closeable_file->remove(handle, false);
+    return;
   }
+
+  closeable_file->remove(handle);
 }
 
 void file_manager::close_all(const std::string &api_path) {
@@ -143,25 +145,9 @@ void file_manager::close_all(const std::string &api_path) {
     return;
   }
 
-  remove_upload(api_path, true);
-
   auto closeable_file = file_iter->second;
+  closeable_file->remove_all();
   open_file_lookup_.erase(api_path);
-
-  auto handles = closeable_file->get_handles();
-  for (auto &&handle : handles) {
-    closeable_file->remove(handle, true);
-  }
-
-  auto result = utils::db::sqlite::db_delete{*db_, resume_table}
-                    .where("api_path")
-                    .equals(api_path)
-                    .go();
-  if (not result.ok()) {
-    utils::error::raise_api_path_error(function_name, api_path,
-                                       api_error::error,
-                                       "failed to remove from resume table");
-  }
 }
 
 void file_manager::close_timed_out_files() {

repertory/librepertory/src/file_manager/open_file.cpp

@@ -117,8 +117,8 @@ void open_file::download_chunk(std::size_t chunk, bool skip_active,
       return;
     }
 
-    const auto data_offset = chunk * chunk_size_;
+    auto data_offset = chunk * chunk_size_;
-    const auto data_size =
+    auto data_size =
         (chunk == read_state_.size() - 1U) ? last_chunk_size_ : chunk_size_;
     if (active_downloads_.empty() && (read_state_.count() == 0U)) {
       event_system::instance().raise<download_begin>(fsi_.api_path,
@@ -258,9 +258,9 @@ auto open_file::native_operation(
   }
   file_lock.unlock();
 
-  const auto is_empty_file = new_file_size == 0U;
+  auto is_empty_file = new_file_size == 0U;
-  const auto last_chunk =
+  auto last_chunk = is_empty_file
-      is_empty_file ? std::size_t(0U)
+                        ? std::size_t(0U)
                         : static_cast<std::size_t>(utils::divide_with_ceiling(
                               new_file_size, chunk_size_)) -
                               1U;
@@ -277,7 +277,7 @@ auto open_file::native_operation(
     file_lock.lock();
   }
 
-  const auto original_file_size = get_file_size();
+  auto original_file_size = get_file_size();
 
   auto res = do_io([&]() -> api_error { return callback(nf_->get_handle()); });
   if (res != api_error::success) {
@@ -319,7 +319,7 @@ auto open_file::native_operation(
     set_modified();
 
     fsi_.size = new_file_size;
-    const auto now = std::to_string(utils::time::get_time_now());
+    auto now = std::to_string(utils::time::get_time_now());
     res = provider_.set_item_meta(
         fsi_.api_path, {
                            {META_CHANGED, now},
@@ -372,8 +372,8 @@ auto open_file::read(std::size_t read_size, std::uint64_t read_offset,
   }
   file_lock.unlock();
 
-  const auto start_chunk = static_cast<std::size_t>(read_offset / chunk_size_);
+  auto start_chunk = static_cast<std::size_t>(read_offset / chunk_size_);
-  const auto end_chunk =
+  auto end_chunk =
       static_cast<std::size_t>((read_size + read_offset) / chunk_size_);
 
   update_background_reader(start_chunk);
@@ -388,10 +388,10 @@ auto open_file::read(std::size_t read_size, std::uint64_t read_offset,
                                                : get_api_error();
 }
 
-void open_file::remove(std::uint64_t handle, bool noQueue) {
+void open_file::remove(std::uint64_t handle) {
   recur_mutex_lock file_lock(file_mtx_);
-  open_file_base::remove(handle, noQueue);
+  open_file_base::remove(handle);
-  if (not noQueue && modified_ && read_state_.all() &&
+  if (modified_ && read_state_.all() &&
       (get_api_error() == api_error::success)) {
     mgr_.queue_upload(*this);
     modified_ = false;
@@ -402,6 +402,18 @@ void open_file::remove(std::uint64_t handle, bool noQueue) {
   }
 }
 
+void open_file::remove_all() {
+  recur_mutex_lock file_lock(file_mtx_);
+  open_file_base::remove_all();
+
+  modified_ = false;
+  removed_ = true;
+
+  set_api_error(api_error::success);
+
+  mgr_.remove_upload(get_api_path());
+}
+
 auto open_file::resize(std::uint64_t new_file_size) -> api_error {
   if (fsi_.directory) {
     return api_error::invalid_operation;
@@ -417,7 +429,10 @@ auto open_file::resize(std::uint64_t new_file_size) -> api_error {
 auto open_file::close() -> bool {
   REPERTORY_USES_FUNCTION_NAME();
 
-  if (not fsi_.directory && not stop_requested_) {
+  if (fsi_.directory || stop_requested_) {
+    return false;
+  }
+
   stop_requested_ = true;
 
   unique_mutex_lock reader_lock(io_thread_mtx_);
@@ -429,29 +444,36 @@ auto open_file::close() -> bool {
     reader_thread_.reset();
   }
 
-    if (open_file_base::close()) {
+  if (not open_file_base::close()) {
-      {
+    return true;
-        const auto err = get_api_error();
+  }
-        if (err == api_error::success ||
+
-            err == api_error::download_incomplete ||
+  auto err = get_api_error();
+  if (err == api_error::success || err == api_error::download_incomplete ||
       err == api_error::download_stopped) {
     if (modified_ && not read_state_.all()) {
       set_api_error(api_error::download_incomplete);
-          } else if (not modified_ && (fsi_.size > 0U) &&
+    } else if (not modified_ && (fsi_.size > 0U) && not read_state_.all()) {
-                     not read_state_.all()) {
       set_api_error(api_error::download_stopped);
     }
   }
-      }
 
   nf_->close();
 
   if (modified_ && (get_api_error() == api_error::success)) {
     mgr_.queue_upload(*this);
-      } else if (modified_ &&
+    return true;
-                 (get_api_error() == api_error::download_incomplete)) {
+  }
+
+  if (modified_ && (get_api_error() == api_error::download_incomplete)) {
     mgr_.store_resume(*this);
-      } else if (get_api_error() != api_error::success) {
+    return true;
+  }
+
+  if (get_api_error() == api_error::success) {
+    return true;
+  }
+
   mgr_.remove_resume(get_api_path(), get_source_path());
   if (not utils::file::file(fsi_.source_path).remove()) {
     utils::error::raise_api_path_error(
@@ -462,20 +484,15 @@ auto open_file::close() -> bool {
   auto parent = utils::path::get_parent_path(fsi_.source_path);
   fsi_.source_path =
       utils::path::combine(parent, {utils::create_uuid_string()});
-        const auto res = provider_.set_item_meta(fsi_.api_path, META_SOURCE,
+  auto res =
-                                                 fsi_.source_path);
+      provider_.set_item_meta(fsi_.api_path, META_SOURCE, fsi_.source_path);
   if (res != api_error::success) {
     utils::error::raise_api_path_error(function_name, get_api_path(),
                                        fsi_.source_path, res,
                                        "failed to set file meta");
   }
-      }
-    }
 
   return true;
-  }
-
-  return false;
 }
 
 void open_file::set_modified() {
@@ -544,8 +561,8 @@ auto open_file::write(std::uint64_t write_offset, const data_buffer &data,
   }
   write_lock.unlock();
 
-  const auto start_chunk = static_cast<std::size_t>(write_offset / chunk_size_);
+  auto start_chunk = static_cast<std::size_t>(write_offset / chunk_size_);
-  const auto end_chunk =
+  auto end_chunk =
       static_cast<std::size_t>((write_offset + data.size()) / chunk_size_);
 
   update_background_reader(start_chunk);
@@ -576,7 +593,7 @@ auto open_file::write(std::uint64_t write_offset, const data_buffer &data,
     return set_api_error(res);
   }
 
-  const auto now = std::to_string(utils::time::get_time_now());
+  auto now = std::to_string(utils::time::get_time_now());
   res = provider_.set_item_meta(fsi_.api_path, {
                                                    {META_CHANGED, now},
                                                    {META_MODIFIED, now},

repertory/librepertory/src/file_manager/open_file_base.cpp

@@ -235,7 +235,7 @@ auto open_file_base::is_modified() const -> bool {
   return modified_;
 }
 
-void open_file_base::remove(std::uint64_t handle, bool /* noQueue */) {
+void open_file_base::remove(std::uint64_t handle) {
   recur_mutex_lock file_lock(file_mtx_);
   open_data_.erase(handle);
   event_system::instance().raise<filesystem_item_handle_closed>(
@@ -246,6 +246,20 @@ void open_file_base::remove(std::uint64_t handle, bool /* noQueue */) {
   }
 }
 
+void open_file_base::remove_all() {
+  recur_mutex_lock file_lock(file_mtx_);
+  auto open_data = open_data_;
+  open_data_.clear();
+
+  for (auto &&data : open_data) {
+    event_system::instance().raise<filesystem_item_handle_closed>(
+        fsi_.api_path, data.first, fsi_.source_path, fsi_.directory, modified_);
+  }
+
+  event_system::instance().raise<filesystem_item_closed>(
+      fsi_.api_path, fsi_.source_path, fsi_.directory, modified_);
+}
+
 void open_file_base::reset_timeout() {
   last_access_ = std::chrono::system_clock::now();
 }

repertory/repertory_test/include/mocks/mock_open_file.hpp

@@ -97,7 +97,9 @@ public:
 
   MOCK_METHOD(bool, is_write_supported, (), (const, override));
 
-  MOCK_METHOD(void, remove, (std::uint64_t handle, bool noQueue), (override));
+  MOCK_METHOD(void, remove, (std::uint64_t handle), (override));
+
+  MOCK_METHOD(void, remove_all, (), (override));
 };
 } // namespace repertory