From 9a8706a65f1737f875eccc9717c8389ff51798d2 Mon Sep 17 00:00:00 2001 From: Anthony Bilinski Date: Thu, 31 Mar 2022 17:34:38 -0700 Subject: [PATCH] fix(history): Heal duplicate peer entries with different case Prior to 2f4e8dc3e8563fe6a9e4f5fa306c44aaf411ef71 we would take the written ToxID and insert that straight into history without any case check Must be done prior to schema 11 since even though the UNIQUE constraint on the peers table is fooled by the different case, the UNIQUE constraint on the new chats and authors table which are stored as BLOBS fail during upgrade when the two different case but equal ToxPks collide. Unfortunately it can't be done as its own upgrade since 11 was already merged, and this is a prerequisite for 11 to pass for some users. Execute prior to starting the split peer upgrade instead of as a larger transaction for simplicity of the split upgrade, and since executing this deduplication is idempotent. --- src/persistence/db/upgrades/dbto11.cpp | 50 +++++++++++++++++++--- src/persistence/db/upgrades/dbto11.h | 6 +++ test/persistence/dbupgrade/dbTo11_test.cpp | 20 ++++++--- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/persistence/db/upgrades/dbto11.cpp b/src/persistence/db/upgrades/dbto11.cpp index 5f14c601c..43829d48e 100644 --- a/src/persistence/db/upgrades/dbto11.cpp +++ b/src/persistence/db/upgrades/dbto11.cpp @@ -20,10 +20,52 @@ #include "dbto11.h" #include "src/core/toxpk.h" +#include "src/persistence/db/upgrades/dbupgrader.h" #include bool DbTo11::dbSchema10to11(RawDatabase& db) +{ + QVector upgradeQueries; + if (!appendDeduplicatePeersQueries(db, upgradeQueries)) { + return false; + } + if (!db.execNow(upgradeQueries)) { + return false; + } + upgradeQueries.clear(); + + if (!appendSplitPeersQueries(db, upgradeQueries)) { + return false; + } + upgradeQueries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 11;")); + bool transactionPass = db.execNow(upgradeQueries); + if (transactionPass) { + return db.execNow("VACUUM"); + } + return transactionPass; +} + +bool DbTo11::appendDeduplicatePeersQueries(RawDatabase& db, QVector& upgradeQueries) +{ + std::vector badPeers; + if (!getInvalidPeers(db, badPeers)) { + return false; + } + DbUpgrader::mergeDuplicatePeers(upgradeQueries, db, badPeers); + return true; +} + +bool DbTo11::getInvalidPeers(RawDatabase& db, std::vector& badPeers) +{ + return db.execNow( + RawDatabase::Query("SELECT id, public_key FROM peers WHERE CAST(public_key AS BLOB) != CAST(UPPER(public_key) AS BLOB)", + [&](const QVector& row) { + badPeers.emplace_back(DbUpgrader::BadEntry{row[0].toInt(), row[1].toString()}); + })); +} + +bool DbTo11::appendSplitPeersQueries(RawDatabase& db, QVector& upgradeQueries) { // splitting peers table into separate chat and authors table. // peers had a dual-meaning of each friend having their own chat, but also each peer @@ -32,7 +74,6 @@ bool DbTo11::dbSchema10to11(RawDatabase& db) // since each group is a chat so is in peers, but authors no messages, and each group // member is an author so is in peers, but has no chat. // Splitting peers makes the relation much clearer and the tables have a single meaning. - QVector upgradeQueries; if (!PeersToAuthors::appendPeersToAuthorsQueries(db, upgradeQueries)) { return false; } @@ -40,12 +81,7 @@ bool DbTo11::dbSchema10to11(RawDatabase& db) return false; } appendDropPeersQueries(upgradeQueries); - upgradeQueries += RawDatabase::Query(QStringLiteral("PRAGMA user_version = 11;")); - bool transactionPass = db.execNow(upgradeQueries); - if (transactionPass) { - return db.execNow("VACUUM"); - } - return transactionPass; + return true; } bool DbTo11::PeersToAuthors::appendPeersToAuthorsQueries(RawDatabase& db, QVector& upgradeQueries) diff --git a/src/persistence/db/upgrades/dbto11.h b/src/persistence/db/upgrades/dbto11.h index 4ac66df99..71ad583d1 100644 --- a/src/persistence/db/upgrades/dbto11.h +++ b/src/persistence/db/upgrades/dbto11.h @@ -20,6 +20,7 @@ #pragma once #include "src/persistence/db/rawdatabase.h" +#include "src/persistence/db/upgrades/dbupgrader.h" #include @@ -28,6 +29,11 @@ class ToxPk; namespace DbTo11 { bool dbSchema10to11(RawDatabase& db); + bool appendDeduplicatePeersQueries(RawDatabase& db, QVector& upgradeQueries); + bool getInvalidPeers(RawDatabase& db, std::vector& badPeers); + bool appendSplitPeersQueries(RawDatabase& db, QVector& upgradeQueries); + + namespace PeersToAuthors { bool appendPeersToAuthorsQueries(RawDatabase& db, QVector& upgradeQueries); diff --git a/test/persistence/dbupgrade/dbTo11_test.cpp b/test/persistence/dbupgrade/dbTo11_test.cpp index c758ffc09..ae6d2c2ab 100644 --- a/test/persistence/dbupgrade/dbTo11_test.cpp +++ b/test/persistence/dbupgrade/dbTo11_test.cpp @@ -30,7 +30,7 @@ namespace { const auto selfPk = ToxPk{QByteArray(32, 0)}; -const auto aPk = ToxPk{QByteArray(32, 1)}; +const auto aPk = ToxPk{QByteArray(32, '=')}; const auto bPk = ToxPk{QByteArray(32, 2)}; const auto cPk = ToxPk{QByteArray(32, 3)}; const auto selfName = QStringLiteral("Self"); @@ -38,13 +38,15 @@ const auto aName = QStringLiteral("Alice"); const auto bName = QStringLiteral("Bob"); const auto selfAliasId = 1; const auto aPeerId = 2; -const auto aChatId = 1; +const auto aPeerDuplicateId = 5; +const auto aChatId = 3; const auto aAliasId = 2; +const auto aAliasDuplicateId = 4; const auto bPeerId = 3; -const auto bChatId = 2; +const auto bChatId = 1; const auto bAliasId = 3; const auto cPeerId = 4; -const auto cChatId = 3; +const auto cChatId = 2; void appendAddPeersQueries(QVector& setupQueries) { @@ -60,6 +62,9 @@ void appendAddPeersQueries(QVector& setupQueries) setupQueries.append(RawDatabase::Query{QStringLiteral( "INSERT INTO peers (id, public_key) VALUES (4, ?)"), {cPk.toString().toUtf8()}}); + setupQueries.append(RawDatabase::Query{QStringLiteral( + "INSERT INTO peers (id, public_key) VALUES (5, ?)"), + {aPk.toString().toLower().toUtf8()}}); } void appendVerifyChatsQueries(QVector& verifyQueries) @@ -113,6 +118,9 @@ void appendAddAliasesQueries(QVector& setupQueries) setupQueries.append(RawDatabase::Query{QStringLiteral( "INSERT INTO aliases (id, owner, display_name) VALUES (3, 3, ?)"), {bName.toUtf8()}}); + setupQueries.append(RawDatabase::Query{QStringLiteral( + "INSERT INTO aliases (id, owner, display_name) VALUES (4, 5, ?)"), + {aName.toUtf8()}}); } void appendVerifyAliasesQueries(QVector& verifyQueries) @@ -146,11 +154,11 @@ void appendAddAChatMessagesQueries(QVector& setupQueries) setupQueries.append(RawDatabase::Query{QStringLiteral( "INSERT INTO history (id, message_type, timestamp, chat_id) VALUES (2, 'T', 0, '%1')").arg(aPeerId)}); setupQueries.append(RawDatabase::Query{QStringLiteral( - "INSERT INTO text_messages (id, message_type, sender_alias, message) VALUES (2, 'T', '%1', ?)").arg(aAliasId), + "INSERT INTO text_messages (id, message_type, sender_alias, message) VALUES (2, 'T', '%1', ?)").arg(aAliasDuplicateId), {QStringLiteral("Message 2 from A to Self").toUtf8()}}); setupQueries.append(RawDatabase::Query{QStringLiteral( - "INSERT INTO history (id, message_type, timestamp, chat_id) VALUES (10, 'F', 0, '%1')").arg(aPeerId)}); + "INSERT INTO history (id, message_type, timestamp, chat_id) VALUES (10, 'F', 0, '%1')").arg(aPeerDuplicateId)}); setupQueries.append(RawDatabase::Query{QStringLiteral( "INSERT INTO file_transfers (id, message_type, sender_alias, file_restart_id, " "file_name, file_path, file_hash, file_size, direction, file_state) "