Less noise and better error handling.

Trying to read from non-existant databases no longer prints error
messages.

7 files changed, 55 insertions, 35 deletions

diff --git a/common/genericresource.cpp b/common/genericresource.cpp
index 112ce64..3ffc56b 100644
--- a/common/genericresource.cpp
+++ b/common/genericresource.cpp
@@ -107,14 +107,15 @@ private slots:
             [queue]() { return !queue->isEmpty(); },
             [this, queue](KAsync::Future<void> &future) {
                 queue->dequeueBatch(100, [this](const QByteArray &data) {
-                        Trace() << "Got value";
                         return processQueuedCommand(data);
                     }
                 ).then<void>([&future, queue](){
                     future.setFinished();
                 },
                 [&future](int i, QString error) {
-                    Warning() << "Error while getting message from messagequeue: " << error;
+                    if (i != MessageQueue::ErrorCodes::NoMessageFound) {
+                        Warning() << "Error while getting message from messagequeue: " << error;
+                    }
                     future.setFinished();
                 }).exec();
             }
diff --git a/common/messagequeue.cpp b/common/messagequeue.cpp
index ab4b1cf..f8bcd46 100644
--- a/common/messagequeue.cpp
+++ b/common/messagequeue.cpp
@@ -113,7 +113,6 @@ void MessageQueue::dequeue(const std::function<void(void *ptr, int size, std::fu
 
 KAsync::Job<void> MessageQueue::dequeueBatch(int maxBatchSize, const std::function<KAsync::Job<void>(const QByteArray &)> &resultHandler)
 {
-    Trace() << "Dequeue batch";
     auto resultCount = QSharedPointer<int>::create(0);
     return KAsync::start<void>([this, maxBatchSize, resultHandler, resultCount](KAsync::Future<void> &future) {
         int count = 0;
@@ -123,14 +122,12 @@ KAsync::Job<void> MessageQueue::dequeueBatch(int maxBatchSize, const std::functi
                 return true;
             }
             *resultCount += 1;
-            Trace() << "Dequeue value";
             //We need a copy of the key here, otherwise we can't store it in the lambda (the pointers will become invalid)
             mPendingRemoval << QByteArray(key.constData(), key.size());
 
             waitCondition << resultHandler(value).exec();
 
             count++;
-            Trace() << count << maxBatchSize;
             if (count < maxBatchSize) {
                 return true;
             }
@@ -145,7 +142,7 @@ KAsync::Job<void> MessageQueue::dequeueBatch(int maxBatchSize, const std::functi
         ::waitForCompletion(waitCondition).then<void>([this, resultCount, &future]() {
             processRemovals();
             if (*resultCount == 0) {
-                future.setError(-1, "No message found");
+                future.setError(static_cast<int>(ErrorCodes::NoMessageFound), "No message found");
                 future.setFinished();
             } else {
                 if (isEmpty()) {
@@ -160,16 +157,20 @@ KAsync::Job<void> MessageQueue::dequeueBatch(int maxBatchSize, const std::functi
 bool MessageQueue::isEmpty()
 {
     int count = 0;
-    mStorage.createTransaction(Akonadi2::Storage::ReadOnly).scan("", [&count, this](const QByteArray &key, const QByteArray &value) -> bool {
-        if (!Akonadi2::Storage::isInternalKey(key) && !mPendingRemoval.contains(key)) {
-            count++;
-            return false;
-        }
-        return true;
-    },
-    [](const Akonadi2::Storage::Error &error) {
-        ErrorMsg() << "Error while checking if empty" << error.message;
-    });
+    auto t = mStorage.createTransaction(Akonadi2::Storage::ReadOnly);
+    auto db = t.openDatabase();
+    if (db) {
+        db.scan("", [&count, this](const QByteArray &key, const QByteArray &value) -> bool {
+            if (!Akonadi2::Storage::isInternalKey(key) && !mPendingRemoval.contains(key)) {
+                count++;
+                return false;
+            }
+            return true;
+        },
+        [](const Akonadi2::Storage::Error &error) {
+            ErrorMsg() << "Error while checking if empty" << error.message;
+        });
+    }
     return count == 0;
 }
 
diff --git a/common/messagequeue.h b/common/messagequeue.h
index b6c2614..a04e22f 100644
--- a/common/messagequeue.h
+++ b/common/messagequeue.h
@@ -15,6 +15,9 @@ class MessageQueue : public QObject
 {
     Q_OBJECT
 public:
+    enum ErrorCodes {
+        NoMessageFound
+    };
     class Error
     {
     public:
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp
index ebb3be3..3e7c962 100644
--- a/common/storage_lmdb.cpp
+++ b/common/storage_lmdb.cpp
@@ -71,18 +71,23 @@ public:
     std::function<void(const Storage::Error &error)> defaultErrorHandler;
     QString name;
 
-    bool openDatabase(std::function<void(const Storage::Error &error)> errorHandler)
+    bool openDatabase(bool readOnly, std::function<void(const Storage::Error &error)> errorHandler)
     {
-        unsigned int flags = MDB_CREATE;
+        unsigned int flags = 0;
+        if (!readOnly) {
+            flags |= MDB_CREATE;
+        }
         if (allowDuplicates) {
             flags |= MDB_DUPSORT;
         }
         if (const int rc = mdb_dbi_open(transaction, db.constData(), flags, &dbi)) {
-            qWarning() << "Failed to open: " << rc << db;
             dbi = 0;
             transaction = 0;
-            Error error(name.toLatin1(), ErrorCodes::GenericError, "Error while opening database: " + QByteArray(mdb_strerror(rc)));
-            errorHandler ? errorHandler(error) : defaultErrorHandler(error);
+            //The database is not existing, ignore in read-only mode
+            if (!(readOnly && rc == MDB_NOTFOUND)) {
+                Error error(name.toLatin1(), ErrorCodes::GenericError, "Error while opening database: " + QByteArray(mdb_strerror(rc)));
+                errorHandler ? errorHandler(error) : defaultErrorHandler(error);
+            }
             return false;
         }
         return true;
@@ -108,7 +113,7 @@ Storage::NamedDatabase::~NamedDatabase()
 bool Storage::NamedDatabase::write(const QByteArray &sKey, const QByteArray &sValue, const std::function<void(const Storage::Error &error)> &errorHandler)
 {
     if (!d || !d->transaction) {
-        Error error(d->name.toLatin1(), ErrorCodes::GenericError, "Not open");
+        Error error("", ErrorCodes::GenericError, "Not open");
         errorHandler ? errorHandler(error) : d->defaultErrorHandler(error);
         return false;
     }
@@ -167,8 +172,7 @@ int Storage::NamedDatabase::scan(const QByteArray &k,
                   const std::function<void(const Storage::Error &error)> &errorHandler) const
 {
     if (!d || !d->transaction) {
-        // Error error(d->name.toLatin1(), ErrorCodes::NotOpen, "Not open");
-        // errorHandler ? errorHandler(error) : d->defaultErrorHandler(error);
+        //Not an error. We rely on this to read nothing from non-existing databases.
         return 0;
     }
 
@@ -328,16 +332,20 @@ Storage::NamedDatabase Storage::Transaction::openDatabase(const QByteArray &db,
     //We don't now if anything changed
     d->implicitCommit = true;
     auto p = new Storage::NamedDatabase::Private(db, d->allowDuplicates, d->defaultErrorHandler, d->name, d->transaction);
-    p->openDatabase(errorHandler);
+    if (!p->openDatabase(d->requestedRead, errorHandler)) {
+        return Storage::NamedDatabase();
+        delete p;
+    }
     return Storage::NamedDatabase(p);
 }
 
 bool Storage::Transaction::write(const QByteArray &key, const QByteArray &value, const std::function<void(const Storage::Error &error)> &errorHandler)
 {
-    openDatabase().write(key, value, [this, errorHandler](const Storage::Error &error) {
+    auto eHandler = [this, errorHandler](const Storage::Error &error) {
         d->error = true;
         errorHandler ? errorHandler(error) : d->defaultErrorHandler(error);
-    });
+    };
+    openDatabase("default", eHandler).write(key, value, eHandler);
     d->implicitCommit = true;
 
     return !d->error;
@@ -346,10 +354,11 @@ bool Storage::Transaction::write(const QByteArray &key, const QByteArray &value,
 void Storage::Transaction::remove(const QByteArray &k,
                      const std::function<void(const Storage::Error &error)> &errorHandler)
 {
-    openDatabase().remove(k, [this, errorHandler](const Storage::Error &error) {
+    auto eHandler = [this, errorHandler](const Storage::Error &error) {
         d->error = true;
         errorHandler ? errorHandler(error) : d->defaultErrorHandler(error);
-    });
+    };
+    openDatabase("default", eHandler).remove(k, eHandler);
     d->implicitCommit = true;
 }
 
@@ -357,7 +366,11 @@ int Storage::Transaction::scan(const QByteArray &k,
                   const std::function<bool(const QByteArray &key, const QByteArray &value)> &resultHandler,
                   const std::function<void(const Storage::Error &error)> &errorHandler) const
 {
-    return openDatabase().scan(k, resultHandler, errorHandler);
+    auto db = openDatabase("default");
+    if (db) {
+        return db.scan(k, resultHandler, errorHandler);
+    }
+    return 0;
 }
 
 
diff --git a/tests/genericresourcetest.cpp b/tests/genericresourcetest.cpp
index 0c5db15..fa6da22 100644
--- a/tests/genericresourcetest.cpp
+++ b/tests/genericresourcetest.cpp
@@ -44,9 +44,6 @@ private Q_SLOTS:
         removeFromDisk("org.kde.test.instance1.userqueue");
         removeFromDisk("org.kde.test.instance1.synchronizerqueue");
         Akonadi2::Log::setDebugOutputLevel(Akonadi2::Log::Trace);
-        qDebug();
-        qDebug() << "-----------------------------------------";
-        qDebug();
     }
 
     void testProcessCommand()
diff --git a/tests/messagequeuetest.cpp b/tests/messagequeuetest.cpp
index 9c2aa16..22ce161 100644
--- a/tests/messagequeuetest.cpp
+++ b/tests/messagequeuetest.cpp
@@ -49,7 +49,7 @@ private Q_SLOTS:
             gotError = true;
         });
         QVERIFY(!gotValue);
-        QVERIFY(gotError);
+        QVERIFY(!gotError);
     }
 
     void testEnqueue()
diff --git a/tests/storagetest.cpp b/tests/storagetest.cpp
index fe80bb7..e872c44 100644
--- a/tests/storagetest.cpp
+++ b/tests/storagetest.cpp
@@ -159,7 +159,12 @@ private Q_SLOTS:
         bool gotResult = false;
         bool gotError = false;
         Akonadi2::Storage store(testDataPath, dbName, Akonadi2::Storage::ReadWrite);
-        int numValues = store.createTransaction(Akonadi2::Storage::ReadOnly).scan("", [&](const QByteArray &key, const QByteArray &value) -> bool {
+        auto transaction = store.createTransaction(Akonadi2::Storage::ReadOnly);
+        auto db = transaction.openDatabase("default", [&](const Akonadi2::Storage::Error &error) {
+            qDebug() << error.message;
+            gotError = true;
+        });
+        int numValues = db.scan("", [&](const QByteArray &key, const QByteArray &value) -> bool {
             gotResult = true;
             return false;
         },