hide the implementation detail of which key/value store we use

also makes transactions "implicit" where necessary. this will make
trying out other k/v stores a bit easier now as well.

3 files changed, 186 insertions, 120 deletions

diff --git a/store/database.cpp b/store/database.cpp
index 2d93266..22d0693 100644
--- a/store/database.cpp
+++ b/store/database.cpp
@@ -1,134 +1,188 @@
 #include "database.h"
 
 #include <iostream>
+
+#include <QAtomicInt>
+#include <QDebug>
 #include <QDir>
+#include <QReadWriteLock>
 #include <QString>
 #include <QTime>
-#include <qdebug.h>
 
-Database::Database(const QString &path)
+#include <lmdb.h>
+
+class Database::Private
 {
-    int rc;
+public:
+    Private(const QString &path);
+    ~Private();
 
+    MDB_dbi dbi;
+    MDB_env *env;
+    MDB_txn *transaction;
+    bool readTransaction;
+};
+
+Database::Private::Private(const QString &path)
+    : transaction(0),
+      readTransaction(false)
+{
     QDir dir;
     dir.mkdir(path);
 
     //create file
-    rc = mdb_env_create(&env);
-    rc = mdb_env_open(env, path.toStdString().data(), 0, 0664);
-    const int dbSize = 10485760*100; //10MB * 100
-    mdb_env_set_mapsize(env, dbSize);
+    if (mdb_env_create(&env)) {
+        // TODO: handle error
+    } else {
+        int rc = mdb_env_open(env, path.toStdString().data(), 0, 0664);
 
-    if (rc) {
-        std::cerr << "mdb_env_open: " << rc << mdb_strerror(rc) << std::endl;
+        if (rc) {
+            std::cerr << "mdb_env_open: " << rc << mdb_strerror(rc) << std::endl;
+            mdb_env_close(env);
+            env = 0;
+        } else {
+            const int dbSize = 10485760*100; //10MB * 100
+            mdb_env_set_mapsize(env, dbSize);
+        }
     }
 }
 
-Database::~Database()
+Database::Private::~Private()
 {
-    mdb_close(env, dbi);
+    if (transaction) {
+        mdb_txn_abort(transaction);
+    }
+
+    // it is still there and still unused, so we can shut it down
+    mdb_dbi_close(env, dbi);
     mdb_env_close(env);
 }
 
-MDB_txn *Database::startTransaction()
+Database::Database(const QString &path)
+    : d(new Private(path))
+{
+}
+
+Database::~Database()
+{
+    delete d;
+}
+
+bool Database::isInTransaction() const
+{
+    return d->transaction;
+}
+
+bool Database::startTransaction(TransactionType type)
 {
+    if (!d->env) {
+        return false;
+    }
+
+    bool requestedRead = type == ReadOnly;
+    if (d->transaction && (!d->readTransaction || requestedRead)) {
+        return true;
+    }
+
+    if (d->transaction) {
+        // we are about to turn a read transaction into a writable one
+        abortTransaction();
+    }
+
+    //TODO handle errors
     int rc;
-    MDB_txn *transaction;
-    rc = mdb_txn_begin(env, NULL, 0, &transaction);
-    rc = mdb_open(transaction, NULL, 0, &dbi);
-    return transaction;
+    rc = mdb_txn_begin(d->env, NULL, requestedRead ? MDB_RDONLY : 0, &d->transaction);
+    if (!rc) {
+        rc = mdb_dbi_open(d->transaction, NULL, 0, &d->dbi);
+    }
+
+    return !rc;
 }
 
-void Database::endTransaction(MDB_txn *transaction)
+bool Database::commitTransaction()
 {
+    if (!d->env) {
+        return false;
+    }
+
+    if (!d->transaction) {
+        return false;
+    }
+
     int rc;
-    rc = mdb_txn_commit(transaction);
+    rc = mdb_txn_commit(d->transaction);
+    d->transaction = 0;
+
     if (rc) {
         std::cerr << "mdb_txn_commit: " << rc << mdb_strerror(rc) << std::endl;
     }
+
+    return !rc;
 }
 
+void Database::abortTransaction()
+{
+    if (!d->env || !d->transaction) {
+        return;
+    }
+
+    mdb_txn_abort(d->transaction);
+    d->transaction = 0;
+}
 
-void Database::write(const std::string &sKey, const std::string &sValue, MDB_txn *transaction)
+bool Database::write(const std::string &sKey, const std::string &sValue)
 {
+    if (!d->env) {
+        return false;
+    }
+
+    const bool implicitTransaction = !d->transaction || d->readTransaction;
+    if (implicitTransaction) {
+        // TODO: if this fails, still try the write below?
+        if (!startTransaction()) {
+            return false;
+        }
+    }
+
     int rc;
     MDB_val key, data;
     key.mv_size = sKey.size();
     key.mv_data = (void*)sKey.data();
     data.mv_size = sValue.size();
     data.mv_data = (void*)sValue.data();
-    rc = mdb_put(transaction, dbi, &key, &data, 0);
+    rc = mdb_put(d->transaction, d->dbi, &key, &data, 0);
+
     if (rc) {
         std::cerr << "mdb_put: " << rc << mdb_strerror(rc) << std::endl;
     }
-}
 
-void Database::read(const std::string &sKey, const std::function<void(const std::string)> &resultHandler)
-{
-    int rc;
-    MDB_txn *txn;
-    MDB_val key;
-    MDB_val data;
-    MDB_cursor *cursor;
-
-    key.mv_size = sKey.size();
-    key.mv_data = (void*)sKey.data();
-
-    rc = mdb_txn_begin(env, NULL, MDB_RDONLY, &txn);
-    rc = mdb_cursor_open(txn, dbi, &cursor);
-    if (sKey.empty()) {
-        std::cout << "Iterating over all values of store!" << std::endl;
-        while ((rc = mdb_cursor_get(cursor, &key, &data, MDB_NEXT)) == 0) {
-            const std::string resultKey(static_cast<char*>(key.mv_data), key.mv_size);
-            const std::string resultValue(static_cast<char*>(data.mv_data), data.mv_size);
-            // std::cout << "key: " << resultKey << " data: " << resultValue << std::endl;
-            resultHandler(resultValue);
-        }
-    } else {
-        if ((rc = mdb_cursor_get(cursor, &key, &data, MDB_SET)) == 0) {
-            const std::string resultKey(static_cast<char*>(key.mv_data), key.mv_size);
-            const std::string resultValue(static_cast<char*>(data.mv_data), data.mv_size);
-            // std::cout << "key: " << resultKey << " data: " << resultValue << std::endl;
-            resultHandler(resultValue);
+    if (implicitTransaction) {
+        if (rc) {
+            abortTransaction();
         } else {
-            std::cout << "couldn't find value " << sKey << std::endl;
+            rc = commitTransaction();
         }
     }
-    if (rc) {
-        std::cerr << "mdb_cursor_get: " << rc << mdb_strerror(rc) << std::endl;
-    }
-    mdb_cursor_close(cursor);
-    mdb_txn_abort(txn);
-}
 
-
-
-ReadTransaction::ReadTransaction(const QString &path)
-{
-    int rc;
-
-    //create file
-    rc = mdb_env_create(&env);
-    //FIXME MDB_NOTLS required to so we can have multiple read-only transactions per resource?
-    rc = mdb_env_open(env, path.toStdString().data(), 0, 0664);
-    const int dbSize = 10485760*100; //10MB * 100
-    mdb_env_set_mapsize(env, dbSize);
-
-    if (rc) {
-        std::cerr << "mdb_env_open: " << rc << mdb_strerror(rc) << std::endl;
-    }
+    return !rc;
 }
 
-ReadTransaction::~ReadTransaction()
+void Database::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
 {
-    mdb_txn_abort(txn);
-
-    mdb_dbi_close(env, dbi);
-    mdb_env_close(env);
+    read(sKey,
+         [&](void *ptr, int size) {
+            const std::string resultValue(static_cast<char*>(ptr), size);
+            resultHandler(resultValue);
+         });
+// std::cout << "key: " << resultKey << " data: " << resultValue << std::endl;
 }
 
-void ReadTransaction::read(const std::string &sKey, const std::function<void(void *data, int size)> &resultHandler)
+void Database::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
 {
+    if (!d->env) {
+        return;
+    }
+
     int rc;
     MDB_val key;
     MDB_val data;
@@ -137,6 +191,8 @@ void ReadTransaction::read(const std::string &sKey, const std::function<void(voi
     key.mv_size = sKey.size();
     key.mv_data = (void*)sKey.data();
 
+    /*
+TODO: do we need a write transaction before a read transaction? only relevant for implicitTransactions in any case
     {
         //A write transaction is at least required the first time
         rc = mdb_txn_begin(env, nullptr, 0, &txn);
@@ -145,18 +201,27 @@ void ReadTransaction::read(const std::string &sKey, const std::function<void(voi
         rc = mdb_dbi_open(txn, nullptr, 0, &dbi);
         mdb_txn_abort(txn);
     }
+    */
+    const bool implicitTransaction = !d->transaction;
+    if (implicitTransaction) {
+        // TODO: if this fails, still try the write below?
+        if (!startTransaction(ReadOnly)) {
+            return;
+        }
+    }
 
-    rc = mdb_txn_begin(env, nullptr, MDB_RDONLY, &txn);
-    rc = mdb_cursor_open(txn, dbi, &cursor);
+    rc = mdb_cursor_open(d->transaction, d->dbi, &cursor);
     if (rc) {
-        std::cerr << "mdb_cursor_open: " << rc << mdb_strerror(rc) << std::endl;
+        std::cerr << "mdb_cursor_get: " << rc << mdb_strerror(rc) << std::endl;
+        return;
     }
+
     if (sKey.empty()) {
         std::cout << "Iterating over all values of store!" << std::endl;
-        rc = mdb_cursor_get(cursor, &key, &data, MDB_FIRST);
         while ((rc = mdb_cursor_get(cursor, &key, &data, MDB_NEXT)) == 0) {
-            resultHandler(data.mv_data, data.mv_size);
+            resultHandler(key.mv_data, data.mv_size);
         }
+
         //We never find the last value
         if (rc == MDB_NOTFOUND) {
             rc = 0;
@@ -168,10 +233,18 @@ void ReadTransaction::read(const std::string &sKey, const std::function<void(voi
             std::cout << "couldn't find value " << sKey << std::endl;
         }
     }
+
     if (rc) {
         std::cerr << "mdb_cursor_get: " << rc << mdb_strerror(rc) << std::endl;
     }
+
     mdb_cursor_close(cursor);
-    //We keep the transaction open since we want to keep the returned values alive
+
+    /**
+      we don't abort the transaction since we need it for reading the values
+    if (implicitTransaction) {
+        abortTransaction();
+    }
+    */
 }
 
diff --git a/store/database.h b/store/database.h
index ab398a4..2bf0556 100644
--- a/store/database.h
+++ b/store/database.h
@@ -1,38 +1,25 @@
 #pragma once
 
-#include <lmdb.h>
 #include <string>
 #include <QString>
 
 class Database {
 public:
+    enum TransactionType { ReadOnly, ReadWrite };
+
     Database(const QString &path);
     ~Database();
-    MDB_txn *startTransaction();
-    void endTransaction(MDB_txn *transaction);
-    void write(const std::string &sKey, const std::string &sValue, MDB_txn *transaction);
+    bool isInTransaction() const;
+    bool startTransaction(TransactionType type = ReadWrite);
+    bool commitTransaction();
+    void abortTransaction();
+    bool write(const std::string &sKey, const std::string &sValue);
     //Perhaps prefer iterators (assuming we need to be able to match multiple values
-    void read(const std::string &sKey, const std::function<void(const std::string)> &);
-
-private:
-    MDB_env *env;
-    MDB_dbi dbi;
-};
-
-/*
- * This opens the db for a single read transaction.
- *
- * The lifetime of all read values is tied to this transaction.
- */
-class ReadTransaction {
-public:
-    ReadTransaction(const QString &path);
-    ~ReadTransaction();
-
+    void read(const std::string &sKey, const std::function<void(const std::string &value)> &);
     void read(const std::string &sKey, const std::function<void(void *ptr, int size)> &);
 
 private:
-    MDB_env *env;
-    MDB_dbi dbi;
-    MDB_txn *txn;
+    class Private;
+    Private * const d;
 };
+
diff --git a/store/test/storagebenchmark.cpp b/store/test/storagebenchmark.cpp
index 6fc50f6..6f9f18b 100644
--- a/store/test/storagebenchmark.cpp
+++ b/store/test/storagebenchmark.cpp
@@ -1,12 +1,14 @@
 #include <QtTest>
 
 #include "calendar_generated.h"
+
 #include <iostream>
 #include <fstream>
+
+#include <QDebug>
 #include <QDir>
 #include <QString>
 #include <QTime>
-#include <qdebug.h>
 
 #include "store/database.h"
 
@@ -75,7 +77,10 @@ private Q_SLOTS:
         QFETCH(bool, useDb);
         QFETCH(int, count);
 
-        Database db(dbPath);
+        Database *db = 0;
+        if (useDb) {
+            db = new Database(dbPath);
+        }
 
         std::ofstream myfile;
         myfile.open(filePath.toStdString());
@@ -85,21 +90,22 @@ private Q_SLOTS:
 
         time.start();
         {
-            auto transaction = db.startTransaction();
             auto event = createEvent();
             for (int i = 0; i < count; i++) {
-                if (useDb && i > 0 && (i % 10000 == 0)) {
-                    db.endTransaction(transaction);
-                    transaction = db.startTransaction();
-                }
-                if (useDb) {
-                    db.write(keyPrefix + std::to_string(i), event, transaction);
+                if (db) {
+                    if (i % 10000 == 0) {
+                        db->commitTransaction();
+                        db->startTransaction();
+                    }
+
+                    db->write(keyPrefix + std::to_string(i), event);
                 } else {
                     myfile << event;
                 }
             }
-            if (useDb) {
-                db.endTransaction(transaction);
+
+            if (db) {
+                db->commitTransaction();
             } else {
                 myfile.close();
             }
@@ -110,8 +116,8 @@ private Q_SLOTS:
         time.start();
         {
             for (int i = 0; i < count; i++) {
-                if (useDb) {
-                    db.read(keyPrefix + std::to_string(i));
+                if (db) {
+                    db->read(keyPrefix + std::to_string(i), [](void *ptr, int size){});
                 }
             }
         }