diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp index 8ccb17b6a..53d69c83f 100644 --- a/src/persistence/history.cpp +++ b/src/persistence/history.cpp @@ -26,7 +26,7 @@ #include "db/rawdatabase.h" namespace { -static constexpr int SCHEMA_VERSION = 1; +static constexpr int SCHEMA_VERSION = 2; bool createCurrentSchema(RawDatabase& db) { @@ -61,7 +61,8 @@ bool createCurrentSchema(RawDatabase& db) "file_size INTEGER NOT NULL, " "direction INTEGER NOT NULL, " "file_state INTEGER NOT NULL);" - "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY);")); + "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY);" + "CREATE TABLE broken_messages (id INTEGER PRIMARY KEY);")); queries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = %1;").arg(SCHEMA_VERSION)); return db.execNow(queries); } @@ -102,6 +103,53 @@ bool dbSchema0to1(RawDatabase& db) return db.execNow(queries); } +bool dbSchema1to2(RawDatabase& db) +{ + // Any faux_offline_pending message, in a chat that has newer delivered + // message is decided to be broken. It must be moved from + // faux_offline_pending to broken_messages + + // the last non-pending message in each chat + QString lastDeliveredQuery = QString( + "SELECT chat_id, MAX(history.id) FROM " + "history JOIN peers chat ON chat_id = chat.id " + "LEFT JOIN faux_offline_pending ON history.id = faux_offline_pending.id " + "WHERE faux_offline_pending.id IS NULL " + "GROUP BY chat_id;"); + + QVector upgradeQueries; + upgradeQueries += + RawDatabase::Query(QStringLiteral( + "CREATE TABLE broken_messages " + "(id INTEGER PRIMARY KEY);")); + + auto rowCallback = [&upgradeQueries](const QVector& row) { + auto chatId = row[0].toLongLong(); + auto lastDeliveredHistoryId = row[1].toLongLong(); + + upgradeQueries += QString("INSERT INTO broken_messages " + "SELECT faux_offline_pending.id FROM " + "history JOIN faux_offline_pending " + "ON faux_offline_pending.id = history.id " + "WHERE history.chat_id=%1 " + "AND history.id < %2;").arg(chatId).arg(lastDeliveredHistoryId); + }; + // note this doesn't modify the db, just generate new queries, so is safe + // to run outside of our upgrade transaction + if (!db.execNow({lastDeliveredQuery, rowCallback})) { + return false; + } + + upgradeQueries += QString( + "DELETE FROM faux_offline_pending " + "WHERE id in (" + "SELECT id FROM broken_messages);"); + + upgradeQueries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 2;")); + + return db.execNow(upgradeQueries); +} + /** * @brief Upgrade the db schema * @note On future alterations of the database all you have to do is bump the SCHEMA_VERSION @@ -160,9 +208,14 @@ void dbSchemaUpgrade(std::shared_ptr& db) } } // fallthrough - // case 1: - // dbSchema1to2(queries); - // //fallthrough + case 1: + if (!dbSchema1to2(*db)) { + qCritical() << "Failed to upgrade db to schema version 2, aborting"; + db.reset(); + return; + } + qDebug() << "Database upgraded incrementally to schema version 2"; + //fallthrough // etc. default: qInfo() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion @@ -262,6 +315,7 @@ void History::eraseHistory() "DELETE FROM aliases;" "DELETE FROM peers;" "DELETE FROM file_transfers;" + "DELETE FROM broken_messages;" "VACUUM;"); } @@ -287,6 +341,12 @@ void History::removeFriendHistory(const QString& friendPk) " LEFT JOIN history ON faux_offline_pending.id = history.id " " WHERE chat_id=%1 " "); " + "DELETE FROM broken_messages " + "WHERE broken_messages.id IN ( " + " SELECT broken_messages.id FROM broken_messages " + " LEFT JOIN history ON broken_messages.id = history.id " + " WHERE chat_id=%1 " + "); " "DELETE FROM history WHERE chat_id=%1; " "DELETE FROM aliases WHERE owner=%1; " "DELETE FROM peers WHERE id=%1; " diff --git a/test/persistence/dbschema_test.cpp b/test/persistence/dbschema_test.cpp index a49a303ec..95fe070e3 100644 --- a/test/persistence/dbschema_test.cpp +++ b/test/persistence/dbschema_test.cpp @@ -35,6 +35,7 @@ private slots: void testCreation(); void testIsNewDb(); void test0to1(); + void test1to2(); void cleanupTestCase(); private: bool initSucess{false}; @@ -46,7 +47,8 @@ const QString testFileList[] = { "testCreation.db", "testIsNewDbTrue.db", "testIsNewDbFalse.db", - "test0to1.db" + "test0to1.db", + "test1to2.db" }; const QMap schema0 { @@ -65,6 +67,16 @@ const QMap schema1 { {"peers", "CREATE TABLE peers (id INTEGER PRIMARY KEY, public_key TEXT NOT NULL UNIQUE)"} }; +// move stuck faux offline messages do a table of "broken" messages +const QMap schema2 { + {"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)"}, + {"broken_messages", "CREATE TABLE broken_messages (id INTEGER PRIMARY KEY)"} +}; + void TestDbSchema::initTestCase() { for (const auto& path : testFileList) { @@ -115,7 +127,7 @@ void TestDbSchema::testCreation() QVector queries; auto db = std::shared_ptr{new RawDatabase{"testCreation.db", {}, {}}}; QVERIFY(createCurrentSchema(*db)); - verifyDb(db, schema1); + verifyDb(db, schema2); } void TestDbSchema::testIsNewDb() @@ -140,5 +152,100 @@ void TestDbSchema::test0to1() verifyDb(db, schema1); } +void TestDbSchema::test1to2() +{ + /* + Due to a long standing bug, faux offline message have been able to become stuck + going back years. Because of recent fixes to history loading, faux offline + messages will correctly all be sent on connection, but this causes an issue of + long stuck messages suddenly being delivered to a friend, out of context, + creating a confusing interaction. To work around this, this upgrade moves any + faux offline messages in a chat that are older than the last successfully + delivered message, indicating they were stuck, to a new table, + `broken_messages`, preventing them from ever being sent in the future. + + https://github.com/qTox/qTox/issues/5776 + */ + + auto db = std::shared_ptr{new RawDatabase{"test1to2.db", {}, {}}}; + createSchemaAtVersion(db, schema1); + + const QString myPk = "AC18841E56CCDEE16E93E10E6AB2765BE54277D67F1372921B5B418A6B330D3D"; + const QString friend1Pk = "FE34BC6D87B66E958C57BBF205F9B79B62BE0AB8A4EFC1F1BB9EC4D0D8FB0663"; + const QString friend2Pk = "2A1CBCE227549459C0C20F199DB86AD9BCC436D35BAA1825FFD4B9CA3290D200"; + + QVector queries; + queries += QString("INSERT INTO peers (id, public_key) VALUES (%1, '%2')").arg(0).arg(myPk); + queries += QString("INSERT INTO peers (id, public_key) VALUES (%1, '%2')").arg(1).arg(friend1Pk); + queries += QString("INSERT INTO peers (id, public_key) VALUES (%1, '%2')").arg(2).arg(friend2Pk); + + // friend 1 + // first message in chat is pending - but the second is delivered. This message is "broken" + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (1, 1, 1, ?, 0)", + {"first message in chat, pending and stuck"}}; + queries += {"INSERT INTO faux_offline_pending (id) VALUES (" + " last_insert_rowid()" + ");"}; + // second message is delivered, causing the first to be considered broken + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (2, 2, 1, ?, 0)", + {"second message in chat, delivered"}}; + + // third message is pending - this is a normal pending message. It should be untouched. + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (3, 3, 1, ?, 0)", + {"third message in chat, pending"}}; + queries += {"INSERT INTO faux_offline_pending (id) VALUES (" + " last_insert_rowid()" + ");"}; + + // friend 2 + // first message is delivered. + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (4, 4, 2, ?, 2)", + {"first message by friend in chat, delivered"}}; + + // second message is also delivered. + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (5, 5, 2, ?, 0)", + {"first message by us in chat, delivered"}}; + + // third message is pending, but not broken since there are no delivered messages after it. + queries += RawDatabase::Query{ + "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (6, 6, 2, ?, 0)", + {"last message in chat, by us, pending"}}; + queries += {"INSERT INTO faux_offline_pending (id) VALUES (" + " last_insert_rowid()" + ");"}; + + QVERIFY(db->execNow(queries)); + QVERIFY(dbSchema1to2(*db)); + verifyDb(db, schema2); + + long brokenCount = -1; + RawDatabase::Query brokenCountQuery = {"SELECT COUNT(*) FROM broken_messages;", [&](const QVector& row) { + brokenCount = row[0].toLongLong(); + }}; + QVERIFY(db->execNow(brokenCountQuery)); + QVERIFY(brokenCount == 1); // only friend 1's first message is "broken" + + int fauxOfflineCount = -1; + RawDatabase::Query fauxOfflineCountQuery = {"SELECT COUNT(*) FROM faux_offline_pending;", [&](const QVector& row) { + fauxOfflineCount = row[0].toLongLong(); + }}; + QVERIFY(db->execNow(fauxOfflineCountQuery)); + // both friend 1's third message and friend 2's third message should still be pending. + //The broken message should no longer be pending. + QVERIFY(fauxOfflineCount == 2); + + int totalHisoryCount = -1; + RawDatabase::Query totalHistoryCountQuery = {"SELECT COUNT(*) FROM history;", [&](const QVector& row) { + totalHisoryCount = row[0].toLongLong(); + }}; + QVERIFY(db->execNow(totalHistoryCountQuery)); + QVERIFY(totalHisoryCount == 6); // all messages should still be in history. +} + QTEST_GUILESS_MAIN(TestDbSchema) #include "dbschema_test.moc"