Skip to content

Commit

Permalink
Improvements to the upgrade routines (#5660)
Browse files Browse the repository at this point in the history
* First commit

This commit starts the improvements on the upgrade methods. We are going to change both the DataFile and ConfigManager to use separate version values from LMMS release versions, so we can easily bump those versions for new upgrade routines. This first commit starts implementing a new version value for the ConfigManager.

* Change code as per requested on review

	As requested, the "configVersion" method was replaced by "legacyConfigVersion" instead, which is only used to return a configuration version if none is present in the configuration file.
	The configuration version of the current build is stored in a local variable called CONFIG_VERSION, making version bumping easier.
	Uses a switch statement instead of if-else to be able to make use of case-cascading.

TODO:
	- Change the CONFIG_VERSION variable to a unsigned int?
	- Start working on the DataFile.cpp.

* Changes the upgrade logic on DataFile.cpp

	Starts refactoring the upgrade logic on DataFile.cpp. Now the "version" attribute is used to indicate which fileVersion we are loading. If the value of version is "1.0", we have a legacy file and use the legacyFileVersion method to retrieve the integer version using the LMMS version. If the value of version is an integer, we just read it. The integer indicates the position in a list of upgrade methods where we should start from and run the upgrade methods. The file version of the build is held in the FILE_VERSION const on DataFile.h. It HAS TO match the number of upgrade methods that we have on our list.
	One of the versions had 2 upgrade routines (upgrade_1_2_0_rc3 and upgrade_1_2_0_rc2_42). They were merged into a single one (upgrade_1_2_0_rc3) because they were both called from a single version check, meaning that they are both part of a single fileVersion.
	Two fatal errors were added (which can later be improved to show an error messagebox): One if the version attribute doesn't exist, and another one if we are using a FILE_VERSION that doesn't match the number of upgrade methods (to avoid mistakes while coding new upgrades later).

	The configVersion variables and methods were changed to use unsigned int instead of int.

TODO:
	- Make the list of upgrade methods static.
	- Add debug messages for each upgrade routine for testing purposes.

* Make method vector a static const variable

	On DataFile.cpp, we now use the vector list of upgrade methods as a static const variable, so it only has to be constructed once.

* Reorganize vector lists

	Reorganize vector lists so they are more easily readable.

	Revert changes on upgrade method names from ConfigManager.cpp.

* Makes the file version bumping automatic

	The file version bumping on DataFile.cpp is now automatic (using the size of the m_upgradeMethods vector as a reference). FILE_VERSION constant was removed, and with it the qFatal error when it doesn't match the size of the methods vector.

* Improve formatting of version and upgrades lists

	Improves the formatting of the vector lists of upgrade routines and LMMS versions (2 upgrade routines per line and 3 LMMS versions per line).
	Adds a qWarning for every upgrade routine for testing purposes, plus a qWarning that tells the current fileVersion/configVersion when upgrade is called.
	Removes extra space characters after the opening bracket of ConfigManager::upgrade_1_1_91.

* Changes ConfigManager to use a vector of methods

	Changes ConfigManager to use a vector of upgrade methods, just like DataFile. The new Config Version can be calculated automatically now, so the CONFIG_VERSION constant was removed.
	Corrects a small comment on Datafile.h.

* Addresses Dom's review requests

	- Changes legacyConfigVersion and legacyFileVersion from const unsigned int to just unsigned int, since the const is irrelevant in this context.
	- Changes the type alias for upgrade methods to start with an uppercase as per the code style guidelines. Moves the aliasing of the type to the class declaration so it can be used on both the method and on the vector list declaration.
	- Changes the vector list names from m_upgradeMethods to UPGRADE_METHODS, so it's more visible they are a static constant.
	- Move the upgradeVersions list from the legacyFileVersion method to the DataFile class, as an static const called UPGRADE_VERSIONS.
	- Uses std::upper_bound instead of std::find_if for the legacyFileVersion method.

* Uses type alias on vector assignment

	Uses the UpgradeMethod type alias when defining the upgrade methods vector from both ConfigManager and DataFile.

* Removes debugging warnings

	Removes the qWarning() calls that were placed for debugging purposes.
  • Loading branch information
IanCaio authored Sep 17, 2020
1 parent 9e3f602 commit 1dab416
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 133 deletions.
12 changes: 11 additions & 1 deletion include/ConfigManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

class LmmsCore;


const QString PROJECTS_PATH = "projects/";
const QString TEMPLATE_PATH = "templates/";
const QString PRESETS_PATH = "presets/";
Expand All @@ -55,6 +54,9 @@ const QString PORTABLE_MODE_FILE = "/portable_mode.txt";
class LMMS_EXPORT ConfigManager : public QObject
{
Q_OBJECT

using UpgradeMethod = void(ConfigManager::*)();

public:
static inline ConfigManager * inst()
{
Expand Down Expand Up @@ -219,6 +221,10 @@ class LMMS_EXPORT ConfigManager : public QObject
return m_version;
}

// Used when the configversion attribute is not present in a configuration file.
// Returns the appropriate config file version based on the LMMS version.
unsigned int legacyConfigVersion();

QString defaultVersion() const;


Expand Down Expand Up @@ -270,6 +276,9 @@ class LMMS_EXPORT ConfigManager : public QObject
void upgrade_1_1_91();
void upgrade();

// List of all upgrade methods
static const std::vector<UpgradeMethod> UPGRADE_METHODS;

QString m_workingDir;
QString m_dataDir;
QString m_vstDir;
Expand All @@ -286,6 +295,7 @@ class LMMS_EXPORT ConfigManager : public QObject
QString m_backgroundPicFile;
QString m_lmmsRcFile;
QString m_version;
unsigned int m_configVersion;
QStringList m_recentlyOpenedProjects;

typedef QVector<QPair<QString, QString> > stringPairVector;
Expand Down
18 changes: 12 additions & 6 deletions include/DataFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@

#include "lmms_export.h"
#include "MemoryManager.h"
#include "ProjectVersion.h"

class QTextStream;

class LMMS_EXPORT DataFile : public QDomDocument
{
MM_OPERATORS

using UpgradeMethod = void(DataFile::*)();

public:
enum Types
{
Expand Down Expand Up @@ -84,6 +88,8 @@ class LMMS_EXPORT DataFile : public QDomDocument
return m_type;
}

unsigned int legacyFileVersion();

private:
static Type type( const QString& typeName );
static QString typeName( Type type );
Expand All @@ -107,9 +113,13 @@ class LMMS_EXPORT DataFile : public QDomDocument
void upgrade_1_1_0();
void upgrade_1_1_91();
void upgrade_1_2_0_rc3();
void upgrade_1_2_0_rc2_42();
void upgrade_1_3_0();

// List of all upgrade methods
static const std::vector<UpgradeMethod> UPGRADE_METHODS;
// List of ProjectVersions for the legacyFileVersion method
static const std::vector<ProjectVersion> UPGRADE_VERSIONS;

void upgrade();

void loadData( const QByteArray & _data, const QString & _sourceFile );
Expand All @@ -125,14 +135,10 @@ class LMMS_EXPORT DataFile : public QDomDocument
QDomElement m_content;
QDomElement m_head;
Type m_type;
unsigned int m_fileVersion;

} ;


const int LDF_MAJOR_VERSION = 1;
const int LDF_MINOR_VERSION = 0;
const QString LDF_VERSION_STRING = QString::number( LDF_MAJOR_VERSION ) + "." + QString::number( LDF_MINOR_VERSION );


#endif

67 changes: 54 additions & 13 deletions src/core/ConfigManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
#include "lmmsversion.h"


// Vector with all the upgrade methods
const std::vector<ConfigManager::UpgradeMethod> ConfigManager::UPGRADE_METHODS = {
&ConfigManager::upgrade_1_1_90 , &ConfigManager::upgrade_1_1_91
};

static inline QString ensureTrailingSlash(const QString & s )
{
if(! s.isEmpty() && !s.endsWith('/') && !s.endsWith('\\'))
Expand All @@ -51,7 +56,9 @@ static inline QString ensureTrailingSlash(const QString & s )
ConfigManager * ConfigManager::s_instanceOfMe = NULL;


ConfigManager::ConfigManager() : m_version(defaultVersion())
ConfigManager::ConfigManager() :
m_version(defaultVersion()),
m_configVersion( UPGRADE_METHODS.size() )
{
if (QFileInfo::exists(qApp->applicationDirPath() + PORTABLE_MODE_FILE))
{
Expand Down Expand Up @@ -114,7 +121,7 @@ void ConfigManager::upgrade_1_1_90()


void ConfigManager::upgrade_1_1_91()
{
{
// rename displaydbv to displaydbfs
if (!value("app", "displaydbv").isNull()) {
setValue("app", "displaydbfs", value("app", "displaydbv"));
Expand All @@ -131,17 +138,15 @@ void ConfigManager::upgrade()
return;
}

ProjectVersion createdWith = m_version;

if (createdWith.setCompareType(ProjectVersion::Build) < "1.1.90")
{
upgrade_1_1_90();
}
// Runs all necessary upgrade methods
std::for_each( UPGRADE_METHODS.begin() + m_configVersion, UPGRADE_METHODS.end(),
[this](UpgradeMethod um)
{
(this->*um)();
}
);

if (createdWith.setCompareType(ProjectVersion::Build) < "1.1.91")
{
upgrade_1_1_91();
}
ProjectVersion createdWith = m_version;

// Don't use old themes as they break the UI (i.e. 0.4 != 1.0, etc)
if (createdWith.setCompareType(ProjectVersion::Minor) != LMMS_VERSION)
Expand All @@ -151,6 +156,7 @@ void ConfigManager::upgrade()

// Bump the version, now that we are upgraded
m_version = LMMS_VERSION;
m_configVersion = UPGRADE_METHODS.size();
}

QString ConfigManager::defaultVersion() const
Expand Down Expand Up @@ -400,11 +406,23 @@ void ConfigManager::loadConfigFile(const QString & configFile)

QDomNode node = root.firstChild();

// Cache the config version for upgrade()
// Cache LMMS version
if (!root.attribute("version").isNull()) {
m_version = root.attribute("version");
}

// Get the version of the configuration file (for upgrade purposes)
if( root.attribute("configversion").isNull() )
{
m_configVersion = legacyConfigVersion(); // No configversion attribute found
}
else
{
bool success;
m_configVersion = root.attribute("configversion").toUInt(&success);
if( !success ) qWarning("Config Version conversion failure.");
}

// create the settings-map out of the DOM
while(!node.isNull())
{
Expand Down Expand Up @@ -565,6 +583,7 @@ void ConfigManager::saveConfigFile()

QDomElement lmms_config = doc.createElement("lmms");
lmms_config.setAttribute("version", m_version);
lmms_config.setAttribute("configversion", m_configVersion);
doc.appendChild(lmms_config);

for(settingsMap::iterator it = m_settings.begin();
Expand Down Expand Up @@ -673,3 +692,25 @@ void ConfigManager::initDevelopmentWorkingDir()
cmakeCache.close();
}
}

// If configversion is not present, we will convert the LMMS version to the appropriate
// configuration file version for backwards compatibility.
unsigned int ConfigManager::legacyConfigVersion()
{
ProjectVersion createdWith = m_version;

createdWith.setCompareType(ProjectVersion::Build);

if( createdWith < "1.1.90" )
{
return 0;
}
else if( createdWith < "1.1.91" )
{
return 1;
}
else
{
return 2;
}
}
Loading

0 comments on commit 1dab416

Please sign in to comment.