swarm: link declineConversationRequest to discardTrustRequest

Also, fix previous illformed files if declineConversationRequest
was called before this patch, some incomingTrustRequests exists
without conversation request linked.

Change-Id: Id34a6f0cfcc9df1a39ffb055b7d62975266e2399

src/client/conversation_interface.cpp

@@ -56,8 +56,7 @@ void
 declineConversationRequest(const std::string& accountId, const std::string& conversationId)
 {
     if (auto acc = jami::Manager::instance().getAccount<jami::JamiAccount>(accountId))
-        if (auto convModule = acc->convModule())
-            convModule->declineConversationRequest(conversationId);
+        acc->declineConversationRequest(conversationId);
 }
 
 bool

src/jamidht/conversation_module.cpp

@@ -1042,6 +1042,39 @@ ConversationModule::loadConversations()
         pimpl_->saveConvInfos();
 
     lk.unlock();
+
+    ////////////////////////////////////////////////////////////////
+    // Note: This is only to homogeneize trust and convRequests
+    std::vector<std::string> invalidPendingRequests;
+    {
+        std::lock_guard<std::mutex> lk(pimpl_->conversationsRequestsMtx_);
+        for (const auto& request: acc->getTrustRequests()) {
+            auto itConvId = request.find(libjami::Account::TrustRequest::CONVERSATIONID);
+            auto itConvFrom = request.find(libjami::Account::TrustRequest::FROM);
+            if (itConvId != request.end() && itConvFrom != request.end())
+            {
+                // Check if requests exists or is declined.
+                auto itReq = pimpl_->conversationsRequests_.find(itConvId->second);
+                auto declined = itReq  == pimpl_->conversationsRequests_.end() || itReq->second.declined;
+                if (declined) {
+                    JAMI_WARNING("Invalid trust request found: {:s}", itConvId->second);
+                    invalidPendingRequests.emplace_back(itConvFrom->second);
+                }
+            }
+        }
+    }
+    dht::ThreadPool::io().run([w=pimpl_->weak(), invalidPendingRequests = std::move(invalidPendingRequests)] () {
+        // Will lock account manager
+        auto shared = w.lock();
+        if (!shared)
+            return;
+        if (auto acc = shared->account_.lock()) {
+            for (const auto& invalidPendingRequest : invalidPendingRequests)
+                acc->discardTrustRequest(invalidPendingRequest);
+        }
+    });
+
+    ////////////////////////////////////////////////////////////////
     for (const auto& conv : toRm) {
         JAMI_ERROR("Remove conversation ({})", conv);
         removeConversation(conv);
@@ -1174,6 +1207,17 @@ ConversationModule::onConversationRequest(const std::string& from, const Json::V
                                                                          reqMap);
 }
 
+std::string
+ConversationModule::peerFromConversationRequest(const std::string& convId) const
+{
+    std::lock_guard<std::mutex> lk(pimpl_->conversationsRequestsMtx_);
+    auto it = pimpl_->conversationsRequests_.find(convId);
+    if (it != pimpl_->conversationsRequests_.end()) {
+        return it->second.from;
+    }
+    return {};
+}
+
 void
 ConversationModule::onNeedConversationRequest(const std::string& from,
                                               const std::string& conversationId)

src/jamidht/conversation_module.h

@@ -20,6 +20,7 @@
 
 #pragma once
 
+#include "conversation.h"
 #include "scheduled_executor.h"
 #include "jamidht/account_manager.h"
 #include "jamidht/conversation.h"
@@ -108,6 +109,13 @@ public:
      */
     void onConversationRequest(const std::string& from, const Json::Value& value);
 
