refactor(db): map foreign key dependency in db schema

Reorder deletion of history to avoid violating constraint.

src/persistence/history.cpp

@@ -27,7 +27,7 @@
 #include "src/core/toxpk.h"
 
 namespace {
-static constexpr int SCHEMA_VERSION = 4;
+static constexpr int SCHEMA_VERSION = 5;
 
 bool createCurrentSchema(RawDatabase& db)
 {
@@ -38,7 +38,8 @@ bool createCurrentSchema(RawDatabase& db)
         "CREATE TABLE aliases (id INTEGER PRIMARY KEY, "
         "owner INTEGER, "
         "display_name BLOB NOT NULL, "
-        "UNIQUE(owner, display_name));"
+        "UNIQUE(owner, display_name), "
+        "FOREIGN KEY (owner) REFERENCES peers(id));"
         "CREATE TABLE history "
         "(id INTEGER PRIMARY KEY, "
         "timestamp INTEGER NOT NULL, "
@@ -46,12 +47,15 @@ bool createCurrentSchema(RawDatabase& db)
         "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
+        // implementation 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);"
+        "file_id INTEGER, "
+        "FOREIGN KEY (file_id) REFERENCES file_transfers(id), "
+        "FOREIGN KEY (chat_id) REFERENCES peers(id), "
+        "FOREIGN KEY (sender_alias) REFERENCES aliases(id));"
         "CREATE TABLE file_transfers "
         "(id INTEGER PRIMARY KEY, "
         "chat_id INTEGER NOT NULL, "
@@ -62,8 +66,10 @@ 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 broken_messages (id INTEGER PRIMARY KEY);"));
+        "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY, "
+        "FOREIGN KEY (id) REFERENCES history(id));"
+        "CREATE TABLE broken_messages (id INTEGER PRIMARY KEY, "
+        "FOREIGN KEY (id) REFERENCES history(id));"));
     // sqlite doesn't support including the index as part of the CREATE TABLE statement, so add a second query
     queries += RawDatabase::Query(
         "CREATE INDEX chat_id_idx on history (chat_id);");
@@ -193,6 +199,83 @@ bool dbSchema3to4(RawDatabase& db)
     return db.execNow(upgradeQueries);
 }
 
