diff --git a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp index 2a1f205e1..247e6d504 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_manager.cpp @@ -274,11 +274,9 @@ void DeckSyncManager::syncNow() localFiles.insert(relativePath, file); } - 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() @@ -309,6 +307,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(); @@ -355,6 +356,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); @@ -434,8 +438,37 @@ void DeckSyncManager::classify(const QMap &serverFiles) 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()) { const DeckSyncRecord known = sidecar.get(it.key()); @@ -467,6 +500,7 @@ void DeckSyncManager::classify(const QMap &serverFiles) 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()) { @@ -481,6 +515,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. @@ -523,12 +575,39 @@ void DeckSyncManager::classify(const QMap &serverFiles) } } +< if (syncMode != DeckSyncMode::UploadOnly) { + 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; + } + 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); + } + } + 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; @@ -536,6 +615,7 @@ void DeckSyncManager::classify(const QMap &serverFiles) pendingDeletes.append(it.key()); } } + } sidecar.save(); processNext(); @@ -1011,7 +1091,13 @@ void DeckSyncManager::applyDownloadedDeck(const QString &relativePath, 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()); diff --git a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp index 17493643d..48dfeba5a 100644 --- a/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp +++ b/cockatrice/src/interface/deck_sync/deck_sync_sidecar.cpp @@ -5,6 +5,7 @@ #include #include #include +#include namespace { @@ -76,12 +77,12 @@ bool DeckSyncSidecar::save() const root.insert("records", records); QDir().mkpath(QFileInfo(filePath).absolutePath()); - QFile file(filePath); - if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) { + QSaveFile file(filePath); + if (!file.open(QIODevice::WriteOnly)) { return false; } file.write(QJsonDocument(root).toJson(QJsonDocument::Compact)); - return true; + return file.commit(); } bool DeckSyncSidecar::contains(const QString &relativePath) const diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index aae42397b..543e514ec 100644 --- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp +++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp @@ -261,6 +261,13 @@ TabDeckStorage::TabDeckStorage(TabSupervisor *_tabSupervisor, syncManager = new DeckSyncManager(this); syncManager->setClient(client); + 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->setColorIdentityProvider( [](const DeckList &deck) { return getDeckColorIdentity(deck, CardDatabaseManager::query()); }); syncManager->setConflictDefault(SettingsCache::instance().deckSync().getConflictDefault()); diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp index 31a6b622b..b918dd9ee 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.cpp @@ -13,12 +13,36 @@ namespace { using ZoneCardKey = QPair; +struct ZoneCardInfo +{ + int count = 0; + QString cardSetShortName; + QString cardSetNumber; + QString cardProviderId; +}; + +QMap zoneCardInfos(const DeckList &deck) +{ + QMap infos; + deck.forEachCard([&infos](InnerDecklistNode *zone, DecklistCardNode *card) { + ZoneCardInfo &info = infos[{zone->getName(), card->getName()}]; + if (info.count == 0) { + info.cardSetShortName = card->getCardSetShortName(); + info.cardSetNumber = card->getCardCollectorNumber(); + info.cardProviderId = card->getCardProviderId(); + } + info.count += card->getNumber(); + }); + return infos; +} + QMap zoneCardCounts(const DeckList &deck) { QMap counts; - deck.forEachCard([&counts](InnerDecklistNode *zone, DecklistCardNode *card) { - counts[{zone->getName(), card->getName()}] += card->getNumber(); - }); + const QMap infos = zoneCardInfos(deck); + for (auto it = infos.begin(); it != infos.end(); ++it) { + counts.insert(it.key(), it.value().count); + } return counts; } @@ -34,17 +58,25 @@ bool changeLessThan(const DeckCardChange &a, const DeckCardChange &b) DeckDiffResult DeckDiff::diff(const DeckList &oldDeck, const DeckList &newDeck) { const QMap oldCounts = zoneCardCounts(oldDeck); - const QMap newCounts = zoneCardCounts(newDeck); + const QMap newInfos = zoneCardInfos(newDeck); QSet keys(oldCounts.keyBegin(), oldCounts.keyEnd()); - keys.unite(QSet(newCounts.keyBegin(), newCounts.keyEnd())); + keys.unite(QSet(newInfos.keyBegin(), newInfos.keyEnd())); DeckDiffResult result; for (const ZoneCardKey &key : keys) { const int oldCount = oldCounts.value(key, 0); - const int newCount = newCounts.value(key, 0); - if (oldCount != newCount) { - result.changes.append({key.first, key.second, oldCount, newCount}); + const ZoneCardInfo newInfo = newInfos.value(key, ZoneCardInfo()); + if (oldCount != newInfo.count) { + DeckCardChange change; + change.zone = key.first; + change.cardName = key.second; + change.oldCount = oldCount; + change.newCount = newInfo.count; + change.cardSetShortName = newInfo.cardSetShortName; + change.cardSetNumber = newInfo.cardSetNumber; + change.cardProviderId = newInfo.cardProviderId; + result.changes.append(change); } } std::sort(result.changes.begin(), result.changes.end(), changeLessThan); @@ -69,21 +101,41 @@ DeckList DeckDiff::merge(const DeckList &baseDeck, const DeckDiffResult &localChanges, const DeckDiffResult &remoteChanges) { QMap counts = zoneCardCounts(baseDeck); + QMap printings; for (const DeckCardChange &change : localChanges.changes) { counts[{change.zone, change.cardName}] = change.newCount; + printings[{change.zone, change.cardName}] = change; } for (const DeckCardChange &change : remoteChanges.changes) { counts[{change.zone, change.cardName}] = change.newCount; + printings[{change.zone, change.cardName}] = change; } - DeckList merged(baseDeck.getMetadata(), DecklistNodeTree(), baseDeck.getSideboardPlans()); - QList keys = counts.keys(); - std::sort(keys.begin(), keys.end()); - for (const ZoneCardKey &key : keys) { - const int count = qMax(0, counts.value(key)); - for (int i = 0; i < count; ++i) { - merged.addCard(key.second, key.first); + DeckList merged = baseDeck; + QMap> nodesByKey; + merged.forEachCard([&nodesByKey](InnerDecklistNode *zone, DecklistCardNode *card) { + nodesByKey[{zone->getName(), card->getName()}].append(card); + }); + + for (auto it = counts.begin(); it != counts.end(); ++it) { + const ZoneCardKey &key = it.key(); + const int target = qMax(0, it.value()); + QList nodes = nodesByKey.value(key); + if (target == 0) { + for (DecklistCardNode *node : nodes) { + merged.getTree()->deleteNode(node); + } + } else if (nodes.isEmpty()) { + const DeckCardChange printing = printings.value(key); + merged.getTree()->addCard(printing.cardName, target, printing.zone, -1, printing.cardSetShortName, + printing.cardSetNumber, printing.cardProviderId, true); + } else { + nodes.first()->setNumber(target); + for (int i = 1; i < nodes.size(); ++i) { + merged.getTree()->deleteNode(nodes.at(i)); + } } } + merged.refreshDeckHash(); return merged; } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h index 2b4fb8732..563b54b70 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_diff.h @@ -14,10 +14,13 @@ class DeckList; /** @brief A single card-count change within one zone. */ struct DeckCardChange { - QString zone; ///< Zone name, e.g. "main". - QString cardName; ///< Card name. - int oldCount = 0; ///< Copies in the old deck. - int newCount = 0; ///< Copies in the new deck. + QString zone; ///< Zone name, e.g. "main". + QString cardName; ///< Card name. + int oldCount = 0; ///< Copies in the old deck. + int newCount = 0; ///< Copies in the new deck. + QString cardSetShortName; ///< Printing set carried from the new side, empty when unchanged. + QString cardSetNumber; ///< Printing collector number carried from the new side. + QString cardProviderId; ///< Printing provider id carried from the new side. /** @brief Copies added (positive) or removed (negative). */ [[nodiscard]] int delta() const { @@ -40,9 +43,9 @@ struct DeckDiffResult * @class DeckDiff * @brief Pure card-count diff and three-way merge for deck sync. * - * Compares card names and copy counts per zone only; printing information - * (set, collector number, provider) is not tracked, so a merged deck keeps - * base printings with merged counts. + * Compares card names and copy counts per zone only; a merged deck keeps + * base printings for existing cards and carries the changed side's printing + * for newly added cards. */ class DeckDiff { diff --git a/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp b/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp index 5f5725395..39314ccf6 100644 --- a/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/paths_settings.cpp @@ -69,6 +69,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 399337bbb..387a467a4 100644 --- a/libcockatrice_settings/libcockatrice/settings/paths_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/paths_settings.h @@ -39,6 +39,7 @@ public: void setSpoilerDatabasePath(const QString &_spoilerDatabasePath); signals: + void deckPathChanged(); void cardDatabasePathChanged(); void picsPathChanged(); void themeChanged(); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index b436f1ddf..894211a39 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -872,51 +872,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; } @@ -931,7 +911,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 9b831a619..2d664202f 100644 --- a/servatrice/src/serversocketinterface.h +++ b/servatrice/src/serversocketinterface.h @@ -159,8 +159,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);