Fix crash in UserModel::currentUser() and add more sanity checks
authorMichael Schuster <michael@schuster.ms>
Thu, 25 Jun 2020 18:02:22 +0000 (20:02 +0200)
committerCamila <smayres@gmail.com>
Thu, 25 Jun 2020 18:52:30 +0000 (20:52 +0200)
Commit 07bede8 (PR #1892) introduced a new helper method currentUser()
that didn't check for _users.count() thus causing to throw an
"index out of range" exception when no accounts are configured.

This commit uses the opportunity to add more sanity checks to UserModel.

Signed-off-by: Michael Schuster <michael@schuster.ms>
src/gui/tray/UserModel.cpp

index 2d60920b3145d1a83382a6714fbd6bee162471cd..77c5e399150fd63a218464f1d20ae92094c1d3cd 100644 (file)
@@ -560,17 +560,15 @@ Q_INVOKABLE int UserModel::currentUserId() const
 
 Q_INVOKABLE bool UserModel::isUserConnected(const int &id)
 {
-    if (!_users.isEmpty()) {
-        return _users[id]->isConnected();
-    } else {
+    if (_users.isEmpty())
         return false;
-    }
 
+    return _users[id]->isConnected();
 }
 
 Q_INVOKABLE QImage UserModel::currentUserAvatar()
 {
-    if (_users.count() >= 1) {
+    if (!_users.isEmpty()) {
         return _users[_currentUserId]->avatar();
     } else {
         QImage image(128, 128, QImage::Format_ARGB32);
@@ -585,25 +583,26 @@ Q_INVOKABLE QImage UserModel::currentUserAvatar()
 
 QImage UserModel::avatarById(const int &id)
 {
+    if (_users.isEmpty())
+        return {};
+
     return _users[id]->avatar(true);
 }
 
 Q_INVOKABLE QString UserModel::currentUserServer()
 {
-    if (_users.count() >= 1) {
-        return _users[_currentUserId]->server();
-    } else {
-        return QString("");
-    }
+    if (_users.isEmpty())
+        return {};
+
+    return _users[_currentUserId]->server();
 }
 
 Q_INVOKABLE bool UserModel::currentServerHasTalk()
 {
-    if (_users.count() >= 1) {
-        return _users[_currentUserId]->serverHasTalk();
-    } else {
+    if (_users.isEmpty())
         return false;
-    }
+
+    return _users[_currentUserId]->serverHasTalk();
 }
 
 void UserModel::addUser(AccountStatePtr &user, const bool &isCurrent)
@@ -636,11 +635,17 @@ int UserModel::currentUserIndex()
 
 Q_INVOKABLE void UserModel::openCurrentAccountLocalFolder()
 {
+    if (_users.isEmpty())
+        return;
+
     _users[_currentUserId]->openLocalFolder();
 }
 
 Q_INVOKABLE void UserModel::openCurrentAccountTalk()
 {
+    if (_users.isEmpty())
+        return;
+
     QString url = _users[_currentUserId]->server(false) + "/apps/spreed";
     if (!(url.contains("http://") || url.contains("https://"))) {
         url = "https://" + _users[_currentUserId]->server(false) + "/apps/spreed";
@@ -651,7 +656,9 @@ Q_INVOKABLE void UserModel::openCurrentAccountTalk()
 Q_INVOKABLE void UserModel::openCurrentAccountServer()
 {
     // Don't open this URL when the QML appMenu pops up on click (see Window.qml)
-    if(appList().count() > 0)
+    if (appList().count() > 0)
+        return;
+    if (_users.isEmpty())
         return;
 
     QString url = _users[_currentUserId]->server(false);
@@ -663,6 +670,9 @@ Q_INVOKABLE void UserModel::openCurrentAccountServer()
 
 Q_INVOKABLE void UserModel::switchCurrentUser(const int &id)
 {
+    if (_users.isEmpty())
+        return;
+
     _users[_currentUserId]->setCurrentUser(false);
     _users[id]->setCurrentUser(true);
     _currentUserId = id;
@@ -672,18 +682,27 @@ Q_INVOKABLE void UserModel::switchCurrentUser(const int &id)
 
 Q_INVOKABLE void UserModel::login(const int &id)
 {
+    if (_users.isEmpty())
+        return;
+
     _users[id]->login();
     emit refreshCurrentUserGui();
 }
 
 Q_INVOKABLE void UserModel::logout(const int &id)
 {
+    if (_users.isEmpty())
+        return;
+
     _users[id]->logout();
     emit refreshCurrentUserGui();
 }
 
 Q_INVOKABLE void UserModel::removeAccount(const int &id)
 {
+    if (_users.isEmpty())
+        return;
+
     QMessageBox messageBox(QMessageBox::Question,
         tr("Confirm Account Removal"),
         tr("<p>Do you really want to remove the connection to the account <i>%1</i>?</p>"
@@ -755,35 +774,47 @@ QHash<int, QByteArray> UserModel::roleNames() const
 
 ActivityListModel *UserModel::currentActivityModel()
 {
+    if (_users.isEmpty())
+        return nullptr;
+
     return _users[currentUserIndex()]->getActivityModel();
 }
 
 bool UserModel::currentUserHasActivities()
 {
+    if (_users.isEmpty())
+        return false;
+
     return _users[currentUserIndex()]->hasActivities();
 }
 
 bool UserModel::currentUserHasLocalFolder()
 {
+    if (_users.isEmpty())
+        return false;
+
     return _users[currentUserIndex()]->getFolder() != nullptr;
 }
 
 void UserModel::fetchCurrentActivityModel()
 {
-    _users[currentUserId()]->slotRefresh();
+    if (!_users.isEmpty())
+        _users[currentUserId()]->slotRefresh();
 }
 
 AccountAppList UserModel::appList() const
 {
-    if (_users.count() > 0) {
-        return _users[_currentUserId]->appList();
-    } else {
+    if (_users.isEmpty())
         return AccountAppList();
-    }
+
+    return _users[_currentUserId]->appList();
 }
 
 User *UserModel::currentUser() const
 {
+    if (_users.isEmpty())
+        return nullptr;
+
     return _users[currentUserId()];
 }
 
@@ -832,10 +863,10 @@ void UserAppsModel::buildAppList()
         endRemoveRows();
     }
 
-    if(UserModel::instance()->appList().count() > 0) {
-        foreach(AccountApp *app, UserModel::instance()->appList()) {
+    if (UserModel::instance()->appList().count() > 0) {
+        foreach (AccountApp *app, UserModel::instance()->appList()) {
             // Filter out Talk because we have a dedicated button for it
-            if(app->id() == QLatin1String("spreed"))
+            if (app->id() == QLatin1String("spreed"))
                 continue;
 
             beginInsertRows(QModelIndex(), rowCount(), rowCount());