+void addForeignKeyToAlias(QVector<RawDatabase::Query>& queries)
+{
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE aliases_new (id INTEGER PRIMARY KEY, owner INTEGER, "
+        "display_name BLOB NOT NULL, UNIQUE(owner, display_name), "
+        "FOREIGN KEY (owner) REFERENCES peers(id));"));
+    queries += RawDatabase::Query(QStringLiteral(
+        "INSERT INTO aliases_new (id, owner, display_name) "
+        "SELECT id, owner, display_name "
+        "FROM aliases;"));
+    queries += RawDatabase::Query(QStringLiteral("DROP TABLE aliases;"));
+    queries += RawDatabase::Query(QStringLiteral("ALTER TABLE aliases_new RENAME TO aliases;"));
+}
+
+void addForeignKeyToHistory(QVector<RawDatabase::Query>& queries)
+{
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE history_new "
+        "(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, "
+        "FOREIGN KEY (file_id) REFERENCES file_transfers(id), "
+        "FOREIGN KEY (chat_id) REFERENCES peers(id), "
+        "FOREIGN KEY (sender_alias) REFERENCES aliases(id));"));
+    queries += RawDatabase::Query(QStringLiteral(
+        "INSERT INTO history_new (id, timestamp, chat_id, sender_alias, message, file_id) "
+        "SELECT id, timestamp, chat_id, sender_alias, message, file_id "
+        "FROM history;"));
+    queries += RawDatabase::Query(QStringLiteral("DROP TABLE history;"));
+    queries += RawDatabase::Query(QStringLiteral("ALTER TABLE history_new RENAME TO history;"));
+}
+
+void addForeignKeyToFauxOfflinePending(QVector<RawDatabase::Query>& queries)
+{
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE new_faux_offline_pending (id INTEGER PRIMARY KEY, "
+        "FOREIGN KEY (id) REFERENCES history(id));"));
+    queries += RawDatabase::Query(QStringLiteral(
+        "INSERT INTO new_faux_offline_pending (id) "
+        "SELECT id "
+        "FROM faux_offline_pending;"));
+    queries += RawDatabase::Query(QStringLiteral("DROP TABLE faux_offline_pending;"));
+    queries += RawDatabase::Query(QStringLiteral("ALTER TABLE new_faux_offline_pending RENAME TO faux_offline_pending;"));
+}
+
+void addForeignKeyToBrokenMessages(QVector<RawDatabase::Query>& queries)
+{
+    queries += RawDatabase::Query(QStringLiteral(
+        "CREATE TABLE new_broken_messages (id INTEGER PRIMARY KEY, "
+        "FOREIGN KEY (id) REFERENCES history(id));"));
+    queries += RawDatabase::Query(QStringLiteral(
+        "INSERT INTO new_broken_messages (id) "
+        "SELECT id "
+        "FROM broken_messages;"));
+    queries += RawDatabase::Query(QStringLiteral("DROP TABLE broken_messages;"));
+    queries += RawDatabase::Query(QStringLiteral("ALTER TABLE new_broken_messages RENAME TO broken_messages;"));
+}
+
+bool dbSchema4to5(RawDatabase& db)
+{
+    // add foreign key contrains to database tables. sqlite doesn't support advanced alter table commands, so instead we
+    // need to copy data to new tables with the foreign key contraints: http://www.sqlitetutorial.net/sqlite-alter-table/
+    QVector<RawDatabase::Query> upgradeQueries;
+    addForeignKeyToAlias(upgradeQueries);
+    addForeignKeyToHistory(upgradeQueries);
+    addForeignKeyToFauxOfflinePending(upgradeQueries);
+    addForeignKeyToBrokenMessages(upgradeQueries);
+    upgradeQueries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 5;"));
+    auto transactionPass = db.execNow(upgradeQueries);
+    if (transactionPass) {
+        db.execNow("VACUUM;"); // after copying all the tables and deleting the old ones, our db file is half empty.
+    }
+    return transactionPass;
+}
 
 /**
 * @brief Upgrade the db schema
@@ -268,6 +351,13 @@ bool dbSchemaUpgrade(std::shared_ptr<RawDatabase>& db)
             return false;
        }
        qDebug() << "Database upgraded incrementally to schema version 4";
+       //fallthrough
+    case 4:
+       if (!dbSchema4to5(*db)) {
+            qCritical() << "Failed to upgrade db to schema version 5, aborting";
+            return false;
+       }
+       qDebug() << "Database upgraded incrementally to schema version 5";
     // etc.
     default:
         qInfo() << "Database upgrade finished (databaseSchemaVersion" << databaseSchemaVersion
@@ -319,6 +409,11 @@ History::History(std::shared_ptr<RawDatabase> db_)
         return;
     }
 
+    // foreign key support is not enabled by default, so needs to be enabled on every connection
+    // support was added in sqlite 3.6.19, which is qTox's minimum supported version
+    db->execNow(
+        "PRAGMA foreign_keys = ON;");
+
     const auto upgradeSucceeded = dbSchemaUpgrade(db);
 
     // dbSchemaUpgrade may have put us in an invalid state
@@ -384,11 +479,11 @@ void History::eraseHistory()
     }
 
     db->execNow("DELETE FROM faux_offline_pending;"
+                "DELETE FROM broken_messages;"
                 "DELETE FROM history;"
                 "DELETE FROM aliases;"
                 "DELETE FROM peers;"
                 "DELETE FROM file_transfers;"
-                "DELETE FROM broken_messages;"
                 "VACUUM;");
 }
 

test/persistence/dbschema_test.cpp

@@ -50,6 +50,7 @@ private slots:
     void test1to2();
     void test2to3();
     void test3to4();
+    void test4to5();
     void cleanupTestCase();
 private:
     bool initSucess{false};
@@ -64,7 +65,8 @@ const QString testFileList[] = {
     "test0to1.db",
     "test1to2.db",
     "test2to3.db",
-    "test3to4.db"
+    "test3to4.db",
+    "test4to5.db"
 };
 
 // db schemas can be select with "SELECT name, sql FROM sqlite_master;" on the database.
@@ -109,6 +111,17 @@ const std::vector<SqliteMasterEntry> schema4 {
     {"chat_id_idx", "CREATE INDEX chat_id_idx on history (chat_id)"}
 };
 
+// added foreign keys
+const std::vector<SqliteMasterEntry> schema5 {
+    {"aliases", "CREATE TABLE aliases (id INTEGER PRIMARY KEY, owner INTEGER, display_name BLOB NOT NULL, UNIQUE(owner, display_name), FOREIGN KEY (owner) REFERENCES peers(id))"},
+    {"faux_offline_pending", "CREATE TABLE faux_offline_pending (id INTEGER PRIMARY KEY, FOREIGN KEY (id) REFERENCES history(id))"},
+    {"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, FOREIGN KEY (file_id) REFERENCES file_transfers(id), FOREIGN KEY (chat_id) REFERENCES peers(id), FOREIGN KEY (sender_alias) REFERENCES aliases(id))"},
+    {"peers", "CREATE TABLE peers (id INTEGER PRIMARY KEY, public_key TEXT NOT NULL UNIQUE)"},
+    {"broken_messages", "CREATE TABLE broken_messages (id INTEGER PRIMARY KEY, FOREIGN KEY (id) REFERENCES history(id))"},
+    {"chat_id_idx", "CREATE INDEX chat_id_idx on history (chat_id)"}
+};
+
 void TestDbSchema::initTestCase()
 {
     for (const auto& path : testFileList) {
@@ -163,7 +176,7 @@ void TestDbSchema::testCreation()
     QVector<RawDatabase::Query> queries;
     auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"testCreation.db", {}, {}}};
     QVERIFY(createCurrentSchema(*db));
-    verifyDb(db, schema4);
+    verifyDb(db, schema5);
 }
 
 void TestDbSchema::testIsNewDb()
@@ -353,5 +366,13 @@ void TestDbSchema::test3to4()
     verifyDb(db, schema4);
 }
 
+void TestDbSchema::test4to5()
+{
+    auto db = std::shared_ptr<RawDatabase>{new RawDatabase{"test4to5.db", {}, {}}};
+    createSchemaAtVersion(db, schema4);
+    QVERIFY(dbSchema4to5(*db));
+    verifyDb(db, schema5);
+}
+
 QTEST_GUILESS_MAIN(TestDbSchema)
 #include "dbschema_test.moc"