From 8c4d5333c1d9deb88f6759d65ca8396da1d62282 Mon Sep 17 00:00:00 2001 From: allexzander Date: Tue, 26 Jan 2021 16:57:46 +0200 Subject: [PATCH] Use push notifications for Tray activities/notifications fetch trigger. Signed-off-by: allexzander --- src/gui/tray/UserModel.cpp | 86 ++++++++++++++++++++++++--- src/gui/tray/UserModel.h | 9 +++ src/libsync/account.cpp | 1 + src/libsync/account.h | 1 + src/libsync/capabilities.cpp | 8 +++ src/libsync/capabilities.h | 4 +- src/libsync/pushnotifications.cpp | 18 ++++-- src/libsync/pushnotifications.h | 12 +++- test/pushnotificationstestutils.cpp | 2 + test/testcapabilities.cpp | 72 +++++++++++++++++++++++ test/testpushnotifications.cpp | 91 +++++++++++++++++++++++++++-- 11 files changed, 282 insertions(+), 22 deletions(-) diff --git a/src/gui/tray/UserModel.cpp b/src/gui/tray/UserModel.cpp index 31919f43f..cafdcbe48 100644 --- a/src/gui/tray/UserModel.cpp +++ b/src/gui/tray/UserModel.cpp @@ -3,6 +3,7 @@ #include "accountmanager.h" #include "owncloudgui.h" +#include #include "syncengine.h" #include "ocsjob.h" #include "configfile.h" @@ -40,7 +41,7 @@ User::User(AccountStatePtr &account, const bool &isCurrent, QObject *parent) this, &User::slotRefresh); connect(_account.data(), &AccountState::stateChanged, - [=]() { if (isConnected()) {slotRefresh();} }); + [=]() { if (isConnected()) {slotRefreshImmediately();} }); connect(_account.data(), &AccountState::stateChanged, this, &User::accountStateChanged); connect(_account.data(), &AccountState::hasFetchedNavigationApps, this, &User::slotRebuildNavigationAppList); @@ -105,19 +106,90 @@ void User::slotBuildNotificationDisplay(const ActivityList &list) void User::setNotificationRefreshInterval(std::chrono::milliseconds interval) { - qCDebug(lcActivity) << "Starting Notification refresh timer with " << interval.count() / 1000 << " sec interval"; - _notificationCheckTimer.start(interval.count()); + if (!checkPushNotificationsAreReady()) { + qCDebug(lcActivity) << "Starting Notification refresh timer with " << interval.count() / 1000 << " sec interval"; + _notificationCheckTimer.start(interval.count()); + } +} + +void User::slotPushNotificationsReady() +{ + qCInfo(lcActivity) << "Push notifications are ready"; + + if (_notificationCheckTimer.isActive()) { + // as we are now able to use push notifications - let's stop the polling timer + _notificationCheckTimer.stop(); + } + + connectPushNotifications(); +} + +void User::slotDisconnectPushNotifications() +{ + disconnect(_account->account()->pushNotifications(), &PushNotifications::notificationsChanged, this, &User::slotReceivedPushNotification); + disconnect(_account->account()->pushNotifications(), &PushNotifications::activitiesChanged, this, &User::slotReceivedPushActivity); + + disconnect(_account->account().data(), &Account::pushNotificationsDisabled, this, &User::slotDisconnectPushNotifications); + + // connection to WebSocket may have dropped or an error occured, so we need to bring back the polling until we have re-established the connection + setNotificationRefreshInterval(ConfigFile().notificationRefreshInterval()); +} + +void User::slotReceivedPushNotification(Account *account) +{ + if (account->id() == _account->account()->id()) { + slotRefreshNotifications(); + } +} + +void User::slotReceivedPushActivity(Account *account) +{ + if (account->id() == _account->account()->id()) { + slotRefreshActivities(); + } +} + +void User::connectPushNotifications() const +{ + connect(_account->account().data(), &Account::pushNotificationsDisabled, this, &User::slotDisconnectPushNotifications, Qt::UniqueConnection); + + connect(_account->account()->pushNotifications(), &PushNotifications::notificationsChanged, this, &User::slotReceivedPushNotification, Qt::UniqueConnection); + connect(_account->account()->pushNotifications(), &PushNotifications::activitiesChanged, this, &User::slotReceivedPushActivity, Qt::UniqueConnection); +} + +bool User::checkPushNotificationsAreReady() const +{ + const auto pushNotifications = _account->account()->pushNotifications(); + + const auto pushActivitiesAvailable = _account->account()->capabilities().availablePushNotifications() & PushNotificationType::Activities; + const auto pushNotificationsAvailable = _account->account()->capabilities().availablePushNotifications() & PushNotificationType::Notifications; + + const auto pushActivitiesAndNotificationsAvailable = pushActivitiesAvailable && pushNotificationsAvailable; + + if (pushActivitiesAndNotificationsAvailable && pushNotifications && pushNotifications->isReady()) { + connectPushNotifications(); + return true; + } else { + connect(_account->account().data(), &Account::pushNotificationsReady, this, &User::slotPushNotificationsReady, Qt::UniqueConnection); + return false; + } } void User::slotRefreshImmediately() { if (_account.data() && _account.data()->isConnected()) { - this->slotRefreshActivities(); + slotRefreshActivities(); } - this->slotRefreshNotifications(); + slotRefreshNotifications(); } void User::slotRefresh() { + if (checkPushNotificationsAreReady()) { + // we are relying on WebSocket push notifications - ignore refresh attempts from UI + _timeSinceLastCheck[_account.data()].invalidate(); + return; + } + // QElapsedTimer isn't actually constructed as invalid. if (!_timeSinceLastCheck.contains(_account.data())) { _timeSinceLastCheck[_account.data()].invalidate(); @@ -131,9 +203,9 @@ void User::slotRefresh() } if (_account.data() && _account.data()->isConnected()) { if (!timer.isValid()) { - this->slotRefreshActivities(); + slotRefreshActivities(); } - this->slotRefreshNotifications(); + slotRefreshNotifications(); timer.start(); } } diff --git a/src/gui/tray/UserModel.h b/src/gui/tray/UserModel.h index 106ee5692..d9f99f1de 100644 --- a/src/gui/tray/UserModel.h +++ b/src/gui/tray/UserModel.h @@ -71,6 +71,15 @@ public slots: void setNotificationRefreshInterval(std::chrono::milliseconds interval); void slotRebuildNavigationAppList(); +private: + void slotPushNotificationsReady(); + void slotDisconnectPushNotifications(); + void slotReceivedPushNotification(Account *account); + void slotReceivedPushActivity(Account *account); + + void connectPushNotifications() const; + bool checkPushNotificationsAreReady() const; + private: AccountStatePtr _account; bool _isCurrentUser; diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 3aa7e7d8d..5c5613e13 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -216,6 +216,7 @@ void Account::trySetupPushNotifications() qCInfo(lcAccount) << "Delete push notifications object because authentication failed or connection lost"; _pushNotifications->deleteLater(); _pushNotifications = nullptr; + emit pushNotificationsDisabled(this); }; connect(_pushNotifications, &PushNotifications::connectionLost, this, deletePushNotifications); diff --git a/src/libsync/account.h b/src/libsync/account.h index 594bfce6e..bce354118 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -283,6 +283,7 @@ signals: void appPasswordRetrieved(QString); void pushNotificationsReady(Account *account); + void pushNotificationsDisabled(Account *account); protected Q_SLOTS: void slotCredentialsFetched(); diff --git a/src/libsync/capabilities.cpp b/src/libsync/capabilities.cpp index 73d5f4b4d..2f980a748 100644 --- a/src/libsync/capabilities.cpp +++ b/src/libsync/capabilities.cpp @@ -190,6 +190,14 @@ PushNotificationTypes Capabilities::availablePushNotifications() const pushNotificationTypes.setFlag(PushNotificationType::Files); } + if (types.contains("activities")) { + pushNotificationTypes.setFlag(PushNotificationType::Activities); + } + + if (types.contains("notifications")) { + pushNotificationTypes.setFlag(PushNotificationType::Notifications); + } + return pushNotificationTypes; } diff --git a/src/libsync/capabilities.h b/src/libsync/capabilities.h index c1d237024..06ecaf4f1 100644 --- a/src/libsync/capabilities.h +++ b/src/libsync/capabilities.h @@ -28,7 +28,9 @@ class DirectEditor; enum PushNotificationType { None = 0, - Files = 1 + Files = 1, + Activities = 2, + Notifications = 4 }; Q_DECLARE_FLAGS(PushNotificationTypes, PushNotificationType) Q_DECLARE_OPERATORS_FOR_FLAGS(PushNotificationTypes) diff --git a/src/libsync/pushnotifications.cpp b/src/libsync/pushnotifications.cpp index 3aae93ef1..7bb8a8916 100644 --- a/src/libsync/pushnotifications.cpp +++ b/src/libsync/pushnotifications.cpp @@ -73,8 +73,10 @@ void PushNotifications::onWebSocketTextMessageReceived(const QString &message) if (message == "notify_file") { handleNotifyFile(); - } else if (message == "notify_activity" || message == "notify_notification") { - handleNotification(); + } else if (message == "notify_activity") { + handleNotifyActivity(); + } else if (message == "notify_notification") { + handleNotifyNotification(); } else if (message == "authenticated") { handleAuthenticated(); } else if (message == "err: Invalid credentials") { @@ -181,9 +183,15 @@ void PushNotifications::handleInvalidCredentials() } } -void PushNotifications::handleNotification() +void PushNotifications::handleNotifyNotification() { - qCInfo(lcPushNotifications) << "Notification or activity push notification arrived"; - emit notification(_account); + qCInfo(lcPushNotifications) << "Push notification arrived"; + emit notificationsChanged(_account); +} + +void PushNotifications::handleNotifyActivity() +{ + qCInfo(lcPushNotifications) << "Push activity arrived"; + emit activitiesChanged(_account); } } diff --git a/src/libsync/pushnotifications.h b/src/libsync/pushnotifications.h index 3c7e0e02c..2ead09a31 100644 --- a/src/libsync/pushnotifications.h +++ b/src/libsync/pushnotifications.h @@ -64,9 +64,14 @@ signals: void filesChanged(Account *account); /** - * Will be emitted if there is a new notification or activity on the server + * Will be emitted if activities have been changed on the server */ - void notification(Account *account); + void activitiesChanged(Account *account); + + /** + * Will be emitted if notifications have been changed on the server + */ + void notificationsChanged(Account *account); /** * Will be emitted if push notifications are unable to authenticate @@ -100,7 +105,8 @@ private: void handleAuthenticated(); void handleNotifyFile(); void handleInvalidCredentials(); - void handleNotification(); + void handleNotifyNotification(); + void handleNotifyActivity(); Account *_account = nullptr; QWebSocket *_webSocket = nullptr; diff --git a/test/pushnotificationstestutils.cpp b/test/pushnotificationstestutils.cpp index fc787fae2..8dde9f930 100644 --- a/test/pushnotificationstestutils.cpp +++ b/test/pushnotificationstestutils.cpp @@ -70,6 +70,8 @@ OCC::AccountPtr FakeWebSocketServer::createAccount() QStringList typeList; typeList.append("files"); + typeList.append("activities"); + typeList.append("notifications"); QString websocketUrl("ws://localhost:12345"); diff --git a/test/testcapabilities.cpp b/test/testcapabilities.cpp index 13670e98a..1d85d0a37 100644 --- a/test/testcapabilities.cpp +++ b/test/testcapabilities.cpp @@ -7,6 +7,40 @@ class TestCapabilities : public QObject Q_OBJECT private slots: + void testPushNotificationsAvailable_pushNotificationsForActivitiesAvailable_returnTrue() + { + QStringList typeList; + typeList.append("activities"); + + QVariantMap notifyPushMap; + notifyPushMap["type"] = typeList; + + QVariantMap capabilitiesMap; + capabilitiesMap["notify_push"] = notifyPushMap; + + const auto &capabilities = OCC::Capabilities(capabilitiesMap); + const auto activitiesPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Activities); + + QCOMPARE(activitiesPushNotificationsAvailable, true); + } + + void testPushNotificationsAvailable_pushNotificationsForActivitiesNotAvailable_returnFalse() + { + QStringList typeList; + typeList.append("noactivities"); + + QVariantMap notifyPushMap; + notifyPushMap["type"] = typeList; + + QVariantMap capabilitiesMap; + capabilitiesMap["notify_push"] = notifyPushMap; + + const auto &capabilities = OCC::Capabilities(capabilitiesMap); + const auto activitiesPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Activities); + + QCOMPARE(activitiesPushNotificationsAvailable, false); + } + void testPushNotificationsAvailable_pushNotificationsForFilesAvailable_returnTrue() { QStringList typeList; @@ -41,12 +75,50 @@ private slots: QCOMPARE(filesPushNotificationsAvailable, false); } + void testPushNotificationsAvailable_pushNotificationsForNotificationsAvailable_returnTrue() + { + QStringList typeList; + typeList.append("notifications"); + + QVariantMap notifyPushMap; + notifyPushMap["type"] = typeList; + + QVariantMap capabilitiesMap; + capabilitiesMap["notify_push"] = notifyPushMap; + + const auto &capabilities = OCC::Capabilities(capabilitiesMap); + const auto notificationsPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Notifications); + + QCOMPARE(notificationsPushNotificationsAvailable, true); + } + + void testPushNotificationsAvailable_pushNotificationsForNotificationsNotAvailable_returnFalse() + { + QStringList typeList; + typeList.append("nonotifications"); + + QVariantMap notifyPushMap; + notifyPushMap["type"] = typeList; + + QVariantMap capabilitiesMap; + capabilitiesMap["notify_push"] = notifyPushMap; + + const auto &capabilities = OCC::Capabilities(capabilitiesMap); + const auto notificationsPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Notifications); + + QCOMPARE(notificationsPushNotificationsAvailable, false); + } + void testPushNotificationsAvailable_pushNotificationsNotAvailable_returnFalse() { const auto &capabilities = OCC::Capabilities(QVariantMap()); + const auto activitiesPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Activities); const auto filesPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Files); + const auto notificationsPushNotificationsAvailable = capabilities.availablePushNotifications().testFlag(OCC::PushNotificationType::Notifications); + QCOMPARE(activitiesPushNotificationsAvailable, false); QCOMPARE(filesPushNotificationsAvailable, false); + QCOMPARE(notificationsPushNotificationsAvailable, false); } void testPushNotificationsWebSocketUrl_urlAvailable_returnUrl() diff --git a/test/testpushnotifications.cpp b/test/testpushnotifications.cpp index d54747274..e5e1010df 100644 --- a/test/testpushnotifications.cpp +++ b/test/testpushnotifications.cpp @@ -84,8 +84,8 @@ private slots: auto account = FakeWebSocketServer::createAccount(); auto credentials = new CredentialsStub(user, password); account->setCredentials(credentials); - QSignalSpy notificationSpy(account->pushNotifications(), &OCC::PushNotifications::notification); - QVERIFY(notificationSpy.isValid()); + QSignalSpy activitySpy(account->pushNotifications(), &OCC::PushNotifications::activitiesChanged); + QVERIFY(activitySpy.isValid()); // Wait for authentication and then send notify_file push notification QVERIFY(processTextMessageSpy.wait()); @@ -94,9 +94,9 @@ private slots: socket->sendTextMessage("notify_activity"); // notification signal should be emitted - QVERIFY(notificationSpy.wait()); - QCOMPARE(notificationSpy.count(), 1); - auto accountFilesChanged = notificationSpy.at(0).at(0).value(); + QVERIFY(activitySpy.wait()); + QCOMPARE(activitySpy.count(), 1); + auto accountFilesChanged = activitySpy.at(0).at(0).value(); QCOMPARE(accountFilesChanged, account.data()); } @@ -111,7 +111,7 @@ private slots: auto account = FakeWebSocketServer::createAccount(); auto credentials = new CredentialsStub(user, password); account->setCredentials(credentials); - QSignalSpy notificationSpy(account->pushNotifications(), &OCC::PushNotifications::notification); + QSignalSpy notificationSpy(account->pushNotifications(), &OCC::PushNotifications::notificationsChanged); QVERIFY(notificationSpy.isValid()); // Wait for authentication and then send notify_file push notification @@ -273,6 +273,85 @@ private slots: auto accountSent = pushNotificationsReady.at(0).at(0).value(); QCOMPARE(accountSent, account.data()); } + + void testAccount_web_socket_connectionLost_emitNotificationsDisabled() + { + FakeWebSocketServer fakeServer; + auto account = FakeWebSocketServer::createAccount(); + QSignalSpy processTextMessageSpy(&fakeServer, &FakeWebSocketServer::processTextMessage); + QVERIFY(processTextMessageSpy.isValid()); + const QString user = "user"; + const QString password = "password"; + auto credentials = new CredentialsStub(user, password); + account->setCredentials(credentials); + + // Need to set reconnect timer interval to zero for tests + account->pushNotifications()->setReconnectTimerInterval(0); + + QSignalSpy connectionLostSpy(account->pushNotifications(), &OCC::PushNotifications::connectionLost); + QVERIFY(connectionLostSpy.isValid()); + + QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled); + QVERIFY(pushNotificationsDisabledSpy.isValid()); + + // Wait for authentication and then sent a network error + processTextMessageSpy.wait(); + QCOMPARE(processTextMessageSpy.count(), 2); + auto socket = processTextMessageSpy.at(0).at(0).value(); + socket->abort(); + + QVERIFY(pushNotificationsDisabledSpy.wait()); + QCOMPARE(pushNotificationsDisabledSpy.count(), 1); + + QCOMPARE(connectionLostSpy.count(), 1); + + auto accountSent = pushNotificationsDisabledSpy.at(0).at(0).value(); + QCOMPARE(accountSent, account.data()); + } + + void testAccount_web_socket_authenticationFailed_emitNotificationsDisabled() + { + FakeWebSocketServer fakeServer; + auto account = FakeWebSocketServer::createAccount(); + QSignalSpy processTextMessageSpy(&fakeServer, &FakeWebSocketServer::processTextMessage); + QVERIFY(processTextMessageSpy.isValid()); + const QString user = "user"; + const QString password = "password"; + auto credentials = new CredentialsStub(user, password); + account->setCredentials(credentials); + + account->pushNotifications()->setReconnectTimerInterval(0); + QSignalSpy authenticationFailedSpy(account->pushNotifications(), &OCC::PushNotifications::authenticationFailed); + QVERIFY(authenticationFailedSpy.isValid()); + + QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled); + QVERIFY(pushNotificationsDisabledSpy.isValid()); + + // Let three authentication attempts fail + QVERIFY(processTextMessageSpy.wait()); + QCOMPARE(processTextMessageSpy.count(), 2); + auto socket = processTextMessageSpy.at(0).at(0).value(); + socket->sendTextMessage("err: Invalid credentials"); + + QVERIFY(processTextMessageSpy.wait()); + QCOMPARE(processTextMessageSpy.count(), 4); + socket = processTextMessageSpy.at(2).at(0).value(); + socket->sendTextMessage("err: Invalid credentials"); + + QVERIFY(processTextMessageSpy.wait()); + QCOMPARE(processTextMessageSpy.count(), 6); + socket = processTextMessageSpy.at(4).at(0).value(); + socket->sendTextMessage("err: Invalid credentials"); + + // Now the authenticationFailed and pushNotificationsDisabled Signals should be emitted + QVERIFY(pushNotificationsDisabledSpy.wait()); + QCOMPARE(pushNotificationsDisabledSpy.count(), 1); + + QCOMPARE(authenticationFailedSpy.count(), 1); + + auto accountSent = pushNotificationsDisabledSpy.at(0).at(0).value(); + QCOMPARE(accountSent, account.data()); + } }; QTEST_GUILESS_MAIN(TestPushNotifications) -- 2.30.2