Better account status aggregation.

Only ever enter error state on non-recoverable errors.
Otherwise:
* Busy state while busy, then go back to online/offline/error.
* If we failed connect during replay/sync we assume we're offline.
* If we failed to login but could connect we have a known error
  condition.
* If we succeeded to replay/sync something we are apprently online.

At the core we have the problem that we have no way of telling wether
we can connect to the server until we actually try (network is not
enough: vpns, firewalls, ....). Further the status always reflects the
latest status, so even if we were in an error state, once we retry we go
out of the error state and either end up back in the error state or not.

When aggregating states we have to similarly adjust the state to the
most relevant among the resources. The states are ordered like this:
* Error
* Busy
* Connected
* Offline

4 files changed, 78 insertions, 38 deletions

diff --git a/common/resourcefacade.cpp b/common/resourcefacade.cpp
index c702777..dee0711 100644
--- a/common/resourcefacade.cpp
+++ b/common/resourcefacade.cpp
@@ -351,7 +351,7 @@ QPair<KAsync::Job<void>, typename Sink::ResultEmitter<typename ApplicationDomain
         const auto resources = Store::read<ApplicationDomain::SinkResource>(query);
         SinkTraceCtx(ctx) << "Found resource belonging to the account " << account.identifier() << " : " << resources;
         auto accountIdentifier = account.identifier();
-        ApplicationDomain::Status status = ApplicationDomain::ConnectedStatus;
+        QList<int> states;
         for (const auto &resource : resources) {
             auto resourceAccess = ResourceAccessFactory::instance().getAccess(resource.identifier(), ResourceConfig::getResourceType(resource.identifier()));
             if (!monitoredResources->contains(resource.identifier())) {
@@ -364,27 +364,20 @@ QPair<KAsync::Job<void>, typename Sink::ResultEmitter<typename ApplicationDomain
                 Q_ASSERT(ret);
                 monitoredResources->insert(resource.identifier());
             }
-
-            //Figure out overall status
-            auto s = resourceAccess->getResourceStatus();
-            switch (s) {
-                case ApplicationDomain::ErrorStatus:
-                    status = ApplicationDomain::ErrorStatus;
-                    break;
-                case ApplicationDomain::OfflineStatus:
-                    if (status == ApplicationDomain::ConnectedStatus) {
-                        status = ApplicationDomain::OfflineStatus;
-                    }
-                    break;
-                case ApplicationDomain::ConnectedStatus:
-                    break;
-                case ApplicationDomain::BusyStatus:
-                    if (status != ApplicationDomain::ErrorStatus) {
-                        status = ApplicationDomain::BusyStatus;
-                    }
-                    break;
-            }
+            states << resourceAccess->getResourceStatus();
         }
+        const auto status = [&] {
+            if (states.contains(ApplicationDomain::ErrorStatus)) {
+                return ApplicationDomain::ErrorStatus;
+            }
+            if (states.contains(ApplicationDomain::BusyStatus)) {
+                return ApplicationDomain::BusyStatus;
+            }
+            if (states.contains(ApplicationDomain::ConnectedStatus)) {
+                return ApplicationDomain::ConnectedStatus;
+            }
+            return ApplicationDomain::OfflineStatus;
+        }();
         account.setStatusStatus(status);
     });
     return qMakePair(KAsync::null<void>(), runner->emitter());
diff --git a/common/synchronizer.cpp b/common/synchronizer.cpp
index 284f867..9451488 100644
--- a/common/synchronizer.cpp
+++ b/common/synchronizer.cpp
@@ -40,6 +40,7 @@ Synchronizer::Synchronizer(const Sink::ResourceContext &context)
     mSyncStorage(Sink::storageLocation(), mResourceContext.instanceId() + ".synchronization", Sink::Storage::DataStore::DataStore::ReadWrite),
     mSyncInProgress(false)
 {
+    mCurrentState.push(ApplicationDomain::Status::OfflineStatus);
     SinkTraceCtx(mLogCtx) << "Starting synchronizer: " << mResourceContext.resourceType << mResourceContext.instanceId();
 }
 
@@ -308,6 +309,23 @@ void Synchronizer::reportProgress(int progress, int total)
     SinkLogCtx(mLogCtx) << "Progress: " << progress << " out of " << total;
 }
 
