From d352fb8f193caa38be3c392e70321044bb28bc51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Thu, 28 Apr 2022 11:03:11 -0400
Subject: [PATCH] cleanup: remove downgrade to non swarm
Now that it's been months since all clients supports swarm, we
can assume that everybody is using a version compatible with
swarm now.
So, downgrading conversations to a non swarm if a DHT message
comes is not necessary and it's just unwanted code.
Change-Id: I83b83d592ea43219cbdecb31fa8b5a71b897e487
GitLab: #312
---
src/jamidht/conversation_module.cpp | 21 ----
src/jamidht/conversation_module.h | 9 --
src/jamidht/jamiaccount.cpp | 15 +--
test/unitTest/conversation/compability.cpp | 113 ---------------------
4 files changed, 1 insertion(+), 157 deletions(-)
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index bd32ff9ce5..59b108a562 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -1731,27 +1731,6 @@ ConversationModule::removeConversation(const std::string& conversationId)
return pimpl_->removeConversation(conversationId);
}
-void
-ConversationModule::checkIfRemoveForCompat(const std::string& peerUri)
-{
- auto convId = getOneToOneConversation(peerUri);
- if (convId.empty())
- return;
- std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_);
- auto it = pimpl_->conversations_.find(convId);
- if (it == pimpl_->conversations_.end()) {
- JAMI_ERR("Conversation %s doesn't exist", convId.c_str());
- return;
- }
- // We will only removes the conversation if the member is invited
- // the contact can have mutiple devices with only some with swarm
- // support, in this case, just go with recent versions.
- if (it->second->isMember(peerUri))
- return;
- lk.unlock();
- removeConversation(convId);
-}
-
void
ConversationModule::initReplay(const std::string& oldConvId, const std::string& newConvId)
{
diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h
index e0454cbd8d..89b9b7907a 100644
--- a/src/jamidht/conversation_module.h
+++ b/src/jamidht/conversation_module.h
@@ -324,15 +324,6 @@ public:
* @return if successfully removed
*/
bool removeConversation(const std::string& conversationId);
-
- /**
- * When a DHT message is coming, during swarm transition
- * check if a swarm is linked to that contact and remove
- * the swarm if needed
- * @param peerUri the one who sent a DHT message
- */
- void checkIfRemoveForCompat(const std::string& peerUri);
-
void initReplay(const std::string& oldConvId, const std::string& newConvId);
// The following methods modify what is stored on the disk
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index f35dbf9425..581983ca3f 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -1195,14 +1195,7 @@ JamiAccount::loadAccount(const std::string& archive_password,
auto requestPath = cachePath_ + DIR_SEPARATOR_STR + "requests" + DIR_SEPARATOR_STR
+ uri;
fileutils::remove(requestPath);
- 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 (!convFromReq.empty()) {
auto oldConv = convModule()->getOneToOneConversation(uri);
// If we previously removed the contact, and re-add it, we may
// receive a convId different from the request. In that case,
@@ -3677,12 +3670,6 @@ JamiAccount::handleMessage(const std::string& from, const std::pair<std::string,
}
convModule()->onConversationRequest(from, json);
return true;
- } else if (m.first == MIME_TYPE_TEXT_PLAIN) {
- // This means that a text is received, so that
- // the conversation is not a swarm. For compatibility,
- // check if we have a swarm created. It can be the case
- // when the trust request confirm was not received.
- convModule()->checkIfRemoveForCompat(from);
} else if (m.first == MIME_TYPE_IM_COMPOSING) {
try {
static const std::regex COMPOSING_REGEX("<state>\\s*(\\w+)\\s*<\\/state>");
diff --git a/test/unitTest/conversation/compability.cpp b/test/unitTest/conversation/compability.cpp
index aebd5cd4ca..7e2606f426 100644
--- a/test/unitTest/conversation/compability.cpp
+++ b/test/unitTest/conversation/compability.cpp
@@ -33,13 +33,9 @@
#include "jamidht/jamiaccount.h"
#include "../../test_runner.h"
#include "jami.h"
-#include "base64.h"
#include "fileutils.h"
#include "account_const.h"
-#include <git2.h>
-#include <filesystem>
-
using namespace std::string_literals;
using namespace DRing::Account;
@@ -59,14 +55,10 @@ public:
private:
void testIsComposing();
- void testRemoveConvOnOldTrustRequestConfirm();
- void testRemoveConvOnDhtTextMessage();
void testSendFileCompatibility();
CPPUNIT_TEST_SUITE(CompabilityTest);
CPPUNIT_TEST(testIsComposing);
- CPPUNIT_TEST(testRemoveConvOnOldTrustRequestConfirm);
- CPPUNIT_TEST(testRemoveConvOnDhtTextMessage);
CPPUNIT_TEST(testSendFileCompatibility);
CPPUNIT_TEST_SUITE_END();
};
@@ -197,7 +189,6 @@ CompabilityTest::testIsComposing()
return !convId.empty() && requestReceived;
}));
CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri, false));
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationRemoved; }));
// Send iscomposing to non swarm compatible contact
@@ -209,110 +200,6 @@ CompabilityTest::testIsComposing()
DRing::unregisterSignalHandlers();
}
-void
-CompabilityTest::testRemoveConvOnOldTrustRequestConfirm()
-{
- 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 conversationRemoved = false, requestReceived = false;
- std::string convId = "";
- confHandlers.insert(DRing::exportable_callback<DRing::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)
- 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;
- }
- cv.notify_one();
- }));
- confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationRemoved>(
- [&](const std::string& accountId, const std::string&) {
- if (accountId == aliceId) {
- conversationRemoved = true;
- }
- cv.notify_one();
- }));
- DRing::registerSignalHandlers(confHandlers);
- aliceAccount->sendTrustRequest(bobUri, {});
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(5), [&]() { return !convId.empty(); }));
- ConversationRepository repo(aliceAccount, convId);
- // Mode must be one to one
- CPPUNIT_ASSERT(repo.mode() == ConversationMode::ONE_TO_ONE);
- // Assert that repository exists
- auto repoPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + aliceAccount->getAccountID()
- + DIR_SEPARATOR_STR + "conversations" + DIR_SEPARATOR_STR + convId;
- CPPUNIT_ASSERT(fileutils::isDirectory(repoPath));
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; }));
- CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri, false));
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationRemoved; }));
-}
-
-void
-CompabilityTest::testRemoveConvOnDhtTextMessage()
-{
- 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 conversationRemoved = false, requestReceived = false;
- std::string convId = "";
- confHandlers.insert(DRing::exportable_callback<DRing::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)
- 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;
- }
- cv.notify_one();
- }));
- confHandlers.insert(DRing::exportable_callback<DRing::ConversationSignal::ConversationRemoved>(
- [&](const std::string& accountId, const std::string&) {
- if (accountId == aliceId) {
- conversationRemoved = true;
- }
- cv.notify_one();
- }));
- DRing::registerSignalHandlers(confHandlers);
- aliceAccount->sendTrustRequest(bobUri, {});
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(5), [&]() { return !convId.empty(); }));
- ConversationRepository repo(aliceAccount, convId);
- // Mode must be one to one
- CPPUNIT_ASSERT(repo.mode() == ConversationMode::ONE_TO_ONE);
- // Assert that repository exists
- auto repoPath = fileutils::get_data_dir() + DIR_SEPARATOR_STR + aliceAccount->getAccountID()
- + DIR_SEPARATOR_STR + "conversations" + DIR_SEPARATOR_STR + convId;
- CPPUNIT_ASSERT(fileutils::isDirectory(repoPath));
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; }));
- bobAccount->sendTextMessage(aliceUri, {{"text/plain", "Hi!"}});
- CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return conversationRemoved; }));
-}
-
void
CompabilityTest::testSendFileCompatibility()
{
--
GitLab