From 6ea828936fca7f609ce3bdaad19b15abc3cda0ca Mon Sep 17 00:00:00 2001 From: alex-z Date: Mon, 10 Jul 2023 22:20:44 +0200 Subject: [PATCH 1/2] Fix crash in _finalList removal loop. Fix incorrect implementation of seen chat notifications removal. Improved notifications fetch code to avoid multiple calls from different threads. Signed-off-by: alex-z --- src/gui/tray/activitylistmodel.cpp | 7 +++++- src/gui/tray/notificationhandler.cpp | 10 +++++--- src/gui/tray/notificationhandler.h | 5 ++-- src/gui/tray/usermodel.cpp | 35 ++++++++++++++++++++-------- src/gui/tray/usermodel.h | 5 ++++ 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp index c1cd0c2c41ebb..9da43924d68b9 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/src/gui/tray/activitylistmodel.cpp @@ -691,11 +691,16 @@ void ActivityListModel::removeActivityFromActivityList(const Activity &activity) void ActivityListModel::checkAndRemoveSeenActivities(const OCC::ActivityList &newActivities) { + ActivityList activitiesToRemove; for (const auto &activity : _finalList) { if (activity._objectType == QStringLiteral("chat") && !newActivities.contains(activity)) { - removeActivityFromActivityList(activity); + activitiesToRemove.push_back(activity); } } + + for (const auto &toRemove : activitiesToRemove) { + removeActivityFromActivityList(toRemove); + } } void ActivityListModel::slotTriggerDefaultAction(const int activityIndex) diff --git a/src/gui/tray/notificationhandler.cpp b/src/gui/tray/notificationhandler.cpp index e41baa4709195..62ff2f32832bb 100644 --- a/src/gui/tray/notificationhandler.cpp +++ b/src/gui/tray/notificationhandler.cpp @@ -24,12 +24,12 @@ ServerNotificationHandler::ServerNotificationHandler(AccountState *accountState, { } -void ServerNotificationHandler::slotFetchNotifications() +bool ServerNotificationHandler::startFetchNotifications() { // check connectivity and credentials if (!(_accountState && _accountState->isConnected() && _accountState->account() && _accountState->account()->credentials() && _accountState->account()->credentials()->ready())) { deleteLater(); - return; + return false; } // check if the account has notifications enabled. If the capabilities are // not yet valid, its assumed that notifications are available. @@ -37,7 +37,7 @@ void ServerNotificationHandler::slotFetchNotifications() if (!_accountState->account()->capabilities().notificationsAvailable()) { qCInfo(lcServerNotification) << "Account" << _accountState->account()->displayName() << "does not have notifications enabled."; deleteLater(); - return; + return false; } } @@ -50,6 +50,7 @@ void ServerNotificationHandler::slotFetchNotifications() _notificationJob->setProperty(propertyAccountStateC, QVariant::fromValue(_accountState)); _notificationJob->addRawHeader("If-None-Match", _accountState->notificationsEtagResponseHeader()); _notificationJob->start(); + return true; } void ServerNotificationHandler::slotEtagResponseHeaderReceived(const QByteArray &value, int statusCode) @@ -66,12 +67,14 @@ void ServerNotificationHandler::slotNotificationsReceived(const QJsonDocument &j if (statusCode != successStatusCode && statusCode != notModifiedStatusCode) { qCWarning(lcServerNotification) << "Notifications failed with status code " << statusCode; deleteLater(); + emit jobFinished(); return; } if (statusCode == notModifiedStatusCode) { qCWarning(lcServerNotification) << "Status code " << statusCode << " Not Modified - No new notifications."; deleteLater(); + emit jobFinished(); return; } @@ -170,6 +173,7 @@ void ServerNotificationHandler::slotNotificationsReceived(const QJsonDocument &j } emit newNotificationList(list); emit newIncomingCallsList(callList); + emit jobFinished(); deleteLater(); } diff --git a/src/gui/tray/notificationhandler.h b/src/gui/tray/notificationhandler.h index 8e8bd361163e1..8c3a079506610 100644 --- a/src/gui/tray/notificationhandler.h +++ b/src/gui/tray/notificationhandler.h @@ -18,9 +18,10 @@ class ServerNotificationHandler : public QObject signals: void newNotificationList(OCC::ActivityList); void newIncomingCallsList(OCC::ActivityList); + void jobFinished(); -public slots: - void slotFetchNotifications(); +public: + bool startFetchNotifications(); private slots: void slotNotificationsReceived(const QJsonDocument &json, int statusCode); diff --git a/src/gui/tray/usermodel.cpp b/src/gui/tray/usermodel.cpp index 6441a2f0ad627..e9a5ed73ae433 100644 --- a/src/gui/tray/usermodel.cpp +++ b/src/gui/tray/usermodel.cpp @@ -119,6 +119,14 @@ bool User::canShowNotification(const long notificationId) !notificationAlreadyShown(notificationId); } +void User::checkAndRemoveSeenActivities(const ActivityList &list, const int numChatNotificationsReceived) +{ + if (numChatNotificationsReceived < _lastChatNotificationsReceivedCount) { + _activityModel->checkAndRemoveSeenActivities(list); + } + _lastChatNotificationsReceivedCount = numChatNotificationsReceived; +} + void User::showDesktopNotification(const QString &title, const QString &message, const long notificationId) { if(!canShowNotification(notificationId)) { @@ -197,6 +205,11 @@ void User::showDesktopTalkNotification(const Activity &activity) void User::slotBuildNotificationDisplay(const ActivityList &list) { + const auto chatNotificationsReceivedCount = std::count_if(std::cbegin(list), std::cend(list), [](const auto &activity) { + return activity._objectType == QStringLiteral("chat"); + }); + checkAndRemoveSeenActivities(list, chatNotificationsReceivedCount); + ActivityList toNotifyList; std::copy_if(list.constBegin(), list.constEnd(), std::back_inserter(toNotifyList), [&](const Activity &activity) { @@ -221,21 +234,18 @@ void User::slotBuildNotificationDisplay(const ActivityList &list) return; } - auto chatNotificationsReceivedCount = 0; - - for(const auto &activity : qAsConst(toNotifyList)) { + for (const auto &activity : qAsConst(toNotifyList)) { if (activity._objectType == QStringLiteral("chat")) { - ++chatNotificationsReceivedCount; showDesktopTalkNotification(activity); } else { showDesktopNotification(activity); } } +} - if (chatNotificationsReceivedCount < _lastChatNotificationsReceivedCount) { - _activityModel->checkAndRemoveSeenActivities(toNotifyList); - } - _lastChatNotificationsReceivedCount = chatNotificationsReceivedCount; +void User::slotNotificationFetchFinished() +{ + _isNotificationFetchRunning = false; } void User::slotBuildIncomingCallDialogs(const ActivityList &list) @@ -440,13 +450,18 @@ void User::slotRefreshNotifications() // start a server notification handler if no notification requests // are running if (_notificationRequestsRunning == 0) { + if (_isNotificationFetchRunning) { + qCDebug(lcActivity) << "Notification fetch is already running."; + return; + } auto *snh = new ServerNotificationHandler(_account.data()); connect(snh, &ServerNotificationHandler::newNotificationList, this, &User::slotBuildNotificationDisplay); connect(snh, &ServerNotificationHandler::newIncomingCallsList, this, &User::slotBuildIncomingCallDialogs); - - snh->slotFetchNotifications(); + connect(snh, &ServerNotificationHandler::jobFinished, + this, &User::slotNotificationFetchFinished); + _isNotificationFetchRunning = snh->startFetchNotifications(); } else { qCWarning(lcActivity) << "Notification request counter not zero."; } diff --git a/src/gui/tray/usermodel.h b/src/gui/tray/usermodel.h index 2b635c66a2dbe..bfeaff46d2487 100644 --- a/src/gui/tray/usermodel.h +++ b/src/gui/tray/usermodel.h @@ -124,6 +124,7 @@ public slots: void slotNotifyServerFinished(const QString &reply, int replyCode); void slotSendNotificationRequest(const QString &accountName, const QString &link, const QByteArray &verb, int row); void slotBuildNotificationDisplay(const OCC::ActivityList &list); + void slotNotificationFetchFinished(); void slotBuildIncomingCallDialogs(const OCC::ActivityList &list); void slotRefreshNotifications(); void slotRefreshActivitiesInitial(); @@ -163,6 +164,8 @@ private slots: bool notificationAlreadyShown(const long notificationId); bool canShowNotification(const long notificationId); + void checkAndRemoveSeenActivities(const ActivityList &list, const int numChatNotificationsReceived); + AccountStatePtr _account; bool _isCurrentUser; ActivityListModel *_activityModel; @@ -184,6 +187,8 @@ private slots: int _notificationRequestsRunning = 0; int _lastChatNotificationsReceivedCount = 0; + + bool _isNotificationFetchRunning = false; }; class UserModel : public QAbstractListModel From 5991e780f553d6388a44461944c5783a8327fc52 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 18 Jul 2023 09:53:12 +0200 Subject: [PATCH 2/2] try to force use of python 3.8 for windows as required by craft Signed-off-by: Matthieu Gallien --- .github/workflows/windows-build-and-test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/windows-build-and-test.yml b/.github/workflows/windows-build-and-test.yml index 5933928531c23..7ba4ae864a743 100644 --- a/.github/workflows/windows-build-and-test.yml +++ b/.github/workflows/windows-build-and-test.yml @@ -19,6 +19,9 @@ jobs: with: fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis + - uses: actions/setup-python@v1 + with: + python-version: '3.8' - name: Install Craft Master with Nextcloud Client Deps shell: pwsh run: |