fix(history): move stuck action messages to broken_messages table

Before a bug in qTox would make it possible for a user to try to send an empty
action type message. This would fail to send at toxcore, but still be persisted
in history, causing it to fail every time FauxOfflineEngine tried to resend
it. Moving these stuck messages into the broke_messages table will stop qTox
from attempting to deliver them on each connect, and display in the GUI to
users that the messages aren't really pending anymore.

src/persistence/history.cpp

@@ -26,7 +26,7 @@
 #include "db/rawdatabase.h"
 
 namespace {
-static constexpr int SCHEMA_VERSION = 2;
+static constexpr int SCHEMA_VERSION = 3;
 
 bool createCurrentSchema(RawDatabase& db)
 {
@@ -150,6 +150,34 @@ bool dbSchema1to2(RawDatabase& db)
     return db.execNow(upgradeQueries);
 }
 
+bool dbSchema2to3(RawDatabase& db)
+{
+    // Any faux_offline_pending message with the content "/me " are action
+    // messages that qTox previously let a user enter, but that will cause an
+    // action type message to be sent to toxcore, with 0 length, which will
+    // always fail. They must be be moved from faux_offline_pending to broken_messages
+    // to avoid qTox from erroring trying to send them on every connect
+
+    const QString emptyActionMessageString = "/me ";
+
+    QVector<RawDatabase::Query> upgradeQueries;
+    upgradeQueries += RawDatabase::Query{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.message = ?;"),
+            {emptyActionMessageString.toUtf8()}};
+
+    upgradeQueries += QString(
+        "DELETE FROM faux_offline_pending "
+        "WHERE id in ("
+            "SELECT id FROM broken_messages);");
+
+    upgradeQueries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 3;"));
+
+    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
@@ -216,6 +244,13 @@ void dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
        }
        qDebug() << "Database upgraded incrementally to schema version 2";
        //fallthrough
+    case 2:
+       if (!dbSchema2to3(*db)) {
+            qCritical() << "Failed to upgrade db to schema version 3, aborting";
+            db.reset();
+            return;
+       }
+       qDebug() << "Database upgraded incrementally to schema version 3";
     // etc.
     default:
         qInfo() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion

test/persistence/dbschema_test.cpp

@@ -36,6 +36,7 @@ private slots:
     void testIsNewDb();
     void test0to1();
     void test1to2();
+    void test2to3();
     void cleanupTestCase();
 private:
     bool initSucess{false};
@@ -48,7 +49,8 @@ const QString testFileList[] = {
     "testIsNewDbTrue.db",
     "testIsNewDbFalse.db",
     "test0to1.db",
-    "test1to2.db"
+    "test1to2.db",
+    "test2to3.db"
 };
 
 const QMap<QString, QString> schema0 {
@@ -77,6 +79,9 @@ const QMap<QString, QString> schema2 {
     {"broken_messages", "CREATE TABLE broken_messages (id INTEGER PRIMARY KEY)"}
 };
 
+// move stuck 0-length action messages to the existing "broken_messages" table. Not a real schema upgrade.
+const auto schema3 = schema2;
+
 void TestDbSchema::initTestCase()
 {
     for (const auto& path : testFileList) {
@@ -127,7 +132,7 @@ void TestDbSchema::testCreation()
     QVector<RawDatabase::Query> queries;
     auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testCreation.db", {}, {}}};
     QVERIFY(createCurrentSchema(*db));
-    verifyDb(db, schema2);
+    verifyDb(db, schema3);
 }
 
 void TestDbSchema::testIsNewDb()
@@ -247,5 +252,67 @@ void TestDbSchema::test1to2()
     QVERIFY(totalHisoryCount == 6); // all messages should still be in history.
 }
 
+void TestDbSchema::test2to3()
+{
+    auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"test2to3.db", {}, {}}};
+    createSchemaAtVersion(db, schema2);
+
+    // since we don't enforce foreign key contraints in the db, we can stick in IDs to other tables
+    // to avoid generating proper entries for peers and aliases tables, since they aren't actually
+    // relevant for the test.
+
+    QVector<RawDatabase::Query> queries;
+    // pending message, should be moved out
+    queries += RawDatabase::Query{
+        "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (1, 1, 0, ?, 0)",
+        {"/me "}};
+    queries += {"INSERT INTO faux_offline_pending (id) VALUES ("
+                                        "    last_insert_rowid()"
+                                        ");"};
+
+    // non pending message with the content "/me ". Maybe it was sent by a friend using a different client.
+    queries += RawDatabase::Query{
+        "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (2, 2, 0, ?, 2)",
+        {"/me "}};
+
+    // non pending message sent by us
+    queries += RawDatabase::Query{
+        "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (3, 3, 0, ?, 1)",
+        {"a normal message"}};
+
+    // pending normal message sent by us
+    queries += RawDatabase::Query{
+        "INSERT INTO history (id, timestamp, chat_id, message, sender_alias) VALUES (4, 3, 0, ?, 1)",
+        {"a normal faux offline message"}};
+    queries += {"INSERT INTO faux_offline_pending (id) VALUES ("
+                                        "    last_insert_rowid()"
+                                        ");"};
+    QVERIFY(db->execNow(queries));
+    QVERIFY(dbSchema2to3(*db));
+
+    long brokenCount = -1;
+    RawDatabase::Query brokenCountQuery = {"SELECT COUNT(*) FROM broken_messages;", [&](const QVector<QVariant>& row) {
+        brokenCount = row[0].toLongLong();
+    }};
+    QVERIFY(db->execNow(brokenCountQuery));
+    QVERIFY(brokenCount == 1);
+
+    int fauxOfflineCount = -1;
+    RawDatabase::Query fauxOfflineCountQuery = {"SELECT COUNT(*) FROM faux_offline_pending;", [&](const QVector<QVariant>& row) {
+        fauxOfflineCount = row[0].toLongLong();
+    }};
+    QVERIFY(db->execNow(fauxOfflineCountQuery));
+    QVERIFY(fauxOfflineCount == 1);
+
+    int totalHisoryCount = -1;
+    RawDatabase::Query totalHistoryCountQuery = {"SELECT COUNT(*) FROM history;", [&](const QVector<QVariant>& row) {
+        totalHisoryCount = row[0].toLongLong();
+    }};
+    QVERIFY(db->execNow(totalHistoryCountQuery));
+    QVERIFY(totalHisoryCount == 4);
+
+    verifyDb(db, schema3);
+}
+
 QTEST_GUILESS_MAIN(TestDbSchema)
 #include "dbschema_test.moc"