+void Synchronizer::setStatusFromResult(const KAsync::Error &error, const QString &s, const QByteArray &requestId)
+{
+    if (error) {
+        if (error.errorCode == ApplicationDomain::ConnectionError) {
+            //Couldn't connect, so we assume we don't have a network connection.
+            setStatus(ApplicationDomain::OfflineStatus, s, requestId);
+        } else if (error.errorCode == ApplicationDomain::LoginError) {
+            //If we failed to login altough we could connect that indicates a problem with our setup.
+            setStatus(ApplicationDomain::ErrorStatus, s, requestId);
+        }
+        //We don't know what kind of error this was, so we assume it's transient and don't change ou status.
+    } else {
+        //An operation against the server worked, so we're probably online.
+        setStatus(ApplicationDomain::ConnectedStatus, s, requestId);
+    }
+}
+
 KAsync::Job<void> Synchronizer::processRequest(const SyncRequest &request)
 {
     if (request.options & SyncRequest::RequestFlush) {
@@ -333,26 +351,20 @@ KAsync::Job<void> Synchronizer::processRequest(const SyncRequest &request)
     } else if (request.requestType == Synchronizer::SyncRequest::Synchronization) {
         return KAsync::start([this, request] {
             SinkLogCtx(mLogCtx) << "Synchronizing: " << request.query;
-            emitNotification(Notification::Status, ApplicationDomain::BusyStatus, "Synchronization has started.", request.requestId);
             emitNotification(Notification::Info, ApplicationDomain::SyncInProgress, {}, {}, request.applicableEntities);
         }).then(synchronizeWithSource(request.query)).then([this] {
             //Commit after every request, so implementations only have to commit more if they add a lot of data.
             commit();
         }).then<void>([this, request](const KAsync::Error &error) {
+            setStatusFromResult(error, "Synchronization has ended.", request.requestId);
             if (error) {
                 //Emit notification with error
                 SinkWarningCtx(mLogCtx) << "Synchronization failed: " << error;
-                if (error.errorCode == ApplicationDomain::ConnectionError) {
-                    emitNotification(Notification::Status, ApplicationDomain::OfflineStatus, "Synchronization has ended.", request.requestId);
-                } else {
-                    emitNotification(Notification::Status, ApplicationDomain::ErrorStatus, "Synchronization has ended.", request.requestId);
-                }
                 emitNotification(Notification::Warning, ApplicationDomain::SyncError, {}, {}, request.applicableEntities);
                 return KAsync::error(error);
             } else {
                 SinkLogCtx(mLogCtx) << "Done Synchronizing";
                 emitNotification(Notification::Info, ApplicationDomain::SyncSuccess, {}, {}, request.applicableEntities);
-                emitNotification(Notification::Status, ApplicationDomain::ConnectedStatus, "Synchronization has ended.", request.requestId);
                 return KAsync::null();
             }
         });
@@ -377,21 +389,15 @@ KAsync::Job<void> Synchronizer::processRequest(const SyncRequest &request)
         } else {
             return KAsync::start([this, request] {
                 SinkLogCtx(mLogCtx) << "Replaying changes.";
-                emitNotification(Notification::Status, ApplicationDomain::BusyStatus, "Changereplay has started.", "changereplay");
             })
             .then(replayNextRevision())
             .then<void>([this, request](const KAsync::Error &error) {
+                setStatusFromResult(error, "Changereplay has ended.", "changereplay");
                 if (error) {
                     SinkWarningCtx(mLogCtx) << "Changereplay failed: " << error.errorMessage;
-                    if (error.errorCode == ApplicationDomain::ConnectionError) {
-                        emitNotification(Notification::Status, ApplicationDomain::OfflineStatus, "Changereplay has ended.", "changereplay");
-                    } else {
-                        emitNotification(Notification::Status, ApplicationDomain::ErrorStatus, "Changereplay has ended.", "changereplay");
-                    }
                     return KAsync::error(error);
                 } else {
                     SinkLogCtx(mLogCtx) << "Done replaying changes";
-                    emitNotification(Notification::Status, ApplicationDomain::ConnectedStatus, "All changes have been replayed.", "changereplay");
                     return KAsync::null();
                 }
             });
@@ -403,6 +409,34 @@ KAsync::Job<void> Synchronizer::processRequest(const SyncRequest &request)
 
 }
 
+void Synchronizer::setStatus(ApplicationDomain::Status state, const QString &reason, const QByteArray requestId)
+{
+    if (state != mCurrentState.top()) {
+        if (mCurrentState.top() == ApplicationDomain::BusyStatus) {
+            mCurrentState.pop();
+        }
+        mCurrentState.push(state);
+        emitNotification(Notification::Status, state, reason, requestId);
+    }
+}
+
+void Synchronizer::resetStatus(const QByteArray requestId)
+{
+    mCurrentState.pop();
+    emitNotification(Notification::Status, mCurrentState.top(), {}, requestId);
+}
+
+void Synchronizer::setBusy(bool busy, const QString &reason, const QByteArray requestId)
+{
+    if (busy) {
+        setStatus(ApplicationDomain::BusyStatus, reason, requestId);
+    } else {
+        if (mCurrentState.top() == ApplicationDomain::BusyStatus) {
+            resetStatus(requestId);
+        }
+    }
+}
+
 KAsync::Job<void> Synchronizer::processSyncQueue()
 {
     if (mSyncRequestQueue.isEmpty()) {
@@ -421,14 +455,20 @@ KAsync::Job<void> Synchronizer::processSyncQueue()
     }
 
     const auto request = mSyncRequestQueue.takeFirst();
-    return KAsync::start([this] {
+    return KAsync::start([=] {
         mMessageQueue->startTransaction();
         mEntityStore->startTransaction(Sink::Storage::DataStore::ReadOnly);
         mSyncInProgress = true;
+        if (request.requestType == Synchronizer::SyncRequest::Synchronization) {
+            setBusy(true, "Synchronization has started.", request.requestId);
+        } else if (request.requestType == Synchronizer::SyncRequest::ChangeReplay) {
+            setBusy(true, "ChangeReplay has started.", "changereplay");
+        }
     })
     .then(processRequest(request))
     .then<void>([this, request](const KAsync::Error &error) {
         SinkTraceCtx(mLogCtx) << "Sync request processed";
+        setBusy(false, {}, request.requestId);
         mEntityStore->abortTransaction();
         mSyncTransaction.abort();
         mMessageQueue->commit();
diff --git a/common/synchronizer.h b/common/synchronizer.h
index 32e93c4..b1ee122 100644
--- a/common/synchronizer.h
+++ b/common/synchronizer.h
@@ -21,6 +21,7 @@
 
 #include "sink_export.h"
 #include <QObject>
+#include <QStack>
 #include <KAsync/Async>
 #include <domainadaptor.h>
 #include <query.h>
@@ -193,6 +194,12 @@ protected:
     Sink::Log::Context mLogCtx;
 
 private:
+    QStack<ApplicationDomain::Status> mCurrentState;
+    void setStatusFromResult(const KAsync::Error &error, const QString &s, const QByteArray &requestId);
+    void setStatus(ApplicationDomain::Status busy, const QString &reason, const QByteArray requestId);
+    void resetStatus(const QByteArray requestId);
+    void setBusy(bool busy, const QString &reason, const QByteArray requestId);
+
     void modifyIfChanged(Storage::EntityStore &store, const QByteArray &bufferType, const QByteArray &sinkId, const Sink::ApplicationDomain::ApplicationDomainType &entity);
     KAsync::Job<void> processRequest(const SyncRequest &request);
     KAsync::Job<void> processSyncQueue();
diff --git a/tests/notificationtest.cpp b/tests/notificationtest.cpp
index 375bfc4..a34d325 100644
--- a/tests/notificationtest.cpp
+++ b/tests/notificationtest.cpp
@@ -71,8 +71,8 @@ private slots:
         QTRY_COMPARE(statusNotifications.size(), 3);
         //Sync
         QCOMPARE(statusNotifications.at(0).code, static_cast<int>(ApplicationDomain::Status::ConnectedStatus));
-        QCOMPARE(statusNotifications.at(1).code, static_cast<int>(Sink::ApplicationDomain::Status::BusyStatus));
-        QCOMPARE(statusNotifications.at(2).code, static_cast<int>(Sink::ApplicationDomain::Status::ConnectedStatus));
+        QCOMPARE(statusNotifications.at(1).code, static_cast<int>(ApplicationDomain::Status::BusyStatus));
+        QCOMPARE(statusNotifications.at(2).code, static_cast<int>(ApplicationDomain::Status::ConnectedStatus));
         //Changereplay
         // It can happen that we get a changereplay notification pair first and then a second one at the end,
         // we therefore currently filter all changereplay notifications (see above).