logic: Application settings migration system

* Added version number to ApplicationSettings
* Added SettingsMigrator for defining migration rules for different ApplicationSettings versions
* Migrate ApplicationSettings when loading

bin/app/user/ApplicationSettings_for_package.xml

@@ -1,14 +1,17 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <config>
-	<application>
+    <application>
-		<indexer_thread_count>4</indexer_thread_count>
+        <window_base_height>500</window_base_height>
-		<window_base_width>500</window_base_width>
+        <window_base_width>500</window_base_width>
-		<window_base_height>500</window_base_height>
+    </application>
-	</application>
+    <indexing>
-	<user>
+        <indexer_thread_count>4</indexer_thread_count>
-		<recent_projects>
+    </indexing>
-			<recent_project>./data/projects/tutorial/tutorial.coatiproject</recent_project>
+    <user>
-			<recent_project>./data/projects/tictactoe/tictactoe.coatiproject</recent_project>
+        <recent_projects>
-		</recent_projects>
+            <recent_project>./data/projects/tutorial/tutorial.coatiproject</recent_project>
-	</user>
+            <recent_project>./data/projects/tictactoe/tictactoe.coatiproject</recent_project>
+        </recent_projects>
+    </user>
+    <version>1</version>
 </config>

bin/app/user/ApplicationSettings_template.xml

@@ -7,6 +7,8 @@
 <!-- COLOR: int int int int - rgba e.g. 125 125 125 255 -->
 
 <config>
+	<version><!-- INTEGER: Version number --></version>
+
 	<application>
 		<font_name><!-- STRING: font name --></font_name>
 		<font_size><!-- INTEGER: font size in pt --></font_size>

src/lib/CMakeLists.txt

