From c87d8c5f8e5c09e409e9d77405d485f8bb1c5388 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Wed, 21 Jun 2023 14:08:50 -0400
Subject: [PATCH] conversation_module: fix decline request on removeContact
erase is ok if conversation is accepted, not removed, else it cannot
be synced with other devices
Change-Id: Ia5425cb8d0dbe635b2d7db3a45de6d6b9683100d
---
src/jamidht/conversation_module.cpp | 14 ++--
.../conversation/conversationRequest.cpp | 72 +++++++++++++++++++
2 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 6cefe3dbb9..8b352e26df 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -2300,19 +2300,19 @@ ConversationModule::removeContact(const std::string& uri, bool banned)
{
std::lock_guard<std::mutex> lk(pimpl_->conversationsRequestsMtx_);
auto update = false;
- auto it = pimpl_->conversationsRequests_.begin();
- while (it != pimpl_->conversationsRequests_.end()) {
- if (it->second.from == uri) {
+ for (auto it = pimpl_->conversationsRequests_.begin(); it != pimpl_->conversationsRequests_.end(); ++it) {
+ if (it->second.from == uri && !it->second.declined) {
+ JAMI_DEBUG("Declining conversation request {:s} from {:s}", it->first, uri);
emitSignal<libjami::ConversationSignal::ConversationRequestDeclined>(
pimpl_->accountId_, it->first);
update = true;
- it = pimpl_->conversationsRequests_.erase(it);
- } else {
- ++it;
+ it->second.declined = std::time(nullptr);
}
}
- if (update)
+ if (update) {
pimpl_->saveConvRequests();
+ pimpl_->needsSyncingCb_({});
+ }
}
if (banned)
return; // Keep the conversation in banned model
diff --git a/test/unitTest/conversation/conversationRequest.cpp b/test/unitTest/conversation/conversationRequest.cpp
index 408914e965..2c4f7511f3 100644
--- a/test/unitTest/conversation/conversationRequest.cpp
+++ b/test/unitTest/conversation/conversationRequest.cpp
@@ -64,6 +64,7 @@ public:
void testAddContactDeleteAndReAdd();
void testInviteFromMessageAfterRemoved();
void testRemoveContact();
+ void testRemoveContactMultiDevice();
void testRemoveSelfDoesntRemoveConversation();
void testRemoveConversationUpdateContactDetails();
void testBanContact();
@@ -93,6 +94,7 @@ private:
CPPUNIT_TEST(testAddContactDeleteAndReAdd);
CPPUNIT_TEST(testInviteFromMessageAfterRemoved);
CPPUNIT_TEST(testRemoveContact);
+ CPPUNIT_TEST(testRemoveContactMultiDevice);
CPPUNIT_TEST(testRemoveSelfDoesntRemoveConversation);
CPPUNIT_TEST(testRemoveConversationUpdateContactDetails);
CPPUNIT_TEST(testBanContact);
@@ -715,6 +717,76 @@ ConversationRequestTest::testRemoveContact()
CPPUNIT_ASSERT(!fileutils::isDirectory(repoPath));
}
+void
+ConversationRequestTest::testRemoveContactMultiDevice()
+{
+ auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+ auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+ auto bobUri = bobAccount->getUsername();
+ auto aliceUri = aliceAccount->getUsername();
+ std::mutex mtx;
+ std::unique_lock<std::mutex> lk {mtx};
+ std::condition_variable cv;
+
+ // Add second device for Bob
+ std::map<std::string, std::shared_ptr<libjami::CallbackWrapperBase>> confHandlers;
+ auto bobArchive = std::filesystem::current_path().string() + "/bob.gz";
+ std::remove(bobArchive.c_str());
+ bobAccount->exportArchive(bobArchive);
+
+ std::map<std::string, std::string> details = libjami::getAccountTemplate("RING");
+ details[ConfProperties::TYPE] = "RING";
+ details[ConfProperties::DISPLAYNAME] = "BOB2";
+ details[ConfProperties::ALIAS] = "BOB2";
+ details[ConfProperties::UPNP_ENABLED] = "true";
+ details[ConfProperties::ARCHIVE_PASSWORD] = "";
+ details[ConfProperties::ARCHIVE_PIN] = "";
+ details[ConfProperties::ARCHIVE_PATH] = bobArchive;
+
+ bob2Id = Manager::instance().addAccount(details);
+
+ wait_for_announcement_of(bob2Id);
+ auto bob2Account = Manager::instance().getAccount<JamiAccount>(bob2Id);
+
+ bool requestB1Removed = false, requestB2Removed = false, requestB1Received = false, requestB2Received = false;
+ confHandlers.insert(
+ libjami::exportable_callback<libjami::ConfigurationSignal::IncomingTrustRequest>(
+ [&](const std::string& account_id,
+ const std::string& /*from*/,
+ const std::string& /*conversationId*/,
+ const std::vector<uint8_t>& /*payload*/,
+ time_t /*received*/) {
+ if (account_id == bobId)
+ requestB1Received = true;
+ else if (account_id == bob2Id)
+ requestB2Received = true;
+ cv.notify_one();
+ }));
+ confHandlers.insert(
+ libjami::exportable_callback<libjami::ConversationSignal::ConversationRequestDeclined>(
+ [&](const std::string& accountId, const std::string&) {
+ if (accountId == bobId) {
+ requestB1Removed = true;
+ } else if (accountId == bob2Id) {
+ requestB2Removed = true;
+ }
+ cv.notify_one();
+ }));
+ libjami::registerSignalHandlers(confHandlers);
+
+ // First, Alice adds Bob
+ aliceAccount->addContact(bobUri);
+ aliceAccount->sendTrustRequest(bobUri, {});
+ CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() {
+ return requestB1Received && requestB2Received;
+ }));
+
+ // Bob1 decline via removeContact, both device should remove the request
+ bobAccount->removeContact(aliceUri, false);
+ CPPUNIT_ASSERT(
+ cv.wait_for(lk, 30s, [&]() { return requestB1Removed && requestB2Removed; }));
+}
+
void
ConversationRequestTest::testRemoveSelfDoesntRemoveConversation()
{
--
GitLab