Track modified properties to detect necessary replay changes

13 files changed, 69 insertions, 34 deletions

diff --git a/common/bufferutils.h b/common/bufferutils.h
index 1eb5d15..d6008c4 100644
--- a/common/bufferutils.h
+++ b/common/bufferutils.h
@@ -2,6 +2,7 @@
 
 #include <flatbuffers/flatbuffers.h>
 #include <QByteArray>
+#include <QList>
 
 namespace Sink {
 namespace BufferUtils {
@@ -21,5 +22,27 @@ static QByteArray extractBuffer(const flatbuffers::FlatBufferBuilder &fbb)
 {
     return QByteArray::fromRawData(reinterpret_cast<char const *>(fbb.GetBufferPointer()), fbb.GetSize());
 }
+
+static QList<QByteArray> fromVector(const flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>> &vector)
+{
+    QList<QByteArray> list;
+    for (const auto &data : vector) {
+        Q_ASSERT(data);
+        list << QByteArray::fromStdString(data->str());
+    }
+    return list;
+}
+
+template <typename T>
+static flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>>> toVector(flatbuffers::FlatBufferBuilder &fbb, const T &list)
+{
+    std::vector<flatbuffers::Offset<flatbuffers::String>> modifiedPropertiesList;
+    for (const auto &change : list) {
+        auto s = fbb.CreateString(change);
+        modifiedPropertiesList.push_back(s);
+    }
+    return fbb.CreateVector(modifiedPropertiesList);
+}
+
 }
 }
diff --git a/common/commands/modifyentity.fbs b/common/commands/modifyentity.fbs
index efa2fa0..da6f0e2 100644
--- a/common/commands/modifyentity.fbs
+++ b/common/commands/modifyentity.fbs
@@ -7,6 +7,7 @@ table ModifyEntity {
     domainType: string;
     delta: [ubyte]; //Contains an entity buffer with all changed properties set
     replayToSource: bool = true;
+    modifiedProperties: [string];
 }
 
 root_type ModifyEntity;
diff --git a/common/domainadaptor.h b/common/domainadaptor.h
index c620f91..3af0fad 100644
--- a/common/domainadaptor.h
+++ b/common/domainadaptor.h
@@ -111,6 +111,9 @@ public:
         return QVariant();
     }
 
+    /**
+     * Returns all available properties for which a mapping exists (no matter what the buffer contains)
+     */
     virtual QList<QByteArray> availableProperties() const
     {
         return mResourceMapper->availableProperties() + mLocalMapper->availableProperties();
diff --git a/common/facade.cpp b/common/facade.cpp
index 1045dac..bebb682 100644
--- a/common/facade.cpp
+++ b/common/facade.cpp
@@ -76,7 +76,7 @@ KAsync::Job<void> GenericFacade<DomainType>::modify(const DomainType &domainObje
     }
     flatbuffers::FlatBufferBuilder entityFbb;
     mDomainTypeAdaptorFactory->createBuffer(domainObject, entityFbb);
-    return mResourceAccess->sendModifyCommand(domainObject.identifier(), domainObject.revision(), bufferTypeForDomainType(), QByteArrayList(), BufferUtils::extractBuffer(entityFbb));
+    return mResourceAccess->sendModifyCommand(domainObject.identifier(), domainObject.revision(), bufferTypeForDomainType(), QByteArrayList(), BufferUtils::extractBuffer(entityFbb), domainObject.changedProperties());
 }
 
 template <class DomainType>
diff --git a/common/metadata.fbs b/common/metadata.fbs
index f2f336d..421d13b 100644
--- a/common/metadata.fbs
+++ b/common/metadata.fbs
@@ -6,6 +6,7 @@ table Metadata {
     revision: ulong;
     replayToSource: bool = true;
     operation: Operation = Modification;
+    modifiedProperties: [string];
 }
 
 root_type Metadata;
diff --git a/common/pipeline.cpp b/common/pipeline.cpp
index 7863f67..a7059c1 100644
--- a/common/pipeline.cpp
+++ b/common/pipeline.cpp
@@ -228,7 +228,8 @@ KAsync::Job<qint64> Pipeline::modifiedEntity(void const *command, size_t size)
     }
     auto modifyEntity = Commands::GetModifyEntity(command);
     Q_ASSERT(modifyEntity);
-
+    Q_ASSERT(modifyEntity->modifiedProperties());
+    auto changeset = BufferUtils::fromVector(*modifyEntity->modifiedProperties());
     const qint64 baseRevision = modifyEntity->revision();
     const bool replayToSource = modifyEntity->replayToSource();
     // TODO rename modifyEntity->domainType to bufferType
