diff --git a/connector/syncthingconnection.cpp b/connector/syncthingconnection.cpp index d161242..880a744 100644 --- a/connector/syncthingconnection.cpp +++ b/connector/syncthingconnection.cpp @@ -66,7 +66,7 @@ SyncthingConnection::SyncthingConnection(const QString &syncthingUrl, const QByt , m_apiKey(apiKey) , m_status(SyncthingStatus::Disconnected) , m_keepPolling(false) - , m_reconnecting(false) + , m_abortingToReconnect(false) , m_requestCompletion(false) , m_lastEventId(0) , m_lastDiskEventId(0) @@ -201,7 +201,7 @@ void SyncthingConnection::connect() } // reset status - m_reconnecting = m_hasConfig = m_hasStatus = m_hasEvents = m_hasDiskEvents = false; + m_abortingToReconnect = m_hasConfig = m_hasStatus = m_hasEvents = m_hasDiskEvents = false; // check configuration if (m_apiKey.isEmpty() || m_syncthingUrl.isEmpty()) { @@ -241,7 +241,7 @@ void SyncthingConnection::connectLater(int milliSeconds) */ void SyncthingConnection::disconnect() { - m_reconnecting = m_keepPolling = false; + m_abortingToReconnect = m_keepPolling = false; m_autoReconnectTries = 0; abortAllRequests(); } @@ -296,16 +296,22 @@ void SyncthingConnection::abortAllRequests() */ void SyncthingConnection::reconnect() { + // reset reconnect timer m_autoReconnectTimer.stop(); m_autoReconnectTries = 0; - // reconnect right now if not connected and no pending requests - if (!isConnected() && !hasPendingRequests()) { + + // reset variables to track connection progress + // note: especially resetting events is important as it influences the subsequent hasPendingRequests() call + m_hasConfig = m_hasStatus = m_hasEvents = m_hasDiskEvents = false; + + // reconnect right now if no pending requests to be aborted + if (!hasPendingRequests()) { continueReconnecting(); return; } + // abort pending requests before connecting again - m_keepPolling = m_reconnecting = true; - m_hasConfig = m_hasStatus = m_hasEvents = m_hasDiskEvents = false; + m_keepPolling = m_abortingToReconnect = true; abortAllRequests(); } @@ -332,17 +338,15 @@ void SyncthingConnection::reconnectLater(int milliSeconds) */ void SyncthingConnection::continueReconnecting() { - // postpone if there are still pending requests - if (hasPendingRequests()) { - return; + // notify that we're about to invalidate the configuration if not already invalidated anyways + const auto isConfigInvalidated = m_rawConfig.isEmpty(); + if (!isConfigInvalidated) { + emit newConfig(m_rawConfig = QJsonObject()); } - // invalidate configuration - emit newConfig(QJsonObject()); - // cleanup information from previous connection m_keepPolling = true; - m_reconnecting = false; + m_abortingToReconnect = false; m_lastEventId = 0; m_lastDiskEventId = 0; m_configDir.clear(); @@ -367,6 +371,11 @@ void SyncthingConnection::continueReconnecting() m_lastFileDeleted = false; m_syncthingVersion.clear(); + // notify that the configuration has been invalidated + if (!isConfigInvalidated) { + emit newConfigApplied(); + } + if (m_apiKey.isEmpty() || m_syncthingUrl.isEmpty()) { emit error(tr("Connection configuration is insufficient."), SyncthingErrorCategory::OverallConnection, QNetworkReply::NoError); return; @@ -391,6 +400,8 @@ void SyncthingConnection::concludeReadingConfigAndStatus() readDevs(m_rawConfig.value(QLatin1String("devices")).toArray()); readDirs(m_rawConfig.value(QLatin1String("folders")).toArray()); + emit newConfigApplied(); + continueConnecting(); } @@ -863,10 +874,14 @@ void SyncthingConnection::handleFatalConnectionError() */ void SyncthingConnection::handleAdditionalRequestCanceled() { - if (m_reconnecting) { + // postpone handling if there are still other requests pending + if (hasPendingRequests()) { + return; + } + if (m_abortingToReconnect) { // if reconnection flag is set, instantly etstablish a new connection ... continueReconnecting(); - } else if (!hasPendingRequests()) { + } else { // ... otherwise declare we're disconnected if that was the last pending request setStatus(SyncthingStatus::Disconnected); } diff --git a/connector/syncthingconnection.h b/connector/syncthingconnection.h index 8aa157c..b14c764 100644 --- a/connector/syncthingconnection.h +++ b/connector/syncthingconnection.h @@ -112,6 +112,7 @@ public: static QString statusText(SyncthingStatus status); bool isConnected() const; bool hasPendingRequests() const; + bool hasPendingRequestsIncludingEvents() const; bool hasUnreadNotifications() const; bool hasOutOfSyncDirs() const; @@ -213,6 +214,7 @@ Q_SIGNALS: void newConfig(const QJsonObject &rawConfig); void newDirs(const std::vector &dirs); void newDevices(const std::vector &devs); + void newConfigApplied(); void newEvents(const QJsonArray &events); void dirStatusChanged(const SyncthingDir &dir, int index); void devStatusChanged(const SyncthingDev &dev, int index); @@ -305,6 +307,9 @@ private: QNetworkRequest prepareRequest(const QString &path, const QUrlQuery &query, bool rest = true); QNetworkReply *requestData(const QString &path, const QUrlQuery &query, bool rest = true); QNetworkReply *postData(const QString &path, const QUrlQuery &query, const QByteArray &data = QByteArray()); + QNetworkReply *prepareReply(); + QNetworkReply *prepareReply(QNetworkReply *&expectedReply); + QNetworkReply *prepareReply(QList &expectedReplies); bool pauseResumeDevice(const QStringList &devIds, bool paused); bool pauseResumeDirectory(const QStringList &dirIds, bool paused); SyncthingDir *addDirInfo(std::vector &dirs, const QString &dirId); @@ -316,7 +321,7 @@ private: QString m_password; SyncthingStatus m_status; bool m_keepPolling; - bool m_reconnecting; + bool m_abortingToReconnect; bool m_requestCompletion; int m_lastEventId; int m_lastDiskEventId; diff --git a/connector/syncthingconnection_requests.cpp b/connector/syncthingconnection_requests.cpp index 75b6464..3c3f2f2 100644 --- a/connector/syncthingconnection_requests.cpp +++ b/connector/syncthingconnection_requests.cpp @@ -81,6 +81,65 @@ QNetworkReply *SyncthingConnection::postData(const QString &path, const QUrlQuer return reply; } +/*! + * \brief Prepares the current reply. + */ +QNetworkReply *SyncthingConnection::prepareReply() +{ + auto *const reply = static_cast(sender()); + reply->deleteLater(); + + // skip further processing if aborting to reconnect + if (m_abortingToReconnect) { + handleAdditionalRequestCanceled(); + return nullptr; + } + + return reply; +} + +/*! + * \brief Prepares the current reply. + */ +QNetworkReply *SyncthingConnection::prepareReply(QNetworkReply *&expectedReply) +{ + auto *const reply = static_cast(sender()); + reply->deleteLater(); + + // unset the expected reply so it is no longer considered pending + if (reply == expectedReply) { + expectedReply = nullptr; + } + + // skip further processing if aborting to reconnect + if (m_abortingToReconnect) { + handleAdditionalRequestCanceled(); + return nullptr; + } + + return reply; +} + +/*! + * \brief Prepares the current reply. + */ +QNetworkReply *SyncthingConnection::prepareReply(QList &expectedReplies) +{ + auto *const reply = static_cast(sender()); + reply->deleteLater(); + + // unset the expected reply so it is no longer considered pending + expectedReplies.removeAll(reply); + + // skip further processing if aborting to reconnect + if (m_abortingToReconnect) { + handleAdditionalRequestCanceled(); + return nullptr; + } + + return reply; +} + // pause/resume devices /*! @@ -159,8 +218,11 @@ bool SyncthingConnection::pauseResumeDevice(const QStringList &devIds, bool paus */ void SyncthingConnection::readDevPauseResume() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } + switch (reply->error()) { case QNetworkReply::NoError: { const QStringList devIds(reply->property("devIds").toStringList()); @@ -260,8 +322,11 @@ bool SyncthingConnection::pauseResumeDirectory(const QStringList &dirIds, bool p void SyncthingConnection::readDirPauseResume() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } + switch (reply->error()) { case QNetworkReply::NoError: { const QStringList dirIds(reply->property("dirIds").toStringList()); @@ -326,8 +391,11 @@ void SyncthingConnection::rescan(const QString &dirId, const QString &relpath) */ void SyncthingConnection::readRescan() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } + switch (reply->error()) { case QNetworkReply::NoError: emit rescanTriggered(reply->property("dirId").toString()); @@ -354,8 +422,11 @@ void SyncthingConnection::restart() */ void SyncthingConnection::readRestart() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } + switch (reply->error()) { case QNetworkReply::NoError: emit restartTriggered(); @@ -380,8 +451,11 @@ void SyncthingConnection::shutdown() */ void SyncthingConnection::readShutdown() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } + switch (reply->error()) { case QNetworkReply::NoError: emit shutdownTriggered(); @@ -409,8 +483,10 @@ void SyncthingConnection::requestClearingErrors() */ void SyncthingConnection::readClearingErrors() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } switch (reply->error()) { case QNetworkReply::NoError: @@ -441,10 +517,9 @@ void SyncthingConnection::requestConfig() */ void SyncthingConnection::readConfig() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_configReply) { - m_configReply = nullptr; + auto *const reply = prepareReply(m_configReply); + if (!reply) { + return; } switch (reply->error()) { @@ -469,6 +544,7 @@ void SyncthingConnection::readConfig() readDevs(m_rawConfig.value(QLatin1String("devices")).toArray()); readDirs(m_rawConfig.value(QLatin1String("folders")).toArray()); + emit newConfigApplied(); break; } case QNetworkReply::OperationCanceledError: @@ -580,10 +656,9 @@ void SyncthingConnection::requestStatus() */ void SyncthingConnection::readStatus() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_statusReply) { - m_statusReply = nullptr; + auto *const reply = prepareReply(m_statusReply); + if (!reply) { + return; } switch (reply->error()) { @@ -647,10 +722,9 @@ void SyncthingConnection::requestConnections() */ void SyncthingConnection::readConnections() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_connectionsReply) { - m_connectionsReply = nullptr; + auto *const reply = prepareReply(m_connectionsReply); + if (!reply) { + return; } switch (reply->error()) { @@ -757,10 +831,9 @@ void SyncthingConnection::requestErrors() */ void SyncthingConnection::readErrors() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_errorsReply) { - m_errorsReply = nullptr; + auto *const reply = prepareReply(m_errorsReply); + if (!reply) { + return; } // ignore any errors occured before connecting @@ -826,10 +899,9 @@ void SyncthingConnection::requestDirStatistics() */ void SyncthingConnection::readDirStatistics() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_dirStatsReply) { - m_dirStatsReply = nullptr; + auto *const reply = prepareReply(m_dirStatsReply); + if (!reply) { + return; } switch (reply->error()) { @@ -913,9 +985,10 @@ void SyncthingConnection::requestDirStatus(const QString &dirId) */ void SyncthingConnection::readDirStatus() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - m_otherReplies.removeAll(reply); + auto *const reply = prepareReply(m_otherReplies); + if (!reply) { + return; + } switch (reply->error()) { case QNetworkReply::NoError: { @@ -975,8 +1048,10 @@ void SyncthingConnection::requestDirPullErrors(const QString &dirId, int page, i */ void SyncthingConnection::readDirPullErrors() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } // determine relevant dir int index; @@ -1028,11 +1103,12 @@ void SyncthingConnection::requestCompletion(const QString &devId, const QString */ void SyncthingConnection::readCompletion() { - auto *const reply = static_cast(sender()); + auto *const reply = prepareReply(m_otherReplies); + if (!reply) { + return; + } const auto devId(reply->property("devId").toString()); const auto dirId(reply->property("dirId").toString()); - reply->deleteLater(); - m_otherReplies.removeAll(reply); switch (reply->error()) { case QNetworkReply::NoError: { @@ -1084,10 +1160,9 @@ void SyncthingConnection::requestDeviceStatistics() */ void SyncthingConnection::readDeviceStatistics() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_devStatsReply) { - m_devStatsReply = nullptr; + auto *const reply = prepareReply(m_devStatsReply); + if (!reply) { + return; } switch (reply->error()) { @@ -1145,10 +1220,9 @@ void SyncthingConnection::requestVersion() */ void SyncthingConnection::readVersion() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_versionReply) { - m_versionReply = nullptr; + auto *const reply = prepareReply(m_versionReply); + if (!reply) { + return; } switch (reply->error()) { @@ -1196,8 +1270,10 @@ void SyncthingConnection::requestQrCode(const QString &text) */ void SyncthingConnection::readQrCode() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); + auto *const reply = prepareReply(); + if (!reply) { + return; + } switch (reply->error()) { case QNetworkReply::NoError: @@ -1229,10 +1305,9 @@ void SyncthingConnection::requestLog() */ void SyncthingConnection::readLog() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_logReply) { - m_logReply = nullptr; + auto *const reply = prepareReply(m_logReply); + if (!reply) { + return; } switch (reply->error()) { @@ -1440,10 +1515,9 @@ void SyncthingConnection::requestEvents() */ void SyncthingConnection::readEvents() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_eventsReply) { - m_eventsReply = nullptr; + auto *const reply = prepareReply(m_eventsReply); + if (!reply) { + return; } switch (reply->error()) { @@ -1975,10 +2049,9 @@ void SyncthingConnection::requestDiskEvents(int limit) */ void SyncthingConnection::readDiskEvents() { - auto *const reply = static_cast(sender()); - reply->deleteLater(); - if (reply == m_diskEventsReply) { - m_diskEventsReply = nullptr; + auto *const reply = prepareReply(m_diskEventsReply); + if (!reply) { + return; } switch (reply->error()) { diff --git a/connector/tests/connectiontests.cpp b/connector/tests/connectiontests.cpp index 4fed711..26ce787 100644 --- a/connector/tests/connectiontests.cpp +++ b/connector/tests/connectiontests.cpp @@ -479,8 +479,9 @@ void ConnectionTests::checkDirectories() const void ConnectionTests::testReconnecting() { - cerr << "\n - Reconnecting ..." << endl; + cerr << "\n - Reconnecting ...\n"; waitForConnection(defaultReconnect(), 1000, connectionSignal(&SyncthingConnection::statusChanged)); + cerr << "\n - Waiting for dirs/devs after reconnect ...\n"; waitForAllDirsAndDevsReady(true); CPPUNIT_ASSERT_EQUAL_MESSAGE("connected again", QStringLiteral("connected, paused"), m_connection.statusText()); }