From 93c9ebb5d0654626cb8bf2e6b00a5845b210f82c Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Thu, 17 May 2018 15:16:29 +0200
Subject: Fixed a readEntity call with empty uid

Filtered entites are still passed through as removal, but if
there is no other value for the reduction, the reduction result is
empty.
---
 common/datastorequery.cpp |  7 ++++++
 tests/querytest.cpp       | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/common/datastorequery.cpp b/common/datastorequery.cpp
index b77dfc9..276a10a 100644
--- a/common/datastorequery.cpp
+++ b/common/datastorequery.cpp
@@ -327,6 +327,10 @@ public:
                     mReducedValues.insert(reductionValueBa);
                     auto reductionResult = reduceOnValue(reductionValue);
 
+                    //This can happen if we get a removal message from a filtered entity and all entites of the reduction are filtered.
+                    if (reductionResult.selection.isEmpty()) {
+                        return;
+                    }
                     mSelectedValues.insert(reductionValueBa, reductionResult.selection);
                     readEntity(reductionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation operation) {
                         callback({entity, operation, reductionResult.aggregateValues, reductionResult.aggregateIds});
@@ -345,11 +349,13 @@ public:
                         auto oldSelectionResult = mSelectedValues.take(reductionValueBa);
                         if (oldSelectionResult == selectionResult.selection) {
                             mSelectedValues.insert(reductionValueBa, selectionResult.selection);
+                            Q_ASSERT(!selectionResult.selection.isEmpty());
                             readEntity(selectionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
                                 callback({entity, Sink::Operation_Modification, selectionResult.aggregateValues, selectionResult.aggregateIds});
                             });
                         } else {
                             //remove old result
+                            Q_ASSERT(!oldSelectionResult.isEmpty());
                             readEntity(oldSelectionResult, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
                                 callback({entity, Sink::Operation_Removal});
                             });
@@ -358,6 +364,7 @@ public:
                             if (!selectionResult.selection.isEmpty()) {
                                 //add new result
                                 mSelectedValues.insert(reductionValueBa, selectionResult.selection);
+                                Q_ASSERT(!selectionResult.selection.isEmpty());
                                 readEntity(selectionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
                                     callback({entity, Sink::Operation_Creation, selectionResult.aggregateValues, selectionResult.aggregateIds});
                                 });
diff --git a/tests/querytest.cpp b/tests/querytest.cpp
index 26564b5..fa016a2 100644
--- a/tests/querytest.cpp
+++ b/tests/querytest.cpp
@@ -954,6 +954,60 @@ private slots:
         QCOMPARE(resetSpy.size(), 0);
     }
 
+    void testFilteredReductionUpdate()
+    {
+        // Setup
+        auto folder1 = Folder::createEntity<Folder>("sink.dummy.instance1");
+        VERIFYEXEC(Sink::Store::create<Folder>(folder1));
+
+        auto folder2 = Folder::createEntity<Folder>("sink.dummy.instance1");
+        VERIFYEXEC(Sink::Store::create<Folder>(folder2));
+
+        // Ensure all local data is processed
+        VERIFYEXEC(Sink::ResourceControl::flushMessageQueue("sink.dummy.instance1"));
+
+        Query query;
+        query.setId("testFilteredReductionUpdate");
+        query.setFlags(Query::LiveQuery);
+        query.filter<Mail::Folder>(folder1);
+        query.reduce<Mail::Folder>(Query::Reduce::Selector::max<Mail::Date>()).count("count").collect<Mail::Folder>("folders");
+        query.sort<Mail::Date>();
+
+        auto model = Sink::Store::loadModel<Mail>(query);
+        QTRY_VERIFY(model->data(QModelIndex(), Sink::Store::ChildrenFetchedRole).toBool());
+        QCOMPARE(model->rowCount(), 0);
+
+        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);
+
+        //Ensure we don't end up with a mail in the thread that was filtered
+        //This tests the case of an otherwise emtpy thread on purpose.
+        {
+            auto mail = Mail::createEntity<Mail>("sink.dummy.instance1");
+            mail.setExtractedMessageId("filtered");
+            mail.setFolder(folder2);
+            mail.setExtractedDate(QDateTime{QDate{2017, 2, 3}, QTime{11, 0, 0}});
+            VERIFYEXEC(Sink::Store::create(mail));
+        }
+
+        VERIFYEXEC(Sink::ResourceControl::flushMessageQueue("sink.dummy.instance1"));
+        QCOMPARE(model->rowCount(), 0);
+
+        //Ensure the non-filtered still get through.
+        {
+            auto mail = Mail::createEntity<Mail>("sink.dummy.instance1");
+            mail.setExtractedMessageId("not-filtered");
+            mail.setFolder(folder1);
+            mail.setExtractedDate(QDateTime{QDate{2017, 2, 3}, QTime{11, 0, 0}});
+            VERIFYEXEC(Sink::Store::create(mail));
+        }
+        VERIFYEXEC(Sink::ResourceControl::flushMessageQueue("sink.dummy.instance1"));
+        QTRY_COMPARE(model->rowCount(), 1);
+    }
+
     void testBloom()
     {
         // Setup
-- 
cgit v1.2.3