@@ -281,9 +282,7 @@ KAsync::Job<qint64> Pipeline::modifiedEntity(void const *command, size_t size)
     // Apply diff
     // FIXME only apply the properties that are available in the buffer
     Trace() << "Applying changed properties: " << diff->availableProperties();
-    QSet<QByteArray> changeset;
-    for (const auto &property : diff->availableProperties()) {
-        changeset << property;
+    for (const auto &property : changeset) {
         const auto value = diff->getProperty(property);
         if (value.isValid()) {
             newAdaptor->setProperty(property, value);
@@ -303,12 +302,16 @@ KAsync::Job<qint64> Pipeline::modifiedEntity(void const *command, size_t size)
 
     // Add metadata buffer
     flatbuffers::FlatBufferBuilder metadataFbb;
-    auto metadataBuilder = MetadataBuilder(metadataFbb);
-    metadataBuilder.add_revision(newRevision);
-    metadataBuilder.add_operation(Operation_Modification);
-    metadataBuilder.add_replayToSource(replayToSource);
-    auto metadataBuffer = metadataBuilder.Finish();
-    FinishMetadataBuffer(metadataFbb, metadataBuffer);
+    {
+        auto modifiedProperties = BufferUtils::toVector(metadataFbb, changeset);
+        auto metadataBuilder = MetadataBuilder(metadataFbb);
+        metadataBuilder.add_revision(newRevision);
+        metadataBuilder.add_operation(Operation_Modification);
+        metadataBuilder.add_replayToSource(replayToSource);
+        metadataBuilder.add_modifiedProperties(modifiedProperties);
+        auto metadataBuffer = metadataBuilder.Finish();
+        FinishMetadataBuffer(metadataFbb, metadataBuffer);
+    }
 
     flatbuffers::FlatBufferBuilder fbb;
     adaptorFactory->createBuffer(newAdaptor, fbb, metadataFbb.GetBufferPointer(), metadataFbb.GetSize());
diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp
index 3696a93..5ad2018 100644
--- a/common/resourceaccess.cpp
+++ b/common/resourceaccess.cpp
@@ -316,16 +316,16 @@ KAsync::Job<void> ResourceAccess::sendCreateCommand(const QByteArray &uid, const
 }
 
 KAsync::Job<void>
-ResourceAccess::sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer)
+ResourceAccess::sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer, const QByteArrayList &changedProperties)
 {
     flatbuffers::FlatBufferBuilder fbb;
     auto entityId = fbb.CreateString(uid.constData());
     // This is the resource buffer type and not the domain type
     auto type = fbb.CreateString(resourceBufferType.constData());
-    // FIXME
-    auto deletions = 0;
+    auto modifiedProperties = BufferUtils::toVector(fbb, changedProperties);
+    auto deletions = BufferUtils::toVector(fbb, deletedProperties);
     auto delta = Sink::EntityBuffer::appendAsVector(fbb, buffer.constData(), buffer.size());
-    auto location = Sink::Commands::CreateModifyEntity(fbb, revision, entityId, deletions, type, delta);
+    auto location = Sink::Commands::CreateModifyEntity(fbb, revision, entityId, deletions, type, delta, true, modifiedProperties);
     Sink::Commands::FinishModifyEntityBuffer(fbb, location);
     open();
     return sendCommand(Sink::Commands::ModifyEntityCommand, fbb);
diff --git a/common/resourceaccess.h b/common/resourceaccess.h
index 82aa4d3..69d52b4 100644
--- a/common/resourceaccess.h
+++ b/common/resourceaccess.h
@@ -54,7 +54,7 @@ public:
     {
         return KAsync::null<void>();
     };
-    virtual KAsync::Job<void> sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer)
+    virtual KAsync::Job<void> sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer, const QByteArrayList &changedProperties)
     {
         return KAsync::null<void>();
     };
@@ -99,7 +99,7 @@ public:
     KAsync::Job<void> synchronizeResource(bool remoteSync, bool localSync) Q_DECL_OVERRIDE;
     KAsync::Job<void> sendCreateCommand(const QByteArray &uid, const QByteArray &resourceBufferType, const QByteArray &buffer) Q_DECL_OVERRIDE;
     KAsync::Job<void>
-    sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer) Q_DECL_OVERRIDE;
+    sendModifyCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType, const QByteArrayList &deletedProperties, const QByteArray &buffer, const QByteArrayList &changedProperties) Q_DECL_OVERRIDE;
     KAsync::Job<void> sendDeleteCommand(const QByteArray &uid, qint64 revision, const QByteArray &resourceBufferType) Q_DECL_OVERRIDE;
     KAsync::Job<void> sendRevisionReplayedCommand(qint64 revision) Q_DECL_OVERRIDE;
     KAsync::Job<void>
