BlockStorage/repertory
1
0
Fork 0
Code Issues 7 Pull Requests Releases 6 Wiki Activity

[bug] Rename file is broken for files that are existing #19

Browse Source
This commit is contained in:
Scott E. Graves 2024-09-28 17:28:47 -05:00
parent 9aecec2d96
commit 439f9cce3d
5 changed files with 87 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
repertory/librepertory/include/file_manager/file_manager.hpp
View File

@ -409,10 +409,10 @@ public:
i_provider &provider_; i_provider &provider_;
private: private:
bool cancelled_ = false; bool cancelled_{false};
api_error error_ = api_error::success; api_error error_{api_error::success};
std::unique_ptr<std::thread> thread_; std::unique_ptr<std::thread> thread_;
stop_type stop_requested_ = false; stop_type stop_requested_{false};
private: private:
void upload_thread(); void upload_thread();

3
repertory/librepertory/include/types/repertory.hpp
View File

@ -86,7 +86,6 @@ enum class api_error {
out_of_memory, out_of_memory,
permission_denied, permission_denied,
upload_failed, upload_failed,
upload_stopped,
xattr_buffer_small, xattr_buffer_small,
xattr_exists, xattr_exists,
xattr_not_found, xattr_not_found,
@ -94,7 +93,7 @@ enum class api_error {
ERROR_COUNT ERROR_COUNT
}; };
[[nodiscard]] auto api_error_from_string(std::string_view s) -> api_error; [[nodiscard]] auto api_error_from_string(std::string_view str) -> api_error;
[[nodiscard]] auto [[nodiscard]] auto
api_error_to_string(const api_error &error) -> const std::string &; api_error_to_string(const api_error &error) -> const std::string &;

32
repertory/librepertory/src/file_manager/file_manager.cpp
View File

