[Settings] Back up the settings folder before migrating (#7386)

* [Settings] Back up the settings folder before migrating

The per-file settings migration only preserved global.ini, as
global.ini.old. Everything else the migrations write was overwritten in
place with no way back: the legacy migration and the 17 per-domain
migrateXxxSettings functions all write straight into the per-domain .ini
files, so any of them that got a key mapping wrong silently replaced a
working file. The re-migration path is the sharpest case, because
migrateSettingsFromGlobalIni() runs again whenever an older build wrote
non-sentinel keys back into global.ini, and those values then land on
top of per-domain files the user had already edited.

Snapshot the whole settings folder before either migration runs, into a
timestamped sibling settings-backup-<yyyyMMdd-HHmmss>. A sibling rather
than a child, so a backup is never copied into the next backup and the
"Open Settings Folder" action keeps showing only live files. Keep every
snapshot; settings files are kilobytes, so growth is not a concern.

SettingsMigration::isMigrationPending() gates the copy so a normal start
creates nothing. It reuses the same condition migrateSettingsFromGlobalIni()
uses to decide whether it has work, factored into a shared helper so the
two cannot drift, and it ignores the legacy migration on an empty profile
where that migration only writes its sentinel. The backup itself skips a
folder whose files are all empty, which covers a fresh profile whose
global.ini was created but never written.

A failed copy logs a warning and returns an empty path rather than
aborting the migration, since a full disk or an unwritable data folder
must not leave the client unable to start. global.ini.old still works as
the single-file escape hatch.

Document the folder layout, the automatic backups, and the fact that
migrations only run forwards, so users know to copy the settings folder
before downgrading to a version that cannot read the per-group files.

* [Client] Match the desktop file name case for the portal app ID

QGuiApplication::setDesktopFileName() was given "Cockatrice"/"Oracle"
while the installed desktop entries are cockatrice.desktop and
oracle.desktop. Linux paths are case-sensitive, so xdg-desktop-portal
failed the app-info lookup for "Cockatrice" and Qt warned on every
QDesktopServices call that it could not register with the host portal.
The same string becomes the xdg-shell appId, so the window icon lookup
failed on Wayland for the same reason.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/client/settings/cache_settings.cpp

@@ -110,6 +110,16 @@ SettingsCache::SettingsCache()
 
     settings = new QSettings(settingsPath + "global.ini", QSettings::IniFormat, this);
 
+    // Both migrations below overwrite per-domain files in place, so snapshot the whole settings
+    // folder first: global.ini is preserved as global.ini.old, but pre-existing per-domain files
+    // (and a re-migration after a downgrade) would otherwise have no way back.
+    if (SettingsMigration::isMigrationPending(settingsPath)) {
+        const QString backupPath = SettingsMigration::backupSettingsDirectory(settingsPath);
+        if (!backupPath.isEmpty()) {
+            qCInfo(SettingsCacheLog) << "[SettingsCache] Settings backed up before migration:" << backupPath;
+        }
+    }
+
     // Migrate from legacy NativeFormat settings to per-class INI files (runs at most once)
     if (!isPortableBuild) {
         SettingsMigration::migrateLegacySettings(settingsPath);

cockatrice/src/main.cpp

@@ -390,8 +390,11 @@ int main(int argc, char *argv[])
     qCInfo(MainLog) << "MainWindow constructor finished";
 
     ui.setWindowIcon(themePixmap(QStringLiteral("cockatrice")));
-    // Set name of the app desktop file; used by wayland to load the window icon
-    QGuiApplication::setDesktopFileName("Cockatrice");
+    // Base name of the desktop file installed next to the binary, without the ".desktop"
+    // extension. Wayland reads the window icon from it, and xdg-desktop-portal looks up
+    // org.freedesktop.host.portal.Registry by this app ID, so it has to match the file name
+    // exactly, down to the case.
+    QGuiApplication::setDesktopFileName("cockatrice");
 
     SettingsCache::instance().network().setClientID(generateClientID());
 

doc/doxygen/extra-pages/user_documentation/index.md

@@ -11,6 +11,10 @@
 
 - @subpage beta_release
 
+## Settings
+
+- @subpage settings_folder
+
 ## Card Pictures
 
 - @subpage custom_card_pictures

doc/doxygen/extra-pages/user_documentation/settings/settings_folder.md

@@ -0,0 +1,43 @@
+@page settings_folder Settings Folder and Backups
+
+Cockatrice keeps all of its preferences in a settings folder on your computer, one `.ini` file per
+group of settings (appearance, sound, decks, and so on). Open it from the main window via
+Help → Open Settings Folder.
+
+# The Per-File Migration
+
+Older versions of Cockatrice stored every setting in a single `global.ini` file. The settings were
+split into per-group `.ini` files so that unrelated groups no longer share one file. The first time
+you start a version that uses the new layout, your settings are moved into the new files
+automatically and nothing is lost. The old `global.ini` is kept next to the new files as
+`global.ini.old`.
+
+# Automatic Backups
+
+Both migrations overwrite files in the settings folder in place, so before either one runs,
+Cockatrice copies the whole folder to a timestamped backup folder:
+
+```
+<data folder>/settings-backup-20261002-193000/
+```
+
+The backup folder sits next to the settings folder, not inside it. Every backup is kept, so if
+settings ever end up wrong you can compare against, or copy files back from, the earlier snapshot.
+Backups are only created when a migration actually has work to do, so a normal start creates
+nothing.
+
+# Downgrading
+
+Settings migrations only run forwards. If you ever go back to an older version of Cockatrice, that
+version does not read the new per-group files, so it will start with default settings.
+
+Before downgrading, copy your settings folder somewhere safe, for example:
+
+-   Linux: `~/.local/share/Cockatrice/settings/`
+-   macOS: `~/Library/Application Support/Cockatrice/settings/`
+-   Windows: `%LOCALAPPDATA%\Cockatrice\settings\`
+
+Portable installs keep the settings folder in the `data` folder next to the executable.
+
+Copying that folder back after downgrading restores your settings. The automatic backup folder from
+the migration is there as a second copy of the same data if you need it.

libcockatrice_settings/libcockatrice/settings/settings_migration.cpp

@@ -1,12 +1,44 @@
 #include "settings_migration.h"
 
+#include <QDateTime>
+#include <QDir>
+#include <QDirIterator>
 #include <QFile>
+#include <QFileInfo>
+#include <QLoggingCategory>
 #include <QMap>
 #include <QSettings>
 #include <QStringList>
+#include <algorithm>
+
+inline Q_LOGGING_CATEGORY(SettingsMigrationLog, "settings_migration");
 
 static const QString MIGRATION_SENTINEL_KEY = QStringLiteral("migration/perfile_complete");
 
+/**
+ * True when global.ini holds keys that migrateSettingsFromGlobalIni() would still move, i.e. the
+ * exact condition that function uses to decide whether it has work to do. If a user runs an older
+ * Cockatrice build that writes to global.ini (non-sentinel keys) and then upgrades back, those new
+ * keys would otherwise be silently ignored, so they are re-migrated instead.
+ */
+static bool hasPendingGlobalIniSettings(const QString &settingsPath)
+{
+    if (!QFile::exists(settingsPath + "global.ini")) {
+        return false;
+    }
+
+    QSettings globalIni(settingsPath + "global.ini", QSettings::IniFormat);
+    if (!globalIni.value(MIGRATION_SENTINEL_KEY, false).toBool()) {
+        return true;
+    }
+
+    // The sentinel is set, so any key besides it was written after the migration, i.e. by an
+    // older build sharing this file. Those are exactly the keys to re-migrate.
+    QStringList otherKeys = globalIni.allKeys();
+    otherKeys.removeAll(MIGRATION_SENTINEL_KEY);
+    return !otherKeys.isEmpty();
+}
+
 static void migrateTabsSettings(const QString &settingsPath, QSettings &globalIni)
 {
     if (!globalIni.contains("tabs/visualDeckStorage") && !globalIni.contains("tabs/server") &&
@@ -658,21 +690,11 @@ bool SettingsMigration::migrateLegacySettings(const QString &settingsPath)
 
 bool SettingsMigration::migrateSettingsFromGlobalIni(const QString &settingsPath)
 {
-    if (!QFile::exists(settingsPath + "global.ini")) {
+    if (!hasPendingGlobalIniSettings(settingsPath)) {
         return false;
     }
 
     QSettings globalIni(settingsPath + "global.ini", QSettings::IniFormat);
-    if (globalIni.value(MIGRATION_SENTINEL_KEY, false).toBool()) {
-        // If a user runs an older Cockatrice build that writes to global.ini (non-sentinel keys)
-        // and then upgrades back, those new keys will be silently ignored. Re-migrate them.
-        globalIni.sync();
-        auto allKeys = globalIni.allKeys();
-        allKeys.removeAll(MIGRATION_SENTINEL_KEY);
-        if (allKeys.isEmpty()) {
-            return false;
-        }
-    }
 
     migrateTabsSettings(settingsPath, globalIni);
     migrateSoundSettings(settingsPath, globalIni);
@@ -700,3 +722,78 @@ bool SettingsMigration::migrateSettingsFromGlobalIni(const QString &settingsPath
 
     return true;
 }
+
+bool SettingsMigration::isMigrationPending(const QString &settingsPath)
+{
+    if (hasPendingGlobalIniSettings(settingsPath)) {
+        return true;
+    }
+
+    QSettings personalIni(settingsPath + "personal.ini", QSettings::IniFormat);
+    if (personalIni.value("migration/legacy_complete", false).toBool()) {
+        return false;
+    }
+
+    // migrateLegacySettings() also runs on an empty profile, where it only writes its sentinel.
+    // Reporting that as pending would leave a backup folder behind on every fresh install, so
+    // only count it when there really are legacy keys left to move.
+    return !QSettings().allKeys().isEmpty();
+}
+
+QString SettingsMigration::backupSettingsDirectory(const QString &settingsPath)
+{
+    QDir settingsDir(QDir::cleanPath(settingsPath));
+    if (!settingsDir.exists()) {
+        return QString();
+    }
+
+    const QDir::Filters fileFilters = QDir::Files | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot;
+
+    // A profile holding nothing but empty files has nothing to lose, and backing it up would only
+    // leave clutter next to the settings folder.
+    const QFileInfoList files = settingsDir.entryInfoList(fileFilters);
+    const bool hasContent =
+        std::any_of(files.cbegin(), files.cend(), [](const QFileInfo &file) { return file.size() > 0; });
+    if (!hasContent) {
+        return QString();
+    }
+
+    // Keep the backup a sibling of the settings folder rather than a child: a folder that is never
+    // copied into its own backup, and one the "Open settings folder" action does not have to skip.
+    QDir backupParentDir(settingsDir.absolutePath());
+    if (!backupParentDir.cdUp()) {
+        qCWarning(SettingsMigrationLog) << "[SettingsMigration] Could not resolve the parent of the settings folder:"
+                                        << settingsDir.absolutePath();
+        return QString();
+    }
+
+    const QString backupName =
+        QStringLiteral("settings-backup-") + QDateTime::currentDateTime().toString("yyyyMMdd-HHmmss");
+    QString backupPath = backupParentDir.absoluteFilePath(backupName);
+    // Two migrations inside the same second must not merge into a single folder.
+    for (int suffix = 1; QFileInfo::exists(backupPath); ++suffix) {
+        backupPath = backupParentDir.absoluteFilePath(backupName + '-' + QString::number(suffix));
+    }
+
+    if (!QDir().mkpath(backupPath)) {
+        qCWarning(SettingsMigrationLog) << "[SettingsMigration] Could not create the settings backup folder:"
+                                        << backupPath;
+        return QString();
+    }
+
+    QDir backupDir(backupPath);
+    QDirIterator it(settingsDir.absolutePath(), fileFilters, QDirIterator::Subdirectories);
+    while (it.hasNext()) {
+        it.next();
+
+        const QString target = backupDir.absoluteFilePath(settingsDir.relativeFilePath(it.filePath()));
+        QDir().mkpath(QFileInfo(target).absolutePath());
+        if (!QFile::copy(it.filePath(), target)) {
+            // A partially copied backup still beats none: report it and let the migration run.
+            qCWarning(SettingsMigrationLog) << "[SettingsMigration] Could not back up settings file:" << it.filePath();
+        }
+    }
+
+    qCInfo(SettingsMigrationLog) << "[SettingsMigration] Settings backed up to" << backupPath;
+    return backupPath;
+}

libcockatrice_settings/libcockatrice/settings/settings_migration.h

@@ -8,6 +8,19 @@ class SettingsMigration
 public:
     static bool migrateSettingsFromGlobalIni(const QString &settingsPath);
     static bool migrateLegacySettings(const QString &settingsPath);
+
+    /**
+     * Returns true when at least one of the two migrations would still do work, i.e. when a
+     * backup of the settings directory is worth taking before they run.
+     */
+    static bool isMigrationPending(const QString &settingsPath);
+
+    /**
+     * Copies the whole settings directory to a timestamped sibling folder, so a migration that
+     * overwrites per-domain files can be undone by hand. Returns the backup path, or an empty
+     * string when there was nothing to back up or the copy failed. Never aborts the migration.
+     */
+    static QString backupSettingsDirectory(const QString &settingsPath);
 };
 
 #endif // SETTINGS_MIGRATION_H

oracle/src/main.cpp

@@ -82,8 +82,9 @@ int main(int argc, char *argv[])
 
     QIcon icon("theme:appicon.svg");
     wizard.setWindowIcon(icon);
-    // set name of the app desktop file; used by wayland to load the window icon
-    QGuiApplication::setDesktopFileName("Oracle");
+    // Base name of the installed oracle.desktop; wayland reads the window icon from it and
+    // xdg-desktop-portal registers it as the app ID, so the case has to match the file name.
+    QGuiApplication::setDesktopFileName("oracle");
 
     wizard.show();
 

tests/settings/settings_migration_test.cpp

@@ -20,7 +20,11 @@ protected:
 
     void SetUp() override
     {
-        settingsPath = tempDir.path() + "/";
+        // Mirror the production layout: the settings folder lives inside the data folder, so
+        // migration backups (siblings of settings/) stay isolated inside tempDir instead of
+        // landing in the shared system temp directory.
+        settingsPath = tempDir.path() + "/settings/";
+        QDir().mkpath(settingsPath);
 
         // Isolate the settings used by the legacy migration tests inside the temporary
         // directory so the tests never read or write the real user config (registry on
@@ -64,6 +68,25 @@ protected:
         }
         return false;
     }
+
+    // The migration backups are timestamped, so tests address them by index rather than by name.
+    QStringList backupFolderNames() const
+    {
+        return QDir(tempDir.path())
+            .entryList(QDir::Dirs | QDir::NoDotAndDotDot, QDir::Name)
+            .filter(QStringLiteral("settings-backup-"));
+    }
+
+    QVariant readFromBackup(const QString &folderName, const QString &fileName, const QString &key) const
+    {
+        QSettings ini(QDir::cleanPath(tempDir.path() + '/' + folderName + '/' + fileName), QSettings::IniFormat);
+        return ini.value(key);
+    }
+
+    bool backupFileExists(const QString &folderName, const QString &fileName) const
+    {
+        return QFile::exists(QDir::cleanPath(tempDir.path() + '/' + folderName + '/' + fileName));
+    }
 };
 
 TEST_F(SettingsMigrationTest, NoGlobalIniDoesNothing)
@@ -725,6 +748,168 @@ TEST_F(SettingsMigrationTest, RenameFailureWithExistingBackup)
     ASSERT_FALSE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath));
 }
 
+// --- pre-migration backup of the settings folder ---
+
+TEST_F(SettingsMigrationTest, BackupHoldsPreMigrationStateOfPerDomainFiles)
+{
+    // A user already migrated once, changed a setting, then ran an older build that wrote to
+    // global.ini again. Re-migrating clobbers the per-domain file, so the backup has to hold the
+    // pre-migration value.
+    {
+        QSettings tabs(settingsPath + "tabs.ini", QSettings::IniFormat);
+        tabs.setValue("tabs/visualDeckStorage", false);
+        tabs.sync();
+    }
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("tabs/visualDeckStorage", true);
+        g.sync();
+    }
+
+    const QString backupPath = SettingsMigration::backupSettingsDirectory(settingsPath);
+    ASSERT_FALSE(backupPath.isEmpty());
+    ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath));
+
+    // The migration overwrote the user's change ...
+    ASSERT_EQ(readFromIni("tabs.ini", "tabs/visualDeckStorage"), QVariant(true));
+
+    // ... but the backup still holds it, so it can be restored by hand.
+    const QStringList backups = backupFolderNames();
+    ASSERT_EQ(backups.size(), 1);
+    ASSERT_EQ(readFromBackup(backups.at(0), "tabs.ini", "tabs/visualDeckStorage"), QVariant(false));
+    ASSERT_EQ(readFromBackup(backups.at(0), "global.ini", "tabs/visualDeckStorage"), QVariant(true));
+}
+
+TEST_F(SettingsMigrationTest, BackupIsASiblingOfTheSettingsFolder)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("sound/enabled", true);
+        g.sync();
+    }
+
+    const QString backupPath = SettingsMigration::backupSettingsDirectory(settingsPath);
+    ASSERT_FALSE(backupPath.isEmpty());
+
+    const QDir backupDir(backupPath);
+    ASSERT_TRUE(backupDir.dirName().startsWith("settings-backup-"));
+    // Same parent as the settings folder, and not inside it, so a backup is never copied into
+    // another backup and "Open settings folder" shows only live files.
+    ASSERT_EQ(QDir::cleanPath(backupDir.absolutePath() + "/.."), QDir::cleanPath(tempDir.path()));
+    ASSERT_FALSE(backupDir.absolutePath().startsWith(QDir::cleanPath(settingsPath) + '/'));
+    ASSERT_TRUE(backupFileExists(backupDir.dirName(), "global.ini"));
+}
+
+TEST_F(SettingsMigrationTest, BackupCopiesNestedFolders)
+{
+    ASSERT_TRUE(QDir().mkpath(settingsPath + "nested"));
+    {
+        QFile f(settingsPath + "nested/deep.ini");
+        ASSERT_TRUE(f.open(QIODevice::WriteOnly));
+        f.write("[visualDeckStorage]\nfirstDeck=1234\n");
+    }
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("sound/enabled", true);
+        g.sync();
+    }
+
+    const QString backupPath = SettingsMigration::backupSettingsDirectory(settingsPath);
+    ASSERT_FALSE(backupPath.isEmpty());
+
+    const QStringList backups = backupFolderNames();
+    ASSERT_EQ(backups.size(), 1);
+    ASSERT_TRUE(backupFileExists(backups.at(0), "nested/deep.ini"));
+    ASSERT_EQ(readFromBackup(backups.at(0), "nested/deep.ini", "visualDeckStorage/firstDeck"), QVariant("1234"));
+}
+
+TEST_F(SettingsMigrationTest, NoBackupWhenSettingsFolderIsEmpty)
+{
+    ASSERT_TRUE(SettingsMigration::backupSettingsDirectory(settingsPath).isEmpty());
+    ASSERT_TRUE(backupFolderNames().isEmpty());
+}
+
+TEST_F(SettingsMigrationTest, NoBackupWhenEverySettingsFileIsEmpty)
+{
+    // A fresh profile: QSettings has created global.ini, but it holds nothing worth preserving.
+    {
+        QFile f(settingsPath + "global.ini");
+        ASSERT_TRUE(f.open(QIODevice::WriteOnly));
+        f.close();
+    }
+
+    ASSERT_TRUE(SettingsMigration::backupSettingsDirectory(settingsPath).isEmpty());
+    ASSERT_TRUE(backupFolderNames().isEmpty());
+}
+
+TEST_F(SettingsMigrationTest, BackupFailsGracefullyOnMissingSettingsFolder)
+{
+    ASSERT_TRUE(SettingsMigration::backupSettingsDirectory(tempDir.path() + "/does-not-exist/").isEmpty());
+}
+
+TEST_F(SettingsMigrationTest, BackupKeepsEveryPreviousSnapshot)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("sound/enabled", true);
+        g.sync();
+    }
+
+    ASSERT_FALSE(SettingsMigration::backupSettingsDirectory(settingsPath).isEmpty());
+    ASSERT_FALSE(SettingsMigration::backupSettingsDirectory(settingsPath).isEmpty());
+
+    // A second snapshot in the same second must not merge into the first one.
+    const QStringList backups = backupFolderNames();
+    ASSERT_EQ(backups.size(), 2);
+    ASSERT_NE(backups.at(0), backups.at(1));
+}
+
+TEST_F(SettingsMigrationTest, IsMigrationPendingWhenGlobalIniHasKeys)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("sound/enabled", true);
+        g.sync();
+    }
+
+    ASSERT_TRUE(SettingsMigration::isMigrationPending(settingsPath));
+}
+
+TEST_F(SettingsMigrationTest, IsNotMigrationPendingAfterSentinelIsSet)
+{
+    {
+        QSettings g(settingsPath + "global.ini", QSettings::IniFormat);
+        g.setValue("sound/enabled", true);
+        g.sync();
+    }
+    ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath));
+
+    // The sentinel-only global.ini and the legacy sentinel leave nothing to migrate.
+    {
+        QSettings personal(settingsPath + "personal.ini", QSettings::IniFormat);
+        personal.setValue("migration/legacy_complete", true);
+        personal.sync();
+    }
+    ASSERT_FALSE(SettingsMigration::isMigrationPending(settingsPath));
+}
+
+TEST_F(SettingsMigrationTest, IsMigrationPendingWhenOnlyLegacySettingsRemain)
+{
+    QSettings().setValue("server/host", "example.com");
+    QSettings().sync();
+
+    ASSERT_TRUE(SettingsMigration::isMigrationPending(settingsPath));
+}
+
+TEST_F(SettingsMigrationTest, IsNotMigrationPendingWithoutLegacyData)
+{
+    // Nothing to migrate: a fresh profile must not leave a backup folder behind on every start.
+    QSettings().clear();
+    QSettings().sync();
+
+    ASSERT_FALSE(SettingsMigration::isMigrationPending(settingsPath));
+}
+
 } // namespace
 
 int main(int argc, char **argv)