From 9ea268a6d0f4054c31b2729ecd6cfcc9d07a2d6a Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Fri, 24 Mar 2017 22:15:18 +0100 Subject: Implemented notification support in the model. This will allow us to fold things like progress and sync status directly into the model. Usecases are mail download progress and folder sync progress. Ideally we would also solve the resource/account state through this. --- common/commands/notification.fbs | 3 +- common/domain/applicationdomaintype.h | 48 ++++++----- common/listener.cpp | 3 + common/modelresult.cpp | 90 ++++++++++++++++++++- common/modelresult.h | 12 ++- common/notification.cpp | 2 +- common/notification.h | 1 + common/query.h | 9 ++- common/resourceaccess.cpp | 7 +- common/store.h | 8 +- common/synchronizer.cpp | 6 +- common/synchronizer.h | 2 +- examples/dummyresource/resourcefactory.cpp | 1 + tests/CMakeLists.txt | 2 + tests/dummyresourcetest.cpp | 9 +-- tests/notificationtest.cpp | 124 +++++++++++++++++++++++++++++ 16 files changed, 290 insertions(+), 37 deletions(-) create mode 100644 tests/notificationtest.cpp diff --git a/common/commands/notification.fbs b/common/commands/notification.fbs index c82fad3..517111c 100644 --- a/common/commands/notification.fbs +++ b/common/commands/notification.fbs @@ -2,9 +2,10 @@ namespace Sink.Commands; table Notification { type: int = 0; //See notification.h - identifier: string; //An identifier that links back to the something related to the notification (e.g. an entity id or a command id) + identifier: string; //An identifier that links back to the something related to the notification (e.g. a command id) message: string; code: int = 0; //See notification.h + entities: [string]; //A list of entities this applies to } root_type Notification; diff --git a/common/domain/applicationdomaintype.h b/common/domain/applicationdomaintype.h index ef38d58..6fd2b90 100644 --- a/common/domain/applicationdomaintype.h +++ b/common/domain/applicationdomaintype.h @@ -93,7 +93,7 @@ namespace Sink { namespace ApplicationDomain { -enum ErrorCode { +enum SINK_EXPORT ErrorCode { NoError = 0, UnknownError, NoServerError, @@ -101,10 +101,33 @@ enum ErrorCode { TransmissionError, }; -enum SuccessCode { +enum SINK_EXPORT SuccessCode { TransmissionSuccess }; +enum SINK_EXPORT SyncStatus { + NoSyncStatus, + SyncInProgress, + SyncError, + SyncSuccess +}; + +/** + * The status of an account or resource. + * + * It is set as follows: + * * By default the status is offline. + * * If a connection to the server could be established the status is Connected. + * * If an error occurred that keeps the resource from operating (so non transient), the resource enters the error state. + * * If a long running operation is started the resource goes to the busy state (and return to the previous state after that). + */ +enum SINK_EXPORT Status { + OfflineStatus, + ConnectedStatus, + BusyStatus, + ErrorStatus +}; + struct SINK_EXPORT Error { }; @@ -113,6 +136,11 @@ struct SINK_EXPORT Progress { }; +/** + * Internal type. + * + * Represents a BLOB property. + */ struct BLOB { BLOB() = default; BLOB(const BLOB &) = default; @@ -410,22 +438,6 @@ struct SINK_EXPORT Mail : public Entity { SINK_EXPORT QDebug operator<< (QDebug d, const Mail::Contact &c); -/** - * The status of an account or resource. - * - * It is set as follows: - * * By default the status is offline. - * * If a connection to the server could be established the status is Connected. - * * If an error occurred that keeps the resource from operating (so non transient), the resource enters the error state. - * * If a long running operation is started the resource goes to the busy state (and return to the previous state after that). - */ -enum SINK_EXPORT Status { - OfflineStatus, - ConnectedStatus, - BusyStatus, - ErrorStatus -}; - struct SINK_EXPORT Identity : public ApplicationDomainType { static constexpr const char *name = "identity"; typedef QSharedPointer Ptr; diff --git a/common/listener.cpp b/common/listener.cpp index f18fe1d..96806ad 100644 --- a/common/listener.cpp +++ b/common/listener.cpp @@ -25,6 +25,7 @@ #include "common/definitions.h" #include "common/resourcecontext.h" #include "common/adaptorfactoryregistry.h" +#include "common/bufferutils.h" // commands #include "common/commandcompletion_generated.h" @@ -406,11 +407,13 @@ void Listener::notify(const Sink::Notification ¬ification) { auto messageString = m_fbb.CreateString(notification.message.toUtf8().constData(), notification.message.toUtf8().size()); auto idString = m_fbb.CreateString(notification.id.constData(), notification.id.size()); + auto entities = Sink::BufferUtils::toVector(m_fbb, notification.entities); Sink::Commands::NotificationBuilder builder(m_fbb); builder.add_type(notification.type); builder.add_code(notification.code); builder.add_identifier(idString); builder.add_message(messageString); + builder.add_entities(entities); auto command = builder.Finish(); Sink::Commands::FinishNotificationBuffer(m_fbb, command); for (Client &client : m_connections) { diff --git a/common/modelresult.cpp b/common/modelresult.cpp index 904766d..3edbec7 100644 --- a/common/modelresult.cpp +++ b/common/modelresult.cpp @@ -24,12 +24,20 @@ #include #include "log.h" +#include "notifier.h" +#include "notification.h" + +using namespace Sink; + +static uint getInternalIdentifer(const QByteArray &resourceId, const QByteArray &entityId) +{ + return qHash(resourceId + entityId); +} static uint qHash(const Sink::ApplicationDomain::ApplicationDomainType &type) { - // Q_ASSERT(!type.resourceInstanceIdentifier().isEmpty()); Q_ASSERT(!type.identifier().isEmpty()); - return qHash(type.resourceInstanceIdentifier() + type.identifier()); + return getInternalIdentifer(type.resourceInstanceIdentifier(), type.identifier()); } static qint64 getIdentifier(const QModelIndex &idx) @@ -44,6 +52,79 @@ template ModelResult::ModelResult(const Sink::Query &query, const QList &propertyColumns, const Sink::Log::Context &ctx) : QAbstractItemModel(), mLogCtx(ctx.subContext("modelresult")), mPropertyColumns(propertyColumns), mQuery(query) { + if (query.flags().testFlag(Sink::Query::UpdateStatus)) { + mNotifier.reset(new Sink::Notifier{query}); + mNotifier->registerHandler([this](const Notification ¬ification) { + switch (notification.type) { + case Notification::Status: + case Notification::Warning: + case Notification::Error: + case Notification::Info: + case Notification::Progress: + //These are the notifications we care about + break; + default: + //We're not interested + return; + }; + if (notification.resource.isEmpty()|| notification.entities.isEmpty()) { + return; + } + + QVector idList; + for (const auto &entity : notification.entities) { + auto id = getInternalIdentifer(notification.resource, entity); + if (mEntities.contains(id)) { + idList << id; + } + } + + if (idList.isEmpty()) { + //We don't have this entity in our model + return; + } + const int newStatus = [&] { + if (notification.type == Notification::Warning || notification.type == Notification::Error) { + return ApplicationDomain::SyncStatus::SyncError; + } + if (notification.type == Notification::Info) { + switch (notification.code) { + case ApplicationDomain::SyncInProgress: + return ApplicationDomain::SyncInProgress; + case ApplicationDomain::SyncSuccess: + return ApplicationDomain::SyncSuccess; + case ApplicationDomain::SyncError: + return ApplicationDomain::SyncError; + case ApplicationDomain::NoSyncStatus: + break; + } + return ApplicationDomain::NoSyncStatus; + } + if (notification.type == Notification::Progress) { + return ApplicationDomain::SyncStatus::SyncInProgress; + } + return ApplicationDomain::NoSyncStatus; + }(); + + for (const auto id : idList) { + const auto oldStatus = mEntityStatus.value(id); + QVector changedRoles; + if (oldStatus != newStatus) { + mEntityStatus.insert(id, newStatus); + changedRoles << StatusRole; + } + + if (notification.type == Notification::Progress) { + changedRoles << ProgressRole; + } else if (notification.type == Notification::Warning || notification.type == Notification::Error) { + changedRoles << WarningRole; + } + + const auto idx = createIndexFromId(id); + emit dataChanged(idx, idx, changedRoles); + } + }); + } } template @@ -60,7 +141,7 @@ qint64 ModelResult::parentId(const Ptr &value) if (!mQuery.parentProperty().isEmpty()) { const auto identifier = value->getProperty(mQuery.parentProperty()).toByteArray(); if (!identifier.isEmpty()) { - return qHash(T(value->resourceInstanceIdentifier(), identifier, 0, QSharedPointer())); + return getInternalIdentifer(value->resourceInstanceIdentifier(), identifier); } } return 0; @@ -106,6 +187,9 @@ QVariant ModelResult::data(const QModelIndex &index, int role) const if (role == ChildrenFetchedRole) { return childrenFetched(index); } + if (role == StatusRole) { + return mEntityStatus.value(index.internalId()); + } if (role == Qt::DisplayRole && index.isValid()) { if (index.column() < mPropertyColumns.size()) { Q_ASSERT(mEntities.contains(index.internalId())); diff --git a/common/modelresult.h b/common/modelresult.h index f30a8e1..cc263cf 100644 --- a/common/modelresult.h +++ b/common/modelresult.h @@ -30,15 +30,23 @@ #include "resultprovider.h" #include "threadboundary.h" +namespace Sink { +class Notifier; +} + template class ModelResult : public QAbstractItemModel { public: + //Update the copy in store.h as well if you modify this enum Roles { DomainObjectRole = Qt::UserRole + 1, ChildrenFetchedRole, - DomainObjectBaseRole + DomainObjectBaseRole, + StatusRole, //ApplicationDomain::SyncStatus + WarningRole, //ApplicationDomain::Warning, only if status == warning || status == error + ProgressRole //ApplicationDomain::Progress }; ModelResult(const Sink::Query &query, const QList &propertyColumns, const Sink::Log::Context &); @@ -77,9 +85,11 @@ private: QSet mEntityChildrenFetched; QSet mEntityChildrenFetchComplete; QSet mEntityAllChildrenFetched; + QMap mEntityStatus; QList mPropertyColumns; Sink::Query mQuery; std::function loadEntities; typename Sink::ResultEmitter::Ptr mEmitter; async::ThreadBoundary threadBoundary; + QScopedPointer mNotifier; }; diff --git a/common/notification.cpp b/common/notification.cpp index b399d50..806d04a 100644 --- a/common/notification.cpp +++ b/common/notification.cpp @@ -21,6 +21,6 @@ QDebug operator<<(QDebug dbg, const Sink::Notification &n) { - dbg << "Notification(Id: " << n.id << ", Type: " << n.type << ", Code: " << n.code << ", Message: " << n.message << ")"; + dbg << "Notification(Type: " << n.type << "Id, : " << n.id << ", Code: " << n.code << ", Message: " << n.message << ", Entities: " << n.entities << ")"; return dbg.space(); } diff --git a/common/notification.h b/common/notification.h index 4b52274..f5379fd 100644 --- a/common/notification.h +++ b/common/notification.h @@ -51,6 +51,7 @@ public: }; QByteArray id; + QByteArrayList entities; int type = 0; QString message; //A return code. Zero typically indicates success. diff --git a/common/query.h b/common/query.h index 49c8d5e..5b37cdd 100644 --- a/common/query.h +++ b/common/query.h @@ -300,7 +300,9 @@ public: /** Leave the query running and continuously update the result set. */ LiveQuery = 1, /** Run the query synchronously. */ - SynchronousQuery = 2 + SynchronousQuery = 2, + /** Include status updates via notifications */ + UpdateStatus = 4 }; Q_DECLARE_FLAGS(Flags, Flag) @@ -410,6 +412,11 @@ public: mFlags = flags; } + Flags flags() const + { + return mFlags; + } + bool liveQuery() const { return mFlags.testFlag(LiveQuery); diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp index e48b624..9f4f14c 100644 --- a/common/resourceaccess.cpp +++ b/common/resourceaccess.cpp @@ -547,6 +547,7 @@ static Sink::Notification getNotification(const Sink::Commands::Notification *bu } n.type = buffer->type(); n.code = buffer->code(); + n.entities = BufferUtils::fromVector(*buffer->entities()); return n; } @@ -608,13 +609,17 @@ bool ResourceAccess::processMessageBuffer() mResourceStatus = buffer->code(); SinkTrace() << "Updated status: " << mResourceStatus; [[clang::fallthrough]]; + case Sink::Notification::Info: + [[clang::fallthrough]]; case Sink::Notification::Warning: [[clang::fallthrough]]; + case Sink::Notification::Error: + [[clang::fallthrough]]; case Sink::Notification::FlushCompletion: [[clang::fallthrough]]; case Sink::Notification::Progress: { auto n = getNotification(buffer); - SinkTrace() << "Received notification: Type:" << n.type << "Message: " << n.message << "Code: " << n.code; + SinkTrace() << "Received notification: " << n; n.resource = d->resourceInstanceIdentifier; emit notification(n); } break; diff --git a/common/store.h b/common/store.h index 86e4d20..fae76e5 100644 --- a/common/store.h +++ b/common/store.h @@ -48,11 +48,15 @@ QString SINK_EXPORT storageLocation(); */ QString SINK_EXPORT getTemporaryFilePath(); +// Must be the same as in ModelResult enum Roles { - DomainObjectRole = Qt::UserRole + 1, // Must be the same as in ModelResult + DomainObjectRole = Qt::UserRole + 1, ChildrenFetchedRole, - DomainObjectBaseRole + DomainObjectBaseRole, + StatusRole, //ApplicationDomain::SyncStatus + WarningRole, //ApplicationDomain::Warning, only if status == warning || status == error + ProgressRole //ApplicationDomain::Progress }; /** diff --git a/common/synchronizer.cpp b/common/synchronizer.cpp index 4ed6e3a..ec896ed 100644 --- a/common/synchronizer.cpp +++ b/common/synchronizer.cpp @@ -292,13 +292,14 @@ void Synchronizer::flushComplete(const QByteArray &flushId) } } -void Synchronizer::emitNotification(Notification::NoticationType type, int code, const QString &message, const QByteArray &id) +void Synchronizer::emitNotification(Notification::NoticationType type, int code, const QString &message, const QByteArray &id, const QByteArrayList &entities) { Sink::Notification n; n.id = id; n.type = type; n.message = message; n.code = code; + n.entities = entities; emit notify(n); } @@ -328,6 +329,7 @@ KAsync::Job Synchronizer::processRequest(const SyncRequest &request) 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.query.ids()); }).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(); @@ -335,10 +337,12 @@ KAsync::Job Synchronizer::processRequest(const SyncRequest &request) if (error) { //Emit notification with error SinkWarningCtx(mLogCtx) << "Synchronization failed: " << error.errorMessage; + emitNotification(Notification::Warning, ApplicationDomain::SyncError, {}, {}, request.query.ids()); emitNotification(Notification::Status, ApplicationDomain::ErrorStatus, "Synchronization has ended.", request.requestId); return KAsync::error(error); } else { SinkLogCtx(mLogCtx) << "Done Synchronizing"; + emitNotification(Notification::Info, ApplicationDomain::SyncSuccess, {}, {}, request.query.ids()); emitNotification(Notification::Status, ApplicationDomain::ConnectedStatus, "Synchronization has ended.", request.requestId); return KAsync::null(); } diff --git a/common/synchronizer.h b/common/synchronizer.h index 28fe645..e3dbddc 100644 --- a/common/synchronizer.h +++ b/common/synchronizer.h @@ -180,7 +180,7 @@ protected: */ virtual void mergeIntoQueue(const Synchronizer::SyncRequest &request, QList &queue); - void emitNotification(Notification::NoticationType type, int code, const QString &message, const QByteArray &id = QByteArray{}); + void emitNotification(Notification::NoticationType type, int code, const QString &message, const QByteArray &id = QByteArray{}, const QByteArrayList &entiteis = QByteArrayList{}); protected: Sink::Log::Context mLogCtx; diff --git a/examples/dummyresource/resourcefactory.cpp b/examples/dummyresource/resourcefactory.cpp index e89edde..ece3440 100644 --- a/examples/dummyresource/resourcefactory.cpp +++ b/examples/dummyresource/resourcefactory.cpp @@ -69,6 +69,7 @@ class DummySynchronizer : public Sink::Synchronizer { Sink::ApplicationDomain::Mail::Ptr createMail(const QByteArray &ridBuffer, const QMap &data) { auto mail = Sink::ApplicationDomain::Mail::Ptr::create(); + mail->setExtractedMessageId(ridBuffer); mail->setExtractedSubject(data.value("subject").toString()); mail->setExtractedSender(Sink::ApplicationDomain::Mail::Contact{data.value("senderName").toString(), data.value("senderEmail").toString()}); mail->setExtractedDate(data.value("date").toDateTime()); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7144d41..fef76bd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -44,6 +44,7 @@ auto_tests ( testaccounttest dummyresourcemailtest interresourcemovetest + notificationtest ) generate_flatbuffers(dummyresourcetest calendar) target_link_libraries(dummyresourcetest sink_resource_dummy) @@ -52,3 +53,4 @@ target_link_libraries(dummyresourcewritebenchmark sink_resource_dummy) target_link_libraries(querytest sink_resource_dummy) target_link_libraries(modelinteractivitytest sink_resource_dummy) target_link_libraries(inspectiontest sink_resource_dummy) +target_link_libraries(notificationtest sink_resource_dummy) diff --git a/tests/dummyresourcetest.cpp b/tests/dummyresourcetest.cpp index eea63c0..17df160 100644 --- a/tests/dummyresourcetest.cpp +++ b/tests/dummyresourcetest.cpp @@ -136,12 +136,7 @@ private slots: void testResourceSync() { ::DummyResource resource(getContext()); - auto job = resource.synchronizeWithSource(Sink::QueryBase()); - // TODO pass in optional timeout? - auto future = job.exec(); - future.waitForFinished(); - QVERIFY(!future.errorCode()); - QVERIFY(future.isFinished()); + VERIFYEXEC(resource.synchronizeWithSource(Sink::QueryBase())); QVERIFY(!resource.error()); auto processAllMessagesFuture = resource.processAllMessages().exec(); processAllMessagesFuture.waitForFinished(); @@ -152,7 +147,7 @@ private slots: const auto query = Query().resourceFilter("sink.dummy.instance1"); // Ensure all local data is processed - Sink::Store::synchronize(query).exec().waitForFinished(); + VERIFYEXEC(Sink::Store::synchronize(query)); VERIFYEXEC(Sink::ResourceControl::flushMessageQueue(QByteArrayList() << "sink.dummy.instance1")); auto model = Sink::Store::loadModel(query); diff --git a/tests/notificationtest.cpp b/tests/notificationtest.cpp new file mode 100644 index 0000000..9433586 --- /dev/null +++ b/tests/notificationtest.cpp @@ -0,0 +1,124 @@ +#include + +#include +#include + +#include "store.h" +#include "resourceconfig.h" +#include "resourcecontrol.h" +#include "modelresult.h" +#include "log.h" +#include "test.h" +#include "testutils.h" +#include "notifier.h" +#include "notification.h" + +using namespace Sink; +using namespace Sink::ApplicationDomain; + +/** + * Test of complete system using the dummy resource. + * + * This test requires the dummy resource installed. + */ +class NotificationTest : public QObject +{ + Q_OBJECT + +private slots: + void initTestCase() + { + Sink::Test::initTest(); + ResourceConfig::addResource("sink.dummy.instance1", "sink.dummy"); + } + + void cleanup() + { + VERIFYEXEC(Sink::Store::removeDataFromDisk(QByteArray("sink.dummy.instance1"))); + } + + void testSyncNotifications() + { + auto query = Query().resourceFilter("sink.dummy.instance1"); + query.setType(); + query.filter("id1"); + query.filter("id2"); + + QList statusNotifications; + QList infoNotifications; + Sink::Notifier notifier("sink.dummy.instance1"); + notifier.registerHandler([&] (const Sink::Notification &n){ + SinkLogCtx(Sink::Log::Context{"dummyresourcetest"}) << "Received notification " << n; + if (n.type == Notification::Status) { + statusNotifications << n; + } + if (n.type == Notification::Info) { + infoNotifications << n; + } + }); + + // Ensure all local data is processed + VERIFYEXEC(Sink::Store::synchronize(query)); + VERIFYEXEC(Sink::ResourceControl::flushMessageQueue(QByteArrayList() << "sink.dummy.instance1")); + + //FIXME it can happen that we get a changereplay notification pair first. + QTRY_COMPARE(statusNotifications.size(), 5); + //Sync + QCOMPARE(statusNotifications.at(0).code, static_cast(ApplicationDomain::Status::ConnectedStatus)); + QCOMPARE(statusNotifications.at(1).code, static_cast(Sink::ApplicationDomain::Status::BusyStatus)); + QCOMPARE(statusNotifications.at(2).code, static_cast(Sink::ApplicationDomain::Status::ConnectedStatus)); + //Changereplay + QCOMPARE(statusNotifications.at(3).code, static_cast(Sink::ApplicationDomain::Status::BusyStatus)); + QCOMPARE(statusNotifications.at(4).code, static_cast(Sink::ApplicationDomain::Status::ConnectedStatus)); + + QTRY_COMPARE(infoNotifications.size(), 2); + QCOMPARE(infoNotifications.at(0).code, static_cast(ApplicationDomain::SyncStatus::SyncInProgress)); + QCOMPARE(infoNotifications.at(0).entities, QList{} << "id1" << "id2"); + QCOMPARE(infoNotifications.at(1).code, static_cast(ApplicationDomain::SyncStatus::SyncSuccess)); + QCOMPARE(infoNotifications.at(1).entities, QList{} << "id1" << "id2"); + + QCOMPARE(infoNotifications.at(1).code, static_cast(ApplicationDomain::SyncStatus::SyncSuccess)); + } + + void testModelNotifications() + { + auto query = Query().resourceFilter("sink.dummy.instance1"); + query.setType(); + query.setFlags(Query::LiveQuery | Query::UpdateStatus); + + VERIFYEXEC(Sink::Store::synchronize(query)); + VERIFYEXEC(Sink::ResourceControl::flushMessageQueue(QByteArrayList() << "sink.dummy.instance1")); + + auto model = Sink::Store::loadModel(query); + QTRY_VERIFY(model->data(QModelIndex(), Sink::Store::ChildrenFetchedRole).toBool()); + QVERIFY(model->rowCount() >= 1); + + QSignalSpy changedSpy(model.data(), &QAbstractItemModel::dataChanged); + auto mail = model->index(0, 0, QModelIndex()).data(Sink::Store::DomainObjectRole).value(); + auto newQuery = query; + newQuery.filter(mail->identifier()); + + QList status; + QObject::connect(model.data(), &QAbstractItemModel::dataChanged, [&] (const QModelIndex &begin, const QModelIndex &end, const QVector &roles) { + QVERIFY(begin.row() == end.row()); + if (begin.row() == 0) { + status << model->data(begin, Store::StatusRole).value(); + // qWarning() << "New status: " << status.last() << roles; + } + }); + + //This will trigger a modification of all previous items as well. + VERIFYEXEC(Sink::Store::synchronize(newQuery)); + VERIFYEXEC(Sink::ResourceControl::flushMessageQueue(QByteArrayList() << "sink.dummy.instance1")); + + QCOMPARE(status.size(), 3); + //Sync progress of item + QCOMPARE(status.at(0), static_cast(ApplicationDomain::SyncStatus::SyncInProgress)); + QCOMPARE(status.at(1), static_cast(ApplicationDomain::SyncStatus::SyncSuccess)); + //Modification triggered during sync + QCOMPARE(status.at(2), static_cast(ApplicationDomain::SyncStatus::SyncSuccess)); + } +}; + +QTEST_MAIN(NotificationTest) +#include "notificationtest.moc" -- cgit v1.2.3