From d10ec8a1138a6537defb4a85aa8ba10f054f2664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Tue, 13 Oct 2026 09:03:48 +0200 Subject: [PATCH] [Installer][Client] Fix review findings: backup markers, NSIS relocate, size cap --- cmake/NSIS.template.in | 88 ++++++++++-- cockatrice/src/interface/window_main.cpp | 94 +++++++++++-- .../utility/user_data_backup.cpp | 128 ++++++++++++++++-- .../libcockatrice/utility/user_data_backup.h | 33 ++++- tests/user_data_backup_test.cpp | 125 +++++++++++++++++ 5 files changed, 426 insertions(+), 42 deletions(-) diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index 0f040644d..54e3eb689 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -17,6 +17,9 @@ Var GuardDir Var OldUninstallString Var OldInstallDir Var DataBackupDir +Var DataBackupDone +Var LooseDecksBackupDone +Var LooseDeckNoticeShown Var StaleFindHandle Var StaleFindName Var IsUpgrade @@ -373,17 +376,17 @@ Function BackupUserDataBeforeUninstall ${If} $OldInstallDir == "" StrCpy $OldInstallDir $INSTDIR ${EndIf} -; Start from a clean slate only on the first backup of this run: %TEMP% survives -; across installer runs, so a stale backup must never be mistaken for this run's - -; but this function runs twice in sequence (32-bit then 64-bit), and an -; unconditional RMDir here would delete the first call's safety copy before the -; second uninstall runs. -${If} $DataBackupDir == "" - RMDir /r "$TEMP\CockatriceDataBackup" -${EndIf} +; No wipe of $TEMP\CockatriceDataBackup here: %TEMP% survives across installer +; runs, so the previous run's safety copy stays on disk until this run overwrites +; it. Stale content can never leak into this run because restore is gated on the +; per-run $DataBackupDone / $LooseDecksBackupDone flags below, not on what happens +; to be on disk. CopyFiles merges/overwrites, so calling this twice in sequence +; (32-bit then 64-bit uninstall) can only add safety, never delete the first +; call's copy. ${If} ${FileExists} "$OldInstallDir\portable.dat" ${AndIf} ${FileExists} "$OldInstallDir\data" StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup" + StrCpy $DataBackupDone "1" !insertmacro CockLog "pre-uninstall backup: copying $OldInstallDir\data (downloaded pics excluded)" DetailPrint "Backing up portable user data before removing previous version..." CreateDirectory "$DataBackupDir\data" @@ -397,16 +400,24 @@ ${AndIf} ${FileExists} "$OldInstallDir\data" ${EndIf} DetailPrint "Portable user data backup complete." ${EndIf} +; Deck extensions mirror DeckLoader::ACCEPTED_FILE_EXTENSIONS, minus *.txt: a +; blanket *.txt copy would also grab payload-adjacent text files, while the +; deck-specific extensions are unambiguous user content. ${If} ${FileExists} "$OldInstallDir\*.cod" ${OrIf} ${FileExists} "$OldInstallDir\*.dec" +${OrIf} ${FileExists} "$OldInstallDir\*.dek" +${OrIf} ${FileExists} "$OldInstallDir\*.mwDeck" ${If} $DataBackupDir == "" StrCpy $DataBackupDir "$TEMP\CockatriceDataBackup" ${EndIf} + StrCpy $LooseDecksBackupDone "1" !insertmacro CockLog "pre-uninstall backup: copying loose deck files" DetailPrint "Backing up deck files stored in the install directory..." CreateDirectory "$DataBackupDir\loose_decks" CopyFiles /SILENT "$OldInstallDir\*.cod" "$DataBackupDir\loose_decks\" CopyFiles /SILENT "$OldInstallDir\*.dec" "$DataBackupDir\loose_decks\" + CopyFiles /SILENT "$OldInstallDir\*.dek" "$DataBackupDir\loose_decks\" + CopyFiles /SILENT "$OldInstallDir\*.mwDeck" "$DataBackupDir\loose_decks\" ${EndIf} FunctionEnd @@ -415,10 +426,13 @@ StrCmp $DataBackupDir "" done ${If} $OldInstallDir == "" StrCpy $OldInstallDir $INSTDIR ${EndIf} -${If} ${FileExists} "$DataBackupDir\data" +${If} $DataBackupDone == "1" +${AndIf} ${FileExists} "$DataBackupDir\data" ${If} ${FileExists} "$OldInstallDir\data" - !insertmacro CockLog "post-uninstall restore: user data survived, backup kept" - DetailPrint "Previous user data survived the uninstall; backup kept at $DataBackupDir\data." + ; Existence-only: a partially-deleted data dir (locked files during the old + ; uninstall) looks the same as a survived one, so the backup is always kept. + !insertmacro CockLog "post-uninstall restore: $OldInstallDir\data still exists, backup kept" + DetailPrint "Previous user data is still in place; pre-uninstall backup kept at $DataBackupDir\data." ${Else} !insertmacro CockLog "post-uninstall restore: restoring $OldInstallDir\data" CreateDirectory "$OldInstallDir\data" @@ -426,15 +440,56 @@ ${If} ${FileExists} "$DataBackupDir\data" DetailPrint "Restored portable user data from pre-uninstall backup." ${EndIf} ${EndIf} -${If} ${FileExists} "$DataBackupDir\loose_decks\*.cod" -${OrIf} ${FileExists} "$DataBackupDir\loose_decks\*.dec" +${If} $LooseDecksBackupDone == "1" +${AndIf} ${FileExists} "$DataBackupDir\loose_decks" !insertmacro CockLog "post-uninstall restore: restoring loose deck files" CreateDirectory "$OldInstallDir" CopyFiles /SILENT "$DataBackupDir\loose_decks\*.cod" "$OldInstallDir\" CopyFiles /SILENT "$DataBackupDir\loose_decks\*.dec" "$OldInstallDir\" + CopyFiles /SILENT "$DataBackupDir\loose_decks\*.dek" "$OldInstallDir\" + CopyFiles /SILENT "$DataBackupDir\loose_decks\*.mwDeck" "$OldInstallDir\" DetailPrint "Restored deck files stored in the install directory." + ; Runs once per uninstall block, and there are two (32-bit, 64-bit): show the + ; notice a single time per installer run. ${IfNot} ${Silent} - MessageBox MB_OK|MB_ICONINFORMATION "Your decks were stored inside the Cockatrice install folder, which the previous version's uninstaller deletes.$\n$\nThey have been restored from a backup. Please move them out of the install folder (Settings -> General -> Paths) so a future uninstall cannot delete them again." + ${AndIf} $LooseDeckNoticeShown != "1" + StrCpy $LooseDeckNoticeShown "1" + MessageBox MB_OK|MB_ICONINFORMATION "Your decks were stored inside the Cockatrice install folder ($OldInstallDir), which the previous version's uninstaller deletes.$\n$\nThey have been restored from a backup to that folder. If you install to a different folder, they are moved there automatically. Please move them out of the install folder afterwards (Settings -> General -> Paths) so a future uninstall cannot delete them again." + ${EndIf} +${EndIf} +done: +FunctionEnd + +; RelocateRestoredDataToInstDir +; +; componentsPagePre backs up and restores around the old uninstaller before the +; directory page, so $OldInstallDir (registry InstallLocation) is authoritative +; there while $INSTDIR is still the default. When the user then picks a different +; target directory, the restored user data would otherwise stay orphaned in the +; old directory while the new install starts empty. Copy it across when they differ. +Function RelocateRestoredDataToInstDir +StrCmp $DataBackupDir "" done +StrCmp $OldInstallDir "" done +StrCmp $OldInstallDir $INSTDIR done +${If} $DataBackupDone == "1" +${AndIf} ${FileExists} "$OldInstallDir\data\*.*" + !insertmacro CockLog "relocate: moving restored data $OldInstallDir\data -> $INSTDIR\data" + CreateDirectory "$INSTDIR\data" + CopyFiles /SILENT "$OldInstallDir\data\*.*" "$INSTDIR\data\" + DetailPrint "Moved restored user data to the new install folder." +${EndIf} +${If} $LooseDecksBackupDone == "1" + ${If} ${FileExists} "$OldInstallDir\*.cod" + ${OrIf} ${FileExists} "$OldInstallDir\*.dec" + ${OrIf} ${FileExists} "$OldInstallDir\*.dek" + ${OrIf} ${FileExists} "$OldInstallDir\*.mwDeck" + !insertmacro CockLog "relocate: moving restored decks $OldInstallDir -> $INSTDIR" + CreateDirectory "$INSTDIR" + CopyFiles /SILENT "$OldInstallDir\*.cod" "$INSTDIR\" + CopyFiles /SILENT "$OldInstallDir\*.dec" "$INSTDIR\" + CopyFiles /SILENT "$OldInstallDir\*.dek" "$INSTDIR\" + CopyFiles /SILENT "$OldInstallDir\*.mwDeck" "$INSTDIR\" + DetailPrint "Moved restored deck files to the new install folder." ${EndIf} ${EndIf} done: @@ -1301,6 +1356,11 @@ ${EndIf} !insertmacro CockLog "section: writing payload to $INSTDIR" @CPACK_NSIS_FULL_INSTALL@ !insertmacro CockLog "section: payload written" +; The pre-uninstall backup/restore above runs before the directory page, so a +; user picking a different target folder would otherwise leave the restored data +; orphaned in the old directory. Move it across when they differ (no-op on the +; /R path where they are equal). +Call RelocateRestoredDataToInstDir !insertmacro CockLogListing "final" "Qt6*.dll" ${If} $PortableMode = 0 diff --git a/cockatrice/src/interface/window_main.cpp b/cockatrice/src/interface/window_main.cpp index 288951936..e950cde30 100644 --- a/cockatrice/src/interface/window_main.cpp +++ b/cockatrice/src/interface/window_main.cpp @@ -643,13 +643,33 @@ void MainWindow::warnAboutDataPathsInProgramDir() } } // A portable user may also have pointed a custom path inside the install - // dir but outside data/ (e.g. appDir/mydecks). Those offenders would be - // wiped by the old uninstaller but are not in the hardcoded list above. + // dir. Offenders outside data/ (e.g. appDir/mydecks) would be wiped by the + // old uninstaller but are not in the hardcoded list above - and neither + // is a custom folder inside data/ itself (e.g. dataDir/mydecks), which the + // old uninstaller deletes all the same. Cover any offender that is not + // already inside a backed-up source; when an offender is a parent of + // already-collected sources it supersedes them. offenders = paths.pathsInsideDir(appDir); for (const QString &offender : offenders) { - if (!PathsSettings::isInsideDir(offender, dataDir) && !backupSources.contains(offender)) { - backupSources.append(offender); + if (!QFileInfo::exists(offender) || backupSources.contains(offender)) { + continue; } + bool alreadyCovered = false; + for (const QString &existing : backupSources) { + if (PathsSettings::isInsideDir(offender, existing)) { + alreadyCovered = true; + break; + } + } + if (alreadyCovered) { + continue; + } + for (int i = backupSources.size() - 1; i >= 0; --i) { + if (PathsSettings::isInsideDir(backupSources.at(i), offender)) { + backupSources.removeAt(i); + } + } + backupSources.append(offender); } if (backupSources.isEmpty()) { return; @@ -660,13 +680,39 @@ void MainWindow::warnAboutDataPathsInProgramDir() if (offenders.isEmpty()) { return; } - backupSources = offenders; } QString backupRoot = UserDataBackup::defaultBackupRoot(); if (PathsSettings::isInsideDir(backupRoot, appDir)) { backupRoot = QDir::temp().absoluteFilePath(QStringLiteral("CockatriceBackups")); } + // Only existing paths can be copied; a configured-but-never-created path has + // nothing to lose yet and must not turn the warning into a bogus "failed". + QStringList sourcesToBackUp; + if (isPortable) { + sourcesToBackUp = backupSources; + } else { + for (const QString &offender : offenders) { + if (QFileInfo::exists(offender)) { + sourcesToBackUp.append(offender); + } + } + } + // The copy runs synchronously on the UI thread at startup, so cap it: a + // pictures folder pointed into the program dir can be gigabytes, and freezing + // the client to duplicate it is worse than asking for a manual copy. + // Portable sources are small by construction (downloaded pictures and replays + // are excluded above); this guard bites for non-portable offenders. + bool backupSkippedDueToSize = false; + qint64 backupBytes = 0; + if (!sourcesToBackUp.isEmpty()) { + backupBytes = UserDataBackup::estimateBackupSize(sourcesToBackUp); + if (backupBytes >= UserDataBackup::MAX_AUTO_BACKUP_BYTES) { + backupSkippedDueToSize = true; + qCWarning(MainWindowLog) << "At-risk user data backup skipped: estimated" << backupBytes + << "bytes exceeds automatic cap"; + } + } // Throttle: dismissing the dialog without "Don't show again" must not create // a full duplicate on every launch. Reuse a recent session instead, and keep // only a handful of sessions so Documents cannot grow without bound. @@ -676,15 +722,23 @@ void MainWindow::warnAboutDataPathsInProgramDir() bool backupReused = false; const QString recentSession = UserDataBackup::findRecentBackupSession(backupRoot, 24); if (!recentSession.isEmpty()) { - sessionDir = recentSession; - backupLocations = UserDataBackup::listSessionBackupLocations(sessionDir); - backupReused = true; - qCInfo(MainWindowLog) << "Reusing recent at-risk user data backup at" << sessionDir; - } else { + const QStringList reused = UserDataBackup::listSessionBackupLocations(recentSession); + if (!reused.isEmpty()) { + sessionDir = recentSession; + backupLocations = reused; + // A reused session keeps the completeness it was recorded with, so a + // previously partial backup is still presented with the manual-copy + // warning instead of as a success. + backupComplete = UserDataBackup::isBackupSessionComplete(recentSession); + backupReused = true; + qCInfo(MainWindowLog) << "Reusing recent at-risk user data backup at" << sessionDir; + } + } + if (!backupReused && !backupSkippedDueToSize && !sourcesToBackUp.isEmpty()) { // One timestamped session directory for all sources, so a second boundary // never splits a single startup check across two backup folders and // same-named sources cannot overwrite each other. - backupComplete = UserDataBackup::backupAtRiskPaths(backupSources, backupRoot, &backupLocations, &sessionDir); + backupComplete = UserDataBackup::backupAtRiskPaths(sourcesToBackUp, backupRoot, &backupLocations, &sessionDir); UserDataBackup::pruneOldBackupSessions(backupRoot, 5); } for (const QString &backupLocation : backupLocations) { @@ -695,6 +749,9 @@ void MainWindow::warnAboutDataPathsInProgramDir() } const QString backupVerb = backupReused ? tr("is already available") : tr("has been created"); + const QString sizeInfo = tr("%1 MB (automatic backups are capped at %2 MB)") + .arg(QString::number(backupBytes / 1048576.0, 'f', 1), + QString::number(UserDataBackup::MAX_AUTO_BACKUP_BYTES / 1048576)); QString message; if (isPortable) { if (!backupLocations.isEmpty()) { @@ -709,6 +766,14 @@ void MainWindow::warnAboutDataPathsInProgramDir() if (!backupComplete) { message += tr("\n\nThe backup may be incomplete; please back up these folders manually as well."); } + } else if (backupSkippedDueToSize) { + message = tr("This portable install keeps its data inside the program directory, where the " + "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " + "update.\n\nThe automatic safety backup was skipped because of its size: %1. " + "Please back up your data folder manually " + "before uninstalling or updating.\n\nReplays and downloaded pictures would not have been " + "included in that backup either.") + .arg(sizeInfo); } else { message = tr("This portable install keeps its data inside the program directory, where the " "uninstallers shipped in Cockatrice 3.1.0/3.1.1 delete everything on uninstall or " @@ -728,6 +793,13 @@ void MainWindow::warnAboutDataPathsInProgramDir() if (!backupComplete) { message += tr("\n\nThe backup may be incomplete; please back up these folders manually as well."); } + } else if (backupSkippedDueToSize) { + message += tr("\n\nThe automatic safety backup was skipped because of its size: %1. " + "Please back up these folders manually.") + .arg(sizeInfo); + } else if (sourcesToBackUp.isEmpty()) { + message += tr("\n\nNone of these paths exist on disk yet, so there was nothing to back up. " + "Files created there later are still at risk until the paths are moved."); } else { message += tr("\n\nThe automatic safety backup failed; please back up these folders manually."); } diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp index 2de08c63b..d07ec86df 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.cpp @@ -13,6 +13,24 @@ namespace UserDataBackup namespace { +const QString COMPLETE_MARKER = QStringLiteral("backup.complete"); +const QString INCOMPLETE_MARKER = QStringLiteral("backup.incomplete"); + +bool isMarkerFile(const QString &fileName) +{ + return fileName == COMPLETE_MARKER || fileName == INCOMPLETE_MARKER; +} + +void writeCompletionMarker(const QString &sessionDir, bool complete) +{ + // Best effort: a missing marker only means isBackupSessionComplete() falls + // back to the legacy content check below. + QFile marker(QDir(sessionDir).absoluteFilePath(complete ? COMPLETE_MARKER : INCOMPLETE_MARKER)); + if (marker.open(QIODevice::WriteOnly | QIODevice::Text)) { + marker.write(complete ? "complete\n" : "incomplete\n"); + } +} + QString uniqueTargetPath(const QString &sessionDir, const QString &fileName) { QDir dir(sessionDir); @@ -114,6 +132,12 @@ bool backupAtRiskPaths(const QStringList &sourcePaths, } } if (existing.isEmpty()) { + if (backupLocationsOut) { + backupLocationsOut->clear(); + } + if (sessionDirOut) { + sessionDirOut->clear(); + } return false; } @@ -146,10 +170,52 @@ bool backupAtRiskPaths(const QStringList &sourcePaths, } allOk = allOk && ok; } + if (locations.isEmpty()) { + // Nothing salvaged: remove the session again so the empty directory + // neither blocks a retry via findRecentBackupSession() nor evicts a + // good session in pruneOldBackupSessions(). + QDir(sessionDir).removeRecursively(); + if (backupLocationsOut) { + backupLocationsOut->clear(); + } + if (sessionDirOut) { + sessionDirOut->clear(); + } + return false; + } + writeCompletionMarker(sessionDir, allOk); if (backupLocationsOut) { *backupLocationsOut = locations; } - return allOk && !locations.isEmpty(); + return allOk; +} + +qint64 estimateBackupSize(const QStringList &sourcePaths, qint64 capBytes) +{ + qint64 total = 0; + for (const QString &sourcePath : sourcePaths) { + const QFileInfo source(sourcePath); + if (!source.exists()) { + continue; + } + if (source.isDir()) { + QDirIterator it(sourcePath, QDir::Files | QDir::Hidden | QDir::NoDotAndDotDot, + QDirIterator::Subdirectories); + while (it.hasNext()) { + it.next(); + total += it.fileInfo().size(); + if (total >= capBytes) { + return total; + } + } + } else { + total += source.size(); + if (total >= capBytes) { + return total; + } + } + } + return total; } QString defaultBackupRoot() @@ -184,31 +250,60 @@ QStringList listBackupSessions(const QString &backupRoot) QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours) { const QStringList sessions = listBackupSessions(backupRoot); - if (sessions.isEmpty()) { - return {}; - } const QDateTime now = QDateTime::currentDateTime(); for (const QString &session : sessions) { const QFileInfo info(session); - // Newest first; the first recent entry is the reuse candidate. Entries - // are timestamped, so an old newest entry means no recent backup exists. - if (info.exists() && info.lastModified().secsTo(now) < static_cast(maxAgeHours) * 3600) { - return session; + if (!info.exists()) { + continue; } - // Sessions are newest-first, so stop after the newest: older ones are older. - break; + // Sessions are newest-first, so once an entry is too old every later + // one is older as well and the search can stop. + if (info.lastModified().secsTo(now) >= static_cast(maxAgeHours) * 3600) { + break; + } + // Skip empty sessions from failed attempts: they must never block a retry. + if (listSessionBackupLocations(session).isEmpty()) { + continue; + } + return session; } return {}; } +bool isBackupSessionComplete(const QString &sessionDir) +{ + const QDir session(sessionDir); + if (!session.exists()) { + return false; + } + if (QFileInfo::exists(session.absoluteFilePath(COMPLETE_MARKER))) { + return true; + } + if (QFileInfo::exists(session.absoluteFilePath(INCOMPLETE_MARKER))) { + return false; + } + // Legacy sessions from before markers existed: content means the copy + // reported success at the time, so treat them as complete. + return !listSessionBackupLocations(session.absolutePath()).isEmpty(); +} + void pruneOldBackupSessions(const QString &backupRoot, int keepCount) { const QStringList sessions = listBackupSessions(backupRoot); - if (sessions.size() <= keepCount) { - return; - } - for (int i = keepCount; i < sessions.size(); ++i) { - QDir(sessions.at(i)).removeRecursively(); + QStringList kept; + kept.reserve(sessions.size()); + for (const QString &session : sessions) { + // Empty sessions are junk from failed attempts: drop them outright so + // they can never evict a good session from the kept set. + if (listSessionBackupLocations(session).isEmpty()) { + QDir(session).removeRecursively(); + continue; + } + if (kept.size() < keepCount) { + kept.append(session); + continue; + } + QDir(session).removeRecursively(); } } @@ -222,6 +317,9 @@ QStringList listSessionBackupLocations(const QString &sessionDir) QStringList locations; locations.reserve(entries.size()); for (const QString &entry : entries) { + if (isMarkerFile(entry)) { + continue; + } locations.append(session.absoluteFilePath(entry)); } return locations; diff --git a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h index 2b98f8af0..9de7515d3 100644 --- a/libcockatrice_utility/libcockatrice/utility/user_data_backup.h +++ b/libcockatrice_utility/libcockatrice/utility/user_data_backup.h @@ -22,6 +22,9 @@ namespace UserDataBackup { +/** @brief Above this total, the client skips the automatic startup backup and asks for a manual copy. */ +constexpr qint64 MAX_AUTO_BACKUP_BYTES = 500LL * 1024 * 1024; + /** * @brief Recursively copies @p sourceDir into @p destDir, creating it as needed. * @returns true when every file was copied (or there was nothing to copy). @@ -56,14 +59,27 @@ bool backupAtRiskPath(const QString &sourcePath, const QString &backupRoot, QStr * @param backupRoot directory OUTSIDE the install tree that will hold timestamped backups. * @param backupLocationsOut receives one entry per source that produced a backup target * (including partial copies). - * @param sessionDirOut optionally receives the shared session directory. + * @param sessionDirOut optionally receives the shared session directory; set to empty + * when nothing was backed up (all sources missing, or every copy failed outright). * @returns true when every existing source was copied completely. + * A false return with a non-empty @p backupLocationsOut means a partial backup exists. + * When nothing at all could be salvaged the session directory is removed again, so a + * failed attempt never leaves an empty session behind that would block retries. + * Completeness is recorded with a marker file; see isBackupSessionComplete(). */ bool backupAtRiskPaths(const QStringList &sourcePaths, const QString &backupRoot, QStringList *backupLocationsOut, QString *sessionDirOut = nullptr); +/** + * @brief Estimates the total bytes that backing up @p sourcePaths would copy. + * @param capBytes stop counting once the total exceeds this; the returned value is then + * at least @p capBytes (used to skip the scan early on huge trees). + * Missing entries contribute 0. Best effort only. + */ +qint64 estimateBackupSize(const QStringList &sourcePaths, qint64 capBytes = MAX_AUTO_BACKUP_BYTES); + /** * @brief Default backup root guaranteed to live outside the install directory. */ @@ -75,21 +91,34 @@ QString defaultBackupRoot(); QStringList listBackupSessions(const QString &backupRoot); /** - * @brief Returns the newest session directory younger than @p maxAgeHours, or empty. + * @brief Returns the newest non-empty session directory younger than @p maxAgeHours, or empty. * * Used to throttle startup safety backups so dismissing the warning without * "Don't show again" does not create a full duplicate on every launch. + * Empty sessions (failed attempts) are skipped so they never block a retry, + * and an expired newest entry stops the search since sessions are newest-first. */ QString findRecentBackupSession(const QString &backupRoot, int maxAgeHours = 24); +/** + * @brief Reports whether @p sessionDir holds a complete backup. + * @returns true when the session carries the complete marker, or when it predates + * markers but holds backup content (legacy sessions); false for partial backups + * and for empty/missing sessions. + */ +bool isBackupSessionComplete(const QString &sessionDir); + /** * @brief Deletes oldest sessions keeping only the newest @p keepCount ones. + * Empty sessions are junk from failed attempts and are always removed, so they + * can never evict a good session from the kept set. */ void pruneOldBackupSessions(const QString &backupRoot, int keepCount = 5); /** * @brief Lists the backup targets inside an existing session directory. * @returns absolute paths of the entries, for reuse messages without a new copy. + * Completeness marker files are internal bookkeeping and are never listed. */ QStringList listSessionBackupLocations(const QString &sessionDir); diff --git a/tests/user_data_backup_test.cpp b/tests/user_data_backup_test.cpp index bfccad92b..85ccb70ca 100644 --- a/tests/user_data_backup_test.cpp +++ b/tests/user_data_backup_test.cpp @@ -264,10 +264,135 @@ TEST(UserDataBackup, FindRecentReturnsEmptyWhenNone) EXPECT_TRUE(UserDataBackup::listBackupSessions(backupRoot.path()).isEmpty()); EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24).isEmpty()); EXPECT_TRUE(UserDataBackup::listSessionBackupLocations(backupRoot.path() + "/missing").isEmpty()); + EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(backupRoot.path() + "/missing")); // Pruning an empty root must not fail UserDataBackup::pruneOldBackupSessions(backupRoot.path(), 5); } +TEST(UserDataBackup, AllMissingSourcesCreateNoSession) +{ + QTemporaryDir backupRoot; + ASSERT_TRUE(backupRoot.isValid()); + QStringList locations; + QString sessionDir; + EXPECT_FALSE(UserDataBackup::backupAtRiskPaths({backupRoot.path() + "/nope1", backupRoot.path() + "/nope2"}, + backupRoot.path(), &locations, &sessionDir)); + EXPECT_TRUE(locations.isEmpty()); + EXPECT_TRUE(sessionDir.isEmpty()); + // No empty session left behind to block a retry or evict good sessions + EXPECT_TRUE(UserDataBackup::listBackupSessions(backupRoot.path()).isEmpty()); +} + +TEST(UserDataBackup, EmptySessionsAreSkippedAndPruned) +{ + QTemporaryDir backupRoot; + ASSERT_TRUE(backupRoot.isValid()); + QString emptySession; + ASSERT_TRUE(UserDataBackup::createBackupSessionDir(backupRoot.path(), &emptySession)); + // An empty session from a failed attempt must never block a retry + EXPECT_TRUE(UserDataBackup::listSessionBackupLocations(emptySession).isEmpty()); + EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(emptySession)); + EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24).isEmpty()); + UserDataBackup::pruneOldBackupSessions(backupRoot.path(), 5); + EXPECT_FALSE(QDir(emptySession).exists()); +} + +TEST(UserDataBackup, SuccessfulBackupIsMarkedComplete) +{ + QTemporaryDir sourceDir; + QTemporaryDir backupRoot; + ASSERT_TRUE(sourceDir.isValid()); + ASSERT_TRUE(backupRoot.isValid()); + QFile deck(QDir(sourceDir.path()).absoluteFilePath("mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("\n"); + deck.close(); + + QStringList locations; + QString sessionDir; + EXPECT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &sessionDir)); + EXPECT_TRUE(UserDataBackup::isBackupSessionComplete(sessionDir)); + // Marker files are bookkeeping and must not leak into the user-facing locations + const QStringList listed = UserDataBackup::listSessionBackupLocations(sessionDir); + EXPECT_FALSE(listed.isEmpty()); + for (const QString &location : listed) { + EXPECT_FALSE(location.endsWith(QStringLiteral("backup.complete"))); + EXPECT_FALSE(location.endsWith(QStringLiteral("backup.incomplete"))); + } +} + +TEST(UserDataBackup, IncompleteMarkerReportsIncompleteButRemainsReusable) +{ + QTemporaryDir sourceDir; + QTemporaryDir backupRoot; + ASSERT_TRUE(sourceDir.isValid()); + ASSERT_TRUE(backupRoot.isValid()); + QFile deck(QDir(sourceDir.path()).absoluteFilePath("mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("\n"); + deck.close(); + + QStringList locations; + QString sessionDir; + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &sessionDir)); + ASSERT_TRUE(QFile::remove(QDir(sessionDir).absoluteFilePath("backup.complete"))); + QFile marker(QDir(sessionDir).absoluteFilePath("backup.incomplete")); + ASSERT_TRUE(marker.open(QIODevice::WriteOnly | QIODevice::Text)); + marker.write("incomplete\n"); + marker.close(); + + EXPECT_FALSE(UserDataBackup::isBackupSessionComplete(sessionDir)); + // Still non-empty, so throttling reuses it and the caller warns instead of copying again + EXPECT_EQ(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24), sessionDir); +} + +TEST(UserDataBackup, LegacySessionWithoutMarkerCountsAsComplete) +{ + QTemporaryDir backupRoot; + ASSERT_TRUE(backupRoot.isValid()); + QString sessionDir; + ASSERT_TRUE(UserDataBackup::createBackupSessionDir(backupRoot.path(), &sessionDir)); + ASSERT_TRUE(QDir(sessionDir).mkpath("decks")); + QFile deck(QDir(sessionDir).absoluteFilePath("decks/mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("\n"); + deck.close(); + EXPECT_TRUE(UserDataBackup::isBackupSessionComplete(sessionDir)); + EXPECT_EQ(UserDataBackup::findRecentBackupSession(backupRoot.path(), 24), sessionDir); +} + +TEST(UserDataBackup, FindRecentIgnoresExpiredSessions) +{ + QTemporaryDir sourceDir; + QTemporaryDir backupRoot; + ASSERT_TRUE(sourceDir.isValid()); + ASSERT_TRUE(backupRoot.isValid()); + QFile deck(QDir(sourceDir.path()).absoluteFilePath("mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("\n"); + deck.close(); + + QStringList locations; + QString sessionDir; + ASSERT_TRUE(UserDataBackup::backupAtRiskPaths({sourceDir.path()}, backupRoot.path(), &locations, &sessionDir)); + EXPECT_TRUE(UserDataBackup::findRecentBackupSession(backupRoot.path(), 0).isEmpty()); +} + +TEST(UserDataBackup, EstimateBackupSize) +{ + QTemporaryDir sourceDir; + ASSERT_TRUE(sourceDir.isValid()); + const QString missing = QDir(sourceDir.path()).absoluteFilePath("missing"); + EXPECT_EQ(UserDataBackup::estimateBackupSize({missing}), 0); + QFile deck(QDir(sourceDir.path()).absoluteFilePath("mydeck.cod")); + ASSERT_TRUE(deck.open(QIODevice::WriteOnly | QIODevice::Text)); + deck.write("123456"); + deck.close(); + EXPECT_EQ(UserDataBackup::estimateBackupSize({sourceDir.path(), missing}), 6); + // Early-out: once over the cap the exact total no longer matters + EXPECT_GE(UserDataBackup::estimateBackupSize({sourceDir.path()}, 1), 1); +} + } // namespace int main(int argc, char **argv)