From 17524a8c42b7a711fa5f8a9ae67a2198e8a4d2be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois-Simon=20Fauteux-Chapleau?=
<francois-simon.fauteux-chapleau@savoirfairelinux.com>
Date: Fri, 18 Apr 2025 17:16:15 -0400
Subject: [PATCH] conversation: fix partial file transfer issues
GitLab: #1118
Change-Id: I4de38d343022ec2712a7e649d73fbdb1da068eb3
---
src/jamidht/conversation.cpp | 33 +++++---
src/jamidht/conversation.h | 6 +-
src/jamidht/conversation_module.cpp | 6 +-
src/jamidht/conversation_module.h | 4 +-
test/unitTest/fileTransfer/fileTransfer.cpp | 94 ++++++++++++++++++++-
5 files changed, 118 insertions(+), 25 deletions(-)
diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index c37aa11d66..a41545e990 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -1943,12 +1943,7 @@ Conversation::sync(const std::string& member,
auto sthis = w.lock();
// For waiting request, downloadFile
for (const auto& wr : sthis->dataTransfer()->waitingRequests()) {
- auto path = fileutils::get_data_dir() / sthis->pimpl_->accountId_
- / "conversation_data" / sthis->id() / wr.fileId;
- auto start = fileutils::size(path);
- if (start < 0)
- start = 0;
- sthis->downloadFile(wr.interactionId, wr.fileId, wr.path, member, deviceId, start);
+ sthis->downloadFile(wr.interactionId, wr.fileId, wr.path, member, deviceId);
}
});
}
@@ -2139,9 +2134,7 @@ Conversation::downloadFile(const std::string& interactionId,
const std::string& fileId,
const std::string& path,
const std::string&,
- const std::string& deviceId,
- std::size_t start,
- std::size_t end)
+ const std::string& deviceId)
{
auto commit = getCommit(interactionId);
if (commit == std::nullopt || commit->at("type") != "application/data-transfer+json") {
@@ -2170,10 +2163,26 @@ Conversation::downloadFile(const std::string& interactionId,
interactionId,
sha3sum = sha3sum->second,
path,
- totalSize,
- start,
- end] {
+ totalSize] {
if (auto shared = w.lock()) {
+ std::filesystem::path filePath(path);
+ if (filePath.empty()) {
+ filePath = shared->dataTransfer()->path(fileId);
+ }
+
+ if (fileutils::size(filePath) == totalSize) {
+ if (fileutils::sha3File(filePath) == sha3sum) {
+ JAMI_WARNING("Ignoring request to download existing file: {}", filePath);
+ return;
+ }
+ }
+
+ auto tempFilePath = filePath + std::string(".tmp");
+ auto start = fileutils::size(tempFilePath);
+ if (start < 0)
+ start = 0;
+ size_t end = 0;
+
auto acc = shared->pimpl_->account_.lock();
if (!acc)
return;
diff --git a/src/jamidht/conversation.h b/src/jamidht/conversation.h
index f4458e7da4..62cf467f6c 100644
--- a/src/jamidht/conversation.h
+++ b/src/jamidht/conversation.h
@@ -426,17 +426,13 @@ public:
* @param path Destination
* @param member Member if we know from who to pull file
* @param deviceId Device if we know from who to pull file
- * @param start Offset (unused for now)
- * @param end Offset (unused)
* @return id of the file
*/
bool downloadFile(const std::string& interactionId,
const std::string& fileId,
const std::string& path,
const std::string& member = "",
- const std::string& deviceId = "",
- std::size_t start = 0,
- std::size_t end = 0);
+ const std::string& deviceId = "");
/**
* Reset fetched information
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index a973d08102..c80b32209b 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -2409,14 +2409,12 @@ bool
ConversationModule::downloadFile(const std::string& conversationId,
const std::string& interactionId,
const std::string& fileId,
- const std::string& path,
- size_t start,
- size_t end)
+ const std::string& path)
{
if (auto conv = pimpl_->getConversation(conversationId)) {
std::lock_guard lk(conv->mtx);
if (conv->conversation)
- return conv->conversation->downloadFile(interactionId, fileId, path, "", "", start, end);
+ return conv->conversation->downloadFile(interactionId, fileId, path, "", "");
}
return false;
}
diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h
index b3024b70fa..71fed14091 100644
--- a/src/jamidht/conversation_module.h
+++ b/src/jamidht/conversation_module.h
@@ -296,9 +296,7 @@ public:
bool downloadFile(const std::string& conversationId,
const std::string& interactionId,
const std::string& fileId,
- const std::string& path,
- size_t start = 0,
- size_t end = 0);
+ const std::string& path);
// Sync
/**
diff --git a/test/unitTest/fileTransfer/fileTransfer.cpp b/test/unitTest/fileTransfer/fileTransfer.cpp
index 81f5701fd7..50d5fcc667 100644
--- a/test/unitTest/fileTransfer/fileTransfer.cpp
+++ b/test/unitTest/fileTransfer/fileTransfer.cpp
@@ -97,7 +97,8 @@ private:
void testBadSha3sumIn();
void testAskToMultipleParticipants();
void testCancelInTransfer();
- void testCancelOutTransfer();
+ void testResumeTransferAfterInterruption();
+ void testDontDownloadExistingFile();
void testTransferInfo();
void testRemoveHardLink();
void testTooLarge();
@@ -111,6 +112,8 @@ private:
CPPUNIT_TEST(testBadSha3sumIn);
CPPUNIT_TEST(testAskToMultipleParticipants);
CPPUNIT_TEST(testCancelInTransfer);
+ CPPUNIT_TEST(testResumeTransferAfterInterruption);
+ CPPUNIT_TEST(testDontDownloadExistingFile);
CPPUNIT_TEST(testTransferInfo);
CPPUNIT_TEST(testRemoveHardLink);
CPPUNIT_TEST(testTooLarge);
@@ -532,6 +535,95 @@ FileTransferTest::testCancelInTransfer()
CPPUNIT_ASSERT(!bobAccount->dataTransfer(convId)->isWaiting(fileId));
}
+void
+FileTransferTest::testResumeTransferAfterInterruption()
+{
+ // Create conversation
+ auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto bobUri = bobAccount->getUsername();
+ auto aliceUri = aliceAccount->getUsername();
+ connectSignals();
+ auto convId = libjami::startConversation(aliceId);
+
+ libjami::addConversationMember(aliceId, convId, bobUri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
+
+ auto aliceMsgSize = aliceData.messages.size();
+ libjami::acceptConversationRequest(bobId, convId);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
+
+ // Create file to send
+ constexpr int64_t totalSize = 12800000;
+ std::ofstream sendFile(sendPath);
+ CPPUNIT_ASSERT(sendFile.is_open());
+ sendFile << std::string(totalSize, 'A');
+ sendFile.close();
+
+ // Send file info
+ auto bobMsgSize = bobData.messages.size();
+ libjami::sendFile(aliceId, convId, sendPath, "SEND", "");
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobMsgSize + 1 == bobData.messages.size(); }));
+ auto id = bobData.messages.rbegin()->id;
+ auto fileId = bobData.messages.rbegin()->body["fileId"];
+
+ libjami::downloadFile(bobId, convId, id, fileId, recvPath);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.code == static_cast<int>(libjami::DataTransferEventCode::ongoing); }));
+
+ Manager::instance().sendRegister(aliceId, false);
+
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.code == static_cast<int>(libjami::DataTransferEventCode::closed_by_host); }));
+
+ int64_t receivedSize = fileutils::size(recvPath + std::string(".tmp"));
+ CPPUNIT_ASSERT(receivedSize < totalSize);
+ CPPUNIT_ASSERT(0 < receivedSize);
+ CPPUNIT_ASSERT(bobAccount->dataTransfer(convId)->isWaiting(fileId));
+
+ Manager::instance().sendRegister(aliceId, true);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.code == static_cast<int>(libjami::DataTransferEventCode::finished); }));
+ CPPUNIT_ASSERT(!bobAccount->dataTransfer(convId)->isWaiting(fileId));
+}
+
+void
+FileTransferTest::testDontDownloadExistingFile()
+{
+ // Create conversation
+ auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto bobUri = bobAccount->getUsername();
+ auto aliceUri = aliceAccount->getUsername();
+ connectSignals();
+ auto convId = libjami::startConversation(aliceId);
+
+ libjami::addConversationMember(aliceId, convId, bobUri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
+
+ auto aliceMsgSize = aliceData.messages.size();
+ libjami::acceptConversationRequest(bobId, convId);
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
+
+ // Create file to send
+ constexpr int64_t totalSize = 128000;
+ std::ofstream sendFile(sendPath);
+ CPPUNIT_ASSERT(sendFile.is_open());
+ sendFile << std::string(totalSize, 'A');
+ sendFile.close();
+
+ std::filesystem::copy(sendPath, recvPath);
+
+ // Send file info
+ auto bobMsgSize = bobData.messages.size();
+ libjami::sendFile(aliceId, convId, sendPath, "SEND", "");
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobMsgSize + 1 == bobData.messages.size(); }));
+ auto id = bobData.messages.rbegin()->id;
+ auto fileId = bobData.messages.rbegin()->body["fileId"];
+
+ libjami::downloadFile(bobId, convId, id, fileId, recvPath);
+ CPPUNIT_ASSERT(!cv.wait_for(lk, 10s, [&]() { return bobData.code > 0; }));
+ CPPUNIT_ASSERT(!bobAccount->dataTransfer(convId)->isWaiting(fileId));
+ CPPUNIT_ASSERT(fileutils::size(recvPath) == totalSize);
+}
+
void
FileTransferTest::testTransferInfo()
{
--
GitLab