From 10d46bafbdbc86c1f12afafa12e8d3290d60a857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:09:05 +0200 Subject: [PATCH 01/16] [Server] Deck sync identity: schema v38 + proto guid/hash fields --- .../protocol/pb/command_deck_upload.proto | 3 ++ .../protocol/pb/response_deck_download.proto | 5 +++ .../protocol/pb/serverinfo_deckstorage.proto | 9 ++++++ .../migrations/servatrice_0037_to_0038.sql | 32 +++++++++++++++++++ servatrice/servatrice.sql | 21 +++++++++++- .../src/servatrice_database_interface.h | 2 +- 6 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 servatrice/migrations/servatrice_0037_to_0038.sql diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_upload.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_upload.proto index 1a5d44e9e..89ad94899 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_upload.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_upload.proto @@ -14,4 +14,7 @@ message Command_DeckUpload { reserved 5, 6, 8; reserved "banner_card_name", "banner_card_provider", "tags"; optional string color_identity = 7; + // Stable per-deck identity for two-way collection sync. Sent on first + // synced upload, echoed back in ServerInfo_DeckStorage_File afterwards. + optional string guid = 9; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_download.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_download.proto index f1839a9e2..11a72763a 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_download.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_download.proto @@ -6,4 +6,9 @@ message Response_DeckDownload { optional Response_DeckDownload ext = 1007; } optional string deck = 1; + // Sync identity echoed so the client can update its sidecar without an + // extra list fetch. + optional string guid = 2; + optional string hash = 3; + optional uint64 last_modified = 4; } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_deckstorage.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_deckstorage.proto index b04d676d8..04dd37cf5 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_deckstorage.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/serverinfo_deckstorage.proto @@ -11,6 +11,15 @@ message ServerInfo_DeckStorage_File { // Tag names associated with the deck. Empty for decks uploaded before the // tags column existed. repeated string tags = 6; + // Stable per-deck identity for two-way collection sync. Empty for decks + // uploaded before the sync migration until they are re-uploaded. + optional string guid = 7; + // Content hash (DeckList::getDeckHash) for change detection without + // downloading the full list. + optional string hash = 8; + // Last modification time, seconds since Unix epoch. Drives newest-wins + // default and conflict-dialog recency highlight. + optional uint64 last_modified = 9; } message ServerInfo_DeckStorage_Folder { diff --git a/servatrice/migrations/servatrice_0037_to_0038.sql b/servatrice/migrations/servatrice_0037_to_0038.sql new file mode 100644 index 000000000..c3c823f9c --- /dev/null +++ b/servatrice/migrations/servatrice_0037_to_0038.sql @@ -0,0 +1,32 @@ +-- Servatrice db migration from version 37 to version 38 + +-- Deck collection sync + versioning. +-- +-- 1. Stable per-deck identity and change detection for two-way sync: +-- guid links a local file to its server row across renames, hash detects +-- content changes without downloading, last_modified drives newest-wins +-- default and conflict highlighting, deleted implements tombstones so a +-- delete on one device does not resurrect from the other. +-- 2. Limited per-deck version history: every hash-changing upload inserts a +-- row; the server prunes beyond the configured retention (default 20). + +ALTER TABLE `cockatrice_decklist_files` + ADD COLUMN `guid` char(36) NULL AFTER `tags`, + ADD COLUMN `hash` char(40) NULL AFTER `guid`, + ADD COLUMN `last_modified` datetime NULL AFTER `hash`, + ADD COLUMN `deleted` tinyint(1) NOT NULL DEFAULT 0 AFTER `last_modified`; + +CREATE TABLE IF NOT EXISTS `cockatrice_deck_versions` ( + `id` int(7) unsigned zerofill NOT NULL auto_increment, + `deck_id` int(7) unsigned zerofill NOT NULL, + `version_no` int(7) unsigned NOT NULL, + `content` text NOT NULL, + `hash` char(40) NULL, + `created_at` datetime NOT NULL, + `created_by_clientid` varchar(15) NULL, + PRIMARY KEY (`id`), + KEY `deck_id` (`deck_id`), + FOREIGN KEY(`deck_id`) REFERENCES `cockatrice_decklist_files`(`id`) ON DELETE CASCADE ON UPDATE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; + +UPDATE cockatrice_schema_version SET version=38 WHERE version=37; diff --git a/servatrice/servatrice.sql b/servatrice/servatrice.sql index 4fbc1d8bd..0143ee6bd 100644 --- a/servatrice/servatrice.sql +++ b/servatrice/servatrice.sql @@ -20,7 +20,7 @@ CREATE TABLE IF NOT EXISTS `cockatrice_schema_version` ( PRIMARY KEY (`version`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; -INSERT INTO cockatrice_schema_version VALUES(37); +INSERT INTO cockatrice_schema_version VALUES(38); -- users and user data tables CREATE TABLE IF NOT EXISTS `cockatrice_users` ( @@ -71,11 +71,30 @@ CREATE TABLE IF NOT EXISTS `cockatrice_decklist_files` ( `banner_card_provider` varchar(32) NULL, `color_identity` varchar(5) NULL, `tags` text NULL, + `guid` char(36) NULL, + `hash` char(40) NULL, + `last_modified` datetime NULL, + `deleted` tinyint(1) NOT NULL DEFAULT 0, PRIMARY KEY (`id`), KEY `FolderPlusUser` (`id_folder`,`id_user`), FOREIGN KEY(`id_user`) REFERENCES `cockatrice_users`(`id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; +-- Per-deck version history for collection sync. +-- One row per hash-changing upload; pruned beyond retention (default 20). +CREATE TABLE IF NOT EXISTS `cockatrice_deck_versions` ( + `id` int(7) unsigned zerofill NOT NULL auto_increment, + `deck_id` int(7) unsigned zerofill NOT NULL, + `version_no` int(7) unsigned NOT NULL, + `content` text NOT NULL, + `hash` char(40) NULL, + `created_at` datetime NOT NULL, + `created_by_clientid` varchar(15) NULL, + PRIMARY KEY (`id`), + KEY `deck_id` (`deck_id`), + FOREIGN KEY(`deck_id`) REFERENCES `cockatrice_decklist_files`(`id`) ON DELETE CASCADE ON UPDATE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE utf8mb4_unicode_ci; + -- Temporary deck shares: a named bundle of decks that can be fetched by -- anyone who knows the (unguessable) token, until the share expires. CREATE TABLE IF NOT EXISTS `cockatrice_deck_share` ( diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 0509d1a03..ee8074171 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -21,7 +21,7 @@ class QSqlQuery; -#define DATABASE_SCHEMA_VERSION 37 +#define DATABASE_SCHEMA_VERSION 38 class Servatrice; From a03f4c2fa4669156f7596ff6b460fae0555ac389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:16:30 +0200 Subject: [PATCH 02/16] [Server] Persist and serve deck sync fields + version rows --- servatrice/servatrice.ini.example | 9 ++ servatrice/src/servatrice.cpp | 5 + servatrice/src/servatrice.h | 1 + servatrice/src/serversocketinterface.cpp | 160 ++++++++++++++++++++--- servatrice/src/serversocketinterface.h | 1 + 5 files changed, 160 insertions(+), 16 deletions(-) diff --git a/servatrice/servatrice.ini.example b/servatrice/servatrice.ini.example index c625705da..209f616f7 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -479,3 +479,12 @@ max_decks_per_share=50 ; A value of 0 disables the limit. ; Default: 50 max_shares_per_day=50 + + +[deck_sync] + +; Maximum number of stored versions kept per deck. Each upload with changed +; content adds a version; older versions beyond this limit are pruned. +; Minimum enforced: 1. +; Default: 20 +max_versions=20 diff --git a/servatrice/src/servatrice.cpp b/servatrice/src/servatrice.cpp index 0329a31fd..a979a74b2 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -1122,6 +1122,11 @@ int Servatrice::getDeckShareMaxSharesPerDay() const return settingsCache->value("deck_share/max_shares_per_day", 50).toInt(); } +int Servatrice::getDeckSyncMaxVersions() const +{ + return qMax(1, settingsCache->value("deck_sync/max_versions", 20).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 242d445ad..a6bbacfc6 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -292,6 +292,7 @@ public: int getDeckShareCleanupInterval() const; int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; + int getDeckSyncMaxVersions() const; int getMaxUsersPerAddress() const; int getMessageCountingInterval() const override; int getMaxMessageCountPerInterval() const override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index d46a9b0ee..4c58748a6 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -70,6 +70,7 @@ #include #include #include +#include #include #include #include @@ -681,8 +682,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 from {prefix}_decklist_files where " - "id_folder = :id_folder and id_user = :id_user"); + "banner_card_provider, color_identity, tags, guid, hash, last_modified from " + "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user"); query->bindValue(":id_folder", folderId); query->bindValue(":id_user", userId); if (!sqlInterface->execSqlQuery(query)) { @@ -708,6 +709,12 @@ bool AbstractServerSocketInterface::deckListHelper(int folderId, for (const QString &tag : deserializeDeckTags(query->value(7).toString())) { newFile->add_tags(tag.toStdString()); } + newFile->set_guid(query->value(8).toString().toStdString()); + newFile->set_hash(query->value(9).toString().toStdString()); + const QDateTime lastModified = query->value(10).toDateTime(); + if (lastModified.isValid()) { + newFile->set_last_modified(lastModified.toSecsSinceEpoch()); + } } return true; @@ -1014,6 +1021,38 @@ QString sanitizeColorIdentity(const QString &colorIdentity) } } // namespace +bool AbstractServerSocketInterface::storeDeckVersion(int deckId, const QString &content, const QString &hash) +{ + QSqlQuery *maxQuery = + sqlInterface->prepareQuery("select coalesce(max(version_no), 0) from {prefix}_deck_versions where deck_id = " + ":deck_id"); + maxQuery->bindValue(":deck_id", deckId); + if (!sqlInterface->execSqlQuery(maxQuery) || !maxQuery->next()) { + return false; + } + const int nextVersion = maxQuery->value(0).toInt() + 1; + + QSqlQuery *insertQuery = + sqlInterface->prepareQuery("insert into {prefix}_deck_versions (deck_id, version_no, content, hash, " + "created_at, created_by_clientid) values(:deck_id, :version_no, :content, :hash, " + "NOW(), :created_by_clientid)"); + insertQuery->bindValue(":deck_id", deckId); + insertQuery->bindValue(":version_no", nextVersion); + insertQuery->bindValue(":content", content); + insertQuery->bindValue(":hash", hash); + insertQuery->bindValue(":created_by_clientid", QString::fromStdString(userInfo->clientid())); + if (!sqlInterface->execSqlQuery(insertQuery)) { + return false; + } + + QSqlQuery *pruneQuery = sqlInterface->prepareQuery("delete from {prefix}_deck_versions where deck_id = :deck_id " + "and version_no <= :min_kept_version"); + pruneQuery->bindValue(":deck_id", deckId); + pruneQuery->bindValue(":min_kept_version", nextVersion - servatrice->getDeckSyncMaxVersions()); + sqlInterface->execSqlQuery(pruneQuery); + return true; +} + Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Command_DeckUpload &cmd, ResponseContainer &rc) { @@ -1049,6 +1088,11 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman const QString bannerCardProvider = deck.getBannerCard().providerId.left(32); const QString tagsJson = serializeDeckTags(deck.getTags()); const QString colorIdentity = sanitizeColorIdentity(nameFromStdString(cmd.color_identity())); + const QString deckHash = deck.getDeckHash(); + QString guid = cmd.has_guid() ? nameFromStdString(cmd.guid()).left(36) : QString(); + if (QUuid::fromString(guid).isNull()) { + guid = QUuid::createUuid().toString(QUuid::WithoutBraces); + } if (cmd.has_path()) { int folderId = getDeckPathId(nameFromStdString(cmd.path())); @@ -1059,8 +1103,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman 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) values(:id_folder, :id_user, :name, NOW(), :content, :is_public, " - ":banner_card_name, :banner_card_provider, :color_identity, :tags)"); + "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); @@ -1070,21 +1115,63 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman 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 (!storeDeckVersion(deckId, deckStr, deckHash)) { + QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + return Response::RespContextError; + } + + QSqlQuery *stampQuery = + sqlInterface->prepareQuery("select unix_timestamp(upload_time), unix_timestamp(last_modified) from " + "{prefix}_decklist_files where id = :id and id_user = :id_user"); + 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); + return Response::RespContextError; + } Response_DeckUpload *re = new Response_DeckUpload; ServerInfo_DeckStorage_TreeItem *fileInfo = re->mutable_new_file(); - fileInfo->set_id(query->lastInsertId().toInt()); + fileInfo->set_id(deckId); fileInfo->set_name(deckName.toStdString()); - fileInfo->mutable_file()->set_creation_time(QDateTime::currentDateTime().toSecsSinceEpoch()); + fileInfo->mutable_file()->set_creation_time(stampQuery->value(0).toUInt()); fileInfo->mutable_file()->set_is_public(cmd.has_is_public() && cmd.is_public()); + fileInfo->mutable_file()->set_guid(guid.toStdString()); + fileInfo->mutable_file()->set_hash(deckHash.toStdString()); + fileInfo->mutable_file()->set_last_modified(stampQuery->value(1).toULongLong()); rc.setResponseExtension(re); } else if (cmd.has_deck_id()) { + QSqlQuery *currentQuery = + sqlInterface->prepareQuery("select guid, hash from {prefix}_decklist_files where id = :id and id_user = " + ":id_user"); + currentQuery->bindValue(":id", cmd.deck_id()); + currentQuery->bindValue(":id_user", userInfo->id()); + if (!sqlInterface->execSqlQuery(currentQuery)) { + return Response::RespContextError; + } + if (!currentQuery->next()) { + return Response::RespNameNotFound; + } + if (!QUuid::fromString(currentQuery->value(0).toString()).isNull()) { + guid = currentQuery->value(0).toString(); + } else if (cmd.has_guid() && !QUuid::fromString(nameFromStdString(cmd.guid()).left(36)).isNull()) { + guid = nameFromStdString(cmd.guid()).left(36); + } + 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"; + "color_identity=:color_identity, tags=:tags, guid=:guid, hash=:hash, last_modified=NOW()"; if (cmd.has_is_public()) { updateQuery += ", is_public=:is_public"; } @@ -1099,33 +1186,56 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman query->bindValue(":banner_card_provider", bannerCardProvider); query->bindValue(":color_identity", colorIdentity); query->bindValue(":tags", tagsJson); + query->bindValue(":guid", guid); + query->bindValue(":hash", deckHash); if (cmd.has_is_public()) { query->bindValue(":is_public", cmd.is_public() ? 1 : 0); } if (!sqlInterface->execSqlQuery(query)) { return Response::RespContextError; } - if (query->numRowsAffected() == 0) { return Response::RespNameNotFound; } + if (oldHash != deckHash) { + if (!storeDeckVersion(cmd.deck_id(), deckStr, deckHash)) { + return Response::RespContextError; + } + } else { + // Self-heal a version gap left by an earlier failed version write: + // a same-content re-upload must still leave a version behind. + QSqlQuery *versionQuery = + sqlInterface->prepareQuery("select count(*) from {prefix}_deck_versions where deck_id = :deck_id and " + "hash = :hash"); + versionQuery->bindValue(":deck_id", cmd.deck_id()); + versionQuery->bindValue(":hash", deckHash); + if (!sqlInterface->execSqlQuery(versionQuery) || !versionQuery->next()) { + return Response::RespContextError; + } + if (versionQuery->value(0).toInt() == 0 && !storeDeckVersion(cmd.deck_id(), deckStr, deckHash)) { + return Response::RespContextError; + } + } + QSqlQuery *visibilityQuery = - sqlInterface->prepareQuery("select is_public from {prefix}_decklist_files where id = :id and " - "id_user = :id_user"); + sqlInterface->prepareQuery("select is_public, unix_timestamp(upload_time), unix_timestamp(last_modified) " + "from {prefix}_decklist_files where id = :id and id_user = :id_user"); visibilityQuery->bindValue(":id", cmd.deck_id()); visibilityQuery->bindValue(":id_user", userInfo->id()); - if (!sqlInterface->execSqlQuery(visibilityQuery)) { + if (!sqlInterface->execSqlQuery(visibilityQuery) || !visibilityQuery->next()) { return Response::RespContextError; } - const bool isPublic = visibilityQuery->next() && visibilityQuery->value(0).toBool(); Response_DeckUpload *re = new Response_DeckUpload; ServerInfo_DeckStorage_TreeItem *fileInfo = re->mutable_new_file(); fileInfo->set_id(cmd.deck_id()); fileInfo->set_name(deckName.toStdString()); - fileInfo->mutable_file()->set_creation_time(QDateTime::currentDateTime().toSecsSinceEpoch()); - fileInfo->mutable_file()->set_is_public(isPublic); + fileInfo->mutable_file()->set_creation_time(visibilityQuery->value(1).toUInt()); + fileInfo->mutable_file()->set_is_public(visibilityQuery->value(0).toBool()); + fileInfo->mutable_file()->set_guid(guid.toStdString()); + fileInfo->mutable_file()->set_hash(deckHash.toStdString()); + fileInfo->mutable_file()->set_last_modified(visibilityQuery->value(2).toULongLong()); rc.setResponseExtension(re); } else { return Response::RespInvalidData; @@ -1147,11 +1257,29 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckDownload(const Comm } catch (Response::ResponseCode &r) { return r; } + const QString deckNative = deck->writeToString_Native(); + delete deck; + + QSqlQuery *syncQuery = + sqlInterface->prepareQuery("select guid, hash, last_modified from {prefix}_decklist_files where id = :id and " + "id_user = :id_user"); + syncQuery->bindValue(":id", cmd.deck_id()); + syncQuery->bindValue(":id_user", userInfo->id()); + if (!sqlInterface->execSqlQuery(syncQuery)) { + return Response::RespContextError; + } Response_DeckDownload *re = new Response_DeckDownload; - re->set_deck(deck->writeToString_Native().toStdString()); + re->set_deck(deckNative.toStdString()); + if (syncQuery->next()) { + re->set_guid(syncQuery->value(0).toString().toStdString()); + re->set_hash(syncQuery->value(1).toString().toStdString()); + const QDateTime lastModified = syncQuery->value(2).toDateTime(); + if (lastModified.isValid()) { + re->set_last_modified(lastModified.toSecsSinceEpoch()); + } + } rc.setResponseExtension(re); - delete deck; return Response::RespOk; } diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index 27708ed27..6f000d478 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -154,6 +154,7 @@ private: Response::ResponseCode cmdDeckDelDir(const Command_DeckDelDir &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckDel(const Command_DeckDel &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckUpload(const Command_DeckUpload &cmd, ResponseContainer &rc); + bool storeDeckVersion(int deckId, const QString &content, const QString &hash); DeckList *getDeckFromDatabase(int deckId); Response::ResponseCode cmdDeckDownload(const Command_DeckDownload &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc); From 8f09048bce091724f3a381c9cacdf505783c64a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:26:12 +0200 Subject: [PATCH 03/16] [Client] DeckSyncProfile + sidecar + DeckDiff with tests --- cockatrice/CMakeLists.txt | 2 + .../interface/deck_sync/deck_sync_profile.cpp | 31 ++++ .../interface/deck_sync/deck_sync_profile.h | 43 ++++++ .../interface/deck_sync/deck_sync_sidecar.cpp | 98 +++++++++++++ .../interface/deck_sync/deck_sync_sidecar.h | 52 +++++++ libcockatrice_deck_list/CMakeLists.txt | 2 + .../libcockatrice/deck_list/deck_diff.cpp | 89 ++++++++++++ .../libcockatrice/deck_list/deck_diff.h | 71 +++++++++ tests/CMakeLists.txt | 23 +++ tests/deck_diff_test.cpp | 137 ++++++++++++++++++ tests/deck_sync_profile_test.cpp | 63 ++++++++ tests/deck_sync_sidecar_test.cpp | 126 ++++++++++++++++ 12 files changed, 737 insertions(+) create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_profile.cpp create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_profile.h create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_sidecar.h create mode 100644 libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp create mode 100644 libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h create mode 100644 tests/deck_diff_test.cpp create mode 100644 tests/deck_sync_profile_test.cpp create mode 100644 tests/deck_sync_sidecar_test.cpp diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 32b81cf61..9212a2902 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -27,6 +27,8 @@ set(cockatrice_SOURCES src/interface/deck_loader/deck_file_format.cpp src/interface/deck_loader/deck_loader.cpp src/interface/deck_loader/loaded_deck.cpp + src/interface/deck_sync/deck_sync_profile.cpp + src/interface/deck_sync/deck_sync_sidecar.cpp src/interface/widgets/dialogs/dlg_connect.cpp src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp src/interface/widgets/dialogs/dlg_create_game.cpp diff --git a/cockatrice/src/interface/deck_sync/deck_sync_profile.cpp b/cockatrice/src/interface/deck_sync/deck_sync_profile.cpp new file mode 100644 index 000000000..a886128a7 --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_profile.cpp @@ -0,0 +1,31 @@ +#include "deck_sync_profile.h" + +#include + +DeckSyncProfile::DeckSyncProfile(const QString &_host, int _port, const QString &_username) + : host(_host), port(_port), username(_username) +{ +} + +bool DeckSyncProfile::isValid() const +{ + return !host.isEmpty() && port > 0 && !username.isEmpty(); +} + +QString DeckSyncProfile::key() const +{ + if (!isValid()) { + return {}; + } + // NUL separators keep the triple injective: neither IPv6 hosts nor + // usernames containing ':' can collide two profiles into one file. + // Lowercasing is safe on both parts: DNS names are case-insensitive, and + // Servatrice usernames are unique under a case-insensitive collation + // (cockatrice_users uses utf8mb4_unicode_ci), so both spellings are one account. + QByteArray blob = host.trimmed().toLower().toUtf8(); + blob.append('\0'); + blob.append(QByteArray::number(port)); + blob.append('\0'); + blob.append(username.trimmed().toLower().toUtf8()); + return QCryptographicHash::hash(blob, QCryptographicHash::Sha1).toHex(); +} diff --git a/cockatrice/src/interface/deck_sync/deck_sync_profile.h b/cockatrice/src/interface/deck_sync/deck_sync_profile.h new file mode 100644 index 000000000..dff61a943 --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_profile.h @@ -0,0 +1,43 @@ +/** + * @file deck_sync_profile.h + * @brief Identity of one sync target (server + user) for deck collection sync. + */ + +#ifndef DECK_SYNC_PROFILE_H +#define DECK_SYNC_PROFILE_H + +#include + +/** + * @class DeckSyncProfile + * @brief Identifies which server and login a sync sidecar belongs to. + * + * A local deck folder can sync against several servers. Every server login + * gets its own profile, and every profile gets its own sidecar file, so + * switching servers never mixes up remote deck ids. + */ +class DeckSyncProfile +{ +public: + QString host; ///< Server hostname as entered by the user. + int port = 0; ///< Server port. + QString username; ///< Login name on that server. + + DeckSyncProfile() = default; + DeckSyncProfile(const QString &_host, int _port, const QString &_username); + + /** @brief False when any of host, port or username is missing. */ + [[nodiscard]] bool isValid() const; + /** + * @brief Stable file-system-safe key for this profile. + * + * SHA1 hex over the NUL-separated lowercased "host", port and "username" + * triple, so the same server reached with different hostname + * capitalisation maps to one file, while IPv6 hosts and usernames + * containing ':' can never collide two profiles. Empty for invalid + * profiles, so no two half-configured profiles share a file. + */ + [[nodiscard]] QString key() const; +}; + +#endif // DECK_SYNC_PROFILE_H diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp new file mode 100644 index 000000000..861149464 --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -0,0 +1,98 @@ +#include "deck_sync_sidecar.h" + +#include +#include +#include +#include +#include + +namespace +{ +constexpr int SIDECAR_FORMAT_VERSION = 1; +} // namespace + +DeckSyncSidecar::DeckSyncSidecar(const QString &_filePath) : filePath(_filePath) +{ +} + +bool DeckSyncSidecar::load() +{ + recordMap.clear(); + + QFile file(filePath); + if (!file.exists()) { + return true; + } + if (!file.open(QIODevice::ReadOnly)) { + return false; + } + const QJsonDocument document = QJsonDocument::fromJson(file.readAll()); + if (!document.isObject() || !document.object().value("records").isObject()) { + return false; + } + const QJsonObject records = document.object().value("records").toObject(); + for (auto it = records.begin(); it != records.end(); ++it) { + if (!it.value().isObject()) { + continue; + } + const QJsonObject entry = it.value().toObject(); + DeckSyncRecord record; + record.guid = entry.value("guid").toString(); + record.remoteId = entry.value("remoteId").toInt(-1); + record.baseHash = entry.value("baseHash").toString(); + record.lastSyncSecs = static_cast(entry.value("lastSync").toDouble()); + record.deleted = entry.value("deleted").toBool(false); + recordMap.insert(it.key(), record); + } + return true; +} + +bool DeckSyncSidecar::save() const +{ + QJsonObject records; + for (auto it = recordMap.begin(); it != recordMap.end(); ++it) { + QJsonObject entry; + entry.insert("guid", it.value().guid); + entry.insert("remoteId", it.value().remoteId); + entry.insert("baseHash", it.value().baseHash); + entry.insert("lastSync", static_cast(it.value().lastSyncSecs)); + entry.insert("deleted", it.value().deleted); + records.insert(it.key(), entry); + } + QJsonObject root; + root.insert("version", SIDECAR_FORMAT_VERSION); + root.insert("records", records); + + QDir().mkpath(QFileInfo(filePath).absolutePath()); + QFile file(filePath); + if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + return false; + } + file.write(QJsonDocument(root).toJson(QJsonDocument::Compact)); + return true; +} + +bool DeckSyncSidecar::contains(const QString &relativePath) const +{ + return recordMap.contains(relativePath); +} + +DeckSyncRecord DeckSyncSidecar::get(const QString &relativePath) const +{ + return recordMap.value(relativePath, DeckSyncRecord()); +} + +void DeckSyncSidecar::set(const QString &relativePath, const DeckSyncRecord &record) +{ + recordMap.insert(relativePath, record); +} + +void DeckSyncSidecar::remove(const QString &relativePath) +{ + recordMap.remove(relativePath); +} + +QMap DeckSyncSidecar::records() const +{ + return recordMap; +} diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h new file mode 100644 index 000000000..7eb2db7ae --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h @@ -0,0 +1,52 @@ +/** + * @file deck_sync_sidecar.h + * @brief Per-profile local bookkeeping for deck collection sync. + */ + +#ifndef DECK_SYNC_SIDECAR_H +#define DECK_SYNC_SIDECAR_H + +#include +#include + +/** @brief Sync bookkeeping for one local deck file. */ +struct DeckSyncRecord +{ + QString guid; ///< Stable deck identity shared with the server. + int remoteId = -1; ///< Server row id, -1 when never uploaded. + QString baseHash; ///< Deck hash at the last successful sync. + qint64 lastSyncSecs = 0; ///< Last sync time, seconds since Unix epoch. + bool deleted = false; ///< Local tombstone waiting to be pushed. +}; + +/** + * @class DeckSyncSidecar + * @brief Loads and stores sync records as JSON next to the deck folder. + * + * One sidecar file per DeckSyncProfile (named by profile key), mapping paths + * relative to the deck folder to their last-synced state. Comparing the live + * file hash against baseHash tells clean apart from locally modified. + */ +class DeckSyncSidecar +{ +public: + QString filePath; ///< Full path of the sidecar JSON file. + + explicit DeckSyncSidecar(const QString &_filePath); + + /** @brief Loads records; a missing file loads as empty and returns true. */ + bool load(); + /** @brief Stores records, creating parent folders as needed. */ + bool save() const; + [[nodiscard]] bool contains(const QString &relativePath) const; + /** @brief Returns the record, or a default record when unknown. */ + [[nodiscard]] DeckSyncRecord get(const QString &relativePath) const; + void set(const QString &relativePath, const DeckSyncRecord &record); + void remove(const QString &relativePath); + [[nodiscard]] QMap records() const; + +private: + QMap recordMap; +}; + +#endif // DECK_SYNC_SIDECAR_H diff --git a/libcockatrice_deck_list/CMakeLists.txt b/libcockatrice_deck_list/CMakeLists.txt index 0c487466c..8b7be3bd0 100644 --- a/libcockatrice_deck_list/CMakeLists.txt +++ b/libcockatrice_deck_list/CMakeLists.txt @@ -7,6 +7,7 @@ set(HEADERS libcockatrice/deck_list/tree/abstract_deck_list_node.h libcockatrice/deck_list/tree/deck_list_card_node.h libcockatrice/deck_list/tree/inner_deck_list_node.h + libcockatrice/deck_list/deck_diff.h libcockatrice/deck_list/deck_list.h libcockatrice/deck_list/deck_list_history_manager.h libcockatrice/deck_list/deck_list_node_tree.h @@ -25,6 +26,7 @@ add_library( libcockatrice/deck_list/tree/abstract_deck_list_node.cpp libcockatrice/deck_list/tree/deck_list_card_node.cpp libcockatrice/deck_list/tree/inner_deck_list_node.cpp + libcockatrice/deck_list/deck_diff.cpp libcockatrice/deck_list/deck_list.cpp libcockatrice/deck_list/deck_list_history_manager.cpp libcockatrice/deck_list/deck_list_node_tree.cpp diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp new file mode 100644 index 000000000..31a6b622b --- /dev/null +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp @@ -0,0 +1,89 @@ +#include "deck_diff.h" + +#include "deck_list.h" +#include "tree/deck_list_card_node.h" +#include "tree/inner_deck_list_node.h" + +#include +#include +#include +#include + +namespace +{ +using ZoneCardKey = QPair; + +QMap zoneCardCounts(const DeckList &deck) +{ + QMap counts; + deck.forEachCard([&counts](InnerDecklistNode *zone, DecklistCardNode *card) { + counts[{zone->getName(), card->getName()}] += card->getNumber(); + }); + return counts; +} + +bool changeLessThan(const DeckCardChange &a, const DeckCardChange &b) +{ + if (a.zone != b.zone) { + return a.zone < b.zone; + } + return a.cardName < b.cardName; +} +} // namespace + +DeckDiffResult DeckDiff::diff(const DeckList &oldDeck, const DeckList &newDeck) +{ + const QMap oldCounts = zoneCardCounts(oldDeck); + const QMap newCounts = zoneCardCounts(newDeck); + + QSet keys(oldCounts.keyBegin(), oldCounts.keyEnd()); + keys.unite(QSet(newCounts.keyBegin(), newCounts.keyEnd())); + + DeckDiffResult result; + for (const ZoneCardKey &key : keys) { + const int oldCount = oldCounts.value(key, 0); + const int newCount = newCounts.value(key, 0); + if (oldCount != newCount) { + result.changes.append({key.first, key.second, oldCount, newCount}); + } + } + std::sort(result.changes.begin(), result.changes.end(), changeLessThan); + return result; +} + +bool DeckDiff::canAutoMerge(const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges) +{ + QSet localKeys; + for (const DeckCardChange &change : localChanges.changes) { + localKeys.insert({change.zone, change.cardName}); + } + for (const DeckCardChange &change : remoteChanges.changes) { + if (localKeys.contains({change.zone, change.cardName})) { + return false; + } + } + return true; +} + +DeckList +DeckDiff::merge(const DeckList &baseDeck, const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges) +{ + QMap counts = zoneCardCounts(baseDeck); + for (const DeckCardChange &change : localChanges.changes) { + counts[{change.zone, change.cardName}] = change.newCount; + } + for (const DeckCardChange &change : remoteChanges.changes) { + counts[{change.zone, change.cardName}] = change.newCount; + } + + DeckList merged(baseDeck.getMetadata(), DecklistNodeTree(), baseDeck.getSideboardPlans()); + QList keys = counts.keys(); + std::sort(keys.begin(), keys.end()); + for (const ZoneCardKey &key : keys) { + const int count = qMax(0, counts.value(key)); + for (int i = 0; i < count; ++i) { + merged.addCard(key.second, key.first); + } + } + return merged; +} diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h new file mode 100644 index 000000000..2b4fb8732 --- /dev/null +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h @@ -0,0 +1,71 @@ +/** + * @file deck_diff.h + * @brief Card-level diff and three-way merge for deck collection sync. + */ + +#ifndef DECK_DIFF_H +#define DECK_DIFF_H + +#include +#include + +class DeckList; + +/** @brief A single card-count change within one zone. */ +struct DeckCardChange +{ + QString zone; ///< Zone name, e.g. "main". + QString cardName; ///< Card name. + int oldCount = 0; ///< Copies in the old deck. + int newCount = 0; ///< Copies in the new deck. + /** @brief Copies added (positive) or removed (negative). */ + [[nodiscard]] int delta() const + { + return newCount - oldCount; + } +}; + +/** @brief Card-level difference between two decks, sorted by zone then card name. */ +struct DeckDiffResult +{ + QList changes; + /** @brief True when both decks hold the same cards in the same zones. */ + [[nodiscard]] bool isEmpty() const + { + return changes.isEmpty(); + } +}; + +/** + * @class DeckDiff + * @brief Pure card-count diff and three-way merge for deck sync. + * + * Compares card names and copy counts per zone only; printing information + * (set, collector number, provider) is not tracked, so a merged deck keeps + * base printings with merged counts. + */ +class DeckDiff +{ +public: + /** @brief Diffs oldDeck against newDeck. */ + static DeckDiffResult diff(const DeckList &oldDeck, const DeckList &newDeck); + /** + * @brief True when both change sets touch disjoint (zone, card) keys and a merge needs no arbitration. + */ + static bool canAutoMerge(const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges); + /** + * @brief Applies both change sets onto a copy of baseDeck. + * + * @precondition localChanges and remoteChanges were both computed with + * diff() against baseDeck. Stale or foreign change sets silently + * resurrect or delete cards. + * + * Counts are clamped at zero. On overlapping keys the remote change wins; + * callers offer this result as "Use Merge" only when canAutoMerge is true. + * Metadata and sideboard plans come from baseDeck. + */ + static DeckList + merge(const DeckList &baseDeck, const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges); +}; + +#endif // DECK_DIFF_H diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2c288c6ce..1823f5b17 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,6 +27,9 @@ add_test(NAME loader_local_matching_test COMMAND loader_local_matching_test) add_test(NAME deck_hash_performance_test COMMAND deck_hash_performance_test) set_tests_properties(deck_hash_performance_test PROPERTIES TIMEOUT 15) +add_test(NAME deck_diff_test COMMAND deck_diff_test) +add_test(NAME deck_sync_profile_test COMMAND deck_sync_profile_test) +add_test(NAME deck_sync_sidecar_test COMMAND deck_sync_sidecar_test) # Find GTest @@ -37,6 +40,17 @@ add_executable(test_age_formatting test_age_formatting.cpp) add_executable(password_hash_test password_hash_test.cpp) add_executable(playmat_resolver_test playmat_resolver_test.cpp) add_executable(deck_hash_performance_test deck_hash_performance_test.cpp) +add_executable(deck_diff_test deck_diff_test.cpp) +add_executable( + deck_sync_profile_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_sync/deck_sync_profile.cpp + deck_sync_profile_test.cpp +) +target_include_directories(deck_sync_profile_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_sync) +add_executable( + deck_sync_sidecar_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp + deck_sync_sidecar_test.cpp +) +target_include_directories(deck_sync_sidecar_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_sync) add_executable(server_card_counter_test server_card_counter_test.cpp) add_executable(server_counter_test server_counter_test.cpp) add_executable(server_rate_limiter_test server_rate_limiter_test.cpp) @@ -137,6 +151,9 @@ if(NOT GTEST_FOUND) add_dependencies(password_hash_test gtest) add_dependencies(playmat_resolver_test gtest) add_dependencies(deck_hash_performance_test gtest) + add_dependencies(deck_diff_test gtest) + add_dependencies(deck_sync_profile_test gtest) + add_dependencies(deck_sync_sidecar_test gtest) add_dependencies(server_card_counter_test gtest) add_dependencies(server_counter_test gtest) add_dependencies(server_rate_limiter_test gtest) @@ -175,6 +192,12 @@ target_link_libraries( deck_hash_performance_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) +target_link_libraries( + deck_diff_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) +target_link_libraries(deck_sync_profile_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) +target_link_libraries(deck_sync_sidecar_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) target_link_libraries( server_card_counter_test libcockatrice_network Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) diff --git a/tests/deck_diff_test.cpp b/tests/deck_diff_test.cpp new file mode 100644 index 000000000..4ff0b556b --- /dev/null +++ b/tests/deck_diff_test.cpp @@ -0,0 +1,137 @@ +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include + +namespace +{ +DeckList makeDeck(std::initializer_list> mainCards, + std::initializer_list> sideCards = {}) +{ + DeckList deck; + for (const auto &[name, count] : mainCards) { + for (int i = 0; i < count; ++i) { + deck.addCard(name, "main"); + } + } + for (const auto &[name, count] : sideCards) { + for (int i = 0; i < count; ++i) { + deck.addCard(name, "side"); + } + } + return deck; +} + +int countOf(const DeckList &deck, const char *zone, const char *name) +{ + int count = 0; + deck.forEachCard([&](InnerDecklistNode *zoneNode, DecklistCardNode *card) { + if (zoneNode->getName() == zone && card->getName() == name) { + count += card->getNumber(); + } + }); + return count; +} +} // namespace + +TEST(DeckDiffTest, IdenticalDecksHaveEmptyDiff) +{ + DeckList first = makeDeck({{"Lightning Bolt", 4}}); + DeckList second = makeDeck({{"Lightning Bolt", 4}}); + EXPECT_TRUE(DeckDiff::diff(first, second).isEmpty()); +} + +TEST(DeckDiffTest, ChangesAreSortedByZoneThenName) +{ + DeckList oldDeck = makeDeck({{"Mountain", 20}, {"Lightning Bolt", 4}}, {{"Shock", 2}}); + DeckList newDeck = makeDeck({{"Mountain", 19}, {"Shock", 5}}, {{"Shock", 1}, {"Teferi", 1}}); + + DeckDiffResult result = DeckDiff::diff(oldDeck, newDeck); + ASSERT_EQ(result.changes.size(), 5); + EXPECT_EQ(result.changes.at(0).zone, "main"); + EXPECT_EQ(result.changes.at(0).cardName, "Lightning Bolt"); + EXPECT_EQ(result.changes.at(1).cardName, "Mountain"); + EXPECT_EQ(result.changes.at(2).cardName, "Shock"); + EXPECT_EQ(result.changes.at(3).zone, "side"); + EXPECT_EQ(result.changes.at(3).cardName, "Shock"); + EXPECT_EQ(result.changes.at(4).cardName, "Teferi"); +} + +TEST(DeckDiffTest, ReportsAddedAndRemovedCopies) +{ + DeckList oldDeck = makeDeck({{"Lightning Bolt", 4}, {"Mountain", 20}}); + DeckList newDeck = makeDeck({{"Lightning Bolt", 2}, {"Shock", 4}, {"Mountain", 20}}); + + DeckDiffResult result = DeckDiff::diff(oldDeck, newDeck); + ASSERT_EQ(result.changes.size(), 2); + EXPECT_EQ(result.changes.at(0).cardName, "Lightning Bolt"); + EXPECT_EQ(result.changes.at(0).oldCount, 4); + EXPECT_EQ(result.changes.at(0).newCount, 2); + EXPECT_EQ(result.changes.at(0).delta(), -2); + EXPECT_EQ(result.changes.at(1).cardName, "Shock"); + EXPECT_EQ(result.changes.at(1).oldCount, 0); + EXPECT_EQ(result.changes.at(1).newCount, 4); +} + +TEST(DeckDiffTest, ZoneMoveIsTwoChanges) +{ + DeckList oldDeck = makeDeck({{"Teferi", 1}}); + DeckList newDeck = makeDeck({}, {{"Teferi", 1}}); + + DeckDiffResult result = DeckDiff::diff(oldDeck, newDeck); + ASSERT_EQ(result.changes.size(), 2); + EXPECT_EQ(result.changes.at(0).zone, "main"); + EXPECT_EQ(result.changes.at(0).newCount, 0); + EXPECT_EQ(result.changes.at(1).zone, "side"); + EXPECT_EQ(result.changes.at(1).oldCount, 0); +} + +TEST(DeckDiffTest, DisjointChangesCanAutoMerge) +{ + DeckList base = makeDeck({{"Lightning Bolt", 4}, {"Mountain", 20}}); + DeckList local = makeDeck({{"Lightning Bolt", 4}, {"Mountain", 18}}); + DeckList remote = makeDeck({{"Shock", 2}, {"Lightning Bolt", 4}, {"Mountain", 20}}); + + DeckDiffResult localChanges = DeckDiff::diff(base, local); + DeckDiffResult remoteChanges = DeckDiff::diff(base, remote); + EXPECT_TRUE(DeckDiff::canAutoMerge(localChanges, remoteChanges)); + + DeckList merged = DeckDiff::merge(base, localChanges, remoteChanges); + EXPECT_EQ(countOf(merged, "main", "Mountain"), 18); + EXPECT_EQ(countOf(merged, "main", "Shock"), 2); + EXPECT_EQ(countOf(merged, "main", "Lightning Bolt"), 4); +} + +TEST(DeckDiffTest, OverlappingChangesCannotAutoMergeAndRemoteWins) +{ + DeckList base = makeDeck({{"Lightning Bolt", 4}}); + DeckList local = makeDeck({{"Lightning Bolt", 3}}); + DeckList remote = makeDeck({{"Lightning Bolt", 1}}); + + DeckDiffResult localChanges = DeckDiff::diff(base, local); + DeckDiffResult remoteChanges = DeckDiff::diff(base, remote); + EXPECT_FALSE(DeckDiff::canAutoMerge(localChanges, remoteChanges)); + + DeckList merged = DeckDiff::merge(base, localChanges, remoteChanges); + EXPECT_EQ(countOf(merged, "main", "Lightning Bolt"), 1); +} + +TEST(DeckDiffTest, MergeKeepsBaseMetadata) +{ + DeckList base = makeDeck({{"Lightning Bolt", 4}}); + base.setName("Affinity v3"); + DeckList remote = makeDeck({{"Lightning Bolt", 4}, {"Shock", 1}}); + + DeckList merged = DeckDiff::merge(base, DeckDiff::diff(base, base), DeckDiff::diff(base, remote)); + EXPECT_EQ(merged.getName(), "Affinity v3"); + EXPECT_EQ(countOf(merged, "main", "Shock"), 1); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tests/deck_sync_profile_test.cpp b/tests/deck_sync_profile_test.cpp new file mode 100644 index 000000000..fc6917559 --- /dev/null +++ b/tests/deck_sync_profile_test.cpp @@ -0,0 +1,63 @@ +#include "gtest/gtest.h" +#include +#include + +TEST(DeckSyncProfileTest, KeyIsStable) +{ + DeckSyncProfile first("woogerworks.com", 4747, "Skog7"); + DeckSyncProfile second("woogerworks.com", 4747, "Skog7"); + EXPECT_EQ(first.key(), second.key()); + EXPECT_FALSE(first.key().isEmpty()); +} + +TEST(DeckSyncProfileTest, KeyIgnoresCase) +{ + DeckSyncProfile lower("woogerworks.com", 4747, "skog7"); + DeckSyncProfile upper("WOOGERWORKS.COM", 4747, "SKOG7"); + EXPECT_EQ(lower.key(), upper.key()); +} + +TEST(DeckSyncProfileTest, KeyDiffersPerServerAndUser) +{ + DeckSyncProfile base("woogerworks.com", 4747, "skog7"); + EXPECT_NE(base.key(), DeckSyncProfile("other.server", 4747, "skog7").key()); + EXPECT_NE(base.key(), DeckSyncProfile("woogerworks.com", 4748, "skog7").key()); + EXPECT_NE(base.key(), DeckSyncProfile("woogerworks.com", 4747, "other").key()); +} + +TEST(DeckSyncProfileTest, KeyIsUnambiguousAcrossSeparators) +{ + DeckSyncProfile left("a:1", 2, "b"); + DeckSyncProfile right("a", 1, "2:b"); + EXPECT_NE(left.key(), right.key()); + EXPECT_NE(DeckSyncProfile("::1", 4747, "skog7").key(), DeckSyncProfile("::2", 4747, "skog7").key()); +} + +TEST(DeckSyncProfileTest, InvalidProfilesHaveNoKey) +{ + EXPECT_TRUE(DeckSyncProfile().key().isEmpty()); + EXPECT_TRUE(DeckSyncProfile("", 4747, "skog7").key().isEmpty()); + EXPECT_TRUE(DeckSyncProfile("woogerworks.com", 0, "skog7").key().isEmpty()); +} + +TEST(DeckSyncProfileTest, KeyIgnoresSurroundingWhitespace) +{ + DeckSyncProfile plain("woogerworks.com", 4747, "skog7"); + DeckSyncProfile padded(" woogerworks.com ", 4747, " skog7 "); + EXPECT_EQ(plain.key(), padded.key()); +} + +TEST(DeckSyncProfileTest, ValidityRequiresAllFields) +{ + EXPECT_TRUE(DeckSyncProfile("woogerworks.com", 4747, "skog7").isValid()); + EXPECT_FALSE(DeckSyncProfile("", 4747, "skog7").isValid()); + EXPECT_FALSE(DeckSyncProfile("woogerworks.com", 0, "skog7").isValid()); + EXPECT_FALSE(DeckSyncProfile("woogerworks.com", 4747, "").isValid()); + EXPECT_FALSE(DeckSyncProfile().isValid()); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tests/deck_sync_sidecar_test.cpp b/tests/deck_sync_sidecar_test.cpp new file mode 100644 index 000000000..4bd04be33 --- /dev/null +++ b/tests/deck_sync_sidecar_test.cpp @@ -0,0 +1,126 @@ +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include + +namespace +{ +QString sidecarPath(QTemporaryDir &dir) +{ + return dir.filePath(".sync/profilekey.json"); +} +} // namespace + +TEST(DeckSyncSidecarTest, MissingFileLoadsEmpty) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + EXPECT_TRUE(sidecar.load()); + EXPECT_TRUE(sidecar.records().isEmpty()); + EXPECT_FALSE(sidecar.contains("affinity.cod")); +} + +TEST(DeckSyncSidecarTest, RoundTripsRecords) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + + DeckSyncRecord record; + record.guid = "123e4567-e89b-12d3-a456-426614174000"; + record.remoteId = 42; + record.baseHash = "5cac19qm"; + record.lastSyncSecs = 1791739200; + record.deleted = true; + sidecar.set("decks/affinity.cod", record); + EXPECT_TRUE(sidecar.save()); + + DeckSyncSidecar reloaded(sidecarPath(dir)); + EXPECT_TRUE(reloaded.load()); + EXPECT_TRUE(reloaded.contains("decks/affinity.cod")); + DeckSyncRecord back = reloaded.get("decks/affinity.cod"); + EXPECT_EQ(back.guid, record.guid); + EXPECT_EQ(back.remoteId, 42); + EXPECT_EQ(back.baseHash, "5cac19qm"); + EXPECT_EQ(back.lastSyncSecs, 1791739200); + EXPECT_TRUE(back.deleted); +} + +TEST(DeckSyncSidecarTest, UnknownPathReturnsDefaultRecord) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + DeckSyncRecord back = sidecar.get("missing.cod"); + EXPECT_EQ(back.remoteId, -1); + EXPECT_TRUE(back.guid.isEmpty()); +} + +namespace +{ +bool writeRaw(const QString &path, const QByteArray &bytes) +{ + QDir().mkpath(QFileInfo(path).absolutePath()); + QFile file(path); + return file.open(QIODevice::WriteOnly | QIODevice::Truncate) && file.write(bytes) == bytes.size(); +} +} // namespace + +TEST(DeckSyncSidecarTest, GarbageFileFailsToLoad) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + ASSERT_TRUE(writeRaw(sidecarPath(dir), "not json at all")); + DeckSyncSidecar sidecar(sidecarPath(dir)); + EXPECT_FALSE(sidecar.load()); +} + +TEST(DeckSyncSidecarTest, WrongRecordsShapeFailsToLoad) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + ASSERT_TRUE(writeRaw(sidecarPath(dir), R"({"records":["a.cod"]})")); + DeckSyncSidecar missing(sidecarPath(dir)); + EXPECT_FALSE(missing.load()); + + ASSERT_TRUE(writeRaw(sidecarPath(dir), R"({"foo":1})")); + DeckSyncSidecar other(sidecarPath(dir)); + EXPECT_FALSE(other.load()); +} + +TEST(DeckSyncSidecarTest, NonObjectEntriesAreSkipped) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + ASSERT_TRUE(writeRaw(sidecarPath(dir), R"({"records":{"a.cod":42,"b.cod":{"guid":"g"}}})")); + DeckSyncSidecar sidecar(sidecarPath(dir)); + EXPECT_TRUE(sidecar.load()); + EXPECT_FALSE(sidecar.contains("a.cod")); + EXPECT_TRUE(sidecar.contains("b.cod")); + EXPECT_EQ(sidecar.get("b.cod").guid, "g"); +} + +TEST(DeckSyncSidecarTest, RemoveDropsRecord) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + sidecar.set("a.cod", DeckSyncRecord()); + sidecar.remove("a.cod"); + EXPECT_FALSE(sidecar.contains("a.cod")); + EXPECT_TRUE(sidecar.save()); + + DeckSyncSidecar reloaded(sidecarPath(dir)); + EXPECT_TRUE(reloaded.load()); + EXPECT_FALSE(reloaded.contains("a.cod")); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 1a70940fc7109a01e7733d77dec3f77b86a3788a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:17:02 +0200 Subject: [PATCH 04/16] Fix ZeldaZach review for PR 7492: preserve printing in DeckDiff::merge, atomic sidecar save with QSaveFile --- .../interface/deck_sync/deck_sync_sidecar.cpp | 7 +- .../libcockatrice/deck_list/deck_diff.cpp | 82 +++++++++++++++---- .../libcockatrice/deck_list/deck_diff.h | 9 +- 3 files changed, 77 insertions(+), 21 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp index 861149464..2e3a5ba49 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace { @@ -64,12 +65,12 @@ bool DeckSyncSidecar::save() const root.insert("records", records); QDir().mkpath(QFileInfo(filePath).absolutePath()); - QFile file(filePath); - if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + QSaveFile file(filePath); + if (!file.open(QIODevice::WriteOnly)) { return false; } file.write(QJsonDocument(root).toJson(QJsonDocument::Compact)); - return true; + return file.commit(); } bool DeckSyncSidecar::contains(const QString &relativePath) const diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp index 31a6b622b..b918dd9ee 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp @@ -13,12 +13,36 @@ namespace { using ZoneCardKey = QPair; +struct ZoneCardInfo +{ + int count = 0; + QString cardSetShortName; + QString cardSetNumber; + QString cardProviderId; +}; + +QMap zoneCardInfos(const DeckList &deck) +{ + QMap infos; + deck.forEachCard([&infos](InnerDecklistNode *zone, DecklistCardNode *card) { + ZoneCardInfo &info = infos[{zone->getName(), card->getName()}]; + if (info.count == 0) { + info.cardSetShortName = card->getCardSetShortName(); + info.cardSetNumber = card->getCardCollectorNumber(); + info.cardProviderId = card->getCardProviderId(); + } + info.count += card->getNumber(); + }); + return infos; +} + QMap zoneCardCounts(const DeckList &deck) { QMap counts; - deck.forEachCard([&counts](InnerDecklistNode *zone, DecklistCardNode *card) { - counts[{zone->getName(), card->getName()}] += card->getNumber(); - }); + const QMap infos = zoneCardInfos(deck); + for (auto it = infos.begin(); it != infos.end(); ++it) { + counts.insert(it.key(), it.value().count); + } return counts; } @@ -34,17 +58,25 @@ bool changeLessThan(const DeckCardChange &a, const DeckCardChange &b) DeckDiffResult DeckDiff::diff(const DeckList &oldDeck, const DeckList &newDeck) { const QMap oldCounts = zoneCardCounts(oldDeck); - const QMap newCounts = zoneCardCounts(newDeck); + const QMap newInfos = zoneCardInfos(newDeck); QSet keys(oldCounts.keyBegin(), oldCounts.keyEnd()); - keys.unite(QSet(newCounts.keyBegin(), newCounts.keyEnd())); + keys.unite(QSet(newInfos.keyBegin(), newInfos.keyEnd())); DeckDiffResult result; for (const ZoneCardKey &key : keys) { const int oldCount = oldCounts.value(key, 0); - const int newCount = newCounts.value(key, 0); - if (oldCount != newCount) { - result.changes.append({key.first, key.second, oldCount, newCount}); + const ZoneCardInfo newInfo = newInfos.value(key, ZoneCardInfo()); + if (oldCount != newInfo.count) { + DeckCardChange change; + change.zone = key.first; + change.cardName = key.second; + change.oldCount = oldCount; + change.newCount = newInfo.count; + change.cardSetShortName = newInfo.cardSetShortName; + change.cardSetNumber = newInfo.cardSetNumber; + change.cardProviderId = newInfo.cardProviderId; + result.changes.append(change); } } std::sort(result.changes.begin(), result.changes.end(), changeLessThan); @@ -69,21 +101,41 @@ DeckList DeckDiff::merge(const DeckList &baseDeck, const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges) { QMap counts = zoneCardCounts(baseDeck); + QMap printings; for (const DeckCardChange &change : localChanges.changes) { counts[{change.zone, change.cardName}] = change.newCount; + printings[{change.zone, change.cardName}] = change; } for (const DeckCardChange &change : remoteChanges.changes) { counts[{change.zone, change.cardName}] = change.newCount; + printings[{change.zone, change.cardName}] = change; } - DeckList merged(baseDeck.getMetadata(), DecklistNodeTree(), baseDeck.getSideboardPlans()); - QList keys = counts.keys(); - std::sort(keys.begin(), keys.end()); - for (const ZoneCardKey &key : keys) { - const int count = qMax(0, counts.value(key)); - for (int i = 0; i < count; ++i) { - merged.addCard(key.second, key.first); + DeckList merged = baseDeck; + QMap> nodesByKey; + merged.forEachCard([&nodesByKey](InnerDecklistNode *zone, DecklistCardNode *card) { + nodesByKey[{zone->getName(), card->getName()}].append(card); + }); + + for (auto it = counts.begin(); it != counts.end(); ++it) { + const ZoneCardKey &key = it.key(); + const int target = qMax(0, it.value()); + QList nodes = nodesByKey.value(key); + if (target == 0) { + for (DecklistCardNode *node : nodes) { + merged.getTree()->deleteNode(node); + } + } else if (nodes.isEmpty()) { + const DeckCardChange printing = printings.value(key); + merged.getTree()->addCard(printing.cardName, target, printing.zone, -1, printing.cardSetShortName, + printing.cardSetNumber, printing.cardProviderId, true); + } else { + nodes.first()->setNumber(target); + for (int i = 1; i < nodes.size(); ++i) { + merged.getTree()->deleteNode(nodes.at(i)); + } } } + merged.refreshDeckHash(); return merged; } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h index 2b4fb8732..372da5599 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h @@ -18,6 +18,9 @@ struct DeckCardChange QString cardName; ///< Card name. int oldCount = 0; ///< Copies in the old deck. int newCount = 0; ///< Copies in the new deck. + QString cardSetShortName; ///< Printing set carried from the new side, empty when unchanged. + QString cardSetNumber; ///< Printing collector number carried from the new side. + QString cardProviderId; ///< Printing provider id carried from the new side. /** @brief Copies added (positive) or removed (negative). */ [[nodiscard]] int delta() const { @@ -40,9 +43,9 @@ struct DeckDiffResult * @class DeckDiff * @brief Pure card-count diff and three-way merge for deck sync. * - * Compares card names and copy counts per zone only; printing information - * (set, collector number, provider) is not tracked, so a merged deck keeps - * base printings with merged counts. + * Compares card names and copy counts per zone only; a merged deck keeps + * base printings for existing cards and carries the changed side's printing + * for newly added cards. */ class DeckDiff { From 4721cde443c52c53f51e44844787a85045f660d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:22:47 +0200 Subject: [PATCH 05/16] Fixup: clang-format alignment in deck_diff.h --- .../libcockatrice/deck_list/deck_diff.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h index 372da5599..563b54b70 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h @@ -14,10 +14,10 @@ class DeckList; /** @brief A single card-count change within one zone. */ struct DeckCardChange { - QString zone; ///< Zone name, e.g. "main". - QString cardName; ///< Card name. - int oldCount = 0; ///< Copies in the old deck. - int newCount = 0; ///< Copies in the new deck. + QString zone; ///< Zone name, e.g. "main". + QString cardName; ///< Card name. + int oldCount = 0; ///< Copies in the old deck. + int newCount = 0; ///< Copies in the new deck. QString cardSetShortName; ///< Printing set carried from the new side, empty when unchanged. QString cardSetNumber; ///< Printing collector number carried from the new side. QString cardProviderId; ///< Printing provider id carried from the new side. From be3cb65af59f755ed9926aa42ff092afd746bd59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 14:37:01 +0200 Subject: [PATCH 06/16] [Client] Manual deck collection sync with Sync Now action --- cockatrice/CMakeLists.txt | 1 + .../interface/deck_sync/deck_sync_manager.cpp | 510 ++++++++++++++++++ .../interface/deck_sync/deck_sync_manager.h | 133 +++++ .../widgets/tabs/tab_deck_storage.cpp | 80 +++ .../interface/widgets/tabs/tab_deck_storage.h | 29 + 5 files changed, 753 insertions(+) create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_manager.cpp create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_manager.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 9212a2902..525abfa72 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -29,6 +29,7 @@ set(cockatrice_SOURCES src/interface/deck_loader/loaded_deck.cpp src/interface/deck_sync/deck_sync_profile.cpp src/interface/deck_sync/deck_sync_sidecar.cpp + src/interface/deck_sync/deck_sync_manager.cpp src/interface/widgets/dialogs/dlg_connect.cpp src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp src/interface/widgets/dialogs/dlg_create_game.cpp diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp new file mode 100644 index 000000000..e32beada8 --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -0,0 +1,510 @@ +#include "deck_sync_manager.h" + +#include "../deck_loader/deck_file_format.h" +#include "../deck_loader/deck_loader.h" +#include "deck_sync_sidecar.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +constexpr const char *SYNC_META_DIR = ".sync"; +constexpr const char *BACKUP_SUFFIX = ".sync-backup-"; + +QString serverPathFor(const QString &relativePath) +{ + QString path = relativePath; + if (path.endsWith(".cod", Qt::CaseInsensitive)) { + path.chop(4); + } + return path; +} +} // namespace + +DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) +{ +} + +void DeckSyncManager::setClient(AbstractClient *_client) +{ + client = _client; +} + +void DeckSyncManager::setProfile(const DeckSyncProfile &_profile) +{ + profile = _profile; +} + +void DeckSyncManager::setDeckPath(const QString &path) +{ + deckPath = path; +} + +void DeckSyncManager::setColorIdentityProvider(ColorIdentityProvider provider) +{ + colorIdentityProvider = std::move(provider); +} + +bool DeckSyncManager::isSyncing() const +{ + return syncing; +} + +void DeckSyncManager::syncNow() +{ + if (syncing) { + return; + } + if (client == nullptr || !profile.isValid() || deckPath.isEmpty()) { + emit syncFinished(0, 0, 0, 0, tr("Not connected to a server.")); + return; + } + QDir deckDir(deckPath); + if (!deckDir.exists()) { + emit syncFinished(0, 0, 0, 0, tr("Deck folder not found.")); + return; + } + + syncing = true; + uploadedCount = 0; + downloadedCount = 0; + unchangedCount = 0; + skippedCount = 0; + pendingUploads.clear(); + pendingDownloads.clear(); + ensuredDirs.clear(); + ensuredDirs.insert(QString()); + localFiles.clear(); + emit syncStarted(); + emit statusMessage(tr("Scanning local decks...")); + + sidecarPath = deckDir.filePath(SYNC_META_DIR + QString("/") + profile.key() + ".json"); + DeckSyncSidecar sidecar(sidecarPath); + if (!sidecar.load()) { + failSync(tr("Could not read sync state.")); + return; + } + + QDirIterator it(deckPath, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + while (it.hasNext()) { + const QString absolutePath = it.next(); + const QString relativePath = deckDir.relativeFilePath(absolutePath); + if (relativePath.startsWith(SYNC_META_DIR + QString("/")) || + !absolutePath.endsWith(".cod", Qt::CaseInsensitive)) { + continue; + } + std::optional loaded = DeckLoader::loadFromFile(absolutePath, DeckFileFormat::Cockatrice); + if (!loaded) { + ++skippedCount; + continue; + } + LocalFile file; + file.absolutePath = absolutePath; + file.hash = loaded->deckList.getDeckHash(); + file.modified = QFileInfo(absolutePath).lastModified(); + localFiles.insert(relativePath, file); + } + + Command_DeckList cmd; + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onListFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, + const QString &prefix, + QMap &files, + QSet &dirs) +{ + for (const auto &item : folder.items()) { + const QString name = QString::fromStdString(item.name()); + if (item.has_folder()) { + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; + dirs.insert(path); + collectServerFiles(item.folder(), path, files, dirs); + } else if (item.has_file()) { + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; + const ServerInfo_DeckStorage_File &file = item.file(); + ServerFile entry; + entry.id = item.id(); + entry.guid = QString::fromStdString(file.guid()); + entry.hash = QString::fromStdString(file.hash()); + entry.lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0; + files.insert(path, entry); + } + } +} + +void DeckSyncManager::onListFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (!syncing) { + return; + } + if (response.response_code() != Response::RespOk) { + failSync(tr("Could not read server decks.")); + return; + } + + QMap serverFiles; + QSet serverDirs; + const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); + collectServerFiles(list.root(), QString(), serverFiles, serverDirs); + ensuredDirs.unite(serverDirs); + emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + + for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { + const QString serverPath = serverPathFor(it.key()); + auto serverIt = serverFiles.find(serverPath); + if (serverIt == serverFiles.end()) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = -1; + op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() + ? sidecar.get(it.key()).guid + : QUuid::createUuid().toString(QUuid::WithoutBraces); + pendingUploads.append(op); + continue; + } + if (it.value().hash == serverIt.value().hash) { + DeckSyncRecord record = sidecar.get(it.key()); + if (record.guid.isEmpty()) { + record.guid = serverIt.value().guid; + } + record.remoteId = serverIt.value().id; + record.baseHash = it.value().hash; + record.lastSyncSecs = nowSecs; + record.deleted = false; + sidecar.set(it.key(), record); + ++unchangedCount; + continue; + } + const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); + // A server row without last_modified predates sync: the local file wins. + // Otherwise strictly newer wins; ties go to the server. + if (serverIt.value().lastModifiedSecs == 0 || localSecs > serverIt.value().lastModifiedSecs) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = serverIt.value().id; + op.guid = serverIt.value().guid; + pendingUploads.append(op); + } else { + DownloadOp op; + op.relativePath = it.key(); + op.deckId = serverIt.value().id; + op.guid = serverIt.value().guid; + op.hash = serverIt.value().hash; + op.lastModifiedSecs = serverIt.value().lastModifiedSecs; + pendingDownloads.append(op); + } + } + + for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; + if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { + continue; + } + const QString relativePath = uniqueLocalPath(wantedPath); + if (relativePath.isEmpty()) { + ++skippedCount; + continue; + } + DownloadOp op; + op.relativePath = relativePath; + op.deckId = it.value().id; + op.guid = it.value().guid; + op.hash = it.value().hash; + op.lastModifiedSecs = it.value().lastModifiedSecs; + pendingDownloads.append(op); + } + + sidecar.save(); + processNext(); +} + +QString DeckSyncManager::parentDir(const QString &path) +{ + const int slash = path.lastIndexOf('/'); + return slash == -1 ? QString() : path.left(slash); +} + +void DeckSyncManager::processNext() +{ + if (!syncing) { + return; + } + if (!pendingUploads.isEmpty()) { + processNextUpload(); + } else if (!pendingDownloads.isEmpty()) { + sendDownload(pendingDownloads.first()); + } else { + finishSync(); + } +} + +void DeckSyncManager::processNextUpload() +{ + const UploadOp op = pendingUploads.first(); + const QString dir = parentDir(op.serverPath); + if (!ensuredDirs.contains(dir)) { + sendMkDirFor(dir); + return; + } + pendingUploads.removeFirst(); + sendUpload(op); +} + +void DeckSyncManager::sendMkDirFor(const QString &dir) +{ + QStringList parts = dir.split('/'); + QString made; + for (const QString &part : parts) { + const QString candidate = made.isEmpty() ? part : made + "/" + part; + if (!ensuredDirs.contains(candidate)) { + Command_DeckNewDir cmd; + cmd.set_path(made.toStdString()); + cmd.set_dir_name(part.toStdString()); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(dir); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onEnsureDirFinished); + client->sendCommand(pend); + return; + } + made = candidate; + } + processNextUpload(); +} + +void DeckSyncManager::onEnsureDirFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + if (response.response_code() != Response::RespOk) { + failSync(tr("Could not create server folder.")); + return; + } + const QString dir = extraData.toString(); + QStringList parts = dir.split('/'); + QString made; + for (const QString &part : parts) { + made = made.isEmpty() ? part : made + "/" + part; + if (!ensuredDirs.contains(made)) { + ensuredDirs.insert(made); + break; + } + } + processNextUpload(); +} + +void DeckSyncManager::sendUpload(const UploadOp &op) +{ + emit statusMessage(tr("Uploading %1...").arg(op.relativePath)); + std::optional loaded = + DeckLoader::loadFromFile(localFiles.value(op.relativePath).absolutePath, DeckFileFormat::Cockatrice); + if (!loaded) { + ++skippedCount; + processNext(); + return; + } + const QString deckString = loaded->deckList.writeToString_Native(); + if (deckString.length() > MAX_FILE_LENGTH) { + ++skippedCount; + processNext(); + return; + } + + Command_DeckUpload cmd; + if (op.deckId == -1) { + cmd.set_path(parentDir(op.serverPath).toStdString()); + } else { + cmd.set_deck_id(op.deckId); + } + cmd.set_deck_list(deckString.toStdString()); + cmd.set_guid(op.guid.toStdString()); + if (colorIdentityProvider) { + cmd.set_color_identity(colorIdentityProvider(loaded->deckList).toStdString()); + } + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(op.relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onUploadFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onUploadFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + if (response.response_code() != Response::RespOk) { + ++skippedCount; + processNext(); + return; + } + + const Response_DeckUpload &upload = response.GetExtension(Response_DeckUpload::ext); + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord record = sidecar.get(relativePath); + if (upload.has_new_file()) { + const auto &file = upload.new_file(); + if (!file.file().guid().empty()) { + record.guid = QString::fromStdString(file.file().guid()); + } + if (!file.file().hash().empty()) { + record.baseHash = QString::fromStdString(file.file().hash()); + } + if (file.file().has_last_modified()) { + record.lastSyncSecs = static_cast(file.file().last_modified()); + } else { + record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + } + record.remoteId = file.id(); + } else if (localFiles.contains(relativePath)) { + record.baseHash = localFiles.value(relativePath).hash; + record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + } + record.deleted = false; + sidecar.set(relativePath, record); + sidecar.save(); + ++uploadedCount; + processNext(); +} + +QString DeckSyncManager::uniqueLocalPath(const QString &relativePath) const +{ + if (!QFileInfo(deckPath + "/" + relativePath).exists()) { + return relativePath; + } + QString base = relativePath; + if (base.endsWith(".cod", Qt::CaseInsensitive)) { + base.chop(4); + } + for (int i = 2; i < 1000; ++i) { + const QString candidate = base + " (" + QString::number(i) + ").cod"; + if (!QFileInfo(deckPath + "/" + candidate).exists()) { + return candidate; + } + } + return QString(); +} + +void DeckSyncManager::sendDownload(const DownloadOp &op) +{ + emit statusMessage(tr("Downloading %1...").arg(op.relativePath)); + Command_DeckDownload cmd; + cmd.set_deck_id(op.deckId); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + pend->setExtraData(op.relativePath); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onDownloadFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onDownloadFinished(const Response &response, + const CommandContainer & /*commandContainer*/, + const QVariant &extraData) +{ + if (!syncing) { + return; + } + const QString relativePath = extraData.toString(); + for (int i = 0; i < pendingDownloads.size(); ++i) { + if (pendingDownloads.at(i).relativePath == relativePath) { + pendingDownloads.removeAt(i); + break; + } + } + if (response.response_code() != Response::RespOk) { + ++skippedCount; + processNext(); + return; + } + + const Response_DeckDownload &download = response.GetExtension(Response_DeckDownload::ext); + DeckList deck; + if (!deck.loadFromString_Native(QString::fromStdString(download.deck()))) { + ++skippedCount; + processNext(); + return; + } + + const QString absolutePath = deckPath + "/" + relativePath; + if (localFiles.contains(relativePath)) { + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath)) { + ++skippedCount; + processNext(); + return; + } + } + QDir().mkpath(QFileInfo(absolutePath).absolutePath()); + if (!DeckLoader::saveToFile(deck, absolutePath, DeckFileFormat::Cockatrice)) { + ++skippedCount; + processNext(); + return; + } + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord record = sidecar.get(relativePath); + if (!download.guid().empty()) { + record.guid = QString::fromStdString(download.guid()); + } + if (!download.hash().empty()) { + record.baseHash = QString::fromStdString(download.hash()); + } else { + record.baseHash = deck.getDeckHash(); + } + record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) + : QDateTime::currentDateTime().toSecsSinceEpoch(); + record.deleted = false; + sidecar.set(relativePath, record); + sidecar.save(); + ++downloadedCount; + processNext(); +} + +void DeckSyncManager::failSync(const QString &error) +{ + syncing = false; + emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, error); +} + +void DeckSyncManager::finishSync() +{ + syncing = false; + emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") + .arg(uploadedCount) + .arg(downloadedCount) + .arg(unchangedCount) + .arg(skippedCount)); + emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, QString()); +} diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h new file mode 100644 index 000000000..99f2040fe --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -0,0 +1,133 @@ +/** + * @file deck_sync_manager.h + * @brief One-shot two-way deck collection sync between local files and the server. + */ + +#ifndef DECK_SYNC_MANAGER_H +#define DECK_SYNC_MANAGER_H + +#include "deck_sync_profile.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +class AbstractClient; +class CommandContainer; +class DeckList; +class Response; +class ServerInfo_DeckStorage_Folder; + +/** + * @class DeckSyncManager + * @brief Runs a manual whole-collection sync: local *.cod files vs server deck storage. + * + * Each syncNow() run loads the profile sidecar, scans the deck folder, fetches + * the server tree and mirrors both sides: unknown local decks upload, unknown + * server decks download, and decks changed on both sides resolve newer-wins + * (ties go to the server). Matching is path-based; a rename looks like a + * delete plus an add. Before a download overwrites a locally modified file, the + * local content is copied to a .sync-backup file next to it. + * + * Ownership: fire-and-forget per run, driven by PendingCommand round trips. + * No watcher, no auto triggers, no conflict UI — those arrive in later layers. + */ +class DeckSyncManager : public QObject +{ + Q_OBJECT +public: + /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ + using ColorIdentityProvider = std::function; + + explicit DeckSyncManager(QObject *parent = nullptr); + + void setClient(AbstractClient *_client); + void setProfile(const DeckSyncProfile &_profile); + void setDeckPath(const QString &path); + void setColorIdentityProvider(ColorIdentityProvider provider); + [[nodiscard]] bool isSyncing() const; + +public slots: + void syncNow(); + +signals: + void syncStarted(); + void statusMessage(const QString &message); + void syncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + +private slots: + void onListFinished(const Response &response, const CommandContainer &commandContainer); + void + onEnsureDirFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onUploadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + void + onDownloadFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); + +private: + struct ServerFile + { + int id = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; + struct LocalFile + { + QString absolutePath; + QString hash; + QDateTime modified; + }; + struct UploadOp + { + QString relativePath; + QString serverPath; + int deckId = -1; + QString guid; + }; + struct DownloadOp + { + QString relativePath; + int deckId = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; + + void failSync(const QString &error); + void finishSync(); + void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, + const QString &prefix, + QMap &files, + QSet &dirs); + void processNext(); + void processNextUpload(); + void sendMkDirFor(const QString &dir); + void sendUpload(const UploadOp &op); + void sendDownload(const DownloadOp &op); + static QString parentDir(const QString &path); + QString uniqueLocalPath(const QString &relativePath) const; + + AbstractClient *client = nullptr; + DeckSyncProfile profile; + QString deckPath; + ColorIdentityProvider colorIdentityProvider; + bool syncing = false; + + QString sidecarPath; + QMap localFiles; + QSet ensuredDirs; + QList pendingUploads; + QList pendingDownloads; + int uploadedCount = 0; + int downloadedCount = 0; + int unchangedCount = 0; + int skippedCount = 0; +}; + +#endif // DECK_SYNC_MANAGER_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 44d04bd24..e4ebcffbb 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -4,6 +4,7 @@ #include "../../deck_loader/deck_file_format.h" #include "../../deck_loader/deck_loader.h" #include "../../deck_loader/loaded_deck.h" +#include "../../deck_sync/deck_sync_manager.h" #include "../../pixel_map_generator.h" #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" @@ -32,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -178,6 +180,9 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightVbox->addWidget(shareBar); rightVbox->addWidget(serverDirView); rightVbox->addLayout(rightToolBarLayout); + syncStatusLabel = new QLabel(this); + syncStatusLabel->setWordWrap(true); + rightVbox->addWidget(syncStatusLabel); rightGroupBox = new QGroupBox; rightGroupBox->setLayout(rightVbox); @@ -229,6 +234,10 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, aPublishDeck->setIcon(QPixmap("theme:icons/lock")); connect(aPublishDeck, &QAction::triggered, this, &TabDeckStorage::actPublishDeck); + aSyncNow = new QAction(this); + aSyncNow->setIcon(qApp->style()->standardIcon(QStyle::SP_BrowserReload)); + connect(aSyncNow, &QAction::triggered, this, &TabDeckStorage::actSyncNow); + // Add actions to toolbars leftToolBar->addAction(aOpenLocalDeck); leftToolBar->addAction(aRenameLocal); @@ -240,11 +249,22 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightToolBar->addAction(aOpenRemoteDeck); rightToolBar->addAction(aDownload); + rightToolBar->addAction(aSyncNow); rightToolBar->addAction(aShareDecks); rightToolBar->addAction(aPublishDeck); rightToolBar->addAction(aNewFolder); rightToolBar->addAction(aDeleteRemoteDeck); + syncManager = new DeckSyncManager(this); + syncManager->setClient(client); + syncManager->setColorIdentityProvider( + [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }); + connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); + connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); + if (currentUserInfo != nullptr) { + syncUsername = QString::fromStdString(currentUserInfo->name()); + } + retranslateUi(); QWidget *mainWidget = new QWidget(this); @@ -273,6 +293,8 @@ void TabDeckStorage::retranslateUi() aShareDecks->setText(tr("Share decks")); aPublishDeck->setText(tr("Publish/unpublish deck")); aOpenDecksFolder->setText(tr("Open decks folder")); + aSyncNow->setText(tr("Sync now")); + renderSyncStatus(); shareBar->retranslateUi(); if (shareBar->isVisible()) { onServerSelectionChanged(); @@ -300,6 +322,7 @@ QString TabDeckStorage::getTargetPath() const void TabDeckStorage::handleConnected(const ServerInfo_User &userInfo) { + syncUsername = QString::fromStdString(userInfo.name()); setRemoteEnabled(userInfo.user_level() & ServerInfo_User::IsRegistered); } @@ -317,6 +340,7 @@ void TabDeckStorage::handleConnectionChanged(ClientStatus status) void TabDeckStorage::setRemoteEnabled(bool enabled) { + remoteEnabled = enabled; aUpload->setEnabled(enabled); aOpenRemoteDeck->setEnabled(enabled); aDownload->setEnabled(enabled); @@ -324,6 +348,7 @@ void TabDeckStorage::setRemoteEnabled(bool enabled) aPublishDeck->setEnabled(enabled); aNewFolder->setEnabled(enabled); aDeleteRemoteDeck->setEnabled(enabled); + aSyncNow->setEnabled(enabled && !syncManager->isSyncing()); if (enabled) { serverDirView->refreshTree(); @@ -527,6 +552,61 @@ void TabDeckStorage::actOpenDecksFolder() QDesktopServices::openUrl(QUrl::fromLocalFile(dir)); } +void TabDeckStorage::actSyncNow() +{ + DeckSyncProfile profile(client->serverName(), client->serverPort(), syncUsername); + syncManager->setProfile(profile); + syncManager->setDeckPath(SettingsCache::instance().paths().getDeckPath()); + aSyncNow->setEnabled(false); + syncManager->syncNow(); +} + +void TabDeckStorage::onSyncStatusMessage(const QString &message) +{ + syncStatusType = SyncStatusType::Working; + syncStatusDetail = message; + renderSyncStatus(); +} + +void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error) +{ + aSyncNow->setEnabled(remoteEnabled); + if (!error.isEmpty()) { + syncStatusType = SyncStatusType::Failed; + syncStatusDetail = error; + } else { + syncStatusType = SyncStatusType::Finished; + syncStatusDetail.clear(); + lastSyncSummary = {uploaded, downloaded, unchanged, skipped}; + } + renderSyncStatus(); + if (error.isEmpty()) { + serverDirView->refreshTree(); + } +} + +void TabDeckStorage::renderSyncStatus() +{ + switch (syncStatusType) { + case SyncStatusType::Working: + syncStatusLabel->setText(syncStatusDetail); + break; + 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)); + break; + case SyncStatusType::Idle: + syncStatusLabel->setText(tr("Not synced yet")); + break; + } +} + void TabDeckStorage::actRemoteDoubleClick(const QModelIndex &curRight) { if (dynamic_cast(serverDirView->getNode(curRight))) { diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index 35a4649c7..f24f07ac7 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -25,9 +25,11 @@ class QFileSystemModel; class QToolBar; class QGroupBox; class QTimer; +class QLabel; class CommandContainer; class Response; class ShareBarWidget; +class DeckSyncManager; class QAction; class QModelIndex; class TabSupervisor; @@ -50,6 +52,29 @@ private: QAction *aOpenLocalDeck, *aRenameLocal, *aUpload, *aNewLocalFolder, *aDeleteLocalDeck; QAction *aOpenDecksFolder; QAction *aOpenRemoteDeck, *aDownload, *aShareDecks, *aPublishDeck, *aNewFolder, *aDeleteRemoteDeck; + QAction *aSyncNow; + QLabel *syncStatusLabel; + DeckSyncManager *syncManager; + QString syncUsername; + bool remoteEnabled = false; + enum class SyncStatusType + { + Idle, + Working, + Finished, + Failed + }; + struct SyncSummary + { + int uploaded = 0; + int downloaded = 0; + int unchanged = 0; + int skipped = 0; + }; + SyncStatusType syncStatusType = SyncStatusType::Idle; + QString syncStatusDetail; + SyncSummary lastSyncSummary; + void renderSyncStatus(); bool visibilityRefreshStarted = false; QTimer *visibilityRefreshTimer; QStringList visibilityFailures; @@ -108,6 +133,10 @@ private slots: void deleteFolderFinished(const Response &response, const CommandContainer &commandContainer); void deleteDeckFinished(const Response &response, const CommandContainer &commandContainer); + void actSyncNow(); + void onSyncStatusMessage(const QString &message); + void onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + public: TabDeckStorage(TabSupervisor *_tabSupervisor, AbstractClient *_client, const ServerInfo_User *currentUserInfo); void retranslateUi() override; From bdb9de0e6ee2979f2a289081bbc1cd2148669c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:18:43 +0200 Subject: [PATCH 07/16] Fix ZeldaZach review for PR 7493: match uploads by sidecar remoteId/guid, validate server paths --- .../interface/deck_sync/deck_sync_manager.cpp | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index e32beada8..eb4987ba9 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -135,6 +135,9 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo { for (const auto &item : folder.items()) { const QString name = QString::fromStdString(item.name()); + if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\')) { + continue; + } if (item.has_folder()) { const QString path = prefix.isEmpty() ? name : prefix + "/" + name; dirs.insert(path); @@ -173,8 +176,37 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont sidecar.load(); const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + QMap serverPathById; + QMap serverPathByGuid; + for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { + serverPathById.insert(sit.value().id, sit.key()); + if (!sit.value().guid.isEmpty()) { + serverPathByGuid.insert(sit.value().guid, sit.key()); + } + } + QSet matchedServerPaths; + for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { - const QString serverPath = serverPathFor(it.key()); + const DeckSyncRecord sidecarRecord = sidecar.get(it.key()); + QString serverPath; + if (sidecarRecord.remoteId != -1 && serverPathById.contains(sidecarRecord.remoteId)) { + serverPath = serverPathById.value(sidecarRecord.remoteId); + } else if (!sidecarRecord.guid.isEmpty() && serverPathByGuid.contains(sidecarRecord.guid)) { + serverPath = serverPathByGuid.value(sidecarRecord.guid); + } else { + serverPath = serverPathFor(it.key()); + if (!serverFiles.contains(serverPath)) { + UploadOp op; + op.relativePath = it.key(); + op.serverPath = serverPath; + op.deckId = -1; + op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() + ? sidecar.get(it.key()).guid + : QUuid::createUuid().toString(QUuid::WithoutBraces); + pendingUploads.append(op); + continue; + } + } auto serverIt = serverFiles.find(serverPath); if (serverIt == serverFiles.end()) { UploadOp op; @@ -187,6 +219,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont pendingUploads.append(op); continue; } + matchedServerPaths.insert(serverIt.key()); if (it.value().hash == serverIt.value().hash) { DeckSyncRecord record = sidecar.get(it.key()); if (record.guid.isEmpty()) { @@ -222,6 +255,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont } for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + if (matchedServerPaths.contains(it.key())) { + continue; + } const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { continue; @@ -455,7 +491,13 @@ void DeckSyncManager::onDownloadFinished(const Response &response, return; } - const QString absolutePath = deckPath + "/" + relativePath; + const QString cleanDeckPath = QDir::cleanPath(deckPath); + const QString absolutePath = QDir::cleanPath(deckPath + "/" + relativePath); + if (absolutePath != cleanDeckPath && !absolutePath.startsWith(cleanDeckPath + "/")) { + ++skippedCount; + processNext(); + return; + } if (localFiles.contains(relativePath)) { const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); From b4a2869579ff22fd8e411d5af6bbe4bd10f9a5b4 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:29 +0200 Subject: [PATCH 08/16] [DeckSync] Move SyncSummary into DeckSyncManager per review --- cockatrice/src/interface/deck_sync/deck_sync_manager.h | 9 +++++++++ .../src/interface/widgets/tabs/tab_deck_storage.h | 10 ++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 99f2040fe..7f759c313 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -44,6 +44,15 @@ public: /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ using ColorIdentityProvider = std::function; + /** @brief Outcome counts for a finished sync run; mirrors syncFinished() parameters. */ + struct SyncSummary + { + int uploaded = 0; + int downloaded = 0; + int unchanged = 0; + int skipped = 0; + }; + explicit DeckSyncManager(QObject *parent = nullptr); void setClient(AbstractClient *_client); diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index f24f07ac7..bd53d5073 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -11,6 +11,7 @@ #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "tab.h" +#include "../deck_sync/deck_sync_manager.h" #include #include #include @@ -29,7 +30,6 @@ class QLabel; class CommandContainer; class Response; class ShareBarWidget; -class DeckSyncManager; class QAction; class QModelIndex; class TabSupervisor; @@ -64,13 +64,7 @@ private: Finished, Failed }; - struct SyncSummary - { - int uploaded = 0; - int downloaded = 0; - int unchanged = 0; - int skipped = 0; - }; + using SyncSummary = DeckSyncManager::SyncSummary; SyncStatusType syncStatusType = SyncStatusType::Idle; QString syncStatusDetail; SyncSummary lastSyncSummary; From 14b3de082654308a29f09124d537ec84b68b35cc Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:44 +0200 Subject: [PATCH 09/16] [DeckSync] Prefer ctor params for set-once client/provider per review --- cockatrice/src/interface/deck_sync/deck_sync_manager.cpp | 5 +++++ cockatrice/src/interface/deck_sync/deck_sync_manager.h | 1 + cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp | 6 ++---- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index eb4987ba9..d9a5c190b 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -42,6 +42,11 @@ DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) { } +DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider provider, QObject *parent) + : QObject(parent), client(_client), colorIdentityProvider(std::move(provider)) +{ +} + void DeckSyncManager::setClient(AbstractClient *_client) { client = _client; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 7f759c313..2bf12f72f 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -54,6 +54,7 @@ public: }; explicit DeckSyncManager(QObject *parent = nullptr); + DeckSyncManager(AbstractClient *client, ColorIdentityProvider provider, QObject *parent = nullptr); void setClient(AbstractClient *_client); void setProfile(const DeckSyncProfile &_profile); diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index e4ebcffbb..50b77a66e 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -255,10 +255,8 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightToolBar->addAction(aNewFolder); rightToolBar->addAction(aDeleteRemoteDeck); - syncManager = new DeckSyncManager(this); - syncManager->setClient(client); - syncManager->setColorIdentityProvider( - [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }); + syncManager = new DeckSyncManager( + client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); if (currentUserInfo != nullptr) { From 317b37db019a2e766bc236a213413d1fbde3dfa8 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:54 +0200 Subject: [PATCH 10/16] [DeckSync] Use designated initializer for ServerFile per review --- .../src/interface/deck_sync/deck_sync_manager.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index d9a5c190b..c97d61c40 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -150,11 +150,12 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo } else if (item.has_file()) { const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); - ServerFile entry; - entry.id = item.id(); - entry.guid = QString::fromStdString(file.guid()); - entry.hash = QString::fromStdString(file.hash()); - entry.lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0; + ServerFile entry{ + .id = item.id(), + .guid = QString::fromStdString(file.guid()), + .hash = QString::fromStdString(file.hash()), + .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, + }; files.insert(path, entry); } } From d3b9ac2b431f492898c6440396ced68fe009783f Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:50:40 +0200 Subject: [PATCH 11/16] [Format] Fix include order in tab_deck_storage.h --- cockatrice/src/interface/widgets/tabs/tab_deck_storage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index bd53d5073..ade10e951 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -8,10 +8,10 @@ #ifndef TAB_DECK_STORAGE_H #define TAB_DECK_STORAGE_H +#include "../deck_sync/deck_sync_manager.h" #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "tab.h" -#include "../deck_sync/deck_sync_manager.h" #include #include #include From e220bb3aa3dbf5264ae80e373eb918d79108dbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 12:41:00 +0100 Subject: [PATCH 12/16] [DeckSync] Pass the sync summary through the signal and tidy the manager API - Emit SyncSummary from syncFinished() instead of four positional ints, and build it from a single currentSummary() helper, so the counts cannot drift between the status message and the signal. - Use a designated initializer for LocalFile, like the other records. - Drop setClient() and setColorIdentityProvider(): both fields are already constructor parameters and the setters had no callers. setProfile() and setDeckPath() stay, since the tab re-reads them before every run. - Fix the include paths in tab_deck_storage.h and the narrowing conversion of the tree item id, so the branch builds again. --- .../interface/deck_sync/deck_sync_manager.cpp | 47 +++++++++---------- .../interface/deck_sync/deck_sync_manager.h | 7 ++- .../widgets/tabs/tab_deck_storage.cpp | 4 +- .../interface/widgets/tabs/tab_deck_storage.h | 6 +-- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index c97d61c40..bef2739d6 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -47,11 +47,6 @@ DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider { } -void DeckSyncManager::setClient(AbstractClient *_client) -{ - client = _client; -} - void DeckSyncManager::setProfile(const DeckSyncProfile &_profile) { profile = _profile; @@ -62,11 +57,6 @@ void DeckSyncManager::setDeckPath(const QString &path) deckPath = path; } -void DeckSyncManager::setColorIdentityProvider(ColorIdentityProvider provider) -{ - colorIdentityProvider = std::move(provider); -} - bool DeckSyncManager::isSyncing() const { return syncing; @@ -78,12 +68,12 @@ void DeckSyncManager::syncNow() return; } if (client == nullptr || !profile.isValid() || deckPath.isEmpty()) { - emit syncFinished(0, 0, 0, 0, tr("Not connected to a server.")); + emit syncFinished(SyncSummary{}, tr("Not connected to a server.")); return; } QDir deckDir(deckPath); if (!deckDir.exists()) { - emit syncFinished(0, 0, 0, 0, tr("Deck folder not found.")); + emit syncFinished(SyncSummary{}, tr("Deck folder not found.")); return; } @@ -120,11 +110,9 @@ void DeckSyncManager::syncNow() ++skippedCount; continue; } - LocalFile file; - file.absolutePath = absolutePath; - file.hash = loaded->deckList.getDeckHash(); - file.modified = QFileInfo(absolutePath).lastModified(); - localFiles.insert(relativePath, file); + localFiles.insert(relativePath, LocalFile{.absolutePath = absolutePath, + .hash = loaded->deckList.getDeckHash(), + .modified = QFileInfo(absolutePath).lastModified()}); } Command_DeckList cmd; @@ -151,7 +139,7 @@ void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &fo const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); ServerFile entry{ - .id = item.id(), + .id = static_cast(item.id()), .guid = QString::fromStdString(file.guid()), .hash = QString::fromStdString(file.hash()), .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, @@ -543,16 +531,27 @@ void DeckSyncManager::onDownloadFinished(const Response &response, void DeckSyncManager::failSync(const QString &error) { syncing = false; - emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, error); + emit syncFinished(currentSummary(), error); } void DeckSyncManager::finishSync() { syncing = false; + const SyncSummary summary = currentSummary(); emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") - .arg(uploadedCount) - .arg(downloadedCount) - .arg(unchangedCount) - .arg(skippedCount)); - emit syncFinished(uploadedCount, downloadedCount, unchangedCount, skippedCount, QString()); + .arg(summary.uploaded) + .arg(summary.downloaded) + .arg(summary.unchanged) + .arg(summary.skipped)); + emit syncFinished(summary, QString()); +} + +DeckSyncManager::SyncSummary DeckSyncManager::currentSummary() const +{ + return SyncSummary{ + .uploaded = uploadedCount, + .downloaded = downloadedCount, + .unchanged = unchangedCount, + .skipped = skippedCount, + }; } diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 2bf12f72f..8ecbaf74c 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -44,7 +44,7 @@ public: /** @brief Computes the color identity string uploaded alongside a deck. May be empty. */ using ColorIdentityProvider = std::function; - /** @brief Outcome counts for a finished sync run; mirrors syncFinished() parameters. */ + /** @brief Outcome counts for a finished sync run, carried by syncFinished(). */ struct SyncSummary { int uploaded = 0; @@ -56,10 +56,8 @@ public: explicit DeckSyncManager(QObject *parent = nullptr); DeckSyncManager(AbstractClient *client, ColorIdentityProvider provider, QObject *parent = nullptr); - void setClient(AbstractClient *_client); void setProfile(const DeckSyncProfile &_profile); void setDeckPath(const QString &path); - void setColorIdentityProvider(ColorIdentityProvider provider); [[nodiscard]] bool isSyncing() const; public slots: @@ -68,7 +66,7 @@ public slots: signals: void syncStarted(); void statusMessage(const QString &message); - void syncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + void syncFinished(const SyncSummary &summary, const QString &error); private slots: void onListFinished(const Response &response, const CommandContainer &commandContainer); @@ -111,6 +109,7 @@ private: void failSync(const QString &error); void finishSync(); + [[nodiscard]] SyncSummary currentSummary() const; void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, QMap &files, diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 50b77a66e..9e800e5d4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -566,7 +566,7 @@ void TabDeckStorage::onSyncStatusMessage(const QString &message) renderSyncStatus(); } -void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error) +void TabDeckStorage::onSyncFinished(const SyncSummary &summary, const QString &error) { aSyncNow->setEnabled(remoteEnabled); if (!error.isEmpty()) { @@ -575,7 +575,7 @@ void TabDeckStorage::onSyncFinished(int uploaded, int downloaded, int unchanged, } else { syncStatusType = SyncStatusType::Finished; syncStatusDetail.clear(); - lastSyncSummary = {uploaded, downloaded, unchanged, skipped}; + lastSyncSummary = summary; } renderSyncStatus(); if (error.isEmpty()) { diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index ade10e951..c7456877e 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -8,8 +8,8 @@ #ifndef TAB_DECK_STORAGE_H #define TAB_DECK_STORAGE_H -#include "../deck_sync/deck_sync_manager.h" -#include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" +#include "../../deck_sync/deck_sync_manager.h" +#include "../server/remote/remote_decklist_tree_widget.h" #include "tab.h" #include @@ -129,7 +129,7 @@ private slots: void actSyncNow(); void onSyncStatusMessage(const QString &message); - void onSyncFinished(int uploaded, int downloaded, int unchanged, int skipped, const QString &error); + void onSyncFinished(const SyncSummary &summary, const QString &error); public: TabDeckStorage(TabSupervisor *_tabSupervisor, AbstractClient *_client, const ServerInfo_User *currentUserInfo); From 543cc16d83b73e7bba75170d531b50f0476eed87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 12:43:02 +0100 Subject: [PATCH 13/16] [DeckSync] Match server rows by identity and hash the whole deck Sync keyed local files to server rows by path, but the server names a row after the deck, not after the file, so any deck whose file name and deck name differ uploaded again on every run. A second pass then brought the server copy back as an extra local file. - Match by the sidecar's row id, then guid, then content, and only then by the server path, so renaming either side no longer duplicates a deck. - Key the server tree by row id: the server allows several rows with the same name in one folder, and keying by path silently dropped all but one. - Stop discarding rows whose name contains '/' ("U/W Control"). They stay matchable, and only the download file name is sanitised. - Build the download path from the folder plus a sanitised row name, including every folder segment, so a hostile server name cannot escape the deck folder. - Download a colliding server row under a unique name and count it, instead of skipping it without a trace. - Hash the whole deck instead of the card hash: SHA-1 over the native deck string covers name, printings, banner, tokens and sideboard plans. The client and the server hash the exact bytes they exchange, so both agree. Adds deck_content_hash_test for the client/server hashing invariant and for the name-only change the card hash cannot see. --- .../interface/deck_sync/deck_sync_manager.cpp | 195 +++++++++++------- .../interface/deck_sync/deck_sync_manager.h | 16 +- .../libcockatrice/deck_list/deck_list.cpp | 11 + .../libcockatrice/deck_list/deck_list.h | 10 + servatrice/src/serversocketinterface.cpp | 4 +- tests/CMakeLists.txt | 7 + tests/deck_content_hash_test.cpp | 51 +++++ 7 files changed, 213 insertions(+), 81 deletions(-) create mode 100644 tests/deck_content_hash_test.cpp diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index bef2739d6..f80fe6c59 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 @@ -36,6 +37,35 @@ QString serverPathFor(const QString &relativePath) } return path; } + +// Rows are named after the deck, not after the file, so a row name may contain +// characters a file name cannot ("U/W Control"). Downloads therefore rebuild the +// local path from the folder plus a sanitised row name, and never from the raw +// server-supplied path. +QString sanitizedSegment(const QString &segment) +{ + QString safe = segment; + safe.replace('/', '-'); + safe.replace('\\', '-'); + safe = safe.trimmed(); + if (safe.isEmpty() || safe == QStringLiteral(".") || safe == QStringLiteral("..")) { + return QStringLiteral("deck"); + } + return safe; +} + +QString localPathFor(const QString &dir, const QString &name) +{ + // Every segment is server-supplied, so each one is sanitised before it can + // become part of a local path. + QStringList segments; + for (const QString &segment : dir.split('/', Qt::SkipEmptyParts)) { + segments << sanitizedSegment(segment); + } + segments << sanitizedSegment(name); + const QString path = segments.join('/'); + return path.endsWith(".cod", Qt::CaseInsensitive) ? path : path + QStringLiteral(".cod"); +} } // namespace DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) @@ -111,7 +141,7 @@ void DeckSyncManager::syncNow() continue; } localFiles.insert(relativePath, LocalFile{.absolutePath = absolutePath, - .hash = loaded->deckList.getDeckHash(), + .hash = loaded->deckList.getContentHash(), .modified = QFileInfo(absolutePath).lastModified()}); } @@ -123,28 +153,30 @@ void DeckSyncManager::syncNow() void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, - QMap &files, + QMap &files, QSet &dirs) { for (const auto &item : folder.items()) { const QString name = QString::fromStdString(item.name()); - if (name.isEmpty() || name == "." || name == ".." || name.contains('/') || name.contains('\\')) { - continue; - } + const QString path = prefix.isEmpty() ? name : prefix + "/" + name; if (item.has_folder()) { - const QString path = prefix.isEmpty() ? name : prefix + "/" + name; dirs.insert(path); collectServerFiles(item.folder(), path, files, dirs); } else if (item.has_file()) { - const QString path = prefix.isEmpty() ? name : prefix + "/" + name; const ServerInfo_DeckStorage_File &file = item.file(); + const int id = static_cast(item.id()); ServerFile entry{ - .id = static_cast(item.id()), + .id = id, + .path = path, + .dir = prefix, + .name = name, .guid = QString::fromStdString(file.guid()), .hash = QString::fromStdString(file.hash()), .lastModifiedSecs = file.has_last_modified() ? static_cast(file.last_modified()) : 0, }; - files.insert(path, entry); + // Keyed by id: the server allows several rows with the same name in a + // folder, and keying by path silently dropped all but one of them. + files.insert(id, entry); } } } @@ -159,7 +191,7 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont return; } - QMap serverFiles; + QMap serverFiles; QSet serverDirs; const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); collectServerFiles(list.root(), QString(), serverFiles, serverDirs); @@ -169,57 +201,78 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); const qint64 nowSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); + // 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) { + const DeckSyncRecord record = sidecar.get(relativePath); + return record.guid.isEmpty() ? QUuid::createUuid().toString(QUuid::WithoutBraces) : record.guid; + }; - QMap serverPathById; - QMap serverPathByGuid; + QMap serverIdByPath; + QMap serverIdByGuid; for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { - serverPathById.insert(sit.value().id, sit.key()); + serverIdByPath.insert(sit.value().path, sit.key()); if (!sit.value().guid.isEmpty()) { - serverPathByGuid.insert(sit.value().guid, sit.key()); + serverIdByGuid.insert(sit.value().guid, sit.key()); } } - QSet matchedServerPaths; + QSet matchedServerIds; + + // A local file without a usable sidecar record still usually has a server + // counterpart: rows are named after the deck, so compare content rather than + // file names, and only upload when no unmatched row has the same content. + auto unmatchedByHash = [&serverFiles, &matchedServerIds](const QString &hash) { + if (hash.isEmpty()) { + return -1; + } + for (auto sit = serverFiles.begin(); sit != serverFiles.end(); ++sit) { + if (!matchedServerIds.contains(sit.key()) && sit.value().hash == hash) { + return sit.key(); + } + } + return -1; + }; for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { const DeckSyncRecord sidecarRecord = sidecar.get(it.key()); - QString serverPath; - if (sidecarRecord.remoteId != -1 && serverPathById.contains(sidecarRecord.remoteId)) { - serverPath = serverPathById.value(sidecarRecord.remoteId); - } else if (!sidecarRecord.guid.isEmpty() && serverPathByGuid.contains(sidecarRecord.guid)) { - serverPath = serverPathByGuid.value(sidecarRecord.guid); + int serverId = -1; + if (sidecarRecord.remoteId != -1 && serverFiles.contains(sidecarRecord.remoteId)) { + serverId = sidecarRecord.remoteId; + } else if (!sidecarRecord.guid.isEmpty() && serverIdByGuid.contains(sidecarRecord.guid)) { + serverId = serverIdByGuid.value(sidecarRecord.guid); } else { - serverPath = serverPathFor(it.key()); - if (!serverFiles.contains(serverPath)) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = -1; - op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() - ? sidecar.get(it.key()).guid - : QUuid::createUuid().toString(QUuid::WithoutBraces); - pendingUploads.append(op); - continue; + // Content first: a row with the same deck is unambiguous, while a + // matching file name may belong to a different deck entirely. + serverId = unmatchedByHash(it.value().hash); + if (serverId == -1) { + const QString wantedPath = serverPathFor(it.key()); + if (serverIdByPath.contains(wantedPath)) { + const int pathId = serverIdByPath.value(wantedPath); + if (!matchedServerIds.contains(pathId)) { + serverId = pathId; + } + } } } - auto serverIt = serverFiles.find(serverPath); - if (serverIt == serverFiles.end()) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = -1; - op.guid = sidecar.contains(it.key()) && !sidecar.get(it.key()).guid.isEmpty() - ? sidecar.get(it.key()).guid - : QUuid::createUuid().toString(QUuid::WithoutBraces); - pendingUploads.append(op); + + if (serverId == -1) { + const QString serverPath = serverPathFor(it.key()); + pendingUploads.append(UploadOp{.relativePath = it.key(), + .serverPath = serverPath, + .serverDir = parentDir(serverPath), + .deckId = -1, + .guid = guidFor(it.key())}); continue; } - matchedServerPaths.insert(serverIt.key()); - if (it.value().hash == serverIt.value().hash) { + + const ServerFile &server = serverFiles.value(serverId); + matchedServerIds.insert(serverId); + if (it.value().hash == server.hash) { DeckSyncRecord record = sidecar.get(it.key()); if (record.guid.isEmpty()) { - record.guid = serverIt.value().guid; + record.guid = server.guid; } - record.remoteId = serverIt.value().id; + record.remoteId = server.id; record.baseHash = it.value().hash; record.lastSyncSecs = nowSecs; record.deleted = false; @@ -230,44 +283,36 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); // A server row without last_modified predates sync: the local file wins. // Otherwise strictly newer wins; ties go to the server. - if (serverIt.value().lastModifiedSecs == 0 || localSecs > serverIt.value().lastModifiedSecs) { - UploadOp op; - op.relativePath = it.key(); - op.serverPath = serverPath; - op.deckId = serverIt.value().id; - op.guid = serverIt.value().guid; - pendingUploads.append(op); + if (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs) { + pendingUploads.append(UploadOp{.relativePath = it.key(), + .serverPath = server.path, + .serverDir = server.dir, + .deckId = server.id, + .guid = server.guid}); } else { - DownloadOp op; - op.relativePath = it.key(); - op.deckId = serverIt.value().id; - op.guid = serverIt.value().guid; - op.hash = serverIt.value().hash; - op.lastModifiedSecs = serverIt.value().lastModifiedSecs; - pendingDownloads.append(op); + pendingDownloads.append(DownloadOp{.relativePath = it.key(), + .deckId = server.id, + .guid = server.guid, + .hash = server.hash, + .lastModifiedSecs = server.lastModifiedSecs}); } } for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { - if (matchedServerPaths.contains(it.key())) { - continue; - } - const QString wantedPath = it.key().endsWith(".cod", Qt::CaseInsensitive) ? it.key() : it.key() + ".cod"; - if (localFiles.contains(wantedPath) || QFileInfo(deckPath + "/" + wantedPath).exists()) { + if (matchedServerIds.contains(it.key())) { continue; } + const QString wantedPath = localPathFor(it.value().dir, it.value().name); const QString relativePath = uniqueLocalPath(wantedPath); if (relativePath.isEmpty()) { ++skippedCount; continue; } - DownloadOp op; - op.relativePath = relativePath; - op.deckId = it.value().id; - op.guid = it.value().guid; - op.hash = it.value().hash; - op.lastModifiedSecs = it.value().lastModifiedSecs; - pendingDownloads.append(op); + pendingDownloads.append(DownloadOp{.relativePath = relativePath, + .deckId = it.value().id, + .guid = it.value().guid, + .hash = it.value().hash, + .lastModifiedSecs = it.value().lastModifiedSecs}); } sidecar.save(); @@ -297,7 +342,7 @@ void DeckSyncManager::processNext() void DeckSyncManager::processNextUpload() { const UploadOp op = pendingUploads.first(); - const QString dir = parentDir(op.serverPath); + const QString dir = op.serverDir; if (!ensuredDirs.contains(dir)) { sendMkDirFor(dir); return; @@ -370,7 +415,7 @@ void DeckSyncManager::sendUpload(const UploadOp &op) Command_DeckUpload cmd; if (op.deckId == -1) { - cmd.set_path(parentDir(op.serverPath).toStdString()); + cmd.set_path(op.serverDir.toStdString()); } else { cmd.set_deck_id(op.deckId); } @@ -416,7 +461,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, } else { record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); } - record.remoteId = file.id(); + record.remoteId = static_cast(file.id()); } else if (localFiles.contains(relativePath)) { record.baseHash = localFiles.value(relativePath).hash; record.lastSyncSecs = QDateTime::currentDateTime().toSecsSinceEpoch(); @@ -517,7 +562,7 @@ void DeckSyncManager::onDownloadFinished(const Response &response, if (!download.hash().empty()) { record.baseHash = QString::fromStdString(download.hash()); } else { - record.baseHash = deck.getDeckHash(); + record.baseHash = deck.getContentHash(); } record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) : QDateTime::currentDateTime().toSecsSinceEpoch(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 8ecbaf74c..c891cbb58 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -30,9 +30,11 @@ class ServerInfo_DeckStorage_Folder; * Each syncNow() run loads the profile sidecar, scans the deck folder, fetches * the server tree and mirrors both sides: unknown local decks upload, unknown * server decks download, and decks changed on both sides resolve newer-wins - * (ties go to the server). Matching is path-based; a rename looks like a - * delete plus an add. Before a download overwrites a locally modified file, the - * local content is copied to a .sync-backup file next to it. + * (ties go to the server). A local file is matched to its server row by the + * sidecar's row id, then guid, then the server path, and finally by content, so + * renaming either side does not duplicate a deck. Before a download overwrites a + * locally modified file, the local content is copied to a .sync-backup file next + * to it. * * Ownership: fire-and-forget per run, driven by PendingCommand round trips. * No watcher, no auto triggers, no conflict UI — those arrive in later layers. @@ -81,6 +83,9 @@ private: struct ServerFile { int id = -1; + QString path; ///< Server-side address: folder prefix plus row name. + QString dir; ///< Folder prefix, empty in the root. + QString name; ///< Row name exactly as the server stores it. QString guid; QString hash; qint64 lastModifiedSecs = 0; @@ -94,7 +99,8 @@ private: struct UploadOp { QString relativePath; - QString serverPath; + QString serverPath; ///< Target address of a brand-new row. + QString serverDir; ///< Folder that must exist before the upload. int deckId = -1; QString guid; }; @@ -112,7 +118,7 @@ private: [[nodiscard]] SyncSummary currentSummary() const; void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, - QMap &files, + QMap &files, QSet &dirs); void processNext(); void processNextUpload(); diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 821b53dff..6f1a358f2 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -9,6 +9,7 @@ #include "tree/inner_deck_list_node.h" #include +#include #include #include #include @@ -453,6 +454,16 @@ void DeckList::refreshDeckHash() cachedDeckHash = QString(); } +QString DeckList::contentHash(const QString &nativeDeck) +{ + return QString::fromLatin1(QCryptographicHash::hash(nativeDeck.toUtf8(), QCryptographicHash::Sha1).toHex()); +} + +QString DeckList::getContentHash() const +{ + return contentHash(writeToString_Native()); +} + /** * Calls a given function on each card in the deck. */ diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index 6796e9c33..9a8b54c24 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h @@ -289,6 +289,16 @@ public: /** @name Deck identity */ ///@{ QString getDeckHash() const; + /** + * @brief SHA-1 of a native deck string, covering everything the card hash + * ignores (name, printings, banner, tokens, comments, sideboard plans). + * + * Deck-collection sync hashes the exact string it sends or receives, so both + * peers compare the same bytes. + */ + static QString contentHash(const QString &nativeDeck); + /** @brief contentHash() of this deck's native serialisation. */ + QString getContentHash() const; void refreshDeckHash(); ///@} diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 4c58748a6..505d1cffd 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1088,7 +1088,9 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman const QString bannerCardProvider = deck.getBannerCard().providerId.left(32); const QString tagsJson = serializeDeckTags(deck.getTags()); const QString colorIdentity = sanitizeColorIdentity(nameFromStdString(cmd.color_identity())); - const QString deckHash = deck.getDeckHash(); + // Hash the exact bytes the client sent: both peers must compare the same + // string, not each side's re-serialisation of it. + const QString deckHash = DeckList::contentHash(deckStr); QString guid = cmd.has_guid() ? nameFromStdString(cmd.guid()).left(36) : QString(); if (QUuid::fromString(guid).isNull()) { guid = QUuid::createUuid().toString(QUuid::WithoutBraces); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1823f5b17..e19379513 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -28,6 +28,7 @@ add_test(NAME loader_local_matching_test COMMAND loader_local_matching_test) add_test(NAME deck_hash_performance_test COMMAND deck_hash_performance_test) set_tests_properties(deck_hash_performance_test PROPERTIES TIMEOUT 15) add_test(NAME deck_diff_test COMMAND deck_diff_test) +add_test(NAME deck_content_hash_test COMMAND deck_content_hash_test) add_test(NAME deck_sync_profile_test COMMAND deck_sync_profile_test) add_test(NAME deck_sync_sidecar_test COMMAND deck_sync_sidecar_test) @@ -41,6 +42,7 @@ add_executable(password_hash_test password_hash_test.cpp) add_executable(playmat_resolver_test playmat_resolver_test.cpp) add_executable(deck_hash_performance_test deck_hash_performance_test.cpp) add_executable(deck_diff_test deck_diff_test.cpp) +add_executable(deck_content_hash_test deck_content_hash_test.cpp) add_executable( deck_sync_profile_test ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/deck_sync/deck_sync_profile.cpp deck_sync_profile_test.cpp @@ -152,6 +154,7 @@ if(NOT GTEST_FOUND) add_dependencies(playmat_resolver_test gtest) add_dependencies(deck_hash_performance_test gtest) add_dependencies(deck_diff_test gtest) + add_dependencies(deck_content_hash_test gtest) add_dependencies(deck_sync_profile_test gtest) add_dependencies(deck_sync_sidecar_test gtest) add_dependencies(server_card_counter_test gtest) @@ -196,6 +199,10 @@ target_link_libraries( deck_diff_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES} ) +target_link_libraries( + deck_content_hash_test libcockatrice_deck_list libcockatrice_utility Threads::Threads ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) target_link_libraries(deck_sync_profile_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) target_link_libraries(deck_sync_sidecar_test Threads::Threads ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES}) target_link_libraries( diff --git a/tests/deck_content_hash_test.cpp b/tests/deck_content_hash_test.cpp new file mode 100644 index 000000000..192abb007 --- /dev/null +++ b/tests/deck_content_hash_test.cpp @@ -0,0 +1,51 @@ +#include "gtest/gtest.h" +#include +#include + +namespace +{ +QString deckWithName(const QString &name) +{ + return QStringLiteral(R"()" + R"(%1)" + R"()" + R"()" + R"()") + .arg(name); +} +} // namespace + +TEST(DeckContentHashTest, MatchesNativeSerialisation) +{ + DeckList deck; + ASSERT_TRUE(deck.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + EXPECT_EQ(deck.getContentHash(), DeckList::contentHash(deck.writeToString_Native())); +} + +TEST(DeckContentHashTest, IsStableAcrossLoads) +{ + DeckList first; + DeckList second; + ASSERT_TRUE(first.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + ASSERT_TRUE(second.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + EXPECT_EQ(first.getContentHash(), second.getContentHash()); +} + +TEST(DeckContentHashTest, CoversDeckName) +{ + DeckList burn; + DeckList renamed; + ASSERT_TRUE(burn.loadFromString_Native(deckWithName(QStringLiteral("Burn")))); + ASSERT_TRUE(renamed.loadFromString_Native(deckWithName(QStringLiteral("Burn v2")))); + + // The card hash only looks at card names and counts, so it cannot tell the + // two decks apart; the content hash must. + EXPECT_EQ(burn.getDeckHash(), renamed.getDeckHash()); + EXPECT_NE(burn.getContentHash(), renamed.getContentHash()); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 07782462f86547ac21ac04811be65d4b1073542b 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 14/16] [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 f80fe6c59..437276b0a 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 @@ -114,6 +115,7 @@ void DeckSyncManager::syncNow() skippedCount = 0; pendingUploads.clear(); pendingDownloads.clear(); + pendingDeletes.clear(); ensuredDirs.clear(); ensuredDirs.insert(QString()); localFiles.clear(); @@ -254,8 +256,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, @@ -277,6 +292,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; } @@ -315,6 +331,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(); } @@ -330,7 +358,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()); @@ -425,7 +458,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); } @@ -437,8 +475,22 @@ void DeckSyncManager::onUploadFinished(const Response &response, if (!syncing) { 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; @@ -469,6 +521,7 @@ void DeckSyncManager::onUploadFinished(const Response &response, record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); + snapshotLocalFile(relativePath, record.guid); ++uploadedCount; processNext(); } @@ -569,10 +622,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 c891cbb58..4d61c0d0a 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; @@ -139,6 +147,7 @@ private: QSet ensuredDirs; 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 209f616f7..e01dc5066 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -488,3 +488,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 a979a74b2..0c9cf7cee 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -460,6 +460,7 @@ 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"; @@ -689,6 +690,11 @@ void Servatrice::cleanupExpiredDeckShares() servatriceDatabaseInterface->cleanupExpiredDeckShares(); } +void Servatrice::purgeDeckTombstones() +{ + servatriceDatabaseInterface->purgeDeckTombstones(getDeckSyncTombstoneDays()); +} + void Servatrice::statusUpdate() { if (!servatriceDatabaseInterface->checkSql()) { @@ -1127,6 +1133,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 a6bbacfc6..64361fb4a 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -157,6 +157,7 @@ private slots: void statusUpdate(); void shutdownTimeout(); void cleanupExpiredDeckShares(); + void purgeDeckTombstones(); protected: void doSendIslMessage(const IslMessage &msg, int _serverId) override; @@ -293,6 +294,7 @@ public: int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; + int getDeckSyncTombstoneDays() const; int getMaxUsersPerAddress() const; int getMessageCountingInterval() const override; int getMaxMessageCountPerInterval() const override; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index ac3a19c8e..832d99efa 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -1088,8 +1088,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); @@ -1240,6 +1240,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 ee8074171..9196508b3 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -140,6 +140,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 505d1cffd..622cb87f9 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -683,7 +683,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)) { @@ -828,8 +829,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); @@ -939,7 +940,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); @@ -990,18 +992,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; } @@ -1171,9 +1172,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"; } @@ -1375,7 +1378,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 884501d416219b6a13edd69070d98f42b1fbac35 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 15/16] 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 437276b0a..0c1489792 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -336,6 +336,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 428bc32b8b1047f75fd78391e3f283487342e111 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 16/16] [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 | 22 ++++- servatrice/servatrice.ini.example | 12 ++- servatrice/src/servatrice.cpp | 17 +++- servatrice/src/servatrice.h | 2 + servatrice/src/serversocketinterface.cpp | 93 ++++++++++++++----- 5 files changed, 112 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 0c1489792..6886989c2 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -4,10 +4,12 @@ #include "../deck_loader/deck_loader.h" #include "deck_sync_sidecar.h" +#include #include #include #include #include +#include #include #include #include @@ -257,9 +259,14 @@ 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. + // 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()); @@ -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"); - 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 e01dc5066..36eec7491 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -490,8 +490,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 0c9cf7cee..d58b116c7 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -460,13 +460,22 @@ 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"; deckShareCleanupClock->start(deckShareCleanupInterval); } + // 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 = @@ -1138,6 +1147,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 64361fb4a..b6dc38353 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -172,6 +172,7 @@ private: DatabaseType databaseType; QTimer *pingClock, *statusUpdateClock; QTimer *deckShareCleanupClock; + QTimer *deckTombstoneClock; Servatrice_GameServer *gameServer; Servatrice_WebsocketGameServer *websocketGameServer; Servatrice_IslServer *islServer; @@ -295,6 +296,7 @@ public: int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; int getDeckSyncTombstoneDays() const; + int getDeckSyncPurgeInterval() const; int getMaxUsersPerAddress() const; int getMessageCountingInterval() const override; int getMaxMessageCountPerInterval() const override; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 622cb87f9..df86bc154 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1103,31 +1103,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; } @@ -1137,9 +1177,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; }