diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml index 9b3dd7760..469048349 100644 --- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml +++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml @@ -1774,14 +1774,17 @@ - + - Add a message to a conversation + Add a message to a conversation. + Flag: 0 = reply (if commitId is not empty), else single message + 1 = edit message - + + diff --git a/bin/dbus/dbusconfigurationmanager.cpp b/bin/dbus/dbusconfigurationmanager.cpp index 3effef201..57466c0c9 100644 --- a/bin/dbus/dbusconfigurationmanager.cpp +++ b/bin/dbus/dbusconfigurationmanager.cpp @@ -921,9 +921,10 @@ void DBusConfigurationManager::sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& replyTo) + const std::string& replyTo, + const int32_t& flag) { - DRing::sendMessage(accountId, conversationId, message, replyTo); + DRing::sendMessage(accountId, conversationId, message, replyTo, flag); } uint32_t diff --git a/bin/dbus/dbusconfigurationmanager.h b/bin/dbus/dbusconfigurationmanager.h index b14548730..f317cd436 100644 --- a/bin/dbus/dbusconfigurationmanager.h +++ b/bin/dbus/dbusconfigurationmanager.h @@ -281,7 +281,8 @@ public: void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& replyTo); + const std::string& replyTo, + const int32_t& flag); uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, diff --git a/bin/jni/conversation.i b/bin/jni/conversation.i index 4437faaf3..6a628904c 100644 --- a/bin/jni/conversation.i +++ b/bin/jni/conversation.i @@ -61,7 +61,7 @@ namespace DRing { std::vector> getConversationMembers(const std::string& accountId, const std::string& conversationId); // Message send/load - void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo); + void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo, const int32_t& flag); uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, size_t n); uint32_t loadConversationUntil(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, const std::string& toMessage); uint32_t countInteractions(const std::string& accountId, const std::string& conversationId, const std::string& toId, const std::string& fromId, const std::string& authorUri); diff --git a/bin/nodejs/conversation.i b/bin/nodejs/conversation.i index f645253f6..ac254bcd2 100644 --- a/bin/nodejs/conversation.i +++ b/bin/nodejs/conversation.i @@ -61,7 +61,7 @@ namespace DRing { std::vector> getConversationMembers(const std::string& accountId, const std::string& conversationId); // Message send/load - void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo); + void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo, const int32_t& flag); uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, size_t n); uint32_t loadConversationUntil(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, const std::string& toMessage); uint32_t countInteractions(const std::string& accountId, const std::string& conversationId, const std::string& toId, const std::string& fromId, const std::string& authorUri); diff --git a/configure.ac b/configure.ac index 3988e089f..27215d044 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ dnl Jami - configure.ac dnl Process this file with autoconf to produce a configure script. AC_PREREQ([2.69]) -AC_INIT([Jami Daemon],[13.5.0],[jami@gnu.org],[jami]) +AC_INIT([Jami Daemon],[13.6.0],[jami@gnu.org],[jami]) dnl Clear the implicit flags that default to '-g -O2', otherwise they dnl take precedence over the values we set via the diff --git a/meson.build b/meson.build index bd14e1b37..3312907c3 100644 --- a/meson.build +++ b/meson.build @@ -1,5 +1,5 @@ project('jami-daemon', ['c', 'cpp'], - version: '13.5.0', + version: '13.6.0', license: 'GPL3+', default_options: ['cpp_std=gnu++17', 'buildtype=debugoptimized'], meson_version:'>= 0.56' diff --git a/src/client/conversation_interface.cpp b/src/client/conversation_interface.cpp index 47607790a..7312f3f2d 100644 --- a/src/client/conversation_interface.cpp +++ b/src/client/conversation_interface.cpp @@ -160,11 +160,17 @@ void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& replyTo) + const std::string& commitId, + const int32_t& flag) { if (auto acc = jami::Manager::instance().getAccount(accountId)) - if (auto convModule = acc->convModule()) - convModule->sendMessage(conversationId, message, replyTo); + if (auto convModule = acc->convModule()) { + if (flag == 0 /* Reply or simple commit */) { + convModule->sendMessage(conversationId, message, commitId); + } else if (flag == 1 /* message edition */) { + convModule->editMessage(conversationId, message, commitId); + } + } } uint32_t diff --git a/src/jami/conversation_interface.h b/src/jami/conversation_interface.h index bb05c1920..404b7332c 100644 --- a/src/jami/conversation_interface.h +++ b/src/jami/conversation_interface.h @@ -69,7 +69,8 @@ DRING_PUBLIC std::vector> getConversationMemb DRING_PUBLIC void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& replyTo); + const std::string& replyTo, + const int32_t& flag = 0); DRING_PUBLIC uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index f9850f904..6bebba879 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -227,6 +227,10 @@ public: OnCommitCb&& onCommit = {}, OnDoneCb&& cb = {}); + void editMessage(const std::string& conversationId, + const std::string& newBody, + const std::string& editedId); + // The following informations are stored on the disk mutable std::mutex convInfosMtx_; // Note, should be locked after conversationsMtx_ if needed std::map convInfos_; @@ -842,6 +846,36 @@ ConversationModule::Impl::sendMessage(const std::string& conversationId, } } +void +ConversationModule::Impl::editMessage(const std::string& conversationId, + const std::string& newBody, + const std::string& editedId) +{ + // Check that editedId is a valid commit, from ourself and plain/text + auto validCommit = false; + { + std::lock_guard lk(conversationsMtx_); + auto conversation = conversations_.find(conversationId); + if (conversation != conversations_.end() && conversation->second) { + auto commit = conversation->second->getCommit(editedId); + if (commit != std::nullopt) { + validCommit = commit->at("author") == username_ + && commit->at("type") == "text/plain"; + } + } + } + if (!validCommit) { + JAMI_ERROR("Cannot edit commit {:s}", editedId); + return; + } + // Commit message edition + Json::Value json; + json["body"] = newBody; + json["edit"] = editedId; + json["type"] = "application/edited-message"; + sendMessage(conversationId, std::move(json)); +} + //////////////////////////////////////////////////////////////// void @@ -1306,6 +1340,14 @@ ConversationModule::sendMessage(const std::string& conversationId, std::move(cb)); } +void +ConversationModule::editMessage(const std::string& conversationId, + const std::string& newBody, + const std::string& editedId) +{ + pimpl_->editMessage(conversationId, newBody, editedId); +} + void ConversationModule::addCallHistoryMessage(const std::string& uri, uint64_t duration_ms) { diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h index 83f3fcd15..7dee281aa 100644 --- a/src/jamidht/conversation_module.h +++ b/src/jamidht/conversation_module.h @@ -160,6 +160,10 @@ public: OnCommitCb&& onCommit = {}, OnDoneCb&& cb = {}); + void editMessage(const std::string& conversationId, + const std::string& newBody, + const std::string& editedId); + /** * Add to the related conversation the call history message * @param uri Peer number diff --git a/src/jamidht/conversationrepository.cpp b/src/jamidht/conversationrepository.cpp index b11d9cc19..f88067231 100644 --- a/src/jamidht/conversationrepository.cpp +++ b/src/jamidht/conversationrepository.cpp @@ -102,6 +102,7 @@ public: bool checkVote(const std::string& userDevice, const std::string& commitId, const std::string& parentId) const; + bool checkEdit(const std::string& userDevice, const ConversationCommit& commit) const; bool isValidUserAtCommit(const std::string& userDevice, const std::string& commitId) const; bool checkInitialCommit(const std::string& userDevice, const std::string& commitId, @@ -178,6 +179,15 @@ public: return members_; } + std::optional getCommit(const std::string& commitId, + bool logIfNotFound = true) const + { + auto commits = log(commitId, "", 1, logIfNotFound); + if (commits.empty()) + return std::nullopt; + return std::move(commits[0]); + } + bool resolveConflicts(git_index* index, const std::string& other_id); std::vector memberUris(std::string_view filter, @@ -862,6 +872,36 @@ ConversationRepository::Impl::checkValidUserDiff(const std::string& userDevice, return true; } +bool +ConversationRepository::Impl::checkEdit(const std::string& userDevice, + const ConversationCommit& commit) const +{ + auto userUri = uriFromDevice(userDevice); + // Check that edited commit is found, for the same author, and editable (plain/text) + auto commitMap = convCommitToMap(commit); + if (commitMap == std::nullopt) { + return false; + } + auto editedId = commitMap->at("edit"); + auto editedCommit = getCommit(editedId); + if (editedCommit == std::nullopt) { + JAMI_ERROR("Commit {:s} not found", editedId); + return false; + } + auto editedCommitMap = convCommitToMap(*editedCommit); + if (editedCommitMap == std::nullopt or editedCommitMap->at("author").empty() + or editedCommitMap->at("author") != commitMap->at("author") + or commitMap->at("author") != userUri) { + 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; + } + return true; +} + bool ConversationRepository::Impl::checkVote(const std::string& userDevice, const std::string& commitId, @@ -2630,6 +2670,15 @@ ConversationRepository::Impl::validCommits( } return false; } + } else if (type == "application/edited-message") { + if (!checkEdit(userDevice, commit)) { + JAMI_ERROR("Commit {:s} malformed", commit.id); + if (auto shared = account_.lock()) { + emitSignal( + shared->getAccountID(), id_, EVALIDFETCH, "Malformed edit commit"); + } + return false; + } } else { // Note: accept all mimetype here, as we can have new mimetypes // Just avoid to add weird files @@ -2970,10 +3019,7 @@ ConversationRepository::search(const Filter& filter) const std::optional ConversationRepository::getCommit(const std::string& commitId, bool logIfNotFound) const { - auto commits = logN(commitId, 1, logIfNotFound); - if (commits.empty()) - return std::nullopt; - return std::move(commits[0]); + return pimpl_->getCommit(commitId, logIfNotFound); } std::pair diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index 1f9280765..c5c7f9995 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -116,6 +116,7 @@ private: void testConversationPreferencesMultiDevices(); void testFixContactDetails(); void testRemoveOneToOneNotInDetails(); + void testMessageEdition(); CPPUNIT_TEST_SUITE(ConversationTest); CPPUNIT_TEST(testCreateConversation); @@ -162,6 +163,7 @@ private: CPPUNIT_TEST(testConversationPreferencesMultiDevices); CPPUNIT_TEST(testFixContactDetails); CPPUNIT_TEST(testRemoveOneToOneNotInDetails); + CPPUNIT_TEST(testMessageEdition); CPPUNIT_TEST_SUITE_END(); }; @@ -3397,6 +3399,100 @@ ConversationTest::testRemoveOneToOneNotInDetails() CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return conversationRemoved; })); } +void +ConversationTest::testMessageEdition() +{ + auto aliceAccount = Manager::instance().getAccount(aliceId); + auto bobAccount = Manager::instance().getAccount(bobId); + auto bobUri = bobAccount->getUsername(); + std::mutex mtx; + std::unique_lock lk {mtx}; + std::condition_variable cv; + std::map> confHandlers; + std::vector> messageBobReceived; + bool conversationReady = false, memberMessageGenerated = false; + confHandlers.insert(DRing::exportable_callback( + [&](const std::string& accountId, + const std::string& /* conversationId */, + std::map message) { + if (accountId == bobId) { + messageBobReceived.emplace_back(message); + JAMI_ERR("@@@ EMPLACE!"); + } else if (accountId == aliceId && message["type"] == "member") { + memberMessageGenerated = true; + } + cv.notify_one(); + })); + bool requestReceived = false; + confHandlers.insert( + DRing::exportable_callback( + [&](const std::string& /*accountId*/, + const std::string& /* conversationId */, + std::map /*metadatas*/) { + requestReceived = true; + cv.notify_one(); + })); + confHandlers.insert(DRing::exportable_callback( + [&](const std::string& accountId, const std::string& /* conversationId */) { + if (accountId == bobId) { + conversationReady = true; + cv.notify_one(); + } + })); + auto errorDetected = false; + confHandlers.insert(DRing::exportable_callback( + [&](const std::string& /* accountId */, + const std::string& /* conversationId */, + int code, + const std::string& /* what */) { + if (code == 3) + errorDetected = true; + cv.notify_one(); + })); + DRing::registerSignalHandlers(confHandlers); + auto convId = DRing::startConversation(aliceId); + DRing::addConversationMember(aliceId, convId, bobUri); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return requestReceived; })); + memberMessageGenerated = false; + DRing::acceptConversationRequest(bobId, convId); + CPPUNIT_ASSERT( + cv.wait_for(lk, 30s, [&]() { return conversationReady && memberMessageGenerated; })); + auto msgSize = messageBobReceived.size(); + JAMI_ERR("@@@@@@@@ %u", msgSize); + DRing::sendMessage(aliceId, convId, "hi"s, ""); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageBobReceived.size() == msgSize + 1; })); + msgSize = messageBobReceived.size(); + auto editedId = messageBobReceived.rbegin()->at("id"); + DRing::sendMessage(aliceId, convId, "New body"s, editedId, 1); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return messageBobReceived.size() == msgSize + 1; })); + CPPUNIT_ASSERT(messageBobReceived.rbegin()->at("edit") == editedId); + CPPUNIT_ASSERT(messageBobReceived.rbegin()->at("body") == "New body"); + // Not an existing message + msgSize = messageBobReceived.size(); + DRing::sendMessage(aliceId, convId, "New body"s, "invalidId", 1); + CPPUNIT_ASSERT( + !cv.wait_for(lk, 10s, [&]() { return messageBobReceived.size() == msgSize + 1; })); + // Invalid author + DRing::sendMessage(aliceId, convId, "New body"s, convId, 1); + CPPUNIT_ASSERT( + !cv.wait_for(lk, 10s, [&]() { return messageBobReceived.size() == msgSize + 1; })); + // Add invalid edition + Json::Value root; + root["type"] = "application/edited-message"; + root["edit"] = convId; + root["body"] = "new"; + Json::StreamWriterBuilder wbuilder; + wbuilder["commentStyle"] = "None"; + wbuilder["indentation"] = ""; + auto repoPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + aliceAccount->getAccountID() + + DIR_SEPARATOR_STR + "conversations" + DIR_SEPARATOR_STR + convId; + auto message = Json::writeString(wbuilder, root); + commitInRepo(repoPath, aliceAccount, message); + errorDetected = false; + DRing::sendMessage(aliceId, convId, "trigger"s, ""); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return errorDetected; })); +} + } // namespace test } // namespace jami