diff --git a/cockatrice/src/interface/theme_manager.cpp b/cockatrice/src/interface/theme_manager.cpp index d93dfcf62..08374c8fd 100644 --- a/cockatrice/src/interface/theme_manager.cpp +++ b/cockatrice/src/interface/theme_manager.cpp @@ -23,6 +23,7 @@ #include #define SYSTEM_THEME_NAME "System" +#define LEGACY_SYSTEM_THEME_NAME "Default" #define FUSION_THEME_NAME "Fusion" #define STYLE_CSS_NAME "style.css" #define HANDZONE_BG_NAME "handzone" @@ -122,12 +123,84 @@ ThemeManager::ThemeManager(QObject *parent) : QObject(parent) themeChangedSlot(); } +// Copy every file of sourceDir into targetDir without ever overwriting an +// existing file, recursing into subdirectories (zones/, backgrounds/, ...). +// Returns false as soon as an entry could not be copied, leaving the source +// untouched so the caller can retry later. +static bool mergeDirWithoutOverwrite(const QString &sourceDir, const QString &targetDir) +{ + const QDir target(targetDir); + bool merged = true; + + const QDir::Filters filters = QDir::AllEntries | QDir::NoDotAndDotDot | QDir::Hidden; + for (const QFileInfo &entry : QDir(sourceDir).entryInfoList(filters)) { + const QString targetEntry = target.absoluteFilePath(entry.fileName()); + if (entry.isDir()) { + if (!QDir().mkpath(targetEntry) || !mergeDirWithoutOverwrite(entry.absoluteFilePath(), targetEntry)) { + merged = false; + } + } else if (!target.exists(entry.fileName()) && !QFile::copy(entry.absoluteFilePath(), targetEntry)) { + qCWarning(ThemeManagerLog) << "Could not migrate theme file:" << entry.absoluteFilePath(); + merged = false; + } + } + + return merged; +} + +void ThemeManager::migrateLegacyThemeDir(const QString &themesPath, const QString ¤tThemeName) +{ + const QString legacyPath = QDir(themesPath).absoluteFilePath(LEGACY_SYSTEM_THEME_NAME); + if (!QDir(legacyPath).exists()) { + return; + } + + // Only the renamed theme's own directory is migrated. A user sitting on a + // different theme may well have hand-made a "Default" folder of their own, + // and silently folding it into "System" would be rude. The check accepts + // both the pre-rename name and the post-rename one, since a user who + // already ran the rename keeps their settings on "System" while their + // customised directory is still called "Default". + if (currentThemeName != LEGACY_SYSTEM_THEME_NAME && currentThemeName != SYSTEM_THEME_NAME) { + return; + } + + const QString targetPath = QDir(themesPath).absoluteFilePath(SYSTEM_THEME_NAME); + if (!QDir(targetPath).exists() && QDir().rename(legacyPath, targetPath)) { + qCInfo(ThemeManagerLog) << "Migrated customised theme directory" << LEGACY_SYSTEM_THEME_NAME << "to" + << SYSTEM_THEME_NAME; + return; + } + + // The rename is only possible when the target does not exist yet. When it + // does (a partial earlier upgrade, or a user who re-customised the theme + // after the rename), fall back to merging so nothing is lost: existing + // "System" files win, legacy files only fill gaps. Because every legacy + // file either already exists at the target or is copied there, removing + // the legacy directory afterwards cannot drop user data. + if (!mergeDirWithoutOverwrite(legacyPath, targetPath)) { + qCWarning(ThemeManagerLog) << "Could not fully migrate theme directory" << legacyPath << "; keeping it for a" + << "later retry"; + return; + } + if (!QDir(legacyPath).removeRecursively()) { + qCWarning(ThemeManagerLog) << "Migrated theme directory but could not remove the legacy one:" << legacyPath; + return; + } + qCInfo(ThemeManagerLog) << "Merged customised theme directory" << LEGACY_SYSTEM_THEME_NAME << "into" + << SYSTEM_THEME_NAME; +} + void ThemeManager::ensureThemeDirectoryExists() { auto &settings = SettingsCache::instance(); + // Carry the user's theme customisations (zone graphics, stylesheets, + // palettes, theme.cfg) across before the stored name is rewritten. + migrateLegacyThemeDir(settings.paths().getThemesPath(), settings.getThemeName()); + // Migrate the old "Default" theme name to "System" - if (settings.getThemeName() == "Default") { + if (settings.getThemeName() == LEGACY_SYSTEM_THEME_NAME) { settings.setThemeName(SYSTEM_THEME_NAME); } diff --git a/cockatrice/src/interface/theme_manager.h b/cockatrice/src/interface/theme_manager.h index aadb38ee9..4c9d070dc 100644 --- a/cockatrice/src/interface/theme_manager.h +++ b/cockatrice/src/interface/theme_manager.h @@ -71,6 +71,14 @@ public: // read-only system themes fall back to the user themes directory, creating // it if needed, so customisations never get lost on upgrade. static QString writableThemeDir(const QString &themeName); + // Carries a user profile's customised "Default" theme directory over to the + // "System" directory the theme was renamed to, so zone graphics, + // stylesheets, palettes and theme.cfg survive the rename. Renames the + // directory when "System" does not exist yet, otherwise merges file by + // file without overwriting what "System" already has. No-op unless the + // active theme is one of the two names, so a hand-made "Default" folder of + // a user on another theme is left alone. + static void migrateLegacyThemeDir(const QString &themesPath, const QString ¤tThemeName); // Probe whether a directory is truly writable by trying to create and remove // a temporary file. QFileInfo::isWritable() on a directory is unreliable // (notably on Windows where UAC VirtualStore can make a system dir appear diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1f9cdb9a3..5c12957fb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -178,6 +178,11 @@ add_subdirectory(movecard_tests) add_subdirectory(oracle) add_subdirectory(settings) +# theme_migration_test compiles client sources, whose headers are only reachable from a client build. +if(WITH_CLIENT) + add_subdirectory(theme_migration) +endif() + # picture_loader_benchmark links libcockatrice_settings, which only exists when # a client/UI-capable target is being built. if(WITH_ORACLE OR WITH_CLIENT) diff --git a/tests/settings/settings_migration_test.cpp b/tests/settings/settings_migration_test.cpp index 8a3d756a0..4fa843abf 100644 --- a/tests/settings/settings_migration_test.cpp +++ b/tests/settings/settings_migration_test.cpp @@ -542,6 +542,34 @@ TEST_F(SettingsMigrationTest, PartialMigrationOnlyMigratesPresentKeys) ASSERT_FALSE(fileExists("chat.ini")); } +TEST_F(SettingsMigrationTest, ThemeNameDefaultIsRewrittenToSystem) +{ + { + QSettings g(settingsPath + "global.ini", QSettings::IniFormat); + g.setValue("theme/name", "Default"); + g.sync(); + } + + ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath)); + + ASSERT_TRUE(fileExists("appearance.ini")); + ASSERT_EQ(readFromIni("appearance.ini", "appearance/themeName"), QVariant("System")); +} + +TEST_F(SettingsMigrationTest, OtherThemeNamesAreLeftAlone) +{ + { + QSettings g(settingsPath + "global.ini", QSettings::IniFormat); + g.setValue("theme/name", "VelvetMarble"); + g.sync(); + } + + ASSERT_TRUE(SettingsMigration::migrateSettingsFromGlobalIni(settingsPath)); + + ASSERT_TRUE(fileExists("appearance.ini")); + ASSERT_EQ(readFromIni("appearance.ini", "appearance/themeName"), QVariant("VelvetMarble")); +} + TEST_F(SettingsMigrationTest, KeyMapTranslationIsCorrect) { { diff --git a/tests/theme_migration/CMakeLists.txt b/tests/theme_migration/CMakeLists.txt new file mode 100644 index 000000000..ac202210e --- /dev/null +++ b/tests/theme_migration/CMakeLists.txt @@ -0,0 +1,30 @@ +add_executable( + theme_migration_test + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_manager.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/theme_config.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/pixel_map_generator.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/cache_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/card_counter_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/settings/shortcuts_settings.cpp + ${CMAKE_SOURCE_DIR}/cockatrice/src/client/network/update/client/release_channel.cpp + ${VERSION_STRING_CPP} + theme_migration_test.cpp +) +target_include_directories(theme_migration_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src) + +target_link_libraries( + theme_migration_test + libcockatrice_settings + libcockatrice_network + libcockatrice_network_server_remote + libcockatrice_protocol + Threads::Threads + ${GTEST_BOTH_LIBRARIES} + ${TEST_QT_MODULES} +) + +add_test(NAME theme_migration_test COMMAND theme_migration_test) + +if(NOT GTEST_FOUND) + add_dependencies(theme_migration_test gtest) +endif() diff --git a/tests/theme_migration/theme_migration_test.cpp b/tests/theme_migration/theme_migration_test.cpp new file mode 100644 index 000000000..147833630 --- /dev/null +++ b/tests/theme_migration/theme_migration_test.cpp @@ -0,0 +1,154 @@ +#include "interface/theme_manager.h" + +#include +#include +#include +#include +#include +#include + +// Owned by main.cpp in the real application; pixel_map_generator.cpp references +// it, so the test binary needs a definition even though it never renders. +ThemeManager *themeManager = nullptr; + +namespace +{ +const QString LEGACY = QStringLiteral("Default"); +const QString CURRENT = QStringLiteral("System"); + +// Writes content at //, creating parents. +void writeThemeFile(const QString &themesPath, + const QString &theme, + const QString &relativePath, + const QByteArray &content) +{ + const QString filePath = QDir(QDir(themesPath).absoluteFilePath(theme)).absoluteFilePath(relativePath); + ASSERT_TRUE(QDir().mkpath(QFileInfo(filePath).absolutePath())); + QFile file(filePath); + ASSERT_TRUE(file.open(QIODevice::WriteOnly)); + file.write(content); + file.close(); +} + +QByteArray readThemeFile(const QString &themesPath, const QString &theme, const QString &relativePath) +{ + QFile file(QDir(QDir(themesPath).absoluteFilePath(theme)).absoluteFilePath(relativePath)); + if (!file.open(QIODevice::ReadOnly)) { + return QByteArray(); + } + return file.readAll(); +} + +class ThemeMigrationTest : public ::testing::Test +{ +protected: + QTemporaryDir tmp; + QString themesPath; + + void SetUp() override + { + ASSERT_TRUE(tmp.isValid()); + themesPath = QDir(tmp.path()).absoluteFilePath("themes"); + ASSERT_TRUE(QDir().mkpath(themesPath)); + } +}; + +} // namespace + +TEST_F(ThemeMigrationTest, NoLegacyDirectoryIsANoOp) +{ + writeThemeFile(themesPath, CURRENT, "theme.cfg", "[Appearance]\n"); + + ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT); + + EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(CURRENT)).exists()); + EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists() == false); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "theme.cfg").toStdString(), std::string("[Appearance]\n")); +} + +TEST_F(ThemeMigrationTest, RenamesDirectoryWhenTargetIsFree) +{ + // A customised "Default": stylesheet, zone graphics in a subdirectory and a + // palette, i.e. everything a user is liable to have personalised. + writeThemeFile(themesPath, LEGACY, "style.css", "QPushButton { color: red; }"); + writeThemeFile(themesPath, LEGACY, "zones/handzone.png", "handzone-bytes"); + writeThemeFile(themesPath, LEGACY, "zones/tablezone-dark.svg", "tablezone-bytes"); + writeThemeFile(themesPath, LEGACY, "palette-dark.toml", "palette"); + + ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY); + + EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists()); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), + std::string("QPushButton { color: red; }")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/handzone.png").toStdString(), std::string("handzone-bytes")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/tablezone-dark.svg").toStdString(), + std::string("tablezone-bytes")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "palette-dark.toml").toStdString(), std::string("palette")); +} + +TEST_F(ThemeMigrationTest, RenamesForAnAlreadyMigratedUserWhoseDirectoryWasLeftBehind) +{ + // The settings value was rewritten to "System" on an earlier run, but the + // customised directory is still called "Default". The customisations are + // still reachable and must still be carried over. + writeThemeFile(themesPath, LEGACY, "style.css", "legacy-css"); + + ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT); + + EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists()); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("legacy-css")); +} + +TEST_F(ThemeMigrationTest, MergesIntoAnExistingTargetWithoutOverwriting) +{ + // Post-rename re-customisation: "System" already exists and holds newer + // files. Those win; the legacy directory only fills the gaps. + writeThemeFile(themesPath, CURRENT, "style.css", "newer-css"); + writeThemeFile(themesPath, CURRENT, "zones/handzone.png", "newer-handzone"); + writeThemeFile(themesPath, LEGACY, "style.css", "legacy-css"); + writeThemeFile(themesPath, LEGACY, "zones/handzone.png", "legacy-handzone"); + writeThemeFile(themesPath, LEGACY, "zones/stackzone.png", "legacy-stackzone"); + writeThemeFile(themesPath, LEGACY, "palette-light.toml", "legacy-palette"); + + ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT); + + EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists()); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("newer-css")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/handzone.png").toStdString(), std::string("newer-handzone")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "zones/stackzone.png").toStdString(), std::string("legacy-stackzone")); + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "palette-light.toml").toStdString(), std::string("legacy-palette")); +} + +TEST_F(ThemeMigrationTest, LeavesAnotherThemesDefaultDirectoryAlone) +{ + // A user on an unrelated theme may legitimately have made a "Default" + // folder of their own; it must not be folded into "System". + writeThemeFile(themesPath, LEGACY, "style.css", "unrelated"); + + ThemeManager::migrateLegacyThemeDir(themesPath, QStringLiteral("Leather")); + + EXPECT_TRUE(QDir(QDir(themesPath).absoluteFilePath(LEGACY)).exists()); + EXPECT_EQ(readThemeFile(themesPath, LEGACY, "style.css").toStdString(), std::string("unrelated")); + EXPECT_FALSE(QDir(QDir(themesPath).absoluteFilePath(CURRENT)).exists()); +} + +TEST_F(ThemeMigrationTest, IsIdempotent) +{ + writeThemeFile(themesPath, LEGACY, "style.css", "css"); + + ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY); + ASSERT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("css")); + + ThemeManager::migrateLegacyThemeDir(themesPath, CURRENT); + ThemeManager::migrateLegacyThemeDir(themesPath, LEGACY); + + EXPECT_EQ(readThemeFile(themesPath, CURRENT, "style.css").toStdString(), std::string("css")); +} + +// Every test brings its own main(): GTest is not guaranteed to be found on the build machines, and +// the fallback then links the bare gtest library without gtest_main. +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}