diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index a58442eeac5ef76b2f9108426c2b3b820c0c4159..218452b1397895394a9cf3b0851ba3c0eca103cf 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -44,6 +44,7 @@ struct PendingConversationFetch bool cloning {false}; std::string deviceId {}; std::string removeId {}; + std::map<std::string, std::string> preferences {}; std::set<std::string> connectingTo {}; }; @@ -617,6 +618,15 @@ ConversationModule::Impl::handlePendingConversation(const std::string& conversat } } } + std::map<std::string, std::string> preferences; + { + std::lock_guard<std::mutex> lk(pendingConversationsFetchMtx_); + auto itFetch = pendingConversationsFetch_.find(conversationId); + if (itFetch != pendingConversationsFetch_.end()) + preferences = std::move(itFetch->second.preferences); + } + if (!preferences.empty()) + conversation->updatePreferences(preferences); } catch (const std::exception& e) { JAMI_WARN("Something went wrong when cloning conversation: %s", e.what()); } @@ -1610,7 +1620,7 @@ ConversationModule::onSyncData(const SyncMsg& msg, // Only reclone if re-added, else the peer is not synced yet (could be offline before) continue; } - JAMI_DBG("Re-add previously removed conversation %s", convId.c_str()); + JAMI_DEBUG("Re-add previously removed conversation {:s}", convId); } pimpl_->cloneConversation(deviceId, peerId, convId, convInfo.lastDisplayed); } else { @@ -1651,19 +1661,19 @@ ConversationModule::onSyncData(const SyncMsg& msg, if (req.declined != 0) { // Request declined - JAMI_INFO("[Account %s] Declined request detected for conversation %s (device %s)", - pimpl_->accountId_.c_str(), - convId.c_str(), - deviceId.c_str()); + JAMI_LOG("[Account {:s}] Declined request detected for conversation {:s} (device {:s})", + pimpl_->accountId_, + convId, + deviceId); emitSignal<libjami::ConversationSignal::ConversationRequestDeclined>(pimpl_->accountId_, convId); continue; } - JAMI_INFO("[Account %s] New request detected for conversation %s (device %s)", - pimpl_->accountId_.c_str(), - convId.c_str(), - deviceId.c_str()); + JAMI_LOG("[Account {:s}] New request detected for conversation {:s} (device {:s})", + pimpl_->accountId_, + convId, + deviceId); emitSignal<libjami::ConversationSignal::ConversationRequestReceived>(pimpl_->accountId_, convId, @@ -1671,11 +1681,19 @@ ConversationModule::onSyncData(const SyncMsg& msg, } // Updates preferences for conversations - std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); for (const auto& [convId, p] : msg.p) { - auto itConv = pimpl_->conversations_.find(convId); - if (itConv != pimpl_->conversations_.end() && itConv->second) - itConv->second->updatePreferences(p); + { + std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); + auto itConv = pimpl_->conversations_.find(convId); + if (itConv != pimpl_->conversations_.end() && itConv->second) { + itConv->second->updatePreferences(p); + continue; + } + } + std::lock_guard<std::mutex> lk(pimpl_->pendingConversationsFetchMtx_); + auto itFetch = pimpl_->pendingConversationsFetch_.find(convId); + if (itFetch != pimpl_->pendingConversationsFetch_.end()) + itFetch->second.preferences = p; } } @@ -1731,20 +1749,20 @@ ConversationModule::onNewCommit(const std::string& peer, // 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()); + JAMI_WARNING("[Account {:s}] Could not find conversation {:s}, ask for an invite", + pimpl_->accountId_, + conversationId); pimpl_->sendMsgCb_(peer, std::map<std::string, std::string> { {"application/invite", conversationId}}, 0); return; } - JAMI_DBG("[Account %s] on new commit notification from %s, for %s, commit %s", - pimpl_->accountId_.c_str(), - peer.c_str(), - conversationId.c_str(), - commitId.c_str()); + JAMI_DEBUG("[Account {:s}] on new commit notification from {:s}, for {:s}, commit {:s}", + pimpl_->accountId_, + peer, + conversationId, + commitId); lk.unlock(); pimpl_->fetchNewCommits(peer, deviceId, conversationId, commitId); } @@ -1758,14 +1776,14 @@ ConversationModule::addConversationMember(const std::string& conversationId, // Add a new member in the conversation auto it = pimpl_->conversations_.find(conversationId); if (it == pimpl_->conversations_.end()) { - JAMI_ERR("Conversation %s doesn't exist", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} doesn't exist", conversationId); return; } if (it->second->isMember(contactUri, true)) { - JAMI_DBG("%s is already a member of %s, resend invite", - contactUri.c_str(), - conversationId.c_str()); + JAMI_DEBUG("{:s} is already a member of {:s}, resend invite", + contactUri, + conversationId); // Note: This should not be necessary, but if for whatever reason the other side didn't join // we should not forbid new invites auto invite = it->second->generateInvitation(); @@ -1863,7 +1881,7 @@ ConversationModule::updateConversationInfos(const std::string& conversationId, std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); auto it = pimpl_->conversations_.find(conversationId); if (it == pimpl_->conversations_.end()) { - JAMI_ERR("Conversation %s doesn't exist", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} doesn't exist", conversationId); return; } @@ -1872,7 +1890,7 @@ ConversationModule::updateConversationInfos(const std::string& conversationId, if (ok && sync) { pimpl_->sendMessageNotification(conversationId, commitId, true); } else if (sync) - JAMI_WARN("Couldn't update infos on %s", conversationId.c_str()); + JAMI_WARNING("Couldn't update infos on {:s}", conversationId); }); } @@ -1891,7 +1909,7 @@ ConversationModule::conversationInfos(const std::string& conversationId) const std::lock_guard<std::mutex> lkCi(pimpl_->convInfosMtx_); auto itConv = pimpl_->convInfos_.find(conversationId); if (itConv == pimpl_->convInfos_.end()) { - JAMI_ERR("Conversation %s doesn't exist", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} doesn't exist", conversationId); return {}; } return {{"syncing", "true"}}; @@ -1907,7 +1925,7 @@ ConversationModule::setConversationPreferences(const std::string& conversationId std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_); auto it = pimpl_->conversations_.find(conversationId); if (it == pimpl_->conversations_.end()) { - JAMI_ERR("Conversation %s doesn't exist", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} doesn't exist", conversationId); return; } @@ -1921,18 +1939,18 @@ ConversationModule::setConversationPreferences(const std::string& conversationId } std::map<std::string, std::string> -ConversationModule::getConversationPreferences(const std::string& conversationId) const +ConversationModule::getConversationPreferences(const std::string& conversationId, bool includeCreated) const { std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); auto it = pimpl_->conversations_.find(conversationId); if (it == pimpl_->conversations_.end() or not it->second) return {}; - return it->second->preferences(false); + return it->second->preferences(includeCreated); } std::map<std::string, std::map<std::string, std::string>> -ConversationModule::getAllConversationsPreferences() const +ConversationModule::convPreferences() const { std::map<std::string, std::map<std::string, std::string>> p; std::lock_guard<std::mutex> lk(pimpl_->conversationsMtx_); @@ -1953,7 +1971,7 @@ ConversationModule::conversationVCard(const std::string& conversationId) const // Add a new member in the conversation auto it = pimpl_->conversations_.find(conversationId); if (it == pimpl_->conversations_.end() || !it->second) { - JAMI_ERR("Conversation %s doesn't exist", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} doesn't exist", conversationId); return {}; } @@ -2111,7 +2129,7 @@ ConversationModule::call(const std::string& url, } else { auto parameters = jami::split_string(url, '/'); if (parameters.size() != 4) { - JAMI_ERR("Incorrect url %s", url.c_str()); + JAMI_ERROR("Incorrect url {:s}", url); return; } conversationId = parameters[0]; @@ -2146,7 +2164,7 @@ ConversationModule::call(const std::string& url, std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_); auto conversation = pimpl_->conversations_.find(conversationId); if (conversation == pimpl_->conversations_.end() || !conversation->second) { - JAMI_ERR("Conversation %s not found", conversationId.c_str()); + JAMI_ERROR("Conversation {:s} not found", conversationId); return; } @@ -2162,7 +2180,7 @@ ConversationModule::call(const std::string& url, if (confId != "") { sendCallRequest = true; confId = confId == "0" ? Manager::instance().callFactory.getNewCallID() : confId; - JAMI_DBG("Calling self, join conference"); + JAMI_DEBUG("Calling self, join conference"); } else if (!activeCalls.empty()) { // Else, we try to join active calls sendCallRequest = true; @@ -2170,14 +2188,14 @@ ConversationModule::call(const std::string& url, confId = ac.at("id"); uri = ac.at("uri"); deviceId = ac.at("device"); - JAMI_DBG("Calling last active call: %s", callUri.c_str()); + JAMI_DEBUG("Calling last active call: {:s}", callUri); } else if (itRdvAccount != infos.end() && itRdvDevice != infos.end()) { // Else, creates "to" (accountId/deviceId/conversationId/confId) and ask remote host sendCallRequest = true; uri = itRdvAccount->second; deviceId = itRdvDevice->second; confId = call->getCallId(); - JAMI_DBG("Remote host detected. Calling %s on device %s", uri.c_str(), deviceId.c_str()); + JAMI_DEBUG("Remote host detected. Calling {:s} on device {:s}", uri, deviceId); } if (sendCallRequest) { @@ -2193,7 +2211,7 @@ ConversationModule::call(const std::string& url, hostConference(conversationId, confId, call->getCallId()); return; } - JAMI_DBG("Calling: %s", callUri.c_str()); + JAMI_DEBUG("Calling: {:s}", callUri); sendCall(); return; } diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h index 8b9d89862732e2f10bfdb75efc62e1c56e5be88c..4735f081ee7791eefbef9de0768b5393e724d4f6 100644 --- a/src/jamidht/conversation_module.h +++ b/src/jamidht/conversation_module.h @@ -356,11 +356,11 @@ public: void setConversationPreferences(const std::string& conversationId, const std::map<std::string, std::string>& prefs); std::map<std::string, std::string> getConversationPreferences( - const std::string& conversationId) const; + const std::string& conversationId, bool includeCreated = false) const; /** * Retrieve all conversation preferences to sync with other devices */ - std::map<std::string, std::map<std::string, std::string>> getAllConversationsPreferences() const; + std::map<std::string, std::map<std::string, std::string>> convPreferences() const; // Get the map into a VCard format for storing std::vector<uint8_t> conversationVCard(const std::string& conversationId) const; diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 7340f639144543703ac383a1f63848f0ac4fca84..cc910d6b34179e930c38ccaeb14d20788b644ffd 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -2022,12 +2022,12 @@ JamiAccount::doRegister_() // Check if pull from banned device if (convModule()->isBannedDevice(conversationId, remoteDevice)) { - JAMI_WARN("[Account %s] Git server requested for conversation %s, but the " + JAMI_WARNING("[Account {:s}] Git server requested for conversation {:s}, but the " "device is " - "unauthorized (%s) ", - getAccountID().c_str(), - conversationId.c_str(), - remoteDevice.c_str()); + "unauthorized ({:s}) ", + getAccountID(), + conversationId, + remoteDevice); channel->shutdown(); return; } @@ -2038,14 +2038,13 @@ JamiAccount::doRegister_() // So it's not the server socket return; } - auto accountId = this->accountID_; - JAMI_WARN("[Account %s] Git server requested for conversation %s, device %s, " - "channel %u", - accountId.c_str(), - conversationId.c_str(), - deviceId.to_c_str(), + JAMI_WARNING("[Account {:s}] Git server requested for conversation {:s}, device {:s}, " + "channel {}", + accountID_, + conversationId, + deviceId.toString(), channel->channel()); - auto gs = std::make_unique<GitServer>(accountId, conversationId, channel); + auto gs = std::make_unique<GitServer>(accountID_, conversationId, channel); gs->setOnFetched( [w = weak(), conversationId, deviceId](const std::string& commit) { if (auto shared = w.lock()) diff --git a/src/jamidht/sync_module.cpp b/src/jamidht/sync_module.cpp index 587684e12599b866db82fb4896e93b8305e544d5..64ef3365701c8b46c1008b33d6a3466edb2b1aae 100644 --- a/src/jamidht/sync_module.cpp +++ b/src/jamidht/sync_module.cpp @@ -105,6 +105,24 @@ SyncModule::Impl::syncInfos(const std::shared_ptr<ChannelSocket>& socket, return; } } + + auto convModule = acc->convModule(); + if (!convModule) + return; + // Sync conversation's preferences + auto p = convModule->convPreferences(); + if (!p.empty()) { + SyncMsg msg; + msg.p = std::move(p); + msgpack::pack(buffer, msg); + socket->write(reinterpret_cast<const unsigned char*>(buffer.data()), buffer.size(), ec); + if (ec) { + JAMI_ERR("%s", ec.message().c_str()); + return; + } + } + buffer.clear(); + } else { msgpack::pack(buffer, *syncMsg); socket->write(reinterpret_cast<const unsigned char*>(buffer.data()), buffer.size(), ec); @@ -199,12 +217,15 @@ SyncModule::syncWith(const DeviceId& deviceId, } void -SyncModule::syncWithConnected(const std::shared_ptr<SyncMsg>& syncMsg) +SyncModule::syncWithConnected(const std::shared_ptr<SyncMsg>& syncMsg, const DeviceId& deviceId) { std::lock_guard<std::mutex> lk(pimpl_->syncConnectionsMtx_); - for (auto& [_deviceId, sockets] : pimpl_->syncConnections_) { - if (not sockets.empty()) - pimpl_->syncInfos(sockets[0], syncMsg); + for (auto& [did, sockets] : pimpl_->syncConnections_) { + if (not sockets.empty()) { + if (!deviceId || deviceId == did) { + pimpl_->syncInfos(sockets[0], syncMsg); + } + } } } } // namespace jami \ No newline at end of file diff --git a/src/jamidht/sync_module.h b/src/jamidht/sync_module.h index ea837738d6ce27570d1e10113eadbd1f73e5d3ec..ddbd1946c69457194afa51e60649b3bbc3c3de32 100644 --- a/src/jamidht/sync_module.h +++ b/src/jamidht/sync_module.h @@ -53,8 +53,9 @@ public: /** * Send sync to all connected devices * @param syncMsg Default message + * @param deviceId If we need to filter on a device */ - void syncWithConnected(const std::shared_ptr<SyncMsg>& syncMsg = nullptr); + void syncWithConnected(const std::shared_ptr<SyncMsg>& syncMsg = nullptr, const DeviceId& deviceId = {}); private: class Impl; diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index 06728b8acc47f3d48e97dbee6195c5e2749c8bfe..88ecfbf969140a0d059dcdc1e764d716ceb52aa3 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -114,6 +114,7 @@ private: void testSendReply(); void testSearchInConv(); void testConversationPreferences(); + void testConversationPreferencesBeforeClone(); void testConversationPreferencesMultiDevices(); void testFixContactDetails(); void testRemoveOneToOneNotInDetails(); @@ -163,6 +164,7 @@ private: CPPUNIT_TEST(testSendReply); CPPUNIT_TEST(testSearchInConv); CPPUNIT_TEST(testConversationPreferences); + CPPUNIT_TEST(testConversationPreferencesBeforeClone); CPPUNIT_TEST(testConversationPreferencesMultiDevices); CPPUNIT_TEST(testFixContactDetails); CPPUNIT_TEST(testRemoveOneToOneNotInDetails); @@ -3339,6 +3341,103 @@ ConversationTest::testConversationPreferences() CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return conversationRemoved; })); CPPUNIT_ASSERT(libjami::getConversationPreferences(aliceId, convId).size() == 0); } + +void +ConversationTest::testConversationPreferencesBeforeClone() +{ + 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; + auto requestReceived = false, requestReceivedBob2 = false; + confHandlers.insert( + libjami::exportable_callback<libjami::ConversationSignal::ConversationRequestReceived>( + [&](const std::string& accountId, + const std::string& conversationId, + std::map<std::string, std::string> /*metadatas*/) { + if (accountId == bobId) + requestReceived = true; + else if (accountId == bob2Id) + requestReceivedBob2 = true; + cv.notify_one(); + })); + std::string convId = ""; + auto conversationReadyBob = false, conversationReadyBob2 = false; + 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) { + conversationReadyBob = true; + } else if (accountId == bob2Id) { + conversationReadyBob2 = true; + } + cv.notify_one(); + })); + auto bob2Started = false; + confHandlers.insert( + libjami::exportable_callback<libjami::ConfigurationSignal::VolatileDetailsChanged>( + [&](const std::string& accountId, const std::map<std::string, std::string>& details) { + if (accountId == bob2Id) { + auto daemonStatus = details.at( + libjami::Account::VolatileProperties::DEVICE_ANNOUNCED); + if (daemonStatus == "true") + bob2Started = true; + } + cv.notify_one(); + })); + std::map<std::string, std::string> preferencesBob, preferencesBob2; + confHandlers.insert( + libjami::exportable_callback<libjami::ConversationSignal::ConversationPreferencesUpdated>( + [&](const std::string& accountId, + const std::string& conversationId, + std::map<std::string, std::string> preferences) { + if (accountId == bobId) + preferencesBob = preferences; + else if (accountId == bob2Id) + preferencesBob2 = preferences; + cv.notify_one(); + })); + libjami::registerSignalHandlers(confHandlers); + // Bob creates a second device + auto bobArchive = std::filesystem::current_path().string() + "/bob.gz"; + std::remove(bobArchive.c_str()); + bobAccount->exportArchive(bobArchive); + // Alice adds bob + requestReceived = false; + aliceAccount->addContact(bobUri); + aliceAccount->sendTrustRequest(bobUri, {}); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return requestReceived; })); + libjami::acceptConversationRequest(bobId, convId); + CPPUNIT_ASSERT( + cv.wait_for(lk, 30s, [&]() { return conversationReadyBob; })); + + // Set preferences + Manager::instance().sendRegister(aliceId, false); + libjami::setConversationPreferences(bobId, convId, {{"foo", "bar"}, {"bar", "foo"}}); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { + return preferencesBob.size() == 2; + })); + CPPUNIT_ASSERT(preferencesBob["foo"] == "bar" && preferencesBob["bar"] == "foo"); + + // Bob2 should sync preferences + std::map<std::string, std::string> details = libjami::getAccountTemplate("RING"); + details[ConfProperties::TYPE] = "RING"; + details[ConfProperties::DISPLAYNAME] = "BOB2"; + details[ConfProperties::ALIAS] = "BOB2"; + details[ConfProperties::UPNP_ENABLED] = "true"; + details[ConfProperties::ARCHIVE_PASSWORD] = ""; + details[ConfProperties::ARCHIVE_PIN] = ""; + details[ConfProperties::ARCHIVE_PATH] = bobArchive; + bob2Id = Manager::instance().addAccount(details); + CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bob2Started && conversationReadyBob2 && !preferencesBob2.empty(); })); + CPPUNIT_ASSERT(preferencesBob2["foo"] == "bar" && preferencesBob2["bar"] == "foo"); +} + void ConversationTest::testConversationPreferencesMultiDevices() {