mirror of
https://github.com/nextcloud/desktop.git
synced 2024-11-26 23:28:14 +03:00
Merge pull request #3193 from nextcloud/bugfix/pn-reconnect-forever-if-possible
Push notifications: reconnect forever if possible
This commit is contained in:
commit
c37a4a6452
5 changed files with 137 additions and 79 deletions
|
@ -47,6 +47,10 @@
|
|||
|
||||
using namespace QKeychain;
|
||||
|
||||
namespace {
|
||||
constexpr int pushNotificationsReconnectInterval = 1000 * 60 * 2;
|
||||
}
|
||||
|
||||
namespace OCC {
|
||||
|
||||
Q_LOGGING_CATEGORY(lcAccount, "nextcloud.sync.account", QtInfoMsg)
|
||||
|
@ -59,6 +63,9 @@ Account::Account(QObject *parent)
|
|||
{
|
||||
qRegisterMetaType<AccountPtr>("AccountPtr");
|
||||
qRegisterMetaType<Account *>("Account*");
|
||||
|
||||
_pushNotificationsReconnectTimer.setInterval(pushNotificationsReconnectInterval);
|
||||
connect(&_pushNotificationsReconnectTimer, &QTimer::timeout, this, &Account::trySetupPushNotifications);
|
||||
}
|
||||
|
||||
AccountPtr Account::create()
|
||||
|
@ -203,29 +210,42 @@ void Account::setCredentials(AbstractCredentials *cred)
|
|||
trySetupPushNotifications();
|
||||
}
|
||||
|
||||
void Account::setPushNotificationsReconnectInterval(int interval)
|
||||
{
|
||||
_pushNotificationsReconnectTimer.setInterval(interval);
|
||||
}
|
||||
|
||||
void Account::trySetupPushNotifications()
|
||||
{
|
||||
// Stop the timer to prevent parallel setup attempts
|
||||
_pushNotificationsReconnectTimer.stop();
|
||||
|
||||
if (_capabilities.availablePushNotifications() != PushNotificationType::None) {
|
||||
qCInfo(lcAccount) << "Try to setup push notifications";
|
||||
|
||||
if (!_pushNotifications) {
|
||||
_pushNotifications = new PushNotifications(this, this);
|
||||
|
||||
connect(_pushNotifications, &PushNotifications::ready, this, [this]() { emit pushNotificationsReady(this); });
|
||||
connect(_pushNotifications, &PushNotifications::ready, this, [this]() {
|
||||
_pushNotificationsReconnectTimer.stop();
|
||||
emit pushNotificationsReady(this);
|
||||
});
|
||||
|
||||
const auto deletePushNotifications = [this]() {
|
||||
qCInfo(lcAccount) << "Delete push notifications object because authentication failed or connection lost";
|
||||
const auto disablePushNotifications = [this]() {
|
||||
qCInfo(lcAccount) << "Disable push notifications object because authentication failed or connection lost";
|
||||
if (!_pushNotifications) {
|
||||
return;
|
||||
}
|
||||
Q_ASSERT(!_pushNotifications->isReady());
|
||||
_pushNotifications->deleteLater();
|
||||
_pushNotifications = nullptr;
|
||||
emit pushNotificationsDisabled(this);
|
||||
if (!_pushNotifications->isReady()) {
|
||||
emit pushNotificationsDisabled(this);
|
||||
}
|
||||
if (!_pushNotificationsReconnectTimer.isActive()) {
|
||||
_pushNotificationsReconnectTimer.start();
|
||||
}
|
||||
};
|
||||
|
||||
connect(_pushNotifications, &PushNotifications::connectionLost, this, deletePushNotifications);
|
||||
connect(_pushNotifications, &PushNotifications::authenticationFailed, this, deletePushNotifications);
|
||||
connect(_pushNotifications, &PushNotifications::connectionLost, this, disablePushNotifications);
|
||||
connect(_pushNotifications, &PushNotifications::authenticationFailed, this, disablePushNotifications);
|
||||
}
|
||||
// If push notifications already running it is no problem to call setup again
|
||||
_pushNotifications->setup();
|
||||
|
|
|
@ -253,6 +253,7 @@ public:
|
|||
|
||||
void trySetupPushNotifications();
|
||||
PushNotifications *pushNotifications() const;
|
||||
void setPushNotificationsReconnectInterval(int interval);
|
||||
|
||||
public slots:
|
||||
/// Used when forgetting credentials
|
||||
|
@ -299,6 +300,7 @@ private:
|
|||
QString _id;
|
||||
QString _davUser;
|
||||
QString _displayName;
|
||||
QTimer _pushNotificationsReconnectTimer;
|
||||
#ifndef TOKEN_AUTH_ONLY
|
||||
QImage _avatarImg;
|
||||
#endif
|
||||
|
|
|
@ -1,3 +1,17 @@
|
|||
/*
|
||||
* Copyright (C) by Felix Weilbach <felix.weilbach@nextcloud.com>
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation; either version 2 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful, but
|
||||
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
|
||||
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
|
||||
* for more details.
|
||||
*/
|
||||
|
||||
#include "pushnotifications.h"
|
||||
#include "creds/abstractcredentials.h"
|
||||
#include "account.h"
|
||||
|
@ -14,7 +28,14 @@ Q_LOGGING_CATEGORY(lcPushNotifications, "nextcloud.sync.pushnotifications", QtIn
|
|||
PushNotifications::PushNotifications(Account *account, QObject *parent)
|
||||
: QObject(parent)
|
||||
, _account(account)
|
||||
, _webSocket(new QWebSocket(QString(), QWebSocketProtocol::VersionLatest, this))
|
||||
{
|
||||
connect(_webSocket, QOverload<QAbstractSocket::SocketError>::of(&QWebSocket::error), this, &PushNotifications::onWebSocketError);
|
||||
connect(_webSocket, &QWebSocket::sslErrors, this, &PushNotifications::onWebSocketSslErrors);
|
||||
connect(_webSocket, &QWebSocket::connected, this, &PushNotifications::onWebSocketConnected);
|
||||
connect(_webSocket, &QWebSocket::disconnected, this, &PushNotifications::onWebSocketDisconnected);
|
||||
connect(_webSocket, &QWebSocket::pong, this, &PushNotifications::onWebSocketPongReceived);
|
||||
|
||||
connect(&_pingTimer, &QTimer::timeout, this, &PushNotifications::pingWebSocketServer);
|
||||
_pingTimer.setSingleShot(true);
|
||||
_pingTimer.setInterval(PING_INTERVAL);
|
||||
|
@ -44,7 +65,7 @@ void PushNotifications::reconnectToWebSocket()
|
|||
|
||||
void PushNotifications::closeWebSocket()
|
||||
{
|
||||
qCInfo(lcPushNotifications) << "Close websocket" << _webSocket << "for account" << _account->url();
|
||||
qCInfo(lcPushNotifications) << "Close websocket for account" << _account->url();
|
||||
|
||||
_pingTimer.stop();
|
||||
_pingTimedOutTimer.stop();
|
||||
|
@ -55,14 +76,12 @@ void PushNotifications::closeWebSocket()
|
|||
_reconnectTimer->stop();
|
||||
}
|
||||
|
||||
if (_webSocket) {
|
||||
_webSocket->close();
|
||||
}
|
||||
_webSocket->close();
|
||||
}
|
||||
|
||||
void PushNotifications::onWebSocketConnected()
|
||||
{
|
||||
qCInfo(lcPushNotifications) << "Connected to websocket" << _webSocket << "for account" << _account->url();
|
||||
qCInfo(lcPushNotifications) << "Connected to websocket for account" << _account->url();
|
||||
|
||||
connect(_webSocket, &QWebSocket::textMessageReceived, this, &PushNotifications::onWebSocketTextMessageReceived, Qt::UniqueConnection);
|
||||
|
||||
|
@ -82,7 +101,7 @@ void PushNotifications::authenticateOnWebSocket()
|
|||
|
||||
void PushNotifications::onWebSocketDisconnected()
|
||||
{
|
||||
qCInfo(lcPushNotifications) << "Disconnected from websocket" << _webSocket << "for account" << _account->url();
|
||||
qCInfo(lcPushNotifications) << "Disconnected from websocket for account" << _account->url();
|
||||
}
|
||||
|
||||
void PushNotifications::onWebSocketTextMessageReceived(const QString &message)
|
||||
|
@ -111,8 +130,8 @@ void PushNotifications::onWebSocketError(QAbstractSocket::SocketError error)
|
|||
return;
|
||||
}
|
||||
|
||||
qCWarning(lcPushNotifications) << "Websocket error on" << _webSocket << "with account" << _account->url() << error;
|
||||
_isReady = false;
|
||||
qCWarning(lcPushNotifications) << "Websocket error on with account" << _account->url() << error;
|
||||
closeWebSocket();
|
||||
emit connectionLost();
|
||||
}
|
||||
|
||||
|
@ -140,8 +159,8 @@ bool PushNotifications::tryReconnectToWebSocket()
|
|||
|
||||
void PushNotifications::onWebSocketSslErrors(const QList<QSslError> &errors)
|
||||
{
|
||||
qCWarning(lcPushNotifications) << "Websocket ssl errors on" << _webSocket << "with account" << _account->url() << errors;
|
||||
_isReady = false;
|
||||
qCWarning(lcPushNotifications) << "Websocket ssl errors on with account" << _account->url() << errors;
|
||||
closeWebSocket();
|
||||
emit authenticationFailed();
|
||||
}
|
||||
|
||||
|
@ -151,21 +170,8 @@ void PushNotifications::openWebSocket()
|
|||
const auto capabilities = _account->capabilities();
|
||||
const auto webSocketUrl = capabilities.pushNotificationsWebSocketUrl();
|
||||
|
||||
if (!_webSocket) {
|
||||
_webSocket = new QWebSocket(QString(), QWebSocketProtocol::VersionLatest, this);
|
||||
qCInfo(lcPushNotifications) << "Created websocket" << _webSocket << "for account" << _account->url();
|
||||
}
|
||||
|
||||
if (_webSocket) {
|
||||
connect(_webSocket, QOverload<QAbstractSocket::SocketError>::of(&QWebSocket::error), this, &PushNotifications::onWebSocketError, Qt::UniqueConnection);
|
||||
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);
|
||||
}
|
||||
qCInfo(lcPushNotifications) << "Open connection to websocket on" << webSocketUrl << "for account" << _account->url();
|
||||
_webSocket->open(webSocketUrl);
|
||||
}
|
||||
|
||||
void PushNotifications::setReconnectTimerInterval(uint32_t interval)
|
||||
|
@ -243,7 +249,6 @@ void PushNotifications::startPingTimedOutTimer()
|
|||
|
||||
void PushNotifications::pingWebSocketServer()
|
||||
{
|
||||
Q_ASSERT(_webSocket);
|
||||
qCDebug(lcPushNotifications, "Ping websocket server");
|
||||
|
||||
_pongReceivedFromWebSocketServer = false;
|
||||
|
|
|
@ -129,7 +129,7 @@ private:
|
|||
void emitActivitiesChanged();
|
||||
|
||||
Account *_account = nullptr;
|
||||
QWebSocket *_webSocket = nullptr;
|
||||
QWebSocket *_webSocket;
|
||||
uint8_t _failedAuthenticationAttemptsCount = 0;
|
||||
QTimer *_reconnectTimer = nullptr;
|
||||
uint32_t _reconnectTimerInterval = 20 * 1000;
|
||||
|
|
|
@ -1,3 +1,17 @@
|
|||
/*
|
||||
* Copyright (C) by Felix Weilbach <felix.weilbach@nextcloud.com>
|
||||
*
|
||||
* This program is free software; you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation; either version 2 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful, but
|
||||
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
|
||||
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
|
||||
* for more details.
|
||||
*/
|
||||
|
||||
#include <QTest>
|
||||
#include <QVector>
|
||||
#include <QWebSocketServer>
|
||||
|
@ -7,17 +21,42 @@
|
|||
#include "pushnotifications.h"
|
||||
#include "pushnotificationstestutils.h"
|
||||
|
||||
bool verifyCalledOnceWithAccount(QSignalSpy &spy, OCC::AccountPtr account)
|
||||
{
|
||||
if (spy.count() != 1) {
|
||||
return false;
|
||||
#define RETURN_FALSE_ON_FAIL(expr) \
|
||||
if (!(expr)) { \
|
||||
return false; \
|
||||
}
|
||||
|
||||
bool verifyCalledOnceWithAccount(QSignalSpy &spy, OCC::AccountPtr account)
|
||||
{
|
||||
RETURN_FALSE_ON_FAIL(spy.count() == 1);
|
||||
auto accountFromSpy = spy.at(0).at(0).value<OCC::Account *>();
|
||||
if (accountFromSpy != account.data()) {
|
||||
return false;
|
||||
RETURN_FALSE_ON_FAIL(accountFromSpy == account.data());
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
bool failThreeAuthenticationAttempts(FakeWebSocketServer &fakeServer, OCC::AccountPtr account)
|
||||
{
|
||||
RETURN_FALSE_ON_FAIL(account);
|
||||
RETURN_FALSE_ON_FAIL(account->pushNotifications());
|
||||
|
||||
account->pushNotifications()->setReconnectTimerInterval(0);
|
||||
|
||||
QSignalSpy authenticationFailedSpy(account->pushNotifications(), &OCC::PushNotifications::authenticationFailed);
|
||||
|
||||
// Let three authentication attempts fail
|
||||
for (uint8_t i = 0; i < 3; ++i) {
|
||||
RETURN_FALSE_ON_FAIL(fakeServer.waitForTextMessages());
|
||||
RETURN_FALSE_ON_FAIL(fakeServer.textMessagesCount() == 2);
|
||||
auto socket = fakeServer.socketForTextMessage(0);
|
||||
fakeServer.clearTextMessages();
|
||||
socket->sendTextMessage("err: Invalid credentials");
|
||||
}
|
||||
|
||||
// Now the authenticationFailed Signal should be emitted
|
||||
RETURN_FALSE_ON_FAIL(authenticationFailedSpy.wait());
|
||||
RETURN_FALSE_ON_FAIL(authenticationFailedSpy.count() == 1);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -124,6 +163,7 @@ private slots:
|
|||
FakeWebSocketServer fakeServer;
|
||||
auto account = FakeWebSocketServer::createAccount();
|
||||
QSignalSpy connectionLostSpy(account->pushNotifications(), &OCC::PushNotifications::connectionLost);
|
||||
QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled);
|
||||
QVERIFY(connectionLostSpy.isValid());
|
||||
|
||||
// Wait for authentication and then sent a network error
|
||||
|
@ -133,38 +173,26 @@ private slots:
|
|||
socket->abort();
|
||||
|
||||
QVERIFY(connectionLostSpy.wait());
|
||||
// Account handled connectionLost signal and deleted PushNotifications
|
||||
QCOMPARE(account->pushNotifications(), nullptr);
|
||||
// Account handled connectionLost signal and disabled push notifications
|
||||
QCOMPARE(pushNotificationsDisabledSpy.count(), 1);
|
||||
}
|
||||
|
||||
void testSetup_maxConnectionAttemptsReached_deletePushNotifications()
|
||||
void testSetup_maxConnectionAttemptsReached_disablePushNotifications()
|
||||
{
|
||||
FakeWebSocketServer fakeServer;
|
||||
auto account = FakeWebSocketServer::createAccount();
|
||||
account->pushNotifications()->setReconnectTimerInterval(0);
|
||||
QSignalSpy authenticationFailedSpy(account->pushNotifications(), &OCC::PushNotifications::authenticationFailed);
|
||||
QVERIFY(authenticationFailedSpy.isValid());
|
||||
QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled);
|
||||
|
||||
// Let three authentication attempts fail
|
||||
for (uint8_t i = 0; i < 3; ++i) {
|
||||
QVERIFY(fakeServer.waitForTextMessages());
|
||||
QCOMPARE(fakeServer.textMessagesCount(), 2);
|
||||
auto socket = fakeServer.socketForTextMessage(0);
|
||||
fakeServer.clearTextMessages();
|
||||
socket->sendTextMessage("err: Invalid credentials");
|
||||
}
|
||||
|
||||
// Now the authenticationFailed Signal should be emitted
|
||||
QVERIFY(authenticationFailedSpy.wait());
|
||||
QCOMPARE(authenticationFailedSpy.count(), 1);
|
||||
// Account deleted the push notifications
|
||||
QCOMPARE(account->pushNotifications(), nullptr);
|
||||
QVERIFY(failThreeAuthenticationAttempts(fakeServer, account));
|
||||
// Account disabled the push notifications
|
||||
QCOMPARE(pushNotificationsDisabledSpy.count(), 1);
|
||||
}
|
||||
|
||||
void testOnWebSocketSslError_sslError_deletePushNotifications()
|
||||
void testOnWebSocketSslError_sslError_disablePushNotifications()
|
||||
{
|
||||
FakeWebSocketServer fakeServer;
|
||||
auto account = FakeWebSocketServer::createAccount();
|
||||
QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled);
|
||||
|
||||
QVERIFY(fakeServer.waitForTextMessages());
|
||||
// FIXME: This a little bit ugly but I had no better idea how to trigger a error on the websocket client.
|
||||
|
@ -173,8 +201,8 @@ private slots:
|
|||
QVERIFY(pushNotificationsWebSocketChildren.size() == 1);
|
||||
emit pushNotificationsWebSocketChildren[0]->sslErrors(QList<QSslError>());
|
||||
|
||||
// Account handled connectionLost signal and deleted PushNotifications
|
||||
QCOMPARE(account->pushNotifications(), nullptr);
|
||||
// Account handled connectionLost signal and the authenticationFailed Signal should be emitted
|
||||
QCOMPARE(pushNotificationsDisabledSpy.count(), 1);
|
||||
}
|
||||
|
||||
void testAccount_web_socket_connectionLost_emitNotificationsDisabled()
|
||||
|
@ -208,25 +236,14 @@ private slots:
|
|||
{
|
||||
FakeWebSocketServer fakeServer;
|
||||
auto account = FakeWebSocketServer::createAccount();
|
||||
account->pushNotifications()->setReconnectTimerInterval(0);
|
||||
QSignalSpy authenticationFailedSpy(account->pushNotifications(), &OCC::PushNotifications::authenticationFailed);
|
||||
QVERIFY(authenticationFailedSpy.isValid());
|
||||
account->setPushNotificationsReconnectInterval(0);
|
||||
QSignalSpy pushNotificationsDisabledSpy(account.data(), &OCC::Account::pushNotificationsDisabled);
|
||||
QVERIFY(pushNotificationsDisabledSpy.isValid());
|
||||
|
||||
// Let three authentication attempts fail
|
||||
for (uint8_t i = 0; i < 3; ++i) {
|
||||
QVERIFY(fakeServer.waitForTextMessages());
|
||||
QCOMPARE(fakeServer.textMessagesCount(), 2);
|
||||
auto socket = fakeServer.socketForTextMessage(0);
|
||||
fakeServer.clearTextMessages();
|
||||
socket->sendTextMessage("err: Invalid credentials");
|
||||
}
|
||||
QVERIFY(failThreeAuthenticationAttempts(fakeServer, account));
|
||||
|
||||
// Now the authenticationFailed and pushNotificationsDisabled Signals should be emitted
|
||||
QVERIFY(pushNotificationsDisabledSpy.wait());
|
||||
// Now the pushNotificationsDisabled Signal should be emitted
|
||||
QCOMPARE(pushNotificationsDisabledSpy.count(), 1);
|
||||
QCOMPARE(authenticationFailedSpy.count(), 1);
|
||||
auto accountSent = pushNotificationsDisabledSpy.at(0).at(0).value<OCC::Account *>();
|
||||
QCOMPARE(accountSent, account.data());
|
||||
}
|
||||
|
@ -255,6 +272,20 @@ private slots:
|
|||
QVERIFY(verifyCalledOnceWithAccount(*activitiesChangedSpy, account));
|
||||
}));
|
||||
}
|
||||
|
||||
void testTryReconnect_capabilitesReportPushNotificationsAvailable_reconnectForEver()
|
||||
{
|
||||
FakeWebSocketServer fakeServer;
|
||||
auto account = FakeWebSocketServer::createAccount();
|
||||
account->setPushNotificationsReconnectInterval(0);
|
||||
|
||||
// Let if fail a few times
|
||||
QVERIFY(failThreeAuthenticationAttempts(fakeServer, account));
|
||||
QVERIFY(failThreeAuthenticationAttempts(fakeServer, account));
|
||||
|
||||
// Push notifications should try to reconnect
|
||||
QVERIFY(fakeServer.authenticateAccount(account));
|
||||
}
|
||||
};
|
||||
|
||||
QTEST_GUILESS_MAIN(TestPushNotifications)
|
||||
|
|
Loading…
Reference in a new issue