From 397bf76c87c56bec83151b7ed5c6442d0e2024c6 Mon Sep 17 00:00:00 2001
From: Kateryna Kostiuk <kateryna.kostiuk@savoirfairelinux.com>
Date: Wed, 24 May 2023 11:43:24 -0400
Subject: [PATCH] connectivity: remove sync in device announcement

It is unnecessary because the device performs synchronization
during the bootstrap. Moreover, it could result in the
failure to receive notifications on iOS because iOS does not
respond to sync connections in the background. However, the
other side may consider it as already connected.

https://git.jami.net/savoirfairelinux/jami-client-ios/-/issues/281

Change-Id: Id55e7bfaafba1e45a6c27afdebf682c33acd9d30
---
 src/jamidht/jamiaccount.cpp | 258 ++++++++++++++++++------------------
 1 file changed, 129 insertions(+), 129 deletions(-)

diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 9fedaf5f5e..8e0b13f5be 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -291,8 +291,7 @@ JamiAccount::JamiAccount(const std::string& accountId)
     , connectionManager_ {}
     , nonSwarmTransferManager_(std::make_shared<TransferManager>(accountId, ""))
     , certStore_ {std::make_unique<tls::CertificateStore>(accountId)}
-{
-}
+{}
 
 JamiAccount::~JamiAccount() noexcept
 {
@@ -347,8 +346,8 @@ JamiAccount::newIncomingCall(const std::string& from,
 
     if (sipTransp) {
         auto call = Manager::instance().callFactory.newSipCall(shared(),
-                                                                Call::CallType::INCOMING,
-                                                                mediaList);
+                                                               Call::CallType::INCOMING,
+                                                               mediaList);
         call->setPeerUri(JAMI_URI_PREFIX + from);
         call->setPeerNumber(from);
 
@@ -1077,14 +1076,16 @@ JamiAccount::loadAccount(const std::string& archive_password,
             if (!id_.first)
                 return;
             if (jami::Manager::instance().syncOnRegister) {
-                dht::ThreadPool::io().run([w=weak(), uri, confirmed] {
+                dht::ThreadPool::io().run([w = weak(), uri, confirmed] {
                     if (auto shared = w.lock()) {
                         if (auto cm = shared->convModule()) {
                             auto activeConv = cm->getOneToOneConversation(uri);
                             if (!activeConv.empty())
                                 cm->bootstrap(activeConv);
                         }
-                        emitSignal<libjami::ConfigurationSignal::ContactAdded>(shared->getAccountID(), uri, confirmed);
+                        emitSignal<libjami::ConfigurationSignal::ContactAdded>(shared->getAccountID(),
+                                                                               uri,
+                                                                               confirmed);
                     }
                 });
             }
@@ -1120,11 +1121,8 @@ JamiAccount::loadAccount(const std::string& archive_password,
                     shared->clearProfileCache(uri);
                     if (conversationId.empty()) {
                         // Old path
-                        emitSignal<libjami::ConfigurationSignal::IncomingTrustRequest>(shared->getAccountID(),
-                                                                                       conversationId,
-                                                                                       uri,
-                                                                                       payload,
-                                                                                       received);
+                        emitSignal<libjami::ConfigurationSignal::IncomingTrustRequest>(
+                            shared->getAccountID(), conversationId, uri, payload, received);
                         return;
                     }
                     // Here account can be initializing
@@ -1158,8 +1156,8 @@ JamiAccount::loadAccount(const std::string& archive_password,
             dht::ThreadPool::io().run([w = weak(), convFromReq, uri] {
                 if (auto shared = w.lock()) {
                     // Remove cached payload if there is one
-                    auto requestPath = shared->cachePath_ + DIR_SEPARATOR_STR + "requests" + DIR_SEPARATOR_STR
-                    + uri;
+                    auto requestPath = shared->cachePath_ + DIR_SEPARATOR_STR + "requests"
+                                       + DIR_SEPARATOR_STR + uri;
                     fileutils::remove(requestPath);
                     if (!convFromReq.empty()) {
                         auto oldConv = shared->convModule()->getOneToOneConversation(uri);
@@ -1169,7 +1167,8 @@ JamiAccount::loadAccount(const std::string& archive_password,
                         // one (given by convFromReq).
                         // TODO: In the future, we may want to re-commit the messages we
                         // may have send in the request we sent.
-                        if (oldConv != convFromReq && shared->updateConvForContact(uri, oldConv, convFromReq)) {
+                        if (oldConv != convFromReq
+                            && shared->updateConvForContact(uri, oldConv, convFromReq)) {
                             shared->convModule()->initReplay(oldConv, convFromReq);
                             shared->convModule()->cloneConversationFrom(convFromReq, uri, oldConv);
                         }
@@ -1648,52 +1647,40 @@ JamiAccount::trackPresence(const dht::InfoHash& h, BuddyInfo& buddy)
     if (not dht or not dht->isRunning()) {
         return;
     }
-    buddy.listenToken = dht->listen<
-        DeviceAnnouncement>(h, [this, h](DeviceAnnouncement&& dev, bool expired) {
-        bool wasConnected, isConnected;
-        {
-            std::lock_guard<std::mutex> lock(buddyInfoMtx);
-            auto buddy = trackedBuddies_.find(h);
-            if (buddy == trackedBuddies_.end())
-                return true;
-            wasConnected = buddy->second.devices_cnt > 0;
-            if (expired)
-                --buddy->second.devices_cnt;
-            else
-                ++buddy->second.devices_cnt;
-            isConnected = buddy->second.devices_cnt > 0;
-        }
-        // NOTE: the rest can use configurationMtx_, that can be locked during unregister so
-        // do not retrigger on dht
-        runOnMainThread([w = weak(), h, dev, expired, isConnected, wasConnected]() {
-            auto sthis = w.lock();
-            if (!sthis)
-                return;
-            if (not expired) {
-                // Retry messages every time a new device announce its presence
-                sthis->messageEngine_.onPeerOnline(h.toString());
-                sthis->findCertificate(
-                    dev.dev, [sthis, h](const std::shared_ptr<dht::crypto::Certificate>& cert) {
-                        if (cert) {
-                            auto pk = cert->getSharedPublicKey();
-                            if (sthis->convModule()->needsSyncingWith(h.toString(),
-                                                                      pk->getLongId().toString()))
-                                sthis->requestSIPConnection(
-                                    h.toString(),
-                                    pk->getLongId(),
-                                    "sync"); // Both sides will sync conversations
-                        }
-                    });
-            }
-            if (isConnected and not wasConnected) {
-                sthis->onTrackedBuddyOnline(h);
-            } else if (not isConnected and wasConnected) {
-                sthis->onTrackedBuddyOffline(h);
-            }
-        });
-
-        return true;
-    });
+    buddy.listenToken
+        = dht->listen<DeviceAnnouncement>(h, [this, h](DeviceAnnouncement&& dev, bool expired) {
+              bool wasConnected, isConnected;
+              {
+                  std::lock_guard<std::mutex> lock(buddyInfoMtx);
+                  auto buddy = trackedBuddies_.find(h);
+                  if (buddy == trackedBuddies_.end())
+                      return true;
+                  wasConnected = buddy->second.devices_cnt > 0;
+                  if (expired)
+                      --buddy->second.devices_cnt;
+                  else
+                      ++buddy->second.devices_cnt;
+                  isConnected = buddy->second.devices_cnt > 0;
+              }
+              // NOTE: the rest can use configurationMtx_, that can be locked during unregister so
+              // do not retrigger on dht
+              runOnMainThread([w = weak(), h, dev, expired, isConnected, wasConnected]() {
+                  auto sthis = w.lock();
+                  if (!sthis)
+                      return;
+                  if (not expired) {
+                      // Retry messages every time a new device announce its presence
+                      sthis->messageEngine_.onPeerOnline(h.toString());
+                  }
+                  if (isConnected and not wasConnected) {
+                      sthis->onTrackedBuddyOnline(h);
+                  } else if (not isConnected and wasConnected) {
+                      sthis->onTrackedBuddyOffline(h);
+                  }
+              });
+
+              return true;
+          });
 }
 
 std::map<std::string, bool>
@@ -1822,7 +1809,12 @@ JamiAccount::doRegister_()
         if (not config.proxy_server.empty()) {
             JAMI_LOG("[Account {}] using proxy server {}", getAccountID(), config.proxy_server);
             if (not config.push_token.empty()) {
-                JAMI_LOG("[Account {}] using push notifications with platform: {}, topic: {}, token: {}", getAccountID(), config.push_platform, config.push_topic, config.push_token);
+                JAMI_LOG(
+                    "[Account {}] using push notifications with platform: {}, topic: {}, token: {}",
+                    getAccountID(),
+                    config.push_platform,
+                    config.push_topic,
+                    config.push_token);
             }
         }
 
@@ -1916,46 +1908,47 @@ JamiAccount::doRegister_()
             if (!ok)
                 return;
             accountManager_->startSync(
-                                       [this](const std::shared_ptr<dht::crypto::Certificate>& crt) {
-                                           if (jami::Manager::instance().syncOnRegister) {
-                                               if (!crt)
-                                                   return;
-                                               auto deviceId = crt->getLongId().toString();
-                                               if (accountManager_->getInfo()->deviceId == deviceId)
-                                                   return;
-
-                                               std::unique_lock<std::mutex> lk(connManagerMtx_);
-                                               initConnectionManager();
-                                               channelHandlers_[Uri::Scheme::SYNC]
-                                               ->connect(crt->getLongId(),
-                                                         "",
-                                                         [this](std::shared_ptr<ChannelSocket> socket,
-                                                                const DeviceId& deviceId) {
-                                                   if (socket)
-                                                       syncModule()->syncWith(deviceId, socket);
-                                               });
-                                               lk.unlock();
-                                               requestSIPConnection(
-                                                                    getUsername(),
-                                                                    crt->getLongId(),
-                                                                    "sync"); // For git notifications, will use the same socket as sync
-                                           }
-                                       },
-                                       [this] {
-                                           if (jami::Manager::instance().syncOnRegister) {
-                                               deviceAnnounced_ = true;
-
-                                               // Bootstrap at the end to avoid to be long to load.
-                                               dht::ThreadPool::io().run([w = weak()] {
-                                                   if (auto shared = w.lock()) {
-                                                       std::lock_guard<std::recursive_mutex> lock(shared->configurationMutex_);
-                                                       shared->convModule()->bootstrap();
-                                                   }
-                                               });
-                                               emitSignal<libjami::ConfigurationSignal::VolatileDetailsChanged>(
-                                                                                                                accountID_, getVolatileAccountDetails());
-                                           }
-                                       });
+                [this](const std::shared_ptr<dht::crypto::Certificate>& crt) {
+                    if (jami::Manager::instance().syncOnRegister) {
+                        if (!crt)
+                            return;
+                        auto deviceId = crt->getLongId().toString();
+                        if (accountManager_->getInfo()->deviceId == deviceId)
+                            return;
+
+                        std::unique_lock<std::mutex> lk(connManagerMtx_);
+                        initConnectionManager();
+                        channelHandlers_[Uri::Scheme::SYNC]
+                            ->connect(crt->getLongId(),
+                                      "",
+                                      [this](std::shared_ptr<ChannelSocket> socket,
+                                             const DeviceId& deviceId) {
+                                          if (socket)
+                                              syncModule()->syncWith(deviceId, socket);
+                                      });
+                        lk.unlock();
+                        requestSIPConnection(
+                            getUsername(),
+                            crt->getLongId(),
+                            "sync"); // For git notifications, will use the same socket as sync
+                    }
+                },
+                [this] {
+                    if (jami::Manager::instance().syncOnRegister) {
+                        deviceAnnounced_ = true;
+
+                        // Bootstrap at the end to avoid to be long to load.
+                        dht::ThreadPool::io().run([w = weak()] {
+                            if (auto shared = w.lock()) {
+                                std::lock_guard<std::recursive_mutex> lock(
+                                    shared->configurationMutex_);
+                                shared->convModule()->bootstrap();
+                            }
+                        });
+                        emitSignal<libjami::ConfigurationSignal::VolatileDetailsChanged>(
+                            accountID_, getVolatileAccountDetails());
+                    }
+                });
         };
 
         setRegistrationState(RegistrationState::TRYING);
@@ -2213,7 +2206,9 @@ JamiAccount::convModule()
                         Manager::instance().ioContext()->post([cb] { cb({}); });
                         return;
                     }
-                    if (!shared->connectionManager_->isConnecting(DeviceId(deviceId), fmt::format("swarm://{}", convId))) {
+                    if (!shared->connectionManager_->isConnecting(DeviceId(deviceId),
+                                                                  fmt::format("swarm://{}",
+                                                                              convId))) {
                         shared->connectionManager_->connectDevice(
                             DeviceId(deviceId),
                             fmt::format("swarm://{}", convId),
@@ -2225,7 +2220,8 @@ JamiAccount::convModule()
                                         return;
                                     auto remoteCert = socket->peerCertificate();
                                     auto uri = remoteCert->issuer->getId().toString();
-                                    if (shared->accountManager()->getCertificateStatus(uri) == tls::TrustStore::PermissionStatus::BANNED) {
+                                    if (shared->accountManager()->getCertificateStatus(uri)
+                                        == tls::TrustStore::PermissionStatus::BANNED) {
                                         cb(nullptr);
                                         return;
                                     }
@@ -2429,10 +2425,11 @@ JamiAccount::setCertificateStatus(const std::string& cert_id,
 
 bool
 JamiAccount::setCertificateStatus(const std::shared_ptr<crypto::Certificate>& cert,
-                              tls::TrustStore::PermissionStatus status,
-                              bool local)
+                                  tls::TrustStore::PermissionStatus status,
+                                  bool local)
 {
-    bool done = accountManager_ ? accountManager_->setCertificateStatus(cert, status, local) : false;
+    bool done = accountManager_ ? accountManager_->setCertificateStatus(cert, status, local)
+                                : false;
     if (done) {
         findCertificate(cert->getId().toString());
         emitSignal<libjami::ConfigurationSignal::CertificateStateChanged>(
@@ -3494,9 +3491,7 @@ JamiAccount::startAccountDiscovery()
                     dp.cleanupTask->cancel();
                 } else {
                     // Avoid Repeat Reception of Same peer
-                    JAMI_LOG("Account discovered: {}: {}",
-                              v.displayName,
-                              v.accountId.to_c_str());
+                    JAMI_LOG("Account discovered: {}: {}", v.displayName, v.accountId.to_c_str());
                     // Send Added Peer and corrsponding accoundID
                     emitSignal<libjami::PresenceSignal::NearbyPeerNotification>(getAccountID(),
                                                                                 v.accountId
@@ -3782,21 +3777,28 @@ JamiAccount::sendProfile(const std::string& convId,
         return;
     }
     // We need a new channel
-    transferFile(convId, profilePath(), deviceId, "profile.vcf", "", 0, 0, currentSha3,
+    transferFile(convId,
+                 profilePath(),
+                 deviceId,
+                 "profile.vcf",
+                 "",
+                 0,
+                 0,
+                 currentSha3,
                  fileutils::lastWriteTimeInSeconds(profilePath()),
                  std::move([accId = getAccountID(), peerUri, deviceId]() {
-        // Mark the VCard as sent
-        auto sendDir = fmt::format("{}/{}/vcard/{}",
-                                    fileutils::get_cache_dir(),
-                                    accId,
-                                    peerUri);
-        auto path = fmt::format("{}/{}", sendDir, deviceId);
-        fileutils::recursive_mkdir(sendDir);
-        std::lock_guard<std::mutex> lock(fileutils::getFileLock(path));
-        if (fileutils::isFile(path))
-            return;
-        fileutils::ofstream(path);
-    }));
+                     // Mark the VCard as sent
+                     auto sendDir = fmt::format("{}/{}/vcard/{}",
+                                                fileutils::get_cache_dir(),
+                                                accId,
+                                                peerUri);
+                     auto path = fmt::format("{}/{}", sendDir, deviceId);
+                     fileutils::recursive_mkdir(sendDir);
+                     std::lock_guard<std::mutex> lock(fileutils::getFileLock(path));
+                     if (fileutils::isFile(path))
+                         return;
+                     fileutils::ofstream(path);
+                 }));
 }
 
 bool
@@ -4118,14 +4120,12 @@ JamiAccount::transferFile(const std::string& conversationId,
         modified = fmt::format("&modified={}", lastWriteTime);
     }
     if (fid == "profile.vcf") {
-        fid =  fmt::format("profile.vcf?sha3={}{}", sha3Sum, modified);
+        fid = fmt::format("profile.vcf?sha3={}{}", sha3Sum, modified);
     }
-    auto channelName = conversationId.empty() ? fmt::format("{}profile.vcf?sha3={}{}", DATA_TRANSFER_URI, sha3Sum, modified)
-                        : fmt::format("{}{}/{}/{}",
-                                   DATA_TRANSFER_URI,
-                                   conversationId,
-                                   currentDeviceId(),
-                                   fid);
+    auto channelName
+        = conversationId.empty()
+              ? fmt::format("{}profile.vcf?sha3={}{}", DATA_TRANSFER_URI, sha3Sum, modified)
+              : fmt::format("{}{}/{}/{}", DATA_TRANSFER_URI, conversationId, currentDeviceId(), fid);
     std::lock_guard<std::mutex> lkCM(connManagerMtx_);
     if (!connectionManager_)
         return;
-- 
GitLab