Remove PausedDueToMetered state and use Connecting state

Like we do for when the client is behind a captive portal.

src/gui/accountsettings.cpp

@@ -449,9 +449,6 @@ void AccountSettings::slotAccountStateChanged()
                                .arg(Utility::escape(safeUrl.toString()));
 
     switch (state) {
-    case AccountState::PausedDueToMetered:
-        showConnectionLabel(tr("Sync to %1 is paused due to metered internet connection.").arg(server));
-        break;
     case AccountState::Connected: {
         QStringList errors;
         if (account->serverSupportLevel() != Account::ServerSupportLevel::Supported) {
@@ -476,6 +473,8 @@ void AccountSettings::slotAccountStateChanged()
     case AccountState::Connecting:
         if (NetworkInformation::instance()->isBehindCaptivePortal()) {
             showConnectionLabel(tr("Captive portal prevents connections to %1.").arg(server));
+        } else if (NetworkInformation::instance()->isMetered()) {
+            showConnectionLabel(tr("Sync to %1 is paused due to metered internet connection.").arg(server));
         } else {
             showConnectionLabel(tr("Connecting to: %1.").arg(server));
         }

src/gui/accountstate.cpp

@@ -138,19 +138,38 @@ AccountState::AccountState(AccountPtr account)
         if (ConfigFile().pauseSyncWhenMetered()) {
             if (state() == State::Connected && isMetered) {
                 qCInfo(lcAccountState) << "Network switched to a metered connection, setting account state to PausedDueToMetered";
-                setState(State::PausedDueToMetered);
-            } else if (state() == State::PausedDueToMetered && !isMetered) {
+                setState(State::Connecting);
+            } else if (state() == State::Connecting && !isMetered) {
                 qCInfo(lcAccountState) << "Network switched to a NON-metered connection, setting account state to Connected";
                 setState(State::Connected);
             }
         }
     });
 
-    connect(NetworkInformation::instance(), &NetworkInformation::isBehindCaptivePortalChanged, this, [this](bool) {
+    connect(NetworkInformation::instance(), &NetworkInformation::isBehindCaptivePortalChanged, this, [this](bool onoff) {
+        if (onoff) {
+            // Block jobs from starting: they will fail because of the captive portal.
+            // Note: this includes the `Drives` jobs started periodically by the `SpacesManager`.
+            _queueGuard.block();
+        } else {
+            // Empty the jobs queue before unblocking it. The client might have been behind a captive
+            // portal for hours, so a whole bunch of jobs might have queued up. If we wouldn't
+            // clear the queue, unleashing all those jobs might look like a DoS attack. Most of them
+            // are also not very useful anymore (e.g. `Drives` jobs), and the important ones (PUT jobs)
+            // will be rescheduled by a directory scan.
+            _account->jobQueue()->clear();
+            _queueGuard.unblock();
+        }
+
         // A direct connect is not possible, because then the state parameter of `isBehindCaptivePortalChanged`
         // would become the `verifyServerState` argument to `checkConnectivity`.
+        // The call is also made for when we "go behind" a captive portal. That ensures that not
+        // only the status is set to `Connecting`, but also makes the UI show that syncing is paused.
         QTimer::singleShot(0, this, [this] { checkConnectivity(false); });
     });
+    if (NetworkInformation::instance()->isBehindCaptivePortal()) {
+        _queueGuard.block();
+    }
 
     // as a fallback and to recover after server issues we also poll
     auto timer = new QTimer(this);
@@ -172,22 +191,6 @@ AccountState::AccountState(AccountPtr account)
         ownCloudGui::raise();
         msgBox->open();
     });
-
-
-    connect(NetworkInformation::instance(), &NetworkInformation::isBehindCaptivePortalChanged, this, [this](bool onoff) {
-        if (onoff) {
-            _queueGuard.block();
-        } else {
-            // TODO: empty queue?
-            _queueGuard.unblock();
-        }
-    });
-    if (NetworkInformation::instance()->isBehindCaptivePortal()) {
-        _queueGuard.block();
-    } else {
-        // TODO: empty queue?
-        _queueGuard.unblock();
-    }
 }
 
 AccountState::~AccountState() { }