+    /**
+     * Retrieve author of a conversation request
+     * @param convId    Conversation's id
+     * @return the author of the conversation request
+     */
+    std::string peerFromConversationRequest(const std::string& convId) const;
+
     /**
      * Called when a peer needs an invite for a conversation (generally after that they received
      * a commit notification for a conversation they don't have yet)

src/jamidht/jamiaccount.cpp

@@ -2885,10 +2885,29 @@ JamiAccount::discardTrustRequest(const std::string& from)
     std::lock_guard<std::recursive_mutex> lock(configurationMutex_);
     if (accountManager_)
         return accountManager_->discardTrustRequest(from);
-    JAMI_WARN("[Account %s] discardTrustRequest: account not loaded", getAccountID().c_str());
+    JAMI_WARNING("[Account {:s}] discardTrustRequest: account not loaded", getAccountID());
     return false;
 }
 
+void
+JamiAccount::declineConversationRequest(const std::string& conversationId)
+{
+    auto peerId = convModule()->peerFromConversationRequest(conversationId);
+    convModule()->declineConversationRequest(conversationId);
+    if (!peerId.empty())  {
+        std::lock_guard<std::recursive_mutex> lock(configurationMutex_);
+        if (auto info = accountManager_->getInfo()) {
+            // Verify if we have a trust request with this peer + convId
+            auto req = info->contacts->getTrustRequest(dht::InfoHash(peerId));
+            if (req.find(libjami::Account::TrustRequest::CONVERSATIONID) != req.end()
+                && req.at(libjami::Account::TrustRequest::CONVERSATIONID) == conversationId) {
+                accountManager_->discardTrustRequest(peerId);
+                JAMI_DEBUG("[Account {:s}] declined trust request with {:s}", getAccountID(), peerId);
+            }
+        }
+    }
+}
+
 void
 JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload)
 {

src/jamidht/jamiaccount.h

@@ -286,6 +286,7 @@ public:
     // Note: includeConversation used for compatibility test. Do not change
     bool acceptTrustRequest(const std::string& from, bool includeConversation = true);
     bool discardTrustRequest(const std::string& from);
+    void declineConversationRequest(const std::string& conversationId);
 
     /**
      * Add contact to the account contact list.

test/unitTest/conversation/conversationRequest.cpp

@@ -59,6 +59,8 @@ public:
     void testGetRequests();
     void testDeclineRequest();
     void testAddContact();
+    void testDeclineConversationRequestRemoveTrustRequest();
+    void testMalformedTrustRequest();
     void testAddContactDeleteAndReAdd();
     void testInviteFromMessageAfterRemoved();
     void testRemoveContact();
@@ -86,6 +88,8 @@ private:
     CPPUNIT_TEST(testGetRequests);
     CPPUNIT_TEST(testDeclineRequest);
     CPPUNIT_TEST(testAddContact);
+    CPPUNIT_TEST(testDeclineConversationRequestRemoveTrustRequest);
+    CPPUNIT_TEST(testMalformedTrustRequest);
     CPPUNIT_TEST(testAddContactDeleteAndReAdd);
     CPPUNIT_TEST(testInviteFromMessageAfterRemoved);
     CPPUNIT_TEST(testRemoveContact);
@@ -361,6 +365,129 @@ ConversationRequestTest::testAddContact()
     CPPUNIT_ASSERT(fileutils::isFile(bobMember));
 }
 
+void
+ConversationRequestTest::testDeclineConversationRequestRemoveTrustRequest()
+{
+    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<libjami::CallbackWrapperBase>> confHandlers;
+    bool conversationReady = false, requestReceived = false, memberMessageGenerated = false;
+    std::string convId = "";
+    confHandlers.insert(libjami::exportable_callback<libjami::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(libjami::exportable_callback<libjami::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(libjami::exportable_callback<libjami::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();
+            }
+        }));
+    libjami::registerSignalHandlers(confHandlers);
+    aliceAccount->addContact(bobUri);
+    aliceAccount->sendTrustRequest(bobUri, {});
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !convId.empty() && requestReceived; }));
+
+    // Decline request
+    auto requests = libjami::getConversationRequests(bobId);
+    CPPUNIT_ASSERT(requests.size() == 1);
+    auto trustRequests = libjami::getTrustRequests(bobId);
+    CPPUNIT_ASSERT(trustRequests.size() == 1);
+    libjami::declineConversationRequest(bobId, convId);
+    requests = libjami::getConversationRequests(bobId);
+    CPPUNIT_ASSERT(requests.size() == 0);
+    trustRequests = libjami::getTrustRequests(bobId);
+    CPPUNIT_ASSERT(trustRequests.size() == 0);
+}
+
+void
+ConversationRequestTest::testMalformedTrustRequest()
+{
+    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<libjami::CallbackWrapperBase>> confHandlers;
+    bool conversationReady = false, requestReceived = false, memberMessageGenerated = false;
+    std::string convId = "";
+    confHandlers.insert(libjami::exportable_callback<libjami::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(libjami::exportable_callback<libjami::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(libjami::exportable_callback<libjami::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();
+            }
+        }));
+    libjami::registerSignalHandlers(confHandlers);
+    aliceAccount->addContact(bobUri);
+    aliceAccount->sendTrustRequest(bobUri, {});
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !convId.empty() && requestReceived; }));
+
+    // Decline request
+    auto requests = libjami::getConversationRequests(bobId);
+    CPPUNIT_ASSERT(requests.size() == 1);
+    auto trustRequests = libjami::getTrustRequests(bobId);
+    CPPUNIT_ASSERT(trustRequests.size() == 1);
+    // This will let the trust request (not libjami::declineConversationRequest)
+    bobAccount->convModule()->declineConversationRequest(convId);
+    requests = libjami::getConversationRequests(bobId);
+    CPPUNIT_ASSERT(requests.size() == 0);
+    trustRequests = libjami::getTrustRequests(bobId);
+    CPPUNIT_ASSERT(trustRequests.size() == 1);
+    // Reload conversation will fix the state
+    bobAccount->convModule()->loadConversations();
+    requests = libjami::getConversationRequests(bobId);
+    CPPUNIT_ASSERT(requests.size() == 0);
+    trustRequests = libjami::getTrustRequests(bobId);
+    CPPUNIT_ASSERT(trustRequests.size() == 0);
+}
+
 void
 ConversationRequestTest::testAddContactDeleteAndReAdd()
 {