@@ -207,6 +207,8 @@ add_files(
 	settings/ProjectSettings.h
 	settings/Settings.cpp
 	settings/Settings.h
+	settings/SettingsMigrator.cpp
+	settings/SettingsMigrator.h
 
 	utility/file/FileInfo.cpp
 	utility/file/FileInfo.h

src/lib/settings/ApplicationSettings.cpp

@@ -3,6 +3,8 @@
 #include "utility/ResourcePaths.h"
 #include "utility/utility.h"
 
+const size_t ApplicationSettings::VERSION = 1;
+
 std::shared_ptr<ApplicationSettings> ApplicationSettings::s_instance;
 
 std::shared_ptr<ApplicationSettings> ApplicationSettings::getInstance()
@@ -23,6 +25,36 @@ ApplicationSettings::~ApplicationSettings()
 {
 }
 
+bool ApplicationSettings::load(const FilePath& filePath)
+{
+	bool loaded = Settings::load(filePath);
+	if (!loaded)
+	{
+		return false;
+	}
+
+	SettingsMigrator migrator;
+
+	migrator.addMigration(1,
+		"source/header_search_paths/header_search_path",
+		"indexing/cxx/header_search_paths/header_search_path");
+	migrator.addMigration(1,
+		"source/framework_search_paths/framework_search_path",
+		"indexing/cxx/framework_search_paths/framework_search_path");
+	migrator.addMigration(1,
+		"application/indexer_thread_count",
+		"indexing/indexer_thread_count");
+
+	bool migrated = migrator.migrate(this, ApplicationSettings::VERSION);
+
+	if (migrated)
+	{
+		save();
+	}
+
+	return true;
+}
+
 bool ApplicationSettings::operator==(const ApplicationSettings& other) const
 {
 	return

src/lib/settings/ApplicationSettings.h

@@ -10,9 +10,14 @@ class ApplicationSettings
 {
 public:
 	static std::shared_ptr<ApplicationSettings> getInstance();
+
+	static const size_t VERSION;
+
 	ApplicationSettings();
 	~ApplicationSettings();
 
+	bool load(const FilePath& filePath);
+
 	bool operator==(const ApplicationSettings& other) const;
 
 	int getMaxRecentProjectsCount() const;

src/lib/settings/Settings.cpp

@@ -37,6 +37,7 @@ bool Settings::load(const FilePath& filePath)
 	else
 	{
 		clear();
+		m_filePath = filePath;
 		LOG_WARNING("File for Settings not found: " + filePath.str());
 		return false;
 	}
@@ -44,7 +45,7 @@ bool Settings::load(const FilePath& filePath)
 
 void Settings::save()
 {
-	if (m_config.get() && m_filePath.exists())
+	if (m_config.get() && !m_filePath.empty())
 	{
 		m_config->save(m_filePath.str());
 	}
@@ -58,14 +59,7 @@ void Settings::save(const FilePath& filePath)
 {
 	setFilePath(filePath);
 
-	if (m_config)
+	save();
-	{
-		m_config->save(filePath.str());
-	}
-	else
-	{
-		LOG_WARNING("Settings were not saved: " + filePath.str());
-	}
 }
 
 void Settings::clear()
@@ -79,6 +73,33 @@ const FilePath& Settings::getFilePath() const
 	return m_filePath;
 }
 
+size_t Settings::getVersion() const
+{
+	return getValue<int>("version", 0);
+}
+
+void Settings::setVersion(size_t version)
+{
+	setValue<int>("version", version);
+}
+
+Settings Settings::createFromText(const std::shared_ptr<TextAccess> textAccess)
+{
+	Settings settings;
+	settings.m_config = ConfigManager::createAndLoad(textAccess);
+	return settings;
+}
+
+std::string Settings::getAsText() const
+{
+	if (m_config)
+	{
+		return m_config->toString();
+	}
+
+	return "";
+}
+
 Settings::Settings()
 {
 	clear();
@@ -126,6 +147,11 @@ bool Settings::isValueDefined(const std::string& key) const
 	return m_config->isValueDefined(key);
 }
 
+void Settings::removeValues(const std::string& key)
+{
+	m_config->removeValues(key);
+}
+
 void Settings::enableWarnings() const
 {
 	m_config->setWarnOnEmptyKey(true);

src/lib/settings/Settings.h

@@ -8,6 +8,8 @@
 #include "utility/ConfigManager.h"
 #include "utility/file/FilePath.h"
 
+#include "settings/SettingsMigrator.h"
+
 class Settings
 {
 public:
@@ -15,14 +17,20 @@ public:
 	Settings& operator=(const Settings& other);
 	virtual ~Settings();
 
-	virtual bool load(const FilePath& filePath);
+	bool load(const FilePath& filePath);
-	virtual void save();
+	void save();
-	virtual void save(const FilePath& filePath);
+	void save(const FilePath& filePath);
 
 	void clear();
 
 	virtual const FilePath& getFilePath() const;
 
+	size_t getVersion() const;
+	void setVersion(size_t version);
+
+	static Settings createFromText(const std::shared_ptr<TextAccess> textAccess);
+	std::string getAsText() const;
+
 protected:
 	Settings();
 
@@ -47,9 +55,13 @@ protected:
 
 	bool isValueDefined(const std::string& key) const;
 
+	void removeValues(const std::string& key);
+
 	void enableWarnings() const;
 	void disableWarnings() const;
 
+	friend bool SettingsMigrator::migrate(Settings* settings, size_t targetVersion) const;
+
 private:
 	FilePath m_filePath;
 	std::shared_ptr<ConfigManager> m_config;

src/lib/settings/SettingsMigrator.cpp

@@ -0,0 +1,55 @@
+#include "settings/SettingsMigrator.h"
+
+#include "settings/Settings.h"
+
+SettingsMigrator::SettingsMigrator()
+{
+}
+
+SettingsMigrator::~SettingsMigrator()
+{
+}
+
+void SettingsMigrator::addMigration(size_t targetVersion, std::string oldKey, std::string newKey)
+{
+	Migration migration;
+	migration.targetVersion = targetVersion;
+	migration.oldKey = oldKey;
+	migration.newKey = newKey;
+
+	m_migrations.emplace(targetVersion, migration);
+}
+
+bool SettingsMigrator::migrate(Settings* settings, size_t targetVersion) const
+{
+	size_t originVersion = settings->getVersion();
+
+	if (originVersion < targetVersion)
+	{
+		for (; originVersion <= targetVersion; originVersion++)
+		{
+			std::pair<std::multimap<size_t, Migration>::const_iterator, std::multimap<size_t, Migration>::const_iterator> ret;
+			ret = m_migrations.equal_range(originVersion);
+
+			for (std::multimap<size_t, Migration>::const_iterator it = ret.first; it != ret.second; it++)
+			{
+				const Migration& migration = it->second;
+
+				if (!settings->isValueDefined(migration.newKey))
+				{
+					settings->setValues<std::string>(
+						migration.newKey,
+						settings->getValues<std::string>(migration.oldKey, std::vector<std::string>())
+					);
+					settings->removeValues(migration.oldKey);
+				}
+			}
+		}
+
+		settings->setVersion(targetVersion);
+
+		return true;
+	}
+
+	return false;
+}

src/lib/settings/SettingsMigrator.h

@@ -0,0 +1,32 @@
+#ifndef SETTNGS_MIGRATOR_H
+#define SETTNGS_MIGRATOR_H
+
+#include <map>
+#include <string>
+
+class Settings;
+
+class SettingsMigrator
+{
+public:
+	static SettingsMigrator createApplicationSettingsMigrator();
+
+	SettingsMigrator();
+	virtual ~SettingsMigrator();
+
+	void addMigration(size_t targetVersion, std::string oldKey, std::string newKey);
+
+	bool migrate(Settings* settings, size_t targetVersion) const;
+
+private:
+	struct Migration
+	{
+		size_t targetVersion;
+		std::string oldKey;
+		std::string newKey;
+	};
+
+	std::multimap<size_t, Migration> m_migrations;
+};
+
+#endif // SETTINGS_MIGRATOR_H

src/lib/utility/ConfigManager.cpp

@@ -220,6 +220,11 @@ void ConfigManager::setValues(const std::string& key, const std::vector<bool>& v
 	setValues(key, stringValues);
 }
 
+void ConfigManager::removeValues(const std::string& key)
+{
+	m_values.erase(key);
+}
+
 bool ConfigManager::isValueDefined(const std::string& key) const
 {
 	std::multimap<std::string, std::string>::const_iterator it = m_values.find(key);

src/lib/utility/ConfigManager.h

@@ -38,6 +38,8 @@ public:
 	void setValues(const std::string& key, const std::vector<float>& values);
 	void setValues(const std::string& key, const std::vector<bool>& values);
 
+	void removeValues(const std::string& key);
+
 	bool isValueDefined(const std::string& key) const;
 
 	bool load(const std::shared_ptr<TextAccess> textAccess);

src/test/CMakeLists.txt

@@ -21,6 +21,7 @@ add_files(
 	MessageQueueTestSuite.h
 	NetworkProtocolHelperTestSuite.h
 	SettingsTestSuite.h
+	SettingsMigratorTestSuite.h
 	SearchIndexTestSuite.h
 	SqliteStorageTestSuite.h
 	StorageTestSuite.h

src/test/SettingsMigratorTestSuite.h

@@ -0,0 +1,289 @@
+#include <cxxtest/TestSuite.h>
+
+#include "settings/Settings.h"
+#include "settings/SettingsMigrator.h"
+
+class SettingsMigratorTestSuite : public CxxTest::TestSuite
+{
+public:
+	void test_migrator_changes_nothing_without_migrations_except_version()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_name()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <int>2</int>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "int");
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_path()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <sub>\n"
+			"        <int>2</int>\n"
+			"    </sub>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "sub/int");
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_group_name()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <values>\n"
+			"        <value>2</value>\n"
+			"        <value>3</value>\n"
+			"        <value>4</value>\n"
+			"    </values>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <vals>\n"
+			"        <value>2</value>\n"
+			"        <value>3</value>\n"
+			"        <value>4</value>\n"
+			"    </vals>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "values/value", "vals/value");
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_group_element_name()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <values>\n"
+			"        <value>2</value>\n"
+			"        <value>3</value>\n"
+			"        <value>4</value>\n"
+			"    </values>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <values>\n"
+			"        <val>2</val>\n"
+			"        <val>3</val>\n"
+			"        <val>4</val>\n"
+			"    </values>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "values/value", "values/val");
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_only_up_specified_version()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <int>2</int>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "int");
+		migrator.addMigration(2, "int", "val");
+		migrator.migrate(&settingsBefore, 1);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_only_from_specified_version()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <int>2</int>\n"
+			"    <version>1</version>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <val>2</val>\n"
+			"    <version>2</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "int");
+		migrator.addMigration(2, "int", "val");
+		migrator.migrate(&settingsBefore, 2);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_for_multiple_versions()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <val>2</val>\n"
+			"    <version>2</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "int");
+		migrator.addMigration(2, "int", "val");
+		migrator.migrate(&settingsBefore, 2);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_for_multiple_migrations()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"    <element>hi there</element>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <val>2</val>\n"
+			"    <ele>hi there</ele>\n"
+			"    <version>2</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "val");
+		migrator.addMigration(1, "element", "ele");
+		migrator.migrate(&settingsBefore, 2);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+	void test_migrator_changes_for_multiple_versions_and_migrations()
+	{
+		Settings settingsBefore = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <value>2</value>\n"
+			"    <element>two</element>\n"
+			"    <element>three</element>\n"
+			"    <element>four</element>\n"
+			"</config>\n"
+		);
+
+		Settings settingsAfter = createSettings(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n"
+			"<config>\n"
+			"    <int>2</int>\n"
+			"    <elements>\n"
+			"        <element>two</element>\n"
+			"        <element>three</element>\n"
+			"        <element>four</element>\n"
+			"    </elements>\n"
+			"    <version>3</version>\n"
+			"</config>\n"
+		);
+
+		SettingsMigrator migrator;
+		migrator.addMigration(1, "value", "int/val");
+		migrator.addMigration(1, "element", "ele");
+		migrator.addMigration(2, "int/val", "int");
+		migrator.addMigration(3, "ele", "elements/element");
+		migrator.migrate(&settingsBefore, 3);
+
+		TS_ASSERT_EQUALS(settingsBefore.getAsText(), settingsAfter.getAsText());
+	}
+
+private:
+	Settings createSettings(const std::string& text)
+	{
+		return Settings::createFromText(TextAccess::createFromString(text));
+	}
+};