diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml index 15fab637c9a19a7fa82ef13c990768594d79d120..891b3137e3ac3758f5f38ad5b83eb9312bf9693d 100644 --- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml +++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml @@ -1576,7 +1576,7 @@ <arg type="s" name="conversationId" direction="in"/> <arg type="s" name="filePath" direction="in"/> <arg type="s" name="fileDisplayName" direction="in"/> - <arg type="s" name="parent" direction="in"/> + <arg type="s" name="replyTo" direction="in"/> </method> <method name="sendFileLegacy" tp:name-for-bindings="sendFileLegacy"> @@ -1759,7 +1759,7 @@ <arg type="s" name="accountId" direction="in"/> <arg type="s" name="conversationId" direction="in"/> <arg type="s" name="message" direction="in"/> - <arg type="s" name="parent" direction="in"/> + <arg type="s" name="replyTo" direction="in"/> </method> <method name="loadConversationMessages" tp:name-for-bindings="loadConversationMessages"> diff --git a/bin/dbus/dbusconfigurationmanager.cpp b/bin/dbus/dbusconfigurationmanager.cpp index b65dd58a361a22c6554ffbdc8cff1b376b2c9acd..ebe07adc767f7ee98d5be49bc9bdf6596633e95f 100644 --- a/bin/dbus/dbusconfigurationmanager.cpp +++ b/bin/dbus/dbusconfigurationmanager.cpp @@ -544,8 +544,8 @@ DBusConfigurationManager::validateCertificatePath(const std::string& accountId, const std::string& privateKey, const std::string& privateKeyPass, const std::string& caList) - -> decltype( - DRing::validateCertificatePath(accountId, certificate, privateKey, privateKeyPass, caList)) + -> decltype(DRing::validateCertificatePath( + accountId, certificate, privateKey, privateKeyPass, caList)) { return DRing::validateCertificatePath(accountId, certificate, privateKey, privateKeyPass, caList); } @@ -757,9 +757,9 @@ DBusConfigurationManager::sendFile(const std::string& accountId, const std::string& conversationId, const std::string& path, const std::string& displayName, - const std::string& parent) + const std::string& replyTo) { - DRing::sendFile(accountId, conversationId, path, displayName, parent); + DRing::sendFile(accountId, conversationId, path, displayName, replyTo); } void @@ -906,9 +906,9 @@ void DBusConfigurationManager::sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& parent) + const std::string& replyTo) { - DRing::sendMessage(accountId, conversationId, message, parent); + DRing::sendMessage(accountId, conversationId, message, replyTo); } uint32_t diff --git a/bin/dbus/dbusconfigurationmanager.h b/bin/dbus/dbusconfigurationmanager.h index 9429a89ac770c9ea6788877e969fbde87f775c2c..0224aa6b89daf24f0b2291ec3603949607191831 100644 --- a/bin/dbus/dbusconfigurationmanager.h +++ b/bin/dbus/dbusconfigurationmanager.h @@ -220,7 +220,7 @@ public: const std::string& conversationId, const std::string& path, const std::string& displayName, - const std::string& parent); + const std::string& replyTo); void dataTransferInfo(const std::string& accountId, const std::string& fileId, uint32_t& error, @@ -276,7 +276,7 @@ public: void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& parent); + const std::string& replyTo); 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 2485823942f735d85c1e2cce59a1e705e4d12c66..db4fc2205979ee7071eea79ddd0445b3877f4078 100644 --- a/bin/jni/conversation.i +++ b/bin/jni/conversation.i @@ -57,7 +57,7 @@ namespace DRing { std::vector<std::map<std::string, std::string>> 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& parent); + void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo); uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, size_t n); 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/jni/datatransfer.i b/bin/jni/datatransfer.i index f3bbe6517c6824b2ecbb8e4a5ad1e373a8aeeff4..bc861c09fe6d3b0a86b70750eb0b44ca4c074172 100644 --- a/bin/jni/datatransfer.i +++ b/bin/jni/datatransfer.i @@ -78,7 +78,7 @@ namespace DRing { std::string mimetype; }; - void sendFile(const std::string& accountId, const std::string& conversationId, const std::string& path, const std::string& displayName, const std::string& parent); + void sendFile(const std::string& accountId, const std::string& conversationId, const std::string& path, const std::string& displayName, const std::string& replyTo); DRing::DataTransferError sendFileLegacy(const DRing::DataTransferInfo info, DRing::DataTransferId& id); DRing::DataTransferError acceptFileTransfer(const std::string& accountId, const std::string& fileId, const std::string& file_path); diff --git a/bin/nodejs/conversation.i b/bin/nodejs/conversation.i index dbc6107758b90b2a07dbd5d0acea8ce1129f82bd..7d0d94c4c4f7b5b9ec536fb957ec977b8d2b3546 100644 --- a/bin/nodejs/conversation.i +++ b/bin/nodejs/conversation.i @@ -57,7 +57,7 @@ namespace DRing { std::vector<std::map<std::string, std::string>> 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& parent); + void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, const std::string& replyTo); uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, size_t n); 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/src/client/conversation_interface.cpp b/src/client/conversation_interface.cpp index a686d74a438fb52d1daa463831fae1f72ef4c83d..91adc77d0a824cd495aef83b92a8b950482cf4ce 100644 --- a/src/client/conversation_interface.cpp +++ b/src/client/conversation_interface.cpp @@ -141,11 +141,11 @@ void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& parent) + const std::string& replyTo) { if (auto acc = jami::Manager::instance().getAccount<jami::JamiAccount>(accountId)) if (auto convModule = acc->convModule()) - convModule->sendMessage(conversationId, message, parent); + convModule->sendMessage(conversationId, message, replyTo); } uint32_t diff --git a/src/client/datatransfer.cpp b/src/client/datatransfer.cpp index eb3ffab3c6f1ca72bb66890745694b4c4636ec7c..3adbbc059b3869ec92f24a73011a66fecfe82bb8 100644 --- a/src/client/datatransfer.cpp +++ b/src/client/datatransfer.cpp @@ -49,10 +49,10 @@ sendFile(const std::string& accountId, const std::string& conversationId, const std::string& path, const std::string& displayName, - const std::string& parent) noexcept + const std::string& replyTo) noexcept { if (auto acc = jami::Manager::instance().getAccount<jami::JamiAccount>(accountId)) { - acc->sendFile(conversationId, path, displayName, parent); + acc->sendFile(conversationId, path, displayName, replyTo); } } diff --git a/src/jami/conversation_interface.h b/src/jami/conversation_interface.h index 06dcaf209dd47d5591f903f7f240690de0176252..63bbdc8012089c54503769e10c1ea56509f963b5 100644 --- a/src/jami/conversation_interface.h +++ b/src/jami/conversation_interface.h @@ -64,7 +64,7 @@ DRING_PUBLIC std::vector<std::map<std::string, std::string>> getConversationMemb DRING_PUBLIC void sendMessage(const std::string& accountId, const std::string& conversationId, const std::string& message, - const std::string& parent); + const std::string& replyTo); DRING_PUBLIC uint32_t loadConversationMessages(const std::string& accountId, const std::string& conversationId, const std::string& fromMessage, diff --git a/src/jami/datatransfer_interface.h b/src/jami/datatransfer_interface.h index 8ea13e74a7f94541eaf33bc58a2848267e456ceb..ac35be225b5b2f1f6697b17112284cf7884ada5b 100644 --- a/src/jami/datatransfer_interface.h +++ b/src/jami/datatransfer_interface.h @@ -115,7 +115,7 @@ DRING_PUBLIC void sendFile(const std::string& accountId, const std::string& conversationId, const std::string& path, const std::string& displayName, - const std::string& parent) noexcept; + const std::string& replyTo) noexcept; /// Accept an incoming file transfer. /// diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp index fa4dd138258e80c9d544c84f50b389adc0758c2d..858cc82e30ac6d1edc84200bdf961ddcbe299e30 100644 --- a/src/jamidht/conversation.cpp +++ b/src/jamidht/conversation.cpp @@ -760,18 +760,26 @@ Conversation::isBanned(const std::string& uri) const void Conversation::sendMessage(std::string&& message, const std::string& type, - const std::string& parent, + const std::string& replyTo, OnDoneCb&& cb) { Json::Value json; json["body"] = std::move(message); json["type"] = type; - sendMessage(std::move(json), parent, std::move(cb)); + sendMessage(std::move(json), replyTo, std::move(cb)); } void -Conversation::sendMessage(Json::Value&& value, const std::string& parent, OnDoneCb&& cb) +Conversation::sendMessage(Json::Value&& value, const std::string& replyTo, OnDoneCb&& cb) { + if (!replyTo.empty()) { + auto commit = pimpl_->repository_->getCommit(replyTo); + if (commit == std::nullopt) { + JAMI_ERR("Replying to invalid commit %s", replyTo.c_str()); + return; + } + value["reply-to"] = replyTo; + } dht::ThreadPool::io().run([w = weak(), value = std::move(value), cb = std::move(cb)] { if (auto sthis = w.lock()) { auto shared = sthis->pimpl_->account_.lock(); @@ -793,9 +801,7 @@ Conversation::sendMessage(Json::Value&& value, const std::string& parent, OnDone } void -Conversation::sendMessages(std::vector<Json::Value>&& messages, - const std::string& /*parent*/, - OnMultiDoneCb&& cb) +Conversation::sendMessages(std::vector<Json::Value>&& messages, OnMultiDoneCb&& cb) { dht::ThreadPool::io().run([w = weak(), messages = std::move(messages), cb = std::move(cb)] { if (auto sthis = w.lock()) { diff --git a/src/jamidht/conversation.h b/src/jamidht/conversation.h index 5d180ec9246cadf3a7934cc54d13043686385209..90bdbfad29ef99a4e881bc130cd64dc18202540a 100644 --- a/src/jamidht/conversation.h +++ b/src/jamidht/conversation.h @@ -179,13 +179,11 @@ public: // Message send void sendMessage(std::string&& message, const std::string& type = "text/plain", - const std::string& parent = "", + const std::string& replyTo = "", OnDoneCb&& cb = {}); - void sendMessage(Json::Value&& message, const std::string& parent = "", OnDoneCb&& cb = {}); + void sendMessage(Json::Value&& message, const std::string& replyTo = "", OnDoneCb&& cb = {}); // Note: used for replay. Should not be used by clients - void sendMessages(std::vector<Json::Value>&& messages, - const std::string& parent = "", - OnMultiDoneCb&& cb = {}); + void sendMessages(std::vector<Json::Value>&& messages, OnMultiDoneCb&& cb = {}); /** * Get a range of messages * @param cb The callback when loaded diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index e789d468dedc64a55f08f9dc51e5bc9b273cdf18..d0009f3c379cace4fb841373be7b93d461bd2071 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -215,13 +215,13 @@ public: // Message send/load void sendMessage(const std::string& conversationId, Json::Value&& value, - const std::string& parent = "", + const std::string& replyTo = "", bool announce = true, OnDoneCb&& cb = {}); void sendMessage(const std::string& conversationId, std::string message, - const std::string& parent = "", + const std::string& replyTo = "", const std::string& type = "text/plain", bool announce = true, OnDoneCb&& cb = {}); @@ -561,7 +561,6 @@ ConversationModule::Impl::handlePendingConversation(const std::string& conversat } if (!values.empty()) conversation->sendMessages(std::move(values), - "", [w = weak(), conversationId](const auto& commits) { auto shared = w.lock(); if (shared and not commits.empty()) @@ -780,7 +779,7 @@ ConversationModule::Impl::sendMessageNotification(const Conversation& conversati void ConversationModule::Impl::sendMessage(const std::string& conversationId, std::string message, - const std::string& parent, + const std::string& replyTo, const std::string& type, bool announce, OnDoneCb&& cb) @@ -788,13 +787,13 @@ ConversationModule::Impl::sendMessage(const std::string& conversationId, Json::Value json; json["body"] = std::move(message); json["type"] = type; - sendMessage(conversationId, std::move(json), parent, announce, std::move(cb)); + sendMessage(conversationId, std::move(json), replyTo, announce, std::move(cb)); } void ConversationModule::Impl::sendMessage(const std::string& conversationId, Json::Value&& value, - const std::string& parent, + const std::string& replyTo, bool announce, OnDoneCb&& cb) { @@ -803,7 +802,7 @@ ConversationModule::Impl::sendMessage(const std::string& conversationId, if (conversation != conversations_.end() && conversation->second) { conversation->second->sendMessage( std::move(value), - parent, + replyTo, [this, conversationId, announce, cb = std::move(cb)](bool ok, const std::string& commitId) { if (cb) @@ -1193,22 +1192,22 @@ ConversationModule::cloneConversationFrom(const std::string& conversationId, void ConversationModule::sendMessage(const std::string& conversationId, std::string message, - const std::string& parent, + const std::string& replyTo, const std::string& type, bool announce, OnDoneCb&& cb) { - pimpl_->sendMessage(conversationId, std::move(message), parent, type, announce, std::move(cb)); + pimpl_->sendMessage(conversationId, std::move(message), replyTo, type, announce, std::move(cb)); } void ConversationModule::sendMessage(const std::string& conversationId, Json::Value&& value, - const std::string& parent, + const std::string& replyTo, bool announce, OnDoneCb&& cb) { - pimpl_->sendMessage(conversationId, std::move(value), parent, announce, std::move(cb)); + pimpl_->sendMessage(conversationId, std::move(value), replyTo, announce, std::move(cb)); } void diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h index 09f60d5c3ffd26d9ede2a184dae72f1a1cd0acc6..2c4d38008316e8368401369bebba91f05c1c339b 100644 --- a/src/jamidht/conversation_module.h +++ b/src/jamidht/conversation_module.h @@ -138,13 +138,13 @@ public: // Message send/load void sendMessage(const std::string& conversationId, Json::Value&& value, - const std::string& parent = "", + const std::string& replyTo = "", bool announce = true, OnDoneCb&& cb = {}); void sendMessage(const std::string& conversationId, std::string message, - const std::string& parent = "", + const std::string& replyTo = "", const std::string& type = "text/plain", bool announce = true, OnDoneCb&& cb = {}); diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index f00fc995de6713d70d6f8dd7da80a2416c451d5e..f35dbf94254665afd2684cfe6460fb5351b882af 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -4215,7 +4215,7 @@ void JamiAccount::sendFile(const std::string& conversationId, const std::string& path, const std::string& name, - const std::string& parent) + const std::string& replyTo) { if (!fileutils::isFile(path)) { JAMI_ERR() << "invalid filename '" << path << "'"; @@ -4224,7 +4224,7 @@ JamiAccount::sendFile(const std::string& conversationId, // NOTE: this sendMessage is in a computation thread because // sha3sum can take quite some time to computer if the user decide // to send a big file - dht::ThreadPool::computation().run([w = weak(), conversationId, path, name, parent]() { + dht::ThreadPool::computation().run([w = weak(), conversationId, path, name, replyTo]() { if (auto shared = w.lock()) { Json::Value value; auto tid = jami::generateUID(); @@ -4238,7 +4238,7 @@ JamiAccount::sendFile(const std::string& conversationId, shared->convModule() ->sendMessage(conversationId, std::move(value), - parent, + replyTo, true, [accId = shared->getAccountID(), conversationId, diff --git a/src/jamidht/jamiaccount.h b/src/jamidht/jamiaccount.h index a50a74bae041d2a1f051c05a9fb71c6c5bc07af2..d47867f14bb6cce5580c2e90a88342ec0106d820 100644 --- a/src/jamidht/jamiaccount.h +++ b/src/jamidht/jamiaccount.h @@ -541,7 +541,7 @@ public: void sendFile(const std::string& conversationId, const std::string& path, const std::string& name, - const std::string& parent); + const std::string& replyTo); // non-swarm version DRing::DataTransferId sendFile(const std::string& peer, diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index a309c29cdd16bc2305c397c63b7ece7fd90c00fb..5876400bdb54f58cf9ae9310b8123eda6d55c955 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -108,6 +108,7 @@ private: void testSyncWithoutPinnedCert(); void testImportMalformedContacts(); void testRemoveReaddMultipleDevice(); + void testSendReply(); CPPUNIT_TEST_SUITE(ConversationTest); CPPUNIT_TEST(testCreateConversation); @@ -146,6 +147,7 @@ private: CPPUNIT_TEST(testSyncWithoutPinnedCert); CPPUNIT_TEST(testImportMalformedContacts); CPPUNIT_TEST(testRemoveReaddMultipleDevice); + CPPUNIT_TEST(testSendReply); CPPUNIT_TEST_SUITE_END(); }; @@ -2819,6 +2821,78 @@ ConversationTest::testRemoveReaddMultipleDevice() cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; })); } +void +ConversationTest::testSendReply() +{ + auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId); + auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId); + auto bobUri = bobAccount->getUsername(); + + std::mutex mtx; + std::unique_lock<std::mutex> lk {mtx}; + std::condition_variable cv; + std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers; + std::vector<std::map<std::string, std::string>> messageBobReceived = {}, + messageAliceReceived = {}; + confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::MessageReceived>( + [&](const std::string& accountId, + const std::string& /* conversationId */, + std::map<std::string, std::string> message) { + if (accountId == bobId) { + messageBobReceived.emplace_back(message); + } else { + messageAliceReceived.emplace_back(message); + } + cv.notify_one(); + })); + bool requestReceived = false; + confHandlers.insert( + DRing::exportable_callback<DRing::ConversationSignal::ConversationRequestReceived>( + [&](const std::string& /*accountId*/, + const std::string& /* conversationId */, + std::map<std::string, std::string> /*metadatas*/) { + requestReceived = true; + cv.notify_one(); + })); + bool conversationReady = false; + confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& /* conversationId */) { + if (accountId == bobId) { + conversationReady = 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; })); + + DRing::acceptConversationRequest(bobId, convId); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return conversationReady; })); + + // Assert that repository exists + auto repoPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + bobAccount->getAccountID() + + DIR_SEPARATOR_STR + "conversations" + DIR_SEPARATOR_STR + convId; + CPPUNIT_ASSERT(fileutils::isDirectory(repoPath)); + // Wait that alice sees Bob + cv.wait_for(lk, 30s, [&]() { return messageAliceReceived.size() == 2; }); + + messageBobReceived.clear(); + DRing::sendMessage(aliceId, convId, "hi"s, ""); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return messageBobReceived.size() == 1; })); + + auto validId = messageBobReceived.at(0).at("id"); + DRing::sendMessage(aliceId, convId, "foo"s, validId); + CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&]() { return messageBobReceived.size() == 2; })); + CPPUNIT_ASSERT(messageBobReceived.rbegin()->at("reply-to") == validId); + + // Check if parent doesn't exists, no message is generated + DRing::sendMessage(aliceId, convId, "foo"s, "invalid"); + CPPUNIT_ASSERT(!cv.wait_for(lk, 10s, [&]() { return messageBobReceived.size() == 3; })); +} + } // namespace test } // namespace jami