From 7287dae6c279583e5c5f0fafd8207341cc5a15af Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Thu, 22 Feb 2018 17:22:26 +0100
Subject: Properly deal with filtered entities in reduced queries.

Filtered entities would still end up in the entities list before.
---
 common/datastorequery.cpp | 22 +++++++++++++++-------
 tests/querytest.cpp       |  6 +++---
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/common/datastorequery.cpp b/common/datastorequery.cpp
index 9e61a3d..b77dfc9 100644
--- a/common/datastorequery.cpp
+++ b/common/datastorequery.cpp
@@ -267,14 +267,14 @@ public:
         for (auto &aggregator : mAggregators) {
             aggregator.reset();
         }
-
+        QVector<QByteArray> reducedAndFilteredResults;
         for (const auto &r : results) {
             readEntity(r, [&, this](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation operation) {
                 //We need to apply all property filters that we have until the reduction, because the index lookup was unfiltered.
                 if (!matchesFilter(entity)) {
                     return;
                 }
-
+                reducedAndFilteredResults << r;
                 Q_ASSERT(operation != Sink::Operation_Removal);
                 for (auto &aggregator : mAggregators) {
                     if (!aggregator.property.isEmpty()) {
@@ -294,14 +294,18 @@ public:
         for (auto &aggregator : mAggregators) {
             aggregateValues.insert(aggregator.resultProperty, aggregator.result());
         }
-        return {selectionResult, results, aggregateValues};
+        return {selectionResult, reducedAndFilteredResults, aggregateValues};
     }
 
     bool next(const std::function<void(const ResultSet::Result &)> &callback) Q_DECL_OVERRIDE {
         bool foundValue = false;
         while(!foundValue && mSource->next([this, callback, &foundValue](const ResultSet::Result &result) {
                 const auto reductionValue = [&] {
-                    if (result.operation == Sink::Operation_Removal) {
+                    const auto v = result.entity.getProperty(mReductionProperty);
+                    //Because we also get Operation_Removal for filtered entities. We use the fact that actually removed entites
+                    //won't have the property to reduce on.
+                    //TODO: Perhaps find a cleaner solutoin than abusing Operation::Removed for filtered properties.
+                    if (v.isNull() && result.operation == Sink::Operation_Removal) {
                         //For removals we have to read the last revision to get a value, and thus be able to find the correct thread.
                         QVariant reductionValue;
                         readPrevious(result.entity.identifier(), [&] (const ApplicationDomain::ApplicationDomainType &prev) {
@@ -309,10 +313,15 @@ public:
                         });
                         return reductionValue;
                     } else {
-                        return result.entity.getProperty(mReductionProperty);
+                        return v;
                     }
                 }();
-                const auto &reductionValueBa = getByteArray(reductionValue);
+                if (reductionValue.isNull()) {
+                    //We failed to find a value to reduce on, so ignore this entity.
+                    //Can happen if the entity was already removed and we have no previous revision.
+                    return;
+                }
+                const auto reductionValueBa = getByteArray(reductionValue);
                 if (!mReducedValues.contains(reductionValueBa)) {
                     //Only reduce every value once.
                     mReducedValues.insert(reductionValueBa);
@@ -356,7 +365,6 @@ public:
                         }
                     }
                 }
-                return false;
             }))
         {}
         return foundValue;
diff --git a/tests/querytest.cpp b/tests/querytest.cpp
index f65d477..52f0024 100644
--- a/tests/querytest.cpp
+++ b/tests/querytest.cpp
@@ -1211,7 +1211,7 @@ private slots:
         Query query;
         query.setId("testLivequeryThreadleaderChange");
         query.setFlags(Query::LiveQuery);
-        query.reduce<Mail::Folder>(Query::Reduce::Selector::max<Mail::Date>()).count("count").collect<Mail::Folder>("folders");
+        query.reduce<Mail::Folder>(Query::Reduce::Selector::max<Mail::Date>()).count().collect<Mail::Folder>();
         query.sort<Mail::Date>();
         query.request<Mail::MessageId>();
         query.filter<Mail::Important>(false);
@@ -1224,8 +1224,8 @@ private slots:
         {
             auto mail = model->data(model->index(0, 0, QModelIndex{}), Sink::Store::DomainObjectRole).value<Mail::Ptr>();
             QCOMPARE(mail->getMessageId(), QByteArray{"mail1"});
-            QCOMPARE(mail->getProperty("count").toInt(), 2);
-            QCOMPARE(mail->getProperty("folders").toList().size(), 2);
+            QCOMPARE(mail->count(), 2);
+            QCOMPARE(mail->getCollectedProperty<Mail::Folder>().size(), 2);
         }
     }
 
-- 
cgit v1.2.3