Finish converting DataStoreQuery to new Key API

7 files changed, 75 insertions, 68 deletions

diff --git a/common/datastorequery.cpp b/common/datastorequery.cpp
index fd910f8..8bfc49b 100644
--- a/common/datastorequery.cpp
+++ b/common/datastorequery.cpp
@@ -41,17 +41,23 @@ class Source : public FilterBase {
     public:
     typedef QSharedPointer<Source> Ptr;
 
-    QVector<QByteArray> mIds;
-    QVector<QByteArray>::ConstIterator mIt;
-    QVector<QByteArray> mIncrementalIds;
-    QVector<QByteArray>::ConstIterator mIncrementalIt;
+    QVector<Identifier> mIds;
+    QVector<Identifier>::ConstIterator mIt;
+    QVector<Identifier> mIncrementalIds;
+    QVector<Identifier>::ConstIterator mIncrementalIt;
 
     Source (const QVector<QByteArray> &ids, DataStoreQuery *store)
         : FilterBase(store),
-        mIds(ids),
-        mIt(mIds.constBegin())
+        mIds()
     {
-
+        // Use reserve + append instead of QVector constructor with size here
+        // to avoid default construction, which would generate a new random
+        // Uuid
+        mIds.reserve(ids.size());
+        for (const auto &id : ids) {
+            mIds.append(Identifier::fromDisplayByteArray(id));
+        }
+        mIt = mIds.constBegin();
     }
 
     virtual ~Source(){}
@@ -63,9 +69,15 @@ class Source : public FilterBase {
         }
     };
 
-    void add(const QVector<QByteArray> &ids)
+    void add(const QVector<Key> &keys)
     {
-        mIncrementalIds = ids;
+        // Use clear + reserve + append instead of resize here to avoid default
+        // construction
+        mIncrementalIds.clear();
+        mIncrementalIds.reserve(keys.size());
+        for (const auto &key : keys) {
+            mIncrementalIds.append(key.identifier());
+        }
         mIncrementalIt = mIncrementalIds.constBegin();
     }
 
@@ -284,7 +296,7 @@ public:
                 if (!matchesFilter(entity)) {
                     return;
                 }
