From 5d6c2ca15916d96c3f94afbd084aead36cd595af Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Wed, 22 Nov 2017 10:45:23 +0100 Subject: [PATCH] SettingsAccessor: Do not change data on invalid upgrade attempts Make sure that the input data is returned unchanged if an invalid update attempt is attempted. Update tests accordingly. Change-Id: If5c410bf1a757f984593bda871763af64d8dd972 Reviewed-by: Marco Bubke --- src/libs/utils/settingsaccessor.cpp | 9 ++---- tests/auto/utils/settings/tst_settings.cpp | 36 ++++------------------ 2 files changed, 8 insertions(+), 37 deletions(-) diff --git a/src/libs/utils/settingsaccessor.cpp b/src/libs/utils/settingsaccessor.cpp index eb9deac61f6..d8ae26e98b3 100644 --- a/src/libs/utils/settingsaccessor.cpp +++ b/src/libs/utils/settingsaccessor.cpp @@ -353,8 +353,7 @@ bool SettingsAccessor::isValidVersionAndId(const int version, const QByteArray & QVariantMap SettingsAccessor::upgradeSettings(const QVariantMap &data) const { const int version = versionFromMap(data); - - if (data.isEmpty()) + if (!isValidVersionAndId(version, settingsIdFromMap(data))) return data; QVariantMap result; @@ -363,11 +362,7 @@ QVariantMap SettingsAccessor::upgradeSettings(const QVariantMap &data) const else result = data; - const int toVersion = currentVersion(); - if (version >= toVersion || version < d->firstVersion()) - return result; - - for (int i = version; i < toVersion; ++i) { + for (int i = version; i < currentVersion(); ++i) { VersionUpgrader *upgrader = d->upgrader(i); QTC_CHECK(upgrader && upgrader->version() == i); result = upgrader->upgrade(result); diff --git a/tests/auto/utils/settings/tst_settings.cpp b/tests/auto/utils/settings/tst_settings.cpp index 68436a70054..93a04219a80 100644 --- a/tests/auto/utils/settings/tst_settings.cpp +++ b/tests/auto/utils/settings/tst_settings.cpp @@ -331,16 +331,8 @@ void tst_SettingsAccessor::upgradeSettings_invalidId() const QVariantMap result = accessor.upgradeSettings(input); - // "OriginalVersion" was added. - for (auto it = result.cbegin(); it != result.cend(); ++it) { - if (it.key() == "OriginalVersion") - QCOMPARE(it.value().toInt(), startVersion); - else if (input.contains(it.key())) // extra settings pass through unchanged! - QCOMPARE(it.value(), input.value(it.key())); - else - QVERIFY2(false, "Unexpected value found in upgraded result!"); - } - QCOMPARE(result.size(), input.size() + 1); // OriginalVersion was added + // Data is unchanged + QCOMPARE(result, input); } void tst_SettingsAccessor::upgradeSettings_tooOld() @@ -351,16 +343,8 @@ void tst_SettingsAccessor::upgradeSettings_tooOld() const QVariantMap result = accessor.upgradeSettings(input); - // "OriginalVersion" was added. - for (auto it = result.cbegin(); it != result.cend(); ++it) { - if (it.key() == "OriginalVersion") - QCOMPARE(it.value().toInt(), startVersion); - else if (input.contains(it.key())) // extra settings pass through unchanged! - QCOMPARE(it.value(), input.value(it.key())); - else - QVERIFY2(false, "Unexpected value found in upgraded result!"); - } - QCOMPARE(result.size(), input.size() + 1); // OriginalVersion was added + // Data is unchanged + QCOMPARE(result, input); } void tst_SettingsAccessor::upgradeSettings_tooNew() @@ -371,16 +355,8 @@ void tst_SettingsAccessor::upgradeSettings_tooNew() const QVariantMap result = accessor.upgradeSettings(input); - // "OriginalVersion" was added. - for (auto it = result.cbegin(); it != result.cend(); ++it) { - if (it.key() == "OriginalVersion") - QCOMPARE(it.value().toInt(), startVersion); - else if (input.contains(it.key())) // extra settings pass through unchanged! - QCOMPARE(it.value(), input.value(it.key())); - else - QVERIFY2(false, "Unexpected value found in upgraded result!"); - } - QCOMPARE(result.size(), input.size() + 1); // OriginalVersion was added + // Data is unchanged + QCOMPARE(result, input); } void tst_SettingsAccessor::upgradeSettings_oneStep()