@ -416,8 +416,6 @@ auto file_manager::handle_file_rename(const std::string &from_api_path,
return ret; return ret;
} }
swap_renamed_items(from_api_path, to_api_path);
if (should_upload) { if (should_upload) {
queue_upload(to_api_path, source_path, false); queue_upload(to_api_path, source_path, false);
} }
@ -603,16 +601,22 @@ void file_manager::remove_upload(const std::string &api_path, bool no_lock) {
lock = std::make_unique<mutex_lock>(upload_mtx_); lock = std::make_unique<mutex_lock>(upload_mtx_);
} }
auto result = db::db_select{*db_.get(), upload_active_table} auto result = db::db_select{*db_.get(), upload_table}
.delete_query() .delete_query()
.where("api_path") .where("api_path")
.equals(api_path) .equals(api_path)
.go(); .go();
auto file_iter = upload_lookup_.find(api_path); result = db::db_select{*db_.get(), upload_active_table}
if (file_iter != upload_lookup_.end()) { .delete_query()
file_iter->second->cancel(); .where("api_path")
.equals(api_path)
.go();
if (upload_lookup_.find(api_path) != upload_lookup_.end()) {
auto ptr = std::move(upload_lookup_.at(api_path));
upload_lookup_.erase(api_path); upload_lookup_.erase(api_path);
ptr->cancel();
} }
if (result.ok()) { if (result.ok()) {
@ -974,8 +978,7 @@ void file_manager::upload_completed(const file_upload_completed &evt) {
if (not utils::string::to_bool(evt.get_cancelled().get<std::string>())) { if (not utils::string::to_bool(evt.get_cancelled().get<std::string>())) {
auto err = api_error_from_string(evt.get_result().get<std::string>()); auto err = api_error_from_string(evt.get_result().get<std::string>());
switch (err) { if (err == api_error::success) {
case api_error::success: {
auto result = db::db_select{*db_.get(), upload_active_table} auto result = db::db_select{*db_.get(), upload_active_table}
.delete_query() .delete_query()
.where("api_path") .where("api_path")
@ -987,16 +990,7 @@ void file_manager::upload_completed(const file_upload_completed &evt) {
evt.get_source().get<std::string>(), evt.get_source().get<std::string>(),
"failed to remove from to upload_active table"); "failed to remove from to upload_active table");
} }
} break; } else {
case api_error::upload_stopped: {
event_system::instance().raise<file_upload_retry>(evt.get_api_path(),
evt.get_source(), err);
queue_upload(evt.get_api_path(), evt.get_source(), true);
upload_notify_.wait_for(upload_lock, 5s);
} break;
default: {
bool exists{}; bool exists{};
auto res = provider_.is_file(evt.get_api_path(), exists); auto res = provider_.is_file(evt.get_api_path(), exists);
if ((res == api_error::success && not exists) || if ((res == api_error::success && not exists) ||
@ -1012,7 +1006,6 @@ void file_manager::upload_completed(const file_upload_completed &evt) {
evt.get_source(), err); evt.get_source(), err);
queue_upload(evt.get_api_path(), evt.get_source(), true); queue_upload(evt.get_api_path(), evt.get_source(), true);
upload_notify_.wait_for(upload_lock, 5s); upload_notify_.wait_for(upload_lock, 5s);
} break;
} }
upload_lookup_.erase(evt.get_api_path()); upload_lookup_.erase(evt.get_api_path());
@ -1051,7 +1044,6 @@ void file_manager::upload_handler() {
switch (res) { switch (res) {
case api_error::item_not_found: { case api_error::item_not_found: {
should_wait = false; should_wait = false;
//
event_system::instance().raise<file_upload_not_found>(api_path, event_system::instance().raise<file_upload_not_found>(api_path,
source_path); source_path);
remove_upload(api_path, true); remove_upload(api_path, true);

13
repertory/librepertory/src/types/repertory.cpp
View File

@ -91,24 +91,23 @@ static const std::unordered_map<api_error, std::string> LOOKUP = {
{api_error::out_of_memory, "out_of_memory"}, {api_error::out_of_memory, "out_of_memory"},
{api_error::permission_denied, "permission_denied"}, {api_error::permission_denied, "permission_denied"},
{api_error::upload_failed, "upload_failed"}, {api_error::upload_failed, "upload_failed"},
{api_error::upload_stopped, "upload_stopped"},
{api_error::xattr_buffer_small, "xattr_buffer_small"}, {api_error::xattr_buffer_small, "xattr_buffer_small"},
{api_error::xattr_exists, "xattr_exists"}, {api_error::xattr_exists, "xattr_exists"},
{api_error::xattr_not_found, "xattr_not_found"}, {api_error::xattr_not_found, "xattr_not_found"},
{api_error::xattr_too_big, "xattr_too_big"}, {api_error::xattr_too_big, "xattr_too_big"},
}; };
auto api_error_from_string(std::string_view s) -> api_error { auto api_error_from_string(std::string_view str) -> api_error {
if (LOOKUP.size() != static_cast<std::size_t>(api_error::ERROR_COUNT)) { if (LOOKUP.size() != static_cast<std::size_t>(api_error::ERROR_COUNT)) {
throw startup_exception("undefined api_error strings"); throw startup_exception("undefined api_error strings");
} }
const auto it = const auto iter = std::find_if(
std::find_if(LOOKUP.begin(), LOOKUP.end(), LOOKUP.begin(), LOOKUP.end(),
[&s](const std::pair<api_error, std::string> &kv) -> bool { [&str](const std::pair<api_error, std::string> &item) -> bool {
return kv.second == s; return item.second == str;
}); });
return it == LOOKUP.end() ? api_error::error : it->first; return iter == LOOKUP.end() ? api_error::error : iter->first;
} }
auto api_error_to_string(const api_error &error) -> const std::string & { auto api_error_to_string(const api_error &error) -> const std::string & {

123
repertory/repertory_test/src/file_manager_upload_test.cpp
View File

@ -23,49 +23,47 @@
#include "file_manager/file_manager.hpp" #include "file_manager/file_manager.hpp"
#include "mocks/mock_provider.hpp" #include "mocks/mock_provider.hpp"
#include "mocks/mock_upload_manager.hpp"
#include "utils/event_capture.hpp" #include "utils/event_capture.hpp"
#include "utils/path.hpp"
namespace repertory { namespace repertory {
static constexpr const std::size_t test_chunk_size = 1024u; static constexpr const std::size_t test_chunk_size{1024U};
TEST(upload, can_upload_a_valid_file) { TEST(upload, can_upload_a_valid_file) {
console_consumer c; console_consumer con;
event_system::instance().start(); event_system::instance().start();
const auto source_path = test::generate_test_file_name("upload_test"); const auto source_path = test::generate_test_file_name("upload_test");
mock_provider mp; mock_provider mock_prov;
EXPECT_CALL(mp, is_direct_only()).WillRepeatedly(Return(false)); EXPECT_CALL(mock_prov, is_direct_only()).WillRepeatedly(Return(false));
filesystem_item fsi; filesystem_item fsi;
fsi.api_path = "/test.txt"; fsi.api_path = "/test.txt";
fsi.size = test_chunk_size * 4u; fsi.size = test_chunk_size * 4U;
fsi.source_path = source_path; fsi.source_path = source_path;
event_consumer ec("file_upload_completed", [&fsi](const event &e) { event_consumer evt_com("file_upload_completed", [&fsi](const event &evt) {
const auto &ee = dynamic_cast<const file_upload_completed &>(e); const auto &comp_evt = dynamic_cast<const file_upload_completed &>(evt);
EXPECT_STREQ(fsi.api_path.c_str(), EXPECT_STREQ(fsi.api_path.c_str(),
ee.get_api_path().get<std::string>().c_str()); comp_evt.get_api_path().get<std::string>().c_str());
EXPECT_STREQ(fsi.source_path.c_str(), EXPECT_STREQ(fsi.source_path.c_str(),
ee.get_source().get<std::string>().c_str()); comp_evt.get_source().get<std::string>().c_str());
EXPECT_STREQ("success", ee.get_result().get<std::string>().c_str()); EXPECT_STREQ("success", comp_evt.get_result().get<std::string>().c_str());
EXPECT_STREQ("0", ee.get_cancelled().get<std::string>().c_str()); EXPECT_STREQ("0", comp_evt.get_cancelled().get<std::string>().c_str());
}); });
EXPECT_CALL(mp, upload_file(fsi.api_path, fsi.source_path, _)) EXPECT_CALL(mock_prov, upload_file(fsi.api_path, fsi.source_path, _))
.WillOnce([&fsi](const std::string &, const std::string &, .WillOnce([](const std::string &, const std::string &,
stop_type &stop_requested) -> api_error { stop_type &stop_requested) -> api_error {
EXPECT_FALSE(stop_requested); EXPECT_FALSE(stop_requested);
return api_error::success; return api_error::success;
}); });
file_manager::upload upload(fsi, mp); file_manager::upload upload(fsi, mock_prov);
event_capture e({"file_upload_completed"}); event_capture evt_cap({"file_upload_completed"});
e.wait_for_empty(); evt_cap.wait_for_empty();
EXPECT_EQ(api_error::success, upload.get_api_error()); EXPECT_EQ(api_error::success, upload.get_api_error());
EXPECT_FALSE(upload.is_cancelled()); EXPECT_FALSE(upload.is_cancelled());
@ -74,109 +72,110 @@ TEST(upload, can_upload_a_valid_file) {
} }
TEST(upload, can_cancel_upload) { TEST(upload, can_cancel_upload) {
console_consumer c; console_consumer con;
event_system::instance().start(); event_system::instance().start();
const auto source_path = test::generate_test_file_name("upload_test"); const auto source_path = test::generate_test_file_name("upload_test");
mock_provider mp; mock_provider mock_provider;
EXPECT_CALL(mp, is_direct_only()).WillRepeatedly(Return(false)); EXPECT_CALL(mock_provider, is_direct_only()).WillRepeatedly(Return(false));
filesystem_item fsi; filesystem_item fsi;
fsi.api_path = "/test.txt"; fsi.api_path = "/test.txt";
fsi.size = test_chunk_size * 4u; fsi.size = test_chunk_size * 4U;
fsi.source_path = source_path; fsi.source_path = source_path;
event_consumer ec("file_upload_completed", [&fsi](const event &e) { event_consumer evt_con("file_upload_completed", [&fsi](const event &evt) {
const auto &ee = dynamic_cast<const file_upload_completed &>(e); const auto &comp_evt = dynamic_cast<const file_upload_completed &>(evt);
EXPECT_STREQ(fsi.api_path.c_str(), EXPECT_STREQ(fsi.api_path.c_str(),
ee.get_api_path().get<std::string>().c_str()); comp_evt.get_api_path().get<std::string>().c_str());
EXPECT_STREQ(fsi.source_path.c_str(), EXPECT_STREQ(fsi.source_path.c_str(),
ee.get_source().get<std::string>().c_str()); comp_evt.get_source().get<std::string>().c_str());
EXPECT_STREQ("upload_stopped", ee.get_result().get<std::string>().c_str()); EXPECT_STREQ("comm_error",
EXPECT_STREQ("1", ee.get_cancelled().get<std::string>().c_str()); comp_evt.get_result().get<std::string>().c_str());
EXPECT_STREQ("1", comp_evt.get_cancelled().get<std::string>().c_str());
}); });
std::mutex mtx; std::mutex mtx;
std::condition_variable cv; std::condition_variable notify;
EXPECT_CALL(mp, upload_file(fsi.api_path, fsi.source_path, _)) EXPECT_CALL(mock_provider, upload_file(fsi.api_path, fsi.source_path, _))
.WillOnce([&cv, &fsi, &mtx](const std::string &, const std::string &, .WillOnce([&notify, &mtx](const std::string &, const std::string &,
stop_type &stop_requested) -> api_error { stop_type &stop_requested) -> api_error {
EXPECT_FALSE(stop_requested); EXPECT_FALSE(stop_requested);
unique_mutex_lock l(mtx); unique_mutex_lock lock(mtx);
cv.notify_one(); notify.notify_one();
l.unlock(); lock.unlock();
l.lock(); lock.lock();
cv.wait(l); notify.wait(lock);
l.unlock(); lock.unlock();
EXPECT_TRUE(stop_requested); EXPECT_TRUE(stop_requested);
return api_error::upload_stopped; return api_error::comm_error;
}); });
unique_mutex_lock l(mtx); unique_mutex_lock lock(mtx);
file_manager::upload upload(fsi, mp); file_manager::upload upload(fsi, mock_provider);
cv.wait(l); notify.wait(lock);
upload.cancel(); upload.cancel();
cv.notify_one(); notify.notify_one();
l.unlock(); lock.unlock();
event_capture e({"file_upload_completed"}); event_capture evt_cap({"file_upload_completed"});
e.wait_for_empty(); evt_cap.wait_for_empty();
EXPECT_EQ(api_error::upload_stopped, upload.get_api_error()); EXPECT_EQ(api_error::comm_error, upload.get_api_error());
EXPECT_TRUE(upload.is_cancelled()); EXPECT_TRUE(upload.is_cancelled());
event_system::instance().stop(); event_system::instance().stop();
} }
TEST(upload, can_stop_upload) { TEST(upload, can_stop_upload) {
console_consumer c; console_consumer con;
event_system::instance().start(); event_system::instance().start();
const auto source_path = test::generate_test_file_name("upload_test"); const auto source_path = test::generate_test_file_name("upload_test");
mock_provider mp; mock_provider mock_provider;
EXPECT_CALL(mp, is_direct_only()).WillRepeatedly(Return(false)); EXPECT_CALL(mock_provider, is_direct_only()).WillRepeatedly(Return(false));
filesystem_item fsi; filesystem_item fsi;
fsi.api_path = "/test.txt"; fsi.api_path = "/test.txt";
fsi.size = test_chunk_size * 4u; fsi.size = test_chunk_size * 4U;
fsi.source_path = source_path; fsi.source_path = source_path;
event_consumer ec("file_upload_completed", [&fsi](const event &e) { event_consumer evt_con("file_upload_completed", [&fsi](const event &evt) {
const auto &ee = dynamic_cast<const file_upload_completed &>(e); const auto &evt_com = dynamic_cast<const file_upload_completed &>(evt);
EXPECT_STREQ(fsi.api_path.c_str(), EXPECT_STREQ(fsi.api_path.c_str(),
ee.get_api_path().get<std::string>().c_str()); evt_com.get_api_path().get<std::string>().c_str());
EXPECT_STREQ(fsi.source_path.c_str(), EXPECT_STREQ(fsi.source_path.c_str(),
ee.get_source().get<std::string>().c_str()); evt_com.get_source().get<std::string>().c_str());
EXPECT_STREQ("upload_stopped", ee.get_result().get<std::string>().c_str()); EXPECT_STREQ("comm_error", evt_com.get_result().get<std::string>().c_str());
EXPECT_STREQ("0", ee.get_cancelled().get<std::string>().c_str()); EXPECT_STREQ("0", evt_com.get_cancelled().get<std::string>().c_str());
}); });
EXPECT_CALL(mp, upload_file(fsi.api_path, fsi.source_path, _)) EXPECT_CALL(mock_provider, upload_file(fsi.api_path, fsi.source_path, _))
.WillOnce([&fsi](const std::string &, const std::string &, .WillOnce([](const std::string &, const std::string &,
stop_type &stop_requested) -> api_error { stop_type &stop_requested) -> api_error {
std::this_thread::sleep_for(3s); std::this_thread::sleep_for(3s);
EXPECT_TRUE(stop_requested); EXPECT_TRUE(stop_requested);
return api_error::upload_stopped; return api_error::comm_error;
}); });
event_capture e({"file_upload_completed"}); event_capture evt_cap({"file_upload_completed"});
{ file_manager::upload upload(fsi, mp); } { file_manager::upload upload(fsi, mock_provider); }
e.wait_for_empty(); evt_cap.wait_for_empty();
event_system::instance().stop(); event_system::instance().stop();
} }