unit tests and fixes

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;

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<std::uint64_t, open_file_data> 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<std::uint64_t> = 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;

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;
 

repertory/librepertory/include/file_manager/open_file_base.hpp

@@ -131,8 +131,6 @@ private:
 protected:
   [[nodiscard]] auto do_io(std::function<api_error()> 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<std::uint64_t, open_file_data> open_data) override;
 };
 } // namespace repertory
 

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;

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,

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_) {

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<std::uint64_t, open_file_data> 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_) {

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<std::uint64_t>, get_handles, (), (const, override));
+
+  MOCK_METHOD((std::map<std::uint64_t, open_file_data> &), get_open_data, (),
+              (override));
+
+  MOCK_METHOD((const std::map<std::uint64_t, open_file_data> &), 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<std::uint64_t>, get_handles, (), (const, override));
-
-  MOCK_METHOD((std::map<std::uint64_t, open_file_data> &), get_open_data, (),
-              (override));
-
-  MOCK_METHOD((const std::map<std::uint64_t, open_file_data> &), 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<std::uint64_t, open_file_data> open_data)),
-              (override));
 };
 } // namespace repertory
 

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<std::string>().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{};
+  event_capture capture({
-  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({
       "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,
+      .WillRepeatedly(
-                              filesystem_item &fsi) -> api_error {
+          [](const std::string &api_path, bool directory,
-        return api_error::error;
+             filesystem_item &fsi) -> api_error { return api_error::error; });
-      });
 
   EXPECT_FALSE(mgr.evict_file("/test_open.txt"));
 }
@@ -822,7 +823,7 @@ 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,
+      .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);