@@ -261,8 +264,11 @@ void AccountState::setState(State state)
             _connectionValidator->deleteLater();
             _connectionValidator.clear();
             checkConnectivity();
-        } else if (_state == Connected && NetworkInformation::instance()->isMetered() && ConfigFile().pauseSyncWhenMetered()) {
-            _state = PausedDueToMetered;
+        } else if (_state == Connected) {
+            if ((NetworkInformation::instance()->isMetered() && ConfigFile().pauseSyncWhenMetered())
+                || NetworkInformation::instance()->isBehindCaptivePortal()) {
+                _state = Connecting;
+            }
         }
     }
 
@@ -323,7 +329,7 @@ void AccountState::signIn()
 
 bool AccountState::isConnected() const
 {
-    return _state == Connected || _state == PausedDueToMetered;
+    return _state == Connected;
 }
 
 void AccountState::tagLastSuccessfullETagRequest(const QDateTime &tp)

src/gui/accountstate.h

@@ -82,11 +82,6 @@ class OWNCLOUDGUI_EXPORT AccountState : public QObject
         /// We are currently asking the user for credentials
         AskingCredentials,
 
-        /// We are on a metered internet connection, and the user preference
-        /// is to pause syncing in this case. This state is entered from and
-        /// left to a `Connected` state.
-        PausedDueToMetered,
-
         Connecting
     };
     Q_ENUM(State)

src/gui/folderman.cpp

@@ -20,6 +20,7 @@
 #include "common/asserts.h"
 #include "configfile.h"
 #include "folder.h"
+#include "gui/networkinformation.h"
 #include "guiutility.h"
 #include "libsync/syncengine.h"
 #include "lockwatcher.h"
@@ -73,7 +74,9 @@ void TrayOverallStatusResult::addResult(Folder *f)
         lastSyncDone = time;
     }
 
-    auto status = f->syncPaused() || f->accountState()->state() == AccountState::PausedDueToMetered ? SyncResult::Paused : f->syncResult().status();
+    auto status = f->syncPaused() || NetworkInformation::instance()->isBehindCaptivePortal() || NetworkInformation::instance()->isMetered()
+        ? SyncResult::Paused
+        : f->syncResult().status();
     if (status == SyncResult::Undefined) {
         status = SyncResult::Problem;
     }

src/gui/folderstatusmodel.cpp

@@ -17,6 +17,7 @@
 #include "accountstate.h"
 #include "common/asserts.h"
 #include "folderman.h"
+#include "gui/networkinformation.h"
 #include "gui/quotainfo.h"
 #include "theme.h"
 
@@ -50,7 +51,7 @@ namespace {
         auto status = f->syncResult();
         if (!f->accountState()->isConnected()) {
             status.setStatus(SyncResult::Status::Offline);
-        } else if (f->syncPaused() || f->accountState()->state() == AccountState::PausedDueToMetered) {
+        } else if (f->syncPaused() || NetworkInformation::instance()->isBehindCaptivePortal() || NetworkInformation::instance()->isMetered()) {
             status.setStatus(SyncResult::Status::Paused);
         }
         return Theme::instance()->syncStateIconName(status);

src/gui/main.cpp

@@ -452,7 +452,7 @@ int main(int argc, char **argv)
         }
 
         setupLogging(options);
-        NetworkInformation::initialize();
+        NetworkInformation::instance(); //
 
         platform->setApplication(&app);
 

src/gui/networkinformation.cpp

@@ -20,9 +20,11 @@ using namespace OCC;
 
 Q_LOGGING_CATEGORY(lcNetInfo, "gui.netinfo", QtInfoMsg)
 
-NetworkInformation *NetworkInformation::_instance;
+NetworkInformation *NetworkInformation::_instance = nullptr;
 