diff --git a/common/sourcewriteback.cpp b/common/sourcewriteback.cpp
index 87f6934..a277606 100644
--- a/common/sourcewriteback.cpp
+++ b/common/sourcewriteback.cpp
@@ -21,6 +21,7 @@
 
 #include "definitions.h"
 #include "log.h"
+#include "bufferutils.h"
 
 #define ENTITY_TYPE_MAIL "mail"
 #define ENTITY_TYPE_FOLDER "folder"
@@ -74,20 +75,25 @@ KAsync::Job<void> SourceWriteBack::replay(const QByteArray &type, const QByteArr
     // const qint64 revision = metadataBuffer ? metadataBuffer->revision() : -1;
     const auto operation = metadataBuffer ? metadataBuffer->operation() : Sink::Operation_Creation;
     const auto uid = Sink::Storage::uidFromKey(key);
+    const auto modifiedProperties = metadataBuffer->modifiedProperties() ? BufferUtils::fromVector(*metadataBuffer->modifiedProperties()) : QByteArrayList();
     QByteArray oldRemoteId;
 
     if (operation != Sink::Operation_Creation) {
         oldRemoteId = syncStore().resolveLocalId(type, uid);
+        if (oldRemoteId.isEmpty()) {
+            Warning() << "Couldn't find the remote id for: " << type << uid;
+            return KAsync::error<void>(1, "Couldn't find the remote id.");
+        }
     }
-    Trace() << "Replaying " << key << type << oldRemoteId;
+    Trace() << "Replaying " << key << type << uid << oldRemoteId;
 
     KAsync::Job<QByteArray> job = KAsync::null<QByteArray>();
     if (type == ENTITY_TYPE_FOLDER) {
         auto folder = store().readFromKey<ApplicationDomain::Folder>(key);
-        job = replay(folder, operation, oldRemoteId);
+        job = replay(folder, operation, oldRemoteId, modifiedProperties);
     } else if (type == ENTITY_TYPE_MAIL) {
         auto mail = store().readFromKey<ApplicationDomain::Mail>(key);
-        job = replay(mail, operation, oldRemoteId);
+        job = replay(mail, operation, oldRemoteId, modifiedProperties);
     }
 
     return job.then<void, QByteArray>([this, operation, type, uid, oldRemoteId](const QByteArray &remoteId) {
@@ -125,12 +131,12 @@ KAsync::Job<void> SourceWriteBack::replay(const QByteArray &type, const QByteArr
     });
 }
 
-KAsync::Job<QByteArray> SourceWriteBack::replay(const ApplicationDomain::Mail &, Sink::Operation, const QByteArray &)
+KAsync::Job<QByteArray> SourceWriteBack::replay(const ApplicationDomain::Mail &, Sink::Operation, const QByteArray &, const QList<QByteArray> &)
 {
     return KAsync::null<QByteArray>();
 }
 
-KAsync::Job<QByteArray> SourceWriteBack::replay(const ApplicationDomain::Folder &, Sink::Operation, const QByteArray &)
+KAsync::Job<QByteArray> SourceWriteBack::replay(const ApplicationDomain::Folder &, Sink::Operation, const QByteArray &, const QList<QByteArray> &)
 {
     return KAsync::null<QByteArray>();
 }
diff --git a/common/sourcewriteback.h b/common/sourcewriteback.h
index 9fe5c66..8531ff5 100644
--- a/common/sourcewriteback.h
+++ b/common/sourcewriteback.h
@@ -42,8 +42,8 @@ protected:
 
 protected:
     ///Implement to write back changes to the server
-    virtual KAsync::Job<QByteArray> replay(const Sink::ApplicationDomain::Mail &, Sink::Operation, const QByteArray &oldRemoteId);
-    virtual KAsync::Job<QByteArray> replay(const Sink::ApplicationDomain::Folder &, Sink::Operation, const QByteArray &oldRemoteId);
+    virtual KAsync::Job<QByteArray> replay(const Sink::ApplicationDomain::Mail &, Sink::Operation, const QByteArray &oldRemoteId, const QList<QByteArray> &);
+    virtual KAsync::Job<QByteArray> replay(const Sink::ApplicationDomain::Folder &, Sink::Operation, const QByteArray &oldRemoteId, const QList<QByteArray> &);
 
     //Read/Write access to sync storage
     RemoteIdMap &syncStore();
diff --git a/examples/imapresource/imapresource.cpp b/examples/imapresource/imapresource.cpp
index 02713b3..b8129cd 100644
--- a/examples/imapresource/imapresource.cpp
+++ b/examples/imapresource/imapresource.cpp
@@ -328,7 +328,7 @@ public:
 
     }
 
