[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().

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 <QDebug>
 #include <QDir>
 #include <QDirIterator>
 #include <QFile>
 #include <QFileInfo>
+#include <QSaveFile>
 #include <QStringList>
 #include <QUuid>
 #include <libcockatrice/deck_list/deck_file_format.h>
@@ -257,9 +259,14 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont
             }
         }
         if (serverId == -1) {
-            if (sidecarRecord.remoteId != -1) {
+            // The sidecar knows a server row that the listing no longer has: it was
-                // The server no longer lists this deck: it was deleted elsewhere.
+            // deleted elsewhere. Only mirror that delete when the local file is
-                // Apply the delete locally, keeping a backup of the file.
+            // 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());
@@ -678,11 +685,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");
+    // QSaveFile: a crash or a full disk must not leave a truncated base behind,
-    if (!snapshot.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
+    // 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)

servatrice/servatrice.ini.example

@@ -600,8 +600,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
+; purge clock removes it (with its versions) for good. Re-uploading the
-; same deck within this window resurrects it instead of creating a copy.
+; same deck within this window revives the tombstone instead of creating a copy.
-; A value of 0 purges tombstones on the next cleanup run.
+; 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

servatrice/src/servatrice.cpp

@@ -622,7 +622,6 @@ bool Servatrice::initServer()
 
     deckShareCleanupClock = new QTimer(this);
     connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(cleanupExpiredDeckShares()));
-    connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones()));
     const int deckShareCleanupInterval = getDeckShareCleanupInterval();
     if (deckShareCleanupInterval > 0) {
         qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms";
@@ -633,6 +632,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, SIGNAL(timeout()), this, SLOT(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 =
@@ -1412,6 +1421,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();

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;

servatrice/src/serversocketinterface.cpp

@@ -1129,31 +1129,71 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman
             return Response::RespNameNotFound;
         }
 
-        QSqlQuery *query =
+        // A deck that was deleted elsewhere leaves a tombstone behind. Revive it
-            sqlInterface->prepareQuery("insert into {prefix}_decklist_files (id_folder, id_user, name, upload_time, "
+        // instead of inserting a second row with the same guid, which would split
-                                       "content, is_public, banner_card_name, banner_card_provider, color_identity, "
+        // the deck's version history and leave the tombstone to be purged.
-                                       "tags, guid, hash, last_modified) values(:id_folder, :id_user, :name, NOW(), "
+        int deckId = -1;
-                                       ":content, :is_public, :banner_card_name, :banner_card_provider, "
+        bool inserted = false;
-                                       ":color_identity, :tags, :guid, :hash, NOW())");
+        QSqlQuery *tombstoneQuery =
-        query->bindValue(":id_folder", folderId);
+            sqlInterface->prepareQuery("select id from {prefix}_decklist_files where id_user = :id_user and guid = "
-        query->bindValue(":id_user", userInfo->id());
+                                       ":guid and deleted = 1 order by last_modified desc limit 1");
-        query->bindValue(":name", deckName);
+        tombstoneQuery->bindValue(":id_user", userInfo->id());
-        query->bindValue(":content", deckStr);
+        tombstoneQuery->bindValue(":guid", guid);
-        query->bindValue(":is_public", cmd.has_is_public() && cmd.is_public() ? 1 : 0);
+        if (!sqlInterface->execSqlQuery(tombstoneQuery)) {
-        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;
         }
-        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");
+            if (inserted) {
-            cleanupQuery->bindValue(":id", deckId);
+                QSqlQuery *cleanupQuery =
-            sqlInterface->execSqlQuery(cleanupQuery);
+                    sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id");
+                cleanupQuery->bindValue(":id", deckId);
+                sqlInterface->execSqlQuery(cleanupQuery);
+            }
             return Response::RespContextError;
         }
 
@@ -1163,9 +1203,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");
+            if (inserted) {
-            cleanupQuery->bindValue(":id", deckId);
+                QSqlQuery *cleanupQuery =
-            sqlInterface->execSqlQuery(cleanupQuery);
+                    sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id");
+                cleanupQuery->bindValue(":id", deckId);
+                sqlInterface->execSqlQuery(cleanupQuery);
+            }
             return Response::RespContextError;
         }