From 956e54409780ffe3219ef1ff6f2459e5159b8358 Mon Sep 17 00:00:00 2001
From: Hannah von Reth <hannah.vonreth@owncloud.com>
Date: Fri, 30 Jun 2023 14:20:45 +0200
Subject: [PATCH] AccountManager manage std::unique_ptr

---
 src/gui/accountmanager.cpp            | 24 +++++++-------
 src/gui/accountmanager.h              |  4 +--
 src/gui/activitywidget.cpp            |  4 +--
 src/gui/application.cpp               |  6 ++--
 src/gui/folderman.cpp                 |  2 +-
 src/gui/models/activitylistmodel.cpp  | 24 ++++++--------
 src/gui/models/activitylistmodel.h    |  2 +-
 src/gui/networksettings.cpp           |  2 +-
 src/gui/owncloudgui.cpp               | 48 +++++++++++++--------------
 src/gui/owncloudgui.h                 |  1 -
 src/gui/settingsdialog.cpp            |  2 +-
 src/gui/socketapi/socketapi.cpp       |  2 +-
 test/modeltests/testactivitymodel.cpp |  2 +-
 test/testfoldermigration.cpp          |  2 +-
 14 files changed, 61 insertions(+), 64 deletions(-)

diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp
index b373817d3e0..5fbe29943e8 100644
--- a/src/gui/accountmanager.cpp
+++ b/src/gui/accountmanager.cpp
@@ -346,14 +346,15 @@ AccountStatePtr AccountManager::account(const QString &name)
 {
     for (const auto &acc : qAsConst(_accounts)) {
         if (acc->account()->displayName() == name) {
-            return acc;
+            return acc.get();
         }
     }
     return AccountStatePtr();
 }
 
 AccountStatePtr AccountManager::account(const QUuid uuid) {
-    return _accounts.value(uuid);
+    // assumes we always find one
+    return std::find_if(_accounts.cbegin(), _accounts.cend(), [uuid](const auto &accountState) { return accountState->account()->uuid() == uuid; })->get();
 }
 
 AccountStatePtr AccountManager::addAccount(const AccountPtr &newAccount)
@@ -369,12 +370,13 @@ AccountStatePtr AccountManager::addAccount(const AccountPtr &newAccount)
 
 void AccountManager::deleteAccount(AccountStatePtr account)
 {
-    auto it = std::find(_accounts.begin(), _accounts.end(), account);
-    if (it == _accounts.end()) {
+    auto it = std::find_if(_accounts.cbegin(), _accounts.cend(), [&](const auto &accountState) { return account.get() == accountState.get(); });
+    if (it == _accounts.cend()) {
         return;
     }
-    // The argument keeps a strong reference to the AccountState, so we can safely remove other
-    // AccountStatePtr occurrences:
+
+    // this unique ptr will delete the object once we leave the scope
+    auto accountStateToBeRemoved = std::move(_accounts.at(std::distance(_accounts.cbegin(), it)));
     _accounts.erase(it);
 
     // Forget account credentials, cookies
@@ -385,7 +387,6 @@ void AccountManager::deleteAccount(AccountStatePtr account)
     settings->remove(account->account()->id());
 
     emit accountRemoved(account);
-    account->deleteLater();
 }
 
 AccountPtr AccountManager::createAccount(const QUuid &uuid)
@@ -398,7 +399,7 @@ void AccountManager::shutdown()
 {
     const auto accounts = std::move(_accounts);
     for (const auto &acc : accounts) {
-        emit accountRemoved(acc);
+        emit accountRemoved(acc.get());
     }
 }
 
@@ -434,9 +435,8 @@ AccountStatePtr AccountManager::addAccountState(std::unique_ptr<AccountState> &&
         saveAccount(rawAccount, false);
     });
 
-    AccountStatePtr statePtr = accountState.release();
-    _accounts.insert(statePtr->account()->uuid(), statePtr);
-    emit accountAdded(statePtr);
-    return statePtr;
+    _accounts.push_back(std::move(accountState));
+    emit accountAdded(_accounts.back().get());
+    return _accounts.back().get();
 }
 }
diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h
index d3b525f4cd5..dffb0e3b811 100644
--- a/src/gui/accountmanager.h
+++ b/src/gui/accountmanager.h
@@ -58,7 +58,7 @@ class AccountManager : public QObject
      * Return a map of all accounts.
      * (this is a map of QSharedPointer for internal reasons, one should normally not keep a copy of them)
      */
