14 files changed, 368 insertions, 81 deletions

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 970990f..7c4630b 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -73,6 +73,7 @@ add_library(${PROJECT_NAME} SHARED
     specialpurposepreprocessor.cpp
     datastorequery.cpp
     storage/entitystore.cpp
+    storage/key.cpp
     indexer.cpp
     mail/threadindexer.cpp
     mail/fulltextindexer.cpp
diff --git a/common/changereplay.cpp b/common/changereplay.cpp
index 0adbd78..d7f46dc 100644
--- a/common/changereplay.cpp
+++ b/common/changereplay.cpp
@@ -23,6 +23,7 @@
 #include "log.h"
 #include "definitions.h"
 #include "bufferutils.h"
+#include "storage/key.h"
 
 #include <QTimer>
 
@@ -113,10 +114,13 @@ KAsync::Job<void> ChangeReplay::replayNextRevision()
                         if (uid.isEmpty() || type.isEmpty()) {
                             SinkErrorCtx(mLogCtx) << "Failed to read uid or type for revison: " << revision << uid << type;
                         } else {
-                            const auto key = DataStore::assembleKey(uid, revision);
+                            // TODO: should not use internal representations
+                            const auto key = Storage::Key(Storage::Identifier::fromDisplayByteArray(uid), revision);
+                            const auto internalKey = key.toInternalByteArray();
+                            const auto displayKey = key.toDisplayByteArray();
                             QByteArray entityBuffer;
                             DataStore::mainDatabase(mMainStoreTransaction, type)
-                                .scan(key,
+                                .scan(internalKey,
                                     [&entityBuffer](const QByteArray &key, const QByteArray &value) -> bool {
                                         entityBuffer = value;
                                         return false;
@@ -126,9 +130,9 @@ KAsync::Job<void> ChangeReplay::replayNextRevision()
                             if (entityBuffer.isEmpty()) {
                                 SinkErrorCtx(mLogCtx) << "Failed to replay change " << key;
                             } else {
-                                if (canReplay(type, key, entityBuffer)) {
-                                    SinkTraceCtx(mLogCtx) << "Replaying " << key;
-                                    replayJob = replay(type, key, entityBuffer);
+                                if (canReplay(type, displayKey, entityBuffer)) {
+                                    SinkTraceCtx(mLogCtx) << "Replaying " << displayKey;
+                                    replayJob = replay(type, displayKey, entityBuffer);
                                     //Set the last revision we tried to replay
                                     *lastReplayedRevision = revision;
                                     //Execute replay job and commit
diff --git a/common/storage.h b/common/storage.h
index a8c486c..8904148 100644
--- a/common/storage.h
+++ b/common/storage.h
@@ -22,8 +22,10 @@
 #pragma once
 
 #include "sink_export.h"
+#include "utils.h"
 #include <string>
 #include <functional>
+#include <QUuid>
 #include <QString>
 #include <QMap>
 
@@ -237,10 +239,6 @@ public:
     static bool isInternalKey(void *key, int keySize);
     static bool isInternalKey(const QByteArray &key);
 
-    static QByteArray assembleKey(const QByteArray &key, qint64 revision);
-    static QByteArray uidFromKey(const QByteArray &key);
-    static qint64 revisionFromKey(const QByteArray &key);
-
     static NamedDatabase mainDatabase(const Transaction &, const QByteArray &type);
 
     static QByteArray generateUid();
diff --git a/common/storage/entitystore.cpp b/common/storage/entitystore.cpp
index dd6bbf0..3addf94 100644
--- a/common/storage/entitystore.cpp
+++ b/common/storage/entitystore.cpp
@@ -237,8 +237,10 @@ bool EntityStore::add(const QByteArray &type, ApplicationDomainType entity, bool
     flatbuffers::FlatBufferBuilder fbb;
     d->resourceContext.adaptorFactory(type).createBuffer(entity, fbb, metadataFbb.GetBufferPointer(), metadataFbb.GetSize());
 
+    const auto key = Key(Identifier::fromDisplayByteArray(entity.identifier()), newRevision);
+
     DataStore::mainDatabase(d->transaction, type)
-        .write(DataStore::assembleKey(entity.identifier(), newRevision), BufferUtils::extractBuffer(fbb),
+        .write(key.toInternalByteArray(), BufferUtils::extractBuffer(fbb),
             [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Failed to write entity" << entity.identifier() << newRevision; });
     DataStore::setMaxRevision(d->transaction, newRevision);
     DataStore::recordRevision(d->transaction, newRevision, entity.identifier(), type);
@@ -311,8 +313,10 @@ bool EntityStore::modify(const QByteArray &type, const ApplicationDomainType &cu
     flatbuffers::FlatBufferBuilder fbb;
     d->resourceContext.adaptorFactory(type).createBuffer(newEntity, fbb, metadataFbb.GetBufferPointer(), metadataFbb.GetSize());
 
+    const auto key = Key(Identifier::fromDisplayByteArray(newEntity.identifier()), newRevision);
+
     DataStore::mainDatabase(d->transaction, type)
-        .write(DataStore::assembleKey(newEntity.identifier(), newRevision), BufferUtils::extractBuffer(fbb),
+        .write(key.toInternalByteArray(), BufferUtils::extractBuffer(fbb),
             [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Failed to write entity" << newEntity.identifier() << newRevision; });
     DataStore::setMaxRevision(d->transaction, newRevision);
     DataStore::recordRevision(d->transaction, newRevision, newEntity.identifier(), type);
@@ -346,8 +350,10 @@ bool EntityStore::remove(const QByteArray &type, const ApplicationDomainType &cu
     flatbuffers::FlatBufferBuilder fbb;
     EntityBuffer::assembleEntityBuffer(fbb, metadataFbb.GetBufferPointer(), metadataFbb.GetSize(), 0, 0, 0, 0);
 
+    const auto key = Key(Identifier::fromDisplayByteArray(uid), newRevision);
+
     DataStore::mainDatabase(d->transaction, type)
-        .write(DataStore::assembleKey(uid, newRevision), BufferUtils::extractBuffer(fbb),
+        .write(key.toInternalByteArray(), BufferUtils::extractBuffer(fbb),
             [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Failed to write entity" << uid << newRevision; });
     DataStore::setMaxRevision(d->transaction, newRevision);
     DataStore::recordRevision(d->transaction, newRevision, uid, type);
@@ -365,8 +371,9 @@ void EntityStore::cleanupEntityRevisionsUntil(qint64 revision)
         return;
     }
     SinkTraceCtx(d->logCtx) << "Cleaning up revision " << revision << uid << bufferType;
+    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
     DataStore::mainDatabase(d->transaction, bufferType)
-        .scan(uid,
+        .scan(internalUid,
             [&](const QByteArray &key, const QByteArray &data) -> bool {
                 EntityBuffer buffer(const_cast<const char *>(data.data()), data.size());
                 if (!buffer.isValid()) {
@@ -428,7 +435,7 @@ QVector<QByteArray> EntityStore::fullScan(const QByteArray &type)
     DataStore::mainDatabase(d->getTransaction(), type)
         .scan(QByteArray(),
             [&](const QByteArray &key, const QByteArray &value) -> bool {
-                const auto uid = DataStore::uidFromKey(key);
+                const auto uid = Sink::Storage::Key::fromInternalByteArray(key).identifier().toDisplayByteArray();
                 if (keys.contains(uid)) {
                     //Not something that should persist if the replay works, so we keep a message for now.
                     SinkTraceCtx(d->logCtx) << "Multiple revisions for key: " << key;
@@ -479,16 +486,26 @@ void EntityStore::indexLookup(const QByteArray &type, const QByteArray &property
     /* }); */
 }
 
-void EntityStore::readLatest(const QByteArray &type, const QByteArray &uid, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
+void EntityStore::readLatest(const QByteArray &type, const QByteArray &key, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
 {
     Q_ASSERT(d);
-    Q_ASSERT(!uid.isEmpty());
+    Q_ASSERT(!key.isEmpty());
+    // TODO: we shouldn't pass whole keys to this function
+    // check the testSingle test from querytest
+    const auto internalKey = [&key]() {
+        if(key.size() == Identifier::DISPLAY_REPR_SIZE) {
+            return Identifier::fromDisplayByteArray(key).toInternalByteArray();
+        } else {
+            return Key::fromDisplayByteArray(key).toInternalByteArray();
+        }
+    }();
     auto db = DataStore::mainDatabase(d->getTransaction(), type);
-    db.findLatest(uid,
+    db.findLatest(internalKey,
         [=](const QByteArray &key, const QByteArray &value) {
-            callback(DataStore::uidFromKey(key), Sink::EntityBuffer(value.data(), value.size()));
+            const auto uid = Sink::Storage::Key::fromInternalByteArray(key).identifier().toDisplayByteArray();
+            callback(uid, Sink::EntityBuffer(value.data(), value.size()));
         },
-        [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Error during readLatest query: " << error.message << uid; });
+        [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Error during readLatest query: " << error.message << key; });
 }
 
 void EntityStore::readLatest(const QByteArray &type, const QByteArray &uid, const std::function<void(const ApplicationDomainType &)> callback)
@@ -516,12 +533,14 @@ ApplicationDomain::ApplicationDomainType EntityStore::readLatest(const QByteArra
     return dt;
 }
 
-void EntityStore::readEntity(const QByteArray &type, const QByteArray &key, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
+void EntityStore::readEntity(const QByteArray &type, const QByteArray &displayKey, const std::function<void(const QByteArray &uid, const EntityBuffer &entity)> callback)
 {
+    const auto key = Key::fromDisplayByteArray(displayKey);
     auto db = DataStore::mainDatabase(d->getTransaction(), type);
-    db.scan(key,
+    db.scan(key.toInternalByteArray(),
         [=](const QByteArray &key, const QByteArray &value) -> bool {
-            callback(DataStore::uidFromKey(key), Sink::EntityBuffer(value.data(), value.size()));
+            const auto uid = Sink::Storage::Key::fromInternalByteArray(key).identifier().toDisplayByteArray();
+            callback(uid, Sink::EntityBuffer(value.data(), value.size()));
             return false;
         },
         [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Error during readEntity query: " << error.message << key; });
@@ -567,9 +586,10 @@ void EntityStore::readRevisions(qint64 baseRevision, const QByteArray &expectedT
             revisionCounter++;
             continue;
         }
-        const auto key = DataStore::assembleKey(uid, revisionCounter);
+        const auto key = Key(Identifier::fromDisplayByteArray(uid), revisionCounter);
+
         revisionCounter++;
-        callback(key);
+        callback(key.toDisplayByteArray());
     }
 }
 
@@ -577,16 +597,18 @@ void EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qi
 {
     auto db = DataStore::mainDatabase(d->getTransaction(), type);
     qint64 latestRevision = 0;
-    db.scan(uid,
+    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
+    db.scan(internalUid,
         [&latestRevision, revision](const QByteArray &key, const QByteArray &) -> bool {
-            const auto foundRevision = DataStore::revisionFromKey(key);
+            const auto foundRevision = Key::fromInternalByteArray(key).revision().toQint64();
             if (foundRevision < revision && foundRevision > latestRevision) {
                 latestRevision = foundRevision;
             }
             return true;
         },
         [&](const DataStore::Error &error) { SinkWarningCtx(d->logCtx) << "Failed to read current value from storage: " << error.message; }, true);
-    readEntity(type, DataStore::assembleKey(uid, latestRevision), callback);
+    const auto key = Key(Identifier::fromDisplayByteArray(uid), latestRevision);
+    readEntity(type, key.toDisplayByteArray(), callback);
 }
 
 void EntityStore::readPrevious(const QByteArray &type, const QByteArray &uid, qint64 revision, const std::function<void(const ApplicationDomainType &)> callback)
@@ -612,15 +634,18 @@ void EntityStore::readAllUids(const QByteArray &type, const std::function<void(c
 
 bool EntityStore::contains(const QByteArray &type, const QByteArray &uid)
 {
-    return DataStore::mainDatabase(d->getTransaction(), type).contains(uid);
+    Q_ASSERT(!uid.isEmpty());
+    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
+    return DataStore::mainDatabase(d->getTransaction(), type).contains(internalUid);
 }
 
 bool EntityStore::exists(const QByteArray &type, const QByteArray &uid)
 {
     bool found = false;
     bool alreadyRemoved = false;
+    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
     DataStore::mainDatabase(d->transaction, type)
-        .findLatest(uid,
+        .findLatest(internalUid,
             [&found, &alreadyRemoved](const QByteArray &key, const QByteArray &data) {
                 auto entity = GetEntity(data.data());
                 if (entity && entity->metadata()) {
@@ -647,12 +672,14 @@ void EntityStore::readRevisions(const QByteArray &type, const QByteArray &uid, q
 {
     Q_ASSERT(d);
     Q_ASSERT(!uid.isEmpty());
+    const auto internalUid = Identifier::fromDisplayByteArray(uid).toInternalByteArray();
     DataStore::mainDatabase(d->transaction, type)
-        .scan(uid,
+        .scan(internalUid,
             [&](const QByteArray &key, const QByteArray &value) -> bool {
-                const auto revision = DataStore::revisionFromKey(key);
+                const auto parsedKey = Key::fromInternalByteArray(key);
+                const auto revision = parsedKey.revision().toQint64();
                 if (revision >= startingRevision) {
-                    callback(DataStore::uidFromKey(key), revision, Sink::EntityBuffer(value.data(), value.size()));
+                    callback(parsedKey.identifier().toDisplayByteArray(), revision, Sink::EntityBuffer(value.data(), value.size()));
                 }
                 return true;
             },
diff --git a/common/storage/entitystore.h b/common/storage/entitystore.h
index 69de76c..1dcd285 100644
--- a/common/storage/entitystore.h
+++ b/common/storage/entitystore.h
@@ -25,6 +25,7 @@
 #include "domaintypeadaptorfactoryinterface.h"
 #include "query.h"
 #include "storage.h"
+#include "key.h"
 #include "resourcecontext.h"
 #include "metadata_generated.h"
 
diff --git a/common/storage/key.cpp b/common/storage/key.cpp
new file mode 100644
index 0000000..01a3e3a
--- /dev/null
+++ b/common/storage/key.cpp
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) 2014 Christian Mollekopf <chrigi_1@fastmail.fm>
+ * Copyright (C) 2018 Rémi Nicole <minijackson@riseup.net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) version 3, or any
+ * later version accepted by the membership of KDE e.V. (or its
+ * successor approved by the membership of KDE e.V.), which shall
+ * act as a proxy defined in Section 6 of version 3 of the license.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "key.h"
+#include "utils.h"
+
+using Sink::Storage::Identifier;
+using Sink::Storage::Key;
+using Sink::Storage::Revision;
+
+QDebug &operator<<(QDebug &dbg, const Identifier &id)
+{
+    dbg << id.toDisplayString();
+    return dbg;
+}
+
+QDebug &operator<<(QDebug &dbg, const Revision &rev)
+{
+    dbg << rev.toDisplayString();
+    return dbg;
+}
+
+QDebug &operator<<(QDebug &dbg, const Key &key)
+{
+    dbg << key.toDisplayString();
+    return dbg;
+}
+
+// Identifier
+
+QByteArray Identifier::toInternalByteArray() const
+{
+    return uid.toRfc4122();
+}
+
+Identifier Identifier::fromInternalByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == INTERNAL_REPR_SIZE);
+    return Identifier(QUuid::fromRfc4122(bytes));
+}
+
+QString Identifier::toDisplayString() const
+{
+    return uid.toString();
+}
+
+QByteArray Identifier::toDisplayByteArray() const
+{
+    return uid.toByteArray();
+}
+
+Identifier Identifier::fromDisplayByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == DISPLAY_REPR_SIZE);
+    return Identifier(QUuid(bytes));
+}
+
+// Revision
+
+QByteArray Revision::toInternalByteArray() const
+{
+    return padNumber(rev);
+}
+
+Revision Revision::fromInternalByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == INTERNAL_REPR_SIZE);
+    return Revision(bytes.toLongLong());
+}
+
+QString Revision::toDisplayString() const
+{
+    return QString::fromUtf8(toInternalByteArray());
+}
+
+QByteArray Revision::toDisplayByteArray() const
+{
+    return toInternalByteArray();
+}
+
+Revision Revision::fromDisplayByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == DISPLAY_REPR_SIZE);
+    return fromInternalByteArray(bytes);
+}
+
+qint64 Revision::toQint64() const
+{
+    return rev;
+}
+
+// Key
+
+QByteArray Key::toInternalByteArray() const
+{
+    return id.toInternalByteArray() + rev.toInternalByteArray();
+}
+
+Key Key::fromInternalByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == INTERNAL_REPR_SIZE);
+    auto idBytes = bytes.mid(0, Identifier::INTERNAL_REPR_SIZE);
+    auto revBytes = bytes.mid(Identifier::INTERNAL_REPR_SIZE);
+    return Key(Identifier::fromInternalByteArray(idBytes), Revision::fromInternalByteArray(revBytes));
+}
+
+QString Key::toDisplayString() const
+{
+    return id.toDisplayString() + rev.toDisplayString();
+}
+
+QByteArray Key::toDisplayByteArray() const
+{
+    return id.toDisplayByteArray() + rev.toDisplayByteArray();
+}
+
+Key Key::fromDisplayByteArray(const QByteArray &bytes)
+{
+    Q_ASSERT(bytes.size() == DISPLAY_REPR_SIZE);
+    auto idBytes = bytes.mid(0, Identifier::DISPLAY_REPR_SIZE);
+    auto revBytes = bytes.mid(Identifier::DISPLAY_REPR_SIZE);
+    return Key(Identifier::fromDisplayByteArray(idBytes), Revision::fromDisplayByteArray(revBytes));
+}
+
+const Identifier &Key::identifier() const
+{
+    return id;
+}
+
+const Revision &Key::revision() const
+{
+    return rev;
+}
+
+void Key::setRevision(const Revision &newRev)
+{
+    rev = newRev;
+}
diff --git a/common/storage/key.h b/common/storage/key.h
new file mode 100644
index 0000000..76dbd13
--- /dev/null
+++ b/common/storage/key.h
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2014 Christian Mollekopf <chrigi_1@fastmail.fm>
+ * Copyright (C) 2018 Rémi Nicole <minijackson@riseup.net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) version 3, or any
+ * later version accepted by the membership of KDE e.V. (or its
+ * successor approved by the membership of KDE e.V.), which shall
+ * act as a proxy defined in Section 6 of version 3 of the license.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "sink_export.h"
+
+#include <QByteArray>
+#include <QDebug>
+#include <QUuid>
+
+namespace Sink {
+namespace Storage {
+
+class Identifier
+{
+public:
+    // RFC 4122 Section 4.1.2 says 128 bits -> 16 bytes
+    static const constexpr size_t INTERNAL_REPR_SIZE = 16;
+    static const constexpr size_t DISPLAY_REPR_SIZE = 38;
+
+    Identifier() : uid(QUuid::createUuid()){};
+
+    QByteArray toInternalByteArray() const;
+    static Identifier fromInternalByteArray(const QByteArray &bytes);
+    QString toDisplayString() const;
+    QByteArray toDisplayByteArray() const;
+    static Identifier fromDisplayByteArray(const QByteArray &bytes);
+
+private:
+    explicit Identifier(const QUuid &uid) : uid(uid) {}
+    QUuid uid;
+};
+
+class Revision
+{
+public:
+    // qint64 has a 19 digit decimal representation
+    static const constexpr size_t INTERNAL_REPR_SIZE = 19;
+    static const constexpr size_t DISPLAY_REPR_SIZE = 19;
+
+    Revision(qint64 rev) : rev(rev) {}
+
+    QByteArray toInternalByteArray() const;
+    static Revision fromInternalByteArray(const QByteArray &bytes);
+    QString toDisplayString() const;
+    QByteArray toDisplayByteArray() const;
+    static Revision fromDisplayByteArray(const QByteArray &bytes);
+    qint64 toQint64() const;
+
+private:
+    qint64 rev;
+};
+
+class Key
+{
+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;
+
+    Key(const Identifier &id, const Revision &rev) : id(id), rev(rev) {}
+
+    QByteArray toInternalByteArray() const;
+    static Key fromInternalByteArray(const QByteArray &bytes);
+    QString toDisplayString() const;
+    QByteArray toDisplayByteArray() const;
+    static Key fromDisplayByteArray(const QByteArray &bytes);
+    const Identifier &identifier() const;
+    const Revision &revision() const;
+    void setRevision(const Revision &newRev);
+
+private:
+    Identifier id;
+    Revision rev;
+};
+
+} // namespace Storage
+} // namespace Sink
+
+SINK_EXPORT QDebug& operator<<(QDebug &dbg, const Sink::Storage::Identifier &);
+SINK_EXPORT QDebug& operator<<(QDebug &dbg, const Sink::Storage::Revision &);
+SINK_EXPORT QDebug& operator<<(QDebug &dbg, const Sink::Storage::Key &);
diff --git a/common/storage_common.cpp b/common/storage_common.cpp
index 057dce4..f96097a 100644
--- a/common/storage_common.cpp
+++ b/common/storage_common.cpp
@@ -194,23 +194,6 @@ bool DataStore::isInternalKey(const QByteArray &key)
     return key.startsWith(s_internalPrefix);
 }
 
-QByteArray DataStore::assembleKey(const QByteArray &key, qint64 revision)
-{
-    Q_ASSERT(revision <= 9223372036854775807);
-    Q_ASSERT(key.size() == s_lengthOfUid);
-    return key + QByteArray::number(revision).rightJustified(19, '0', false);
-}
-
-QByteArray DataStore::uidFromKey(const QByteArray &key)
-{
-    return key.mid(0, s_lengthOfUid);
-}
-
-qint64 DataStore::revisionFromKey(const QByteArray &key)
-{
-    return key.mid(s_lengthOfUid + 1).toLongLong();
-}
-
 QByteArray DataStore::generateUid()
 {
     return createUuid();
diff --git a/common/synchronizer.cpp b/common/synchronizer.cpp
index 71091a6..a43ec06 100644
--- a/common/synchronizer.cpp
+++ b/common/synchronizer.cpp
@@ -616,7 +616,8 @@ KAsync::Job<void> Synchronizer::replay(const QByteArray &type, const QByteArray
     Q_ASSERT(mEntityStore->hasTransaction());
 
     const auto operation = metadataBuffer ? metadataBuffer->operation() : Sink::Operation_Creation;
-    const auto uid = Sink::Storage::DataStore::uidFromKey(key);
+    // TODO: should not use internal representations
+    const auto uid = Sink::Storage::Key::fromDisplayByteArray(key).identifier().toDisplayByteArray();
     const auto modifiedProperties = metadataBuffer->modifiedProperties() ? BufferUtils::fromVector(*metadataBuffer->modifiedProperties()) : QByteArrayList();
     QByteArray oldRemoteId;
 
diff --git a/common/typeindex.cpp b/common/typeindex.cpp
index b18791f..0b78d59 100644
--- a/common/typeindex.cpp
+++ b/common/typeindex.cpp
@@ -25,8 +25,6 @@
 #include <QDateTime>
 #include <QDataStream>
 
-#include <cmath>
-
 using namespace Sink;
 
 static QByteArray getByteArray(const QVariant &value)
@@ -53,13 +51,6 @@ static QByteArray getByteArray(const QVariant &value)
     return "toplevel";
 }
 
-template <typename T>
-static QByteArray padNumber(T number)
-{
-    static T uint_num_digits = (T)std::log10(std::numeric_limits<T>::max()) + 1;
-    return QByteArray::number(number).rightJustified(uint_num_digits, '0');
-}
-
 static QByteArray toSortableByteArrayImpl(const QDateTime &date)
 {
     // Sort invalid last
diff --git a/common/utils.h b/common/utils.h
index 253de61..7066d79 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -20,6 +20,17 @@
 
 #include <QByteArray>
 
+#include <cmath>
+
 namespace Sink {
-    QByteArray createUuid();
+
+QByteArray createUuid();
+
+template <typename T>
+static QByteArray padNumber(T number)
+{
+    static T uint_num_digits = (T)std::log10(std::numeric_limits<T>::max()) + 1;
+    return QByteArray::number(number).rightJustified(uint_num_digits, '0');
 }
+
+} // namespace Sink
diff --git a/sinksh/syntax_modules/sink_inspect.cpp b/sinksh/syntax_modules/sink_inspect.cpp
index e0e30c7..7a41ef0 100644
--- a/sinksh/syntax_modules/sink_inspect.cpp
+++ b/sinksh/syntax_modules/sink_inspect.cpp
@@ -32,6 +32,8 @@
 #include "common/bufferutils.h"
 #include "common/fulltextindex.h"
 
+#include "storage/key.h"
+
 #include "sinksh_utils.h"
 #include "state.h"
 #include "syntaxtree.h"
@@ -88,7 +90,7 @@ bool inspect(const QStringList &args, State &state)
 
         QSet<QByteArray> uids;
         db.scan("", [&] (const QByteArray &key, const QByteArray &data) {
-                    uids.insert(Sink::Storage::DataStore::uidFromKey(key));
+                    uids.insert(Sink::Storage::Key::fromInternalByteArray(key).identifier().toDisplayByteArray());
                     return true;
                 },
                 [&](const Sink::Storage::DataStore::Error &e) {
diff --git a/tests/pipelinetest.cpp b/tests/pipelinetest.cpp
index 45e2fbb..b41a5c2 100644
--- a/tests/pipelinetest.cpp
+++ b/tests/pipelinetest.cpp
@@ -20,6 +20,7 @@
 #include "domainadaptor.h"
 #include "definitions.h"
 #include "adaptorfactoryregistry.h"
+#include "storage/key.h"
 
 static void removeFromDisk(const QString &name)
 {
@@ -250,8 +251,8 @@ private slots:
         // Get uid of written entity
         auto keys = getKeys(instanceIdentifier(), "event.main");
         QCOMPARE(keys.size(), 1);
-        const auto key = keys.first();
-        const auto uid = Sink::Storage::DataStore::uidFromKey(key);
+        auto key = Sink::Storage::Key::fromInternalByteArray(keys.first());
+        const auto uid = key.identifier().toDisplayByteArray();
 
         // Execute the modification
         entityFbb.Clear();
@@ -260,8 +261,10 @@ private slots:
         pipeline.modifiedEntity(modifyCommand.constData(), modifyCommand.size());
         pipeline.commit();
 
+        key.setRevision(2);
+
         // Ensure we've got the new revision with the modification
-        auto buffer = getEntity(instanceIdentifier(), "event.main", Sink::Storage::DataStore::assembleKey(uid, 2));
+        auto buffer = getEntity(instanceIdentifier(), "event.main", key.toInternalByteArray());
         QVERIFY(!buffer.isEmpty());
         Sink::EntityBuffer entityBuffer(buffer.data(), buffer.size());
         auto adaptor = adaptorFactory->createAdaptor(entityBuffer.entity());
@@ -296,7 +299,8 @@ private slots:
         // Get uid of written entity
         auto keys = getKeys(instanceIdentifier(), "event.main");
         QCOMPARE(keys.size(), 1);
-        const auto uid = Sink::Storage::DataStore::uidFromKey(keys.first());
+        auto key = Sink::Storage::Key::fromInternalByteArray(keys.first());
+        const auto uid = key.identifier().toDisplayByteArray();
 
 
         // Create another operation inbetween
@@ -315,8 +319,10 @@ private slots:
         pipeline.modifiedEntity(modifyCommand.constData(), modifyCommand.size());
         pipeline.commit();
 
+        key.setRevision(3);
+
         // Ensure we've got the new revision with the modification
-        auto buffer = getEntity(instanceIdentifier(), "event.main", Sink::Storage::DataStore::assembleKey(uid, 3));
+        auto buffer = getEntity(instanceIdentifier(), "event.main", key.toInternalByteArray());
         QVERIFY(!buffer.isEmpty());
         Sink::EntityBuffer entityBuffer(buffer.data(), buffer.size());
         auto adaptor = adaptorFactory->createAdaptor(entityBuffer.entity());
@@ -337,7 +343,7 @@ private slots:
         auto result = getKeys(instanceIdentifier(), "event.main");
         QCOMPARE(result.size(), 1);
 
-        const auto uid = Sink::Storage::DataStore::uidFromKey(result.first());
+        const auto uid = Sink::Storage::Key::fromInternalByteArray(result.first()).identifier().toDisplayByteArray();
 
         // Delete entity
         auto deleteCommand = deleteEntityCommand(uid, 1);
@@ -372,22 +378,22 @@ private slots:
             pipeline.newEntity(command.constData(), command.size());
             QCOMPARE(testProcessor->newUids.size(), 1);
             QCOMPARE(testProcessor->newRevisions.size(), 1);
-            // Key doesn't contain revision and is just the uid
-            QCOMPARE(testProcessor->newUids.at(0), Sink::Storage::DataStore::uidFromKey(testProcessor->newUids.at(0)));
+            const auto uid = Sink::Storage::Identifier::fromDisplayByteArray(testProcessor->newUids.at(0)).toDisplayByteArray();
+            QCOMPARE(testProcessor->newUids.at(0), uid);
         }
         pipeline.commit();
         entityFbb.Clear();
         pipeline.startTransaction();
         auto keys = getKeys(instanceIdentifier(), "event.main");
         QCOMPARE(keys.size(), 1);
-        const auto uid = Sink::Storage::DataStore::uidFromKey(keys.first());
+        const auto uid = Sink::Storage::Key::fromInternalByteArray(keys.first()).identifier().toDisplayByteArray();
         {
             auto modifyCommand = modifyEntityCommand(createEvent(entityFbb, "summary2"), uid, 1);
             pipeline.modifiedEntity(modifyCommand.constData(), modifyCommand.size());
             QCOMPARE(testProcessor->modifiedUids.size(), 1);
             QCOMPARE(testProcessor->modifiedRevisions.size(), 1);
-            // Key doesn't contain revision and is just the uid
-            QCOMPARE(testProcessor->modifiedUids.at(0), Sink::Storage::DataStore::uidFromKey(testProcessor->modifiedUids.at(0)));
+            const auto uid2 = Sink::Storage::Identifier::fromDisplayByteArray(testProcessor->modifiedUids.at(0)).toDisplayByteArray();
+            QCOMPARE(testProcessor->modifiedUids.at(0), uid2);
         }
         pipeline.commit();
         entityFbb.Clear();
@@ -398,8 +404,8 @@ private slots:
             QCOMPARE(testProcessor->deletedUids.size(), 1);
             QCOMPARE(testProcessor->deletedUids.size(), 1);
             QCOMPARE(testProcessor->deletedSummaries.size(), 1);
-            // Key doesn't contain revision and is just the uid
-            QCOMPARE(testProcessor->deletedUids.at(0), Sink::Storage::DataStore::uidFromKey(testProcessor->deletedUids.at(0)));
+            const auto uid2 = Sink::Storage::Identifier::fromDisplayByteArray(testProcessor->modifiedUids.at(0)).toDisplayByteArray();
+            QCOMPARE(testProcessor->deletedUids.at(0), uid2);
             QCOMPARE(testProcessor->deletedSummaries.at(0), QByteArray("summary2"));
         }
     }
@@ -421,8 +427,8 @@ private slots:
         // Get uid of written entity
         auto keys = getKeys(instanceIdentifier(), "event.main");
         QCOMPARE(keys.size(), 1);
-        const auto key = keys.first();
-        const auto uid = Sink::Storage::DataStore::uidFromKey(key);
+        auto key = Sink::Storage::Key::fromInternalByteArray(keys.first());
+        const auto uid = key.identifier().toDisplayByteArray();
 
         //Simulate local modification
         {
@@ -444,8 +450,10 @@ private slots:
             pipeline.commit();
         }
 
+        key.setRevision(3);
+
         // Ensure we've got the new revision with the modification
-        auto buffer = getEntity(instanceIdentifier(), "event.main", Sink::Storage::DataStore::assembleKey(uid, 3));
+        auto buffer = getEntity(instanceIdentifier(), "event.main", key.toInternalByteArray());
         QVERIFY(!buffer.isEmpty());
         Sink::EntityBuffer entityBuffer(buffer.data(), buffer.size());
         auto adaptor = adaptorFactory->createAdaptor(entityBuffer.entity());
diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp
index 96368a7..e165616 100644
--- a/tests/storagetest.cpp
+++ b/tests/storagetest.cpp
@@ -7,6 +7,7 @@
 #include <QtConcurrent/QtConcurrentRun>
 
 #include "common/storage.h"
+#include "storage/key.h"
 
 /**
  * Test of the storage implementation to ensure it can do the low level operations as expected.
@@ -499,8 +500,11 @@ private slots:
         auto db = transaction.openDatabase("test", nullptr, false);
         const auto uid = "{c5d06a9f-1534-4c52-b8ea-415db68bdadf}";
         //Ensure we can sort 1 and 10 properly (by default string comparison 10 comes before 6)
-        db.write(Sink::Storage::DataStore::assembleKey(uid, 6), "value1");
-        db.write(Sink::Storage::DataStore::assembleKey(uid, 10), "value2");
+        const auto id = Sink::Storage::Identifier::fromDisplayByteArray(uid);
+        auto key = Sink::Storage::Key(id, 6);
+        db.write(key.toInternalByteArray(), "value1");
+        key.setRevision(10);
+        db.write(key.toInternalByteArray(), "value2");
         db.findLatest(uid, [&](const QByteArray &key, const QByteArray &value) { result = value; });
         QCOMPARE(result, QByteArray("value2"));
     }
@@ -732,7 +736,7 @@ private slots:
         Sink::Storage::DataStore::clearEnv();
 
         //Try to read-only dynamic opening of the db.
-        //This is the case if we don't have all databases available upon initializatoin and we don't (e.g. because the db hasn't been created yet) 
+        //This is the case if we don't have all databases available upon initializatoin and we don't (e.g. because the db hasn't been created yet)
         {
             // Trick the db into not loading all dbs by passing in a bogus layout.
             Sink::Storage::DataStore store(testDataPath, {dbName, {{"bogus", 0}}}, Sink::Storage::DataStore::ReadOnly);
@@ -750,7 +754,7 @@ private slots:
 
         Sink::Storage::DataStore::clearEnv();
         //Try to read-write dynamic opening of the db.
-        //This is the case if we don't have all databases available upon initializatoin and we don't (e.g. because the db hasn't been created yet) 
+        //This is the case if we don't have all databases available upon initializatoin and we don't (e.g. because the db hasn't been created yet)
         {
             // Trick the db into not loading all dbs by passing in a bogus layout.
             Sink::Storage::DataStore store(testDataPath, {dbName, {{"bogus", 0}}}, Sink::Storage::DataStore::ReadWrite);