From: Christian Kamm Date: Mon, 14 May 2018 12:37:48 +0000 (+0200) Subject: Settings migration: Preserve future settings where possible X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~583 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=97f7b5abeb08f7c136de9c45b60a610e963ff733;p=nextcloud-desktop.git Settings migration: Preserve future settings where possible See discussion in #6506 --- diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp index ff4375bc7..9071b0d1f 100644 --- a/src/gui/accountmanager.cpp +++ b/src/gui/accountmanager.cpp @@ -55,6 +55,9 @@ AccountManager *AccountManager::instance() bool AccountManager::restore() { + QStringList skipSettingsKeys; + backwardMigrationSettingsKeys(&skipSettingsKeys, &skipSettingsKeys); + auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC)); if (settings->status() != QSettings::NoError || !settings->isWritable()) { qCWarning(lcAccountManager) << "Could not read settings from" << settings->fileName() @@ -62,6 +65,12 @@ bool AccountManager::restore() return false; } + if (skipSettingsKeys.contains(settings->group())) { + // Should not happen: bad container keys should have been deleted + qCWarning(lcAccountManager) << "Accounts structure is too new, ignoring"; + return true; + } + // If there are no accounts, check the old format. if (settings->childGroups().isEmpty() && !settings->contains(QLatin1String(versionC))) { @@ -71,11 +80,16 @@ bool AccountManager::restore() for (const auto &accountId : settings->childGroups()) { settings->beginGroup(accountId); - if (auto acc = loadAccountHelper(*settings)) { - acc->_id = accountId; - if (auto accState = AccountState::loadFromSettings(acc, *settings)) { - addAccountState(accState); + if (!skipSettingsKeys.contains(settings->group())) { + if (auto acc = loadAccountHelper(*settings)) { + acc->_id = accountId; + if (auto accState = AccountState::loadFromSettings(acc, *settings)) { + addAccountState(accState); + } } + } else { + qCInfo(lcAccountManager) << "Account" << accountId << "is too new, ignoring"; + _additionalBlockedAccountIds.insert(accountId); } settings->endGroup(); } @@ -83,25 +97,22 @@ bool AccountManager::restore() return true; } -QStringList AccountManager::backwardMigrationKeys() +void AccountManager::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys) { auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC)); - QStringList badKeys; - const int accountsVersion = settings->value(QLatin1String(versionC)).toInt(); if (accountsVersion <= maxAccountsVersion) { foreach (const auto &accountId, settings->childGroups()) { settings->beginGroup(accountId); const int accountVersion = settings->value(QLatin1String(versionC), 1).toInt(); if (accountVersion > maxAccountVersion) { - badKeys.append(settings->group()); + ignoreKeys->append(settings->group()); } settings->endGroup(); } } else { - badKeys.append(settings->group()); + deleteKeys->append(settings->group()); } - return badKeys; } bool AccountManager::restoreFromLegacySettings() @@ -398,6 +409,9 @@ void AccountManager::shutdown() bool AccountManager::isAccountIdAvailable(const QString &id) const { + if (_additionalBlockedAccountIds.contains(id)) + return false; + return std::none_of(_accounts.cbegin(), _accounts.cend(), [id](const auto &acc) { return acc->account()->id() == id; }); diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h index 2a07eb3c0..d96ad2e4a 100644 --- a/src/gui/accountmanager.h +++ b/src/gui/accountmanager.h @@ -80,7 +80,7 @@ public: * Returns the list of settings keys that can't be read because * they are from the future. */ - static QStringList backwardMigrationKeys(); + static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys); private: // saving and loading Account to settings @@ -97,6 +97,8 @@ private: AccountManager() = default; QList _accounts; + /// Account ids from settings that weren't read + QSet _additionalBlockedAccountIds; public slots: /// Saves account data, not including the credentials diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 306eeb181..b1910be3c 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -102,47 +102,63 @@ namespace { // ---------------------------------------------------------------------------------- -bool Application::configBackwardMigration() +bool Application::configVersionMigration() { - auto accountKeys = AccountManager::backwardMigrationKeys(); - auto folderKeys = FolderMan::backwardMigrationKeys(); + QStringList deleteKeys, ignoreKeys; + AccountManager::backwardMigrationSettingsKeys(&deleteKeys, &ignoreKeys); + FolderMan::backwardMigrationSettingsKeys(&deleteKeys, &ignoreKeys); - bool containsFutureData = !accountKeys.isEmpty() || !folderKeys.isEmpty(); + ConfigFile configFile; - // Deal with unreadable accounts - if (!containsFutureData) + // Did the client version change? + // (The client version is adjusted further down) + bool versionChanged = configFile.clientVersionString() != MIRALL_VERSION_STRING; + + // We want to message the user either for destructive changes, + // or if we're ignoring something and the client version changed. + bool warningMessage = !deleteKeys.isEmpty() || (!ignoreKeys.isEmpty() && versionChanged); + + if (!versionChanged && !warningMessage) return true; - const auto backupFile = ConfigFile().backup(); - - QMessageBox box( - QMessageBox::Warning, - APPLICATION_SHORTNAME, - tr("Some settings were configured in newer versions of this client and " - "use features that are not available in this version.
" - "
" - "Continuing will mean losing these settings.
" - "
" - "The current configuration file was already backed up to %1.") - .arg(backupFile)); - box.addButton(tr("Quit"), QMessageBox::AcceptRole); - auto continueBtn = box.addButton(tr("Continue"), QMessageBox::DestructiveRole); - - box.exec(); - if (box.clickedButton() != continueBtn) { - QTimer::singleShot(0, qApp, SLOT(quit())); - return false; - } + const auto backupFile = configFile.backup(); + + if (warningMessage) { + QString boldMessage; + if (!deleteKeys.isEmpty()) { + boldMessage = tr("Continuing will mean deleting these settings."); + } else { + boldMessage = tr("Continuing will mean ignoring these settings."); + } - auto settings = ConfigFile::settingsWithGroup("foo"); - settings->endGroup(); + QMessageBox box( + QMessageBox::Warning, + APPLICATION_SHORTNAME, + tr("Some settings were configured in newer versions of this client and " + "use features that are not available in this version.
" + "
" + "%1
" + "
" + "The current configuration file was already backed up to %2.") + .arg(boldMessage, backupFile)); + box.addButton(tr("Quit"), QMessageBox::AcceptRole); + auto continueBtn = box.addButton(tr("Continue"), QMessageBox::DestructiveRole); + + box.exec(); + if (box.clickedButton() != continueBtn) { + QTimer::singleShot(0, qApp, SLOT(quit())); + return false; + } - // Wipe the keys from the future - for (const auto &badKey : accountKeys) - settings->remove(badKey); - for (const auto &badKey : folderKeys) - settings->remove(badKey); + auto settings = ConfigFile::settingsWithGroup("foo"); + settings->endGroup(); + + // Wipe confusing keys from the future, ignore the others + for (const auto &badKey : deleteKeys) + settings->remove(badKey); + } + configFile.setClientVersionString(MIRALL_VERSION_STRING); return true; } @@ -231,7 +247,7 @@ Application::Application(int &argc, char **argv) setupLogging(); setupTranslations(); - if (!configBackwardMigration()) { + if (!configVersionMigration()) { return; } diff --git a/src/gui/application.h b/src/gui/application.h index 5a3b6884a..72a9f1f4e 100644 --- a/src/gui/application.h +++ b/src/gui/application.h @@ -107,7 +107,7 @@ private: * Maybe a newer version of the client was used with this config file: * if so, backup, confirm with user and remove the config that can't be read. */ - bool configBackwardMigration(); + bool configVersionMigration(); QPointer _gui; diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 33f201f9c..fd9639241 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -162,6 +162,9 @@ int FolderMan::setupFolders() { unloadAndDeleteAllFolders(); + QStringList skipSettingsKeys; + backwardMigrationSettingsKeys(&skipSettingsKeys, &skipSettingsKeys); + auto settings = ConfigFile::settingsWithGroup(QLatin1String("Accounts")); const auto accountsWithSettings = settings->childGroups(); if (accountsWithSettings.isEmpty()) { @@ -181,19 +184,24 @@ int FolderMan::setupFolders() } settings->beginGroup(id); - settings->beginGroup(QLatin1String("Folders")); - setupFoldersHelper(*settings, account, true); - settings->endGroup(); + // The "backwardsCompatible" flag here is related to migrating old + // database locations + auto process = [&](const QString &groupName, bool backwardsCompatible = false) { + settings->beginGroup(groupName); + if (skipSettingsKeys.contains(settings->group())) { + // Should not happen: bad container keys should have been deleted + qCWarning(lcFolderMan) << "Folder structure" << groupName << "is too new, ignoring"; + } else { + setupFoldersHelper(*settings, account, backwardsCompatible, skipSettingsKeys); + } + settings->endGroup(); + }; - // See Folder::saveToSettings for details about why this exists. - settings->beginGroup(QLatin1String("Multifolders")); - setupFoldersHelper(*settings, account, false); - settings->endGroup(); + process(QStringLiteral("Folders"), true); - // See Folder::saveToSettings for details about why this exists. - settings->beginGroup(QLatin1String("FoldersWithPlaceholders")); - setupFoldersHelper(*settings, account, false); - settings->endGroup(); + // See Folder::saveToSettings for details about why these exists. + process(QStringLiteral("Multifolders")); + process(QStringLiteral("FoldersWithPlaceholders")); settings->endGroup(); // } @@ -203,9 +211,19 @@ int FolderMan::setupFolders() return _folderMap.size(); } -void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible) +void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible, const QStringList &ignoreKeys) { for (const auto &folderAlias : settings.childGroups()) { + // Skip folders with too-new version + settings.beginGroup(folderAlias); + if (ignoreKeys.contains(settings.group())) { + qCInfo(lcFolderMan) << "Folder" << folderAlias << "is too new, ignoring"; + _additionalBlockedFolderAliases.insert(folderAlias); + settings.endGroup(); + continue; + } + settings.endGroup(); + FolderDefinition folderDefinition; if (FolderDefinition::load(settings, folderAlias, &folderDefinition)) { auto defaultJournalPath = folderDefinition.defaultJournalPath(account->account()); @@ -306,9 +324,8 @@ int FolderMan::setupFoldersMigration() return _folderMap.size(); } -QStringList FolderMan::backwardMigrationKeys() +void FolderMan::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys) { - QStringList badKeys; auto settings = ConfigFile::settingsWithGroup(QLatin1String("Accounts")); auto processSubgroup = [&](const QString &name) { @@ -319,12 +336,12 @@ QStringList FolderMan::backwardMigrationKeys() settings->beginGroup(folderAlias); const int folderVersion = settings->value(QLatin1String(versionC), 1).toInt(); if (folderVersion > FolderDefinition::maxSettingsVersion()) { - badKeys.append(settings->group()); + ignoreKeys->append(settings->group()); } settings->endGroup(); } } else { - badKeys.append(settings->group()); + deleteKeys->append(settings->group()); } settings->endGroup(); }; @@ -336,7 +353,6 @@ QStringList FolderMan::backwardMigrationKeys() processSubgroup("FoldersWithPlaceholders"); settings->endGroup(); } - return badKeys; } bool FolderMan::ensureJournalGone(const QString &journalDbFile) @@ -977,7 +993,9 @@ Folder *FolderMan::addFolderInternal(FolderDefinition folderDefinition, { auto alias = folderDefinition.alias; int count = 0; - while (folderDefinition.alias.isEmpty() || _folderMap.contains(folderDefinition.alias)) { + while (folderDefinition.alias.isEmpty() + || _folderMap.contains(folderDefinition.alias) + || _additionalBlockedFolderAliases.contains(folderDefinition.alias)) { // There is already a folder configured with this name and folder names need to be unique folderDefinition.alias = alias + QString::number(++count); } diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 7264e1f82..f9e73199c 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -72,7 +72,7 @@ public: * Returns a list of keys that can't be read because they are from * future versions. */ - static QStringList backwardMigrationKeys(); + static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys); OCC::Folder::Map map(); @@ -305,7 +305,7 @@ private: // restarts the application (Linux only) void restartApplication(); - void setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible); + void setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible, const QStringList &ignoreKeys); QSet _disabledFolders; Folder::Map _folderMap; @@ -314,6 +314,9 @@ private: QPointer _lastSyncFolder; bool _syncEnabled = true; + /// Folder aliases from the settings that weren't read + QSet _additionalBlockedFolderAliases; + /// Starts regular etag query jobs QTimer _etagPollTimer; /// The currently running etag query diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 468d207f3..9f0a39219 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -16,6 +16,7 @@ #include "configfile.h" #include "theme.h" +#include "version.h" #include "common/utility.h" #include "common/asserts.h" #include "version.h" @@ -81,6 +82,7 @@ static const char logDebugC[] = "logDebug"; static const char logExpireC[] = "logExpire"; static const char logFlushC[] = "logFlush"; static const char showExperimentalOptionsC[] = "showExperimentalOptions"; +static const char clientVersionC[] = "clientVersion"; static const char proxyHostC[] = "Proxy/host"; static const char proxyTypeC[] = "Proxy/type"; @@ -414,7 +416,14 @@ QString ConfigFile::excludeFileFromSystem() QString ConfigFile::backup() const { QString baseFile = configFile(); - QString backupFile = QString("%1.backup_%2").arg(baseFile, QDateTime::currentDateTime().toString("yyyyMMdd_HHmmss")); + auto versionString = clientVersionString(); + if (!versionString.isEmpty()) + versionString.prepend('_'); + QString backupFile = + QString("%1.backup_%2%3") + .arg(baseFile) + .arg(QDateTime::currentDateTime().toString("yyyyMMdd_HHmmss")) + .arg(versionString); // If this exact file already exists it's most likely that a backup was // already done. (two backup calls directly after each other, potentially @@ -1018,6 +1027,18 @@ void ConfigFile::setCertificatePasswd(const QString &cPasswd) settings.sync(); } +QString ConfigFile::clientVersionString() const +{ + QSettings settings(configFile(), QSettings::IniFormat); + return settings.value(QLatin1String(clientVersionC), QString()).toString(); +} + +void ConfigFile::setClientVersionString(const QString &version) +{ + QSettings settings(configFile(), QSettings::IniFormat); + settings.setValue(QLatin1String(clientVersionC), version); +} + Q_GLOBAL_STATIC(QString, g_configFileName) std::unique_ptr ConfigFile::settingsWithGroup(const QString &group, QObject *parent) diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h index 66dd2fef4..ac664447b 100644 --- a/src/libsync/configfile.h +++ b/src/libsync/configfile.h @@ -192,6 +192,11 @@ public: QString certificatePasswd() const; void setCertificatePasswd(const QString &cPasswd); + /** The client version that last used this settings file. + Updated by configVersionMigration() at client startup. */ + QString clientVersionString() const; + void setClientVersionString(const QString &version); + /** Returns a new settings pre-set in a specific group. The Settings will be created with the given parent. If no parent is specified, the caller must destroy the settings */ static std::unique_ptr settingsWithGroup(const QString &group, QObject *parent = nullptr);