fix(history): handle errors during db upgrade

qTox · Oct 20, 2019 · f72f3f7 · f72f3f7
1 parent dacfcda
commit f72f3f7
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 17 deletions.
diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp
@@ -28,7 +28,7 @@
 namespace {
 static constexpr int SCHEMA_VERSION = 1;
 
-bool createCurrentSchema(std::shared_ptr<RawDatabase> db)
+bool createCurrentSchema(RawDatabase& db)
 {
     QVector<RawDatabase::Query> queries;
     queries += RawDatabase::Query(QStringLiteral(
@@ -63,23 +63,25 @@ bool createCurrentSchema(std::shared_ptr<RawDatabase> db)
         "file_state INTEGER NOT NULL);"
         "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY);"));
     queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = %1;").arg(SCHEMA_VERSION));
-    return db->execNow(queries);
+    return db.execNow(queries);
 }
 
-bool isNewDb(std::shared_ptr<RawDatabase> db)
+bool isNewDb(std::shared_ptr<RawDatabase>& db, bool& success)
 {
     bool newDb;
     if (!db->execNow(RawDatabase::Query("SELECT COUNT(*) FROM sqlite_master;",
                                         [&](const QVector<QVariant>& row) {
                                             newDb = row[0].toLongLong() == 0;
                                         }))) {
         db.reset();
-        return false; // TODO: propogate error
+        success = false;
+        return false;
     }
+    success = true;
     return newDb;
 }
 
-bool dbSchema0to1(std::shared_ptr<RawDatabase> db)
+bool dbSchema0to1(RawDatabase& db)
 {
     QVector<RawDatabase::Query> queries;
     queries +=
@@ -97,15 +99,15 @@ bool dbSchema0to1(std::shared_ptr<RawDatabase> db)
     queries +=
         RawDatabase::Query(QStringLiteral("ALTER TABLE history ADD file_id INTEGER;"));
     queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 1;"));
-    return db->execNow(queries);
+    return db.execNow(queries);
 }
 
 /**
 * @brief Upgrade the db schema
 * @note On future alterations of the database all you have to do is bump the SCHEMA_VERSION
 * variable and add another case to the switch statement below. Make sure to fall through on each case.
 */
-void dbSchemaUpgrade(std::shared_ptr<RawDatabase> db)
+void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
 {
     int64_t databaseSchemaVersion;
 
@@ -127,22 +129,36 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase> db)
         return;
     }
 
-    QVector<RawDatabase::Query> queries;
-    // Make sure to handle the un-created case as well in the following upgrade code
     switch (databaseSchemaVersion) {
-    case 0:
+    case 0: {
         // Note: 0 is a special version that is actually two versions.
         //   possibility 1) it is a newly created database and it neesds the current schema to be created.
         //   possibility 2) it is a old existing database, before version 1 and before we saved schema version,
         //       and needs to be updated.
-        if (isNewDb(db)) {
-            createCurrentSchema(db);
+        bool success = false;
+        const bool newDb = isNewDb(db, success);
+        if (!success) {
+            qCritical() << "Failed to create current db schema";
+            db.reset();
+            return;
+        }
+        if (newDb) {
+            if (!createCurrentSchema(*db)) {
+                qCritical() << "Failed to create current db schema";
+                db.reset();
+                return;
+            }
             qDebug() << "Database created at schema version" << SCHEMA_VERSION;
             break; // new db is the only case where we don't incrementally upgrade through each version
         } else {
-            dbSchema0to1(db);
+            if (!dbSchema0to1(*db)) {
+                qCritical() << "Failed to upgrade db to schema version 1, aborting";
+                db.reset();
+                return;
+            }
             qDebug() << "Database upgraded incrementally to schema version 1";
         }
+    }
         // fallthrough
     // case 1:
     //    dbSchema1to2(queries);

diff --git a/test/persistence/dbschema_test.cpp b/test/persistence/dbschema_test.cpp
@@ -114,24 +114,29 @@ void TestDbSchema::testCreation()
 {
     QVector<RawDatabase::Query> queries;
     auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testCreation.db", {}, {}}};
-    QVERIFY(createCurrentSchema(db));
+    QVERIFY(createCurrentSchema(*db));
     verifyDb(db, schema1);
 }
 
 void TestDbSchema::testIsNewDb()
 {
     auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testIsNewDbTrue.db", {}, {}}};
-    QVERIFY(isNewDb(db) == true);
+    bool success = false;
+    bool newDb = isNewDb(db, success);
+    QVERIFY(success);
+    QVERIFY(newDb == true);
     db = std::shared_ptr<RawDatabase>{new RawDatabase{"testIsNewDbFalse.db", {}, {}}};
     createSchemaAtVersion(db, schema0);
-    QVERIFY(isNewDb(db) == false);
+    newDb = isNewDb(db, success);
+    QVERIFY(success);
+    QVERIFY(newDb == false);
 }
 
 void TestDbSchema::test0to1()
 {
     auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"test0to1.db", {}, {}}};
     createSchemaAtVersion(db, schema0);
-    QVERIFY(dbSchema0to1(db));
+    QVERIFY(dbSchema0to1(*db));
     verifyDb(db, schema1);
 }