-static void loadQNetworkInformationBackend()
+namespace {
+
+void loadQNetworkInformationBackend()
 {
     if (!QNetworkInformation::loadDefaultBackend()) {
         qCWarning(lcNetInfo) << "Failed to load default backend of QNetworkInformation.";
@@ -44,25 +46,24 @@ static void loadQNetworkInformationBackend()
     }
 }
 
-void NetworkInformation::initialize()
-{
-    if (_instance) {
-        return;
-    }
+} // anonymous namespace
 
-    _instance = new NetworkInformation;
+NetworkInformation::NetworkInformation() { }
 
-    loadQNetworkInformationBackend();
+NetworkInformation *NetworkInformation::instance()
+{
+    if (!_instance) {
+        _instance = new NetworkInformation;
 
-    if (auto qni = QNetworkInformation::instance()) {
-        connect(qni, &QNetworkInformation::isMeteredChanged, _instance, &NetworkInformation::isMeteredChanged);
-        connect(qni, &QNetworkInformation::reachabilityChanged, _instance, &NetworkInformation::reachabilityChanged);
-        connect(qni, &QNetworkInformation::isBehindCaptivePortalChanged, _instance, &NetworkInformation::slotIsBehindCaptivePortalChanged);
+        loadQNetworkInformationBackend();
+
+        if (auto qni = QNetworkInformation::instance()) {
+            connect(qni, &QNetworkInformation::isMeteredChanged, _instance, &NetworkInformation::isMeteredChanged);
+            connect(qni, &QNetworkInformation::reachabilityChanged, _instance, &NetworkInformation::reachabilityChanged);
+            connect(qni, &QNetworkInformation::isBehindCaptivePortalChanged, _instance, &NetworkInformation::slotIsBehindCaptivePortalChanged);
+        }
     }
-}
 
-NetworkInformation *NetworkInformation::instance()
-{
     return _instance;
 }
 

src/gui/networkinformation.h

@@ -31,7 +31,6 @@ class OWNCLOUDGUI_EXPORT NetworkInformation : public QObject
     Q_OBJECT
 
 public:
-    static void initialize();
     static NetworkInformation *instance();
 
     bool isMetered();
@@ -55,6 +54,8 @@ private Q_SLOTS:
     void slotIsBehindCaptivePortalChanged(bool state);
 
 private:
+    NetworkInformation();
+
     static NetworkInformation *_instance;
 
     bool _forcedCaptivePortal = false;

src/libsync/jobqueue.h

@@ -47,14 +47,15 @@ class OWNCLOUDSYNC_EXPORT JobQueue
 
     size_t size() const;
 
-private:
-    void block();
-    void unblock();
     /**
      * Clear the queue and abort all jobs
      */
     void clear();
 
+private:
+    void block();
+    void unblock();
+
     Account *_account;
     uint _blocked = 0;
     std::vector<QPointer<AbstractNetworkJob>> _jobs;

test/testfoldermigration.cpp

@@ -92,7 +92,6 @@ private Q_SLOTS:
 
         // init folder man
         std::ignore = TestUtils::folderMan();
-        NetworkInformation::initialize();
         AccountManager::instance()->restore();
 
         settings->beginGroup(QStringLiteral("0/Folders"));

test/testutils/testutils.cpp

@@ -2,7 +2,6 @@
 
 #include "common/checksums.h"
 #include "gui/accountmanager.h"
-#include "gui/networkinformation.h"
 #include "libsync/creds/httpcredentials.h"
 
 #include <QCoreApplication>
@@ -28,8 +27,6 @@ namespace OCC {
 namespace TestUtils {
     TestUtilsPrivate::AccountStateRaii createDummyAccount()
     {
-        // ensure we have an instance of NetworkInformation
-        NetworkInformation::initialize();
         // ensure we have an instance of folder man
         std::ignore = folderMan();
         // don't use the account manager to create the account, it would try to use widgets

test/testutils/testutilsloader.cpp

@@ -1,4 +1,5 @@
 #include "configfile.h"
+#include "gui/networkinformation.h"
 #include "logger.h"
 #include "resources/loadresources.h"
 #include "testutils.h"
@@ -19,6 +20,9 @@ void setUpTests()
     OCC::Logger::instance()->setLogDebug(true);
 
     OCC::Account::setCommonCacheDirectory(QStringLiteral("%1/cache").arg(dir.path()));
+
+    // ensure we have an instance of NetworkInformation
+    OCC::NetworkInformation::instance();
 }
 Q_COREAPP_STARTUP_FUNCTION(setUpTests)
 }