-    const QMap<QUuid, AccountStatePtr> &accounts() { return _accounts; }
+    const auto &accounts() { return _accounts; }
 
     /**
      * Return the account state pointer for an account identified by its display name
@@ -116,7 +116,7 @@ public slots:
 
 private:
     AccountManager() {}
-    QMap<QUuid, AccountStatePtr> _accounts;
+    std::vector<std::unique_ptr<AccountState>> _accounts;
     /// Account ids from settings that weren't read
     QSet<QString> _additionalBlockedAccountIds;
 };
diff --git a/src/gui/activitywidget.cpp b/src/gui/activitywidget.cpp
index 148fec7fc51..c27fda12cda 100644
--- a/src/gui/activitywidget.cpp
+++ b/src/gui/activitywidget.cpp
@@ -178,7 +178,7 @@ void ActivityWidget::slotAccountActivityStatus(AccountStatePtr ast, int statusCo
 
 void ActivityWidget::checkActivityTabVisibility()
 {
-    int accountCount = AccountManager::instance()->accounts().count();
+    int accountCount = AccountManager::instance()->accounts().size();
     bool hasAccountsWithActivity =
         _accountsWithoutActivities.count() != accountCount;
     bool hasNotifications = !_widgetForNotifId.isEmpty();
@@ -568,7 +568,7 @@ void ActivitySettings::slotRefresh(AccountStatePtr ptr)
 void ActivitySettings::slotRegularNotificationCheck()
 {
     for (const auto &a : AccountManager::instance()->accounts()) {
-        slotRefresh(a);
+        slotRefresh(a.get());
     }
 }
 
diff --git a/src/gui/application.cpp b/src/gui/application.cpp
index 4af7efee357..41a94ac31bf 100644
--- a/src/gui/application.cpp
+++ b/src/gui/application.cpp
@@ -229,7 +229,7 @@ Application::Application(Platform *platform, bool debugMode, QObject *parent)
     connect(AccountManager::instance(), &AccountManager::accountAdded, this, &Application::slotAccountStateAdded);
     connect(AccountManager::instance(), &AccountManager::accountRemoved, this, &Application::slotAccountStateRemoved);
     for (const auto &ai : AccountManager::instance()->accounts()) {
-        slotAccountStateAdded(ai);
+        slotAccountStateAdded(ai.get());
     }
 
     connect(FolderMan::instance()->socketApi(), &SocketApi::shareCommandReceived, _gui.data(), &ownCloudGui::slotShowShareDialog);
@@ -255,7 +255,7 @@ Application::Application(Platform *platform, bool debugMode, QObject *parent)
     }
 #endif
 
-    if (AccountManager::instance()->accounts().isEmpty()) {
+    if (AccountManager::instance()->accounts().empty()) {
         // display the wizard if we don't have an account yet
         QTimer::singleShot(0, gui(), &ownCloudGui::runNewAccountWizard);
     }
@@ -273,7 +273,7 @@ Application::~Application()
 void Application::slotAccountStateRemoved() const
 {
     // if there is no more account, show the wizard.
-    if (_gui && AccountManager::instance()->accounts().isEmpty()) {
+    if (_gui && AccountManager::instance()->accounts().empty()) {
         // allow to add a new account if there is non any more. Always think
         // about single account theming!
         gui()->runNewAccountWizard();
diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
index fe042bed3cb..a1e92eba4fd 100644
--- a/src/gui/folderman.cpp
+++ b/src/gui/folderman.cpp
@@ -237,7 +237,7 @@ int FolderMan::setupFolders()
                 // Should not happen: bad container keys should have been deleted
                 qCWarning(lcFolderMan) << "Folder structure" << groupName << "is too new, ignoring";
             } else {
-                setupFoldersHelper(*settings, account, skipSettingsKeys, backwardsCompatible, foldersWithPlaceholders);
+                setupFoldersHelper(*settings, account.get(), skipSettingsKeys, backwardsCompatible, foldersWithPlaceholders);
             }
             settings->endGroup();
         };
diff --git a/src/gui/models/activitylistmodel.cpp b/src/gui/models/activitylistmodel.cpp
index f6390207d00..4563d1691c9 100644
--- a/src/gui/models/activitylistmodel.cpp
+++ b/src/gui/models/activitylistmodel.cpp
@@ -172,11 +172,9 @@ bool ActivityListModel::canFetchMore(const QModelIndex &) const
     if (_activityLists.isEmpty())
         return true;
 
-    for (auto i = _activityLists.begin(); i != _activityLists.end(); ++i) {
-        AccountStatePtr accountState = i.key();
+    for (const auto &[accountState, activityList] : Utility::asKeyValueRange(_activityLists)) {
         if (accountState && accountState->isConnected()) {
-            ActivityList activities = i.value();
-            if (activities.count() == 0 && !_currentlyFetching.contains(accountState)) {
+            if (activityList.count() == 0 && !_currentlyFetching.contains(accountState)) {
                 return true;
             }
         }
@@ -218,7 +216,7 @@ void ActivityListModel::startFetchJob(AccountStatePtr ast)
                     QDateTime::fromString(json.value(QStringLiteral("date")).toString(), Qt::ISODate)});
             }
 
-            _activityLists[ast] = std::move(list);
+            _activityLists[ast.get()] = std::move(list);
 
             emit activityJobStatusCode(ast, job->ocsStatus());
 
@@ -249,25 +247,25 @@ void ActivityListModel::setActivityList(const ActivityList &&resultList)
 
 void ActivityListModel::fetchMore(const QModelIndex &)
 {
-    for (const AccountStatePtr &asp : AccountManager::instance()->accounts()) {
-        if (!_activityLists.contains(asp) && asp->isConnected()) {
-            _activityLists[asp] = ActivityList();
-            startFetchJob(asp);
+    for (const auto &accountState : AccountManager::instance()->accounts()) {
+        if (!_activityLists.contains(accountState.get()) && accountState->isConnected()) {
+            _activityLists[accountState.get()] = ActivityList();
+            startFetchJob(accountState.get());
         }
     }
 }
 
 void ActivityListModel::slotRefreshActivity(const AccountStatePtr &ast)
 {
-    if (ast && _activityLists.contains(ast)) {
-        _activityLists.remove(ast);
+    if (ast && _activityLists.contains(ast.get())) {
+        _activityLists.remove(ast.get());
     }
     startFetchJob(ast);
 }
 
 void ActivityListModel::slotRemoveAccount(AccountStatePtr ast)
 {
-    if (_activityLists.contains(ast)) {
+    if (_activityLists.contains(ast.get())) {
         const auto accountToRemove = ast->account()->uuid();
 
         QMutableListIterator<Activity> it(_finalList);
@@ -283,7 +281,7 @@ void ActivityListModel::slotRemoveAccount(AccountStatePtr ast)
                 ++i;
             }
         }
-        _activityLists.remove(ast);
+        _activityLists.remove(ast.get());
         _currentlyFetching.remove(ast);
     }
 }
diff --git a/src/gui/models/activitylistmodel.h b/src/gui/models/activitylistmodel.h
index 0b63a30cac7..082a07d900b 100644
--- a/src/gui/models/activitylistmodel.h
+++ b/src/gui/models/activitylistmodel.h
@@ -73,7 +73,7 @@ public slots:
     void startFetchJob(AccountStatePtr s);
     void combineActivityLists();
 
-    QMap<AccountStatePtr, ActivityList> _activityLists;
+    QMap<AccountState *, ActivityList> _activityLists;
     ActivityList _finalList;
     QSet<AccountStatePtr> _currentlyFetching;
 
diff --git a/src/gui/networksettings.cpp b/src/gui/networksettings.cpp
index e34b980ecad..0cd753af293 100644
--- a/src/gui/networksettings.cpp
+++ b/src/gui/networksettings.cpp
@@ -202,7 +202,7 @@ void NetworkSettings::saveProxySettings()
     // start the sync.
     FolderMan::instance()->setDirtyProxy();
 
-    for (auto account : AccountManager::instance()->accounts()) {
+    for (auto &account : AccountManager::instance()->accounts()) {
         account->freshConnectionAttempt();
     }
 }
diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp
index 698fee94b1d..c991f220db6 100644
--- a/src/gui/owncloudgui.cpp
+++ b/src/gui/owncloudgui.cpp
@@ -54,9 +54,23 @@
 
 using namespace std::chrono_literals;
 
+using namespace OCC;
+
 namespace {
 
-using namespace OCC;
+template <typename T>
+void setPauseOnAllFoldersHelper(const std::vector<T> &accounts, bool pause)
+{
+    for (auto *f : FolderMan::instance()->folders()) {
+        auto it = std::find_if(accounts.cbegin(), accounts.cend(), [f](const auto &accountState) { return accountState.get() == f->accountState(); });
+        if (it != accounts.cend()) {
+            f->setSyncPaused(pause);
+            if (pause) {
+                f->slotTerminateSync();
+            }
+        }
+    }
+}
 
 void setUpInitialSyncFolder(AccountStatePtr accountStatePtr, bool useVfs)
 {
@@ -156,7 +170,7 @@ ownCloudGui::~ownCloudGui()
 void ownCloudGui::slotOpenSettingsDialog()
 {
     // if account is set up, start the configuration wizard.
-    if (!AccountManager::instance()->accounts().isEmpty()) {
+    if (!AccountManager::instance()->accounts().empty()) {
         if (QApplication::activeWindow() != _settingsDialog) {
             slotShowSettings();
         } else {
@@ -260,7 +274,7 @@ void ownCloudGui::slotComputeOverallSyncStatus()
             allSignedOut = false;
         }
         if (!a->isConnected()) {
-            problemAccounts.append(a);
+            problemAccounts.append(a.get());
         } else {
             allDisconnected = false;
         }
@@ -387,9 +401,9 @@ void ownCloudGui::addAccountContextMenu(AccountStatePtr accountState, QMenu *men
 
     menu->addSeparator();
     if (onePaused) {
-        menu->addAction(tr("Resume synchronization"), this, [accountState, this] { setPauseOnAllFoldersHelper({accountState}, false); });
+        menu->addAction(tr("Resume synchronization"), this, [accountState, this] { setPauseOnAllFoldersHelper<AccountStatePtr>({accountState}, false); });
     } else {
-        menu->addAction(tr("Stop synchronization"), this, [accountState, this] { setPauseOnAllFoldersHelper({accountState}, true); });
+        menu->addAction(tr("Stop synchronization"), this, [accountState, this] { setPauseOnAllFoldersHelper<AccountStatePtr>({accountState}, true); });
     }
 
     if (accountState->isSignedOut()) {
@@ -596,7 +610,7 @@ void ownCloudGui::updateContextMenu()
 
     const auto &accountList = AccountManager::instance()->accounts();
 
-    bool isConfigured = (!accountList.isEmpty());
+    bool isConfigured = (!accountList.empty());
     bool atLeastOneConnected = false;
     bool atLeastOnePaused = false;
     bool atLeastOneNotPaused = false;
@@ -617,15 +631,13 @@ void ownCloudGui::updateContextMenu()
     _contextMenu->addAction(Theme::instance()->applicationIcon(), tr("Show %1").arg(Theme::instance()->appNameGUI()), this, &ownCloudGui::slotShowSettings);
     _contextMenu->addSeparator();
     if (atLeastOnePaused) {
-        _contextMenu->addAction(
-            tr("Resume synchronization"), this, [this] { setPauseOnAllFoldersHelper(AccountManager::instance()->accounts().values(), false); });
+        _contextMenu->addAction(tr("Resume synchronization"), this, [this] { setPauseOnAllFoldersHelper(AccountManager::instance()->accounts(), false); });
     } else {
-        _contextMenu->addAction(
-            tr("Stop synchronization"), this, [this] { setPauseOnAllFoldersHelper(AccountManager::instance()->accounts().values(), true); });
+        _contextMenu->addAction(tr("Stop synchronization"), this, [this] { setPauseOnAllFoldersHelper(AccountManager::instance()->accounts(), true); });
     }
     _contextMenu->addSeparator();
 
-    if (accountList.isEmpty()) {
+    if (accountList.empty()) {
         _contextMenu->addAction(tr("Create a new account"), this, &ownCloudGui::runNewAccountWizard);
     } else {
         // submenus for accounts
@@ -634,7 +646,7 @@ void ownCloudGui::updateContextMenu()
             _accountMenus.append(accountMenu);
             _contextMenu->addMenu(accountMenu);
 
-            addAccountContextMenu(account, accountMenu);
+            addAccountContextMenu(account.get(), accountMenu);
         }
     }
 
@@ -938,18 +950,6 @@ void ownCloudGui::runNewAccountWizard()
     }
 }
 
-void ownCloudGui::setPauseOnAllFoldersHelper(const QList<AccountStatePtr> &accounts, bool pause)
-{
-    for (auto *f : FolderMan::instance()->folders()) {
-        if (accounts.contains(f->accountState())) {
-            f->setSyncPaused(pause);
-            if (pause) {
-                f->slotTerminateSync();
-            }
-        }
-    }
-}
-
 void ownCloudGui::slotShowSettings()
 {
     raiseDialog(_settingsDialog);
diff --git a/src/gui/owncloudgui.h b/src/gui/owncloudgui.h
index ff7f1a7a949..30ef68611aa 100644
--- a/src/gui/owncloudgui.h
+++ b/src/gui/owncloudgui.h
@@ -114,7 +114,6 @@ public slots:
     void slotRemoveDestroyedShareDialogs();
 
 private:
-    void setPauseOnAllFoldersHelper(const QList<AccountStatePtr> &accounts, bool pause);
     void setupActions();
     void addAccountContextMenu(AccountStatePtr accountState, QMenu *menu);
 
diff --git a/src/gui/settingsdialog.cpp b/src/gui/settingsdialog.cpp
index cf717ecb7cc..7a4fe2facc5 100644
--- a/src/gui/settingsdialog.cpp
+++ b/src/gui/settingsdialog.cpp
@@ -262,7 +262,7 @@ SettingsDialog::SettingsDialog(ownCloudGui *gui, QWidget *parent)
     connect(AccountManager::instance(), &AccountManager::accountRemoved,
         this, &SettingsDialog::accountRemoved);
     for (const auto &ai : AccountManager::instance()->accounts()) {
-        accountAdded(ai);
+        accountAdded(ai.get());
     }
 
     QTimer::singleShot(0, this, &SettingsDialog::showFirstPage);
diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp
index fdce06fffaf..16219e02659 100644
--- a/src/gui/socketapi/socketapi.cpp
+++ b/src/gui/socketapi/socketapi.cpp
@@ -795,7 +795,7 @@ Q_INVOKABLE void OCC::SocketApi::command_OPEN_APP_LINK(const QString &localFile,
 void SocketApi::command_V2_LIST_ACCOUNTS(const QSharedPointer<SocketApiJobV2> &job) const
 {
     QJsonArray out;
-    for (auto acc : AccountManager::instance()->accounts()) {
+    for (const auto &acc : AccountManager::instance()->accounts()) {
         out << QJsonObject({ { QStringLiteral("name"), acc->account()->displayName() },
             { QStringLiteral("id"), acc->account()->id() },
             { QStringLiteral("uuid"), acc->account()->uuid().toString(QUuid::WithoutBraces) } });
diff --git a/test/modeltests/testactivitymodel.cpp b/test/modeltests/testactivitymodel.cpp
index 30a9ae49d9d..b144ddb302e 100644
--- a/test/modeltests/testactivitymodel.cpp
+++ b/test/modeltests/testactivitymodel.cpp
@@ -46,7 +46,7 @@ private Q_SLOTS:
             Activity{Activity::ActivityType, QStringLiteral("021ad48a-80ae-4af6-b878-aeb836bd367d"), acc2, QStringLiteral("test"), QStringLiteral("test"),
                 QStringLiteral("foo.cpp"), QUrl(QStringLiteral("https://owncloud.com")), QDateTime::currentDateTime()},
         });
-        model->slotRemoveAccount(AccountManager::instance()->accounts().first());
+        model->slotRemoveAccount(AccountManager::instance()->accounts().front().get());
     }
 };
 }
diff --git a/test/testfoldermigration.cpp b/test/testfoldermigration.cpp
index 2f3c1086744..2d5c22bab17 100644
--- a/test/testfoldermigration.cpp
+++ b/test/testfoldermigration.cpp
@@ -92,7 +92,7 @@ private slots:
         AccountManager::instance()->restore();
 
         settings->beginGroup(QStringLiteral("0/Folders"));
-        TestUtils::folderMan()->setupFoldersHelper(*settings.get(), AccountManager::instance()->accounts().first(), {}, true, false);
+        TestUtils::folderMan()->setupFoldersHelper(*settings.get(), AccountManager::instance()->accounts().front().get(), {}, true, false);
         settings->endGroup();
 
         QCOMPARE(journalPaths.first(), settings->value(QStringLiteral("0/Folders/1/journalPath")));