read takes an error handler rather than returns a bool

7 files changed, 79 insertions, 34 deletions

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 9b3f777..68330ce 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -13,6 +13,7 @@ endif (STORAGE_KYOTO)
 set(command_SRCS
    commands.cpp
    console.cpp
+   storage_common.cpp
    ${storage_SRCS})
 
 add_library(${PROJECT_NAME} ${command_SRCS})
diff --git a/common/storage.h b/common/storage.h
index 846e15b..e5e592c 100644
--- a/common/storage.h
+++ b/common/storage.h
@@ -7,6 +7,16 @@ class Storage {
 public:
     enum AccessMode { ReadOnly, ReadWrite };
 
+    class Error
+    {
+    public:
+        Error(const std::string &s, int c, const std::string &m)
+            : store(s), message(m), code(c) {}
+        std::string store;
+        std::string message;
+        int code;
+    };
+
     Storage(const QString &storageRoot, const QString &name, AccessMode mode = ReadOnly);
     ~Storage();
     bool isInTransaction() const;
@@ -15,9 +25,15 @@ public:
     void abortTransaction();
     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
-    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)> &);
+    void read(const std::string &sKey,
+              const std::function<bool(const std::string &value)> &resultHandler);
+    void read(const std::string &sKey,
+              const std::function<bool(const std::string &value)> &resultHandler,
+              const std::function<void(const Storage::Error &error)> &errors);
+    void read(const std::string &sKey, const std::function<bool(void *ptr, int size)> &resultHandler);
+    void read(const std::string &sKey,
+              const std::function<bool(void *ptr, int size)> & resultHandler,
+              const std::function<void(const Storage::Error &error)> &errorHandler);
 
     qint64 diskUsage() const;
     void removeFromDisk() const;
diff --git a/common/storage_kyoto.cpp b/common/storage_kyoto.cpp
index 3d7ab8c..539ada1 100644
--- a/common/storage_kyoto.cpp
+++ b/common/storage_kyoto.cpp
@@ -18,14 +18,16 @@ public:
     Private(const QString &storageRoot, const QString &name, AccessMode m);
     ~Private();
 
+    QString name;
     kyotocabinet::TreeDB db;
     AccessMode mode;
     bool dbOpen;
     bool inTransaction;
 };
 
-Storage::Private::Private(const QString &storageRoot, const QString &name, AccessMode m)
-    : mode(m),
+Storage::Private::Private(const QString &storageRoot, const QString &n, AccessMode m)
+    : name(n),
+      mode(m),
       dbOpen(false),
       inTransaction(false)
 {
@@ -129,34 +131,45 @@ bool Storage::write(const std::string &sKey, const std::string &sValue)
     return success; 
 }
 
-bool Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
+void Storage::read(const std::string &sKey,
+                   const std::function<bool(const std::string &value)> &resultHandler,
+                   const std::function<void(const Storage::Error &error)> &errorHandler)
 {
     if (!d->dbOpen) {
-        return false;
+        Error error(d->name.toStdString(), -1, "Not open");
+        errorHandler(error);
+        return;
     }
 
     std::string value;
     if (d->db.get(sKey, &value)) {
         resultHandler(value);
-        return true;
+        return;
     }
 
-    return false;
+    Error error(d->name.toStdString(), d->db.error().code(), d->db.error().message());
+    errorHandler(error);
 }
 
-bool Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
+void Storage::read(const std::string &sKey,
+                   const std::function<bool(void *ptr, int size)> &resultHandler,
+                   const std::function<void(const Storage::Error &error)> &errorHandler)
 {
     if (!d->dbOpen) {
-        return false;
+        Error error(d->name.toStdString(), -1, "Not open");
+        errorHandler(error);
+        return;
     }
 
     size_t valueSize;
     char *valueBuffer = d->db.get(sKey.data(), sKey.size(), &valueSize);
     if (valueBuffer) {
         resultHandler(valueBuffer, valueSize);
+    } else {
+        Error error(d->name.toStdString(), d->db.error().code(), d->db.error().message());
+        errorHandler(error);
     }
     delete[] valueBuffer;
-    return valueBuffer != nullptr;
 }
 
 qint64 Storage::diskUsage() const
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp
index 7e23fd3..b7102aa 100644
--- a/common/storage_lmdb.cpp
+++ b/common/storage_lmdb.cpp
@@ -195,20 +195,26 @@ bool Storage::write(const std::string &sKey, const std::string &sValue)
     return !rc;
 }
 
