From 80afd7070f2d8e57cab2fe55fef611623fdb75f0 Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Mon, 26 Mar 2018 15:05:41 +0200
Subject: Make sure we initialize the environment correctly and only set the db
 version when creating it

---
 common/pipeline.cpp            |  2 +-
 common/storage.h               |  1 +
 common/storage/entitystore.cpp | 18 +++++++++++++++---
 common/storage/entitystore.h   |  2 +-
 common/storage_lmdb.cpp        | 13 +++++++++++--
 5 files changed, 29 insertions(+), 7 deletions(-)

(limited to 'common')

diff --git a/common/pipeline.cpp b/common/pipeline.cpp
index ee9d3af..4afe9f3 100644
--- a/common/pipeline.cpp
+++ b/common/pipeline.cpp
@@ -63,7 +63,7 @@ public:
 Pipeline::Pipeline(const ResourceContext &context, const Sink::Log::Context &ctx) : QObject(nullptr), d(new Private(context, ctx))
 {
     //Create main store immediately on first start
-    d->entityStore.createIfMissing();
+    d->entityStore.initialize();
 }
 
 Pipeline::~Pipeline()
diff --git a/common/storage.h b/common/storage.h
index 5a4fb45..2451040 100644
--- a/common/storage.h
+++ b/common/storage.h
@@ -222,6 +222,7 @@ public:
     static void getUids(const QByteArray &type, const Transaction &, const std::function<void(const QByteArray &uid)> &);
 
     bool exists() const;
+    static bool exists(const QString &storageRoot, const QString &name);
 
     static bool isInternalKey(const char *key);
     static bool isInternalKey(void *key, int keySize);
diff --git a/common/storage/entitystore.cpp b/common/storage/entitystore.cpp
index 6021344..d5a7c5f 100644
--- a/common/storage/entitystore.cpp
+++ b/common/storage/entitystore.cpp
@@ -158,13 +158,23 @@ EntityStore::EntityStore(const ResourceContext &context, const Log::Context &ctx
 
 }
 
-void EntityStore::createIfMissing()
+void EntityStore::initialize()
 {
-    if (!d->exists()) {
+    //This function is only called in the resource code where we want to be able to write to the databse.
+
+    //Check for the existience of the db without creating it or the envrionment.
+    //This is required to be able to set the database version only in the case where we create a new database.
+    if (!Storage::DataStore::exists(Sink::storageLocation(), d->resourceContext.instanceId())) {
+        //The first time we open the environment we always want it to be read/write. Otherwise subsequent tries to open a write transaction will fail.
         startTransaction(Sink::Storage::DataStore::ReadWrite);
+        //Create the database with the correct version if it wasn't existing before
+        SinkLogCtx(d->logCtx) << "Creating resource database.";
         Storage::DataStore::setDatabaseVersion(d->transaction, Sink::latestDatabaseVersion());
-        commitTransaction();
+    } else {
+        //The first time we open the environment we always want it to be read/write. Otherwise subsequent tries to open a write transaction will fail.
+        startTransaction(Sink::Storage::DataStore::ReadWrite);
     }
+    commitTransaction();
 }
 
 void EntityStore::startTransaction(Sink::Storage::DataStore::AccessMode accessMode)
@@ -383,9 +393,11 @@ void EntityStore::cleanupEntityRevisionsUntil(qint64 revision)
 
 bool EntityStore::cleanupRevisions(qint64 revision)
 {
+    Q_ASSERT(d->exists());
     bool implicitTransaction = false;
     if (!d->transaction) {
         startTransaction(Sink::Storage::DataStore::ReadWrite);
+        Q_ASSERT(d->transaction);
         implicitTransaction = true;
     }
     const auto lastCleanRevision = DataStore::cleanedUpRevision(d->transaction);
diff --git a/common/storage/entitystore.h b/common/storage/entitystore.h
index 985e7f9..003a2ca 100644
--- a/common/storage/entitystore.h
+++ b/common/storage/entitystore.h
@@ -39,7 +39,7 @@ public:
     EntityStore(const ResourceContext &resourceContext, const Sink::Log::Context &);
     ~EntityStore() = default;
 
-    void createIfMissing();
+    void initialize();
 
     //Only the pipeline may call the following functions outside of tests
     bool add(const QByteArray &type, ApplicationDomain::ApplicationDomainType newEntity, bool replayToSource);
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp
index 9d0bd6b..49f30b5 100644
--- a/common/storage_lmdb.cpp
+++ b/common/storage_lmdb.cpp
@@ -566,6 +566,11 @@ public:
         const int rc = mdb_txn_begin(env, NULL, requestedRead ? MDB_RDONLY : 0, &transaction);
         // Trace_area("storage." + name.toLatin1()) << "Started transaction " << mdb_txn_id(transaction) << transaction;
         if (rc) {
+            unsigned int flags;
+            mdb_env_get_flags(env, &flags);
+            if (flags & MDB_RDONLY && !requestedRead) {
+                SinkError() << "Tried to open a write transation in a read-only enironment";
+            }
             defaultErrorHandler(Error(name.toLatin1(), ErrorCodes::GenericError, "Error while opening transaction: " + QByteArray(mdb_strerror(rc))));
         }
     }
@@ -916,10 +921,14 @@ DataStore::~DataStore()
     delete d;
 }
 
+bool DataStore::exists(const QString &storageRoot, const QString &name)
+{
+    return QFileInfo(storageRoot + '/' + name + "/data.mdb").exists();
+}
+
 bool DataStore::exists() const
 {
-    QFileInfo info(d->storageRoot + '/' + d->name + "/data.mdb");
-    return (d->env != 0) && info.exists();
+    return (d->env != 0) && DataStore::exists(d->storageRoot, d->name);
 }
 
 DataStore::Transaction DataStore::createTransaction(AccessMode type, const std::function<void(const DataStore::Error &error)> &errorHandlerArg)
-- 
cgit v1.2.3