-                reducedAndFilteredResults << r;
+                reducedAndFilteredResults << r.toDisplayByteArray();
                 Q_ASSERT(operation != Sink::Operation_Removal);
                 for (auto &aggregator : mAggregators) {
                     if (!aggregator.property.isEmpty()) {
@@ -318,7 +330,8 @@ public:
                     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) {
+                        const auto id = Identifier::fromDisplayByteArray(result.entity.identifier());
+                        readPrevious(id, [&] (const ApplicationDomain::ApplicationDomainType &prev) {
                             Q_ASSERT(result.entity.identifier() == prev.identifier());
                             reductionValue = prev.getProperty(mReductionProperty);
                         });
@@ -345,7 +358,7 @@ public:
                         return;
                     }
                     mSelectedValues.insert(reductionValueBa, reductionResult.selection);
-                    readEntity(reductionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation operation) {
+                    readEntity(Identifier::fromDisplayByteArray(reductionResult.selection), [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation operation) {
                         callback({entity, operation, reductionResult.aggregateValues, reductionResult.aggregateIds});
                         foundValue = true;
                     });
@@ -368,13 +381,13 @@ public:
                         if (oldSelectionResult == selectionResult.selection) {
                             mSelectedValues.insert(reductionValueBa, selectionResult.selection);
                             Q_ASSERT(!selectionResult.selection.isEmpty());
-                            readEntity(selectionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
+                            readEntity(Identifier::fromDisplayByteArray(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) {
+                            readEntity(Identifier::fromDisplayByteArray(oldSelectionResult), [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
                                 callback({entity, Sink::Operation_Removal});
                             });
 
@@ -383,7 +396,7 @@ public:
                                 //add new result
                                 mSelectedValues.insert(reductionValueBa, selectionResult.selection);
                                 Q_ASSERT(!selectionResult.selection.isEmpty());
-                                readEntity(selectionResult.selection, [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
+                                readEntity(Identifier::fromDisplayByteArray(selectionResult.selection), [&](const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation) {
                                     callback({entity, Sink::Operation_Creation, selectionResult.aggregateValues, selectionResult.aggregateIds});
                                 });
                             }
@@ -477,17 +490,17 @@ DataStoreQuery::State::Ptr DataStoreQuery::getState()
     return state;
 }
 
-void DataStoreQuery::readEntity(const QByteArray &key, const BufferCallback &resultCallback)
+void DataStoreQuery::readEntity(const Identifier &id, const BufferCallback &resultCallback)
 {
-    mStore.readLatest(mType, key, resultCallback);
+    mStore.readLatest(mType, id, resultCallback);
 }
 
-void DataStoreQuery::readPrevious(const QByteArray &key, const std::function<void (const ApplicationDomain::ApplicationDomainType &)> &callback)
+void DataStoreQuery::readPrevious(const Identifier &id, const std::function<void (const ApplicationDomain::ApplicationDomainType &)> &callback)
 {
-    mStore.readPrevious(mType, key, mStore.maxRevision(), callback);
+    mStore.readPrevious(mType, id, mStore.maxRevision(), callback);
 }
 
-QVector<QByteArray> DataStoreQuery::indexLookup(const QByteArray &property, const QVariant &value)
+QVector<Identifier> DataStoreQuery::indexLookup(const QByteArray &property, const QVariant &value)
 {
     return mStore.indexLookup(mType, property, value);
 }
@@ -654,10 +667,10 @@ void DataStoreQuery::setupQuery(const Sink::QueryBase &query_)
     mCollector = Collector::Ptr::create(baseSet, this);
 }
 
-QVector<QByteArray> DataStoreQuery::loadIncrementalResultSet(qint64 baseRevision)
+QVector<Key> DataStoreQuery::loadIncrementalResultSet(qint64 baseRevision)
 {
-    QVector<QByteArray> changedKeys;
-    mStore.readRevisions(baseRevision, mType, [&](const QByteArray &key) {
+    QVector<Key> changedKeys;
+    mStore.readRevisions(baseRevision, mType, [&](const Key &key) {
         changedKeys << key;
     });
     return changedKeys;
diff --git a/common/datastorequery.h b/common/datastorequery.h
index 2311585..a224e64 100644
--- a/common/datastorequery.h
+++ b/common/datastorequery.h
@@ -22,7 +22,7 @@
 #include "resultset.h"
 #include "log.h"
 #include "storage/entitystore.h"
-
+#include "storage/key.h"
 
 class Source;
 class Bloom;
@@ -59,13 +59,13 @@ private:
     typedef std::function<bool(const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation)> FilterFunction;
     typedef std::function<void(const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation)> BufferCallback;
 
-    QVector<QByteArray> indexLookup(const QByteArray &property, const QVariant &value);
+    QVector<Sink::Storage::Identifier> indexLookup(const QByteArray &property, const QVariant &value);
 
-    void readEntity(const QByteArray &key, const BufferCallback &resultCallback);
-    void readPrevious(const QByteArray &key, const std::function<void (const Sink::ApplicationDomain::ApplicationDomainType &)> &callback);
+    void readEntity(const Sink::Storage::Identifier &id, const BufferCallback &resultCallback);
+    void readPrevious(const Sink::Storage::Identifier &id, const std::function<void (const Sink::ApplicationDomain::ApplicationDomainType &)> &callback);
 
     ResultSet createFilteredSet(ResultSet &resultSet, const FilterFunction &);
-    QVector<QByteArray> loadIncrementalResultSet(qint64 baseRevision);
+    QVector<Sink::Storage::Key> loadIncrementalResultSet(qint64 baseRevision);
 
     void setupQuery(const Sink::QueryBase &query_);
     QByteArrayList executeSubquery(const Sink::QueryBase &subquery);
@@ -96,22 +96,22 @@ public:
 
     virtual ~FilterBase(){}
 
-    void readEntity(const QByteArray &key, const std::function<void(const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation)> &callback)
+    void readEntity(const Sink::Storage::Identifier &id, const std::function<void(const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Operation)> &callback)
     {
         Q_ASSERT(mDatastore);
-        mDatastore->readEntity(key, callback);
+        mDatastore->readEntity(id, callback);
     }
 
-    QVector<QByteArray> indexLookup(const QByteArray &property, const QVariant &value)
+    QVector<Sink::Storage::Identifier> indexLookup(const QByteArray &property, const QVariant &value)
     {
         Q_ASSERT(mDatastore);
         return mDatastore->indexLookup(property, value);
     }
 
-    void readPrevious(const QByteArray &key, const std::function<void (const Sink::ApplicationDomain::ApplicationDomainType &)> &callback)
+    void readPrevious(const Sink::Storage::Identifier &id, const std::function<void (const Sink::ApplicationDomain::ApplicationDomainType &)> &callback)
     {
         Q_ASSERT(mDatastore);
-        mDatastore->readPrevious(key, callback);
+        mDatastore->readPrevious(id, callback);
     }
 
     virtual void skip() { mSource->skip(); }
diff --git a/common/storage/entitystore.cpp b/common/storage/entitystore.cpp
index be51c2c..672eea1 100644
--- a/common/storage/entitystore.cpp
+++ b/common/storage/entitystore.cpp
@@ -461,11 +461,11 @@ QVector<QByteArray> EntityStore::indexLookup(const QByteArray &type, const Query
     return d->typeIndex(type).query(query, appliedFilters, appliedSorting, d->getTransaction(), d->resourceContext.instanceId());
 }
 
-QVector<QByteArray> EntityStore::indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value)
+QVector<Identifier> EntityStore::indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value)
 {
     if (!d->exists()) {
         SinkTraceCtx(d->logCtx) << "Database is not existing: " << type;
-        return QVector<QByteArray>();
+        return {};
     }
     return d->typeIndex(type).lookup(property, value, d->getTransaction());
 }
@@ -476,9 +476,9 @@ void EntityStore::indexLookup(const QByteArray &type, const QByteArray &property
         SinkTraceCtx(d->logCtx) << "Database is not existing: " << type;
         return;
     }
-    auto list =  d->typeIndex(type).lookup(property, value, d->getTransaction());
-    for (const auto &uid : list) {
-        callback(uid);
+    auto list = indexLookup(type, property, value);
+    for (const auto &id : list) {
+        callback(id.toDisplayByteArray());
     }
     /* Index index(type + ".index." + property, d->transaction); */
     /* index.lookup(value, [&](const QByteArray &sinkId) { */
@@ -530,16 +530,7 @@ void EntityStore::readLatest(const QByteArray &type, const Identifier &uid, cons
 
 void EntityStore::readLatest(const QByteArray &type, const QByteArray &uid, const std::function<void(const ApplicationDomainType &, Sink::Operation)> callback)
 {
-    // TODO: we shouldn't pass whole keys to this function
-    // check the testSingle test from querytest
-    const auto id = [&uid]() {
-        if(uid.size() == Identifier::DISPLAY_REPR_SIZE) {
-            return Identifier::fromDisplayByteArray(uid);
-        } else {
-            return Key::fromDisplayByteArray(uid).identifier();
-        }
-    }();
-    readLatest(type, id, callback);
+    readLatest(type, Identifier::fromDisplayByteArray(uid), callback);
 }
 
 ApplicationDomain::ApplicationDomainType EntityStore::readLatest(const QByteArray &type, const QByteArray &uid)
@@ -588,7 +579,7 @@ void EntityStore::readAll(const QByteArray &type, const std::function<void(const
     });
 }
 
-void EntityStore::readRevisions(qint64 baseRevision, const QByteArray &expectedType, const std::function<void(const QByteArray &key)> &callback)
+void EntityStore::readRevisions(qint64 baseRevision, const QByteArray &expectedType, const std::function<void(const Key &key)> &callback)
 {
     qint64 revisionCounter = baseRevision;
     const qint64 topRevision = DataStore::maxRevision(d->getTransaction());
@@ -607,15 +598,15 @@ void EntityStore::readRevisions(qint64 baseRevision, const QByteArray &expectedT
         const auto key = Key(Identifier::fromDisplayByteArray(uid), revisionCounter);
 
         revisionCounter++;
-        callback(key.toDisplayByteArray());
+        callback(key);
     }
 }
 
-void EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
+void EntityStore::readPrevious(const QByteArray &type, const Identifier &id, qint64 revision, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
 {
     auto db = DataStore::mainDatabase(d->getTransaction(), type);
     qint64 latestRevision = 0;
-    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
+    const auto internalUid = id.toInternalByteArray();
     db.scan(internalUid,
         [&latestRevision, revision](const QByteArray &key, const QByteArray &) -> bool {
             const auto foundRevision = Key::fromInternalByteArray(key).revision().toQint64();
@@ -625,21 +616,21 @@ void EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qi
             return true;
         },
         [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Failed to read current value from storage: " << error.message; }, true);
-    const auto key = Key(Identifier::fromDisplayByteArray(uid), latestRevision);
+    const auto key = Key(id, latestRevision);
     readEntity(type, key.toDisplayByteArray(), callback);
 }
 
-void EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision, const std::function<void(const ApplicationDomainType &)> callback)
+void EntityStore::readPrevious(const QByteArray &type, const Identifier &id, qint64 revision, const std::function<void(const ApplicationDomainType &)> callback)
 {
-    readPrevious(type, uid, revision, [&](const QByteArray &uid, const EntityBuffer &buffer) {
+    readPrevious(type, id, revision, [&](const QByteArray &uid, const EntityBuffer &buffer) {
         callback(d->createApplicationDomainType(type, uid, DataStore::maxRevision(d->getTransaction()), buffer));
     });
 }
 
-ApplicationDomain::ApplicationDomainType EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision)
+ApplicationDomain::ApplicationDomainType EntityStore::readPrevious(const QByteArray &type, const Identifier &id, qint64 revision)
 {
     ApplicationDomainType dt;
-    readPrevious(type, uid, revision, [&](const ApplicationDomainType &entity) {
+    readPrevious(type, id, revision, [&](const ApplicationDomainType &entity) {
         dt = *ApplicationDomainType::getInMemoryRepresentation<ApplicationDomainType>(entity, entity.availableProperties());
     });
     return dt;
diff --git a/common/storage/entitystore.h b/common/storage/entitystore.h
index c23a659..f3aa07d 100644
--- a/common/storage/entitystore.h
+++ b/common/storage/entitystore.h
@@ -59,7 +59,7 @@ public:
 
     QVector<QByteArray> fullScan(const QByteArray &type);
     QVector<QByteArray> indexLookup(const QByteArray &type, const QueryBase &query, QSet<QByteArrayList> &appliedFilters, QByteArray &appliedSorting);
-    QVector<QByteArray> indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value);
+    QVector<Sink::Storage::Identifier> indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value);
     void indexLookup(const QByteArray &type, const QByteArray &property, const QVariant &value, const std::function<void(const QByteArray &uid)> &callback);
     template<typename EntityType, typename PropertyType>
     void indexLookup(const QVariant &value, const std::function<void(const QByteArray &uid)> &callback) {
@@ -97,10 +97,10 @@ public:
     }
 
 
-    void readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback);
-    void readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision, const std::function<void(const ApplicationDomainType &entity)> callback);
+    void readPrevious(const QByteArray &type, const Sink::Storage::Identifier &uid, qint64 revision, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback);
+    void readPrevious(const QByteArray &type, const Sink::Storage::Identifier &id, qint64 revision, const std::function<void(const ApplicationDomainType &entity)> callback);
     ///Returns a copy
-    ApplicationDomainType readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision);
+    ApplicationDomainType readPrevious(const QByteArray &type, const Sink::Storage::Identifier &id, qint64 revision);
 
     template<typename T>
     T readPrevious(const QByteArray &uid, qint64 revision) {
@@ -118,7 +118,7 @@ public:
         });
     }
 
-    void readRevisions(qint64 baseRevision, const QByteArray &type, const std::function<void(const QByteArray &key)> &callback);
+    void readRevisions(qint64 baseRevision, const QByteArray &type, const std::function<void(const Key &key)> &callback);
 
     ///Db contains entity (but may already be marked as removed
     bool contains(const QByteArray &type, const QByteArray &uid);
diff --git a/common/storage/key.h b/common/storage/key.h
index 76dbd13..40f4aca 100644
--- a/common/storage/key.h
+++ b/common/storage/key.h
@@ -76,6 +76,9 @@ public:
     static const constexpr size_t INTERNAL_REPR_SIZE = Identifier::INTERNAL_REPR_SIZE + Revision::INTERNAL_REPR_SIZE;
     static const constexpr size_t DISPLAY_REPR_SIZE = Identifier::DISPLAY_REPR_SIZE + Revision::DISPLAY_REPR_SIZE;
 
+    // Just to be able to store keys into Qt containers
+    [[deprecated("Don't use the default constructor")]]
+    Key() : id(), rev(0) {}
     Key(const Identifier &id, const Revision &rev) : id(id), rev(rev) {}
 
     QByteArray toInternalByteArray() const;
diff --git a/common/typeindex.cpp b/common/typeindex.cpp
index 887a146..1dd54e8 100644
--- a/common/typeindex.cpp
+++ b/common/typeindex.cpp
@@ -427,17 +427,17 @@ QVector<QByteArray> TypeIndex::query(const Sink::QueryBase &query, QSet<QByteArr
     return {};
 }
 
-QVector<QByteArray> TypeIndex::lookup(const QByteArray &property, const QVariant &value,
+QVector<Identifier> TypeIndex::lookup(const QByteArray &property, const QVariant &value,
     Sink::Storage::DataStore::Transaction &transaction)
 {
     SinkTraceCtx(mLogCtx) << "Index lookup on property: " << property << mSecondaryProperties.keys() << mProperties;
     if (mProperties.contains(property)) {
-        QVector<QByteArray> keys;
+        QVector<Identifier> keys;
         Index index(indexName(property), transaction);
         const auto lookupKey = getByteArray(value);
         index.lookup(lookupKey,
             [&](const QByteArray &value) {
-                keys << Identifier::fromInternalByteArray(value).toDisplayByteArray();
+                keys << Identifier::fromInternalByteArray(value);
             },
             [property](const Index::Error &error) {
                 SinkWarning() << "Error in index: " << error.message << property;
@@ -447,7 +447,7 @@ QVector<QByteArray> TypeIndex::lookup(const QByteArray &property, const QVariant
     } else if (mSecondaryProperties.contains(property)) {
         // Lookups on secondary indexes first lookup the key, and then lookup the results again to
         // resolve to entity id's
-        QVector<QByteArray> keys;
+        QVector<Identifier> keys;
         auto resultProperty = mSecondaryProperties.value(property);
 
         QVector<QByteArray> secondaryKeys;
@@ -466,7 +466,7 @@ QVector<QByteArray> TypeIndex::lookup(const QByteArray &property, const QVariant
     } else {
         SinkWarning() << "Tried to lookup " << property << " but couldn't find value";
     }
-    return QVector<QByteArray>();
+    return {};
 }
 
 template <>
diff --git a/common/typeindex.h b/common/typeindex.h
index a701e9c..f2cabaf 100644
--- a/common/typeindex.h
+++ b/common/typeindex.h
@@ -95,7 +95,7 @@ public:
     void remove(const Sink::Storage::Identifier &identifier, const Sink::ApplicationDomain::ApplicationDomainType &entity, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId);
 
     QVector<QByteArray> query(const Sink::QueryBase &query, QSet<QByteArrayList> &appliedFilters, QByteArray &appliedSorting, Sink::Storage::DataStore::Transaction &transaction, const QByteArray &resourceInstanceId);
-    QVector<QByteArray> lookup(const QByteArray &property, const QVariant &value, Sink::Storage::DataStore::Transaction &transaction);
+    QVector<Sink::Storage::Identifier> lookup(const QByteArray &property, const QVariant &value, Sink::Storage::DataStore::Transaction &transaction);
 
     template <typename Left, typename Right>
     QVector<QByteArray> secondaryLookup(const QVariant &value)