-    KAsync::Job<QByteArray> replay(const ApplicationDomain::Mail &mail, Sink::Operation operation, const QByteArray &oldRemoteId)
+    KAsync::Job<QByteArray> replay(const ApplicationDomain::Mail &mail, Sink::Operation operation, const QByteArray &oldRemoteId, const QList<QByteArray> &changedProperties) Q_DECL_OVERRIDE
     {
         auto imap = QSharedPointer<ImapServerProxy>::create(mServer, mPort);
         auto login = imap->login(mUser, mPassword);
@@ -372,10 +372,8 @@ public:
             auto uid = ridParts.takeLast().toLongLong();
             //FIXME don't hardcode the separator
             auto mailbox = ridParts.join('.');
-            Trace() << "Modifying a mail: " << oldRemoteId << " in the mailbox: " << mailbox;
-            //TODO if the message changed, remove old message and create a new one,
-            //otherwise only change flags
- 
+            Trace() << "Modifying a mail: " << oldRemoteId << " in the mailbox: " << mailbox << changedProperties;
+
             QByteArrayList flags;
             if (!mail.getUnread()) {
                 flags << Imap::Flags::Seen;
@@ -384,7 +382,7 @@ public:
                 flags << Imap::Flags::Flagged;
             }
 
-            bool messageChanged = false;
+            const bool messageChanged = changedProperties.contains(Sink::ApplicationDomain::Mail::MimeMessage::name);
             if (messageChanged) {
                 QByteArray content = KMime::LFtoCRLF(mail.getMimeMessage());
                 QDateTime internalDate = mail.getDate();
@@ -431,7 +429,7 @@ public:
         return separator + folder.getName();
     }
 
-    KAsync::Job<QByteArray> replay(const ApplicationDomain::Folder &folder, Sink::Operation operation, const QByteArray &oldRemoteId)
+    KAsync::Job<QByteArray> replay(const ApplicationDomain::Folder &folder, Sink::Operation operation, const QByteArray &oldRemoteId, const QList<QByteArray> &changedProperties) Q_DECL_OVERRIDE
     {
         auto imap = QSharedPointer<ImapServerProxy>::create(mServer, mPort);
         auto login = imap->login(mUser, mPassword);
diff --git a/examples/maildirresource/maildirresource.cpp b/examples/maildirresource/maildirresource.cpp
index d67eb50..18f8334 100644
--- a/examples/maildirresource/maildirresource.cpp
+++ b/examples/maildirresource/maildirresource.cpp
@@ -405,7 +405,7 @@ public:
 
     }
 
-    KAsync::Job<QByteArray> replay(const ApplicationDomain::Mail &mail, Sink::Operation operation, const QByteArray &oldRemoteId)
+    KAsync::Job<QByteArray> replay(const ApplicationDomain::Mail &mail, Sink::Operation operation, const QByteArray &oldRemoteId, const QList<QByteArray> &changedProperties) Q_DECL_OVERRIDE
     {
         if (operation == Sink::Operation_Creation) {
             const auto remoteId = getFilePathFromMimeMessagePath(mail.getMimeMessagePath());
@@ -426,7 +426,7 @@ public:
         return KAsync::null<QByteArray>();
     }
 
-    KAsync::Job<QByteArray> replay(const ApplicationDomain::Folder &folder, Sink::Operation operation, const QByteArray &oldRemoteId)
+    KAsync::Job<QByteArray> replay(const ApplicationDomain::Folder &folder, Sink::Operation operation, const QByteArray &oldRemoteId, const QList<QByteArray> &changedProperties) Q_DECL_OVERRIDE
     {
         if (operation == Sink::Operation_Creation) {
             auto folderName = folder.getName();
diff --git a/tests/mailtest.cpp b/tests/mailtest.cpp
index 9411f91..9046180 100644
--- a/tests/mailtest.cpp
+++ b/tests/mailtest.cpp
@@ -221,7 +221,7 @@ void MailTest::testCreateModifyDeleteMail()
 void MailTest::testMarkMailAsRead()
 {
     auto folder = Folder::create(mResourceInstanceIdentifier);
-    folder.setName("folder");
+    folder.setName("anotherfolder");
     VERIFYEXEC(Store::create(folder));
 
     auto message = KMime::Message::Ptr::create();