conversation_module: fix needsSyncingWith

If a conversation is not cloned, when detecting a new presence,
needsSyncingWith MUST return true if one conversation needs to be
cloned. Else, it will result in an infinite syncing.

Change-Id: I3a9bcab9f8a845161ceac7d75d8842f8b338ed57

src/jamidht/conversation_module.cpp

@@ -1185,10 +1185,22 @@ ConversationModule::onSyncData(const SyncMsg& msg,
 bool
 ConversationModule::needsSyncingWith(const std::string& memberUri, const std::string& deviceId) const
 {
-    std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_);
-    for (const auto& [_, conv] : pimpl_->conversations_)
-        if (conv->isMember(memberUri, false) && conv->needsFetch(deviceId))
+    // Check if a conversation needs to fetch remote or to be cloned
+    std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_);
+    std::lock_guard<std::mutex> lkCI(pimpl_->convInfosMtx_);
+    for (const auto& [key, ci] : pimpl_->convInfos_) {
+        auto it = pimpl_->conversations_.find(key);
+        if (it != pimpl_->conversations_.end() && it->second) {
+            if (!it->second->isRemoving() && it->second->isMember(memberUri, false)
+                && it->second->needsFetch(deviceId))
                 return true;
+        } else if (!ci.removed
+                   && std::find(ci.members.begin(), ci.members.end(), memberUri)
+                          != ci.members.end()) {
+            // In this case the conversation was never cloned (can be after an import)
+            return true;
+        }
+    }
     return false;
 }
 

test/unitTest/conversation/conversationRequest.cpp

@@ -68,6 +68,7 @@ public:
     void testRemoveContactRemoveSyncing();
     void testRemoveConversationRemoveSyncing();
     void testCacheRequestFromClient();
+    void testNeedsSyncingWithForCloning();
 
     std::string aliceId;
     std::string bobId;
@@ -91,6 +92,7 @@ private:
     CPPUNIT_TEST(testRemoveContactRemoveSyncing);
     CPPUNIT_TEST(testRemoveConversationRemoveSyncing);
     CPPUNIT_TEST(testCacheRequestFromClient);
+    CPPUNIT_TEST(testNeedsSyncingWithForCloning);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -980,6 +982,52 @@ ConversationRequestTest::testCacheRequestFromClient()
     CPPUNIT_ASSERT(!fileutils::isFile(cachedPath));
 }
 
+void
+ConversationRequestTest::testNeedsSyncingWithForCloning()
+{
+    auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
+    auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+    auto bobUri = bobAccount->getUsername();
+    auto aliceUri = aliceAccount->getUsername();
+    auto aliceDevice = std::string(aliceAccount->currentDeviceId());
+    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 contactAdded = 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& convId,
+            const std::vector<uint8_t>& /*payload*/,
+            time_t /*received*/) {
+            if (account_id == bobId && !convId.empty())
+                requestReceived = true;
+            cv.notify_one();
+        }));
+    confHandlers.insert(DRing::exportable_callback<DRing::ConfigurationSignal::ContactAdded>(
+        [&](const std::string& accountId, const std::string& uri, bool confirmed) {
+            if (accountId == bobId && uri == aliceUri) {
+                contactAdded = true;
+            }
+            cv.notify_one();
+        }));
+    DRing::registerSignalHandlers(confHandlers);
+    CPPUNIT_ASSERT(!bobAccount->convModule()->needsSyncingWith(aliceUri, aliceDevice));
+    aliceAccount->addContact(bobUri);
+    aliceAccount->sendTrustRequest(bobUri, {});
+    CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return requestReceived; }));
+
+    Manager::instance().sendRegister(aliceId, false); // This avoid to sync immediately
+    CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri));
+    CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(30), [&]() { return contactAdded; }));
+    // At this point the conversation should be there and syncing.
+
+    CPPUNIT_ASSERT(DRing::getConversations(bobId).size() == 1);
+    CPPUNIT_ASSERT(bobAccount->convModule()->needsSyncingWith(aliceUri, aliceDevice));
+}
+
 } // namespace test
 } // namespace jami