From 316893ace9e54fcbccdcd9d53a615638a242b36a Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Wed, 1 May 2019 00:50:19 -0700 Subject: [PATCH] test(db): add db schema upgrade test --- cmake/Testing.cmake | 1 + src/persistence/history.cpp | 244 +++++++++++++++-------------- src/persistence/history.h | 3 - test/persistence/dbschema_test.cpp | 146 +++++++++++++++++ 4 files changed, 275 insertions(+), 119 deletions(-) create mode 100644 test/persistence/dbschema_test.cpp diff --git a/cmake/Testing.cmake b/cmake/Testing.cmake index 6906df5f2..1170c0156 100644 --- a/cmake/Testing.cmake +++ b/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) diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp index 14f8a3e4a..0d9cb0868 100644 --- a/src/persistence/history.cpp +++ b/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& 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 db) +{ + bool newDb; + if (!db->execNow(RawDatabase::Query("SELECT COUNT(*) FROM sqlite_master;", + [&](const QVector& row) { + newDb = row[0].toLongLong() == 0; + }))) { + db.reset(); + return false; // TODO: propogate error + } + return newDb; +} + +void dbSchema0to1(std::shared_ptr db, QVector& 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 db) +{ + int64_t databaseSchemaVersion; + + if (!db->execNow(RawDatabase::Query("PRAGMA user_version", [&](const QVector& 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 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 db) return; } - dbSchemaUpgrade(); + dbSchemaUpgrade(db); // dbSchemaUpgrade may have put us in an invalid state if (!isValid()) { @@ -664,117 +790,3 @@ QList 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& 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 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& 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 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; -} diff --git a/src/persistence/history.h b/src/persistence/history.h index c3f176585..fe0a74739 100644 --- a/src/persistence/history.h +++ b/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 db; diff --git a/test/persistence/dbschema_test.cpp b/test/persistence/dbschema_test.cpp new file mode 100644 index 000000000..69da1883a --- /dev/null +++ b/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 . +*/ + +#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 +#include + +#include + +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 db, const QMap& 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 db, const QMap& expectedSql) +{ + QVERIFY(db->execNow(RawDatabase::Query(QStringLiteral("SELECT name, sql FROM sqlite_master " + "WHERE type='table' " + "ORDER BY name;"), + [&](const QVector& 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 queries; + auto db = std::shared_ptr{new RawDatabase{"testCreation.db", {}, {}}}; + generateCurrentSchema(queries); + QVERIFY(db->execNow(queries)); + const QMap 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{new RawDatabase{"testIsNewDbTrue.db", {}, {}}}; + QVERIFY(isNewDb(db) == true); + db = std::shared_ptr{new RawDatabase{"testIsNewDbFalse.db", {}, {}}}; + QVector queries; + generateCurrentSchema(queries); + QVERIFY(db->execNow(queries)); + QVERIFY(isNewDb(db) == false); +} + +void TestDbSchema::test0to1() +{ + const QMap 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{new RawDatabase{"test0to1.db", {}, {}}}; + QVector 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"