From 91163b8bedb2d569691591afcd6459853c5591a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Thu, 6 Apr 2023 14:27:46 -0400
Subject: [PATCH] migration: avoid race conditions for account status
Setting an account to migrating is asynchronous. So the status
of the account should be checked via dedicated signals and not
just at the startup, else the migration view is not shown.
Also removing two accounts to migrate wasn't working correctly.
Change-Id: I0bb716982ec5fe76aaba3ad4cb6f47b1ef0aa710
GitLab: #1062
---
extras/ci/client-qt-gnulinux/Jenkinsfile | 2 +-
src/app/AccountMigrationView.qml | 13 +---
src/app/MainApplicationWindow.qml | 29 ++++---
src/app/ViewCoordinator.qml | 1 +
src/app/constant/JamiQmlUtils.qml | 1 -
src/app/currentaccounttomigrate.cpp | 96 ++++++++++++++++--------
src/app/currentaccounttomigrate.h | 6 ++
7 files changed, 92 insertions(+), 56 deletions(-)
diff --git a/extras/ci/client-qt-gnulinux/Jenkinsfile b/extras/ci/client-qt-gnulinux/Jenkinsfile
index 1c4e4f256..2d5f1961e 100644
--- a/extras/ci/client-qt-gnulinux/Jenkinsfile
+++ b/extras/ci/client-qt-gnulinux/Jenkinsfile
@@ -119,7 +119,7 @@ pipeline {
// Run tests
exec_cmd("""
cd ${dockerTopDir}/build/tests
- HOME=/tmp ctest -j${cpuCount} -V -C Release
+ HOME=/tmp ctest -V -C Release -j${cpuCount}
""")
}
}
diff --git a/src/app/AccountMigrationView.qml b/src/app/AccountMigrationView.qml
index 37a59468f..c3c5f6bf9 100644
--- a/src/app/AccountMigrationView.qml
+++ b/src/app/AccountMigrationView.qml
@@ -28,7 +28,7 @@ import net.jami.Constants 1.1
import "commoncomponents"
// Account Migration Dialog for migrating account
-Rectangle {
+BaseView {
id: root
enum AccountMigrationStep {
@@ -54,8 +54,6 @@ Rectangle {
CurrentAccountToMigrate.removeCurrentAccountToMigrate()
}
- color: JamiTheme.backgroundColor
-
Timer {
id: timerFailureReturn
@@ -94,15 +92,6 @@ Rectangle {
stackedWidget.currentIndex =
AccountMigrationView.AccountMigrationStep.PasswordEnter
}
-
- function onAllMigrationsFinished() {
- if (UtilsAdapter.getAccountListSize() === 0)
- root.loaderSourceChangeRequested(
- MainApplicationWindow.LoadedSource.WizardView)
- else
- root.loaderSourceChangeRequested(
- MainApplicationWindow.LoadedSource.MainView)
- }
}
ColumnLayout {
diff --git a/src/app/MainApplicationWindow.qml b/src/app/MainApplicationWindow.qml
index b8f10e34d..46d995cd6 100644
--- a/src/app/MainApplicationWindow.qml
+++ b/src/app/MainApplicationWindow.qml
@@ -78,10 +78,6 @@ ApplicationWindow {
}
}
- function startAccountMigration() {
- setMainLoaderSource(JamiQmlUtils.accountMigrationViewLoadPath)
- }
-
function setMainLoaderSource(source) {
if (checkLoadedSource() === MainApplicationWindow.LoadedSource.MainView) {
cleanupMainView()
@@ -142,12 +138,29 @@ ApplicationWindow {
}
}
+ Connections {
+ id: connectionMigrationEnded
+
+ target: CurrentAccountToMigrate
+
+ function onAccountNeedsMigration(accountId) {
+ viewCoordinator.present("AccountMigrationView")
+ }
+
+ function onAllMigrationsFinished() {
+ viewCoordinator.dismiss("AccountMigrationView")
+ startClient()
+ }
+ }
+
Connections {
target: mainApplicationLoader.item
function onLoaderSourceChangeRequested(sourceToLoad) {
if (sourceToLoad === MainApplicationWindow.LoadedSource.WizardView)
setMainLoaderSource(JamiQmlUtils.wizardViewLoadPath)
+ else if (sourceToLoad === MainApplicationWindow.LoadedSource.AccountMigrationView)
+ setMainLoaderSource(JamiQmlUtils.accountMigrationViewLoadPath)
else
setMainLoaderSource(JamiQmlUtils.mainViewLoadPath)
}
@@ -180,6 +193,8 @@ ApplicationWindow {
})
// Set the viewCoordinator's root item.
viewCoordinator.init(item)
+ if (CurrentAccountToMigrate.accountToMigrateListSize > 0)
+ viewCoordinator.present("AccountMigrationView")
}
if (Qt.platform.os.toString() === "osx") {
MainApplication.setEventFilter()
@@ -248,11 +263,7 @@ ApplicationWindow {
onClosing: root.close()
Component.onCompleted: {
- if (CurrentAccountToMigrate.accountToMigrateListSize <= 0)
- startClient()
- else
- startAccountMigration()
-
+ startClient()
if (Qt.platform.os.toString() !== "windows" && Qt.platform.os.toString() !== "osx")
DBusErrorHandler.setActive(true)
}
diff --git a/src/app/ViewCoordinator.qml b/src/app/ViewCoordinator.qml
index c4c9148d8..59e9925ea 100644
--- a/src/app/ViewCoordinator.qml
+++ b/src/app/ViewCoordinator.qml
@@ -39,6 +39,7 @@ QtObject {
"ConversationView": "mainview/ConversationView.qml",
"NewSwarmPage": "mainview/components/NewSwarmPage.qml",
"WizardView": "wizardview/WizardView.qml",
+ "AccountMigrationView": "AccountMigrationView.qml",
"SettingsView": "settingsview/SettingsView.qml",
"SettingsSidePanel": "settingsview/SettingsSidePanel.qml",
}
diff --git a/src/app/constant/JamiQmlUtils.qml b/src/app/constant/JamiQmlUtils.qml
index 4d051a980..82334258e 100644
--- a/src/app/constant/JamiQmlUtils.qml
+++ b/src/app/constant/JamiQmlUtils.qml
@@ -28,7 +28,6 @@ Item {
readonly property string mainViewLoadPath: "qrc:/mainview/MainView.qml"
readonly property string wizardViewLoadPath: "qrc:/wizardview/WizardView.qml"
- readonly property string accountMigrationViewLoadPath: "qrc:/AccountMigrationView.qml"
readonly property string base64StringTitle: "data:image/png;base64,"
property var accountCreationInputParaObject: ({})
diff --git a/src/app/currentaccounttomigrate.cpp b/src/app/currentaccounttomigrate.cpp
index 93e34e87c..8292f68be 100644
--- a/src/app/currentaccounttomigrate.cpp
+++ b/src/app/currentaccounttomigrate.cpp
@@ -40,28 +40,22 @@ CurrentAccountToMigrate::CurrentAccountToMigrate(LRCInstance* instance, QObject*
}
if (accountToMigrateList_.size()) {
- migrationEndedConnection_ = connect(
- &lrcInstance_->accountModel(),
- &lrc::api::AccountModel::migrationEnded,
- this,
- [this](const QString& accountId, bool ok) {
- if (ok && accountToMigrateList_.removeOne(accountId)) {
- updateData();
- }
-
- if (accountToMigrateList_.isEmpty()) {
- disconnect(migrationEndedConnection_);
- Q_EMIT allMigrationsFinished();
-
- return;
- }
-
- Q_EMIT migrationEnded(ok);
- },
- Qt::ConnectionType::QueuedConnection);
-
+ connectMigrationEnded();
updateData();
}
+
+ connect(&lrcInstance_->accountModel(),
+ &AccountModel::accountStatusChanged,
+ this,
+ &CurrentAccountToMigrate::slotAccountStatusChanged);
+ connect(&lrcInstance_->accountModel(),
+ &AccountModel::accountAdded,
+ this,
+ &CurrentAccountToMigrate::slotAccountStatusChanged);
+ connect(&lrcInstance_->accountModel(),
+ &AccountModel::accountRemoved,
+ this,
+ &CurrentAccountToMigrate::slotAccountRemoved);
}
CurrentAccountToMigrate::~CurrentAccountToMigrate() {}
@@ -69,19 +63,6 @@ CurrentAccountToMigrate::~CurrentAccountToMigrate() {}
void
CurrentAccountToMigrate::removeCurrentAccountToMigrate()
{
- if (accountToMigrateList_.removeOne(get_accountId())) {
- updateData();
- }
-
- Utils::oneShotConnect(&lrcInstance_->accountModel(),
- &lrc::api::AccountModel::accountRemoved,
- [this] {
- if (accountToMigrateList_.isEmpty())
- Q_EMIT allMigrationsFinished();
- else
- Q_EMIT currentAccountToMigrateRemoved();
- });
-
lrcInstance_->accountModel().removeAccount(get_accountId());
}
@@ -102,3 +83,52 @@ CurrentAccountToMigrate::updateData()
set_username(avatarInfo.confProperties.username);
set_alias(lrcInstance_->accountModel().getAccountInfo(accountId).profileInfo.alias);
}
+
+void
+CurrentAccountToMigrate::connectMigrationEnded()
+{
+ migrationEndedConnection_ = connect(
+ &lrcInstance_->accountModel(),
+ &lrc::api::AccountModel::migrationEnded,
+ this,
+ [this](const QString& accountId, bool ok) {
+ if (ok && accountToMigrateList_.removeOne(accountId)) {
+ updateData();
+ }
+
+ if (accountToMigrateList_.isEmpty()) {
+ disconnect(migrationEndedConnection_);
+ Q_EMIT allMigrationsFinished();
+ return;
+ }
+
+ Q_EMIT migrationEnded(ok);
+ },
+ Qt::ConnectionType::QueuedConnection);
+}
+
+void
+CurrentAccountToMigrate::slotAccountStatusChanged(const QString& accountId)
+{
+ try {
+ auto& accountInfo = lrcInstance_->getAccountInfo(accountId);
+ if (accountInfo.status == lrc::api::account::Status::ERROR_NEED_MIGRATION) {
+ accountToMigrateList_.append(accountId);
+ updateData();
+ connectMigrationEnded();
+ Q_EMIT accountNeedsMigration(accountId);
+ }
+ } catch (...) {
+ }
+}
+
+void
+CurrentAccountToMigrate::slotAccountRemoved(const QString& accountId)
+{
+ if (accountToMigrateList_.removeOne(accountId))
+ updateData();
+ if (accountToMigrateList_.isEmpty())
+ Q_EMIT allMigrationsFinished();
+ else
+ Q_EMIT currentAccountToMigrateRemoved();
+}
\ No newline at end of file
diff --git a/src/app/currentaccounttomigrate.h b/src/app/currentaccounttomigrate.h
index 611fb52bb..8f1bbd0ec 100644
--- a/src/app/currentaccounttomigrate.h
+++ b/src/app/currentaccounttomigrate.h
@@ -45,8 +45,14 @@ Q_SIGNALS:
void migrationEnded(bool success);
void allMigrationsFinished();
void currentAccountToMigrateRemoved();
+ void accountNeedsMigration(const QString& accountId);
+
+private Q_SLOTS:
+ void slotAccountStatusChanged(const QString& accountId);
+ void slotAccountRemoved(const QString& accountId);
private:
+ void connectMigrationEnded();
void updateData();
LRCInstance* lrcInstance_;
--
GitLab