Skip to content

Commit 5811c74

Browse files
ckammckamm
committedMay 24, 2018
Settings migration: Preserve future settings where possible
See discussion in #6506
·
v6.0.0-rc.1v2.5.0-alpha
1 parent 57a2881 commit 5811c74

8 files changed

+145
-67
lines changed
 

‎src/gui/accountmanager.cpp

+23-10
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,22 @@ AccountManager *AccountManager::instance()
5252

5353
bool AccountManager::restore()
5454
{
55+
QStringList skipSettingsKeys;
56+
backwardMigrationSettingsKeys(&skipSettingsKeys, &skipSettingsKeys);
57+
5558
auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
5659
if (settings->status() != QSettings::NoError) {
5760
qCWarning(lcAccountManager) << "Could not read settings from" << settings->fileName()
5861
<< settings->status();
5962
return false;
6063
}
6164

65+
if (skipSettingsKeys.contains(settings->group())) {
66+
// Should not happen: bad container keys should have been deleted
67+
qCWarning(lcAccountManager) << "Accounts structure is too new, ignoring";
68+
return true;
69+
}
70+
6271
// If there are no accounts, check the old format.
6372
if (settings->childGroups().isEmpty()
6473
&& !settings->contains(QLatin1String(versionC))) {
@@ -68,37 +77,39 @@ bool AccountManager::restore()
6877

6978
foreach (const auto &accountId, settings->childGroups()) {
7079
settings->beginGroup(accountId);
71-
if (auto acc = loadAccountHelper(*settings)) {
72-
acc->_id = accountId;
73-
if (auto accState = AccountState::loadFromSettings(acc, *settings)) {
74-
addAccountState(accState);
80+
if (!skipSettingsKeys.contains(settings->group())) {
81+
if (auto acc = loadAccountHelper(*settings)) {
82+
acc->_id = accountId;
83+
if (auto accState = AccountState::loadFromSettings(acc, *settings)) {
84+
addAccountState(accState);
85+
}
7586
}
87+
} else {
88+
qCInfo(lcAccountManager) << "Account" << accountId << "is too new, ignoring";
89+
_additionalBlockedAccountIds.insert(accountId);
7690
}
7791
settings->endGroup();
7892
}
7993

8094
return true;
8195
}
8296

83-
QStringList AccountManager::backwardMigrationKeys()
97+
void AccountManager::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys)
8498
{
8599
auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
86-
QStringList badKeys;
87-
88100
const int accountsVersion = settings->value(QLatin1String(versionC)).toInt();
89101
if (accountsVersion <= maxAccountsVersion) {
90102
foreach (const auto &accountId, settings->childGroups()) {
91103
settings->beginGroup(accountId);
92104
const int accountVersion = settings->value(QLatin1String(versionC), 1).toInt();
93105
if (accountVersion > maxAccountVersion) {
94-
badKeys.append(settings->group());
106+
ignoreKeys->append(settings->group());
95107
}
96108
settings->endGroup();
97109
}
98110
} else {
99-
badKeys.append(settings->group());
111+
deleteKeys->append(settings->group());
100112
}
101-
return badKeys;
102113
}
103114

104115
bool AccountManager::restoreFromLegacySettings()
@@ -366,6 +377,8 @@ bool AccountManager::isAccountIdAvailable(const QString &id) const
366377
return false;
367378
}
368379
}
380+
if (_additionalBlockedAccountIds.contains(id))
381+
return false;
369382
return true;
370383
}
371384

‎src/gui/accountmanager.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class AccountManager : public QObject
8181
* Returns the list of settings keys that can't be read because
8282
* they are from the future.
8383
*/
84-
static QStringList backwardMigrationKeys();
84+
static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys);
8585

8686
private:
8787
// saving and loading Account to settings
@@ -111,5 +111,7 @@ public slots:
111111
private:
112112
AccountManager() {}
113113
QList<AccountStatePtr> _accounts;
114+
/// Account ids from settings that weren't read
115+
QSet<QString> _additionalBlockedAccountIds;
114116
};
115117
}

‎src/gui/application.cpp

