diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp index e7689f2e8..a5dadb0b3 100644 --- a/src/gui/accountstate.cpp +++ b/src/gui/accountstate.cpp @@ -328,6 +328,9 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta // Get the Apps available on the server. fetchNavigationApps(); + + // Setup push notifications after a successful connection + account()->trySetupPushNotifications(); } break; case ConnectionValidator::Undefined: diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp index 012d13360..ac89dc754 100644 --- a/src/libsync/account.cpp +++ b/src/libsync/account.cpp @@ -215,6 +215,10 @@ void Account::trySetupPushNotifications() const auto deletePushNotifications = [this]() { qCInfo(lcAccount) << "Delete push notifications object because authentication failed or connection lost"; + if (!_pushNotifications) { + return; + } + Q_ASSERT(!_pushNotifications->isReady()); _pushNotifications->deleteLater(); _pushNotifications = nullptr; emit pushNotificationsDisabled(this); diff --git a/src/libsync/account.h b/src/libsync/account.h index bce354118..e20b8c3a6 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -251,6 +251,7 @@ public: // Check for the directEditing capability void fetchDirectEditors(const QUrl &directEditingURL, const QString &directEditingETag); + void trySetupPushNotifications(); PushNotifications *pushNotifications() const; public slots: @@ -293,7 +294,6 @@ protected Q_SLOTS: private: Account(QObject *parent = nullptr); void setSharedThis(AccountPtr sharedThis); - void trySetupPushNotifications(); QWeakPointer _sharedThis; QString _id; diff --git a/src/libsync/pushnotifications.cpp b/src/libsync/pushnotifications.cpp index 7bb8a8916..71bc391bc 100644 --- a/src/libsync/pushnotifications.cpp +++ b/src/libsync/pushnotifications.cpp @@ -4,6 +4,7 @@ namespace { static constexpr int MAX_ALLOWED_FAILED_AUTHENTICATION_ATTEMPTS = 3; +static constexpr int PING_INTERVAL = 30 * 1000; } namespace OCC { @@ -14,6 +15,13 @@ PushNotifications::PushNotifications(Account *account, QObject *parent) : QObject(parent) , _account(account) { + connect(&_pingTimer, &QTimer::timeout, this, &PushNotifications::pingWebSocketServer); + _pingTimer.setSingleShot(true); + _pingTimer.setInterval(PING_INTERVAL); + + connect(&_pingTimedOutTimer, &QTimer::timeout, this, &PushNotifications::onPingTimedOut); + _pingTimedOutTimer.setSingleShot(true); + _pingTimedOutTimer.setInterval(PING_INTERVAL); } PushNotifications::~PushNotifications() @@ -23,7 +31,7 @@ PushNotifications::~PushNotifications() void PushNotifications::setup() { - _isReady = false; + qCInfo(lcPushNotifications) << "Setup push notifications"; _failedAuthenticationAttemptsCount = 0; reconnectToWebSocket(); } @@ -36,15 +44,25 @@ void PushNotifications::reconnectToWebSocket() void PushNotifications::closeWebSocket() { + qCInfo(lcPushNotifications) << "Close websocket" << _webSocket << "for account" << _account->url(); + + _pingTimer.stop(); + _pingTimedOutTimer.stop(); + _isReady = false; + + // Maybe there run some reconnection attempts + if (_reconnectTimer) { + _reconnectTimer->stop(); + } + if (_webSocket) { - qCInfo(lcPushNotifications) << "Close websocket"; _webSocket->close(); } } void PushNotifications::onWebSocketConnected() { - qCInfo(lcPushNotifications) << "Connected to websocket"; + qCInfo(lcPushNotifications) << "Connected to websocket" << _webSocket << "for account" << _account->url(); connect(_webSocket, &QWebSocket::textMessageReceived, this, &PushNotifications::onWebSocketTextMessageReceived, Qt::UniqueConnection); @@ -64,7 +82,7 @@ void PushNotifications::authenticateOnWebSocket() void PushNotifications::onWebSocketDisconnected() { - qCInfo(lcPushNotifications) << "Disconnected from websocket"; + qCInfo(lcPushNotifications) << "Disconnected from websocket" << _webSocket << "for account" << _account->url(); } void PushNotifications::onWebSocketTextMessageReceived(const QString &message) @@ -93,8 +111,7 @@ void PushNotifications::onWebSocketError(QAbstractSocket::SocketError error) return; } - qCWarning(lcPushNotifications) << "Websocket error" << error; - + qCWarning(lcPushNotifications) << "Websocket error on" << _webSocket << "with account" << _account->url() << error; _isReady = false; emit connectionLost(); } @@ -123,7 +140,7 @@ bool PushNotifications::tryReconnectToWebSocket() void PushNotifications::onWebSocketSslErrors(const QList &errors) { - qCWarning(lcPushNotifications) << "Received websocket ssl errors:" << errors; + qCWarning(lcPushNotifications) << "Websocket ssl errors on" << _webSocket << "with account" << _account->url() << errors; _isReady = false; emit authenticationFailed(); } @@ -135,8 +152,8 @@ void PushNotifications::openWebSocket() const auto webSocketUrl = capabilities.pushNotificationsWebSocketUrl(); if (!_webSocket) { - qCInfo(lcPushNotifications) << "Create websocket"; _webSocket = new QWebSocket(QString(), QWebSocketProtocol::VersionLatest, this); + qCInfo(lcPushNotifications) << "Created websocket" << _webSocket << "for account" << _account->url(); } if (_webSocket) { @@ -144,6 +161,7 @@ void PushNotifications::openWebSocket() connect(_webSocket, &QWebSocket::sslErrors, this, &PushNotifications::onWebSocketSslErrors, Qt::UniqueConnection); connect(_webSocket, &QWebSocket::connected, this, &PushNotifications::onWebSocketConnected, Qt::UniqueConnection); connect(_webSocket, &QWebSocket::disconnected, this, &PushNotifications::onWebSocketDisconnected, Qt::UniqueConnection); + connect(_webSocket, &QWebSocket::pong, this, &PushNotifications::onWebSocketPongReceived, Qt::UniqueConnection); qCInfo(lcPushNotifications) << "Open connection to websocket on:" << webSocketUrl; _webSocket->open(webSocketUrl); @@ -165,20 +183,28 @@ void PushNotifications::handleAuthenticated() qCInfo(lcPushNotifications) << "Authenticated successful on websocket"; _failedAuthenticationAttemptsCount = 0; _isReady = true; + startPingTimer(); emit ready(); + + // We maybe reconnected to websocket while being offline for a + // while. To not miss any notifications that may have happend, + // emit all the signals once. + emitFilesChanged(); + emitNotificationsChanged(); + emitActivitiesChanged(); } void PushNotifications::handleNotifyFile() { qCInfo(lcPushNotifications) << "Files push notification arrived"; - emit filesChanged(_account); + emitFilesChanged(); } void PushNotifications::handleInvalidCredentials() { qCInfo(lcPushNotifications) << "Invalid credentials submitted to websocket"; if (!tryReconnectToWebSocket()) { - _isReady = false; + closeWebSocket(); emit authenticationFailed(); } } @@ -186,12 +212,76 @@ void PushNotifications::handleInvalidCredentials() void PushNotifications::handleNotifyNotification() { qCInfo(lcPushNotifications) << "Push notification arrived"; - emit notificationsChanged(_account); + emitNotificationsChanged(); } void PushNotifications::handleNotifyActivity() { qCInfo(lcPushNotifications) << "Push activity arrived"; + emitActivitiesChanged(); +} + +void PushNotifications::onWebSocketPongReceived(quint64 /*elapsedTime*/, const QByteArray & /*payload*/) +{ + qCDebug(lcPushNotifications) << "Pong received in time"; + // We are fine with every kind of pong and don't care about the + // payload. As long as we receive pongs the server is still alive. + _pongReceivedFromWebSocketServer = true; + startPingTimer(); +} + +void PushNotifications::startPingTimer() +{ + _pingTimedOutTimer.stop(); + _pingTimer.start(); +} + +void PushNotifications::startPingTimedOutTimer() +{ + _pingTimedOutTimer.start(); +} + +void PushNotifications::pingWebSocketServer() +{ + Q_ASSERT(_webSocket); + qCDebug(lcPushNotifications, "Ping websocket server"); + + _pongReceivedFromWebSocketServer = false; + + _webSocket->ping({}); + startPingTimedOutTimer(); +} + +void PushNotifications::onPingTimedOut() +{ + if (_pongReceivedFromWebSocketServer) { + qCDebug(lcPushNotifications) << "Websocket respond with a pong in time."; + return; + } + + qCInfo(lcPushNotifications) << "Websocket did not respond with a pong in time. Try to reconnect."; + // Try again to connect + setup(); +} + +void PushNotifications::setPingInterval(int timeoutInterval) +{ + _pingTimer.setInterval(timeoutInterval); + _pingTimedOutTimer.setInterval(timeoutInterval); +} + +void PushNotifications::emitFilesChanged() +{ + emit filesChanged(_account); +} + +void PushNotifications::emitNotificationsChanged() +{ + emit notificationsChanged(_account); +} + +void PushNotifications::emitActivitiesChanged() +{ emit activitiesChanged(_account); } } diff --git a/src/libsync/pushnotifications.h b/src/libsync/pushnotifications.h index 2ead09a31..ebfc9ef09 100644 --- a/src/libsync/pushnotifications.h +++ b/src/libsync/pushnotifications.h @@ -42,6 +42,8 @@ public: /** * Set the interval for reconnection attempts + * + * @param interval Interval in milliseconds. */ void setReconnectTimerInterval(uint32_t interval); @@ -52,6 +54,15 @@ public: */ bool isReady() const; + /** + * Set the interval in which the websocket will ping the server if it is still alive. + * + * If the websocket does not respond in timeoutInterval, the connection will be terminated. + * + * @param interval Interval in milliseconds. + */ + void setPingInterval(int interval); + signals: /** * Will be emitted after a successful connection and authentication @@ -93,6 +104,8 @@ private slots: void onWebSocketTextMessageReceived(const QString &message); void onWebSocketError(QAbstractSocket::SocketError error); void onWebSocketSslErrors(const QList &errors); + void onWebSocketPongReceived(quint64 elapsedTime, const QByteArray &payload); + void onPingTimedOut(); private: void openWebSocket(); @@ -101,6 +114,9 @@ private: void authenticateOnWebSocket(); bool tryReconnectToWebSocket(); void initReconnectTimer(); + void pingWebSocketServer(); + void startPingTimer(); + void startPingTimedOutTimer(); void handleAuthenticated(); void handleNotifyFile(); @@ -108,12 +124,19 @@ private: void handleNotifyNotification(); void handleNotifyActivity(); + void emitFilesChanged(); + void emitNotificationsChanged(); + void emitActivitiesChanged(); + Account *_account = nullptr; QWebSocket *_webSocket = nullptr; uint8_t _failedAuthenticationAttemptsCount = 0; QTimer *_reconnectTimer = nullptr; uint32_t _reconnectTimerInterval = 20 * 1000; bool _isReady = false; -}; + QTimer _pingTimer; + QTimer _pingTimedOutTimer; + bool _pongReceivedFromWebSocketServer = false; +}; } diff --git a/test/testpushnotifications.cpp b/test/testpushnotifications.cpp index 161ae94b8..969f0da8f 100644 --- a/test/testpushnotifications.cpp +++ b/test/testpushnotifications.cpp @@ -230,6 +230,31 @@ private slots: auto accountSent = pushNotificationsDisabledSpy.at(0).at(0).value(); QCOMPARE(accountSent, account.data()); } + + void testPingTimeout_pingTimedOut_reconnect() + { + FakeWebSocketServer fakeServer; + std::unique_ptr filesChangedSpy; + std::unique_ptr notificationsChangedSpy; + std::unique_ptr activitiesChangedSpy; + auto account = FakeWebSocketServer::createAccount(); + QVERIFY(fakeServer.authenticateAccount(account)); + + // Set the ping timeout interval to zero and check if the server attemps to authenticate again + fakeServer.clearTextMessages(); + account->pushNotifications()->setPingInterval(0); + QVERIFY(fakeServer.authenticateAccount( + account, [&](OCC::PushNotifications *pushNotifications) { + filesChangedSpy.reset(new QSignalSpy(pushNotifications, &OCC::PushNotifications::filesChanged)); + notificationsChangedSpy.reset(new QSignalSpy(pushNotifications, &OCC::PushNotifications::notificationsChanged)); + activitiesChangedSpy.reset(new QSignalSpy(pushNotifications, &OCC::PushNotifications::activitiesChanged)); + }, + [&] { + QVERIFY(verifyCalledOnceWithAccount(*filesChangedSpy, account)); + QVERIFY(verifyCalledOnceWithAccount(*notificationsChangedSpy, account)); + QVERIFY(verifyCalledOnceWithAccount(*activitiesChangedSpy, account)); + })); + } }; QTEST_GUILESS_MAIN(TestPushNotifications)