From 4141b5e8e6296ca8ab94553e27257f8c2b107461 Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Sun, 23 Aug 2015 13:02:04 +0200 Subject: Less noise and better error handling. Trying to read from non-existant databases no longer prints error messages. --- common/genericresource.cpp | 5 +++-- common/messagequeue.cpp | 29 +++++++++++++++-------------- common/messagequeue.h | 3 +++ common/storage_lmdb.cpp | 41 +++++++++++++++++++++++++++-------------- tests/genericresourcetest.cpp | 3 --- tests/messagequeuetest.cpp | 2 +- tests/storagetest.cpp | 7 ++++++- 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 &future) { queue->dequeueBatch(100, [this](const QByteArray &data) { - Trace() << "Got value"; return processQueuedCommand(data); } ).then([&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 MessageQueue::dequeueBatch(int maxBatchSize, const std::function(const QByteArray &)> &resultHandler) { - Trace() << "Dequeue batch"; auto resultCount = QSharedPointer::create(0); return KAsync::start([this, maxBatchSize, resultHandler, resultCount](KAsync::Future &future) { int count = 0; @@ -123,14 +122,12 @@ KAsync::Job 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 MessageQueue::dequeueBatch(int maxBatchSize, const std::functi ::waitForCompletion(waitCondition).then([this, resultCount, &future]() { processRemovals(); if (*resultCount == 0) { - future.setError(-1, "No message found"); + future.setError(static_cast(ErrorCodes::NoMessageFound), "No message found"); future.setFinished(); } else { if (isEmpty()) { @@ -160,16 +157,20 @@ KAsync::Job 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 defaultErrorHandler; QString name; - bool openDatabase(std::function errorHandler) + bool openDatabase(bool readOnly, std::function 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 &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 &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 &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 &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 &resultHandler, const std::function &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; }, -- cgit v1.2.3