test(db): add db schema upgrade test

cmake/Testing.cmake

@@ -26,6 +26,7 @@ auto_test(chatlog textformatter)
 auto_test(net toxmedata)
 auto_test(net bsu)
 auto_test(persistence paths)
+auto_test(persistence dbschema)
 
 if (UNIX)
   auto_test(platform posixsignalnotifier)

src/persistence/history.cpp

@@ -25,6 +25,133 @@
 #include "settings.h"
 #include "db/rawdatabase.h"
 
+namespace {
+static constexpr int SCHEMA_VERSION = 1;
+
+void generateCurrentSchema(QVector<RawDatabase::Query>& queries)
+{
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE peers (id INTEGER PRIMARY KEY, "
+        "public_key TEXT NOT NULL UNIQUE);"
+        "CREATE TABLE aliases (id INTEGER PRIMARY KEY, "
+        "owner INTEGER, "
+        "display_name BLOB NOT NULL, "
+        "UNIQUE(owner, display_name));"
+        "CREATE TABLE history "
+        "(id INTEGER PRIMARY KEY, "
+        "timestamp INTEGER NOT NULL, "
+        "chat_id INTEGER NOT NULL, "
+        "sender_alias INTEGER NOT NULL, "
+        // even though technically a message can be null for file transfer, we've opted
+        // to just insert an empty string when there's no content, this moderately simplifies
+        // implementating to leakon as currently our database doesn't have support for optional
+        // fields. We would either have to insert "?" or "null" based on if message exists and then
+        // ensure that our blob vector always has the right number of fields. Better to just
+        // leave this as NOT NULL for now.
+        "message BLOB NOT NULL, "
+        "file_id INTEGER);"
+        "CREATE TABLE file_transfers "
+        "(id INTEGER PRIMARY KEY, "
+        "chat_id INTEGER NOT NULL, "
+        "file_restart_id BLOB NOT NULL, "
+        "file_name BLOB NOT NULL, "
+        "file_path BLOB NOT NULL, "
+        "file_hash BLOB NOT NULL, "
+        "file_size INTEGER NOT NULL, "
+        "direction INTEGER NOT NULL, "
+        "file_state INTEGER NOT NULL);"
+        "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY);"));
+}
+
+bool isNewDb(std::shared_ptr<RawDatabase> db)
+{
+    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
+    }
+    return newDb;
+}
+
+void dbSchema0to1(std::shared_ptr<RawDatabase> db, QVector<RawDatabase::Query>& queries)
+{
+    queries +=
+        RawDatabase::Query(QStringLiteral(
+            "CREATE TABLE file_transfers "
+            "(id INTEGER PRIMARY KEY, "
+            "chat_id INTEGER NOT NULL, "
+            "file_restart_id BLOB NOT NULL, "
+            "file_name BLOB NOT NULL, "
+            "file_path BLOB NOT NULL, "
+            "file_hash BLOB NOT NULL, "
+            "file_size INTEGER NOT NULL, "
+            "direction INTEGER NOT NULL, "
+            "file_state INTEGER NOT NULL);"));
+    queries +=
+        RawDatabase::Query(QStringLiteral("ALTER TABLE history ADD file_id INTEGER;"));
+}
+
+/**
+* @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)
+{
+    int64_t databaseSchemaVersion;
+
+    if (!db->execNow(RawDatabase::Query("PRAGMA user_version", [&](const QVector<QVariant>& row) {
+            databaseSchemaVersion = row[0].toLongLong();
+        }))) {
+        qCritical() << "History failed to read user_version";
+        db.reset();
+        return;
+    }
+
+    if (databaseSchemaVersion > SCHEMA_VERSION) {
+        qWarning() << "Database version is newer than we currently support. Please upgrade qTox";
+        // We don't know what future versions have done, we have to disable db access until we re-upgrade
+        db.reset();
+        return;
+    } else if (databaseSchemaVersion == SCHEMA_VERSION) {
+        // No work to do
+        return;
+    }
+
+    QVector<RawDatabase::Query> queries;
+    // Make sure to handle the un-created case as well in the following upgrade code
+    switch (databaseSchemaVersion) {
+    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 need to be updated.
+        if (isNewDb(db)) {
+            generateCurrentSchema(queries);
+            queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = %1;").arg(SCHEMA_VERSION));
+            db->execLater(queries);
+            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, queries);
+        }
+        // fallthrough
+    // case 1:
+    //    dbSchema1to2(queries);
+    //    //fallthrough
+    // etc.
+    default:
+        queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = %1;").arg(SCHEMA_VERSION));
+        db->execLater(queries);
+        qDebug() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion
+                << "->" << SCHEMA_VERSION << ")";
+    }
+}
+} // namespace
+
 /**
  * @class History
  * @brief Interacts with the profile database to save the chat history.
@@ -36,7 +163,6 @@
 
 static constexpr int NUM_MESSAGES_DEFAULT =
     100; // arbitrary number of messages loaded when not loading by date
-static constexpr int SCHEMA_VERSION = 1;
 
 FileDbInsertionData::FileDbInsertionData()
 {
@@ -56,7 +182,7 @@ History::History(std::shared_ptr<RawDatabase> db)
         return;
     }
 
-    dbSchemaUpgrade();
+    dbSchemaUpgrade(db);
 
     // dbSchemaUpgrade may have put us in an invalid state
     if (!isValid()) {
@@ -664,117 +790,3 @@ QList<History::HistMessage> History::getChatHistory(const QString& friendPk, con
 
     return messages;
 }
-
-/**
- * @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 History::dbSchemaUpgrade()
-{
-    int64_t databaseSchemaVersion;
-
-    if (!db->execNow(RawDatabase::Query("PRAGMA user_version", [&](const QVector<QVariant>& row) {
-            databaseSchemaVersion = row[0].toLongLong();
-        }))) {
-        qCritical() << "History failed to read user_version";
-        db.reset();
-        return;
-    }
-
-    if (databaseSchemaVersion > SCHEMA_VERSION) {
-        qWarning() << "Database version is newer than we currently support. Please upgrade qTox";
-        // We don't know what future versions have done, we have to disable db access until we re-upgrade
-        db.reset();
-        return;
-    } else if (databaseSchemaVersion == SCHEMA_VERSION) {
-        // No work to do
-        return;
-    }
-
-    QVector<RawDatabase::Query> queries;
-    // Make sure to handle the un-created case as well in the following upgrade code
-    switch (databaseSchemaVersion) {
-    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 need to be updated.
-        bool newDb;
-        if (!db->execNow(RawDatabase::Query("SELECT COUNT(*) FROM sqlite_master;",
-                                            [&](const QVector<QVariant>& row) {
-                                                newDb = row[0].toLongLong() == 0;
-                                            }))) {
-            db.reset();
-            return;
-        }
-        if (newDb) {
-            createCurrentSchema();
-            break; // new db is the only case where we don't incrementally upgrade through each version
-        } else {
-            // it's a db that exists but at the first versioned schema, version 0.
-            queries +=
-                RawDatabase::Query(QStringLiteral(
-                    "CREATE TABLE file_transfers "
-                    "(id INTEGER PRIMARY KEY,"
-                    "chat_id INTEGER NOT NULL,"
-                    "file_restart_id BLOB NOT NULL,"
-                    "file_name BLOB NOT NULL, "
-                    "file_path BLOB NOT NULL,"
-                    "file_hash BLOB NOT NULL,"
-                    "file_size INTEGER NOT NULL,"
-                    "direction INTEGER NOT NULL,"
-                    "file_state INTEGER NOT NULL);"));
-            queries +=
-                RawDatabase::Query(QStringLiteral("ALTER TABLE history ADD file_id INTEGER;"));
-        }
-    }
-        // fallthrough
-    // case 1:
-    //    do 1 -> 2 upgrade
-    //    //fallthrough
-    // etc.
-    default:
-        queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = %1;").arg(SCHEMA_VERSION));
-        db->execLater(queries);
-        qDebug() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion
-                 << "->" << SCHEMA_VERSION << ")";
-    }
-}
-
-void History::createCurrentSchema()
-{
-    QVector<RawDatabase::Query> queries;
-    queries += RawDatabase::Query(QStringLiteral(
-        "CREATE TABLE peers (id INTEGER PRIMARY KEY, public_key TEXT NOT NULL "
-        "UNIQUE);"
-        "CREATE TABLE aliases (id INTEGER PRIMARY KEY, owner INTEGER,"
-        "display_name BLOB NOT NULL, UNIQUE(owner, display_name));"
-        "CREATE TABLE history "
-        "(id INTEGER PRIMARY KEY,"
-        "timestamp INTEGER NOT NULL,"
-        "chat_id INTEGER NOT NULL,"
-        "sender_alias INTEGER NOT NULL,"
-        // even though technically a message can be null for file transfer, we've opted
-        // to just insert an empty string when there's no content, this moderately simplifies
-        // implementating to leakon as currently our database doesn't have support for optional
-        // fields. We would either have to insert "?" or "null" based on if message exists and then
-        // ensure that our blob vector always has the right number of fields. Better to just
-        // leave this as NOT NULL for now.
-        "message BLOB NOT NULL,"
-        "file_id INTEGER);"
-        "CREATE TABLE file_transfers "
-        "(id INTEGER PRIMARY KEY,"
-        "chat_id INTEGER NOT NULL,"
-        "file_restart_id BLOB NOT NULL,"
-        "file_name BLOB NOT NULL, "
-        "file_path BLOB NOT NULL,"
-        "file_hash BLOB NOT NULL,"
-        "file_size INTEGER NOT NULL,"
-        "direction INTEGER NOT NULL,"
-        "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));
-    db->execLater(queries);
-    qDebug() << "Database created at schema version" << SCHEMA_VERSION;
-}

src/persistence/history.h

@@ -199,9 +199,6 @@ private:
 
     static RawDatabase::Query generateFileFinished(RowId fileId, bool success,
                                                    const QString& filePath, const QByteArray& fileHash);
-    void dbSchemaUpgrade();
-    void createCurrentSchema();
-
     std::shared_ptr<RawDatabase> db;
 
 

test/persistence/dbschema_test.cpp

@@ -0,0 +1,146 @@
+/*
+    Copyright © 2019 by The qTox Project Contributors
+
+    This file is part of qTox, a Qt-based graphical interface for Tox.
+
+    qTox is libre software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    qTox is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with qTox.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "src/persistence/db/rawdatabase.h"
+// normally we should only test public API instead of implementation,  but there's no reason to expose db schema
+// upgrade externally, and the complexity of each version upgrade benefits from being individually testable
+#include "src/persistence/history.cpp"
+
+#include <QtTest/QtTest>
+#include <QString>
+
+#include <memory>
+
+class TestDbSchema : public QObject
+{
+    Q_OBJECT
+private slots:
+    void initTestCase();
+    void testCreation();
+    void testIsNewDb();
+    void test0to1();
+    void cleanupTestCase();
+private:
+    bool initSucess{false};
+    void verifyDb(std::shared_ptr<RawDatabase> db, const QMap<QString, QString>& expectedSql);
+};
+
+const QString testFileList[] = {
+    "testCreation.db",
+    "testIsNewDbTrue.db",
+    "testIsNewDbFalse.db",
+    "test0to1.db"
+};
+
+void TestDbSchema::initTestCase()
+{
+    for (const auto& path : testFileList) {
+        QVERIFY(!QFileInfo{path}.exists());
+    }
+    initSucess = true;
+}
+
+void TestDbSchema::cleanupTestCase()
+{
+    if (!initSucess) {
+        qWarning() << "init failed, skipping cleanup to avoid loss of data";
+        return;
+    }
+    for (const auto& path : testFileList) {
+        QFile::remove(path);
+    }
+}
+
+void TestDbSchema::verifyDb(std::shared_ptr<RawDatabase> db, const QMap<QString, QString>& expectedSql)
+{
+    QVERIFY(db->execNow(RawDatabase::Query(QStringLiteral("SELECT name, sql FROM sqlite_master "
+        "WHERE type='table' "
+        "ORDER BY name;"),
+        [&](const QVector<QVariant>& row) {
+            const QString tableName = row[0].toString();
+            const QString tableSql = row[1].toString();
+            QVERIFY(expectedSql.contains(tableName));
+            QVERIFY(expectedSql.value(tableName) == tableSql);
+        })));
+}
+
+void TestDbSchema::testCreation()
+{
+    QVector<RawDatabase::Query> queries;
+    auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testCreation.db", {}, {}}};
+    generateCurrentSchema(queries);
+    QVERIFY(db->execNow(queries));
+    const QMap<QString, QString> expectedSql {
+        {"aliases", "CREATE TABLE aliases (id INTEGER PRIMARY KEY, owner INTEGER, display_name BLOB NOT NULL, UNIQUE(owner, display_name))"},
+        {"faux_offline_pending", "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY)"},
+        {"file_transfers", "CREATE TABLE file_transfers (id INTEGER PRIMARY KEY, chat_id INTEGER NOT NULL, file_restart_id BLOB NOT NULL, file_name BLOB NOT NULL, file_path BLOB NOT NULL, file_hash BLOB NOT NULL, file_size INTEGER NOT NULL, direction INTEGER NOT NULL, file_state INTEGER NOT NULL)"},
+        {"history", "CREATE TABLE history (id INTEGER PRIMARY KEY, timestamp INTEGER NOT NULL, chat_id INTEGER NOT NULL, sender_alias INTEGER NOT NULL, message BLOB NOT NULL, file_id INTEGER)"},
+        {"peers", "CREATE TABLE peers (id INTEGER PRIMARY KEY, public_key TEXT NOT NULL UNIQUE)"}
+    };
+    verifyDb(db, expectedSql);
+}
+
+void TestDbSchema::testIsNewDb()
+{
+    auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testIsNewDbTrue.db", {}, {}}};
+    QVERIFY(isNewDb(db) == true);
+    db = std::shared_ptr<RawDatabase>{new RawDatabase{"testIsNewDbFalse.db", {}, {}}};
+    QVector<RawDatabase::Query> queries;
+    generateCurrentSchema(queries);
+    QVERIFY(db->execNow(queries));
+    QVERIFY(isNewDb(db) == false);
+}
+
+void TestDbSchema::test0to1()
+{
+    const QMap<QString, QString> expectedSql {
+        {"aliases", "CREATE TABLE aliases (id INTEGER PRIMARY KEY, owner INTEGER, display_name BLOB NOT NULL, UNIQUE(owner, display_name))"},
+        {"faux_offline_pending", "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY)"},
+        {"file_transfers", "CREATE TABLE file_transfers (id INTEGER PRIMARY KEY, chat_id INTEGER NOT NULL, file_restart_id BLOB NOT NULL, file_name BLOB NOT NULL, file_path BLOB NOT NULL, file_hash BLOB NOT NULL, file_size INTEGER NOT NULL, direction INTEGER NOT NULL, file_state INTEGER NOT NULL)"},
+        {"history", "CREATE TABLE history (id INTEGER PRIMARY KEY, timestamp INTEGER NOT NULL, chat_id INTEGER NOT NULL, sender_alias INTEGER NOT NULL, message BLOB NOT NULL, file_id INTEGER)"},
+        {"peers", "CREATE TABLE peers (id INTEGER PRIMARY KEY, public_key TEXT NOT NULL UNIQUE)"}
+    };
+    auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"test0to1.db", {}, {}}};
+    QVector<RawDatabase::Query> queries;
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE peers "
+        "(id INTEGER PRIMARY KEY, "
+        "public_key TEXT NOT NULL UNIQUE);"
+        "CREATE TABLE aliases "
+        "(id INTEGER PRIMARY KEY, "
+        "owner INTEGER, "
+        "display_name BLOB NOT NULL, "
+        "UNIQUE(owner, display_name));"
+        "CREATE TABLE history "
+        "(id INTEGER PRIMARY KEY, "
+        "timestamp INTEGER NOT NULL, "
+        "chat_id INTEGER NOT NULL, "
+        "sender_alias INTEGER NOT NULL, "
+        "message BLOB NOT NULL);"
+        "CREATE TABLE faux_offline_pending "
+        "(id INTEGER PRIMARY KEY);"));
+    QVERIFY(db->execNow(queries));
+    queries.clear();
+    dbSchema0to1(db, queries);
+    QVERIFY(db->execNow(queries));
+    verifyDb(db, expectedSql);
+}
+
+QTEST_GUILESS_MAIN(TestDbSchema)
+#include "dbschema_test.moc"