Fetch more data on demand

We skip values we've already seen and only retrieve the new ones.
This currently only properly works in a non-live query and we don't
give the model any feedback when we can't fetch more data anymore.

However, it generally works and we get the desired effect.

8 files changed, 171 insertions, 97 deletions

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index fe72605..0d61d90 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -58,6 +58,7 @@ set(command_SRCS
     typeindex.cpp
     resourcefacade.cpp
     resourceconfig.cpp
+    resultset.cpp
     domain/applicationdomaintype.cpp
     domain/event.cpp
     domain/mail.cpp
diff --git a/common/modelresult.cpp b/common/modelresult.cpp
index 5b5a473..ceefa76 100644
--- a/common/modelresult.cpp
+++ b/common/modelresult.cpp
@@ -155,7 +155,8 @@ bool ModelResult<T, Ptr>::hasChildren(const QModelIndex &parent) const
 template<class T, class Ptr>
 bool ModelResult<T, Ptr>::canFetchMore(const QModelIndex &parent) const
 {
-    return !mEntityChildrenFetched.contains(parent.internalId());
+    const auto id = parent.internalId();
+    return !mEntityChildrenFetched.contains(id) || mEntityChildrenFetchComplete.contains(id);
 }
 
 template<class T, class Ptr>
@@ -218,6 +219,7 @@ template<class T, class Ptr>
 void ModelResult<T, Ptr>::fetchEntities(const QModelIndex &parent)
 {
     const auto id = getIdentifier(parent);
+    mEntityChildrenFetchComplete.remove(id);
     mEntityChildrenFetched.insert(id);
     Trace() << "Loading child entities of parent " << id;
     if (loadEntities) {
diff --git a/common/queryrunner.cpp b/common/queryrunner.cpp
index 2f627bf..5ac1344 100644
--- a/common/queryrunner.cpp
+++ b/common/queryrunner.cpp
@@ -79,9 +79,12 @@ QueryRunner<DomainType>::QueryRunner(const Sink::Query &query, const Sink::Resou
     mBatchSize(query.limit)
 {
     Trace() << "Starting query";
+    if (query.limit && query.sortProperty.isEmpty()) {
+        Warning() << "A limited query without sorting is typically a bad idea.";
+    }
     //We delegate loading of initial data to the result provider, os it can decide for itself what it needs to load.
     mResultProvider->setFetcher([=](const typename DomainType::Ptr &parent) {
-        Trace() << "Running fetcher";
+        Trace() << "Running fetcher. Offset: " << mOffset << " Batchsize: " << mBatchSize;
         auto resultProvider = mResultProvider;
         async::run<qint64>([=]() -> qint64 {
                 QueryWorker<DomainType> worker(query, instanceIdentifier, factory, bufferType, mResultTransformation);
@@ -89,6 +92,7 @@ QueryRunner<DomainType>::QueryRunner(const Sink::Query &query, const Sink::Resou
                 return newRevision;
             })
             .template then<void, qint64>([query, this](qint64 newRevision) {
+                mOffset += mBatchSize;
                 //Only send the revision replayed information if we're connected to the resource, there's no need to start the resource otherwise.
                 if (query.liveQuery) {
                     mResourceAccess->sendRevisionReplayedCommand(newRevision);
@@ -325,7 +329,9 @@ ResultSet QueryWorker<DomainType>::filterAndSortSet(ResultSet &resultSet, const
         };
 
         auto skip = [iterator]() {
-            iterator->next();
+            if (iterator->hasNext()) {
+                iterator->next();
+            }
         };
         return ResultSet(generator, skip);
     } else {
@@ -339,8 +345,7 @@ ResultSet QueryWorker<DomainType>::filterAndSortSet(ResultSet &resultSet, const
                         if ((operation != Sink::Operation_Removal) && filter(domainObject)) {
                             //In the initial set every entity is new
                             callback(domainObject, Sink::Operation_Creation);
-                        }
-                    } else {
+                        } } else {
                         //Always remove removals, they probably don't match due to non-available properties
                         if ((operation == Sink::Operation_Removal) || filter(domainObject)) {
                             //TODO only replay if this is in the currently visible set (or just always replay, worst case we have a couple to many results)
diff --git a/common/resultset.cpp b/common/resultset.cpp
new file mode 100644
index 0000000..6e1479a
--- /dev/null
+++ b/common/resultset.cpp
@@ -0,0 +1,134 @@
+/*
+ *   Copyright (C) 2016 Christian Mollekopf <chrigi_1@fastmail.fm>
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the
+ *   Free Software Foundation, Inc.,
+ *   51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ */
+#include "resultset.h"
+
+#include "common/log.h"
+
+ResultSet::ResultSet()
+    : mIt(nullptr)
+{
+
+}
+
+ResultSet::ResultSet(const ValueGenerator &generator, const SkipValue &skip)
+    : mIt(nullptr),
+    mValueGenerator(generator),
+    mSkip(skip)
+{
+
+}
+
+ResultSet::ResultSet(const IdGenerator &generator)
+    : mIt(nullptr),
+    mGenerator(generator),
+    mSkip([this]() {
+        next();
+    })
+{
+
+}
+
+ResultSet::ResultSet(const QVector<QByteArray> &resultSet)
+    : mResultSet(resultSet),
+    mIt(mResultSet.constBegin()),
+    mSkip([this]() {
+        if (mIt != mResultSet.constEnd()) {
+            mIt++;
+        }
+    }),
+    mFirst(true)
+{
+
+}
+
+ResultSet::ResultSet(const ResultSet &other)
+    : mResultSet(other.mResultSet),
+    mIt(nullptr),
+    mFirst(true)
+{
+    if (other.mValueGenerator) {
+        mValueGenerator = other.mValueGenerator;
+        mSkip = other.mSkip;
+    } else if (other.mGenerator) {
+        mGenerator = other.mGenerator;
+        mSkip = [this]() {
+            next();
+        };
+    } else {
+        mResultSet = other.mResultSet;
+        mIt = mResultSet.constBegin();
+        mSkip = [this]() {
+            if (mIt != mResultSet.constEnd()) {
+                mIt++;
+            }
+        };
+    }
+}
+
+bool ResultSet::next()
+{
+    if (mIt) {
+        if (mIt != mResultSet.constEnd() && !mFirst) {
+            mIt++;
+        }
+        mFirst = false;
+        return mIt != mResultSet.constEnd();
+    } else if (mGenerator) {
+        Q_ASSERT(mGenerator);
+        mCurrentValue = mGenerator();
+        if (!mCurrentValue.isNull()) {
+            return true;
+        }
+    } else {
+        next([](const Sink::ApplicationDomain::ApplicationDomainType::Ptr &value, Sink::Operation){ return false; });
+    }
+    return false;
+}
+
+bool ResultSet::next(std::function<bool(const Sink::ApplicationDomain::ApplicationDomainType::Ptr &value, Sink::Operation)> callback)
+{
+    Q_ASSERT(mValueGenerator);
+    return mValueGenerator(callback);
+}
+
+void ResultSet::skip(int number)
+{
+    Q_ASSERT(mSkip);
+    for (int i = 0; i < number; i++) {
+        mSkip();
+    }
+}
+
+QByteArray ResultSet::id()
+{
+    if (mIt) {
+        if (mIt == mResultSet.constEnd()) {
+            return QByteArray();
+        }
+        Q_ASSERT(mIt != mResultSet.constEnd());
+        return *mIt;
+    } else {
+        return mCurrentValue;
+    }
+}
+
+bool ResultSet::isEmpty()
+{
+    return mResultSet.isEmpty();
+}
diff --git a/common/resultset.h b/common/resultset.h
index 8a0720d..e513460 100644
--- a/common/resultset.h
+++ b/common/resultset.h
@@ -34,100 +34,20 @@ class ResultSet {
         typedef std::function<QByteArray()> IdGenerator;
         typedef std::function<void()> SkipValue;
 
-        ResultSet()
-            : mIt(nullptr)
-        {
+        ResultSet();
+        ResultSet(const ValueGenerator &generator, const SkipValue &skip);
+        ResultSet(const IdGenerator &generator);
+        ResultSet(const QVector<QByteArray> &resultSet);
+        ResultSet(const ResultSet &other);
 
-        }
+        bool next();
+        bool next(std::function<bool(const Sink::ApplicationDomain::ApplicationDomainType::Ptr &value, Sink::Operation)> callback);
 
-        ResultSet(const ValueGenerator &generator, const SkipValue &skip)
-            : mIt(nullptr),
-            mValueGenerator(generator),
-            mSkip(skip)
-        {
+        void skip(int number);
 
-        }
+        QByteArray id();
 
-        ResultSet(const IdGenerator &generator)
-            : mIt(nullptr),
-            mGenerator(generator),
-            mSkip([this]() {
-                mGenerator();
-            })
-        {
-
-        }
-
-        ResultSet(const QVector<QByteArray> &resultSet)
-            : mResultSet(resultSet),
-            mIt(nullptr),
-            mSkip([this]() {
-                mGenerator();
-            })
-        {
-
-        }
-
-        bool next()
-        {
-            if (mGenerator) {
-                mCurrentValue = mGenerator();
-            } else {
-                if (!mIt) {
-                    mIt = mResultSet.constBegin();
-                } else {
-                    mIt++;
-                }
-                return mIt != mResultSet.constEnd();
-            }
-            if (!mCurrentValue.isNull()) {
-                return true;
-            }
-            return false;
-        }
-
-        bool next(std::function<bool(const Sink::ApplicationDomain::ApplicationDomainType::Ptr &value, Sink::Operation)> callback)
-        {
-            Q_ASSERT(mValueGenerator);
-            return mValueGenerator(callback);
-        }
-
-        bool next(std::function<void(const QByteArray &key)> callback)
-        {
-            if (mGenerator) {
-                mCurrentValue = mGenerator();
-            } else {
-                if (!mIt) {
-                    mIt = mResultSet.constBegin();
-                } else {
-                    mIt++;
-                }
-                return mIt != mResultSet.constEnd();
-            }
-            return false;
-        }
-
-        void skip(int number)
-        {
-            Q_ASSERT(mSkip);
-            for (int i = 0; i < number; i++) {
-                mSkip();
-            }
-        }
-
-        QByteArray id()
-        {
-            if (mIt) {
-                return *mIt;
-            } else {
-                return mCurrentValue;
-            }
-        }
-
-        bool isEmpty()
-        {
-            return mResultSet.isEmpty();
-        }
+        bool isEmpty();
 
     private:
         QVector<QByteArray> mResultSet;
@@ -136,5 +56,6 @@ class ResultSet {
         IdGenerator mGenerator;
         ValueGenerator mValueGenerator;
         SkipValue mSkip;
+        bool mFirst;
 };
 
diff --git a/common/store.cpp b/common/store.cpp
index 2f88c6d..6847d22 100644
--- a/common/store.cpp
+++ b/common/store.cpp
@@ -75,12 +75,13 @@ static QList<QByteArray> getResources(const QList<QByteArray> &resourceFilter, c
 template <class DomainType>
 QSharedPointer<QAbstractItemModel> Store::loadModel(Query query)
 {
-    Trace() << "Query: ";
+    Trace() << "Query: " << ApplicationDomain::getTypeName<DomainType>();
     Trace() << "  Requested: " << query.requestedProperties;
     Trace() << "  Filter: " << query.propertyFilter;
     Trace() << "  Parent: " << query.parentProperty;
     Trace() << "  Ids: " << query.ids;
     Trace() << "  IsLive: " << query.liveQuery;
+    Trace() << "  Sorting: " << query.sortProperty;
     auto model = QSharedPointer<ModelResult<DomainType, typename DomainType::Ptr> >::create(query, query.requestedProperties);
 
     //* Client defines lifetime of model
diff --git a/common/typeindex.cpp b/common/typeindex.cpp
index 3ee2492..ddf5df5 100644
--- a/common/typeindex.cpp
+++ b/common/typeindex.cpp
@@ -36,6 +36,10 @@ static QByteArray getByteArray(const QVariant &value)
 
 static QByteArray toSortableByteArray(const QDateTime &date)
 {
+    //Sort invalid last
+    if (!date.isValid()) {
+        return QByteArray::number(std::numeric_limits<unsigned int>::max());
+    }
     return QByteArray::number(std::numeric_limits<unsigned int>::max() - date.toTime_t());
 }
 
diff --git a/tests/querytest.cpp b/tests/querytest.cpp
index 3bb0427..2531c25 100644
--- a/tests/querytest.cpp
+++ b/tests/querytest.cpp
@@ -305,16 +305,22 @@ private slots:
         query.propertyFilter.insert("folder", folderEntity->identifier());
         query.sortProperty = "date";
         query.limit = 1;
+        query.liveQuery = false;
 
         //Ensure all local data is processed
         Sink::ResourceControl::flushMessageQueue(query.resources).exec().waitForFinished();
 
-        //We fetch before the data is available and rely on the live query mechanism to deliver the actual data
         auto model = Sink::Store::loadModel<Sink::ApplicationDomain::Mail>(query);
         QTRY_VERIFY(model->data(QModelIndex(), Sink::Store::ChildrenFetchedRole).toBool());
         //The model is not sorted, but the limited set is sorted, so we can only test for the latest result.
         QCOMPARE(model->rowCount(), 1);
         QCOMPARE(model->index(0, 0).data(Sink::Store::DomainObjectRole).value<Sink::ApplicationDomain::Mail::Ptr>()->getProperty("uid").toByteArray(), QByteArray("testLatest"));
+
+        model->fetchMore(QModelIndex());
+        QTRY_VERIFY(model->data(QModelIndex(), Sink::Store::ChildrenFetchedRole).toBool());
+        QCOMPARE(model->rowCount(), 2);
+        //We can't make any assumptions about the order of the indexes
+        // QCOMPARE(model->index(1, 0).data(Sink::Store::DomainObjectRole).value<Sink::ApplicationDomain::Mail::Ptr>()->getProperty("uid").toByteArray(), QByteArray("testSecond"));
     }
 };