diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp index 76d02531871d51de20e68728e81b5bf609102be3..d4b613fa6441a872ac57f1b54b675500a6e7115e 100644 --- a/src/jamidht/conversation.cpp +++ b/src/jamidht/conversation.cpp @@ -931,10 +931,13 @@ Conversation::Impl::handleEdition(History& history, auto baseCommit = it->second; if (baseCommit) { auto itReact = baseCommit->body.find("react-to"); - auto body = sharedCommit->body.at("body"); + auto toReplace = "body"; + if (baseCommit->type == "application/data-transfer+json") + toReplace = "tid"; + auto body = sharedCommit->body.at(toReplace); // Edit reaction if (itReact != baseCommit->body.end()) { - baseCommit->body["body"] = body; // Replace body if pending + baseCommit->body[toReplace] = body; // Replace body if pending it = history.quickAccess.find(itReact->second); auto itPending = history.pendingReactions.find(itReact->second); if (it != history.quickAccess.end()) { @@ -945,7 +948,7 @@ Conversation::Impl::handleEdition(History& history, return reaction.at("id") == editId; }); if (itPreviousReact != baseCommit->reactions.end()) { - (*itPreviousReact)["body"] = body; + (*itPreviousReact)[toReplace] = body; if (body.empty()) { baseCommit->reactions.erase(itPreviousReact); emitSignal<libjami::ConversationSignal::ReactionRemoved>(accountId_, @@ -963,7 +966,7 @@ Conversation::Impl::handleEdition(History& history, return reaction.at("id") == editId; }); if (itReaction != itPending->second.end()) { - (*itReaction)["body"] = body; + (*itReaction)[toReplace] = body; if (body.empty()) itPending->second.erase(itReaction); } @@ -975,13 +978,15 @@ Conversation::Impl::handleEdition(History& history, } else { // Normal message it->second->editions.emplace(it->second->editions.begin(), it->second->body); - it->second->body["body"] = sharedCommit->body["body"]; + it->second->body[toReplace] = sharedCommit->body[toReplace]; + if (toReplace == "tid") { + // Avoid to replace fileId in client + it->second->body["fileId"] = ""; + } // Remove reactions - if (sharedCommit->body.at("body").empty()) + if (sharedCommit->body.at(toReplace).empty()) it->second->reactions.clear(); - emitSignal<libjami::ConversationSignal::SwarmMessageUpdated>(accountId_, - repository_->id(), - *it->second); + emitSignal<libjami::ConversationSignal::SwarmMessageUpdated>(accountId_, repository_->id(), *it->second); } } } else { @@ -1017,7 +1022,11 @@ Conversation::Impl::handleMessage(History& history, auto peditIt = history.pendingEditions.find(sharedCommit->id); if (peditIt != history.pendingEditions.end()) { auto oldBody = sharedCommit->body; - sharedCommit->body["body"] = peditIt->second.front()->body["body"]; + if (sharedCommit->type == "application/data-transfer+json") { + sharedCommit->body["tid"] = peditIt->second.front()->body["tid"]; + } else { + sharedCommit->body["body"] = peditIt->second.front()->body["body"]; + } peditIt->second.pop_front(); for (const auto& commit : peditIt->second) { sharedCommit->editions.emplace_back(commit->body); diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index 75044ef2010abbb1e87c8f85d9a2829e238f7478..53a73f3a75ca28674eedf7f61471ab9bafabea51 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -1198,13 +1198,17 @@ ConversationModule::Impl::editMessage(const std::string& conversationId, { // Check that editedId is a valid commit, from ourself and plain/text auto validCommit = false; + std::string type, tid; if (auto conv = getConversation(conversationId)) { std::lock_guard lk(conv->mtx); if (conv->conversation) { auto commit = conv->conversation->getCommit(editedId); if (commit != std::nullopt) { + type = commit->at("type"); + if (type == "application/data-transfer+json") + tid = commit->at("tid"); validCommit = commit->at("author") == username_ - && commit->at("type") == "text/plain"; + && (type == "text/plain" || type == "application/data-transfer+json"); } } } @@ -1214,9 +1218,18 @@ ConversationModule::Impl::editMessage(const std::string& conversationId, } // Commit message edition Json::Value json; - json["body"] = newBody; + if (type == "application/data-transfer+json") { + json["tid"] = ""; + // Remove file! + auto path = fileutils::get_data_dir() / accountId_ + / "conversation_data" / conversationId + / fmt::format("{}_{}", editedId, tid); + dhtnet::fileutils::remove(path, true); + } else { + json["body"] = newBody; + } json["edit"] = editedId; - json["type"] = "application/edited-message"; + json["type"] = type; sendMessage(conversationId, std::move(json)); } diff --git a/src/jamidht/conversationrepository.cpp b/src/jamidht/conversationrepository.cpp index bc276f266040cb5537f4815303c10d3b05148728..0eb630cab585b2e3d53bc117e279c09d9f507907 100644 --- a/src/jamidht/conversationrepository.cpp +++ b/src/jamidht/conversationrepository.cpp @@ -186,7 +186,6 @@ public: GitObject memberCertificate(std::string_view memberUri, const GitTree& tree) const; // NOTE! GitDiff needs to be deteleted before repo GitTree treeAtCommit(git_repository* repo, const std::string& commitId) const; - std::string getCommitType(const std::string& commitMsg) const; std::vector<std::string> getInitialMembers() const; @@ -1040,11 +1039,15 @@ ConversationRepository::Impl::checkEdit(const std::string& userDevice, JAMI_ERROR("Edited commit {:s} got a different author ({:s})", editedId, commit.id); return false; } - if (editedCommitMap->at("type") != "text/plain") { - JAMI_ERROR("Edited commit {:s} is not text!", editedId); - return false; + if (editedCommitMap->at("type") == "text/plain") { + return true; } - return true; + if (editedCommitMap->at("type") == "application/data-transfer+json") { + if (editedCommitMap->find("tid") != editedCommitMap->end()) + return true; + } + JAMI_ERROR("Edited commit {:s} is not valid!", editedId); + return false; } bool @@ -2308,22 +2311,6 @@ ConversationRepository::Impl::treeAtCommit(git_repository* repo, const std::stri return GitTree {tree, git_tree_free}; } -std::string -ConversationRepository::Impl::getCommitType(const std::string& commitMsg) const -{ - std::string type = {}; - std::string err; - Json::Value cm; - Json::CharReaderBuilder rbuilder; - auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader()); - if (reader->parse(commitMsg.data(), commitMsg.data() + commitMsg.size(), &cm, &err)) { - type = cm["type"].asString(); - } else { - JAMI_WARNING("{}", err); - } - return type; -} - std::vector<std::string> ConversationRepository::Impl::getInitialMembers() const { @@ -2522,10 +2509,16 @@ ConversationRepository::Impl::convCommitToMap(const ConversationCommit& commit) return std::nullopt; } else if (type == "application/data-transfer+json") { // Avoid the client to do the concatenation - message["fileId"] = commit.id + "_" + message["tid"]; - auto extension = fileutils::getFileExtension(message["displayName"]); - if (!extension.empty()) - message["fileId"] += "." + extension; + auto tid = message["tid"]; + if (not tid.empty()) { + auto extension = fileutils::getFileExtension(message["displayName"]); + if (!extension.empty()) + message["fileId"] = fmt::format("{}_{}.{}", commit.id, tid, extension); + else + message["fileId"] = fmt::format("{}_{}", commit.id, tid); + } else { + message["fileId"] = ""; + } } message["id"] = commit.id; message["parents"] = parents; @@ -2693,7 +2686,23 @@ ConversationRepository::Impl::validCommits( return false; } } else if (commit.parents.size() == 1) { - auto type = getCommitType(commit.commit_msg); + std::string type = {}, editId = {}; + std::string err; + Json::Value cm; + Json::CharReaderBuilder rbuilder; + auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader()); + if (reader->parse(commit.commit_msg.data(), commit.commit_msg.data() + commit.commit_msg.size(), &cm, &err)) { + type = cm["type"].asString(); + editId = cm["edit"].asString(); + } else { + JAMI_WARNING("{}", err); + if (auto shared = account_.lock()) { + emitSignal<libjami::ConversationSignal::OnConversationError>( + shared->getAccountID(), id_, EVALIDFETCH, "Malformed commit"); + } + return false; + } + if (type == "vote") { // Check that vote is valid if (!checkVote(userDevice, commit.id, commit.parents[0])) { @@ -2708,23 +2717,8 @@ ConversationRepository::Impl::validCommits( return false; } } else if (type == "member") { - std::string err; - Json::Value root; - Json::CharReaderBuilder rbuilder; - auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader()); - if (!reader->parse(commit.commit_msg.data(), - commit.commit_msg.data() + commit.commit_msg.size(), - &root, - &err)) { - JAMI_ERROR("Failed to parse: {}", err); - if (auto shared = account_.lock()) { - emitSignal<libjami::ConversationSignal::OnConversationError>( - shared->getAccountID(), id_, EVALIDFETCH, "Malformed member commit"); - } - return false; - } - std::string action = root["action"].asString(); - std::string uriMember = root["uri"].asString(); + std::string action = cm["action"].asString(); + std::string uriMember = cm["uri"].asString(); if (action == "add") { if (!checkValidAdd(userDevice, uriMember, commit.id, commit.parents[0])) { JAMI_WARNING( @@ -2821,7 +2815,7 @@ ConversationRepository::Impl::validCommits( } return false; } - } else if (type == "application/edited-message") { + } else if (type == "application/edited-message" || !editId.empty()) { if (!checkEdit(userDevice, commit)) { JAMI_ERROR("Commit {:s} malformed", commit.id); if (auto shared = account_.lock()) { diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index 1fc3690f8b4bfb429b888b68be457d8a49b78454..4f49b61bba26ca61597179f731d60376febc3aed 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -2227,7 +2227,7 @@ ConversationTest::testMessageEdition() !cv.wait_for(lk, 10s, [&]() { return bobData.messagesUpdated.size() == bobMsgSize + 1; })); // Add invalid edition Json::Value root; - root["type"] = "application/edited-message"; + root["type"] = "text/plain"; root["edit"] = convId; root["body"] = "new"; Json::StreamWriterBuilder wbuilder; diff --git a/test/unitTest/fileTransfer/fileTransfer.cpp b/test/unitTest/fileTransfer/fileTransfer.cpp index 852b527111d3f1d62a2ab2199a3f97c5ea2e853b..3bb3490fa7116fb3d4043d5c4e3cee7b6c5184a0 100644 --- a/test/unitTest/fileTransfer/fileTransfer.cpp +++ b/test/unitTest/fileTransfer/fileTransfer.cpp @@ -102,6 +102,7 @@ private: void testTransferInfo(); void testRemoveHardLink(); void testTooLarge(); + void testDeleteFile(); CPPUNIT_TEST_SUITE(FileTransferTest); CPPUNIT_TEST(testConversationFileTransfer); @@ -114,6 +115,7 @@ private: CPPUNIT_TEST(testTransferInfo); CPPUNIT_TEST(testRemoveHardLink); CPPUNIT_TEST(testTooLarge); + CPPUNIT_TEST(testDeleteFile); CPPUNIT_TEST_SUITE_END(); }; @@ -643,6 +645,71 @@ FileTransferTest::testTooLarge() CPPUNIT_ASSERT(!dhtnet::fileutils::isFile(recvPath)); } +void +FileTransferTest::testDeleteFile() +{ + auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId); + auto convId = libjami::startConversation(aliceId); + + std::map<std::string, std::shared_ptr<libjami::CallbackWrapperBase>> confHandlers; + bool conversationReady = false; + std::string iid, tid; + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::SwarmMessageReceived>( + [&](const std::string& accountId, + const std::string& /* conversationId */, + libjami::SwarmMessage message) { + if (message.type == "application/data-transfer+json") { + if (accountId == aliceId) { + iid = message.id; + tid = message.body["tid"]; + } + } + cv.notify_one(); + })); + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& /* conversationId */) { + if (accountId == bobId) { + conversationReady = true; + cv.notify_one(); + } + })); + bool messageUpdated = false; + confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::SwarmMessageUpdated>( + [&](const std::string& accountId, + const std::string& /* conversationId */, + libjami::SwarmMessage message) { + if (accountId == aliceId && message.type == "application/data-transfer+json" && message.body["tid"].empty()) { + messageUpdated = true; + } + cv.notify_one(); + })); + libjami::registerSignalHandlers(confHandlers); + + // Create file to send + std::ofstream sendFile(sendPath); + CPPUNIT_ASSERT(sendFile.is_open()); + sendFile << std::string(64000, 'A'); + sendFile.close(); + + libjami::sendFile(aliceId, convId, sendPath, "SEND", ""); + + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !iid.empty(); })); + auto dataPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + aliceId + + DIR_SEPARATOR_STR + "conversation_data" + DIR_SEPARATOR_STR + convId; + CPPUNIT_ASSERT(fileutils::isFile(fmt::format("{}/{}_{}", dataPath, iid, tid))); + + // Delete file + libjami::sendMessage(aliceId, convId, ""s, iid, 1); + + // Verify message is updated + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageUpdated; })); + // Verify file is deleted + CPPUNIT_ASSERT(!fileutils::isFile(fmt::format("{}/{}_{}", dataPath, iid, tid))); + + libjami::unregisterSignalHandlers(); + std::this_thread::sleep_for(5s); +} + } // namespace test } // namespace jami