From dca84be45cc969ab8d1a3ef8dc2e857a4f885727 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Mon, 18 Jul 2022 15:51:17 -0400
Subject: [PATCH] swarm: add ability to answer to a message
GitLab: #318
Change-Id: I12c04096a91c40b5e6c9c435373cce23b4065d7e
---
.../cx.ring.Ring.ConfigurationManager.xml | 4 +-
bin/dbus/dbusconfigurationmanager.cpp | 12 +--
bin/dbus/dbusconfigurationmanager.h | 4 +-
bin/jni/conversation.i | 2 +-
bin/jni/datatransfer.i | 2 +-
bin/nodejs/conversation.i | 2 +-
src/client/conversation_interface.cpp | 4 +-
src/client/datatransfer.cpp | 4 +-
src/jami/conversation_interface.h | 2 +-
src/jami/datatransfer_interface.h | 2 +-
src/jamidht/conversation.cpp | 18 +++--
src/jamidht/conversation.h | 8 +-
src/jamidht/conversation_module.cpp | 21 +++---
src/jamidht/conversation_module.h | 4 +-
src/jamidht/jamiaccount.cpp | 6 +-
src/jamidht/jamiaccount.h | 2 +-
test/unitTest/conversation/conversation.cpp | 74 +++++++++++++++++++
17 files changed, 124 insertions(+), 47 deletions(-)
diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
index 15fab637c9..891b3137e3 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 b65dd58a36..ebe07adc76 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 9429a89ac7..0224aa6b89 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 2485823942..db4fc22059 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 f3bbe6517c..bc861c09fe 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 dbc6107758..7d0d94c4c4 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 a686d74a43..91adc77d0a 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 eb3ffab3c6..3adbbc059b 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 06dcaf209d..63bbdc8012 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 8ea13e74a7..ac35be225b 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 fa4dd13825..858cc82e30 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 5d180ec924..90bdbfad29 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 e789d468de..d0009f3c37 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 09f60d5c3f..2c4d380083 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 f00fc995de..f35dbf9425 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 a50a74bae0..d47867f14b 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 a309c29cdd..5876400bdb 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
--
GitLab