From d2b88b1c5a830e03a6c9ee64e91657117a782dfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Fri, 3 Sep 2021 09:57:35 -0400 Subject: [PATCH] swarm: retrieve old swarm for one to one when possible With this patch, contact left is removed in one to one. So, if a contact left, they still receive message notification and they will generate conversation requests and retrieve the old history. Moreover, if they remove and re-add a contact, they will receives a confirmation for the previous conversation and retrieve the old history. This avoid duplicates and read-only conversation. GitLab: https://git.jami.net/savoirfairelinux/ring-project/-/issues/1282 Change-Id: Id820482e82e23334632e76bf4920255efb45f4cb --- src/jamidht/account_manager.cpp | 52 ++-- src/jamidht/contact_list.cpp | 29 ++- src/jamidht/contact_list.h | 1 + src/jamidht/conversation_module.cpp | 251 +++++++++++--------- src/jamidht/conversation_module.h | 19 +- src/jamidht/jamiaccount.cpp | 39 ++- test/unitTest/conversation/conversation.cpp | 91 +++++-- 7 files changed, 312 insertions(+), 170 deletions(-) diff --git a/src/jamidht/account_manager.cpp b/src/jamidht/account_manager.cpp index cf3c1178a0..b6b821995a 100644 --- a/src/jamidht/account_manager.cpp +++ b/src/jamidht/account_manager.cpp @@ -248,27 +248,37 @@ AccountManager::startSync(const OnNewDeviceCb& cb, const OnDeviceAnnouncedCb& dc return true; // allowPublic always true for trust requests (only forbidden if banned) - onPeerMessage(*v.owner, - true, - [this, v](const std::shared_ptr<dht::crypto::Certificate>&, - dht::InfoHash peer_account) mutable { - JAMI_WARN( - "Got trust request (confirm: %u) from: %s / %s. ConversationId: %s", - v.confirm, - peer_account.toString().c_str(), - v.from.toString().c_str(), - v.conversationId.c_str()); - if (info_) - if (info_->contacts->onTrustRequest(peer_account, - v.owner, - time(nullptr), - v.confirm, - v.conversationId, - std::move(v.payload))) { - sendTrustRequestConfirm(peer_account, v.conversationId); - info_->contacts->saveTrustRequests(); - } - }); + onPeerMessage( + *v.owner, + true, + [this, v](const std::shared_ptr<dht::crypto::Certificate>&, + dht::InfoHash peer_account) mutable { + JAMI_WARN("Got trust request (confirm: %u) from: %s / %s. ConversationId: %s", + v.confirm, + peer_account.toString().c_str(), + v.from.toString().c_str(), + v.conversationId.c_str()); + if (info_) + if (info_->contacts->onTrustRequest(peer_account, + v.owner, + time(nullptr), + v.confirm, + v.conversationId, + std::move(v.payload))) { + auto conversationId = v.conversationId; + // Check if there was an old active conversation. + auto details = info_->contacts->getContactDetails(peer_account); + auto oldConvIt = details.find(DRing::Account::TrustRequest::CONVERSATIONID); + if (oldConvIt != details.end() && oldConvIt->second != "") { + if (conversationId == oldConvIt->second) + return; + conversationId = oldConvIt->second; + JAMI_WARN("Accept with old convId: %s", conversationId.c_str()); + } + sendTrustRequestConfirm(peer_account, conversationId); + info_->contacts->saveTrustRequests(); + } + }); return true; }); } diff --git a/src/jamidht/contact_list.cpp b/src/jamidht/contact_list.cpp index 351bc2b66b..dcfbb86746 100644 --- a/src/jamidht/contact_list.cpp +++ b/src/jamidht/contact_list.cpp @@ -94,6 +94,16 @@ ContactList::addContact(const dht::InfoHash& h, bool confirmed, const std::strin return true; } +void +ContactList::updateConversation(const dht::InfoHash& h, const std::string& conversationId) +{ + auto c = contacts_.find(h); + if (c != contacts_.end()) { + c->second.conversationId = conversationId; + saveContacts(); + } +} + bool ContactList::removeContact(const dht::InfoHash& h, bool ban) { @@ -404,7 +414,8 @@ ContactList::loadKnownDevices() if (not foundAccountDevice(crt, d.second.first, clock::from_time_t(d.second.second))) JAMI_WARN("[Contacts] can't add device %s", d.first.toString().c_str()); } else { - JAMI_WARN("[Contacts] can't find certificate for device %s", d.first.toString().c_str()); + JAMI_WARN("[Contacts] can't find certificate for device %s", + d.first.toString().c_str()); } } } catch (const std::exception& e) { @@ -415,8 +426,11 @@ ContactList::loadKnownDevices() std::map<dht::InfoHash, std::pair<std::string, uint64_t>> knownDevices; oh.get().convert(knownDevices); for (const auto& d : knownDevices) { - if (auto crt = tls::CertificateStore::instance().getCertificate(d.first.toString())) { - if (not foundAccountDevice(crt, d.second.first, clock::from_time_t(d.second.second))) + if (auto crt = tls::CertificateStore::instance().getCertificate( + d.first.toString())) { + if (not foundAccountDevice(crt, + d.second.first, + clock::from_time_t(d.second.second))) JAMI_WARN("[Contacts] can't add device %s", d.first.toString().c_str()); } } @@ -430,8 +444,7 @@ ContactList::loadKnownDevices() void ContactList::saveKnownDevices() const { - std::ofstream file(path_ + DIR_SEPARATOR_STR "knownDevices", - std::ios::trunc | std::ios::binary); + std::ofstream file(path_ + DIR_SEPARATOR_STR "knownDevices", std::ios::trunc | std::ios::binary); std::map<dht::PkId, std::pair<std::string, uint64_t>> devices; for (const auto& id : knownDevices_) @@ -574,9 +587,9 @@ ContactList::getSyncData() const } for (const auto& dev : knownDevices_) { - sync_data.devices.emplace(dev.second.certificate->getLongId(), KnownDeviceSync { - dev.second.name, dev.second.certificate->getId() - }); + sync_data.devices.emplace(dev.second.certificate->getLongId(), + KnownDeviceSync {dev.second.name, + dev.second.certificate->getId()}); } return sync_data; } diff --git a/src/jamidht/contact_list.h b/src/jamidht/contact_list.h index 7fddb61964..0933a86e4e 100644 --- a/src/jamidht/contact_list.h +++ b/src/jamidht/contact_list.h @@ -71,6 +71,7 @@ public: bool addContact(const dht::InfoHash&, bool confirmed = false, const std::string& conversationId = ""); + void updateConversation(const dht::InfoHash& h, const std::string& conversationId); bool setCertificateStatus(const std::string& cert_id, const tls::TrustStore::PermissionStatus status); diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index a863e3076f..1cb774a63a 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -293,47 +293,47 @@ ConversationModule::Impl::fetchNewCommits(const std::string& peer, return; } onNeedSocket_(conversationId, - deviceId, - [this, - conversationId = std::move(conversationId), - peer = std::move(peer), - deviceId = std::move(deviceId), - commitId = std::move(commitId)](const auto& channel) { - auto conversation = conversations_.find(conversationId); - auto acc = account_.lock(); - if (!channel || !acc || conversation == conversations_.end() - || !conversation->second) { - std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); - pendingConversationsFetch_.erase(conversationId); - return false; - } - acc->addGitSocket(channel->deviceId(), conversationId, channel); - conversation->second->sync( - peer, - deviceId, - [this, - conversationId = std::move(conversationId), - peer = std::move(peer), - deviceId = std::move(deviceId), - commitId = std::move(commitId)](bool ok) { - if (!ok) { - JAMI_WARN("[Account %s] Could not fetch new commit from " - "%s for %s, other " - "peer may be disconnected", - accountId_.c_str(), - deviceId.c_str(), - conversationId.c_str()); - JAMI_INFO("[Account %s] Relaunch sync with %s for %s", - accountId_.c_str(), - deviceId.c_str(), - conversationId.c_str()); - } - std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); - pendingConversationsFetch_.erase(conversationId); - }, - commitId); - return true; - }); + deviceId, + [this, + conversationId = std::move(conversationId), + peer = std::move(peer), + deviceId = std::move(deviceId), + commitId = std::move(commitId)](const auto& channel) { + auto conversation = conversations_.find(conversationId); + auto acc = account_.lock(); + if (!channel || !acc || conversation == conversations_.end() + || !conversation->second) { + std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); + pendingConversationsFetch_.erase(conversationId); + return false; + } + acc->addGitSocket(channel->deviceId(), conversationId, channel); + conversation->second->sync( + peer, + deviceId, + [this, + conversationId = std::move(conversationId), + peer = std::move(peer), + deviceId = std::move(deviceId), + commitId = std::move(commitId)](bool ok) { + if (!ok) { + JAMI_WARN("[Account %s] Could not fetch new commit from " + "%s for %s, other " + "peer may be disconnected", + accountId_.c_str(), + deviceId.c_str(), + conversationId.c_str()); + JAMI_INFO("[Account %s] Relaunch sync with %s for %s", + accountId_.c_str(), + deviceId.c_str(), + conversationId.c_str()); + } + std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); + pendingConversationsFetch_.erase(conversationId); + }, + commitId); + return true; + }); } else { if (getRequest(conversationId) != std::nullopt) return; @@ -657,6 +657,13 @@ ConversationModule::onTrustRequest(const std::string& uri, const std::vector<uint8_t>& payload, time_t received) { + if (getOneToOneConversation(uri) != "") { + // If there is already an active one to one conversation here, it's an active + // contact and the contact will reclone this activeConv, so ignore the request + JAMI_WARN("Contact is sending a request for a non active conversation. Ignore. They will " + "clone the old one"); + return; + } { std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); auto itConv = pimpl_->conversations_.find(conversationId); @@ -746,61 +753,16 @@ ConversationModule::onNeedConversationRequest(const std::string& from, void ConversationModule::acceptConversationRequest(const std::string& conversationId) { - auto acc = pimpl_->account_.lock(); // For all conversation members, try to open a git channel with this conversation ID auto request = pimpl_->getRequest(conversationId); - if (!acc || request == std::nullopt) { + if (request == std::nullopt) { JAMI_WARN("[Account %s] Request not found for conversation %s", pimpl_->accountId_.c_str(), conversationId.c_str()); return; } - auto memberHash = dht::InfoHash(request->from); - if (!memberHash) { - JAMI_WARN("Invalid member detected: %s", request->from.c_str()); - return; - } - acc->forEachDevice(memberHash, - [w = pimpl_->weak(), request = *request, conversationId]( - const std::shared_ptr<dht::crypto::PublicKey>& pk) { - auto sthis = w.lock(); - auto deviceId = pk->getLongId().toString(); - if (!sthis or deviceId == sthis->deviceId_) - return; - - if (!sthis->startFetch(conversationId, deviceId)) { - JAMI_WARN("[Account %s] Already fetching %s", - sthis->accountId_.c_str(), - conversationId.c_str()); - return; - } - sthis->onNeedSocket_( - conversationId, pk->getLongId().toString(), [=](const auto& channel) { - auto acc = sthis->account_.lock(); - std::unique_lock<std::mutex> lk( - sthis->pendingConversationsFetchMtx_); - auto& pending = sthis->pendingConversationsFetch_[conversationId]; - if (channel && !pending.ready) { - pending.ready = true; - pending.deviceId = channel->deviceId().toString(); - lk.unlock(); - // Save the git socket - acc->addGitSocket(channel->deviceId(), - conversationId, - channel); - sthis->checkConversationsEvents(); - return true; - } - return false; - }); - }); pimpl_->rmConversationRequest(conversationId); - ConvInfo info; - info.id = conversationId; - info.created = std::time(nullptr); - info.members.emplace_back(pimpl_->username_); - info.members.emplace_back(request->from); - addConvInfo(info); + cloneConversationFrom(conversationId, request->from); } void @@ -851,6 +813,53 @@ ConversationModule::startConversation(ConversationMode mode, const std::string& return convId; } +void +ConversationModule::cloneConversationFrom(const std::string& conversationId, const std::string& uri) +{ + auto acc = pimpl_->account_.lock(); + auto memberHash = dht::InfoHash(uri); + if (!acc || !memberHash) { + JAMI_WARN("Invalid member detected: %s", uri.c_str()); + return; + } + acc->forEachDevice( + memberHash, + [w = pimpl_->weak(), conversationId](const std::shared_ptr<dht::crypto::PublicKey>& pk) { + auto sthis = w.lock(); + auto deviceId = pk->getLongId().toString(); + if (!sthis or deviceId == sthis->deviceId_) + return; + + if (!sthis->startFetch(conversationId, deviceId)) { + JAMI_WARN("[Account %s] Already fetching %s", + sthis->accountId_.c_str(), + conversationId.c_str()); + return; + } + sthis->onNeedSocket_(conversationId, pk->getLongId().toString(), [=](const auto& channel) { + auto acc = sthis->account_.lock(); + std::unique_lock<std::mutex> lk(sthis->pendingConversationsFetchMtx_); + auto& pending = sthis->pendingConversationsFetch_[conversationId]; + if (channel && !pending.ready) { + pending.ready = true; + pending.deviceId = channel->deviceId().toString(); + lk.unlock(); + // Save the git socket + acc->addGitSocket(channel->deviceId(), conversationId, channel); + sthis->checkConversationsEvents(); + return true; + } + return false; + }); + }); + ConvInfo info; + info.id = conversationId; + info.created = std::time(nullptr); + info.members.emplace_back(pimpl_->username_); + info.members.emplace_back(uri); + addConvInfo(info); +} + // Message send/load void ConversationModule::sendMessage(const std::string& conversationId, @@ -1115,14 +1124,24 @@ ConversationModule::setFetched(const std::string& conversationId, const std::str void ConversationModule::onNewCommit(const std::string& peer, - const std::string& deviceId, - const std::string& conversationId, - const std::string& commitId) + const std::string& deviceId, + const std::string& conversationId, + const std::string& commitId) { std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_); auto itConv = pimpl_->convInfos_.find(conversationId); - if (itConv != pimpl_->convInfos_.end() && itConv->second.removed) - return; // ignore new commits for removed conversation + if (itConv != pimpl_->convInfos_.end() && itConv->second.removed) { + // If the conversation is removed and we receives a new commit, + // it means that the contact was removed but not banned. So we can generate + // a new trust request + JAMI_WARN("[Account %s] Could not find conversation %s, ask for an invite", + pimpl_->accountId_.c_str(), + conversationId.c_str()); + pimpl_->sendMsgCb_(peer, + std::move(std::map<std::string, std::string> { + {"application/invite", conversationId}})); + return; + } JAMI_DBG("[Account %s] on new commit notification from %s, for %s, commit %s", pimpl_->accountId_.c_str(), peer.c_str(), @@ -1318,8 +1337,14 @@ ConversationModule::removeContact(const std::string& uri, bool ban) auto initMembers = itConv->second->getInitialMembers(); if ((isSelf && initMembers.size() == 1) || std::find(initMembers.begin(), initMembers.end(), uri) - != initMembers.end()) + != initMembers.end()) { + // Mark as removed + conv.removed = std::time(nullptr); + emitSignal<DRing::ConversationSignal::ConversationRemoved>( + pimpl_->accountId_, convId); + updateConvInfos = true; toRm.emplace_back(convId); + } } } catch (const std::exception& e) { JAMI_WARN("%s", e.what()); @@ -1337,10 +1362,7 @@ ConversationModule::removeContact(const std::string& uri, bool ban) // Note, if we ban the device, we don't send the leave cause the other peer will just // never got the notifications, so just erase the datas for (const auto& id : toRm) - if (ban) - pimpl_->removeRepository(id, false, true); - else - removeConversation(id); + pimpl_->removeRepository(id, true, true); } bool @@ -1366,22 +1388,27 @@ ConversationModule::removeConversation(const std::string& conversationId) } pimpl_->saveConvInfos(); lockCi.unlock(); - auto commitId = it->second->leave(); emitSignal<DRing::ConversationSignal::ConversationRemoved>(pimpl_->accountId_, conversationId); - if (hasMembers) { - JAMI_DBG() << "Wait that someone sync that user left conversation " << conversationId; - // Commit that we left - if (!commitId.empty()) { - // Do not sync as it's synched by convInfos - pimpl_->sendMessageNotification(*it->second, commitId, false); - } else { - JAMI_ERR("Failed to send message to conversation %s", conversationId.c_str()); + if (it->second->mode() != ConversationMode::ONE_TO_ONE) { + // For one to one, we do not notify the leave. The other can still generate request + // and this is managed by the banned part. If we re-accept, the old conversation will be + // retrieven + auto commitId = it->second->leave(); + if (hasMembers) { + JAMI_DBG() << "Wait that someone sync that user left conversation " << conversationId; + // Commit that we left + if (!commitId.empty()) { + // Do not sync as it's synched by convInfos + pimpl_->sendMessageNotification(*it->second, commitId, false); + } else { + JAMI_ERR("Failed to send message to conversation %s", conversationId.c_str()); + } + // In this case, we wait that another peer sync the conversation + // to definitely remove it from the device. This is to inform the + // peer that we left the conversation and never want to receive + // any messages + return true; } - // In this case, we wait that another peer sync the conversation - // to definitely remove it from the device. This is to inform the - // peer that we left the conversation and never want to receive - // any messages - return true; } lk.unlock(); // Else we are the last member, so we can remove diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h index ff8209520e..eaeccb53a9 100644 --- a/src/jamidht/conversation_module.h +++ b/src/jamidht/conversation_module.h @@ -102,7 +102,7 @@ public: void onNeedConversationRequest(const std::string& from, const std::string& conversationId); /** - * Accepts a conversation's request + * Accept a conversation's request * @param convId */ void acceptConversationRequest(const std::string& conversationId); @@ -113,6 +113,14 @@ public: */ void declineConversationRequest(const std::string& conversationId); + /** + * Clone conversation from a member + * @note used to clone an old conversation after deleting/re-adding a contact + * @param conversationId + * @param uri + */ + void cloneConversationFrom(const std::string& conversationId, const std::string& uri); + /** * Starts a new conversation * @param mode Wanted mode @@ -216,7 +224,8 @@ public: /** * Notify that a peer fetched a commit - * @note: this definitely remove the repository when needed (when we left and someone fetched the information) + * @note: this definitely remove the repository when needed (when we left and someone fetched + * the information) * @param conversationId Related conv * @param deviceId Device who synced */ @@ -230,9 +239,9 @@ public: * @param commitId Commit to retrieve */ void onNewCommit(const std::string& peer, - const std::string& deviceId, - const std::string& conversationId, - const std::string& commitId); + const std::string& deviceId, + const std::string& conversationId, + const std::string& commitId); // Conversation's member /** diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 1dee510574..cf0ec1c6e6 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -1135,8 +1135,13 @@ JamiAccount::loadAccount(const std::string& archive_password, received); return; } - if (auto cm = convModule()) // Here account can be initializing - cm->onTrustRequest(uri, conversationId, payload, received); + // Here account can be initializing + if (auto cm = convModule()) { + auto activeConv = cm->getOneToOneConversation(uri); + if (activeConv != conversationId) { + cm->onTrustRequest(uri, conversationId, payload, received); + } + } }, [this](const std::map<DeviceId, KnownDevice>& devices) { std::map<std::string, std::string> ids; @@ -1156,13 +1161,31 @@ JamiAccount::loadAccount(const std::string& archive_password, }); }, [this](const std::string& uri, const std::string& convFromReq) { - // If we receives a confirmation of a trust request - // but without the conversation, this means that the peer is - // using an old version of Jami, without swarm support. - // In this case, delete current conversation linked with that - // contact because he will not get messages anyway. - if (convFromReq.empty()) + if (convFromReq.empty()) { + // If we receives a confirmation of a trust request + // but without the conversation, this means that the peer is + // using an old version of Jami, without swarm support. + // In this case, delete current conversation linked with that + // contact because he will not get messages anyway. convModule()->checkIfRemoveForCompat(uri); + } else { + // If we previously removed the contact, and re-add it, we may + // receive a convId different from the request. In that case, + // we need to remove the current conversation and clone the old + // one (given by convFromReq). + // TODO: In the future, we may want to re-commit the messages we + // may have send in the request we sent. + auto oldConv = convModule()->getOneToOneConversation(uri); + if (convFromReq != oldConv) { + convModule()->removeConversation(oldConv); + { + std::lock_guard<std::mutex> lock(configurationMutex_); + if (auto info = accountManager_->getInfo()) + info->contacts->updateConversation(dht::InfoHash(uri), convFromReq); + } + convModule()->cloneConversationFrom(convFromReq, uri); + } + } }}; try { diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index b407317e6e..797803a51b 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -140,6 +140,7 @@ private: void testMemberJoinsInviteRemoved(); void testAddContact(); void testAddContactDeleteAndReAdd(); + void testInviteFromMessageAfterRemoved(); void testFailAddMemberInOneToOne(); void testUnknownModeDetected(); void testRemoveContact(); @@ -201,6 +202,7 @@ private: CPPUNIT_TEST(testMemberJoinsInviteRemoved); CPPUNIT_TEST(testAddContact); CPPUNIT_TEST(testAddContactDeleteAndReAdd); + CPPUNIT_TEST(testInviteFromMessageAfterRemoved); CPPUNIT_TEST(testFailAddMemberInOneToOne); CPPUNIT_TEST(testUnknownModeDetected); CPPUNIT_TEST(testRemoveContact); @@ -3552,16 +3554,76 @@ ConversationTest::testAddContactDeleteAndReAdd() std::this_thread::sleep_for(std::chrono::seconds(5)); // wait a bit that connections are closed // re-add + CPPUNIT_ASSERT(convId != ""); + auto oldConvId = convId; + convId = ""; + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + // Should retrieve previous conversation + CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return oldConvId == convId; })); +} + +void +ConversationTest::testInviteFromMessageAfterRemoved() +{ + 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; + bool conversationReady = false, requestReceived = false, memberMessageGenerated = false; + std::string convId = ""; + 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; + cv.notify_one(); + })); + confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationReady>( + [&](const std::string& accountId, const std::string& conversationId) { + if (accountId == aliceId) { + convId = conversationId; + } else if (accountId == bobId) { + conversationReady = true; + } + cv.notify_one(); + })); + 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 == aliceId && conversationId == convId && message["type"] == "member") { + memberMessageGenerated = true; + cv.notify_one(); + } + })); + DRing::registerSignalHandlers(confHandlers); requestReceived = false; aliceAccount->addContact(bobUri); aliceAccount->sendTrustRequest(bobUri, {}); CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; })); - conversationReady = false; - memberMessageGenerated = false; - // NOTE: automatically accepted as Alice is still a contact! + CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri)); CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationReady && memberMessageGenerated; })); + + // removeContact + bobAccount->removeContact(aliceUri, false); + std::this_thread::sleep_for(std::chrono::seconds(10)); // wait a bit that connections are closed + + // bob sends a message, this should generate a new request for Alice + requestReceived = false; + DRing::sendMessage(aliceId, convId, "hi"s, ""); + CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; })); + conversationReady = false; + DRing::acceptConversationRequest(bobId, convId); + CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationReady; })); } void @@ -3690,7 +3752,8 @@ ConversationTest::testRemoveContact() std::unique_lock<std::mutex> lk {mtx}; std::condition_variable cv; std::map<std::string, std::shared_ptr<DRing::CallbackWrapperBase>> confHandlers; - bool conversationReady = false, requestReceived = false, memberMessageGenerated = false; + bool conversationReady = false, requestReceived = false, memberMessageGenerated = false, + conversationRemoved = false; std::string convId = ""; confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::IncomingTrustRequest>( [&](const std::string& account_id, @@ -3711,6 +3774,12 @@ ConversationTest::testRemoveContact() } cv.notify_one(); })); + confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationRemoved>( + [&](const std::string& accountId, const std::string& conversationId) { + if (accountId == bobId) + conversationRemoved = true; + cv.notify_one(); + })); confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::MessageReceived>( [&](const std::string& accountId, const std::string& conversationId, @@ -3733,19 +3802,9 @@ ConversationTest::testRemoveContact() return conversationReady && memberMessageGenerated; })); - memberMessageGenerated = false; + conversationRemoved = false; bobAccount->removeContact(aliceUri, false); - CPPUNIT_ASSERT( - cv.wait_for(lk, std::chrono::seconds(30), [&]() { return memberMessageGenerated; })); - - // Check that getConversationMembers return "role":"left" - auto members = DRing::getConversationMembers(aliceId, convId); - CPPUNIT_ASSERT(std::find_if(members.begin(), - members.end(), - [&](auto member) { - return member["uri"] == bobUri && member["role"] == "left"; - }) - != members.end()); + CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationRemoved; })); aliceAccount->removeContact(bobUri, false); cv.wait_for(lk, std::chrono::seconds(20)); -- GitLab