diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 52a4e9189a7c155bb5c6e9b39cd19f3569bdbc2e..5e8ae81136c64221cf89b94dc5f45daf80e006fe 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1127,8 +1127,27 @@ JamiAccount::loadAccount(const std::string& archive_password, [this](const std::string& uri, bool banned) { if (!id_.first) return; - runOnMainThread([id = getAccountID(), uri, banned] { - emitSignal<DRing::ConfigurationSignal::ContactRemoved>(id, uri, banned); + + dht::ThreadPool::io().run([w = weak(), uri, banned] { + if (auto shared = w.lock()) { + // Erase linked conversation's requests + if (auto convModule = shared->convModule()) + convModule->removeContact(uri, banned); + + // Remove current connections with contact + std::unique_lock<std::mutex> lk(shared->sipConnsMtx_); + for (auto it = shared->sipConns_.begin(); it != shared->sipConns_.end();) { + const auto& [key, value] = *it; + if (key.first == uri) + it = shared->sipConns_.erase(it); + else + ++it; + } + // Update client. + emitSignal<DRing::ConfigurationSignal::ContactRemoved>(shared->getAccountID(), + uri, + banned); + } }); }, [this](const std::string& uri, @@ -2982,26 +3001,13 @@ JamiAccount::addContact(const std::string& uri, bool confirmed) void JamiAccount::removeContact(const std::string& uri, bool ban) { - std::unique_lock<std::recursive_mutex> lock(configurationMutex_); + std::lock_guard<std::recursive_mutex> lock(configurationMutex_); if (accountManager_) { accountManager_->removeContact(uri, ban); } else { JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str()); return; } - lock.unlock(); - - convModule()->removeContact(uri, ban); - - // Remove current connections with contact - std::unique_lock<std::mutex> lk(sipConnsMtx_); - for (auto it = sipConns_.begin(); it != sipConns_.end();) { - const auto& [key, value] = *it; - if (key.first == uri) - it = sipConns_.erase(it); - else - ++it; - } } bool diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp index 9eb06ba4ddc26f6d697dd502839964f3bac5d0b6..12e69db0ea7830860738f2f1829db18323619c9e 100644 --- a/test/unitTest/conversation/conversationMembersEvent.cpp +++ b/test/unitTest/conversation/conversationMembersEvent.cpp @@ -83,6 +83,7 @@ public: void testGetConversationMembersWithSelfOneOne(); void testAvoidTwoOneToOne(); void testAvoidTwoOneToOneMultiDevices(); + void testRemoveRequestBannedMultiDevices(); std::string aliceId; std::string bobId; @@ -119,6 +120,7 @@ private: CPPUNIT_TEST(testGetConversationMembersWithSelfOneOne); CPPUNIT_TEST(testAvoidTwoOneToOne); CPPUNIT_TEST(testAvoidTwoOneToOneMultiDevices); + CPPUNIT_TEST(testRemoveRequestBannedMultiDevices); CPPUNIT_TEST_SUITE_END(); }; @@ -2193,6 +2195,81 @@ ConversationMembersEventTest::testAvoidTwoOneToOneMultiDevices() cv.wait_for(lk, 30s, [&]() { return conversationReadyBob && conversationReadyBob2; })); } +void +ConversationMembersEventTest::testRemoveRequestBannedMultiDevices() +{ + 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; + std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers; + auto requestReceived = false, requestReceivedBob2 = 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 == bobId) + requestReceived = true; + else if (accountId == bob2Id) + requestReceivedBob2 = true; + cv.notify_one(); + })); + auto bob2Started = false; + confHandlers.insert( + DRing::exportable_callback<DRing::ConfigurationSignal::VolatileDetailsChanged>( + [&](const std::string& accountId, const std::map<std::string, std::string>& details) { + if (accountId == bob2Id) { + auto daemonStatus = details.at( + DRing::Account::VolatileProperties::DEVICE_ANNOUNCED); + if (daemonStatus == "true") + bob2Started = true; + } + cv.notify_one(); + })); + auto bob2ContactRemoved = false; + confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::ContactRemoved>( + [&](const std::string& accountId, const std::string& uri, bool banned) { + if (accountId == bob2Id && uri == aliceUri && banned) { + bob2ContactRemoved = true; + } + cv.notify_one(); + })); + DRing::registerSignalHandlers(confHandlers); + + // Bob creates a second device + auto bobArchive = std::filesystem::current_path().string() + "/bob.gz"; + std::remove(bobArchive.c_str()); + bobAccount->exportArchive(bobArchive); + std::map<std::string, std::string> details = DRing::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); + + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2Started; })); + + // Alice adds bob + requestReceived = false; + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return requestReceived && requestReceivedBob2; })); + CPPUNIT_ASSERT(DRing::getConversationRequests(bob2Id).size() == 1); + + // Bob bans alice, should update bob2 + bobAccount->removeContact(aliceUri, true); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2ContactRemoved; })); + CPPUNIT_ASSERT(DRing::getConversationRequests(bob2Id).size() == 0); +} + } // namespace test } // namespace jami