From 147687ad3dd45ab21cd406564a8cba090b5e3a28 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Fri, 13 Oct 2023 13:08:37 -0400
Subject: [PATCH] conversations: do not remove all syncing conversation if
 remove self

If a ContactRemoved event occured while some conversations were in
a syncing state, the conversations were removed.
This can cause random conversation deletion when importing a JAMS
account who already deleted itself as a contact.
The correct condition was just checked when the conversation is
already cloned.

Change-Id: I378f765480dcd92c7315579ba5c26a4ca50ddb23
---
 src/jamidht/conversation_module.cpp    | 31 +++++++++++++-------------
 src/jamidht/jamiaccount.cpp            |  1 -
 src/jamidht/server_account_manager.cpp | 10 ++++-----
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 8ee12157ff..e738fbb8ae 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -2404,6 +2404,19 @@ ConversationModule::removeContact(const std::string& uri, bool banned)
             pimpl_->updateConvReqCb_(convId, uri, false);
         emitSignal<libjami::ConversationSignal::ConversationRemoved>(pimpl_->accountId_, convId);
     };
+    auto removeConvInfo = [&](const auto& conv, const auto& members) {
+        if ((isSelf && members.size() == 1)
+            || (!isSelf
+                && std::find(members.begin(), members.end(), uri)
+                        != members.end())) {
+            // Mark as removed
+            conv->info.removed = std::time(nullptr);
+            updateClient(conv->info.id);
+            pimpl_->addConvInfo(conv->info);
+            return true;
+        }
+        return false;
+    };
     {
         std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_);
         for (auto& [convId, conv] : pimpl_->conversations_) {
@@ -2414,26 +2427,14 @@ ConversationModule::removeContact(const std::string& uri, bool banned)
                     // removing self can remove all conversations
                     if (conv->conversation->mode() == ConversationMode::ONE_TO_ONE) {
                         auto initMembers = conv->conversation->getInitialMembers();
-                        if ((isSelf && initMembers.size() == 1)
-                            || (!isSelf
-                                && std::find(initMembers.begin(), initMembers.end(), uri)
-                                       != initMembers.end())) {
-                            // Mark as removed
-                            conv->info.removed = std::time(nullptr);
+                        if (removeConvInfo(conv, initMembers))
                             toRm.emplace_back(convId);
-                            updateClient(convId);
-                            pimpl_->addConvInfo(conv->info);
-                        }
                     }
                 } catch (const std::exception& e) {
                     JAMI_WARN("%s", e.what());
                 }
-            } else if (std::find(conv->info.members.begin(), conv->info.members.end(), uri)
-                       != conv->info.members.end()) {
-                // It's syncing with uri, mark as removed!
-                conv->info.removed = std::time(nullptr);
-                updateClient(convId);
-                pimpl_->addConvInfo(conv->info);
+            } else {
+                removeConvInfo(conv, conv->info.members);
             }
         }
     }
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 6f6eb411ea..d76b8264a9 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -1096,7 +1096,6 @@ JamiAccount::loadAccount(const std::string& archive_password,
         [this](const std::string& uri, bool banned) {
             if (!id_.first)
                 return;
-
             dht::ThreadPool::io().run([w = weak(), uri, banned] {
                 if (auto shared = w.lock()) {
                     // Erase linked conversation's requests
diff --git a/src/jamidht/server_account_manager.cpp b/src/jamidht/server_account_manager.cpp
index c138f4fe6a..ece91945bf 100644
--- a/src/jamidht/server_account_manager.cpp
+++ b/src/jamidht/server_account_manager.cpp
@@ -395,7 +395,7 @@ ServerAccountManager::syncDevices()
                 JAMI_DBG("[Auth] Got contact sync request callback with status code=%u",
                          response.status_code);
             auto this_ = std::static_pointer_cast<ServerAccountManager>(w.lock());
-            if (!this_) return; 
+            if (!this_) return;
             if (response.status_code >= 200 && response.status_code < 300) {
                 try {
                     JAMI_WARN("[Auth] Got server response: %s", response.body.c_str());
@@ -427,7 +427,7 @@ ServerAccountManager::syncDevices()
         [w=weak_from_this()](Json::Value json, const dht::http::Response& response) {
             JAMI_DBG("[Auth] Got request callback with status code=%u", response.status_code);
             auto this_ = std::static_pointer_cast<ServerAccountManager>(w.lock());
-            if (!this_) return; 
+            if (!this_) return;
             if (response.status_code >= 200 && response.status_code < 300) {
                 try {
                     JAMI_WARN("[Auth] Got server response: %s", response.body.c_str());
@@ -467,7 +467,7 @@ ServerAccountManager::syncBlueprintConfig(SyncBlueprintCallback onSuccess)
         [syncBlueprintCallback, w=weak_from_this()](Json::Value json, const dht::http::Response& response) {
             JAMI_DEBUG("[Auth] Got sync request callback with status code={}", response.status_code);
             auto this_ = std::static_pointer_cast<ServerAccountManager>(w.lock());
-            if (!this_) return; 
+            if (!this_) return;
             if (response.status_code >= 200 && response.status_code < 300) {
                 try {
                     std::map<std::string, std::string> config;
@@ -504,7 +504,7 @@ ServerAccountManager::revokeDevice(const std::string& password,
         [cb, w=weak_from_this()](Json::Value json, const dht::http::Response& response) {
             JAMI_DBG("[Revoke] Got request callback with status code=%u", response.status_code);
             auto this_ = std::static_pointer_cast<ServerAccountManager>(w.lock());
-            if (!this_) return; 
+            if (!this_) return;
             if (response.status_code >= 200 && response.status_code < 300) {
                 try {
                     JAMI_WARN("[Revoke] Got server response");
@@ -543,7 +543,7 @@ ServerAccountManager::searchUser(const std::string& query, SearchCallback cb)
         [cb, w=weak_from_this()](Json::Value json, const dht::http::Response& response) {
             JAMI_DBG("[Search] Got request callback with status code=%u", response.status_code);
             auto this_ = std::static_pointer_cast<ServerAccountManager>(w.lock());
-            if (!this_) return; 
+            if (!this_) return;
             if (response.status_code >= 200 && response.status_code < 300) {
                 try {
                     const auto& profiles = json["profiles"];
-- 
GitLab