From 351a66b5fb1c8659bff8ea20d60f5a6d2d3263ad Mon Sep 17 00:00:00 2001
From: Aaron Seigo <aseigo@kde.org>
Date: Fri, 5 Dec 2014 09:46:53 +0100
Subject: make read return a bool on success

not happy with this API, but we need to discuss the whole read
thing anyways
---
 common/storage.h         |  4 ++--
 common/storage_kyoto.cpp | 16 +++++++++++-----
 common/storage_lmdb.cpp  | 18 ++++++++++--------
 tests/storagetest.cpp    | 23 ++++++++++++-----------
 4 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/common/storage.h b/common/storage.h
index f7dbd89..0b548fb 100644
--- a/common/storage.h
+++ b/common/storage.h
@@ -16,8 +16,8 @@ public:
     bool write(const char *key, size_t keySize, const char *value, size_t valueSize);
     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 &value)> &);
-    void read(const std::string &sKey, const std::function<void(void *ptr, int size)> &);
+    bool read(const std::string &sKey, const std::function<void(const std::string &value)> &);
+    bool read(const std::string &sKey, const std::function<void(void *ptr, int size)> &);
 
     qint64 diskUsage() const;
     void removeFromDisk() const;
diff --git a/common/storage_kyoto.cpp b/common/storage_kyoto.cpp
index 05942c2..40bd3e6 100644
--- a/common/storage_kyoto.cpp
+++ b/common/storage_kyoto.cpp
@@ -118,28 +118,34 @@ bool Storage::write(const std::string &sKey, const std::string &sValue)
     return success; 
 }
 
-void Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
+bool Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
 {
     if (!d->dbOpen) {
-        return;
+        return false;
     }
 
     std::string value;
     if (d->db.get(sKey, &value)) {
         resultHandler(value);
+        return true;
     }
+
+    return false;
 }
 
-void Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
+bool Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
 {
     if (!d->dbOpen) {
-        return;
+        return false;
     }
 
     size_t valueSize;
     char *valueBuffer = d->db.get(sKey.data(), sKey.size(), &valueSize);
-    resultHandler(valueBuffer, valueSize);
+    if (valueBuffer) {
+        resultHandler(valueBuffer, valueSize);
+    }
     delete[] valueBuffer;
+    return valueBuffer != nullptr;
 }
 
 qint64 Storage::diskUsage() const
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp
index 6c25448..18a5aa6 100644
--- a/common/storage_lmdb.cpp
+++ b/common/storage_lmdb.cpp
@@ -180,9 +180,9 @@ bool Storage::write(const std::string &sKey, const std::string &sValue)
     return !rc;
 }
 
-void Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
+bool Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
 {
-    read(sKey,
+    return read(sKey,
          [&](void *ptr, int size) {
             const std::string resultValue(static_cast<char*>(ptr), size);
             resultHandler(resultValue);
@@ -190,10 +190,10 @@ void Storage::read(const std::string &sKey, const std::function<void(const std::
 // std::cout << "key: " << resultKey << " data: " << resultValue << std::endl;
 }
 
-void Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
+bool Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
 {
     if (!d->env) {
-        return;
+        return false;
     }
 
     int rc;
@@ -208,14 +208,14 @@ void Storage::read(const std::string &sKey, const std::function<void(void *ptr,
     if (implicitTransaction) {
         // TODO: if this fails, still try the write below?
         if (!startTransaction(ReadOnly)) {
-            return;
+            return false;
         }
     }
 
     rc = mdb_cursor_open(d->transaction, d->dbi, &cursor);
     if (rc) {
         std::cerr << "mdb_cursor_get: " << rc << " " << mdb_strerror(rc) << std::endl;
-        return;
+        return false;
     }
 
     if (sKey.empty()) {
@@ -237,18 +237,20 @@ void Storage::read(const std::string &sKey, const std::function<void(void *ptr,
         }
     }
 
+    mdb_cursor_close(cursor);
+
     if (rc) {
         std::cerr << "mdb_cursor_get: " << rc << " " << mdb_strerror(rc) << std::endl;
+        return false
     }
 
-    mdb_cursor_close(cursor);
-
     /**
       we don't abort the transaction since we need it for reading the values
     if (implicitTransaction) {
         abortTransaction();
     }
     */
+    return true;
 }
 
 qint64 Storage::diskUsage() const
diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp
index 1b105af..2b2805d 100644
--- a/tests/storagetest.cpp
+++ b/tests/storagetest.cpp
@@ -6,7 +6,7 @@
 #include <QString>
 #include <QtConcurrent/QtConcurrentRun>
 
-#include "store/database.h"
+#include "common/storage.h"
 
 class StorageTest : public QObject
 {
@@ -19,31 +19,32 @@ private:
 
     void populate(int count)
     {
-        Database db(testDataPath, dbName);
+        Storage storage(testDataPath, dbName);
         for (int i = 0; i < count; i++) {
             //This should perhaps become an implementation detail of the db?
             if (i % 10000 == 0) {
                 if (i > 0) {
-                    db.commitTransaction();
+                    storage.commitTransaction();
                 }
-                db.startTransaction();
+                storage.startTransaction();
             }
-            db.write(keyPrefix + std::to_string(i), keyPrefix + std::to_string(i));
+            storage.write(keyPrefix + std::to_string(i), keyPrefix + std::to_string(i));
         }
-        db.commitTransaction();
+        storage.commitTransaction();
     }
 
-    bool verify(Database &db, int i)
+    bool verify(Storage &storage, int i)
     {
-        bool error = false;
+        bool success = true;
+        bool keyMatch = true;
         const auto reference = keyPrefix + std::to_string(i);
-        db.read(keyPrefix + std::to_string(i), [&error, &reference](const std::string &value) {
+        success = storage.read(keyPrefix + std::to_string(i), [&error, &reference](const std::string &value) {
             if (value != reference) {
                 qDebug() << "Mismatch while reading";
-                error = true;
+                keyMatch = false;
             }
         });
-        return !error;
+        return succes && keyMatch;
     }
 
 private Q_SLOTS:
-- 
cgit v1.2.3