-bool Storage::read(const std::string &sKey, const std::function<void(const std::string &value)> &resultHandler)
+void Storage::read(const std::string &sKey,
+                   const std::function<bool(const std::string &value)> &resultHandler,
+                   const std::function<void(const Storage::Error &error)> &errorHandler)
 {
-    return read(sKey,
-         [&](void *ptr, int size) {
+    read(sKey,
+         [&](void *ptr, int size) -> bool {
             const std::string resultValue(static_cast<char*>(ptr), size);
-            resultHandler(resultValue);
-         });
+            return resultHandler(resultValue);
+         }, errorHandler);
 // std::cout << "key: " << resultKey << " data: " << resultValue << std::endl;
 }
 
-bool Storage::read(const std::string &sKey, const std::function<void(void *ptr, int size)> &resultHandler)
+void Storage::read(const std::string &sKey,
+                   const std::function<bool(void *ptr, int size)> &resultHandler,
+                   const std::function<void(const Storage::Error &error)> &errorHandler)
 {
     if (!d->env) {
-        return false;
+        Error error(d->name.toStdString(), -1, "Not open");
+        errorHandler(error);
+        return;
     }
 
     int rc;
@@ -223,21 +229,26 @@ bool 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 false;
+            Error error(d->name.toStdString(), -2, "Could not start transaction");
+            errorHandler(error);
+            return;
         }
     }
 
     rc = mdb_cursor_open(d->transaction, d->dbi, &cursor);
     if (rc) {
-        std::cerr << "mdb_cursor_get: " << rc << " " << mdb_strerror(rc) << std::endl;
-        return false;
+        Error error(d->name.toStdString(), rc, mdb_strerror(rc));
+        errorHandler(error);
+        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(key.mv_data, data.mv_size);
+            if (!resultHandler(key.mv_data, data.mv_size)) {
+                break;
+            }
         }
 
         //We never find the last value
@@ -255,8 +266,8 @@ bool 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;
+        Error error(d->name.toStdString(), rc, mdb_strerror(rc));
+        errorHandler(error);
     }
 
     /**
@@ -265,7 +276,6 @@ bool Storage::read(const std::string &sKey, const std::function<void(void *ptr,
         abortTransaction();
     }
     */
-    return true;
 }
 
 qint64 Storage::diskUsage() const
diff --git a/dummyresource/facade.cpp b/dummyresource/facade.cpp
index 9758c1b..bfe1de4 100644
--- a/dummyresource/facade.cpp
+++ b/dummyresource/facade.cpp
@@ -77,13 +77,14 @@ void DummyResourceFacade::load(const Akonadi2::Query &query, const std::function
     qDebug() << "load called";
     //TODO only read values matching the query
     auto storage = QSharedPointer<Storage>::create(Akonadi2::Store::storageLocation(), "dummyresource");
-    storage->read("", [resultCallback, storage](void *data, int size) {
+    storage->read("", [resultCallback, storage](void *data, int size) -> bool {
         //TODO read second buffer as well
         auto eventBuffer = GetDummyEvent(data);
         auto event = QSharedPointer<DummyEventAdaptor>::create("dummyresource", "key", 0);
         event->buffer = eventBuffer;
         event->storage = storage;
         resultCallback(event);
+        return true;
     });
 }
 
diff --git a/tests/storagebenchmark.cpp b/tests/storagebenchmark.cpp
index 6010539..9c95d3b 100644
--- a/tests/storagebenchmark.cpp
+++ b/tests/storagebenchmark.cpp
@@ -118,7 +118,7 @@ private Q_SLOTS:
         {
             for (int i = 0; i < count; i++) {
                 if (store) {
-                    store->read(keyPrefix + std::to_string(i), [](std::string value){});
+                    store->read(keyPrefix + std::to_string(i), [](std::string value) -> bool { return true; });
                 }
             }
         }
diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp
index 3981c4b..1974355 100644
--- a/tests/storagetest.cpp
+++ b/tests/storagetest.cpp
@@ -38,12 +38,16 @@ private:
         bool success = true;
         bool keyMatch = true;
         const auto reference = keyPrefix + std::to_string(i);
-        success = storage.read(keyPrefix + std::to_string(i), [&keyMatch, &reference](const std::string &value) {
-            if (value != reference) {
-                qDebug() << "Mismatch while reading";
-                keyMatch = false;
-            }
-        });
+        storage.read(keyPrefix + std::to_string(i),
+            [&keyMatch, &reference](const std::string &value) -> bool {
+                if (value != reference) {
+                    qDebug() << "Mismatch while reading";
+                    keyMatch = false;
+                }
+                return keyMatch;
+            },
+            [&success](const Storage::Error &) { success = false; }
+            );
         return success && keyMatch;
     }