From 3bed5ae5f5c1add38a9dab8e2a2691ef185fc565 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 01/18] [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 d76d89211..5519d2f3b 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -28,6 +28,7 @@ set(cockatrice_SOURCES src/interface/deck_loader/deck_loader.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 3dfdc956b..90cd24290 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -2,6 +2,7 @@ #include "../../../client/settings/cache_settings.h" #include "../../deck_loader/deck_loader.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" @@ -29,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -177,6 +179,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); @@ -228,6 +233,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); @@ -239,11 +248,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); @@ -272,6 +292,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(); @@ -299,6 +321,7 @@ QString TabDeckStorage::getTargetPath() const void TabDeckStorage::handleConnected(const ServerInfo_User &userInfo) { + syncUsername = QString::fromStdString(userInfo.name()); setRemoteEnabled(userInfo.user_level() & ServerInfo_User::IsRegistered); } @@ -316,6 +339,7 @@ void TabDeckStorage::handleConnectionChanged(ClientStatus status) void TabDeckStorage::setRemoteEnabled(bool enabled) { + remoteEnabled = enabled; aUpload->setEnabled(enabled); aOpenRemoteDeck->setEnabled(enabled); aDownload->setEnabled(enabled); @@ -323,6 +347,7 @@ void TabDeckStorage::setRemoteEnabled(bool enabled) aPublishDeck->setEnabled(enabled); aNewFolder->setEnabled(enabled); aDeleteRemoteDeck->setEnabled(enabled); + aSyncNow->setEnabled(enabled && !syncManager->isSyncing()); if (enabled) { serverDirView->refreshTree(); @@ -526,6 +551,61 @@ void TabDeckStorage::actOpenDecksFolder() QtUtils::openPathInFileManager(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 0ba9bd531be9656d0ec805ffd2eb6b473e37ac8e 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 02/18] 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 d4ce4759c5966a4133799b56cd182cf4b0f8ea96 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:29 +0200 Subject: [PATCH 03/18] [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 0cd238d296b85be697ed7c0275327a3abbeb8a6c Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:44 +0200 Subject: [PATCH 04/18] [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 90cd24290..74154f3da 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -254,10 +254,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 f22256886fa0f21f9ec441cf6074d800c8f00ed2 Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:45:54 +0200 Subject: [PATCH 05/18] [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 f9cbeb2f21ec9294fb9eb7300d82d995c4fd09ef Mon Sep 17 00:00:00 2001 From: BruebachL Date: Tue, 13 Oct 2026 11:50:40 +0200 Subject: [PATCH 06/18] [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 4fd03576e5737e6c9980880486252fa1702e2227 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 07/18] [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 74154f3da..2ca46d4f4 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -565,7 +565,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()) { @@ -574,7 +574,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 8d0468402e4ead3cf666bee5462913393251277f 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 08/18] [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 | 9 + tests/deck_content_hash_test.cpp | 51 +++++ 7 files changed, 215 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 e21b3b10f..f9e602f82 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1114,7 +1114,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 32ee9b7f1..5163db067 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -416,6 +416,15 @@ cockatrice_add_test( libcockatrice_deck_list libcockatrice_utility ) +cockatrice_add_test( + NAME + deck_content_hash_test + SOURCES + deck_content_hash_test.cpp + LIBS + libcockatrice_deck_list + libcockatrice_utility +) cockatrice_add_test( NAME deck_sync_profile_test 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 34844c52b0f678226e47dfc3cc19d935d581c7aa 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 09/18] [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 4a26ba1d9..08046a489 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -598,3 +598,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 feb178b08..f5a1f0e93 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -622,6 +622,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"; @@ -856,6 +857,11 @@ void Servatrice::cleanupExpiredDeckShares() servatriceDatabaseInterface->cleanupExpiredDeckShares(); } +void Servatrice::purgeDeckTombstones() +{ + servatriceDatabaseInterface->purgeDeckTombstones(getDeckSyncTombstoneDays()); +} + void Servatrice::statusUpdate() { if (!servatriceDatabaseInterface->checkSql()) { @@ -1401,6 +1407,11 @@ int Servatrice::getDeckSyncMaxVersions() const return qMax(1, settingsCache->value("deck_sync/max_versions", 20).toInt()); } +int Servatrice::getDeckSyncTombstoneDays() const +{ + return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index 3be364aa3..f925c0984 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -166,6 +166,7 @@ private slots: void sweepLegacyBackups(); void shutdownTimeout(); void cleanupExpiredDeckShares(); + void purgeDeckTombstones(); protected: void doSendIslMessage(const IslMessage &msg, int _serverId) override; @@ -321,6 +322,7 @@ public: int getDeckShareMaxDecksPerShare() const; int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; + int getDeckSyncTombstoneDays() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index b5c45065c..8d5459a88 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -1370,8 +1370,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); @@ -1522,6 +1522,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 d0fd8bd12..04005e160 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -152,6 +152,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 f9e602f82..36099e394 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -709,7 +709,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)) { @@ -854,8 +855,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); @@ -965,7 +966,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); @@ -1016,18 +1018,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; } @@ -1197,9 +1198,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"; } @@ -1401,7 +1404,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 f1f92880faa4b74e189a3c6358898fb97a3daad3 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 10/18] 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 b0f5cb4c42d0deb1863b267bc7d38da8877ed61d 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 11/18] [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 08046a489..5edb38695 100644 --- a/servatrice/servatrice.ini.example +++ b/servatrice/servatrice.ini.example @@ -600,8 +600,14 @@ max_shares_per_day=50 max_versions=20 ; How many days a deleted deck stays restorable as a tombstone before the -; cleanup clock purges it (with its versions) for good. Re-uploading the -; 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 f5a1f0e93..34e5573c0 100644 --- a/servatrice/src/servatrice.cpp +++ b/servatrice/src/servatrice.cpp @@ -622,7 +622,6 @@ bool Servatrice::initServer() deckShareCleanupClock = new QTimer(this); connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(cleanupExpiredDeckShares())); - connect(deckShareCleanupClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones())); const int deckShareCleanupInterval = getDeckShareCleanupInterval(); if (deckShareCleanupInterval > 0) { qDebug() << "Starting deck share cleanup clock, interval" << deckShareCleanupInterval << "ms"; @@ -633,6 +632,16 @@ bool Servatrice::initServer() connect(legacyBackupSweepClock, &QTimer::timeout, this, &Servatrice::sweepLegacyBackups); legacyBackupSweepClock->start(kLegacyBackupSweepIntervalMs); + // Tombstone purging runs on its own clock: turning share cleanup off must not + // silently disable it as well. + deckTombstoneClock = new QTimer(this); + connect(deckTombstoneClock, SIGNAL(timeout()), this, SLOT(purgeDeckTombstones())); + const int deckSyncPurgeInterval = getDeckSyncPurgeInterval(); + if (deckSyncPurgeInterval > 0) { + qDebug() << "Starting deck tombstone purge clock, interval" << deckSyncPurgeInterval << "ms"; + deckTombstoneClock->start(deckSyncPurgeInterval); + } + // SOCKET SERVER if (getNumberOfTCPPools() > 0) { gameServer = @@ -1412,6 +1421,12 @@ int Servatrice::getDeckSyncTombstoneDays() const return qMax(0, settingsCache->value("deck_sync/tombstone_days", 30).toInt()); } +int Servatrice::getDeckSyncPurgeInterval() const +{ + // default: every 60 minutes, independent of deck_share/cleanup_interval + return qMax(0, settingsCache->value("deck_sync/purge_interval", 60).toInt()) * 60000; +} + int Servatrice::getNumberOfTCPPools() const { return settingsCache->value("server/number_pools", 1).toInt(); diff --git a/servatrice/src/servatrice.h b/servatrice/src/servatrice.h index f925c0984..ed8d82ddb 100644 --- a/servatrice/src/servatrice.h +++ b/servatrice/src/servatrice.h @@ -182,6 +182,7 @@ private: QTimer *pingClock, *statusUpdateClock; QTimer *deckShareCleanupClock; QTimer *legacyBackupSweepClock; + QTimer *deckTombstoneClock; Servatrice_GameServer *gameServer; Servatrice_WebsocketGameServer *websocketGameServer; Servatrice_IslServer *islServer; @@ -323,6 +324,7 @@ public: int getDeckShareMaxSharesPerDay() const; int getDeckSyncMaxVersions() const; int getDeckSyncTombstoneDays() const; + int getDeckSyncPurgeInterval() const; int getMaxUsersPerAddress() const; /// Raw security/trusted_sources list; match addresses against it with isTrustedSource(). QString getTrustedSources() const; diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 36099e394..e88c74527 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -1129,31 +1129,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; } @@ -1163,9 +1203,12 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckUpload(const Comman stampQuery->bindValue(":id", deckId); stampQuery->bindValue(":id_user", userInfo->id()); if (!sqlInterface->execSqlQuery(stampQuery) || !stampQuery->next()) { - QSqlQuery *cleanupQuery = sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); - cleanupQuery->bindValue(":id", deckId); - sqlInterface->execSqlQuery(cleanupQuery); + if (inserted) { + QSqlQuery *cleanupQuery = + sqlInterface->prepareQuery("delete from {prefix}_decklist_files where id = :id"); + cleanupQuery->bindValue(":id", deckId); + sqlInterface->execSqlQuery(cleanupQuery); + } return Response::RespContextError; } From 339d712308b7696e9b5ae43a134afd9953703ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 16:46:46 +0200 Subject: [PATCH 12/18] [Client] Sync conflict dialog, onboarding and conflict default --- cockatrice/CMakeLists.txt | 2 + .../src/client/settings/cache_settings.cpp | 7 + .../src/client/settings/cache_settings.h | 3 + .../interface/deck_sync/deck_sync_conflict.h | 50 +++ .../interface/deck_sync/deck_sync_manager.cpp | 401 ++++++++++++++++-- .../interface/deck_sync/deck_sync_manager.h | 68 ++- .../interface/deck_sync/deck_sync_sidecar.cpp | 9 + .../interface/deck_sync/deck_sync_sidecar.h | 9 +- .../widgets/dialogs/dlg_sync_conflict.cpp | 178 ++++++++ .../widgets/dialogs/dlg_sync_conflict.h | 63 +++ .../widgets/dialogs/dlg_sync_onboarding.cpp | 59 +++ .../widgets/dialogs/dlg_sync_onboarding.h | 43 ++ .../settings_page/general_settings_page.cpp | 29 ++ .../settings_page/general_settings_page.h | 4 + .../widgets/tabs/tab_deck_storage.cpp | 38 ++ .../interface/widgets/tabs/tab_deck_storage.h | 4 + libcockatrice_settings/CMakeLists.txt | 2 + .../settings/deck_sync_settings.cpp | 21 + .../settings/deck_sync_settings.h | 38 ++ tests/deck_sync_sidecar_test.cpp | 20 + tests/settings/settings_defaults_test.cpp | 20 + 21 files changed, 1020 insertions(+), 48 deletions(-) create mode 100644 cockatrice/src/interface/deck_sync/deck_sync_conflict.h create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h create mode 100644 libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp create mode 100644 libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 5519d2f3b..7ffe97994 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -58,6 +58,8 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_shared_decks_preview.cpp src/interface/widgets/dialogs/dlg_settings.cpp src/interface/widgets/dialogs/dlg_startup_card_check.cpp + src/interface/widgets/dialogs/dlg_sync_conflict.cpp + src/interface/widgets/dialogs/dlg_sync_onboarding.cpp src/interface/widgets/dialogs/dlg_tip_of_the_day.cpp src/interface/widgets/dialogs/dlg_update.cpp src/interface/widgets/dialogs/dlg_view_log.cpp diff --git a/cockatrice/src/client/settings/cache_settings.cpp b/cockatrice/src/client/settings/cache_settings.cpp index 08eccf1e4..6848e7b04 100644 --- a/cockatrice/src/client/settings/cache_settings.cpp +++ b/cockatrice/src/client/settings/cache_settings.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -159,6 +160,7 @@ SettingsCache::SettingsCache() cardsDisplaySettings = new CardsDisplaySettings(settingsPath, this); interfaceSettings = new InterfaceSettings(settingsPath, this); deckEditorSettings = new DeckEditorSettings(settingsPath, this); + deckSyncSettings = new DeckSyncSettings(settingsPath, this); pathsSettings = new PathsSettings(settingsPath, this); visualDeckStorageSettings = new VisualDeckStorageSettings(settingsPath, this); appearanceSettings = new AppearanceSettings(settingsPath, this); @@ -460,6 +462,11 @@ DeckEditorSettings &SettingsCache::deckEditor() const return *deckEditorSettings; } +DeckSyncSettings &SettingsCache::deckSync() const +{ + return *deckSyncSettings; +} + VisualDeckStorageSettings &SettingsCache::visualDeckStorage() const { return *visualDeckStorageSettings; diff --git a/cockatrice/src/client/settings/cache_settings.h b/cockatrice/src/client/settings/cache_settings.h index 9aeb88a18..97198466f 100644 --- a/cockatrice/src/client/settings/cache_settings.h +++ b/cockatrice/src/client/settings/cache_settings.h @@ -32,6 +32,7 @@ class ChatSettings; class CommanderBracketSettings; class DebugSettings; class DeckEditorSettings; +class DeckSyncSettings; class DownloadSettings; class GameFiltersSettings; class GameSettings; @@ -81,6 +82,7 @@ private: CardsDisplaySettings *cardsDisplaySettings = nullptr; InterfaceSettings *interfaceSettings = nullptr; DeckEditorSettings *deckEditorSettings = nullptr; + DeckSyncSettings *deckSyncSettings = nullptr; PathsSettings *pathsSettings = nullptr; VisualDeckStorageSettings *visualDeckStorageSettings = nullptr; AppearanceSettings *appearanceSettings = nullptr; @@ -149,6 +151,7 @@ public: [[nodiscard]] CardsDisplaySettings &cardsDisplay() const; [[nodiscard]] InterfaceSettings &userInterface() const; [[nodiscard]] DeckEditorSettings &deckEditor() const; + [[nodiscard]] DeckSyncSettings &deckSync() const; [[nodiscard]] PathsSettings &paths() const; [[nodiscard]] VisualDeckStorageSettings &visualDeckStorage() const; [[nodiscard]] AppearanceSettings &appearance() const; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_conflict.h b/cockatrice/src/interface/deck_sync/deck_sync_conflict.h new file mode 100644 index 000000000..a5b272ecd --- /dev/null +++ b/cockatrice/src/interface/deck_sync/deck_sync_conflict.h @@ -0,0 +1,50 @@ +/** + * @file deck_sync_conflict.h + * @brief Conflict payload between the sync engine and the conflict dialog. + */ + +#ifndef DECK_SYNC_CONFLICT_H +#define DECK_SYNC_CONFLICT_H + +#include + +/** @brief How a both-sides-changed deck is resolved. */ +enum DeckSyncResolutionAction +{ + DeckSyncKeepMine, ///< Upload the local deck, discarding the server version. + DeckSyncKeepServer, ///< Download the server deck, discarding local changes. + DeckSyncKeepBoth, ///< Keep the local deck and store the server version aside. + DeckSyncUseMerge, ///< Upload the auto-merged deck (only when mergeable). + DeckSyncSkipConflict, ///< Leave both sides untouched for this run. +}; + +/** @brief Sync direction for one run. */ +enum class DeckSyncMode +{ + TwoWay, ///< Mirror both sides (default). + UploadOnly, ///< Push local content; never download or propagate deletes. + DownloadOnly, ///< Take server content; never upload or push deletes. + Separate, ///< First-sync verdict only: keep this profile out of sync. +}; + +/** @brief One deck that changed both locally and on the server. */ +struct DeckSyncConflict +{ + QString relativePath; ///< Local deck path relative to the deck folder. + QString localAbsolutePath; ///< Absolute local file path. + QString serverDeckString; ///< Server deck content in native format. + QString baseDeckString; ///< Last-synced content, empty when no snapshot exists. + bool hasBase = false; ///< False when the base snapshot is missing. + qint64 localSecs = 0; ///< Local modification time, seconds since epoch. + qint64 serverSecs = 0; ///< Server modification time, seconds since epoch. +}; + +/** @brief The dialog verdict for one DeckSyncConflict. */ +struct DeckSyncResolution +{ + DeckSyncResolutionAction action = DeckSyncKeepMine; + bool rememberSession = false; ///< Reuse this verdict for later conflicts this run. + bool rememberAlways = false; ///< Store this verdict as the conflict default setting. +}; + +#endif // DECK_SYNC_CONFLICT_H diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 6886989c2..19919808b 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 @@ -90,6 +91,16 @@ void DeckSyncManager::setDeckPath(const QString &path) deckPath = path; } +void DeckSyncManager::setConflictDefault(int _conflictDefault) +{ + conflictDefault = _conflictDefault; +} + +void DeckSyncManager::setSyncMode(DeckSyncMode mode) +{ + syncMode = mode; +} + bool DeckSyncManager::isSyncing() const { return syncing; @@ -118,18 +129,29 @@ void DeckSyncManager::syncNow() pendingUploads.clear(); pendingDownloads.clear(); pendingDeletes.clear(); + conflictPaths.clear(); + stashedServerFiles.clear(); + activeConflict = ActiveConflict(); ensuredDirs.clear(); ensuredDirs.insert(QString()); localFiles.clear(); + sessionDefault = -1; + syncMode = DeckSyncMode::TwoWay; + firstSyncRun = false; emit syncStarted(); emit statusMessage(tr("Scanning local decks...")); - sidecarPath = deckDir.filePath(SYNC_META_DIR + QString("/") + profile.key() + ".json"); + sidecarPath = DeckSyncSidecar::pathForDeckPath(deckPath, profile.key()); DeckSyncSidecar sidecar(sidecarPath); if (!sidecar.load()) { failSync(tr("Could not read sync state.")); return; } + if (sidecar.mode == DeckSyncSidecar::MODE_SEPARATE) { + failSync(tr("Sync is disabled for this server.")); + return; + } + firstSyncRun = !QFileInfo(sidecarPath).exists(); QDirIterator it(deckPath, QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); while (it.hasNext()) { @@ -200,6 +222,47 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const Response_DeckList &list = response.GetExtension(Response_DeckList::ext); collectServerFiles(list.root(), QString(), serverFiles, serverDirs); ensuredDirs.unite(serverDirs); + + if (firstSyncRun && !localFiles.isEmpty() && !serverFiles.isEmpty()) { + stashedServerFiles = serverFiles; + emit firstSyncDetected(localFiles.size(), serverFiles.size()); + return; + } + classify(serverFiles); +} + +void DeckSyncManager::continueFirstSync(DeckSyncMode mode) +{ + if (!syncing || !firstSyncRun) { + return; + } + firstSyncRun = false; + if (mode == DeckSyncMode::Separate) { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + sidecar.mode = DeckSyncSidecar::MODE_SEPARATE; + sidecar.save(); + stashedServerFiles.clear(); + failSync(tr("Sync is disabled for this server.")); + return; + } + syncMode = mode; + classify(stashedServerFiles); + stashedServerFiles.clear(); +} + +void DeckSyncManager::cancelFirstSync() +{ + if (!syncing || !firstSyncRun) { + return; + } + firstSyncRun = false; + stashedServerFiles.clear(); + failSync(tr("Sync cancelled.")); +} + +void DeckSyncManager::classify(const QMap &serverFiles) +{ emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); DeckSyncSidecar sidecar(sidecarPath); @@ -267,6 +330,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont const bool unchangedSinceSync = !sidecarRecord.baseHash.isEmpty() && it.value().hash == sidecarRecord.baseHash; if (knownToServer && unchangedSinceSync) { + if (syncMode == DeckSyncMode::UploadOnly) { + continue; + } const QString absolutePath = deckPath + "/" + it.key(); const QString backupPath = absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); @@ -278,6 +344,9 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont ++downloadedCount; continue; } + if (syncMode == DeckSyncMode::DownloadOnly) { + continue; + } const QString serverPath = serverPathFor(it.key()); pendingUploads.append(UploadOp{.relativePath = it.key(), .serverPath = serverPath, @@ -304,53 +373,57 @@ void DeckSyncManager::onListFinished(const Response &response, const CommandCont continue; } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); + const int effective = sessionDefault >= 0 ? sessionDefault : conflictDefault; // A server row without last_modified predates sync: the local file wins. // Otherwise strictly newer wins; ties go to the server. - 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}); + if (syncMode == DeckSyncMode::UploadOnly || effective == DeckSyncConflictKeepMine) { + applyUpload(it.key(), server.path, server.id, server.guid); + } else if (syncMode == DeckSyncMode::DownloadOnly || effective == DeckSyncConflictKeepServer) { + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); + } else if (effective == DeckSyncConflictKeepNewer && + (server.lastModifiedSecs == 0 || localSecs > server.lastModifiedSecs)) { + applyUpload(it.key(), server.path, server.id, server.guid); + } else if (effective == DeckSyncConflictKeepNewer) { + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); } else { - pendingDownloads.append(DownloadOp{.relativePath = it.key(), - .deckId = server.id, - .guid = server.guid, - .hash = server.hash, - .lastModifiedSecs = server.lastModifiedSecs}); + queueConflict(it.key(), server.path, server.id, server.guid, server.hash, server.lastModifiedSecs); } } - for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { - if (matchedServerIds.contains(it.key())) { - continue; + if (syncMode != DeckSyncMode::UploadOnly) { + for (auto it = serverFiles.begin(); it != serverFiles.end(); ++it) { + 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; + } + pendingDownloads.append(DownloadOp{.relativePath = relativePath, + .deckId = it.value().id, + .guid = it.value().guid, + .hash = it.value().hash, + .lastModifiedSecs = it.value().lastModifiedSecs}); } - const QString wantedPath = localPathFor(it.value().dir, it.value().name); - const QString relativePath = uniqueLocalPath(wantedPath); - if (relativePath.isEmpty()) { - ++skippedCount; - continue; - } - pendingDownloads.append(DownloadOp{.relativePath = relativePath, - .deckId = it.value().id, - .guid = it.value().guid, - .hash = it.value().hash, - .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 (syncMode != DeckSyncMode::DownloadOnly) { + const QMap knownRecords = sidecar.records(); + for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { + if (localFiles.contains(it.key())) { + continue; + } + if (QFileInfo::exists(deckPath + "/" + it.key())) { + continue; + } + if (it.value().remoteId == -1) { + sidecar.remove(it.key()); + continue; + } + pendingDeletes.append(it.key()); } - if (QFileInfo::exists(deckPath + "/" + it.key())) { - continue; - } - if (it.value().remoteId == -1) { - sidecar.remove(it.key()); - continue; - } - pendingDeletes.append(it.key()); } sidecar.save(); @@ -365,7 +438,7 @@ QString DeckSyncManager::parentDir(const QString &path) void DeckSyncManager::processNext() { - if (!syncing) { + if (!syncing || activeConflict.active) { return; } if (!pendingDeletes.isEmpty()) { @@ -382,6 +455,220 @@ void DeckSyncManager::processNext() } } +void DeckSyncManager::applyUpload(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &guid) +{ + UploadOp op; + op.relativePath = relativePath; + op.serverPath = serverPath; + op.deckId = deckId; + op.guid = guid; + pendingUploads.append(op); +} + +void DeckSyncManager::applyDownload(const QString &relativePath, + int deckId, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs) +{ + DownloadOp op; + op.relativePath = relativePath; + op.deckId = deckId; + op.guid = guid; + op.hash = hash; + op.lastModifiedSecs = lastModifiedSecs; + pendingDownloads.append(op); +} + +QString DeckSyncManager::readBaseSnapshot(const QString &guid) const +{ + if (guid.isEmpty()) { + return {}; + } + QFile snapshot(deckPath + "/" + SYNC_META_DIR + "/bases/" + guid + ".cod"); + if (!snapshot.open(QIODevice::ReadOnly)) { + return {}; + } + return QString::fromUtf8(snapshot.readAll()); +} + +void DeckSyncManager::queueConflict(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &serverGuid, + const QString &serverHash, + qint64 serverLastModifiedSecs) +{ + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = !serverGuid.isEmpty() ? serverGuid : sidecar.get(relativePath).guid; + if (readBaseSnapshot(guid).isEmpty()) { + emit statusMessage(tr("No base snapshot for %1, keeping the newer version.").arg(relativePath)); + const qint64 localSecs = localFiles.value(relativePath).modified.toSecsSinceEpoch(); + if (serverLastModifiedSecs == 0 || localSecs > serverLastModifiedSecs) { + applyUpload(relativePath, serverPath, deckId, serverGuid); + } else { + applyDownload(relativePath, deckId, serverGuid, serverHash, serverLastModifiedSecs); + } + return; + } + ConflictDownload ctx; + ctx.serverPath = serverPath; + ctx.deckId = deckId; + ctx.guid = serverGuid; + ctx.hash = serverHash; + ctx.lastModifiedSecs = serverLastModifiedSecs; + pendingConflicts.insert(relativePath, ctx); + conflictPaths.insert(relativePath); + applyDownload(relativePath, deckId, serverGuid, serverHash, serverLastModifiedSecs); +} + +void DeckSyncManager::handleConflictDownload(const QString &relativePath, const QString &serverDeckString) +{ + const ConflictDownload ctx = pendingConflicts.take(relativePath); + if (sessionDefault == DeckSyncConflictKeepMine) { + applyUpload(relativePath, ctx.serverPath, ctx.deckId, ctx.guid); + processNext(); + return; + } + if (sessionDefault == DeckSyncConflictKeepServer) { + applyDownloadedDeck(relativePath, ctx.deckId, serverDeckString, ctx.guid, ctx.hash, ctx.lastModifiedSecs, + ctx.lastModifiedSecs != 0); + return; + } + std::optional loaded = + DeckLoader::loadFromFile(localFiles.value(relativePath).absolutePath, DeckFileFormat::Cockatrice); + DeckList serverDeck; + if (!loaded || !serverDeck.loadFromString_Native(serverDeckString)) { + ++skippedCount; + processNext(); + return; + } + + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = !ctx.guid.isEmpty() ? ctx.guid : sidecar.get(relativePath).guid; + activeConflict.active = true; + activeConflict.relativePath = relativePath; + activeConflict.serverPath = ctx.serverPath; + activeConflict.deckId = ctx.deckId; + activeConflict.serverGuid = ctx.guid; + activeConflict.serverHash = ctx.hash; + activeConflict.serverLastModifiedSecs = ctx.lastModifiedSecs; + activeConflict.serverDeckString = serverDeckString; + + DeckSyncConflict conflict; + conflict.relativePath = relativePath; + conflict.localAbsolutePath = localFiles.value(relativePath).absolutePath; + conflict.serverDeckString = serverDeckString; + conflict.baseDeckString = readBaseSnapshot(guid); + conflict.hasBase = !conflict.baseDeckString.isEmpty(); + conflict.localSecs = localFiles.value(relativePath).modified.toSecsSinceEpoch(); + conflict.serverSecs = ctx.lastModifiedSecs; + emit conflictRequest(conflict); +} + +void DeckSyncManager::resolveConflict(const DeckSyncResolution &resolution) +{ + if (!syncing || !activeConflict.active) { + return; + } + if (resolution.rememberSession && + (resolution.action == DeckSyncKeepMine || resolution.action == DeckSyncKeepServer)) { + sessionDefault = resolution.action == DeckSyncKeepMine ? DeckSyncConflictKeepMine : DeckSyncConflictKeepServer; + } + const ActiveConflict conflict = activeConflict; + activeConflict.active = false; + + switch (resolution.action) { + case DeckSyncKeepMine: + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + case DeckSyncKeepServer: + applyDownload(conflict.relativePath, conflict.deckId, conflict.serverGuid, conflict.serverHash, + conflict.serverLastModifiedSecs); + break; + case DeckSyncKeepBoth: { + // Persist the already-fetched server bytes synchronously: queuing a + // download here would race the upload below and fetch back Mine. + // The aside copy is a new local file, so it gets a fresh guid and + // uploads as its own deck on the next run. + QString asideBase = conflict.relativePath; + if (asideBase.endsWith(".cod", Qt::CaseInsensitive)) { + asideBase.chop(4); + } + const QString aside = uniqueLocalPath(asideBase + " (server).cod"); + DeckList serverDeck; + if (aside.isEmpty() || !serverDeck.loadFromString_Native(conflict.serverDeckString)) { + emit statusMessage(tr("Could not store the server version of %1 aside, keeping the local deck.") + .arg(conflict.relativePath)); + ++skippedCount; + } else { + const QString asideAbsolute = deckPath + "/" + aside; + QDir().mkpath(QFileInfo(asideAbsolute).absolutePath()); + if (!DeckLoader::saveToFile(serverDeck, asideAbsolute, DeckFileFormat::Cockatrice)) { + ++skippedCount; + } else { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + DeckSyncRecord asideRecord; + asideRecord.guid = QUuid::createUuid().toString(QUuid::WithoutBraces); + asideRecord.baseHash = serverDeck.getDeckHash(); + sidecar.set(aside, asideRecord); + sidecar.save(); + snapshotLocalFile(aside, asideRecord.guid); + } + } + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + case DeckSyncUseMerge: { + DeckSyncSidecar sidecar(sidecarPath); + sidecar.load(); + const QString guid = + !conflict.serverGuid.isEmpty() ? conflict.serverGuid : sidecar.get(conflict.relativePath).guid; + DeckList localDeck; + DeckList serverDeck; + DeckList baseDeck; + std::optional loaded = DeckLoader::loadFromFile( + localFiles.value(conflict.relativePath).absolutePath, DeckFileFormat::Cockatrice); + if (!loaded || !serverDeck.loadFromString_Native(conflict.serverDeckString) || + !baseDeck.loadFromString_Native(readBaseSnapshot(guid))) { + emit statusMessage( + tr("Could not build the merge for %1, keeping the local deck.").arg(conflict.relativePath)); + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + localDeck = loaded->deckList; + const DeckDiffResult localChanges = DeckDiff::diff(baseDeck, localDeck); + const DeckDiffResult remoteChanges = DeckDiff::diff(baseDeck, serverDeck); + const DeckList merged = DeckDiff::merge(baseDeck, localChanges, remoteChanges); + const QString absolutePath = deckPath + "/" + conflict.relativePath; + const QString backupPath = + absolutePath + BACKUP_SUFFIX + QString::number(QDateTime::currentDateTime().toSecsSinceEpoch()); + if (!QFile::copy(absolutePath, backupPath) || + !DeckLoader::saveToFile(merged, absolutePath, DeckFileFormat::Cockatrice)) { + ++skippedCount; + break; + } + std::optional reloaded = DeckLoader::loadFromFile(absolutePath, DeckFileFormat::Cockatrice); + if (reloaded) { + localFiles[conflict.relativePath].hash = reloaded->deckList.getDeckHash(); + localFiles[conflict.relativePath].modified = QFileInfo(absolutePath).lastModified(); + } + applyUpload(conflict.relativePath, conflict.serverPath, conflict.deckId, conflict.serverGuid); + break; + } + case DeckSyncSkipConflict: + ++skippedCount; + break; + } + processNext(); +} + void DeckSyncManager::processNextUpload() { const UploadOp op = pendingUploads.first(); @@ -573,8 +860,10 @@ void DeckSyncManager::onDownloadFinished(const Response &response, return; } const QString relativePath = extraData.toString(); + int finishedDeckId = -1; for (int i = 0; i < pendingDownloads.size(); ++i) { if (pendingDownloads.at(i).relativePath == relativePath) { + finishedDeckId = pendingDownloads.at(i).deckId; pendingDownloads.removeAt(i); break; } @@ -586,8 +875,26 @@ void DeckSyncManager::onDownloadFinished(const Response &response, } const Response_DeckDownload &download = response.GetExtension(Response_DeckDownload::ext); + if (conflictPaths.contains(relativePath)) { + conflictPaths.remove(relativePath); + handleConflictDownload(relativePath, QString::fromStdString(download.deck())); + return; + } + applyDownloadedDeck(relativePath, finishedDeckId, QString::fromStdString(download.deck()), + QString::fromStdString(download.guid()), QString::fromStdString(download.hash()), + static_cast(download.last_modified()), download.has_last_modified()); +} + +void DeckSyncManager::applyDownloadedDeck(const QString &relativePath, + int deckId, + const QString &deckString, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs, + bool hasLastModified) +{ DeckList deck; - if (!deck.loadFromString_Native(QString::fromStdString(download.deck()))) { + if (!deck.loadFromString_Native(deckString)) { ++skippedCount; processNext(); return; @@ -619,16 +926,18 @@ void DeckSyncManager::onDownloadFinished(const Response &response, DeckSyncSidecar sidecar(sidecarPath); sidecar.load(); DeckSyncRecord record = sidecar.get(relativePath); - if (!download.guid().empty()) { - record.guid = QString::fromStdString(download.guid()); + if (deckId != -1) { + record.remoteId = deckId; } - if (!download.hash().empty()) { - record.baseHash = QString::fromStdString(download.hash()); + if (!guid.isEmpty()) { + record.guid = guid; + } + if (!hash.isEmpty()) { + record.baseHash = hash; } else { record.baseHash = deck.getContentHash(); } - record.lastSyncSecs = download.has_last_modified() ? static_cast(download.last_modified()) - : QDateTime::currentDateTime().toSecsSinceEpoch(); + record.lastSyncSecs = hasLastModified ? lastModifiedSecs : QDateTime::currentDateTime().toSecsSinceEpoch(); record.deleted = false; sidecar.set(relativePath, record); sidecar.save(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 4d61c0d0a..389763243 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -6,6 +6,7 @@ #ifndef DECK_SYNC_MANAGER_H #define DECK_SYNC_MANAGER_H +#include "deck_sync_conflict.h" #include "deck_sync_profile.h" #include @@ -16,6 +17,7 @@ #include #include #include +#include class AbstractClient; class CommandContainer; @@ -37,7 +39,10 @@ class ServerInfo_DeckStorage_Folder; * 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. + * Both-sides-changed decks pause the queue for a verdict via conflictRequest / + * resolveConflict (or the conflict default setting); the first sync of an + * unknown profile pauses for an onboarding verdict via firstSyncDetected / + * continueFirstSync. No watcher and no auto triggers — those arrive later. * * Deletes are tombstones, never silent: a locally deleted deck is pushed with * Command_DeckDel, a server-side delete is applied locally only after copying @@ -64,15 +69,27 @@ public: void setProfile(const DeckSyncProfile &_profile); void setDeckPath(const QString &path); + void setConflictDefault(int conflictDefault); + void setSyncMode(DeckSyncMode mode); [[nodiscard]] bool isSyncing() const; public slots: void syncNow(); + /** @brief Continues a paused first sync after the onboarding verdict. */ + void continueFirstSync(DeckSyncMode mode); + /** @brief Aborts a paused first sync without writing any verdict. */ + void cancelFirstSync(); + /** @brief Applies the dialog verdict for the active conflict and resumes. */ + void resolveConflict(const DeckSyncResolution &resolution); signals: void syncStarted(); void statusMessage(const QString &message); void syncFinished(const SyncSummary &summary, const QString &error); + /** @brief First sync with content on both sides: the tab must ask how to proceed. */ + void firstSyncDetected(int localCount, int serverCount); + /** @brief A both-sides-changed deck needs a verdict; the queue pauses until resolveConflict. */ + void conflictRequest(const DeckSyncConflict &conflict); private slots: void onListFinished(const Response &response, const CommandContainer &commandContainer); @@ -118,6 +135,25 @@ private: QString hash; qint64 lastModifiedSecs = 0; }; + struct ActiveConflict + { + bool active = false; + QString relativePath; + QString serverPath; + int deckId = -1; + QString serverGuid; + QString serverHash; + qint64 serverLastModifiedSecs = 0; + QString serverDeckString; + }; + struct ConflictDownload + { + QString serverPath; + int deckId = -1; + QString guid; + QString hash; + qint64 lastModifiedSecs = 0; + }; void failSync(const QString &error); void finishSync(); @@ -133,6 +169,28 @@ private: void sendDownload(const DownloadOp &op); void sendDelete(const QString &relativePath, int deckId); void snapshotLocalFile(const QString &relativePath, const QString &guid); + QString readBaseSnapshot(const QString &guid) const; + void classify(const QMap &serverFiles); + void queueConflict(const QString &relativePath, + const QString &serverPath, + int deckId, + const QString &serverGuid, + const QString &serverHash, + qint64 serverLastModifiedSecs); + void handleConflictDownload(const QString &relativePath, const QString &serverDeckString); + void applyDownloadedDeck(const QString &relativePath, + int deckId, + const QString &deckString, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs, + bool hasLastModified); + void applyUpload(const QString &relativePath, const QString &serverPath, int deckId, const QString &guid); + void applyDownload(const QString &relativePath, + int deckId, + const QString &guid, + const QString &hash, + qint64 lastModifiedSecs); static QString parentDir(const QString &path); QString uniqueLocalPath(const QString &relativePath) const; @@ -148,6 +206,14 @@ private: QList pendingUploads; QList pendingDownloads; QList pendingDeletes; + QSet conflictPaths; + QMap pendingConflicts; + QMap stashedServerFiles; + ActiveConflict activeConflict; + DeckSyncMode syncMode = DeckSyncMode::TwoWay; + int conflictDefault = DeckSyncConflictAsk; + int sessionDefault = -1; + bool firstSyncRun = false; int uploadedCount = 0; int downloadedCount = 0; int unchangedCount = 0; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp index 2e3a5ba49..6c5a65e73 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -16,9 +16,15 @@ DeckSyncSidecar::DeckSyncSidecar(const QString &_filePath) : filePath(_filePath) { } +QString DeckSyncSidecar::pathForDeckPath(const QString &deckPath, const QString &profileKey) +{ + return QDir(deckPath).filePath(".sync/" + profileKey + ".json"); +} + bool DeckSyncSidecar::load() { recordMap.clear(); + mode = MODE_LINKED; QFile file(filePath); if (!file.exists()) { @@ -31,6 +37,8 @@ bool DeckSyncSidecar::load() if (!document.isObject() || !document.object().value("records").isObject()) { return false; } + const QJsonValue modeValue = document.object().value("mode"); + mode = modeValue.isString() ? modeValue.toString() : MODE_LINKED; const QJsonObject records = document.object().value("records").toObject(); for (auto it = records.begin(); it != records.end(); ++it) { if (!it.value().isObject()) { @@ -62,6 +70,7 @@ bool DeckSyncSidecar::save() const } QJsonObject root; root.insert("version", SIDECAR_FORMAT_VERSION); + root.insert("mode", mode); root.insert("records", records); QDir().mkpath(QFileInfo(filePath).absolutePath()); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h index 7eb2db7ae..2266922bc 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h @@ -30,10 +30,17 @@ struct DeckSyncRecord class DeckSyncSidecar { public: - QString filePath; ///< Full path of the sidecar JSON file. + static constexpr const char *MODE_LINKED = "linked"; + static constexpr const char *MODE_SEPARATE = "separate"; + + QString filePath; ///< Full path of the sidecar JSON file. + QString mode = MODE_LINKED; ///< Linked normally; separate keeps this profile out of sync. explicit DeckSyncSidecar(const QString &_filePath); + /** @brief Sidecar file path for a deck folder and profile key. */ + static QString pathForDeckPath(const QString &deckPath, const QString &profileKey); + /** @brief Loads records; a missing file loads as empty and returns true. */ bool load(); /** @brief Stores records, creating parent folders as needed. */ diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp new file mode 100644 index 000000000..7d548993d --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.cpp @@ -0,0 +1,178 @@ +#include "dlg_sync_conflict.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +QString formatTime(qint64 secsSinceEpoch) +{ + return QLocale().toString(QDateTime::fromSecsSinceEpoch(secsSinceEpoch), QLocale::ShortFormat); +} + +int zoneCardCount(const DeckList &deck, const QString &zone) +{ + int count = 0; + deck.forEachCard([&count, &zone](InnerDecklistNode *zoneNode, DecklistCardNode *card) { + if (zoneNode->getName() == zone) { + count += card->getNumber(); + } + }); + return count; +} +} // namespace + +DlgSyncConflict::DlgSyncConflict(const DeckSyncConflict &_conflict, QWidget *parent) + : QDialog(parent), conflict(_conflict) +{ + auto *layout = new QVBoxLayout(this); + + titleLabel = new QLabel(this); + detailLabel = new QLabel(this); + detailLabel->setWordWrap(true); + keepMineRadio = new QRadioButton(this); + keepServerRadio = new QRadioButton(this); + keepBothRadio = new QRadioButton(this); + mergeRadio = new QRadioButton(this); + changesList = new QListWidget(this); + mergeSummaryLabel = new QLabel(this); + mergeSummaryLabel->setWordWrap(true); + rememberSessionBox = new QCheckBox(this); + rememberAlwaysBox = new QCheckBox(this); + + layout->addWidget(titleLabel); + layout->addWidget(detailLabel); + layout->addWidget(keepMineRadio); + layout->addWidget(keepServerRadio); + layout->addWidget(keepBothRadio); + layout->addWidget(mergeRadio); + layout->addWidget(changesList); + layout->addWidget(mergeSummaryLabel); + layout->addWidget(rememberSessionBox); + layout->addWidget(rememberAlwaysBox); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + DeckList localDeck; + DeckList serverDeck; + QFile localFile(conflict.localAbsolutePath); + decksParsed = localFile.open(QIODevice::ReadOnly) && localDeck.loadFromFile_Native(&localFile) && + serverDeck.loadFromString_Native(conflict.serverDeckString); + localIsNewer = conflict.localSecs >= conflict.serverSecs; + + if (decksParsed) { + shownChanges = DeckDiff::diff(localDeck, serverDeck); + if (conflict.hasBase) { + DeckList baseDeck; + if (baseDeck.loadFromString_Native(conflict.baseDeckString)) { + const DeckDiffResult localChanges = DeckDiff::diff(baseDeck, localDeck); + const DeckDiffResult remoteChanges = DeckDiff::diff(baseDeck, serverDeck); + mergeable = DeckDiff::canAutoMerge(localChanges, remoteChanges); + if (mergeable) { + const DeckList merged = DeckDiff::merge(baseDeck, localChanges, remoteChanges); + mergedMainCount = zoneCardCount(merged, "main"); + mergedSideCount = zoneCardCount(merged, "side"); + } + } + } + } + + if (localIsNewer) { + keepMineRadio->setChecked(true); + } else { + keepServerRadio->setChecked(true); + } + mergeRadio->setEnabled(mergeable); + + connect(keepMineRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(keepServerRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(keepBothRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + connect(mergeRadio, &QRadioButton::toggled, this, &DlgSyncConflict::updateRememberBoxes); + updateRememberBoxes(); + retranslateUi(); +} + +void DlgSyncConflict::retranslateUi() +{ + setWindowTitle(tr("Sync conflict")); + titleLabel->setText(conflict.relativePath); + if (!decksParsed) { + detailLabel->setText(tr("This deck changed on both sides, but one version could not be read.")); + } else { + detailLabel->setText(tr("This deck changed both on this device and on the server.")); + } + + keepMineRadio->setText( + tr("Keep my deck (%1%2)").arg(formatTime(conflict.localSecs), localIsNewer ? tr(", newest") : QString())); + keepServerRadio->setText(tr("Keep the server deck (%1%2)") + .arg(formatTime(conflict.serverSecs), localIsNewer ? QString() : tr(", newest"))); + keepBothRadio->setText(tr("Keep both (store the server version aside)")); + mergeRadio->setText(mergeable ? tr("Use the auto-merge") : tr("Use the auto-merge (not available)")); + + changesList->clear(); + for (const DeckCardChange &change : shownChanges.changes) { + changesList->addItem( + tr("%1: %2 %3 → %4").arg(change.zone).arg(change.cardName).arg(change.oldCount).arg(change.newCount)); + } + + if (mergeable) { + mergeSummaryLabel->setText(tr("Auto-merge proposal: union of both sides (%1 main, %2 side).") + .arg(mergedMainCount) + .arg(mergedSideCount)); + } else if (!conflict.hasBase) { + mergeSummaryLabel->setText(tr("No base snapshot exists, so the versions cannot be merged automatically.")); + } else { + mergeSummaryLabel->setText(tr("The changes overlap, so they cannot be merged automatically.")); + } + + rememberSessionBox->setText(tr("Remember for the rest of this sync")); + rememberAlwaysBox->setText(tr("Always use this rule")); +} + +void DlgSyncConflict::updateRememberBoxes() +{ + const bool single = keepMineRadio->isChecked() || keepServerRadio->isChecked(); + rememberSessionBox->setEnabled(single); + rememberAlwaysBox->setEnabled(single); + if (!single) { + rememberSessionBox->setChecked(false); + rememberAlwaysBox->setChecked(false); + } +} + +DeckSyncResolutionAction DlgSyncConflict::selectedAction() const +{ + if (keepServerRadio->isChecked()) { + return DeckSyncKeepServer; + } + if (keepBothRadio->isChecked()) { + return DeckSyncKeepBoth; + } + if (mergeRadio->isChecked()) { + return DeckSyncUseMerge; + } + return DeckSyncKeepMine; +} + +DeckSyncResolution DlgSyncConflict::resolution() const +{ + DeckSyncResolution result; + result.action = selectedAction(); + result.rememberSession = rememberSessionBox->isEnabled() && rememberSessionBox->isChecked(); + result.rememberAlways = rememberAlwaysBox->isEnabled() && rememberAlwaysBox->isChecked(); + return result; +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h new file mode 100644 index 000000000..0a969f53e --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_conflict.h @@ -0,0 +1,63 @@ +/** + * @file dlg_sync_conflict.h + * @brief Dialog for resolving a deck that changed both locally and on the server. + */ + +#ifndef DLG_SYNC_CONFLICT_H +#define DLG_SYNC_CONFLICT_H + +#include "../../deck_sync/deck_sync_conflict.h" + +#include +#include +#include + +class QCheckBox; +class QLabel; +class QListWidget; +class QRadioButton; + +/** + * @class DlgSyncConflict + * @brief Shows both versions of a conflicted deck and returns the verdict. + * + * Radio choice between Keep mine / Keep server (newest preselected), Keep both + * and Use merge (enabled only when the changes touch disjoint cards). The + * change list shows card-count deltas between the two versions; the merge + * summary describes the auto-merge proposal. Rejected dialogs resolve as Skip. + */ +class DlgSyncConflict : public QDialog +{ + Q_OBJECT +public: + explicit DlgSyncConflict(const DeckSyncConflict &conflict, QWidget *parent = nullptr); + [[nodiscard]] DeckSyncResolution resolution() const; + void retranslateUi(); + +private slots: + void updateRememberBoxes(); + +private: + DeckSyncResolutionAction selectedAction() const; + + DeckSyncConflict conflict; + bool decksParsed = false; + bool localIsNewer = true; + DeckDiffResult shownChanges; + bool mergeable = false; + int mergedMainCount = 0; + int mergedSideCount = 0; + + QLabel *titleLabel; + QLabel *detailLabel; + QRadioButton *keepMineRadio; + QRadioButton *keepServerRadio; + QRadioButton *keepBothRadio; + QRadioButton *mergeRadio; + QListWidget *changesList; + QLabel *mergeSummaryLabel; + QCheckBox *rememberSessionBox; + QCheckBox *rememberAlwaysBox; +}; + +#endif // DLG_SYNC_CONFLICT_H diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp new file mode 100644 index 000000000..ae0e9446b --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.cpp @@ -0,0 +1,59 @@ +#include "dlg_sync_onboarding.h" + +#include +#include +#include +#include + +DlgSyncOnboarding::DlgSyncOnboarding(int _localCount, int _serverCount, QWidget *parent) + : QDialog(parent), localCount(_localCount), serverCount(_serverCount) +{ + auto *layout = new QVBoxLayout(this); + + detailLabel = new QLabel(this); + detailLabel->setWordWrap(true); + mergeRadio = new QRadioButton(this); + uploadRadio = new QRadioButton(this); + downloadRadio = new QRadioButton(this); + separateRadio = new QRadioButton(this); + mergeRadio->setChecked(true); + + layout->addWidget(detailLabel); + layout->addWidget(mergeRadio); + layout->addWidget(uploadRadio); + layout->addWidget(downloadRadio); + layout->addWidget(separateRadio); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + retranslateUi(); +} + +void DlgSyncOnboarding::retranslateUi() +{ + setWindowTitle(tr("Sync decks to this server")); + detailLabel->setText(tr("You have %1 local decks and %2 decks stored on this server. How should they be combined?") + .arg(localCount) + .arg(serverCount)); + mergeRadio->setText(tr("Merge: keep everything, sync both ways (recommended)")); + uploadRadio->setText(tr("Upload mine: push local decks to the server")); + downloadRadio->setText(tr("Download server: take the server decks")); + separateRadio->setText(tr("Keep separate: do not sync with this server")); +} + +DeckSyncMode DlgSyncOnboarding::selectedMode() const +{ + if (uploadRadio->isChecked()) { + return DeckSyncMode::UploadOnly; + } + if (downloadRadio->isChecked()) { + return DeckSyncMode::DownloadOnly; + } + if (separateRadio->isChecked()) { + return DeckSyncMode::Separate; + } + return DeckSyncMode::TwoWay; +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h new file mode 100644 index 000000000..9d811a28e --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_sync_onboarding.h @@ -0,0 +1,43 @@ +/** + * @file dlg_sync_onboarding.h + * @brief First-sync choice when local decks and server decks meet. + */ + +#ifndef DLG_SYNC_ONBOARDING_H +#define DLG_SYNC_ONBOARDING_H + +#include "../../deck_sync/deck_sync_conflict.h" + +#include + +class QLabel; +class QRadioButton; + +/** + * @class DlgSyncOnboarding + * @brief Asks how to combine a non-empty local collection with a non-empty server collection. + * + * Merge (two-way) is the default. Upload and download restrict the run to one + * direction; keeping separate records the profile as sync-disabled. + * Rejected dialogs cancel the sync without recording anything. + */ +class DlgSyncOnboarding : public QDialog +{ + Q_OBJECT +public: + DlgSyncOnboarding(int _localCount, int _serverCount, QWidget *parent = nullptr); + [[nodiscard]] DeckSyncMode selectedMode() const; + void retranslateUi(); + +private: + int localCount = 0; + int serverCount = 0; + + QLabel *detailLabel; + QRadioButton *mergeRadio; + QRadioButton *uploadRadio; + QRadioButton *downloadRadio; + QRadioButton *separateRadio; +}; + +#endif // DLG_SYNC_ONBOARDING_H diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index 7b89b744d..6b6678467 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -359,6 +360,25 @@ GeneralSettingsPage::GeneralSettingsPage() pathsGroupBox = new QGroupBox; pathsGroupBox->setLayout(pathsGrid); + // deck sync settings + conflictDefaultSelector.addItem("", DeckSyncConflictAsk); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepMine); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepServer); + conflictDefaultSelector.addItem("", DeckSyncConflictKeepNewer); + int conflictIndex = conflictDefaultSelector.findData(settings.deckSync().getConflictDefault()); + if (conflictIndex >= 0) { + conflictDefaultSelector.setCurrentIndex(conflictIndex); + } + connect(&conflictDefaultSelector, qOverload(&QComboBox::currentIndexChanged), this, [this](int index) { + SettingsCache::instance().deckSync().setConflictDefault(conflictDefaultSelector.itemData(index).toInt()); + }); + + auto *deckSyncGrid = new QGridLayout; + deckSyncGrid->addWidget(&conflictDefaultLabel, 0, 0); + deckSyncGrid->addWidget(&conflictDefaultSelector, 0, 1); + deckSyncGroupBox = new QGroupBox; + deckSyncGroupBox->setLayout(deckSyncGrid); + auto *mainLayout = new QVBoxLayout; mainLayout->addWidget(languageGroupBox); mainLayout->addWidget(versionGroupBox); @@ -366,6 +386,7 @@ GeneralSettingsPage::GeneralSettingsPage() mainLayout->addWidget(cardDatabaseGroupBox); mainLayout->addWidget(startupGroupBox); mainLayout->addWidget(pathsGroupBox); + mainLayout->addWidget(deckSyncGroupBox); mainLayout->addStretch(); GeneralSettingsPage::retranslateUi(); @@ -647,5 +668,13 @@ void GeneralSettingsPage::retranslateUi() cardDatabasePathLabel.setText(tr("Card database:")); customCardDatabasePathLabel.setText(tr("Custom database directory:")); tokenDatabasePathLabel.setText(tr("Token database:")); + deckSyncGroupBox->setTitle(tr("Deck sync")); + conflictDefaultLabel.setText(tr("When a deck changed on both sides:")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictAsk), tr("Ask me every time")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepMine), tr("Keep my deck")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepServer), + tr("Keep the server deck")); + conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepNewer), + tr("Keep the newer deck")); resetAllPathsButton->setText(tr("Reset all paths")); } diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index 9451db434..92d46c265 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -52,6 +52,7 @@ private: QGroupBox *cardDatabaseGroupBox; QGroupBox *startupGroupBox; QGroupBox *pathsGroupBox; + QGroupBox *deckSyncGroupBox; QLabel languageLabel; QComboBox languageBox; @@ -104,6 +105,9 @@ private: QLineEdit *tokenDatabasePathEdit; QPushButton *resetAllPathsButton; QLabel *allPathsResetLabel; + + QLabel conflictDefaultLabel; + QComboBox conflictDefaultSelector; }; #endif // COCKATRICE_GENERAL_SETTINGS_PAGE_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 2ca46d4f4..cc68f9fc1 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -2,11 +2,15 @@ #include "../../../client/settings/cache_settings.h" #include "../../deck_loader/deck_loader.h" +#include "../../deck_sync/deck_sync_conflict.h" #include "../../deck_sync/deck_sync_manager.h" +#include "../../deck_sync/deck_sync_profile.h" #include "../../pixel_map_generator.h" #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" #include "../deck_share/share_bar_widget.h" +#include "../dialogs/dlg_sync_conflict.h" +#include "../dialogs/dlg_sync_onboarding.h" #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" #include "../interface/widgets/utility/get_text_with_max.h" #include "abstract_client.h" @@ -63,6 +67,7 @@ #include #include #include +#include #include #include #include @@ -256,8 +261,13 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, syncManager = new DeckSyncManager( client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); + syncManager->setConflictDefault(SettingsCache::instance().deckSync().getConflictDefault()); + connect(&SettingsCache::instance().deckSync(), &DeckSyncSettings::conflictDefaultChanged, syncManager, + &DeckSyncManager::setConflictDefault); connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); + connect(syncManager, &DeckSyncManager::firstSyncDetected, this, &TabDeckStorage::onFirstSyncDetected); + connect(syncManager, &DeckSyncManager::conflictRequest, this, &TabDeckStorage::onConflictRequest); if (currentUserInfo != nullptr) { syncUsername = QString::fromStdString(currentUserInfo->name()); } @@ -558,6 +568,34 @@ void TabDeckStorage::actSyncNow() syncManager->syncNow(); } +void TabDeckStorage::onFirstSyncDetected(int localCount, int serverCount) +{ + DlgSyncOnboarding dialog(localCount, serverCount, this); + if (dialog.exec() == QDialog::Accepted) { + syncManager->continueFirstSync(dialog.selectedMode()); + } else { + syncManager->cancelFirstSync(); + } +} + +void TabDeckStorage::onConflictRequest(const DeckSyncConflict &conflict) +{ + DlgSyncConflict dialog(conflict, this); + if (dialog.exec() != QDialog::Accepted) { + DeckSyncResolution skip; + skip.action = DeckSyncSkipConflict; + syncManager->resolveConflict(skip); + return; + } + const DeckSyncResolution resolution = dialog.resolution(); + if (resolution.rememberAlways && + (resolution.action == DeckSyncKeepMine || resolution.action == DeckSyncKeepServer)) { + SettingsCache::instance().deckSync().setConflictDefault( + resolution.action == DeckSyncKeepMine ? DeckSyncConflictKeepMine : DeckSyncConflictKeepServer); + } + syncManager->resolveConflict(resolution); +} + void TabDeckStorage::onSyncStatusMessage(const QString &message) { syncStatusType = SyncStatusType::Working; diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h index c7456877e..610cce891 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -20,6 +20,8 @@ #include struct LoadedDeck; +struct DeckSyncConflict; +struct DeckSyncResolution; class ServerInfo_User; class QTreeView; class QFileSystemModel; @@ -128,6 +130,8 @@ private slots: void deleteDeckFinished(const Response &response, const CommandContainer &commandContainer); void actSyncNow(); + void onFirstSyncDetected(int localCount, int serverCount); + void onConflictRequest(const DeckSyncConflict &conflict); void onSyncStatusMessage(const QString &message); void onSyncFinished(const SyncSummary &summary, const QString &error); diff --git a/libcockatrice_settings/CMakeLists.txt b/libcockatrice_settings/CMakeLists.txt index d23fb3350..7772d812a 100644 --- a/libcockatrice_settings/CMakeLists.txt +++ b/libcockatrice_settings/CMakeLists.txt @@ -12,6 +12,7 @@ set(HEADERS libcockatrice/settings/commander_bracket_settings.h libcockatrice/settings/debug_settings.h libcockatrice/settings/deck_editor_settings.h + libcockatrice/settings/deck_sync_settings.h libcockatrice/settings/download_settings.h libcockatrice/settings/game_filters_settings.h libcockatrice/settings/game_settings.h @@ -43,6 +44,7 @@ add_library( libcockatrice/settings/chat_settings.cpp libcockatrice/settings/debug_settings.cpp libcockatrice/settings/deck_editor_settings.cpp + libcockatrice/settings/deck_sync_settings.cpp libcockatrice/settings/download_settings.cpp libcockatrice/settings/game_filters_settings.cpp libcockatrice/settings/game_settings.cpp diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp new file mode 100644 index 000000000..0b86d3c84 --- /dev/null +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp @@ -0,0 +1,21 @@ +#include "deck_sync_settings.h" + +#include "libcockatrice/settings/settings_manager.h" + +#include + +DeckSyncSettings::DeckSyncSettings(const QString &settingPath, QObject *parent) + : SettingsManager(settingPath + "deck_sync.ini", "decksync", QString(), parent) +{ +} + +int DeckSyncSettings::getConflictDefault() const +{ + return getValue("conflictDefault", QString(), QString(), DeckSyncConflictAsk).toInt(); +} + +void DeckSyncSettings::setConflictDefault(int _conflictDefault) +{ + setValue(_conflictDefault, "conflictDefault"); + emit conflictDefaultChanged(_conflictDefault); +} diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h new file mode 100644 index 000000000..f984292f3 --- /dev/null +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h @@ -0,0 +1,38 @@ +#ifndef DECK_SYNC_SETTINGS_H +#define DECK_SYNC_SETTINGS_H + +#include "settings_manager.h" + +#include +#include + +class QObject; + +enum DeckSyncConflictDefault +{ + DeckSyncConflictAsk = 0, ///< Ask every time a deck changed on both sides. + DeckSyncConflictKeepMine, ///< Silently keep the local deck. + DeckSyncConflictKeepServer, ///< Silently keep the server deck. + DeckSyncConflictKeepNewer, ///< Silently keep whichever side changed last. +}; + +class DeckSyncSettings : public SettingsManager +{ + Q_OBJECT + friend class SettingsCache; + +public: + [[nodiscard]] int getConflictDefault() const; + void setConflictDefault(int _conflictDefault); + +signals: + void conflictDefaultChanged(int conflictDefault); + +public: + explicit DeckSyncSettings(const QString &settingPath, QObject *parent = nullptr); + +private: + DeckSyncSettings(const DeckSyncSettings & /*other*/); +}; + +#endif // DECK_SYNC_SETTINGS_H diff --git a/tests/deck_sync_sidecar_test.cpp b/tests/deck_sync_sidecar_test.cpp index 4bd04be33..76e18d797 100644 --- a/tests/deck_sync_sidecar_test.cpp +++ b/tests/deck_sync_sidecar_test.cpp @@ -119,6 +119,26 @@ TEST(DeckSyncSidecarTest, RemoveDropsRecord) EXPECT_FALSE(reloaded.contains("a.cod")); } +TEST(DeckSyncSidecarTest, ModeRoundTripsAndDefaultsToLinked) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + DeckSyncSidecar sidecar(sidecarPath(dir)); + EXPECT_EQ(sidecar.mode, DeckSyncSidecar::MODE_LINKED); + sidecar.mode = DeckSyncSidecar::MODE_SEPARATE; + EXPECT_TRUE(sidecar.save()); + + DeckSyncSidecar reloaded(sidecarPath(dir)); + EXPECT_TRUE(reloaded.load()); + EXPECT_EQ(reloaded.mode, DeckSyncSidecar::MODE_SEPARATE); +} + +TEST(DeckSyncSidecarTest, PathForDeckPathBuildsSidecarPath) +{ + QString path = DeckSyncSidecar::pathForDeckPath("/decks", "abc123"); + EXPECT_EQ(path, QString("/decks/.sync/abc123.json")); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index aea7f1d54..0a2fb31e0 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -586,6 +587,25 @@ TEST_F(SettingsDefaultsTest, DeckEditor_VdeStartupTab_SetAndGet) ASSERT_EQ(s.getVdeStartupTab(), VdeStartupTabContext); } +// --- DeckSyncSettings --- + +TEST_F(SettingsDefaultsTest, DeckSync_ConflictDefault_Default) +{ + DeckSyncSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictAsk); +} + +TEST_F(SettingsDefaultsTest, DeckSync_ConflictDefault_SetAndGet) +{ + DeckSyncSettings s(settingsPath, nullptr); + s.setConflictDefault(DeckSyncConflictKeepNewer); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictKeepNewer); + s.setConflictDefault(DeckSyncConflictKeepMine); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictKeepMine); + s.setConflictDefault(DeckSyncConflictAsk); + ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictAsk); +} + // --- NetworkSettings --- TEST_F(SettingsDefaultsTest, Network_ClientID_Default) From 0f413b14662ef6113b91f4adf324f5ab0c2ac160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:19:55 +0200 Subject: [PATCH 13/18] Fix ZeldaZach review for PR 7495: three-way baseHash check before conflict --- .../interface/deck_sync/deck_sync_manager.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 19919808b..5fc18cd4d 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -372,6 +372,24 @@ void DeckSyncManager::classify(const QMap &serverFiles) ++unchangedCount; continue; } + const DeckSyncRecord baseRecord = sidecar.get(it.key()); + const bool localChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != it.value().hash; + const bool serverChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != serverIt.value().hash; + if (localChanged && !serverChanged) { + if (syncMode == DeckSyncMode::DownloadOnly) { + continue; + } + applyUpload(it.key(), serverPath, serverIt.value().id, serverIt.value().guid); + continue; + } + if (!localChanged && serverChanged) { + if (syncMode == DeckSyncMode::UploadOnly) { + continue; + } + applyDownload(it.key(), serverIt.value().id, serverIt.value().guid, serverIt.value().hash, + serverIt.value().lastModifiedSecs); + continue; + } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); const int effective = sessionDefault >= 0 ? sessionDefault : conflictDefault; // A server row without last_modified predates sync: the local file wins. From 829d3786400fa5a227d440039493f2196b7af103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 05:08:27 +0100 Subject: [PATCH 14/18] Fix remaining review threads on the conflict classification - Remove the duplicated download loop and the stray '<' plus extra brace that left the head unbuildable. - Skip the fallback upload under DownloadMode so a local-only deck is not pushed when the user asked for downloads only. - Treat a server row without a hash as unknown rather than changed, so pre-migration decks are not re-downloaded on every sync. --- .../src/interface/deck_sync/deck_sync_manager.cpp | 12 +++++++----- .../src/interface/deck_sync/deck_sync_manager.h | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 5fc18cd4d..0e52b2e9e 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -261,7 +261,7 @@ void DeckSyncManager::cancelFirstSync() failSync(tr("Sync cancelled.")); } -void DeckSyncManager::classify(const QMap &serverFiles) +void DeckSyncManager::classify(const QMap &serverFiles) { emit statusMessage(tr("Comparing %1 local and %2 server decks...").arg(localFiles.size()).arg(serverFiles.size())); @@ -374,20 +374,22 @@ void DeckSyncManager::classify(const QMap &serverFiles) } const DeckSyncRecord baseRecord = sidecar.get(it.key()); const bool localChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != it.value().hash; - const bool serverChanged = baseRecord.baseHash.isEmpty() || baseRecord.baseHash != serverIt.value().hash; + // A server row without a hash predates sync: treat it as unknown rather than changed, + // otherwise every sync re-downloads the deck. + const bool serverChanged = + !server.hash.isEmpty() && (baseRecord.baseHash.isEmpty() || baseRecord.baseHash != server.hash); if (localChanged && !serverChanged) { if (syncMode == DeckSyncMode::DownloadOnly) { continue; } - applyUpload(it.key(), serverPath, serverIt.value().id, serverIt.value().guid); + applyUpload(it.key(), server.path, server.id, server.guid); continue; } if (!localChanged && serverChanged) { if (syncMode == DeckSyncMode::UploadOnly) { continue; } - applyDownload(it.key(), serverIt.value().id, serverIt.value().guid, serverIt.value().hash, - serverIt.value().lastModifiedSecs); + applyDownload(it.key(), server.id, server.guid, server.hash, server.lastModifiedSecs); continue; } const qint64 localSecs = it.value().modified.toSecsSinceEpoch(); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 389763243..eecfceb7d 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -170,7 +170,7 @@ private: void sendDelete(const QString &relativePath, int deckId); void snapshotLocalFile(const QString &relativePath, const QString &guid); QString readBaseSnapshot(const QString &guid) const; - void classify(const QMap &serverFiles); + void classify(const QMap &serverFiles); void queueConflict(const QString &relativePath, const QString &serverPath, int deckId, @@ -208,7 +208,7 @@ private: QList pendingDeletes; QSet conflictPaths; QMap pendingConflicts; - QMap stashedServerFiles; + QMap stashedServerFiles; ActiveConflict activeConflict; DeckSyncMode syncMode = DeckSyncMode::TwoWay; int conflictDefault = DeckSyncConflictAsk; From 477605d1e01804f805e15d2bd255dc30a5030598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 17:02:30 +0200 Subject: [PATCH 15/18] [Client+Server] Auto deck sync with change polling --- .../interface/deck_sync/deck_sync_manager.cpp | 221 +++++++++++++++++- .../interface/deck_sync/deck_sync_manager.h | 28 ++- .../interface/deck_sync/deck_sync_sidecar.cpp | 3 + .../interface/deck_sync/deck_sync_sidecar.h | 1 + .../settings_page/general_settings_page.cpp | 16 ++ .../settings_page/general_settings_page.h | 3 + .../widgets/tabs/tab_deck_storage.cpp | 11 +- .../libcockatrice/protocol/pb/CMakeLists.txt | 2 + .../protocol/pb/command_deck_sync_check.proto | 11 + .../libcockatrice/protocol/pb/response.proto | 1 + .../pb/response_deck_sync_status.proto | 21 ++ .../protocol/pb/session_commands.proto | 1 + .../settings/deck_sync_settings.cpp | 23 ++ .../settings/deck_sync_settings.h | 11 +- servatrice/src/serversocketinterface.cpp | 78 +++++++ servatrice/src/serversocketinterface.h | 5 + tests/deck_sync_sidecar_test.cpp | 3 + tests/settings/settings_defaults_test.cpp | 27 +++ 18 files changed, 448 insertions(+), 18 deletions(-) create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_sync_check.proto create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_sync_status.proto diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 0e52b2e9e..768ba3411 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -9,9 +9,12 @@ #include #include #include +#include #include #include +#include #include +#include #include #include #include @@ -19,12 +22,15 @@ #include #include #include +#include #include #include #include #include +#include #include #include +#include #include #include @@ -32,6 +38,7 @@ namespace { constexpr const char *SYNC_META_DIR = ".sync"; constexpr const char *BACKUP_SUFFIX = ".sync-backup-"; +constexpr int WATCH_DEBOUNCE_MS = 2000; QString serverPathFor(const QString &relativePath) { @@ -74,21 +81,145 @@ QString localPathFor(const QString &dir, const QString &name) DeckSyncManager::DeckSyncManager(QObject *parent) : QObject(parent) { + watcher = new QFileSystemWatcher(this); + debounceTimer = new QTimer(this); + debounceTimer->setSingleShot(true); + debounceTimer->setInterval(WATCH_DEBOUNCE_MS); + intervalTimer = new QTimer(this); + connect(watcher, &QFileSystemWatcher::fileChanged, this, + [this](const QString &path) { onWatchedPathChanged(path); }); + connect(watcher, &QFileSystemWatcher::directoryChanged, this, [this](const QString &path) { + refreshWatcher(); + onWatchedPathChanged(path); + }); + connect(debounceTimer, &QTimer::timeout, this, &DeckSyncManager::onDebounceTimeout); + connect(intervalTimer, &QTimer::timeout, this, [this] { autoSyncSoon(); }); + refreshInterval(); } DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider provider, QObject *parent) : QObject(parent), client(_client), colorIdentityProvider(std::move(provider)) { + if (client == nullptr) { + return; + } + connect( + client, &AbstractClient::statusChanged, this, + [this](ClientStatus status) { + if (status == StatusDisconnected) { + offline = true; + debounceTimer->stop(); + } + }, + Qt::UniqueConnection); + connect( + client, &AbstractClient::userInfoChanged, this, + [this](const ServerInfo_User &userInfo) { + offline = false; + serverUsername = QString::fromStdString(userInfo.name()); + autoSyncSoon(); + }, + Qt::UniqueConnection); } -void DeckSyncManager::setProfile(const DeckSyncProfile &_profile) +void DeckSyncManager::setServerIdentity(const QString &host, int port, const QString &username) { - profile = _profile; + serverHost = host; + serverPort = port; + serverUsername = username; } void DeckSyncManager::setDeckPath(const QString &path) { deckPath = path; + refreshWatcher(); +} + +void DeckSyncManager::setAutoSyncEnabled(bool enabled) +{ + autoSyncEnabled = enabled; + refreshInterval(); +} + +void DeckSyncManager::setSyncIntervalMinutes(int minutes) +{ + syncIntervalMinutes = qBound(1, minutes, 60); + refreshInterval(); +} + +DeckSyncProfile DeckSyncManager::buildProfile() const +{ + return DeckSyncProfile(serverHost, serverPort, serverUsername); +} + +void DeckSyncManager::refreshInterval() +{ + intervalTimer->stop(); + if (autoSyncEnabled) { + intervalTimer->start(syncIntervalMinutes * 60000); + } +} + +void DeckSyncManager::refreshWatcher() +{ + watcher->removePaths(watcher->files() + watcher->directories()); + if (deckPath.isEmpty() || !QDir(deckPath).exists()) { + return; + } + QStringList watched; + watched.append(deckPath); + QDirIterator it(deckPath, QDir::Dirs | QDir::Files | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + while (it.hasNext()) { + const QString path = it.next(); + if (!path.contains("/" + QString(SYNC_META_DIR) + "/") && !path.endsWith("/" + QString(SYNC_META_DIR))) { + watched.append(path); + } + } + watcher->addPaths(watched); +} + +void DeckSyncManager::onWatchedPathChanged(const QString &path) +{ + if (path.contains("/" + QString(SYNC_META_DIR) + "/")) { + return; + } + autoSyncSoon(); +} + +void DeckSyncManager::autoSyncSoon() +{ + if (!autoSyncEnabled) { + return; + } + debounceTimer->start(WATCH_DEBOUNCE_MS); +} + +void DeckSyncManager::onDebounceTimeout() +{ + if (syncing) { + debounceTimer->start(WATCH_DEBOUNCE_MS); + return; + } + if (canAutoRun()) { + syncNow(); + } +} + +bool DeckSyncManager::canAutoRun() const +{ + if (!autoSyncEnabled || offline || client == nullptr || deckPath.isEmpty()) { + return false; + } + const DeckSyncProfile autoProfile = buildProfile(); + if (!autoProfile.isValid()) { + return false; + } + const QString autoSidecarPath = DeckSyncSidecar::pathForDeckPath(deckPath, autoProfile.key()); + if (!QFileInfo::exists(autoSidecarPath)) { + return false; + } + DeckSyncSidecar sidecar(autoSidecarPath); + return sidecar.load() && sidecar.mode == DeckSyncSidecar::MODE_LINKED; } void DeckSyncManager::setConflictDefault(int _conflictDefault) @@ -96,11 +227,6 @@ void DeckSyncManager::setConflictDefault(int _conflictDefault) conflictDefault = _conflictDefault; } -void DeckSyncManager::setSyncMode(DeckSyncMode mode) -{ - syncMode = mode; -} - bool DeckSyncManager::isSyncing() const { return syncing; @@ -111,7 +237,8 @@ void DeckSyncManager::syncNow() if (syncing) { return; } - if (client == nullptr || !profile.isValid() || deckPath.isEmpty()) { + const DeckSyncProfile runProfile = buildProfile(); + if (client == nullptr || !runProfile.isValid() || deckPath.isEmpty()) { emit syncFinished(SyncSummary{}, tr("Not connected to a server.")); return; } @@ -138,10 +265,11 @@ void DeckSyncManager::syncNow() sessionDefault = -1; syncMode = DeckSyncMode::TwoWay; firstSyncRun = false; + pendingServerTime = -1; emit syncStarted(); emit statusMessage(tr("Scanning local decks...")); - sidecarPath = DeckSyncSidecar::pathForDeckPath(deckPath, profile.key()); + sidecarPath = DeckSyncSidecar::pathForDeckPath(deckPath, runProfile.key()); DeckSyncSidecar sidecar(sidecarPath); if (!sidecar.load()) { failSync(tr("Could not read sync state.")); @@ -171,12 +299,80 @@ void DeckSyncManager::syncNow() .modified = QFileInfo(absolutePath).lastModified()}); } + if (!firstSyncRun && sidecar.lastSyncSecs > 0) { + sendSyncCheck(sidecar.lastSyncSecs); + return; + } + fetchFullList(); +} + +void DeckSyncManager::fetchFullList() +{ Command_DeckList cmd; PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onListFinished); client->sendCommand(pend); } +void DeckSyncManager::sendSyncCheck(qint64 lastSyncSecs) +{ + emit statusMessage(tr("Checking for server changes...")); + Command_DeckSyncCheck cmd; + cmd.set_last_sync_time(static_cast(lastSyncSecs)); + PendingCommand *pend = AbstractClient::prepareSessionCommand(cmd); + connect(pend, &PendingCommand::finished, this, &DeckSyncManager::onCheckFinished); + client->sendCommand(pend); +} + +void DeckSyncManager::onCheckFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (!syncing) { + return; + } + if (response.response_code() != Response::RespOk) { + fetchFullList(); + return; + } + const Response_DeckSyncStatus &status = response.GetExtension(Response_DeckSyncStatus::ext); + DeckSyncSidecar sidecar(sidecarPath); + if (!sidecar.load()) { + fetchFullList(); + return; + } + + bool localClean = status.changed_files_size() == 0; + if (localClean) { + for (auto it = localFiles.begin(); it != localFiles.end(); ++it) { + const DeckSyncRecord record = sidecar.get(it.key()); + if (record.remoteId == -1 || record.baseHash != it.value().hash) { + localClean = false; + break; + } + } + } + if (localClean) { + const QMap knownRecords = sidecar.records(); + for (auto it = knownRecords.begin(); it != knownRecords.end(); ++it) { + if (!localFiles.contains(it.key())) { + localClean = false; + break; + } + } + } + if (!localClean) { + if (status.changed_files_size() > 0) { + emit statusMessage(tr("%1 server changes, syncing...").arg(status.changed_files_size())); + } + fetchFullList(); + return; + } + + pendingServerTime = status.has_server_time() ? static_cast(status.server_time()) : -1; + unchangedCount = localFiles.size(); + emit statusMessage(tr("Decks are already in sync.")); + finishSync(); +} + void DeckSyncManager::collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, QMap &files, @@ -1035,6 +1231,13 @@ void DeckSyncManager::failSync(const QString &error) void DeckSyncManager::finishSync() { syncing = false; + DeckSyncSidecar sidecar(sidecarPath); + if (sidecar.load()) { + sidecar.lastSyncSecs = + pendingServerTime >= 0 ? pendingServerTime : QDateTime::currentDateTime().toSecsSinceEpoch(); + sidecar.save(); + } + pendingServerTime = -1; const SyncSummary summary = currentSummary(); emit statusMessage(tr("Sync complete: %1 uploaded, %2 downloaded, %3 unchanged, %4 skipped.") .arg(summary.uploaded) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index eecfceb7d..98c9301ac 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -22,6 +22,8 @@ class AbstractClient; class CommandContainer; class DeckList; +class QFileSystemWatcher; +class QTimer; class Response; class ServerInfo_DeckStorage_Folder; @@ -67,10 +69,11 @@ public: explicit DeckSyncManager(QObject *parent = nullptr); DeckSyncManager(AbstractClient *client, ColorIdentityProvider provider, QObject *parent = nullptr); - void setProfile(const DeckSyncProfile &_profile); + void setServerIdentity(const QString &host, int port, const QString &username); void setDeckPath(const QString &path); void setConflictDefault(int conflictDefault); - void setSyncMode(DeckSyncMode mode); + void setAutoSyncEnabled(bool enabled); + void setSyncIntervalMinutes(int minutes); [[nodiscard]] bool isSyncing() const; public slots: @@ -93,6 +96,7 @@ signals: private slots: void onListFinished(const Response &response, const CommandContainer &commandContainer); + void onCheckFinished(const Response &response, const CommandContainer &commandContainer); void onEnsureDirFinished(const Response &response, const CommandContainer &commandContainer, const QVariant &extraData); void @@ -158,6 +162,15 @@ private: void failSync(const QString &error); void finishSync(); [[nodiscard]] SyncSummary currentSummary() const; + void fetchFullList(); + void sendSyncCheck(qint64 lastSyncSecs); + void autoSyncSoon(); + [[nodiscard]] bool canAutoRun() const; + void onDebounceTimeout(); + void onWatchedPathChanged(const QString &path); + void refreshWatcher(); + void refreshInterval(); + [[nodiscard]] DeckSyncProfile buildProfile() const; void collectServerFiles(const ServerInfo_DeckStorage_Folder &folder, const QString &prefix, QMap &files, @@ -195,7 +208,9 @@ private: QString uniqueLocalPath(const QString &relativePath) const; AbstractClient *client = nullptr; - DeckSyncProfile profile; + QString serverHost; + int serverPort = 0; + QString serverUsername; QString deckPath; ColorIdentityProvider colorIdentityProvider; bool syncing = false; @@ -211,6 +226,13 @@ private: QMap stashedServerFiles; ActiveConflict activeConflict; DeckSyncMode syncMode = DeckSyncMode::TwoWay; + QFileSystemWatcher *watcher = nullptr; + QTimer *debounceTimer = nullptr; + QTimer *intervalTimer = nullptr; + bool autoSyncEnabled = true; + int syncIntervalMinutes = 5; + bool offline = false; + qint64 pendingServerTime = -1; int conflictDefault = DeckSyncConflictAsk; int sessionDefault = -1; bool firstSyncRun = false; diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp index 6c5a65e73..48dfeba5a 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -25,6 +25,7 @@ bool DeckSyncSidecar::load() { recordMap.clear(); mode = MODE_LINKED; + lastSyncSecs = 0; QFile file(filePath); if (!file.exists()) { @@ -39,6 +40,7 @@ bool DeckSyncSidecar::load() } const QJsonValue modeValue = document.object().value("mode"); mode = modeValue.isString() ? modeValue.toString() : MODE_LINKED; + lastSyncSecs = static_cast(document.object().value("lastSync").toDouble()); const QJsonObject records = document.object().value("records").toObject(); for (auto it = records.begin(); it != records.end(); ++it) { if (!it.value().isObject()) { @@ -71,6 +73,7 @@ bool DeckSyncSidecar::save() const QJsonObject root; root.insert("version", SIDECAR_FORMAT_VERSION); root.insert("mode", mode); + root.insert("lastSync", static_cast(lastSyncSecs)); root.insert("records", records); QDir().mkpath(QFileInfo(filePath).absolutePath()); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h index 2266922bc..de746b566 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.h @@ -35,6 +35,7 @@ public: QString filePath; ///< Full path of the sidecar JSON file. QString mode = MODE_LINKED; ///< Linked normally; separate keeps this profile out of sync. + qint64 lastSyncSecs = 0; ///< Last completed sync, seconds since epoch; 0 means never. explicit DeckSyncSidecar(const QString &_filePath); diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index 6b6678467..6a24f7db6 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -373,9 +373,22 @@ GeneralSettingsPage::GeneralSettingsPage() SettingsCache::instance().deckSync().setConflictDefault(conflictDefaultSelector.itemData(index).toInt()); }); + autoSyncCheckBox.setChecked(settings.deckSync().getAutoSync()); + connect(&autoSyncCheckBox, &QCheckBox::toggled, &SettingsCache::instance().deckSync(), + &DeckSyncSettings::setAutoSync); + + syncIntervalEdit.setMinimum(1); + syncIntervalEdit.setMaximum(60); + syncIntervalEdit.setValue(settings.deckSync().getSyncIntervalMinutes()); + connect(&syncIntervalEdit, qOverload(&QSpinBox::valueChanged), &SettingsCache::instance().deckSync(), + &DeckSyncSettings::setSyncIntervalMinutes); + auto *deckSyncGrid = new QGridLayout; deckSyncGrid->addWidget(&conflictDefaultLabel, 0, 0); deckSyncGrid->addWidget(&conflictDefaultSelector, 0, 1); + deckSyncGrid->addWidget(&autoSyncCheckBox, 1, 0, 1, 2); + deckSyncGrid->addWidget(&syncIntervalLabel, 2, 0); + deckSyncGrid->addWidget(&syncIntervalEdit, 2, 1); deckSyncGroupBox = new QGroupBox; deckSyncGroupBox->setLayout(deckSyncGrid); @@ -676,5 +689,8 @@ void GeneralSettingsPage::retranslateUi() tr("Keep the server deck")); conflictDefaultSelector.setItemText(conflictDefaultSelector.findData(DeckSyncConflictKeepNewer), tr("Keep the newer deck")); + autoSyncCheckBox.setText(tr("Sync decks automatically")); + syncIntervalLabel.setText(tr("Check every:")); + syncIntervalEdit.setSuffix(" " + tr("min")); resetAllPathsButton->setText(tr("Reset all paths")); } diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index 92d46c265..f410abdf2 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -108,6 +108,9 @@ private: QLabel conflictDefaultLabel; QComboBox conflictDefaultSelector; + QCheckBox autoSyncCheckBox; + QLabel syncIntervalLabel; + QSpinBox syncIntervalEdit; }; #endif // COCKATRICE_GENERAL_SETTINGS_PAGE_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index cc68f9fc1..48111f6ea 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -4,7 +4,6 @@ #include "../../deck_loader/deck_loader.h" #include "../../deck_sync/deck_sync_conflict.h" #include "../../deck_sync/deck_sync_manager.h" -#include "../../deck_sync/deck_sync_profile.h" #include "../../pixel_map_generator.h" #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" @@ -262,8 +261,14 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, syncManager = new DeckSyncManager( client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); syncManager->setConflictDefault(SettingsCache::instance().deckSync().getConflictDefault()); + syncManager->setAutoSyncEnabled(SettingsCache::instance().deckSync().getAutoSync()); + syncManager->setSyncIntervalMinutes(SettingsCache::instance().deckSync().getSyncIntervalMinutes()); connect(&SettingsCache::instance().deckSync(), &DeckSyncSettings::conflictDefaultChanged, syncManager, &DeckSyncManager::setConflictDefault); + connect(&SettingsCache::instance().deckSync(), &DeckSyncSettings::autoSyncChanged, syncManager, + &DeckSyncManager::setAutoSyncEnabled); + connect(&SettingsCache::instance().deckSync(), &DeckSyncSettings::syncIntervalMinutesChanged, syncManager, + &DeckSyncManager::setSyncIntervalMinutes); connect(syncManager, &DeckSyncManager::statusMessage, this, &TabDeckStorage::onSyncStatusMessage); connect(syncManager, &DeckSyncManager::syncFinished, this, &TabDeckStorage::onSyncFinished); connect(syncManager, &DeckSyncManager::firstSyncDetected, this, &TabDeckStorage::onFirstSyncDetected); @@ -271,6 +276,7 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, if (currentUserInfo != nullptr) { syncUsername = QString::fromStdString(currentUserInfo->name()); } + syncManager->setServerIdentity(client->serverName(), client->serverPort(), syncUsername); retranslateUi(); @@ -330,6 +336,7 @@ QString TabDeckStorage::getTargetPath() const void TabDeckStorage::handleConnected(const ServerInfo_User &userInfo) { syncUsername = QString::fromStdString(userInfo.name()); + syncManager->setServerIdentity(client->serverName(), client->serverPort(), syncUsername); setRemoteEnabled(userInfo.user_level() & ServerInfo_User::IsRegistered); } @@ -561,8 +568,6 @@ void TabDeckStorage::actOpenDecksFolder() void TabDeckStorage::actSyncNow() { - DeckSyncProfile profile(client->serverName(), client->serverPort(), syncUsername); - syncManager->setProfile(profile); syncManager->setDeckPath(SettingsCache::instance().paths().getDeckPath()); aSyncNow->setEnabled(false); syncManager->syncNow(); diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt index 059e83096..0860b407b 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt @@ -26,6 +26,7 @@ set(PROTO_FILES command_deck_share_list.proto command_deck_share_list_mine.proto command_deck_share_remove.proto + command_deck_sync_check.proto command_deck_upload.proto command_del_counter.proto command_delete_arrow.proto @@ -151,6 +152,7 @@ set(PROTO_FILES response_deck_share_download.proto response_deck_share_list.proto response_deck_share_list_mine.proto + response_deck_sync_status.proto response_deck_upload.proto response_dump_zone.proto response_forgotpasswordrequest.proto diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_sync_check.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_sync_check.proto new file mode 100644 index 000000000..1afdfee42 --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_sync_check.proto @@ -0,0 +1,11 @@ +syntax = "proto2"; +import "session_commands.proto"; + +message Command_DeckSyncCheck { + extend SessionCommand { + optional Command_DeckSyncCheck ext = 1034; + } + // Seconds since Unix epoch of the client's last completed sync. + // 0 means "never synced": the server reports every stored row. + optional uint64 last_sync_time = 1 [default = 0]; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto index caf9febde..8ec4c3554 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto @@ -85,6 +85,7 @@ message Response { DECK_SHARE_LIST = 1104; // Response listing shared decks DECK_SHARE_DOWNLOAD = 1105; // Response for shared deck download DECK_SHARE_LIST_MINE = 1106; // Response listing the caller's own shares + DECK_SYNC_STATUS = 1107; // Response with changed decks since a timestamp CARD_ART_RULE_LIST = 1200; // Response containing a list of card art rules } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_sync_status.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_sync_status.proto new file mode 100644 index 000000000..6d6bd2cd1 --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_sync_status.proto @@ -0,0 +1,21 @@ +syntax = "proto2"; +import "response.proto"; + +message DeckSyncChangedFile { + // Server-relative folder path without file extension, e.g. "Modern/Affinity". + optional string path = 1; + optional uint32 deck_id = 2; + optional string guid = 3; + optional string hash = 4; + optional uint64 last_modified = 5; + optional bool deleted = 6 [default = false]; +} + +message Response_DeckSyncStatus { + extend Response { + optional Response_DeckSyncStatus ext = 1107; + } + repeated DeckSyncChangedFile changed_files = 1; + // Server clock at query time; the client adopts it as its sync baseline. + optional uint64 server_time = 2; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index 10aefe7b5..b9a80021f 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -37,6 +37,7 @@ message SessionCommand { DECK_SHARE_LIST_MINE = 1032; DECK_SHARE_REMOVE = 1033; SUBMIT_PASSWORD_VERIFIER = 1034; + DECK_SYNC_CHECK = 1035; REPLAY_LIST = 1100; REPLAY_DOWNLOAD = 1101; REPLAY_MODIFY_MATCH = 1102; diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp index 0b86d3c84..caa9ce9c6 100644 --- a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp @@ -3,6 +3,7 @@ #include "libcockatrice/settings/settings_manager.h" #include +#include DeckSyncSettings::DeckSyncSettings(const QString &settingPath, QObject *parent) : SettingsManager(settingPath + "deck_sync.ini", "decksync", QString(), parent) @@ -19,3 +20,25 @@ void DeckSyncSettings::setConflictDefault(int _conflictDefault) setValue(_conflictDefault, "conflictDefault"); emit conflictDefaultChanged(_conflictDefault); } + +bool DeckSyncSettings::getAutoSync() const +{ + return getValue("autoSync", QString(), QString(), true).toBool(); +} + +void DeckSyncSettings::setAutoSync(bool _autoSync) +{ + setValue(_autoSync, "autoSync"); + emit autoSyncChanged(_autoSync); +} + +int DeckSyncSettings::getSyncIntervalMinutes() const +{ + return qBound(1, getValue("syncIntervalMinutes", QString(), QString(), 5).toInt(), 60); +} + +void DeckSyncSettings::setSyncIntervalMinutes(int _syncIntervalMinutes) +{ + setValue(_syncIntervalMinutes, "syncIntervalMinutes"); + emit syncIntervalMinutesChanged(_syncIntervalMinutes); +} diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h index f984292f3..2552f52a9 100644 --- a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h @@ -22,14 +22,19 @@ class DeckSyncSettings : public SettingsManager friend class SettingsCache; public: + explicit DeckSyncSettings(const QString &settingPath, QObject *parent = nullptr); + [[nodiscard]] int getConflictDefault() const; void setConflictDefault(int _conflictDefault); + [[nodiscard]] bool getAutoSync() const; + void setAutoSync(bool _autoSync); + [[nodiscard]] int getSyncIntervalMinutes() const; + void setSyncIntervalMinutes(int _syncIntervalMinutes); signals: void conflictDefaultChanged(int conflictDefault); - -public: - explicit DeckSyncSettings(const QString &settingPath, QObject *parent = nullptr); + void autoSyncChanged(bool autoSync); + void syncIntervalMinutesChanged(int syncIntervalMinutes); private: DeckSyncSettings(const DeckSyncSettings & /*other*/); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index e88c74527..6da45fd41 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -98,6 +98,7 @@ #include #include #include +#include #include #include #include @@ -134,6 +135,7 @@ #include #include #include +#include #include #include #include @@ -371,6 +373,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm return cmdDeckUpload(cmd.GetExtension(Command_DeckUpload::ext), rc); case SessionCommand::DECK_DOWNLOAD: return cmdDeckDownload(cmd.GetExtension(Command_DeckDownload::ext), rc); + case SessionCommand::DECK_SYNC_CHECK: + return cmdDeckSyncCheck(cmd.GetExtension(Command_DeckSyncCheck::ext), rc); case SessionCommand::REPLAY_LIST: return cmdReplayList(cmd.GetExtension(Command_ReplayList::ext), rc); case SessionCommand::REPLAY_DOWNLOAD: @@ -894,6 +898,80 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSetVisibility(const return Response::RespOk; } +bool AbstractServerSocketInterface::syncCheckHelper(int folderId, + const QString &prefix, + int userId, + qint64 sinceSecs, + Response_DeckSyncStatus *status) +{ + QSqlQuery *query = sqlInterface->prepareQuery("select id, name from {prefix}_decklist_folders where id_parent = " + ":id_parent and id_user = :id_user"); + query->bindValue(":id_parent", folderId); + query->bindValue(":id_user", userId); + if (!sqlInterface->execSqlQuery(query)) { + return false; + } + QList> subfolders; + while (query->next()) { + subfolders.append({query->value(0).toInt(), query->value(1).toString()}); + } + for (const auto &[subfolderId, subfolderName] : subfolders) { + const QString path = prefix.isEmpty() ? subfolderName : prefix + "/" + subfolderName; + if (!syncCheckHelper(subfolderId, path, userId, sinceSecs, status)) { + return false; + } + } + + query = sqlInterface->prepareQuery("select id, name, guid, hash, last_modified, deleted from " + "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user " + "and (last_modified is null or last_modified > FROM_UNIXTIME(:since))"); + query->bindValue(":id_folder", folderId); + query->bindValue(":id_user", userId); + query->bindValue(":since", sinceSecs); + if (!sqlInterface->execSqlQuery(query)) { + return false; + } + while (query->next()) { + const QString name = query->value(1).toString(); + DeckSyncChangedFile *changed = status->add_changed_files(); + changed->set_path((prefix.isEmpty() ? name : prefix + "/" + name).toStdString()); + changed->set_deck_id(query->value(0).toUInt()); + changed->set_guid(query->value(2).toString().toStdString()); + changed->set_hash(query->value(3).toString().toStdString()); + const QDateTime lastModified = query->value(4).toDateTime(); + if (lastModified.isValid()) { + changed->set_last_modified(lastModified.toSecsSinceEpoch()); + } + changed->set_deleted(query->value(5).toBool()); + } + return true; +} + +Response::ResponseCode AbstractServerSocketInterface::cmdDeckSyncCheck(const Command_DeckSyncCheck &cmd, + ResponseContainer &rc) +{ + if (authState != PasswordRight) { + return Response::RespFunctionNotAllowed; + } + + sqlInterface->checkSql(); + + Response_DeckSyncStatus *re = new Response_DeckSyncStatus; + if (!syncCheckHelper(0, QString(), userInfo->id(), static_cast(cmd.last_sync_time()), re)) { + delete re; + return Response::RespContextError; + } + + QSqlQuery *clockQuery = sqlInterface->prepareQuery("select unix_timestamp(now())"); + if (!sqlInterface->execSqlQuery(clockQuery) || !clockQuery->next()) { + delete re; + return Response::RespContextError; + } + re->set_server_time(clockQuery->value(0).toULongLong()); + rc.setResponseExtension(re); + return Response::RespOk; +} + Response::ResponseCode AbstractServerSocketInterface::cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc) { diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index f05fd5ac1..7a01f7c1c 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -56,6 +56,7 @@ class Command_DeckShareList; class Command_DeckShareListMine; class Command_DeckShareRemove; class Command_DeckShareDownload; +class Command_DeckSyncCheck; class Command_ReplayList; class Command_ReplayDownload; class Command_ReplayModifyMatch; @@ -111,6 +112,7 @@ class Command_ViewLogHistory; class Command_WarnUser; class QObject; class ResponseContainer; +class Response_DeckSyncStatus; class AbstractServerSocketInterface : public Server_ProtocolHandler { @@ -163,6 +165,9 @@ private: bool storeDeckVersion(int deckId, const QString &content, const QString &hash); DeckList *getDeckFromDatabase(int deckId); Response::ResponseCode cmdDeckDownload(const Command_DeckDownload &cmd, ResponseContainer &rc); + bool + syncCheckHelper(int folderId, const QString &prefix, int userId, qint64 sinceSecs, Response_DeckSyncStatus *status); + Response::ResponseCode cmdDeckSyncCheck(const Command_DeckSyncCheck &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckShareCreate(const Command_DeckShareCreate &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckShareList(const Command_DeckShareList &cmd, ResponseContainer &rc); diff --git a/tests/deck_sync_sidecar_test.cpp b/tests/deck_sync_sidecar_test.cpp index 76e18d797..a86c8dc6d 100644 --- a/tests/deck_sync_sidecar_test.cpp +++ b/tests/deck_sync_sidecar_test.cpp @@ -125,12 +125,15 @@ TEST(DeckSyncSidecarTest, ModeRoundTripsAndDefaultsToLinked) ASSERT_TRUE(dir.isValid()); DeckSyncSidecar sidecar(sidecarPath(dir)); EXPECT_EQ(sidecar.mode, DeckSyncSidecar::MODE_LINKED); + EXPECT_EQ(sidecar.lastSyncSecs, 0); sidecar.mode = DeckSyncSidecar::MODE_SEPARATE; + sidecar.lastSyncSecs = 1791739200; EXPECT_TRUE(sidecar.save()); DeckSyncSidecar reloaded(sidecarPath(dir)); EXPECT_TRUE(reloaded.load()); EXPECT_EQ(reloaded.mode, DeckSyncSidecar::MODE_SEPARATE); + EXPECT_EQ(reloaded.lastSyncSecs, 1791739200); } TEST(DeckSyncSidecarTest, PathForDeckPathBuildsSidecarPath) diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index 0a2fb31e0..cc8b21ecd 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -606,6 +606,33 @@ TEST_F(SettingsDefaultsTest, DeckSync_ConflictDefault_SetAndGet) ASSERT_EQ(s.getConflictDefault(), DeckSyncConflictAsk); } +TEST_F(SettingsDefaultsTest, DeckSync_AutoSync_Default) +{ + DeckSyncSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getAutoSync(), true); +} + +TEST_F(SettingsDefaultsTest, DeckSync_AutoSync_SetAndGet) +{ + DeckSyncSettings s(settingsPath, nullptr); + s.setAutoSync(false); + ASSERT_EQ(s.getAutoSync(), false); + s.setAutoSync(true); + ASSERT_EQ(s.getAutoSync(), true); +} + +TEST_F(SettingsDefaultsTest, DeckSync_SyncIntervalMinutes_DefaultAndBounds) +{ + DeckSyncSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getSyncIntervalMinutes(), 5); + s.setSyncIntervalMinutes(15); + ASSERT_EQ(s.getSyncIntervalMinutes(), 15); + s.setSyncIntervalMinutes(0); + ASSERT_EQ(s.getSyncIntervalMinutes(), 1); + s.setSyncIntervalMinutes(600); + ASSERT_EQ(s.getSyncIntervalMinutes(), 60); +} + // --- NetworkSettings --- TEST_F(SettingsDefaultsTest, Network_ClientID_Default) From 1433af321d541641b20ee090c3e01df79fdfef19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 07:21:42 +0200 Subject: [PATCH 16/18] Fix ZeldaZach review for PR 7496: deck path in ctor, server-time baseline, single-query sync check --- .../interface/deck_sync/deck_sync_manager.cpp | 11 +++-- .../widgets/tabs/tab_deck_storage.cpp | 7 +++ .../libcockatrice/settings/paths_settings.cpp | 1 + .../libcockatrice/settings/paths_settings.h | 1 + servatrice/src/serversocketinterface.cpp | 48 ++++++------------- servatrice/src/serversocketinterface.h | 3 +- 6 files changed, 30 insertions(+), 41 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 768ba3411..4eceeee9c 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -299,11 +299,9 @@ void DeckSyncManager::syncNow() .modified = QFileInfo(absolutePath).lastModified()}); } - if (!firstSyncRun && sidecar.lastSyncSecs > 0) { - sendSyncCheck(sidecar.lastSyncSecs); - return; - } - fetchFullList(); + // Always ask the server first: its server_time becomes the sync baseline, + // so uploads racing the DeckList response cannot fall outside the window. + sendSyncCheck(sidecar.lastSyncSecs); } void DeckSyncManager::fetchFullList() @@ -334,6 +332,9 @@ void DeckSyncManager::onCheckFinished(const Response &response, const CommandCon return; } const Response_DeckSyncStatus &status = response.GetExtension(Response_DeckSyncStatus::ext); + if (status.has_server_time()) { + pendingServerTime = static_cast(status.server_time()); + } DeckSyncSidecar sidecar(sidecarPath); if (!sidecar.load()) { fetchFullList(); diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 48111f6ea..8f8555292 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -260,6 +260,13 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, syncManager = new DeckSyncManager( client, [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }, this); + syncManager->setDeckPath(SettingsCache::instance().paths().getDeckPath()); + connect(&SettingsCache::instance().paths(), &PathsSettings::deckPathChanged, this, [this] { + const QString deckPath = SettingsCache::instance().paths().getDeckPath(); + localDirModel->setRootPath(deckPath); + localDirView->setRootIndex(localDirModel->index(localDirModel->rootPath(), 0)); + syncManager->setDeckPath(deckPath); + }); syncManager->setConflictDefault(SettingsCache::instance().deckSync().getConflictDefault()); syncManager->setAutoSyncEnabled(SettingsCache::instance().deckSync().getAutoSync()); syncManager->setSyncIntervalMinutes(SettingsCache::instance().deckSync().getSyncIntervalMinutes()); diff --git a/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp b/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp index 41ccf6460..4f92b017c 100644 --- a/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp @@ -71,6 +71,7 @@ QString PathsSettings::getRedirectCachePath() const void PathsSettings::setDeckPath(const QString &_deckPath) { setValue(_deckPath, "decks"); + emit deckPathChanged(); } void PathsSettings::setFiltersPath(const QString &_filtersPath) diff --git a/libcockatrice_settings/libcockatrice/settings/paths_settings.h b/libcockatrice_settings/libcockatrice/settings/paths_settings.h index fa00428e3..aef65aff5 100644 --- a/libcockatrice_settings/libcockatrice/settings/paths_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/paths_settings.h @@ -59,6 +59,7 @@ public: void setAppDirWarningAcknowledged(bool _appDirWarningAcknowledged); signals: + void deckPathChanged(); void cardDatabasePathChanged(); void picsPathChanged(); void themeChanged(); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 6da45fd41..50a544538 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -898,51 +898,31 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSetVisibility(const return Response::RespOk; } -bool AbstractServerSocketInterface::syncCheckHelper(int folderId, - const QString &prefix, - int userId, - qint64 sinceSecs, - Response_DeckSyncStatus *status) +bool AbstractServerSocketInterface::syncCheckHelper(int userId, qint64 sinceSecs, Response_DeckSyncStatus *status) { - QSqlQuery *query = sqlInterface->prepareQuery("select id, name from {prefix}_decklist_folders where id_parent = " - ":id_parent and id_user = :id_user"); - query->bindValue(":id_parent", folderId); - query->bindValue(":id_user", userId); - if (!sqlInterface->execSqlQuery(query)) { - return false; - } - QList> subfolders; - while (query->next()) { - subfolders.append({query->value(0).toInt(), query->value(1).toString()}); - } - for (const auto &[subfolderId, subfolderName] : subfolders) { - const QString path = prefix.isEmpty() ? subfolderName : prefix + "/" + subfolderName; - if (!syncCheckHelper(subfolderId, path, userId, sinceSecs, status)) { - return false; - } - } - - query = sqlInterface->prepareQuery("select id, name, guid, hash, last_modified, deleted from " - "{prefix}_decklist_files where id_folder = :id_folder and id_user = :id_user " - "and (last_modified is null or last_modified > FROM_UNIXTIME(:since))"); - query->bindValue(":id_folder", folderId); + // Single query over the user's files: folder rows are hard-deleted while + // their files linger as tombstones, so walking the live folder tree would + // miss deletes pointing at nonexistent folders. The client only checks + // changed_files_size(), so folder paths are not reconstructed here. + QSqlQuery *query = sqlInterface->prepareQuery("select id, guid, hash, last_modified, deleted from " + "{prefix}_decklist_files where id_user = :id_user " + "and (last_modified is null or last_modified >= " + "FROM_UNIXTIME(:since))"); query->bindValue(":id_user", userId); query->bindValue(":since", sinceSecs); if (!sqlInterface->execSqlQuery(query)) { return false; } while (query->next()) { - const QString name = query->value(1).toString(); DeckSyncChangedFile *changed = status->add_changed_files(); - changed->set_path((prefix.isEmpty() ? name : prefix + "/" + name).toStdString()); changed->set_deck_id(query->value(0).toUInt()); - changed->set_guid(query->value(2).toString().toStdString()); - changed->set_hash(query->value(3).toString().toStdString()); - const QDateTime lastModified = query->value(4).toDateTime(); + changed->set_guid(query->value(1).toString().toStdString()); + changed->set_hash(query->value(2).toString().toStdString()); + const QDateTime lastModified = query->value(3).toDateTime(); if (lastModified.isValid()) { changed->set_last_modified(lastModified.toSecsSinceEpoch()); } - changed->set_deleted(query->value(5).toBool()); + changed->set_deleted(query->value(4).toBool()); } return true; } @@ -957,7 +937,7 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSyncCheck(const Com sqlInterface->checkSql(); Response_DeckSyncStatus *re = new Response_DeckSyncStatus; - if (!syncCheckHelper(0, QString(), userInfo->id(), static_cast(cmd.last_sync_time()), re)) { + if (!syncCheckHelper(userInfo->id(), static_cast(cmd.last_sync_time()), re)) { delete re; return Response::RespContextError; } diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index 7a01f7c1c..a55f8df0f 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -165,8 +165,7 @@ private: bool storeDeckVersion(int deckId, const QString &content, const QString &hash); DeckList *getDeckFromDatabase(int deckId); Response::ResponseCode cmdDeckDownload(const Command_DeckDownload &cmd, ResponseContainer &rc); - bool - syncCheckHelper(int folderId, const QString &prefix, int userId, qint64 sinceSecs, Response_DeckSyncStatus *status); + bool syncCheckHelper(int userId, qint64 sinceSecs, Response_DeckSyncStatus *status); Response::ResponseCode cmdDeckSyncCheck(const Command_DeckSyncCheck &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckShareCreate(const Command_DeckShareCreate &cmd, ResponseContainer &rc); From ad37bee0a3605c003b89582ba32ce61eb92811ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 1 Nov 2026 13:40:50 +0100 Subject: [PATCH 17/18] [DeckSync] Address auto-sync review: disconnect, backfill and modal conflicts - Fail the run when the connection drops while a sync is running, and clear the active conflict, so the queue is not sent on a dead client and a re-login as another account never receives this profile's uploads. - Backfill last_modified from upload_time in the 37 -> 38 migration and drop the "last_modified is null" arm from the sync-check query, so rows that predate the column stop reporting as changed on every poll. - Skip conflicts during auto runs and report how many were skipped, instead of opening the modal conflict dialog from a timer or a file-watcher event. --- .../interface/deck_sync/deck_sync_manager.cpp | 30 ++++++++++++++++++- .../interface/deck_sync/deck_sync_manager.h | 3 ++ .../migrations/servatrice_0040_to_0041.sql | 6 ++++ servatrice/src/serversocketinterface.cpp | 5 ++-- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 4eceeee9c..3f8d54559 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -109,6 +109,14 @@ DeckSyncManager::DeckSyncManager(AbstractClient *_client, ColorIdentityProvider if (status == StatusDisconnected) { offline = true; debounceTimer->stop(); + // A run cannot survive the session it was started on: the in-flight + // command comes back RespNotConnected, the remaining queue would be + // sent on a dead client, and a re-login as another account would + // receive this profile's uploads and deletes. + if (syncing) { + activeConflict = ActiveConflict(); + failSync(tr("Disconnected.")); + } } }, Qt::UniqueConnection); @@ -201,7 +209,7 @@ void DeckSyncManager::onDebounceTimeout() return; } if (canAutoRun()) { - syncNow(); + startSync(true); } } @@ -233,10 +241,16 @@ bool DeckSyncManager::isSyncing() const } void DeckSyncManager::syncNow() +{ + startSync(false); +} + +void DeckSyncManager::startSync(bool _autoRun) { if (syncing) { return; } + autoRun = _autoRun; const DeckSyncProfile runProfile = buildProfile(); if (client == nullptr || !runProfile.isValid() || deckPath.isEmpty()) { emit syncFinished(SyncSummary{}, tr("Not connected to a server.")); @@ -253,6 +267,7 @@ void DeckSyncManager::syncNow() downloadedCount = 0; unchangedCount = 0; skippedCount = 0; + conflictCount = 0; pendingUploads.clear(); pendingDownloads.clear(); pendingDeletes.clear(); @@ -746,6 +761,16 @@ void DeckSyncManager::queueConflict(const QString &relativePath, void DeckSyncManager::handleConflictDownload(const QString &relativePath, const QString &serverDeckString) { const ConflictDownload ctx = pendingConflicts.take(relativePath); + if (autoRun) { + // An auto run has no user in front of it: opening the modal conflict + // dialog from a timer or a file-watcher event would pop up mid-game. + // Leave the path untouched so the next manual sync classifies it again + // and asks. + ++conflictCount; + ++skippedCount; + processNext(); + return; + } if (sessionDefault == DeckSyncConflictKeepMine) { applyUpload(relativePath, ctx.serverPath, ctx.deckId, ctx.guid); processNext(); @@ -1245,6 +1270,9 @@ void DeckSyncManager::finishSync() .arg(summary.downloaded) .arg(summary.unchanged) .arg(summary.skipped)); + if (conflictCount > 0) { + emit statusMessage(tr("%1 conflicting decks skipped. Press Sync now to resolve them.").arg(conflictCount)); + } emit syncFinished(summary, QString()); } diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.h b/cockatrice/src/interface/deck_sync/deck_sync_manager.h index 98c9301ac..7d3c6358d 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.h +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.h @@ -159,6 +159,7 @@ private: qint64 lastModifiedSecs = 0; }; + void startSync(bool autoRun); void failSync(const QString &error); void finishSync(); [[nodiscard]] SyncSummary currentSummary() const; @@ -240,6 +241,8 @@ private: int downloadedCount = 0; int unchangedCount = 0; int skippedCount = 0; + int conflictCount = 0; + bool autoRun = false; }; #endif // DECK_SYNC_MANAGER_H diff --git a/servatrice/migrations/servatrice_0040_to_0041.sql b/servatrice/migrations/servatrice_0040_to_0041.sql index 83edf871f..aed483d04 100644 --- a/servatrice/migrations/servatrice_0040_to_0041.sql +++ b/servatrice/migrations/servatrice_0040_to_0041.sql @@ -16,6 +16,12 @@ ALTER TABLE `cockatrice_decklist_files` ADD COLUMN `last_modified` datetime NULL AFTER `hash`, ADD COLUMN `deleted` tinyint(1) NOT NULL DEFAULT 0 AFTER `last_modified`; +-- Rows created before this migration only get a last_modified when a client +-- re-uploads them, so every sync check would keep reporting them as changed. +-- Backfill from the upload time once, here, instead of carrying a +-- "last_modified is null" branch in the poll query forever. +UPDATE `cockatrice_decklist_files` SET `last_modified` = `upload_time` WHERE `last_modified` IS NULL; + 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, diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 50a544538..51c5b8d92 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -904,10 +904,11 @@ bool AbstractServerSocketInterface::syncCheckHelper(int userId, qint64 sinceSecs // their files linger as tombstones, so walking the live folder tree would // miss deletes pointing at nonexistent folders. The client only checks // changed_files_size(), so folder paths are not reconstructed here. + // last_modified is backfilled by the 37 -> 38 migration, so a plain range + // check is enough. QSqlQuery *query = sqlInterface->prepareQuery("select id, guid, hash, last_modified, deleted from " "{prefix}_decklist_files where id_user = :id_user " - "and (last_modified is null or last_modified >= " - "FROM_UNIXTIME(:since))"); + "and last_modified >= FROM_UNIXTIME(:since)"); query->bindValue(":id_user", userId); query->bindValue(":since", sinceSecs); if (!sqlInterface->execSqlQuery(query)) { From 87635bd9748840fac2596ce17532633a317029b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 11 Oct 2026 17:10:01 +0200 Subject: [PATCH 18/18] [Client+Server] Deck version history with restore --- cockatrice/CMakeLists.txt | 1 + .../widgets/dialogs/dlg_deck_history.cpp | 176 ++++++++++++++++++ .../widgets/dialogs/dlg_deck_history.h | 73 ++++++++ .../settings_page/general_settings_page.cpp | 9 + .../settings_page/general_settings_page.h | 2 + .../widgets/tabs/tab_deck_storage.cpp | 60 ++++++ .../interface/widgets/tabs/tab_deck_storage.h | 4 + .../libcockatrice/protocol/pb/CMakeLists.txt | 2 + .../pb/command_deck_history_list.proto | 12 ++ .../libcockatrice/protocol/pb/response.proto | 1 + .../pb/response_deck_history_list.proto | 19 ++ .../protocol/pb/session_commands.proto | 1 + .../settings/deck_sync_settings.cpp | 11 ++ .../settings/deck_sync_settings.h | 3 + servatrice/src/serversocketinterface.cpp | 48 +++++ servatrice/src/serversocketinterface.h | 3 + tests/settings/settings_defaults_test.cpp | 12 ++ 17 files changed, 437 insertions(+) create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_deck_history.cpp create mode 100644 cockatrice/src/interface/widgets/dialogs/dlg_deck_history.h create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_history_list.proto create mode 100644 libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_history_list.proto diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 7ffe97994..fd5e230b1 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -33,6 +33,7 @@ set(cockatrice_SOURCES src/interface/widgets/dialogs/dlg_convert_deck_to_cod_format.cpp src/interface/widgets/dialogs/dlg_create_game.cpp src/interface/widgets/dialogs/dlg_crash_report.cpp + src/interface/widgets/dialogs/dlg_deck_history.cpp src/interface/widgets/dialogs/dlg_default_tags_editor.cpp src/interface/widgets/dialogs/dlg_edit_avatar.cpp src/interface/widgets/dialogs/dlg_edit_password.cpp diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.cpp new file mode 100644 index 000000000..1e76ff7ed --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.cpp @@ -0,0 +1,176 @@ +#include "dlg_deck_history.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ +QString formatVersionTime(qint64 secsSinceEpoch) +{ + return QLocale().toString(QDateTime::fromSecsSinceEpoch(secsSinceEpoch), QLocale::ShortFormat); +} +} // namespace + +DlgDeckHistory::DlgDeckHistory(int _deckId, + const QString &_deckName, + int _limit, + AbstractClient *_client, + QWidget *parent) + : QDialog(parent), deckId(_deckId), deckName(_deckName), limit(_limit), client(_client) +{ + auto *layout = new QVBoxLayout(this); + + detailLabel = new QLabel(this); + detailLabel->setWordWrap(true); + versionsList = new QListWidget(this); + previewList = new QListWidget(this); + restoreButton = new QPushButton(this); + restoreButton->setEnabled(false); + + layout->addWidget(detailLabel); + layout->addWidget(versionsList); + layout->addWidget(previewList); + layout->addWidget(restoreButton); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, this); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + layout->addWidget(buttons); + + connect(versionsList, &QListWidget::currentRowChanged, this, &DlgDeckHistory::onSelectionChanged); + connect(restoreButton, &QPushButton::clicked, this, &DlgDeckHistory::onRestoreClicked); + + Command_DeckHistoryList listCmd; + listCmd.set_deck_id(deckId); + listCmd.set_limit(static_cast(limit)); + PendingCommand *listPend = AbstractClient::prepareSessionCommand(listCmd); + connect(listPend, &PendingCommand::finished, this, &DlgDeckHistory::onHistoryFinished); + client->sendCommand(listPend); + + Command_DeckDownload downloadCmd; + downloadCmd.set_deck_id(deckId); + PendingCommand *downloadPend = AbstractClient::prepareSessionCommand(downloadCmd); + connect(downloadPend, &PendingCommand::finished, this, &DlgDeckHistory::onCurrentFinished); + client->sendCommand(downloadPend); + + retranslateUi(); +} + +void DlgDeckHistory::retranslateUi() +{ + setWindowTitle(tr("Deck history")); + detailLabel->setText( + tr("Stored versions of %1 (newest first). Restoring uploads the version as new content.").arg(deckName)); + restoreButton->setText(tr("Restore selected version")); + renderVersions(); + renderPreview(); +} + +void DlgDeckHistory::onHistoryFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (response.response_code() != Response::RespOk) { + detailLabel->setText(tr("Could not load version history.")); + return; + } + const Response_DeckHistoryList &list = response.GetExtension(Response_DeckHistoryList::ext); + versions.clear(); + for (const DeckVersionInfo &version : list.versions()) { + DeckHistoryVersion entry; + entry.versionNo = version.version_no(); + entry.createdAtSecs = static_cast(version.created_at()); + entry.createdByClientId = QString::fromStdString(version.created_by_clientid()); + entry.hash = QString::fromStdString(version.hash()); + entry.content = QString::fromStdString(version.content()); + versions.append(entry); + } + historyArrived = true; + renderVersions(); + renderPreview(); +} + +void DlgDeckHistory::onCurrentFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (response.response_code() != Response::RespOk) { + detailLabel->setText(tr("Could not load the current deck.")); + return; + } + const Response_DeckDownload &download = response.GetExtension(Response_DeckDownload::ext); + currentContent = QString::fromStdString(download.deck()); + currentArrived = true; + renderPreview(); +} + +void DlgDeckHistory::renderVersions() +{ + const int row = versionsList->currentRow(); + versionsList->clear(); + for (const DeckHistoryVersion &version : versions) { + QString label = tr("Version %1 — %2").arg(version.versionNo).arg(formatVersionTime(version.createdAtSecs)); + if (!version.createdByClientId.isEmpty()) { + label += tr(" — %1").arg(version.createdByClientId); + } + versionsList->addItem(label); + } + if (row >= 0 && row < versionsList->count()) { + versionsList->setCurrentRow(row); + } else if (versionsList->count() > 0) { + versionsList->setCurrentRow(0); + } + onSelectionChanged(); +} + +void DlgDeckHistory::renderPreview() +{ + previewList->clear(); + restoreButton->setEnabled(false); + const int row = versionsList->currentRow(); + if (!historyArrived || !currentArrived || row < 0 || row >= versions.size()) { + return; + } + DeckList currentDeck; + DeckList selectedDeck; + if (!currentDeck.loadFromString_Native(currentContent) || + !selectedDeck.loadFromString_Native(versions.at(row).content)) { + return; + } + const DeckDiffResult shown = DeckDiff::diff(currentDeck, selectedDeck); + for (const DeckCardChange &change : shown.changes) { + previewList->addItem( + tr("%1: %2 %3 → %4").arg(change.zone).arg(change.cardName).arg(change.oldCount).arg(change.newCount)); + } + restoreButton->setEnabled(true); +} + +void DlgDeckHistory::onSelectionChanged() +{ + renderPreview(); +} + +void DlgDeckHistory::onRestoreClicked() +{ + const int row = versionsList->currentRow(); + if (row < 0 || row >= versions.size()) { + return; + } + if (QMessageBox::question(this, tr("Restore version"), + tr("Restore version %1 as the current deck content? This becomes a new version.") + .arg(versions.at(row).versionNo)) != QMessageBox::Yes) { + return; + } + emit restoreRequested(deckId, versions.at(row).content); + accept(); +} diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.h b/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.h new file mode 100644 index 000000000..cba196915 --- /dev/null +++ b/cockatrice/src/interface/widgets/dialogs/dlg_deck_history.h @@ -0,0 +1,73 @@ +/** + * @file dlg_deck_history.h + * @brief Browser for the stored versions of one server deck. + */ + +#ifndef DLG_DECK_HISTORY_H +#define DLG_DECK_HISTORY_H + +#include +#include +#include + +class AbstractClient; +class CommandContainer; +class QLabel; +class QListWidget; +class QPushButton; +class Response; + +/** @brief One stored deck version for display. */ +struct DeckHistoryVersion +{ + unsigned int versionNo = 0; + qint64 createdAtSecs = 0; + QString createdByClientId; + QString hash; + QString content; +}; + +/** + * @class DlgDeckHistory + * @brief Lists stored versions with a diff preview against the current deck. + * + * Restoring uploads the selected version as the new current content through + * the regular deck-upload path, so it becomes a version itself instead of + * rewriting history. Rejected dialogs restore nothing. + */ +class DlgDeckHistory : public QDialog +{ + Q_OBJECT +public: + DlgDeckHistory(int deckId, const QString &deckName, int limit, AbstractClient *client, QWidget *parent = nullptr); + void retranslateUi(); + +signals: + void restoreRequested(int deckId, const QString &content); + +private slots: + void onHistoryFinished(const Response &response, const CommandContainer &commandContainer); + void onCurrentFinished(const Response &response, const CommandContainer &commandContainer); + void onSelectionChanged(); + void onRestoreClicked(); + +private: + void renderVersions(); + void renderPreview(); + + int deckId = -1; + QString deckName; + int limit = 20; + AbstractClient *client = nullptr; + QList versions; + QString currentContent; + bool historyArrived = false; + bool currentArrived = false; + + QLabel *detailLabel; + QListWidget *versionsList; + QListWidget *previewList; + QPushButton *restoreButton; +}; + +#endif // DLG_DECK_HISTORY_H diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index 6a24f7db6..84e47c33b 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -383,12 +383,20 @@ GeneralSettingsPage::GeneralSettingsPage() connect(&syncIntervalEdit, qOverload(&QSpinBox::valueChanged), &SettingsCache::instance().deckSync(), &DeckSyncSettings::setSyncIntervalMinutes); + historyLimitEdit.setMinimum(1); + historyLimitEdit.setMaximum(100); + historyLimitEdit.setValue(settings.deckSync().getHistoryLimit()); + connect(&historyLimitEdit, qOverload(&QSpinBox::valueChanged), &SettingsCache::instance().deckSync(), + &DeckSyncSettings::setHistoryLimit); + auto *deckSyncGrid = new QGridLayout; deckSyncGrid->addWidget(&conflictDefaultLabel, 0, 0); deckSyncGrid->addWidget(&conflictDefaultSelector, 0, 1); deckSyncGrid->addWidget(&autoSyncCheckBox, 1, 0, 1, 2); deckSyncGrid->addWidget(&syncIntervalLabel, 2, 0); deckSyncGrid->addWidget(&syncIntervalEdit, 2, 1); + deckSyncGrid->addWidget(&historyLimitLabel, 3, 0); + deckSyncGrid->addWidget(&historyLimitEdit, 3, 1); deckSyncGroupBox = new QGroupBox; deckSyncGroupBox->setLayout(deckSyncGrid); @@ -692,5 +700,6 @@ void GeneralSettingsPage::retranslateUi() autoSyncCheckBox.setText(tr("Sync decks automatically")); syncIntervalLabel.setText(tr("Check every:")); syncIntervalEdit.setSuffix(" " + tr("min")); + historyLimitLabel.setText(tr("Kept versions per deck:")); resetAllPathsButton->setText(tr("Reset all paths")); } diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h index f410abdf2..4ed25b279 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.h @@ -111,6 +111,8 @@ private: QCheckBox autoSyncCheckBox; QLabel syncIntervalLabel; QSpinBox syncIntervalEdit; + QLabel historyLimitLabel; + QSpinBox historyLimitEdit; }; #endif // COCKATRICE_GENERAL_SETTINGS_PAGE_H diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index 8f8555292..a7522d256 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -8,6 +8,7 @@ #include "../cards/additional_info/deck_color_identity.h" #include "../deck_share/deck_share_utils.h" #include "../deck_share/share_bar_widget.h" +#include "../dialogs/dlg_deck_history.h" #include "../dialogs/dlg_sync_conflict.h" #include "../dialogs/dlg_sync_onboarding.h" #include "../interface/widgets/server/remote/remote_decklist_tree_widget.h" @@ -241,6 +242,10 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, aSyncNow->setIcon(qApp->style()->standardIcon(QStyle::SP_BrowserReload)); connect(aSyncNow, &QAction::triggered, this, &TabDeckStorage::actSyncNow); + aDeckHistory = new QAction(this); + aDeckHistory->setIcon(qApp->style()->standardIcon(QStyle::SP_FileDialogDetailedView)); + connect(aDeckHistory, &QAction::triggered, this, &TabDeckStorage::actDeckHistory); + // Add actions to toolbars leftToolBar->addAction(aOpenLocalDeck); leftToolBar->addAction(aRenameLocal); @@ -253,6 +258,7 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, rightToolBar->addAction(aOpenRemoteDeck); rightToolBar->addAction(aDownload); rightToolBar->addAction(aSyncNow); + rightToolBar->addAction(aDeckHistory); rightToolBar->addAction(aShareDecks); rightToolBar->addAction(aPublishDeck); rightToolBar->addAction(aNewFolder); @@ -314,6 +320,7 @@ void TabDeckStorage::retranslateUi() aPublishDeck->setText(tr("Publish/unpublish deck")); aOpenDecksFolder->setText(tr("Open decks folder")); aSyncNow->setText(tr("Sync now")); + aDeckHistory->setText(tr("Version history")); renderSyncStatus(); shareBar->retranslateUi(); if (shareBar->isVisible()) { @@ -369,6 +376,7 @@ void TabDeckStorage::setRemoteEnabled(bool enabled) aPublishDeck->setEnabled(enabled); aNewFolder->setEnabled(enabled); aDeleteRemoteDeck->setEnabled(enabled); + aDeckHistory->setEnabled(enabled); aSyncNow->setEnabled(enabled && !syncManager->isSyncing()); if (enabled) { @@ -654,6 +662,58 @@ void TabDeckStorage::renderSyncStatus() } } +void TabDeckStorage::actDeckHistory() +{ + QList files; + for (const auto curRight : serverDirView->getCurrentSelection()) { + if (auto *file = dynamic_cast(curRight)) { + files.append(file); + } + } + if (files.size() != 1) { + QMessageBox::information(this, tr("Version history"), tr("Select a single server deck to view its history.")); + return; + } + + auto *dialog = new DlgDeckHistory(files.first()->getId(), files.first()->getName(), + SettingsCache::instance().deckSync().getHistoryLimit(), client, this); + connect(dialog, &DlgDeckHistory::restoreRequested, this, &TabDeckStorage::onHistoryRestoreRequested); + dialog->setAttribute(Qt::WA_DeleteOnClose); + dialog->show(); +} + +void TabDeckStorage::onHistoryRestoreRequested(int deckId, const QString &content) +{ + DeckList deck; + if (!deck.loadFromString_Native(content)) { + return; + } + const QString deckString = deck.writeToString_Native(); + if (deckString.length() > MAX_FILE_LENGTH) { + QMessageBox::critical(this, tr("Error"), tr("Restored deck is too large")); + return; + } + + Command_DeckUpload cmd; + cmd.set_deck_id(deckId); + cmd.set_deck_list(deckString.toStdString()); + cmd.set_color_identity(getDeckColorIdentity(deck, CardDatabaseManager::query()).toStdString()); + + PendingCommand *pend = client->prepareSessionCommand(cmd); + connect(pend, &PendingCommand::finished, this, &TabDeckStorage::onHistoryRestoreFinished); + client->sendCommand(pend); +} + +void TabDeckStorage::onHistoryRestoreFinished(const Response &response, const CommandContainer & /*commandContainer*/) +{ + if (response.response_code() != Response::RespOk) { + QMessageBox::critical(this, tr("Error"), tr("Failed to restore deck version")); + return; + } + onSyncStatusMessage(tr("Version restored.")); + serverDirView->refreshTree(); +} + 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 610cce891..6254c6505 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.h @@ -55,6 +55,7 @@ private: QAction *aOpenDecksFolder; QAction *aOpenRemoteDeck, *aDownload, *aShareDecks, *aPublishDeck, *aNewFolder, *aDeleteRemoteDeck; QAction *aSyncNow; + QAction *aDeckHistory; QLabel *syncStatusLabel; DeckSyncManager *syncManager; QString syncUsername; @@ -130,6 +131,9 @@ private slots: void deleteDeckFinished(const Response &response, const CommandContainer &commandContainer); void actSyncNow(); + void actDeckHistory(); + void onHistoryRestoreRequested(int deckId, const QString &content); + void onHistoryRestoreFinished(const Response &response, const CommandContainer &commandContainer); void onFirstSyncDetected(int localCount, int serverCount); void onConflictRequest(const DeckSyncConflict &conflict); void onSyncStatusMessage(const QString &message); diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt index 0860b407b..75345f236 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/CMakeLists.txt @@ -16,6 +16,7 @@ set(PROTO_FILES command_deck_del_dir.proto command_deck_download.proto command_deck_download_public.proto + command_deck_history_list.proto command_deck_list.proto command_deck_list_other_user.proto command_deck_new_dir.proto @@ -147,6 +148,7 @@ set(PROTO_FILES response_ban_history.proto response_card_art_rule_entry.proto response_deck_download.proto + response_deck_history_list.proto response_deck_list.proto response_deck_share_create.proto response_deck_share_download.proto diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_history_list.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_history_list.proto new file mode 100644 index 000000000..5931503f4 --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/command_deck_history_list.proto @@ -0,0 +1,12 @@ +syntax = "proto2"; +import "session_commands.proto"; + +message Command_DeckHistoryList { + extend SessionCommand { + optional Command_DeckHistoryList ext = 1035; + } + optional sint32 deck_id = 1 [default = -1]; + // Maximum versions to return, newest first. Capped server-side by + // deck_sync/max_versions. + optional uint32 limit = 2 [default = 20]; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto index 8ec4c3554..65986c153 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response.proto @@ -86,6 +86,7 @@ message Response { DECK_SHARE_DOWNLOAD = 1105; // Response for shared deck download DECK_SHARE_LIST_MINE = 1106; // Response listing the caller's own shares DECK_SYNC_STATUS = 1107; // Response with changed decks since a timestamp + DECK_HISTORY_LIST = 1108; // Response listing deck versions CARD_ART_RULE_LIST = 1200; // Response containing a list of card art rules } diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_history_list.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_history_list.proto new file mode 100644 index 000000000..bcb8da7ca --- /dev/null +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/response_deck_history_list.proto @@ -0,0 +1,19 @@ +syntax = "proto2"; +import "response.proto"; + +message DeckVersionInfo { + optional uint32 version_no = 1; + optional uint64 created_at = 2; + optional string created_by_clientid = 3; + optional string hash = 4; + // Full deck content in native format, so the client can diff and + // restore without further round trips. + optional string content = 5; +} + +message Response_DeckHistoryList { + extend Response { + optional Response_DeckHistoryList ext = 1108; + } + repeated DeckVersionInfo versions = 1; +} diff --git a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto index b9a80021f..13ef307a9 100644 --- a/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto +++ b/libcockatrice_protocol/libcockatrice/protocol/pb/session_commands.proto @@ -38,6 +38,7 @@ message SessionCommand { DECK_SHARE_REMOVE = 1033; SUBMIT_PASSWORD_VERIFIER = 1034; DECK_SYNC_CHECK = 1035; + DECK_HISTORY_LIST = 1036; REPLAY_LIST = 1100; REPLAY_DOWNLOAD = 1101; REPLAY_MODIFY_MATCH = 1102; diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp index caa9ce9c6..2b03f4119 100644 --- a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.cpp @@ -42,3 +42,14 @@ void DeckSyncSettings::setSyncIntervalMinutes(int _syncIntervalMinutes) setValue(_syncIntervalMinutes, "syncIntervalMinutes"); emit syncIntervalMinutesChanged(_syncIntervalMinutes); } + +int DeckSyncSettings::getHistoryLimit() const +{ + return qBound(1, getValue("historyLimit", QString(), QString(), 20).toInt(), 100); +} + +void DeckSyncSettings::setHistoryLimit(int _historyLimit) +{ + setValue(_historyLimit, "historyLimit"); + emit historyLimitChanged(_historyLimit); +} diff --git a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h index 2552f52a9..5475ff343 100644 --- a/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/deck_sync_settings.h @@ -30,11 +30,14 @@ public: void setAutoSync(bool _autoSync); [[nodiscard]] int getSyncIntervalMinutes() const; void setSyncIntervalMinutes(int _syncIntervalMinutes); + [[nodiscard]] int getHistoryLimit() const; + void setHistoryLimit(int _historyLimit); signals: void conflictDefaultChanged(int conflictDefault); void autoSyncChanged(bool autoSync); void syncIntervalMinutesChanged(int syncIntervalMinutes); + void historyLimitChanged(int historyLimit); private: DeckSyncSettings(const DeckSyncSettings & /*other*/); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index 51c5b8d92..8f448d170 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -89,6 +89,7 @@ #include #include #include +#include #include #include #include @@ -130,6 +131,7 @@ #include #include #include +#include #include #include #include @@ -375,6 +377,8 @@ Response::ResponseCode AbstractServerSocketInterface::processExtendedSessionComm return cmdDeckDownload(cmd.GetExtension(Command_DeckDownload::ext), rc); case SessionCommand::DECK_SYNC_CHECK: return cmdDeckSyncCheck(cmd.GetExtension(Command_DeckSyncCheck::ext), rc); + case SessionCommand::DECK_HISTORY_LIST: + return cmdDeckHistoryList(cmd.GetExtension(Command_DeckHistoryList::ext), rc); case SessionCommand::REPLAY_LIST: return cmdReplayList(cmd.GetExtension(Command_ReplayList::ext), rc); case SessionCommand::REPLAY_DOWNLOAD: @@ -953,6 +957,50 @@ Response::ResponseCode AbstractServerSocketInterface::cmdDeckSyncCheck(const Com return Response::RespOk; } +Response::ResponseCode AbstractServerSocketInterface::cmdDeckHistoryList(const Command_DeckHistoryList &cmd, + ResponseContainer &rc) +{ + if (authState != PasswordRight) { + return Response::RespFunctionNotAllowed; + } + + sqlInterface->checkSql(); + + QSqlQuery *ownerQuery = + sqlInterface->prepareQuery("select 1 from {prefix}_decklist_files where id = :id and id_user = :id_user"); + ownerQuery->bindValue(":id", cmd.deck_id()); + ownerQuery->bindValue(":id_user", userInfo->id()); + if (!sqlInterface->execSqlQuery(ownerQuery)) { + return Response::RespContextError; + } + if (!ownerQuery->next()) { + return Response::RespNameNotFound; + } + + const int limit = qMin(cmd.has_limit() ? static_cast(cmd.limit()) : 20, servatrice->getDeckSyncMaxVersions()); + QSqlQuery *query = + sqlInterface->prepareQuery("select version_no, unix_timestamp(created_at), created_by_clientid, hash, " + "content from {prefix}_deck_versions where deck_id = :deck_id order by version_no " + "desc limit :limit"); + query->bindValue(":deck_id", cmd.deck_id()); + query->bindValue(":limit", qMax(1, limit)); + if (!sqlInterface->execSqlQuery(query)) { + return Response::RespContextError; + } + + Response_DeckHistoryList *re = new Response_DeckHistoryList; + while (query->next()) { + DeckVersionInfo *version = re->add_versions(); + version->set_version_no(query->value(0).toUInt()); + version->set_created_at(query->value(1).toULongLong()); + version->set_created_by_clientid(query->value(2).toString().toStdString()); + version->set_hash(query->value(3).toString().toStdString()); + version->set_content(query->value(4).toString().toStdString()); + } + rc.setResponseExtension(re); + return Response::RespOk; +} + Response::ResponseCode AbstractServerSocketInterface::cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc) { diff --git a/servatrice/src/serversocketinterface.h b/servatrice/src/serversocketinterface.h index a55f8df0f..20695accd 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -56,6 +56,7 @@ class Command_DeckShareList; class Command_DeckShareListMine; class Command_DeckShareRemove; class Command_DeckShareDownload; +class Command_DeckHistoryList; class Command_DeckSyncCheck; class Command_ReplayList; class Command_ReplayDownload; @@ -112,6 +113,7 @@ class Command_ViewLogHistory; class Command_WarnUser; class QObject; class ResponseContainer; +class Response_DeckHistoryList; class Response_DeckSyncStatus; class AbstractServerSocketInterface : public Server_ProtocolHandler @@ -167,6 +169,7 @@ private: Response::ResponseCode cmdDeckDownload(const Command_DeckDownload &cmd, ResponseContainer &rc); bool syncCheckHelper(int userId, qint64 sinceSecs, Response_DeckSyncStatus *status); Response::ResponseCode cmdDeckSyncCheck(const Command_DeckSyncCheck &cmd, ResponseContainer &rc); + Response::ResponseCode cmdDeckHistoryList(const Command_DeckHistoryList &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckDownloadPublic(const Command_DeckDownloadPublic &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckShareCreate(const Command_DeckShareCreate &cmd, ResponseContainer &rc); Response::ResponseCode cmdDeckShareList(const Command_DeckShareList &cmd, ResponseContainer &rc); diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index cc8b21ecd..04545b533 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -633,6 +633,18 @@ TEST_F(SettingsDefaultsTest, DeckSync_SyncIntervalMinutes_DefaultAndBounds) ASSERT_EQ(s.getSyncIntervalMinutes(), 60); } +TEST_F(SettingsDefaultsTest, DeckSync_HistoryLimit_DefaultAndBounds) +{ + DeckSyncSettings s(settingsPath, nullptr); + ASSERT_EQ(s.getHistoryLimit(), 20); + s.setHistoryLimit(50); + ASSERT_EQ(s.getHistoryLimit(), 50); + s.setHistoryLimit(0); + ASSERT_EQ(s.getHistoryLimit(), 1); + s.setHistoryLimit(500); + ASSERT_EQ(s.getHistoryLimit(), 100); +} + // --- NetworkSettings --- TEST_F(SettingsDefaultsTest, Network_ClientID_Default)