+50-34
Original file line numberDiff line numberDiff line change
@@ -95,47 +95,63 @@ namespace {
9595

9696
// ----------------------------------------------------------------------------------
9797

98-
bool Application::configBackwardMigration()
98+
bool Application::configVersionMigration()
9999
{
100-
auto accountKeys = AccountManager::backwardMigrationKeys();
101-
auto folderKeys = FolderMan::backwardMigrationKeys();
100+
QStringList deleteKeys, ignoreKeys;
101+
AccountManager::backwardMigrationSettingsKeys(&deleteKeys, &ignoreKeys);
102+
FolderMan::backwardMigrationSettingsKeys(&deleteKeys, &ignoreKeys);
102103

103-
bool containsFutureData = !accountKeys.isEmpty() || !folderKeys.isEmpty();
104+
ConfigFile configFile;
104105

105-
// Deal with unreadable accounts
106-
if (!containsFutureData)
106+
// Did the client version change?
107+
// (The client version is adjusted further down)
108+
bool versionChanged = configFile.clientVersionString() != MIRALL_VERSION_STRING;
109+
110+
// We want to message the user either for destructive changes,
111+
// or if we're ignoring something and the client version changed.
112+
bool warningMessage = !deleteKeys.isEmpty() || (!ignoreKeys.isEmpty() && versionChanged);
113+
114+
if (!versionChanged && !warningMessage)
107115
return true;
108116

109-
const auto backupFile = ConfigFile().backup();
110-
111-
QMessageBox box(
112-
QMessageBox::Warning,
113-
APPLICATION_SHORTNAME,
114-
tr("Some settings were configured in newer versions of this client and "
115-
"use features that are not available in this version.<br>"
116-
"<br>"
117-
"<b>Continuing will mean losing these settings.</b><br>"
118-
"<br>"
119-
"The current configuration file was already backed up to <i>%1</i>.")
120-
.arg(backupFile));
121-
box.addButton(tr("Quit"), QMessageBox::AcceptRole);
122-
auto continueBtn = box.addButton(tr("Continue"), QMessageBox::DestructiveRole);
123-
124-
box.exec();
125-
if (box.clickedButton() != continueBtn) {
126-
QTimer::singleShot(0, qApp, SLOT(quit()));
127-
return false;
128-
}
117+
const auto backupFile = configFile.backup();
118+
119+
if (warningMessage) {
120+
QString boldMessage;
121+
if (!deleteKeys.isEmpty()) {
122+
boldMessage = tr("Continuing will mean <b>deleting these settings</b>.");
123+
} else {
124+
boldMessage = tr("Continuing will mean <b>ignoring these settings</b>.");
125+
}
129126

130-
auto settings = ConfigFile::settingsWithGroup("foo");
131-
settings->endGroup();
127+
QMessageBox box(
128+
QMessageBox::Warning,
129+
APPLICATION_SHORTNAME,
130+
tr("Some settings were configured in newer versions of this client and "
131+
"use features that are not available in this version.<br>"
132+
"<br>"
133+
"%1<br>"
134+
"<br>"
135+
"The current configuration file was already backed up to <i>%2</i>.")
136+
.arg(boldMessage, backupFile));
137+
box.addButton(tr("Quit"), QMessageBox::AcceptRole);
138+
auto continueBtn = box.addButton(tr("Continue"), QMessageBox::DestructiveRole);
139+
140+
box.exec();
141+
if (box.clickedButton() != continueBtn) {
142+
QTimer::singleShot(0, qApp, SLOT(quit()));
143+
return false;
144+
}
132145

133-
// Wipe the keys from the future
134-
for (const auto &badKey : accountKeys)
135-
settings->remove(badKey);
136-
for (const auto &badKey : folderKeys)
137-
settings->remove(badKey);
146+
auto settings = ConfigFile::settingsWithGroup("foo");
147+
settings->endGroup();
148+
149+
// Wipe confusing keys from the future, ignore the others
150+
for (const auto &badKey : deleteKeys)
151+
settings->remove(badKey);
152+
}
138153

154+
configFile.setClientVersionString(MIRALL_VERSION_STRING);
139155
return true;
140156
}
141157

@@ -212,7 +228,7 @@ Application::Application(int &argc, char **argv)
212228
setupLogging();
213229
setupTranslations();
214230

215-
if (!configBackwardMigration()) {
231+
if (!configVersionMigration()) {
216232
return;
217233
}
218234

‎src/gui/application.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected slots:
104104
* Maybe a newer version of the client was used with this config file:
105105
* if so, backup, confirm with user and remove the config that can't be read.
106106
*/
107-
bool configBackwardMigration();
107+
bool configVersionMigration();
108108

109109
QPointer<ownCloudGui> _gui;
110110

‎src/gui/folderman.cpp

+36-18
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ int FolderMan::setupFolders()
165165
{
166166
unloadAndDeleteAllFolders();
167167

168+
QStringList skipSettingsKeys;
169+
backwardMigrationSettingsKeys(&skipSettingsKeys, &skipSettingsKeys);
170+
168171
auto settings = ConfigFile::settingsWithGroup(QLatin1String("Accounts"));
169172
const auto accountsWithSettings = settings->childGroups();
170173
if (accountsWithSettings.isEmpty()) {
@@ -184,19 +187,24 @@ int FolderMan::setupFolders()
184187
}
185188
settings->beginGroup(id);
186189

187-
settings->beginGroup(QLatin1String("Folders"));
188-
setupFoldersHelper(*settings, account, true);
189-
settings->endGroup();
190+
// The "backwardsCompatible" flag here is related to migrating old
191+
// database locations
192+
auto process = [&](const QString &groupName, bool backwardsCompatible = false) {
193+
settings->beginGroup(groupName);
194+
if (skipSettingsKeys.contains(settings->group())) {
195+
// Should not happen: bad container keys should have been deleted
196+
qCWarning(lcFolderMan) << "Folder structure" << groupName << "is too new, ignoring";
197+
} else {
198+
setupFoldersHelper(*settings, account, backwardsCompatible, skipSettingsKeys);
199+
}
200+
settings->endGroup();
201+
};
190202

191-
// See Folder::saveToSettings for details about why this exists.
192-
settings->beginGroup(QLatin1String("Multifolders"));
193-
setupFoldersHelper(*settings, account, false);
194-
settings->endGroup();
203+
process(QStringLiteral("Folders"), true);
195204

196-
// See Folder::saveToSettings for details about why this exists.
197-
settings->beginGroup(QLatin1String("FoldersWithPlaceholders"));
198-
setupFoldersHelper(*settings, account, false);
199-
settings->endGroup();
205+
// See Folder::saveToSettings for details about why these exists.
206+
process(QStringLiteral("Multifolders"));
207+
process(QStringLiteral("FoldersWithPlaceholders"));
200208

201209
settings->endGroup(); // <account>
202210
}
@@ -206,9 +214,19 @@ int FolderMan::setupFolders()
206214
return _folderMap.size();
207215
}
208216

209-
void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible)
217+
void FolderMan::setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible, const QStringList &ignoreKeys)
210218
{
211219
foreach (const auto &folderAlias, settings.childGroups()) {
220+
// Skip folders with too-new version
221+
settings.beginGroup(folderAlias);
222+
if (ignoreKeys.contains(settings.group())) {
223+
qCInfo(lcFolderMan) << "Folder" << folderAlias << "is too new, ignoring";
224+
_additionalBlockedFolderAliases.insert(folderAlias);
225+
settings.endGroup();
226+
continue;
227+
}
228+
settings.endGroup();
229+
212230
FolderDefinition folderDefinition;
213231
if (FolderDefinition::load(settings, folderAlias, &folderDefinition)) {
214232
auto defaultJournalPath = folderDefinition.defaultJournalPath(account->account());
@@ -274,9 +292,8 @@ int FolderMan::setupFoldersMigration()
274292
return _folderMap.size();
275293
}
276294

277-
QStringList FolderMan::backwardMigrationKeys()
295+
void FolderMan::backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys)
278296
{
279-
QStringList badKeys;
280297
auto settings = ConfigFile::settingsWithGroup(QLatin1String("Accounts"));
281298

282299
auto processSubgroup = [&](const QString &name) {
@@ -287,12 +304,12 @@ QStringList FolderMan::backwardMigrationKeys()
287304
settings->beginGroup(folderAlias);
288305
const int folderVersion = settings->value(QLatin1String(versionC), 1).toInt();
289306
if (folderVersion > FolderDefinition::maxSettingsVersion()) {
290-
badKeys.append(settings->group());
307+
ignoreKeys->append(settings->group());
291308
}
292309
settings->endGroup();
293310
}
294311
} else {
295-
badKeys.append(settings->group());
312+
deleteKeys->append(settings->group());
296313
}
297314
settings->endGroup();
298315
};
@@ -304,7 +321,6 @@ QStringList FolderMan::backwardMigrationKeys()
304321
processSubgroup("FoldersWithPlaceholders");
305322
settings->endGroup();
306323
}
307-
return badKeys;
308324
}
309325

