From 674547fca02e3c82daf3994af1bce9849d7fe81f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 16:05:22 +0200 Subject: [PATCH 1/6] [Server+Client] Tombstone deck deletes with purge and base snapshots --- .../interface/deck_sync/deck_sync_manager.cpp | 117 +++++++++++++++++- .../interface/deck_sync/deck_sync_manager.h | 9 ++ servatrice/servatrice.ini.example | 7 ++ servatrice/src/servatrice.cpp | 11 ++ servatrice/src/servatrice.h | 2 + .../src/servatrice_database_interface.cpp | 17 ++- .../src/servatrice_database_interface.h | 5 + servatrice/src/serversocketinterface.cpp | 33 ++--- 8 files changed, 180 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index e5748c512..6dbb71998 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -116,6 +117,7 @@ void DeckSyncManager::syncNow() skippedCount = 0; pendingUploads.clear(); pendingDownloads.clear(); + pendingDeletes.clear(); plannedLocalPaths.clear(); ensuredDirs.clear(); ensuredDirs.insert(QString()); @@ -275,8 +277,21 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } } } - if (serverId == -1) { + if (sidecarRecord.remoteId != -1) { + // The server no longer lists this deck: it was deleted elsewhere. + // Apply the delete locally, keeping a backup of the file. + const QString absolutePath = deckPath + "/" + it.key(); + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath) || !QFile::remove(absolutePath)) { + ++skippedCount; + continue; + } + sidecar.remove(it.key()); + ++downloadedCount; + continue; + } const QString serverPath = serverPathFor(it.key()); pendingUploads.append(UploadOp{.relativePath = it.key(), .serverPath = serverPath, @@ -298,6 +313,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont record.lastSyncSecs = nowSecs; record.deleted = false; sidecar.set(it.key(), record); + snapshotLocalFile(it.key(), record.guid); ++unchangedCount; continue; } @@ -337,6 +353,18 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont .lastModifiedSecs = it.value().lastModifiedSecs}); } + const QMap knownRecords = sidecar.records(); + for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { + if (localFiles.contains(it.key())) { + continue; + } + if (it.value().remoteId == -1) { + sidecar.remove(it.key()); + continue; + } + pendingDeletes.append(it.key()); + } + sidecar.save(); processNext(); } @@ -352,7 +380,12 @@ void DeckSyncManager::processNext() if (!syncing) { return; } - if (!pendingUploads.isEmpty()) { + if (!pendingDeletes.isEmpty()) { + const QString relativePath = pendingDeletes.first(); + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sendDelete(relativePath, sidecar.get(relativePath).remoteId); + } else if (!pendingUploads.isEmpty()) { processNextUpload(); } else if (!pendingDownloads.isEmpty()) { sendDownload(pendingDownloads.first()); @@ -453,7 +486,12 @@ void DeckSyncManager::sendUpload(const UploadOp &op) cmd.set_color_identity(colorIdentityProvider(loaded->deckList).toStdString()); } PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); - pend->setExtraData(op.relativePath); + QVariantMap context; + context.insert("path", op.relativePath); + context.insert("serverPath", op.serverPath); + context.insert("deckId", op.deckId); + context.insert("guid", op.guid); + pend->setExtraData(context); connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onUploadFinished); client->sendCommand(pend); } @@ -469,8 +507,22 @@ void DeckSyncManager::onUploadFinished(const Response &response, failSync(tr("Server connection lost.")); return; } - const QString relativePath = extraData.toString(); + const QVariantMap context = extraData.toMap(); + const QString relativePath = context.value("path").toString(); + const int deckId = context.value("deckId", -1).toInt(); if (response.response_code() != Response::RespOk) { + if (response.response_code() == Response::RespNameNotFound && deckId != -1) { + // The server row is gone (purged tombstone or removed elsewhere): + // upload the deck as new instead of updating a dead id. + UploadOp retry; + retry.relativePath = relativePath; + retry.serverPath = context.value("serverPath").toString(); + retry.deckId = -1; + retry.guid = context.value("guid").toString(); + pendingUploads.prepend(retry); + processNext(); + return; + } ++skippedCount; processNext(); return; @@ -501,6 +553,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); + snapshotLocalFile(relativePath, record.guid); ++uploadedCount; processNext(); } @@ -615,10 +668,66 @@ void DeckSyncManager::onDownloadFinished(const Response &response, record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); + snapshotLocalFile(relativePath, record.guid); ++downloadedCount; processNext(); } +void DeckSyncManager::sendDelete(const QString &relativePath, int deckId) +{ + emit statusMessage(tr("Deleting %1 from server...").arg(relativePath)); + Command_DeckDel cmd; + cmd.set_deck_id(deckId); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onDeleteFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onDeleteFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + for (int i = 0; i < pendingDeletes.size(); ++i) { + if (pendingDeletes.at(i) == relativePath) { + pendingDeletes.removeAt(i); + break; + } + } + // Both outcomes converge the record: pushed deletes and server-side + // deletes (already tombstoned, answering NameNotFound) are done. + if (response.response_code() == Response::RespOk || response.response_code() == Response::RespNameNotFound) { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sidecar.remove(relativePath); + sidecar.save(); + } else { + ++skippedCount; + } + processNext(); +} + +void DeckSyncManager::snapshotLocalFile(const QString &relativePath, const QString &guid) +{ + if (guid.isEmpty()) { + return; + } + QFile source(deckPath + "/" + relativePath); + if (!source.open(QIODevice::ReadOnly)) { + return; + } + QDir().mkpath(deckPath + "/" + SYNC_META_DIR + "/bases"); + QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + return; + } + snapshot.write(source.readAll()); +} + void DeckSyncManager::failSync(const QString &error) { syncing = false; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 30de5b35a..b0020b33b 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -38,6 +38,10 @@ class ServerInfo_DeckStorage_Folder; * * Ownership: fire-and-forget per run, driven by PendingCommand round trips. * No watcher, no auto triggers, no conflict UI — those arrive in later layers. + * + * Deletes are tombstones, never silent: a locally deleted deck is pushed with + * Command_DeckDel, a server-side delete is applied locally only after copying + * the file to a .sync-backup, and resurrection reuses the stored deck id. */ class DeckSyncManager : public QObject { @@ -78,6 +82,8 @@ private slots: onUploadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); void onDownloadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onDeleteFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); private: struct ServerFile @@ -125,6 +131,8 @@ private: void sendMkDirFor(const QString &dir); void sendUpload(const UploadOp &op); void sendDownload(const DownloadOp &op); + void sendDelete(const QString &relativePath, int deckId); + void snapshotLocalFile(const QString &relativePath, const QString &guid); static QString parentDir(const QString &path); QString uniqueLocalPath(const QString &relativePath) const; @@ -140,6 +148,7 @@ private: QSet plannedLocalPaths; ///< Download targets reserved during this run, not yet on disk. QList pendingUploads; QList pendingDownloads; + QList pendingDeletes; int uploadedCount = 0; int downloadedCount = 0; int unchangedCount = 0; diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 378f83422..1ac2e67eb 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -600,3 +600,10 @@ max_shares_per_day=50 ; Minimum enforced: 1. ; Default: 20 max_versions=20 + +; How many days a deleted deck stays restorable as a tombstone before the +; cleanup clock purges it (with its versions) for good. Re-uploading the +; same deck within this window resurrects it instead of creating a copy. +; A value of 0 purges tombstones on the next cleanup run. +; Default: 30 +tombstone_days=30 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index c023c5ae0..15d8ee016 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -629,6 +629,7 @@ bool Servatrice::initServer() deckShareCleanupClock = new QTimer(this); connect(deckShareCleanupClock, &QTimer::timeout, this, &Servatrice::cleanupExpiredDeckShares); + connect(deckShareCleanupClock, &QTimer::timeout, this, &Servatrice::purgeDeckTombstones); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -863,6 +864,11 @@ void Servatrice::cleanupExpiredDeckShares() servatriceDatabaseInterface->cleanupExpiredDeckShares(); } +void Servatrice::purgeDeckTombstones() +{ + servatriceDatabaseInterface->purgeDeckTombstones(getDeckSyncTombstoneDays()); +} + void Servatrice::statusUpdate() { if (!servatriceDatabaseInterface->checkSql()) { @@ -1402,6 +1408,11 @@ int Servatrice::getDeckSyncMaxVersions() const return qMax(1, settingsCache->value("deck_sync/max_versions", 20).toInt()); } +int Servatrice::getDeckSyncTombstoneDays() const +{ + return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index 3be364aa3..f925c0984 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -166,6 +166,7 @@ private slots: void sweepLegacyBackups(); void shutdownTimeout(); void cleanupExpiredDeckShares(); + void purgeDeckTombstones(); protected: void doSendIslMessage(const IslMessage &msg, int _serverId) override; @@ -321,6 +322,7 @@ public: int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; + int getDeckSyncTombstoneDays() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index a4f184e38..ceba342ea 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -1384,8 +1384,8 @@ DeckList *Servatrice_DatabaseInterface::getDeckFromDatabase(int deckId, int user { checkSql(); - QSqlQuery *query = - prepareQuery("select content from {prefix}_decklist_files where id = :id and id_user = :id_user"); + QSqlQuery *query = prepareQuery( + "select content from {prefix}_decklist_files where id = :id and id_user = :id_user and deleted = 0"); query->bindValue(":id", deckId); query->bindValue(":id_user", userId); execSqlQuery(query); @@ -1536,6 +1536,19 @@ void Servatrice_DatabaseInterface::cleanupExpiredDeckShares() execSqlQuery(query); } +void Servatrice_DatabaseInterface::purgeDeckTombstones(int olderThanDays) +{ + checkSql(); + + // DB-native clock: tombstones are stamped with NOW() on this same + // database, so the grace window is immune to app/db clock skew. + // The null arm only catches manually flagged rows. + QSqlQuery *query = prepareQuery("delete from {prefix}_decklist_files where deleted = 1 and (last_modified < " + "DATE_SUB(NOW(), INTERVAL :days DAY) or last_modified is null)"); + query->bindValue(":days", qMax(0, olderThanDays)); + execSqlQuery(query); +} + bool Servatrice_DatabaseInterface::getDeckSharesForUser(int userId, QList &shares) { checkSql(); diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index b053335c4..041923b0a 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -155,6 +155,11 @@ public: bool getDeckShareItem(const QString &token, int itemId, QString &content); /** @brief Deletes all expired share bundles (cascades to their items). */ void cleanupExpiredDeckShares(); + /** + * @brief Hard-deletes tombstoned decks older than the grace period (cascades to versions). + * @param olderThanDays Tombstones older than this many days are purged; 0 purges all tombstones. + */ + void purgeDeckTombstones(int olderThanDays); int getNextGameId() override; int getNextReplayId() override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 4eaf9d769..7f0403a94 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -710,7 +710,8 @@ bool AbstractServerSocketInterface::deckListHelper(int folderId, query = sqlInterface->prepareQuery("select id, name, upload_time, is_public, banner_card_name, " "banner_card_provider, color_identity, tags, guid, hash, last_modified from " - "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user"); + "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user " + "and deleted = 0"); query->bindValue(":id_folder", folderId); query->bindValue(":id_user", userId); if (!sqlInterface->execSqlQuery(query)) { @@ -855,8 +856,8 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSetVisibility(const sqlInterface->checkSql(); if (cmd.has_deck_id()) { - QSqlQuery *query = - sqlInterface->prepareQuery("select 1 from {prefix}_decklist_files where id = :id and id_user = :id_user"); + QSqlQuery *query = sqlInterface->prepareQuery( + "select 1 from {prefix}_decklist_files where id = :id and id_user = :id_user and deleted = 0"); query->bindValue(":id", cmd.deck_id()); query->bindValue(":id_user", userInfo->id()); sqlInterface->execSqlQuery(query); @@ -966,7 +967,8 @@ void AbstractServerSocketInterface::deckDelDirHelper(int basePathId) deckDelDirHelper(query->value(0).toInt()); } - query = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id_folder = :id_folder"); + query = sqlInterface->prepareQuery("update {prefix}_decklist_files set deleted = 1, last_modified = NOW() " + "where id_folder = :id_folder and deleted = 0"); query->bindValue(":id_folder", basePathId); sqlInterface->execSqlQuery(query); @@ -1017,18 +1019,17 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckDel(const Command_D sqlInterface->checkSql(); QSqlQuery *query = - sqlInterface->prepareQuery("select id from {prefix}_decklist_files where id = :id and id_user = :id_user"); + sqlInterface->prepareQuery("update {prefix}_decklist_files set deleted = 1, last_modified = NOW() where id = " + ":id and id_user = :id_user and deleted = 0"); query->bindValue(":id", cmd.deck_id()); query->bindValue(":id_user", userInfo->id()); - sqlInterface->execSqlQuery(query); - if (!query->next()) { + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespContextError; + } + if (query->numRowsAffected() == 0) { return Response::RespNameNotFound; } - query = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - query->bindValue(":id", cmd.deck_id()); - sqlInterface->execSqlQuery(query); - return Response::RespOk; } @@ -1198,9 +1199,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman } const QString oldHash = currentQuery->value(1).toString(); - QString updateQuery = "update {prefix}_decklist_files set name=:name, upload_time=NOW(), content=:content, " - "banner_card_name=:banner_card_name, banner_card_provider=:banner_card_provider, " - "color_identity=:color_identity, tags=:tags, guid=:guid, hash=:hash, last_modified=NOW()"; + QString updateQuery = + "update {prefix}_decklist_files set name=:name, upload_time=NOW(), content=:content, " + "banner_card_name=:banner_card_name, banner_card_provider=:banner_card_provider, " + "color_identity=:color_identity, tags=:tags, guid=:guid, hash=:hash, last_modified=NOW(), " + "deleted=0"; if (cmd.has_is_public()) { updateQuery += ", is_public=:is_public"; } @@ -1402,7 +1405,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckShareCreate(const C // issues its own query on the same cached statement set. QSqlQuery *query = sqlInterface->prepareQuery("select id, color_identity from {prefix}_decklist_files where id_folder = " - ":id_folder and id_user = :id_user"); + ":id_folder and id_user = :id_user and deleted = 0"); query->bindValue(":id_folder", folderId); query->bindValue(":id_user", userInfo->id()); if (!sqlInterface->execSqlQuery(query)) { From 4f715b0800ff10296c8955cea2a348d776731806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:19:32 +0200 Subject: [PATCH 2/6] Fix ZeldaZach review for PR 7494: match by remoteId/guid before remote-delete, only tombstone when file truly missing --- cockatrice/src/interface/deck_sync/deck_sync_manager.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 6dbb71998..09eabea6b 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -358,6 +358,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont if (localFiles.contains(it.key())) { continue; } + if (QFileInfo::exists(deckPath + "/" + it.key())) { + continue; + } if (it.value().remoteId == -1) { sidecar.remove(it.key()); continue; From 70b75018beb0ca8569f33e292fe152604066a497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 13:21:05 +0100 Subject: [PATCH 3/6] [DeckSync] Address tombstone review: remote deletes, revival and purge clock - Treat a record whose server row is gone as a remote delete only when the local file still matches the sidecar's base hash. An edit made after the remote delete is newer than the delete, so it uploads and revives the row instead of being removed. - Revive a tombstone with the same user and guid in cmdDeckUpload instead of inserting a second row next to it, so the deck keeps its id and version history and the promise in servatrice.ini.example holds. - Run tombstone purging on its own clock, driven by the new [deck_sync] purge_interval option, so disabling share cleanup no longer disables purging silently. - Write the base snapshot through QSaveFile and log a failed commit, so a crash or a full disk cannot leave a truncated base for DeckDiff::merge(). --- .../interface/deck_sync/deck_sync_manager.cpp | 23 ++++- servatrice/servatrice.ini.example | 12 ++- servatrice/src/servatrice.cpp | 17 +++- servatrice/src/servatrice.h | 2 + servatrice/src/serversocketinterface.cpp | 93 ++++++++++++++----- 5 files changed, 113 insertions(+), 34 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 09eabea6b..461041148 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -3,10 +3,12 @@ #include "../deck_loader/deck_loader.h" #include "deck_sync_sidecar.h" +#include #include #include #include #include +#include #include #include #include @@ -278,9 +280,15 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } } if (serverId == -1) { - if (sidecarRecord.remoteId != -1) { - // The server no longer lists this deck: it was deleted elsewhere. - // Apply the delete locally, keeping a backup of the file. + const DeckSyncRecord sidecarRecord = sidecar.get(it.key()); + // The sidecar knows a server row that the listing no longer has: it was + // deleted elsewhere. Only mirror that delete when the local file is + // still exactly what was synced - an edit made after the remote delete + // is newer than the delete and gets uploaded again. + const bool knownToServer = sidecarRecord.remoteId != -1 || !sidecarRecord.guid.isEmpty(); + const bool unchangedSinceSync = + !sidecarRecord.baseHash.isEmpty() && it.value().hash == sidecarRecord.baseHash; + if (knownToServer && unchangedSinceSync) { const QString absolutePath = deckPath + "/" + it.key(); const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); @@ -724,11 +732,16 @@ void DeckSyncManager::snapshotLocalFile(const QString &relativePath, const QStri return; } QDir().mkpath(deckPath + "/" + SYNC_META_DIR + "/bases"); - QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); - if (!snapshot.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + // QSaveFile: a crash or a full disk must not leave a truncated base behind, + // because DeckDiff::merge() against a partial base re-adds or drops cards. + QSaveFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::WriteOnly)) { return; } snapshot.write(source.readAll()); + if (!snapshot.commit()) { + qWarning() << "DeckSync: could not write the base snapshot for" << relativePath; + } } void DeckSyncManager::failSync(const QString &error) diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index 1ac2e67eb..82fae45ad 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -602,8 +602,14 @@ max_shares_per_day=50 max_versions=20 ; How many days a deleted deck stays restorable as a tombstone before the -; cleanup clock purges it (with its versions) for good. Re-uploading the -; same deck within this window resurrects it instead of creating a copy. -; A value of 0 purges tombstones on the next cleanup run. +; purge clock removes it (with its versions) for good. Re-uploading the +; same deck within this window revives the tombstone instead of creating a copy. +; A value of 0 purges tombstones on the next purge run. ; Default: 30 tombstone_days=30 + +; How often, in minutes, the tombstone purge runs. This is independent of +; [deck_share] cleanup_interval, so disabling share cleanup does not disable +; purging. A value of 0 disables purging entirely. +; Default: 60 +purge_interval=60 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index 15d8ee016..a87d0bffc 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -629,7 +629,6 @@ bool Servatrice::initServer() deckShareCleanupClock = new QTimer(this); connect(deckShareCleanupClock, &QTimer::timeout, this, &Servatrice::cleanupExpiredDeckShares); - connect(deckShareCleanupClock, &QTimer::timeout, this, &Servatrice::purgeDeckTombstones); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -640,6 +639,16 @@ bool Servatrice::initServer() connect(legacyBackupSweepClock, &QTimer::timeout, this, &Servatrice::sweepLegacyBackups); legacyBackupSweepClock->start(kLegacyBackupSweepIntervalMs); + // Tombstone purging runs on its own clock: turning share cleanup off must not + // silently disable it as well. + deckTombstoneClock = new QTimer(this); + connect(deckTombstoneClock, &QTimer::timeout, this, &Servatrice::purgeDeckTombstones); + const int deckSyncPurgeInterval = getDeckSyncPurgeInterval(); + if (deckSyncPurgeInterval > 0) { + qDebug() << "Starting deck tombstone purge clock, interval" << deckSyncPurgeInterval << "ms"; + deckTombstoneClock->start(deckSyncPurgeInterval); + } + // SOCKET SERVER if (getNumberOfTCPPools() > 0) { gameServer = @@ -1413,6 +1422,12 @@ int Servatrice::getDeckSyncTombstoneDays() const return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); } +int Servatrice::getDeckSyncPurgeInterval() const +{ + // default: every 60 minutes, independent of deck_share/cleanup_interval + return qMax(0, settingsCache->value("deck_sync/purge_interval", 60).toInt()) * 60000; +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index f925c0984..ed8d82ddb 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -182,6 +182,7 @@ private: QTimer *pingClock, *statusUpdateClock; QTimer *deckShareCleanupClock; QTimer *legacyBackupSweepClock; + QTimer *deckTombstoneClock; Servatrice_GameServer *gameServer; Servatrice_WebsocketGameServer *websocketGameServer; Servatrice_IslServer *islServer; @@ -323,6 +324,7 @@ public: int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; int getDeckSyncTombstoneDays() const; + int getDeckSyncPurgeInterval() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 7f0403a94..38acf2942 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1130,31 +1130,71 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman return Response::RespNameNotFound; } - QSqlQuery *query = - sqlInterface->prepareQuery("insert into {prefix}_decklist_files (id_folder, id_user, name, upload_time, " - "content, is_public, banner_card_name, banner_card_provider, color_identity, " - "tags, guid, hash, last_modified) values(:id_folder, :id_user, :name, NOW(), " - ":content, :is_public, :banner_card_name, :banner_card_provider, " - ":color_identity, :tags, :guid, :hash, NOW())"); - query->bindValue(":id_folder", folderId); - query->bindValue(":id_user", userInfo->id()); - query->bindValue(":name", deckName); - query->bindValue(":content", deckStr); - query->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); - query->bindValue(":banner_card_name", bannerCardName); - query->bindValue(":banner_card_provider", bannerCardProvider); - query->bindValue(":color_identity", colorIdentity); - query->bindValue(":tags", tagsJson); - query->bindValue(":guid", guid); - query->bindValue(":hash", deckHash); - if (!sqlInterface->execSqlQuery(query)) { + // A deck that was deleted elsewhere leaves a tombstone behind. Revive it + // instead of inserting a second row with the same guid, which would split + // the deck's version history and leave the tombstone to be purged. + int deckId = -1; + bool inserted = false; + QSqlQuery *tombstoneQuery = + sqlInterface->prepareQuery("select id from {prefix}_decklist_files where id_user = :id_user and guid = " + ":guid and deleted = 1 order by last_modified desc limit 1"); + tombstoneQuery->bindValue(":id_user", userInfo->id()); + tombstoneQuery->bindValue(":guid", guid); + if (!sqlInterface->execSqlQuery(tombstoneQuery)) { return Response::RespContextError; } - const int deckId = query->lastInsertId().toInt(); + if (tombstoneQuery->next()) { + deckId = tombstoneQuery->value(0).toInt(); + QSqlQuery *reviveQuery = sqlInterface->prepareQuery( + "update {prefix}_decklist_files set id_folder=:id_folder, name=:name, upload_time=NOW(), " + "content=:content, is_public=:is_public, banner_card_name=:banner_card_name, " + "banner_card_provider=:banner_card_provider, color_identity=:color_identity, tags=:tags, " + "hash=:hash, last_modified=NOW(), deleted=0 where id=:id and id_user=:id_user"); + reviveQuery->bindValue(":id_folder", folderId); + reviveQuery->bindValue(":id_user", userInfo->id()); + reviveQuery->bindValue(":id", deckId); + reviveQuery->bindValue(":name", deckName); + reviveQuery->bindValue(":content", deckStr); + reviveQuery->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); + reviveQuery->bindValue(":banner_card_name", bannerCardName); + reviveQuery->bindValue(":banner_card_provider", bannerCardProvider); + reviveQuery->bindValue(":color_identity", colorIdentity); + reviveQuery->bindValue(":tags", tagsJson); + reviveQuery->bindValue(":hash", deckHash); + if (!sqlInterface->execSqlQuery(reviveQuery)) { + return Response::RespContextError; + } + } else { + QSqlQuery *query = sqlInterface->prepareQuery( + "insert into {prefix}_decklist_files (id_folder, id_user, name, upload_time, " + "content, is_public, banner_card_name, banner_card_provider, color_identity, " + "tags, guid, hash, last_modified) values(:id_folder, :id_user, :name, NOW(), " + ":content, :is_public, :banner_card_name, :banner_card_provider, " + ":color_identity, :tags, :guid, :hash, NOW())"); + query->bindValue(":id_folder", folderId); + query->bindValue(":id_user", userInfo->id()); + query->bindValue(":name", deckName); + query->bindValue(":content", deckStr); + query->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0); + query->bindValue(":banner_card_name", bannerCardName); + query->bindValue(":banner_card_provider", bannerCardProvider); + query->bindValue(":color_identity", colorIdentity); + query->bindValue(":tags", tagsJson); + query->bindValue(":guid", guid); + query->bindValue(":hash", deckHash); + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespContextError; + } + deckId = query->lastInsertId().toInt(); + inserted = true; + } if (!storeDeckVersion(deckId, deckStr, deckHash)) { - QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - cleanupQuery->bindValue(":id", deckId); - sqlInterface->execSqlQuery(cleanupQuery); + if (inserted) { + QSqlQuery *cleanupQuery = + sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + } return Response::RespContextError; } @@ -1164,9 +1204,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman stampQuery->bindValue(":id", deckId); stampQuery->bindValue(":id_user", userInfo->id()); if (!sqlInterface->execSqlQuery(stampQuery) || !stampQuery->next()) { - QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - cleanupQuery->bindValue(":id", deckId); - sqlInterface->execSqlQuery(cleanupQuery); + if (inserted) { + QSqlQuery *cleanupQuery = + sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + } return Response::RespContextError; } From 34e28dfa220ebdaa83084bcd85b8842586941a09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Fri, 6 Nov 2026 09:15:28 +0100 Subject: [PATCH 4/6] [DeckSync] Address tombstone review nits: log category and designated initializer --- .../src/interface/deck_sync/deck_sync_manager.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 461041148..a3f383b56 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,8 @@ #include #include +inline Q_LOGGING_CATEGORY(DeckSyncLog, "deck_sync"); + namespace { constexpr const char *SYNC_META_DIR = ".sync"; @@ -525,11 +528,11 @@ void DeckSyncManager::onUploadFinished(const Response &response, if (response.response_code() == Response::RespNameNotFound && deckId != -1) { // The server row is gone (purged tombstone or removed elsewhere): // upload the deck as new instead of updating a dead id. - UploadOp retry; - retry.relativePath = relativePath; - retry.serverPath = context.value("serverPath").toString(); - retry.deckId = -1; - retry.guid = context.value("guid").toString(); + const UploadOp retry{.relativePath = relativePath, + .serverPath = context.value("serverPath").toString(), + .serverDir = {}, + .deckId = -1, + .guid = context.value("guid").toString()}; pendingUploads.prepend(retry); processNext(); return; @@ -740,7 +743,7 @@ void DeckSyncManager::snapshotLocalFile(const QString &relativePath, const QStri } snapshot.write(source.readAll()); if (!snapshot.commit()) { - qWarning() << "DeckSync: could not write the base snapshot for" << relativePath; + qCWarning(DeckSyncLog) << "DeckSync: could not write the base snapshot for" << relativePath; } } From 885b7f82ae792839d724781b79342ea1881a55ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Sun, 8 Nov 2026 14:42:33 +0100 Subject: [PATCH 5/6] Address review findings --- .../interface/deck_sync/deck_sync_manager.cpp | 67 +++++++++++++++++-- .../interface/deck_sync/deck_sync_manager.h | 9 ++- .../widgets/tabs/tab_deck_storage.cpp | 18 +++-- 3 files changed, 80 insertions(+), 14 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index a3f383b56..92fa1e132 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -118,8 +118,10 @@ void DeckSyncManager::syncNow() syncing = true; uploadedCount = 0; downloadedCount = 0; + removedCount = 0; unchangedCount = 0; skippedCount = 0; + syncNote.clear(); pendingUploads.clear(); pendingDownloads.clear(); pendingDeletes.clear(); @@ -215,6 +217,22 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + // A listing that comes back empty while the sidecar still remembers rows that + // were uploaded means the server lost its deck data (a purge, or an account + // that was re-created), not that every deck was just deleted. Obeying that + // listing would remove the whole collection, so removals are held back. + bool sidecarHasRemoteRows = false; + // The records as they were before this run starts mutating them; only used to + // judge whether an empty listing is credible. + const QMap sidecarAtStart = sidecar.records(); + for (auto rit = sidecarAtStart.begin(); rit != sidecarAtStart.end(); ++rit) { + if (rit.value().remoteId != -1) { + sidecarHasRemoteRows = true; + break; + } + } + const bool remoteStateLost = serverFiles.isEmpty() && sidecarHasRemoteRows; + bool heldBackRemovals = false; // Reuse the sidecar's guid so the server keeps recognizing the row, and mint // one only for a deck that has never been synced. auto guidFor = [&sidecar](const QString &relativePath) { @@ -292,6 +310,15 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const bool unchangedSinceSync = !sidecarRecord.baseHash.isEmpty() && it.value().hash == sidecarRecord.baseHash; if (knownToServer && unchangedSinceSync) { + if (remoteStateLost) { + // The listing is empty although this deck was uploaded, so the + // row is not known to be deleted, only missing. Keep the file + // and the record; re-uploading it blind could duplicate the + // deck once the real listing returns. + heldBackRemovals = true; + ++skippedCount; + continue; + } const QString absolutePath = deckPath + "/" + it.key(); const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); @@ -300,7 +327,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont continue; } sidecar.remove(it.key()); - ++downloadedCount; + ++removedCount; continue; } const QString serverPath = serverPathFor(it.key()); @@ -373,12 +400,30 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont continue; } if (it.value().remoteId == -1) { + // Never uploaded, so there is no server row to remove. sidecar.remove(it.key()); continue; } + if (matchedServerIds.contains(it.value().remoteId)) { + // Another local file (a rename or a copy) now owns this row: drop the + // stale record, but never tombstone the row it took over. + sidecar.remove(it.key()); + continue; + } + if (remoteStateLost) { + // The whole listing is missing, not this one deck: pushing the delete + // would purge rows that are probably still there. + heldBackRemovals = true; + continue; + } pendingDeletes.append(it.key()); } + if (heldBackRemovals) { + syncNote = tr("The server listed no decks although this collection was synced before. " + "No deletions were applied this run; the decks were kept."); + } + sidecar.save(); processNext(); } @@ -530,7 +575,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, // upload the deck as new instead of updating a dead id. const UploadOp retry{.relativePath = relativePath, .serverPath = context.value("serverPath").toString(), - .serverDir = {}, + .serverDir = parentDir(context.value("serverPath").toString()), .deckId = -1, .guid = context.value("guid").toString()}; pendingUploads.prepend(retry); @@ -719,6 +764,7 @@ void DeckSyncManager::onDeleteFinished(const Response &response, sidecar.load(); sidecar.remove(relativePath); sidecar.save(); + ++removedCount; } else { ++skippedCount; } @@ -757,11 +803,16 @@ void DeckSyncManager::finishSync() { syncing = false; const SyncSummary summary = currentSummary(); - emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") - .arg(summary.uploaded) - .arg(summary.downloaded) - .arg(summary.unchanged) - .arg(summary.skipped)); + QString message = tr("Sync complete: %1 uploaded, %2 downloaded, %3 removed, %4 unchanged, %5 skipped.") + .arg(summary.uploaded) + .arg(summary.downloaded) + .arg(summary.removed) + .arg(summary.unchanged) + .arg(summary.skipped); + if (!summary.note.isEmpty()) { + message += QStringLiteral(" ") + summary.note; + } + emit statusMessage(message); emit syncFinished(summary, QString()); } @@ -770,7 +821,9 @@ DeckSyncManager::SyncSummary DeckSyncManager::currentSummary() const return SyncSummary{ .uploaded = uploadedCount, .downloaded = downloadedCount, + .removed = removedCount, .unchanged = unchangedCount, .skipped = skippedCount, + .note = syncNote, }; } diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index b0020b33b..c5284d33a 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -41,7 +41,10 @@ class ServerInfo_DeckStorage_Folder; * * Deletes are tombstones, never silent: a locally deleted deck is pushed with * Command_DeckDel, a server-side delete is applied locally only after copying - * the file to a .sync-backup, and resurrection reuses the stored deck id. + * the file to a .sync-backup, and resurrection reuses the stored deck id. A + * listing that comes back empty while the sidecar still remembers uploaded rows + * is treated as lost server state rather than a collection-wide delete, so that + * run applies no removals and says so in its summary. */ class DeckSyncManager : public QObject { @@ -55,8 +58,10 @@ public: { int uploaded = 0; int downloaded = 0; + int removed = 0; ///< Decks deleted on the server, or locally to mirror a server delete. int unchanged = 0; int skipped = 0; + QString note; ///< Non-error caveat about this run, empty when there is none. }; explicit DeckSyncManager(QObject *parent = nullptr); @@ -149,8 +154,10 @@ private: QList pendingUploads; QList pendingDownloads; QList pendingDeletes; + QString syncNote; int uploadedCount = 0; int downloadedCount = 0; + int removedCount = 0; int unchangedCount = 0; int skippedCount = 0; }; diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index ef25d5c38..a2263e092 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -591,13 +591,19 @@ void TabDeckStorage::renderSyncStatus() case SyncStatusType::Failed: syncStatusLabel->setText(tr("Sync failed: %1").arg(syncStatusDetail)); break; - case SyncStatusType::Finished: - syncStatusLabel->setText(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") - .arg(lastSyncSummary.uploaded) - .arg(lastSyncSummary.downloaded) - .arg(lastSyncSummary.unchanged) - .arg(lastSyncSummary.skipped)); + case SyncStatusType::Finished: { + QString text = tr("Sync complete: %1 uploaded, %2 downloaded, %3 removed, %4 unchanged, %5 skipped.") + .arg(lastSyncSummary.uploaded) + .arg(lastSyncSummary.downloaded) + .arg(lastSyncSummary.removed) + .arg(lastSyncSummary.unchanged) + .arg(lastSyncSummary.skipped); + if (!lastSyncSummary.note.isEmpty()) { + text += QStringLiteral("\n") + lastSyncSummary.note; + } + syncStatusLabel->setText(text); break; + } case SyncStatusType::Idle: syncStatusLabel->setText(tr("Not synced yet")); break; From d0cd13fce751290e18d5cb178fc3d6856ab69e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <44814898+BruebachL@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:33:44 +0100 Subject: [PATCH 6/6] [DeckSync] Drop the queued download when a local delete wins The unmatched-server-rows pass queued a download for every row with no local file, including rows whose sidecar record is being deleted in the same run. The delete ran first, so the download then failed with NameNotFound and the summary reported a clean delete as removed plus skipped. Cancel the pending download for the deleted row id instead; a failed delete keeps it, which preserves the restore-the-file fallback. --- cockatrice/src/interface/deck_sync/deck_sync_manager.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 92fa1e132..560f238d6 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -762,8 +762,16 @@ void DeckSyncManager::onDeleteFinished(const Response &response, if (response.response_code() == Response::RespOk || response.response_code() == Response::RespNameNotFound) { DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); + const int deletedDeckId = sidecar.get(relativePath).remoteId; sidecar.remove(relativePath); sidecar.save(); + if (deletedDeckId != -1) { + // The unmatched-server-rows pass queued a download for this row before + // the delete ran. The delete won, so drop the download instead of + // letting it fail with NameNotFound and count as a skipped deck. A + // failed delete leaves the download in place as the fallback. + pendingDownloads.removeIf([deletedDeckId](const DownloadOp &op) { return op.deckId == deletedDeckId; }); + } ++removedCount; } else { ++skippedCount;