From 720079d6a839b4d03eb0ceaa06d0ad2b446f7de1 Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Sat, 11 Feb 2017 12:04:38 +0100
Subject: Don't emit superfluous remove signals.

We often let removal updates through and expect the model to deal with
superfluous updates, this now actually implements that.
---
 common/modelresult.cpp | 14 ++++++++------
 tests/querytest.cpp    | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/common/modelresult.cpp b/common/modelresult.cpp
index 0825518..f935419 100644
--- a/common/modelresult.cpp
+++ b/common/modelresult.cpp
@@ -228,12 +228,14 @@ void ModelResult<T, Ptr>::remove(const Ptr &value)
     auto parent = createIndexFromId(id);
     SinkTraceCtx(mLogCtx) << "Removed entity" << childId;
     auto index = mTree[id].indexOf(childId);
-    beginRemoveRows(parent, index, index);
-    mEntities.remove(childId);
-    mTree[id].removeAll(childId);
-    mParents.remove(childId);
-    // TODO remove children
-    endRemoveRows();
+    if (index >= 0) {
+        beginRemoveRows(parent, index, index);
+        mEntities.remove(childId);
+        mTree[id].removeAll(childId);
+        mParents.remove(childId);
+        // TODO remove children
+        endRemoveRows();
+    }
 }
 
 template <class T, class Ptr>
diff --git a/tests/querytest.cpp b/tests/querytest.cpp
index 328448f..b358621 100644
--- a/tests/querytest.cpp
+++ b/tests/querytest.cpp
@@ -1,6 +1,7 @@
 #include <QtTest>
 
 #include <QString>
+#include <QSignalSpy>
 
 #include "resource.h"
 #include "store.h"
@@ -786,6 +787,12 @@ private slots:
         auto model = Sink::Store::loadModel<Mail>(query);
         QTRY_COMPARE(model->rowCount(), 1);
 
+        QSignalSpy insertedSpy(model.data(), &QAbstractItemModel::rowsInserted);
+        QSignalSpy removedSpy(model.data(), &QAbstractItemModel::rowsRemoved);
+        QSignalSpy changedSpy(model.data(), &QAbstractItemModel::dataChanged);
+        QSignalSpy layoutChangedSpy(model.data(), &QAbstractItemModel::layoutChanged);
+        QSignalSpy resetSpy(model.data(), &QAbstractItemModel::modelReset);
+
         //The leader should change to mail2 after the modification
         {
             auto mail2 = Mail::createEntity<Mail>("sink.dummy.instance1");
@@ -802,6 +809,13 @@ private slots:
         QTRY_COMPARE(mail->getMessageId(), QByteArray{"mail2"});
         QCOMPARE(mail->getProperty("count").toInt(), 2);
         QCOMPARE(mail->getProperty("folders").toList().size(), 2);
+
+        //This should eventually be just one modification instead of remove + add (See datastorequery reduce component)
+        QCOMPARE(insertedSpy.size(), 1);
+        QCOMPARE(removedSpy.size(), 1);
+        QCOMPARE(changedSpy.size(), 0);
+        QCOMPARE(layoutChangedSpy.size(), 0);
+        QCOMPARE(resetSpy.size(), 0);
     }
 
     void testBloom()
@@ -875,12 +889,28 @@ private slots:
         auto model = Sink::Store::loadModel<Mail>(query);
         QTRY_COMPARE(model->rowCount(), 1);
 
+        QSignalSpy insertedSpy(model.data(), &QAbstractItemModel::rowsInserted);
+        QSignalSpy removedSpy(model.data(), &QAbstractItemModel::rowsRemoved);
+        QSignalSpy changedSpy(model.data(), &QAbstractItemModel::dataChanged);
+        QSignalSpy layoutChangedSpy(model.data(), &QAbstractItemModel::layoutChanged);
+        QSignalSpy resetSpy(model.data(), &QAbstractItemModel::modelReset);
+
+        //This modification should make it through
+        {
+            //This should not trigger an entity already in model warning
+            mail1.setUnread(false);
+            VERIFYEXEC(Sink::Store::modify(mail1));
+        }
+
+        //This mail should make it through
         {
             auto mail = Mail::createEntity<Mail>("sink.dummy.instance1");
             mail.setExtractedMessageId("mail2");
             mail.setFolder(folder1);
             VERIFYEXEC(Sink::Store::create(mail));
         }
+
+        //This mail shouldn't make it through
         {
             auto mail = Mail::createEntity<Mail>("sink.dummy.instance1");
             mail.setExtractedMessageId("mail3");
@@ -892,6 +922,14 @@ private slots:
         QTRY_COMPARE(model->rowCount(), 2);
         QTest::qWait(100);
         QCOMPARE(model->rowCount(), 2);
+
+        //From mail2
+        QCOMPARE(insertedSpy.size(), 1);
+        QCOMPARE(removedSpy.size(), 0);
+        //From the modification
+        QCOMPARE(changedSpy.size(), 1);
+        QCOMPARE(layoutChangedSpy.size(), 0);
+        QCOMPARE(resetSpy.size(), 0);
     }
 };
 
-- 
cgit v1.2.3