From 9b256c6868dccdacf1bab7d1cea94e81731216f5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Mon, 2 Aug 2021 13:51:09 -0400
Subject: [PATCH] conversation: add signal to detect declined requests
The API doesn't provide any signal to detect when a device
discards a conversation's request. So, there is no way to correctly
update the UI when needed. This patch introduces a signal for this.
Change-Id: I78e39c65ff1ed037ff38b92368c770f73f3e5006
GitLab: #600
---
.../cx.ring.Ring.ConfigurationManager.xml | 17 ++++
bin/dbus/dbusclient.cpp | 2 +
bin/jni/conversation.i | 2 +
bin/jni/jni_interface.i | 1 +
bin/nodejs/callback.h | 20 +++++
bin/nodejs/conversation.i | 2 +
bin/nodejs/nodejs_interface.i | 1 +
src/client/ring_signal.cpp | 1 +
src/jami/conversation_interface.h | 6 ++
src/jamidht/account_manager.cpp | 17 ++--
src/jamidht/account_manager.h | 2 +-
src/jamidht/conversation.cpp | 2 +
src/jamidht/conversation.h | 7 ++
src/jamidht/jamiaccount.cpp | 19 +++-
test/unitTest/syncHistory/syncHistory.cpp | 89 +++++++++++++++++++
15 files changed, 178 insertions(+), 10 deletions(-)
diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
index 145ab26582..0071e04706 100644
--- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
+++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
@@ -1946,6 +1946,23 @@
</arg>
</signal>
+ <signal name="conversationRequestDeclined" tp:name-for-bindings="conversationRequestDeclined">
+ <tp:added version="10.1.0"/>
+ <tp:docstring>
+ Notify clients when a new conversation request is declined
+ </tp:docstring>
+ <arg type="s" name="account_id">
+ <tp:docstring>
+ Account id related
+ </tp:docstring>
+ </arg>
+ <arg type="s" name="conversation_id">
+ <tp:docstring>
+ Conversation id
+ </tp:docstring>
+ </arg>
+ </signal>
+
<signal name="conversationReady" tp:name-for-bindings="conversationReady">
<tp:added version="10.0.0"/>
<tp:docstring>
diff --git a/bin/dbus/dbusclient.cpp b/bin/dbus/dbusclient.cpp
index 3a4b6af132..248585343f 100644
--- a/bin/dbus/dbusclient.cpp
+++ b/bin/dbus/dbusclient.cpp
@@ -303,6 +303,8 @@ DBusClient::initLibrary(int flags)
bind(&DBusConfigurationManager::messageReceived, confM, _1, _2, _3)),
exportable_callback<ConversationSignal::ConversationRequestReceived>(
bind(&DBusConfigurationManager::conversationRequestReceived, confM, _1, _2, _3)),
+ exportable_callback<ConversationSignal::ConversationRequestDeclined>(
+ bind(&DBusConfigurationManager::conversationRequestDeclined, confM, _1, _2)),
exportable_callback<ConversationSignal::ConversationReady>(
bind(&DBusConfigurationManager::conversationReady, confM, _1, _2)),
exportable_callback<ConversationSignal::ConversationRemoved>(
diff --git a/bin/jni/conversation.i b/bin/jni/conversation.i
index 7858045b84..ba31e46e9f 100644
--- a/bin/jni/conversation.i
+++ b/bin/jni/conversation.i
@@ -28,6 +28,7 @@ public:
virtual void conversationLoaded(uint32_t /* id */, const std::string& /*accountId*/, const std::string& /* conversationId */, std::vector<std::map<std::string, std::string>> /*messages*/){}
virtual void messageReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*message*/){}
virtual void conversationRequestReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*metadatas*/){}
+ virtual void conversationRequestDeclined(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationReady(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationRemoved(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationMemberEvent(const std::string& /*accountId*/, const std::string& /* conversationId */, const std::string& /* memberUri */, int /* event */){}
@@ -66,6 +67,7 @@ public:
virtual void conversationLoaded(uint32_t /* id */, const std::string& /*accountId*/, const std::string& /* conversationId */, std::vector<std::map<std::string, std::string>> /*messages*/){}
virtual void messageReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*message*/){}
virtual void conversationRequestReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*metadatas*/){}
+ virtual void conversationRequestDeclined(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationReady(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationRemoved(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationMemberEvent(const std::string& /*accountId*/, const std::string& /* conversationId */, const std::string& /* memberUri */, int /* event */){}
diff --git a/bin/jni/jni_interface.i b/bin/jni/jni_interface.i
index 5e83fcac81..010ad84cc6 100644
--- a/bin/jni/jni_interface.i
+++ b/bin/jni/jni_interface.i
@@ -320,6 +320,7 @@ void init(ConfigurationCallback* confM, Callback* callM, PresenceCallback* presM
exportable_callback<ConversationSignal::ConversationLoaded>(bind(&ConversationCallback::conversationLoaded, convM, _1, _2, _3, _4)),
exportable_callback<ConversationSignal::MessageReceived>(bind(&ConversationCallback::messageReceived, convM, _1, _2, _3)),
exportable_callback<ConversationSignal::ConversationRequestReceived>(bind(&ConversationCallback::conversationRequestReceived, convM, _1, _2, _3)),
+ exportable_callback<ConversationSignal::ConversationRequestDeclined>(bind(&ConversationCallback::conversationRequestDeclined, convM, _1, _2)),
exportable_callback<ConversationSignal::ConversationReady>(bind(&ConversationCallback::conversationReady, convM, _1, _2)),
exportable_callback<ConversationSignal::ConversationRemoved>(bind(&ConversationCallback::conversationRemoved, convM, _1, _2)),
exportable_callback<ConversationSignal::ConversationMemberEvent>(bind(&ConversationCallback::conversationMemberEvent, convM, _1, _2, _3, _4)),
diff --git a/bin/nodejs/callback.h b/bin/nodejs/callback.h
index c1da6535c2..480a7c88f9 100644
--- a/bin/nodejs/callback.h
+++ b/bin/nodejs/callback.h
@@ -30,6 +30,7 @@ Persistent<Function> incomingCallWithMediaCb;
Persistent<Function> conversationLoadedCb;
Persistent<Function> messageReceivedCb;
Persistent<Function> conversationRequestReceivedCb;
+Persistent<Function> conversationRequestDeclinedCb;
Persistent<Function> conversationReadyCb;
Persistent<Function> conversationRemovedCb;
Persistent<Function> conversationMemberEventCb;
@@ -93,6 +94,8 @@ getPresistentCb(std::string_view signal)
return &conversationRemovedCb;
else if (signal == "ConversationRequestReceived")
return &conversationRequestReceivedCb;
+ else if (signal == "ConversationRequestDeclined")
+ return &conversationRequestDeclinedCb;
else if (signal == "ConversationMemberEvent")
return &conversationMemberEventCb;
else if (signal == "OnConversationError")
@@ -569,6 +572,23 @@ conversationRequestReceived(const std::string& accountId, const std::string& con
uv_async_send(&signalAsync);
}
+void
+conversationRequestDeclined(const std::string& accountId, const std::string& conversationId)
+{
+ std::lock_guard<std::mutex> lock(pendingSignalsLock);
+ pendingSignals.emplace([accountId, conversationId, message]() {
+ Local<Function> func = Local<Function>::New(Isolate::GetCurrent(), conversationRequestDeclinedCb);
+ if (!func.IsEmpty()) {
+ SWIGV8_VALUE callback_args[] = {
+ V8_STRING_NEW_LOCAL(accountId),
+ V8_STRING_NEW_LOCAL(conversationId)
+ };
+ func->Call(SWIGV8_CURRENT_CONTEXT(), SWIGV8_NULL(), 2, callback_args);
+ }
+ });
+ uv_async_send(&signalAsync);
+}
+
void
conversationReady(const std::string& accountId, const std::string& conversationId)
{
diff --git a/bin/nodejs/conversation.i b/bin/nodejs/conversation.i
index 45605ae27b..5a143ad151 100644
--- a/bin/nodejs/conversation.i
+++ b/bin/nodejs/conversation.i
@@ -28,6 +28,7 @@ public:
virtual void conversationLoaded(uint32_t /* id */, const std::string& /*accountId*/, const std::string& /* conversationId */, std::vector<std::map<std::string, std::string>> /*messages*/){}
virtual void messageReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*message*/){}
virtual void conversationRequestReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*metadatas*/){}
+ virtual void conversationRequestDeclined(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationReady(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationRemoved(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationMemberEvent(const std::string& /*accountId*/, const std::string& /* conversationId */, const std::string& /* memberUri */, int /* event */){}
@@ -67,6 +68,7 @@ public:
virtual void conversationLoaded(uint32_t /* id */, const std::string& /*accountId*/, const std::string& /* conversationId */, std::vector<std::map<std::string, std::string>> /*messages*/){}
virtual void messageReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*message*/){}
virtual void conversationRequestReceived(const std::string& /*accountId*/, const std::string& /* conversationId */, std::map<std::string, std::string> /*metadatas*/){}
+ virtual void conversationRequestDeclined(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationReady(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationRemoved(const std::string& /*accountId*/, const std::string& /* conversationId */){}
virtual void conversationMemberEvent(const std::string& /*accountId*/, const std::string& /* conversationId */, const std::string& /* memberUri */, int /* event */){}
diff --git a/bin/nodejs/nodejs_interface.i b/bin/nodejs/nodejs_interface.i
index d64d875ee7..1dd9511dc8 100644
--- a/bin/nodejs/nodejs_interface.i
+++ b/bin/nodejs/nodejs_interface.i
@@ -148,6 +148,7 @@ void init(const SWIGV8_VALUE& funcMap){
exportable_callback<ConversationSignal::ConversationLoaded>(bind(&conversationLoaded, _1, _2, _3, _4)),
exportable_callback<ConversationSignal::MessageReceived>(bind(&messageReceived, _1, _2, _3)),
exportable_callback<ConversationSignal::ConversationRequestReceived>(bind(&conversationRequestReceived, _1, _2, _3)),
+ exportable_callback<ConversationSignal::ConversationRequestDeclined>(bind(&conversationRequestDeclined, _1, _2)),
exportable_callback<ConversationSignal::ConversationReady>(bind(&conversationReady, _1, _2)),
exportable_callback<ConversationSignal::ConversationRemoved>(bind(&conversationRemoved, _1, _2)),
exportable_callback<ConversationSignal::ConversationMemberEvent>(bind(&conversationMemberEvent, _1, _2, _3, _4)),
diff --git a/src/client/ring_signal.cpp b/src/client/ring_signal.cpp
index a306c07654..dcccdedbd7 100644
--- a/src/client/ring_signal.cpp
+++ b/src/client/ring_signal.cpp
@@ -131,6 +131,7 @@ getSignalHandlers()
exported_callback<DRing::ConversationSignal::ConversationLoaded>(),
exported_callback<DRing::ConversationSignal::MessageReceived>(),
exported_callback<DRing::ConversationSignal::ConversationRequestReceived>(),
+ exported_callback<DRing::ConversationSignal::ConversationRequestDeclined>(),
exported_callback<DRing::ConversationSignal::ConversationReady>(),
exported_callback<DRing::ConversationSignal::ConversationRemoved>(),
exported_callback<DRing::ConversationSignal::ConversationMemberEvent>(),
diff --git a/src/jami/conversation_interface.h b/src/jami/conversation_interface.h
index ada5792746..69890a9a54 100644
--- a/src/jami/conversation_interface.h
+++ b/src/jami/conversation_interface.h
@@ -98,6 +98,12 @@ struct DRING_PUBLIC ConversationSignal
const std::string& /* conversationId */,
std::map<std::string, std::string> /*metadatas*/);
};
+ struct DRING_PUBLIC ConversationRequestDeclined
+ {
+ constexpr static const char* name = "ConversationRequestDeclined";
+ using cb_type = void(const std::string& /*accountId*/,
+ const std::string& /* conversationId */);
+ };
struct DRING_PUBLIC ConversationReady
{
constexpr static const char* name = "ConversationReady";
diff --git a/src/jamidht/account_manager.cpp b/src/jamidht/account_manager.cpp
index 0272f6c558..ec8a9494c0 100644
--- a/src/jamidht/account_manager.cpp
+++ b/src/jamidht/account_manager.cpp
@@ -532,14 +532,22 @@ AccountManager::getRequest(const std::string& id) const
return std::nullopt;
}
-void
+bool
AccountManager::addConversationRequest(const std::string& id, const ConversationRequest& req)
{
if (info_) {
std::lock_guard<std::mutex> lk(conversationsRequestsMtx);
+ auto it = info_->conversationsRequests.find(id);
+ if (it != info_->conversationsRequests.end()) {
+ // Check if updated
+ if (req == it->second)
+ return false;
+ }
info_->conversationsRequests[id] = req;
saveConvRequests();
+ return true;
}
+ return false;
}
void
@@ -724,7 +732,8 @@ AccountManager::forEachDevice(
struct State
{
- unsigned remaining {1}; // Note: state is initialized to 1, because we need to wait that the get is finished
+ unsigned remaining {
+ 1}; // Note: state is initialized to 1, because we need to wait that the get is finished
std::set<dht::PkId> treatedDevices {};
std::function<void(const std::shared_ptr<dht::crypto::PublicKey>&)> onDevice;
std::function<void(bool)> onEnd;
@@ -767,9 +776,7 @@ AccountManager::forEachDevice(
});
return true;
},
- [state](bool /*ok*/) {
- state->found({});
- });
+ [state](bool /*ok*/) { state->found({}); });
}
void
diff --git a/src/jamidht/account_manager.h b/src/jamidht/account_manager.h
index 0fb6177b69..1fe1fc379f 100644
--- a/src/jamidht/account_manager.h
+++ b/src/jamidht/account_manager.h
@@ -231,7 +231,7 @@ public:
void addConversation(const ConvInfo& info);
void setConversationsRequests(const std::map<std::string, ConversationRequest>& newConvReq);
std::optional<ConversationRequest> getRequest(const std::string& id) const;
- void addConversationRequest(const std::string& id, const ConversationRequest& req);
+ bool addConversationRequest(const std::string& id, const ConversationRequest& req);
void rmConversationRequest(const std::string& id);
mutable std::mutex conversationsRequestsMtx;
diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp
index b802bd87d3..3e873b002a 100644
--- a/src/jamidht/conversation.cpp
+++ b/src/jamidht/conversation.cpp
@@ -103,6 +103,8 @@ ConversationRequest::toMap() const
auto result = metadatas;
result["id"] = conversationId;
result["from"] = from;
+ if (declined)
+ result["declined"] = std::to_string(declined);
result["received"] = std::to_string(received);
return result;
}
diff --git a/src/jamidht/conversation.h b/src/jamidht/conversation.h
index fa0fa2047e..80555c666a 100644
--- a/src/jamidht/conversation.h
+++ b/src/jamidht/conversation.h
@@ -52,6 +52,13 @@ struct ConversationRequest
Json::Value toJson() const;
std::map<std::string, std::string> toMap() const;
+ bool operator==(const ConversationRequest& o) const
+ {
+ auto m = toMap();
+ auto om = o.toMap();
+ return m.size() == om.size() && std::equal(m.begin(), m.end(), om.begin());
+ }
+
MSGPACK_DEFINE_MAP(from, conversationId, metadatas, received, declined)
};
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 566427027c..5910a1c50d 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -3888,6 +3888,8 @@ JamiAccount::declineConversationRequest(const std::string& conversationId)
return;
request->declined = std::time(nullptr);
accountManager_->addConversationRequest(conversationId, std::move(*request));
+ emitSignal<DRing::ConversationSignal::ConversationRequestDeclined>(getAccountID(),
+ conversationId);
syncWithConnected();
}
@@ -5037,10 +5039,19 @@ JamiAccount::cacheSyncConnection(std::shared_ptr<ChannelSocket>&& socket,
}
// New request
- accountManager_->addConversationRequest(convId, req);
-
- if (req.declined != 0)
- continue; // Request removed, do not emit signal
+ if (!accountManager_->addConversationRequest(convId, req))
+ continue; // Already added
+
+ if (req.declined != 0) {
+ // Request declined
+ JAMI_INFO("[Account %s] Declined request detected for conversation %s (device %s)",
+ getAccountID().c_str(),
+ convId.c_str(),
+ deviceId.to_c_str());
+ emitSignal<DRing::ConversationSignal::ConversationRequestDeclined>(getAccountID(),
+ convId);
+ continue;
+ }
JAMI_INFO("[Account %s] New request detected for conversation %s (device %s)",
getAccountID().c_str(),
diff --git a/test/unitTest/syncHistory/syncHistory.cpp b/test/unitTest/syncHistory/syncHistory.cpp
index bc4dae93ee..be8e461671 100644
--- a/test/unitTest/syncHistory/syncHistory.cpp
+++ b/test/unitTest/syncHistory/syncHistory.cpp
@@ -68,6 +68,7 @@ private:
void testSyncCreateAccountExportDeleteReimportWithConvId();
void testSyncCreateAccountExportDeleteReimportWithConvReq();
void testSyncOneToOne();
+ void testConversationRequestRemoved();
CPPUNIT_TEST_SUITE(SyncHistoryTest);
CPPUNIT_TEST(testCreateConversationThenSync);
@@ -80,6 +81,7 @@ private:
CPPUNIT_TEST(testSyncCreateAccountExportDeleteReimportWithConvId);
CPPUNIT_TEST(testSyncCreateAccountExportDeleteReimportWithConvReq);
CPPUNIT_TEST(testSyncOneToOne);
+ CPPUNIT_TEST(testConversationRequestRemoved);
CPPUNIT_TEST_SUITE_END();
};
@@ -862,6 +864,93 @@ SyncHistoryTest::testSyncOneToOne()
std::remove(aliceArchive.c_str());
}
+void
+SyncHistoryTest::testConversationRequestRemoved()
+{
+ auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+
+ // Export alice
+ auto aliceArchive = std::filesystem::current_path().string() + "/alice.gz";
+ std::remove(aliceArchive.c_str());
+ aliceAccount->exportArchive(aliceArchive);
+
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto uri = aliceAccount->getUsername();
+
+ // Start conversation for Alice
+ auto convId = bobAccount->startConversation();
+
+ // Check that alice receives the request
+ std::mutex mtx;
+ std::unique_lock<std::mutex> lk {mtx};
+ std::condition_variable cv;
+ std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers;
+ auto 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*/) {
+ if (accountId == aliceId && conversationId == convId) {
+ requestReceived = true;
+ cv.notify_one();
+ }
+ }));
+ DRing::registerSignalHandlers(confHandlers);
+
+ bobAccount->addConversationMember(convId, uri);
+ CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&] { return requestReceived; }));
+ DRing::unregisterSignalHandlers();
+ confHandlers.clear();
+
+ // Now create alice2
+ std::map<std::string, std::string> details = DRing::getAccountTemplate("RING");
+ details[ConfProperties::TYPE] = "RING";
+ details[ConfProperties::DISPLAYNAME] = "ALICE2";
+ details[ConfProperties::ALIAS] = "ALICE2";
+ details[ConfProperties::UPNP_ENABLED] = "true";
+ details[ConfProperties::ARCHIVE_PASSWORD] = "";
+ details[ConfProperties::ARCHIVE_PIN] = "";
+ details[ConfProperties::ARCHIVE_PATH] = aliceArchive;
+ alice2Id = Manager::instance().addAccount(details);
+
+ requestReceived = false;
+ bool requestDeclined = false, requestDeclined2 = false;
+ confHandlers.insert(
+ DRing::exportable_callback<DRing::ConversationSignal::ConversationRequestReceived>(
+ [&](const std::string& accountId,
+ const std::string& conversationId,
+ std::map<std::string, std::string> /*metadatas*/) {
+ if (accountId == alice2Id && conversationId == convId) {
+ requestReceived = true;
+ cv.notify_one();
+ }
+ }));
+ confHandlers.insert(
+ DRing::exportable_callback<DRing::ConversationSignal::ConversationRequestDeclined>(
+ [&](const std::string& accountId, const std::string& conversationId) {
+ if (conversationId != convId)
+ return;
+ if (accountId == aliceId)
+ requestDeclined = true;
+ if (accountId == alice2Id)
+ requestDeclined2 = true;
+ cv.notify_one();
+ }));
+ DRing::registerSignalHandlers(confHandlers);
+
+ CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&] { return requestReceived; }));
+ // Now decline trust request, this should trigger ConversationRequestDeclined both sides for Alice
+ aliceAccount->declineConversationRequest(convId);
+
+ CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&] {
+ return requestDeclined && requestDeclined2;
+ }));
+
+ DRing::unregisterSignalHandlers();
+ std::remove(aliceArchive.c_str());
+}
+
} // namespace test
} // namespace jami
--
GitLab