310326
bool FolderMan::ensureJournalGone(const QString &journalDbFile)
@@ -946,7 +962,9 @@ Folder *FolderMan::addFolderInternal(FolderDefinition folderDefinition,
946962
{
947963
auto alias = folderDefinition.alias;
948964
int count = 0;
949-
while (folderDefinition.alias.isEmpty() || _folderMap.contains(folderDefinition.alias)) {
965+
while (folderDefinition.alias.isEmpty()
966+
|| _folderMap.contains(folderDefinition.alias)
967+
|| _additionalBlockedFolderAliases.contains(folderDefinition.alias)) {
950968
// There is already a folder configured with this name and folder names need to be unique
951969
folderDefinition.alias = alias + QString::number(++count);
952970
}

‎src/gui/folderman.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class FolderMan : public QObject
7272
* Returns a list of keys that can't be read because they are from
7373
* future versions.
7474
*/
75-
static QStringList backwardMigrationKeys();
75+
static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys);
7676

7777
OCC::Folder::Map map();
7878

@@ -304,7 +304,7 @@ private slots:
304304
// restarts the application (Linux only)
305305
void restartApplication();
306306

307-
void setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible);
307+
void setupFoldersHelper(QSettings &settings, AccountStatePtr account, bool backwardsCompatible, const QStringList &ignoreKeys);
308308

309309
QSet<Folder *> _disabledFolders;
310310
Folder::Map _folderMap;
@@ -313,6 +313,9 @@ private slots:
313313
QPointer<Folder> _lastSyncFolder;
314314
bool _syncEnabled;
315315

316+
/// Folder aliases from the settings that weren't read
317+
QSet<QString> _additionalBlockedFolderAliases;
318+
316319
/// Starts regular etag query jobs
317320
QTimer _etagPollTimer;
318321
/// The currently running etag query

‎src/libsync/configfile.cpp

+22-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "configfile.h"
1818
#include "theme.h"
19+
#include "version.h"
1920
#include "common/utility.h"
2021
#include "common/asserts.h"
2122
#include "version.h"
@@ -68,6 +69,7 @@ static const char maxChunkSizeC[] = "maxChunkSize";
6869
static const char targetChunkUploadDurationC[] = "targetChunkUploadDuration";
6970
static const char automaticLogDirC[] = "logToTemporaryLogDir";
7071
static const char showExperimentalOptionsC[] = "showExperimentalOptions";
72+
static const char clientVersionC[] = "clientVersion";
7173

7274
static const char proxyHostC[] = "Proxy/host";
7375
static const char proxyTypeC[] = "Proxy/type";
@@ -346,7 +348,14 @@ QString ConfigFile::excludeFileFromSystem()
346348
QString ConfigFile::backup() const
347349
{
348350
QString baseFile = configFile();
349-
QString backupFile = QString("%1.backup_%2").arg(baseFile, QDateTime::currentDateTime().toString("yyyyMMdd_HHmmss"));
351+
auto versionString = clientVersionString();
352+
if (!versionString.isEmpty())
353+
versionString.prepend('_');
354+
QString backupFile =
355+
QString("%1.backup_%2%3")
356+
.arg(baseFile)
357+
.arg(QDateTime::currentDateTime().toString("yyyyMMdd_HHmmss"))
358+
.arg(versionString);
350359

351360
// If this exact file already exists it's most likely that a backup was
352361
// already done. (two backup calls directly after each other, potentially
@@ -819,6 +828,18 @@ void ConfigFile::setCertificatePasswd(const QString &cPasswd)
819828
settings.sync();
820829
}
821830

831+
QString ConfigFile::clientVersionString() const
832+
{
833+
QSettings settings(configFile(), QSettings::IniFormat);
834+
return settings.value(QLatin1String(clientVersionC), QString()).toString();
835+
}
836+
837+
void ConfigFile::setClientVersionString(const QString &version)
838+
{
839+
QSettings settings(configFile(), QSettings::IniFormat);
840+
settings.setValue(QLatin1String(clientVersionC), version);
841+
}
842+
822843
Q_GLOBAL_STATIC(QString, g_configFileName)
823844

824845
std::unique_ptr<QSettings> ConfigFile::settingsWithGroup(const QString &group, QObject *parent)

‎src/libsync/configfile.h

+5
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,11 @@ class OWNCLOUDSYNC_EXPORT ConfigFile
170170
QString certificatePasswd() const;
171171
void setCertificatePasswd(const QString &cPasswd);
172172

173+
/** The client version that last used this settings file.
174+
Updated by configVersionMigration() at client startup. */
175+
QString clientVersionString() const;
176+
void setClientVersionString(const QString &version);
177+
173178
/** Returns a new settings pre-set in a specific group. The Settings will be created
174179
with the given parent. If no parent is specified, the caller must destroy the settings */
175180
static std::unique_ptr<QSettings> settingsWithGroup(const QString &group, QObject *parent = 0);

0 commit comments

Comments
 (0)
Please sign in to comment.