GH-1365 rework java version parsing and sorting

9 files changed, 258 insertions, 109 deletions

diff --git a/logic/java/JavaChecker.h b/logic/java/JavaChecker.h
index 3399f07e..650e7ce3 100644
--- a/logic/java/JavaChecker.h
+++ b/logic/java/JavaChecker.h
@@ -5,15 +5,16 @@
 
 #include "multimc_logic_export.h"
 
-class JavaChecker;
+#include "JavaVersion.h"
 
+class JavaChecker;
 
-struct JavaCheckResult
+struct MULTIMC_LOGIC_EXPORT JavaCheckResult
 {
 	QString path;
 	QString mojangPlatform;
 	QString realPlatform;
-	QString javaVersion;
+	JavaVersion javaVersion;
 	QString errorLog;
 	bool valid = false;
 	bool is_64bit = false;
diff --git a/logic/java/JavaInstall.cpp b/logic/java/JavaInstall.cpp
new file mode 100644
index 00000000..bb262b6e
--- /dev/null
+++ b/logic/java/JavaInstall.cpp
@@ -0,0 +1,28 @@
+#include "JavaInstall.h"
+#include <MMCStrings.h>
+
+bool JavaInstall::operator<(const JavaInstall &rhs)
+{
+	auto archCompare = Strings::naturalCompare(arch, rhs.arch, Qt::CaseInsensitive);
+	if(archCompare != 0)
+		return archCompare < 0;
+	if(id < rhs.id)
+	{
+		return true;
+	}
+	if(id > rhs.id)
+	{
+		return false;
+	}
+	return Strings::naturalCompare(path, rhs.path, Qt::CaseInsensitive) < 0;
+}
+
+bool JavaInstall::operator==(const JavaInstall &rhs)
+{
+	return arch == rhs.arch && id == rhs.id && path == rhs.path;
+}
+
+bool JavaInstall::operator>(const JavaInstall &rhs)
+{
+	return (!operator<(rhs)) && (!operator==(rhs));
+}
diff --git a/logic/java/JavaInstall.h b/logic/java/JavaInstall.h
new file mode 100644
index 00000000..882c7386
--- /dev/null
+++ b/logic/java/JavaInstall.h
@@ -0,0 +1,38 @@
+#pragma once
+
+#include "BaseVersion.h"
+#include "JavaVersion.h"
+
+struct JavaInstall : public BaseVersion
+{
+	JavaInstall(){}
+	JavaInstall(QString id, QString arch, QString path)
+	: id(id), arch(arch), path(path)
+	{
+	}
+	virtual QString descriptor()
+	{
+		return id.toString();
+	}
+
+	virtual QString name()
+	{
+		return id.toString();
+	}
+
+	virtual QString typeString() const
+	{
+		return arch;
+	}
+
+	bool operator<(const JavaInstall & rhs);
+	bool operator==(const JavaInstall & rhs);
+	bool operator>(const JavaInstall & rhs);
+
+	JavaVersion id;
+	QString arch;
+	QString path;
+	bool recommended = false;
+};
+
+typedef std::shared_ptr<JavaInstall> JavaInstallPtr;
diff --git a/logic/java/JavaVersionList.cpp b/logic/java/JavaInstallList.cpp
index e3929f45..fbd8ee9b 100644
--- a/logic/java/JavaVersionList.cpp
+++ b/logic/java/JavaInstallList.cpp
@@ -19,37 +19,37 @@
 
 #include <QDebug>
 
-#include "java/JavaVersionList.h"
+#include "java/JavaInstallList.h"
 #include "java/JavaCheckerJob.h"
 #include "java/JavaUtils.h"
 #include "MMCStrings.h"
 #include "minecraft/VersionFilterData.h"
 
-JavaVersionList::JavaVersionList(QObject *parent) : BaseVersionList(parent)
+JavaInstallList::JavaInstallList(QObject *parent) : BaseVersionList(parent)
 {
 }
 
-Task *JavaVersionList::getLoadTask()
+Task *JavaInstallList::getLoadTask()
 {
 	return new JavaListLoadTask(this);
 }
 
-const BaseVersionPtr JavaVersionList::at(int i) const
+const BaseVersionPtr JavaInstallList::at(int i) const
 {
 	return m_vlist.at(i);
 }
 
-bool JavaVersionList::isLoaded()
+bool JavaInstallList::isLoaded()
 {
 	return m_loaded;
 }
 
-int JavaVersionList::count() const
+int JavaInstallList::count() const
 {
 	return m_vlist.count();
 }
 
-QVariant JavaVersionList::data(const QModelIndex &index, int role) const
+QVariant JavaInstallList::data(const QModelIndex &index, int role) const
 {
 	if (!index.isValid())
 		return QVariant();
@@ -57,7 +57,7 @@ QVariant JavaVersionList::data(const QModelIndex &index, int role) const
 	if (index.row() > count())
 		return QVariant();
 
-	auto version = std::dynamic_pointer_cast<JavaVersion>(m_vlist[index.row()]);
+	auto version = std::dynamic_pointer_cast<JavaInstall>(m_vlist[index.row()]);
 	switch (role)
 	{
 		case VersionPointerRole:
@@ -65,7 +65,7 @@ QVariant JavaVersionList::data(const QModelIndex &index, int role) const
 		case VersionIdRole:
 			return version->descriptor();
 		case VersionRole:
-			return version->id;
+			return version->id.toString();
 		case RecommendedRole:
 			return version->recommended;
 		case PathRole:
@@ -77,37 +77,21 @@ QVariant JavaVersionList::data(const QModelIndex &index, int role) const
 	}
 }
 
-BaseVersionList::RoleList JavaVersionList::providesRoles()
+BaseVersionList::RoleList JavaInstallList::providesRoles()
 {
 	return {VersionPointerRole, VersionIdRole, VersionRole, RecommendedRole, PathRole, ArchitectureRole};
 }
 
 
-void JavaVersionList::updateListData(QList<BaseVersionPtr> versions)
+void JavaInstallList::updateListData(QList<BaseVersionPtr> versions)
 {
 	beginResetModel();
 	m_vlist = versions;
 	m_loaded = true;
-	// manual testing fakery
-	/*
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_33", "64", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_44", "64", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_55", "64", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.7.0_44", "64", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.8.0_44", "64", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_33", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_44", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.6.0_55", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.7.0_44", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.8.0_44", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.9.0_1231", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.9.0_1", "32", "/foo/bar/baz"));
-	m_vlist.push_back(std::make_shared<JavaVersion>("1.9.0_1", "64", "/foo/bar/baz"));
-	*/
 	sortVersions();
 	if(m_vlist.size())
 	{
-		auto best = std::dynamic_pointer_cast<JavaVersion>(m_vlist[0]);
+		auto best = std::dynamic_pointer_cast<JavaInstall>(m_vlist[0]);
 		best->recommended = true;
 	}
 	endResetModel();
@@ -115,35 +99,19 @@ void JavaVersionList::updateListData(QList<BaseVersionPtr> versions)
 
 bool sortJavas(BaseVersionPtr left, BaseVersionPtr right)
 {
-	auto rleft = std::dynamic_pointer_cast<JavaVersion>(left);
-	auto rright = std::dynamic_pointer_cast<JavaVersion>(right);
-	// prefer higher arch
-	auto archCompare = Strings::naturalCompare(rleft->arch, rright->arch, Qt::CaseInsensitive);
-	if(archCompare != 0)
-		return archCompare > 0;
-	// dirty hack - 1.9 and above is too new
-	auto labove19 = Strings::naturalCompare(rleft->name(), g_VersionFilterData.discouragedJavaVersion, Qt::CaseInsensitive) >= 0;
-	auto rabove19 = Strings::naturalCompare(rright->name(), g_VersionFilterData.discouragedJavaVersion, Qt::CaseInsensitive) >= 0;
-	if(labove19 == rabove19)
-	{
-		// prefer higher versions in general
-		auto nameCompare = Strings::naturalCompare(rleft->name(), rright->name(), Qt::CaseInsensitive);
-		if(nameCompare != 0)
-			return nameCompare > 0;
-		// if all else is equal, sort by path
-		return Strings::naturalCompare(rleft->path, rright->path, Qt::CaseInsensitive) < 0;
-	}
-	return labove19 < rabove19;
+	auto rleft = std::dynamic_pointer_cast<JavaInstall>(left);
+	auto rright = std::dynamic_pointer_cast<JavaInstall>(right);
+	return (*rleft) > (*rright);
 }
 
-void JavaVersionList::sortVersions()
+void JavaInstallList::sortVersions()
 {
 	beginResetModel();
 	std::sort(m_vlist.begin(), m_vlist.end(), sortJavas);
 	endResetModel();
 }
 
-JavaListLoadTask::JavaListLoadTask(JavaVersionList *vlist) : Task()
+JavaListLoadTask::JavaListLoadTask(JavaInstallList *vlist) : Task()
 {
 	m_list = vlist;
 	m_currentRecommended = NULL;
@@ -183,21 +151,21 @@ void JavaListLoadTask::executeTask()
 
 void JavaListLoadTask::javaCheckerFinished(QList<JavaCheckResult> results)
 {
-	QList<JavaVersionPtr> candidates;
+	QList<JavaInstallPtr> candidates;
 
 	qDebug() << "Found the following valid Java installations:";
 	for(JavaCheckResult result : results)
 	{
 		if(result.valid)
 		{
-			JavaVersionPtr javaVersion(new JavaVersion());
+			JavaInstallPtr javaVersion(new JavaInstall());
 
 			javaVersion->id = result.javaVersion;
 			javaVersion->arch = result.mojangPlatform;
 			javaVersion->path = result.path;
 			candidates.append(javaVersion);
 
-			qDebug() << " " << javaVersion->id << javaVersion->arch << javaVersion->path;
+			qDebug() << " " << javaVersion->id.toString() << javaVersion->arch << javaVersion->path;
 		}
 	}
 
diff --git a/logic/java/JavaVersionList.h b/logic/java/JavaInstallList.h
index 3acb5343..f2ec20f7 100644
--- a/logic/java/JavaVersionList.h
+++ b/logic/java/JavaInstallList.h
@@ -20,47 +20,19 @@
 
 #include "BaseVersionList.h"
 #include "tasks/Task.h"
-#include "java/JavaCheckerJob.h"
+
+#include "JavaCheckerJob.h"
+#include "JavaInstall.h"
 
 #include "multimc_logic_export.h"
 
 class JavaListLoadTask;
 
-struct JavaVersion : public BaseVersion
-{
-	JavaVersion(){}
-	JavaVersion(QString id, QString arch, QString path)
-	: id(id), arch(arch), path(path)
-	{
-	}
-	virtual QString descriptor()
-	{
-		return id;
-	}
-
-	virtual QString name()
-	{
-		return id;
-	}
-
-	virtual QString typeString() const
-	{
-		return arch;
-	}
-
-	QString id;
-	QString arch;
-	QString path;
-	bool recommended = false;
-};
-
-typedef std::shared_ptr<JavaVersion> JavaVersionPtr;
-
-class MULTIMC_LOGIC_EXPORT JavaVersionList : public BaseVersionList
+class MULTIMC_LOGIC_EXPORT JavaInstallList : public BaseVersionList
 {
 	Q_OBJECT
 public:
-	explicit JavaVersionList(QObject *parent = 0);
+	explicit JavaInstallList(QObject *parent = 0);
 
 	virtual Task *getLoadTask() override;
 	virtual bool isLoaded() override;
@@ -85,7 +57,7 @@ class JavaListLoadTask : public Task
 	Q_OBJECT
 
 public:
-	explicit JavaListLoadTask(JavaVersionList *vlist);
+	explicit JavaListLoadTask(JavaInstallList *vlist);
 	~JavaListLoadTask();
 
 	virtual void executeTask();
@@ -94,6 +66,6 @@ public slots:
 
 protected:
 	std::shared_ptr<JavaCheckerJob> m_job;
-	JavaVersionList *m_list;
-	JavaVersion *m_currentRecommended;
+	JavaInstallList *m_list;
+	JavaInstall *m_currentRecommended;
 };
diff --git a/logic/java/JavaUtils.cpp b/logic/java/JavaUtils.cpp
index 7c3cc2a1..9dfb7897 100644
--- a/logic/java/JavaUtils.cpp
+++ b/logic/java/JavaUtils.cpp
@@ -23,15 +23,15 @@
 #include <QDebug>
 #include "java/JavaUtils.h"
 #include "java/JavaCheckerJob.h"
-#include "java/JavaVersionList.h"
+#include "java/JavaInstallList.h"
 
 JavaUtils::JavaUtils()
 {
 }
 
-JavaVersionPtr JavaUtils::MakeJavaPtr(QString path, QString id, QString arch)
+JavaInstallPtr JavaUtils::MakeJavaPtr(QString path, QString id, QString arch)
 {
-	JavaVersionPtr javaVersion(new JavaVersion());
+	JavaInstallPtr javaVersion(new JavaInstall());
 
 	javaVersion->id = id;
 	javaVersion->arch = arch;
@@ -40,9 +40,9 @@ JavaVersionPtr JavaUtils::MakeJavaPtr(QString path, QString id, QString arch)
 	return javaVersion;
 }
 
-JavaVersionPtr JavaUtils::GetDefaultJava()
+JavaInstallPtr JavaUtils::GetDefaultJava()
 {
-	JavaVersionPtr javaVersion(new JavaVersion());
+	JavaInstallPtr javaVersion(new JavaInstall());
 
 	javaVersion->id = "java";
 	javaVersion->arch = "unknown";
@@ -52,9 +52,9 @@ JavaVersionPtr JavaUtils::GetDefaultJava()
 }
 
 #if WINDOWS
-QList<JavaVersionPtr> JavaUtils::FindJavaFromRegistryKey(DWORD keyType, QString keyName)
+QList<JavaInstallPtr> JavaUtils::FindJavaFromRegistryKey(DWORD keyType, QString keyName)
 {
-	QList<JavaVersionPtr> javas;
+	QList<JavaInstallPtr> javas;
 
 	QString archType = "unknown";
 	if (keyType == KEY_WOW64_64KEY)
@@ -114,7 +114,7 @@ QList<JavaVersionPtr> JavaUtils::FindJavaFromRegistryKey(DWORD keyType, QString
 											&valueSz);
 
 							// Now, we construct the version object and add it to the list.
-							JavaVersionPtr javaVersion(new JavaVersion());
+							JavaInstallPtr javaVersion(new JavaInstall());
 
 							javaVersion->id = subKeyName;
 							javaVersion->arch = archType;
@@ -137,15 +137,15 @@ QList<JavaVersionPtr> JavaUtils::FindJavaFromRegistryKey(DWORD keyType, QString
 
 QList<QString> JavaUtils::FindJavaPaths()
 {
-	QList<JavaVersionPtr> java_candidates;
+	QList<JavaInstallPtr> java_candidates;
 
-	QList<JavaVersionPtr> JRE64s = this->FindJavaFromRegistryKey(
+	QList<JavaInstallPtr> JRE64s = this->FindJavaFromRegistryKey(
 		KEY_WOW64_64KEY, "SOFTWARE\\JavaSoft\\Java Runtime Environment");
-	QList<JavaVersionPtr> JDK64s = this->FindJavaFromRegistryKey(
+	QList<JavaInstallPtr> JDK64s = this->FindJavaFromRegistryKey(
 		KEY_WOW64_64KEY, "SOFTWARE\\JavaSoft\\Java Development Kit");
-	QList<JavaVersionPtr> JRE32s = this->FindJavaFromRegistryKey(
+	QList<JavaInstallPtr> JRE32s = this->FindJavaFromRegistryKey(
 		KEY_WOW64_32KEY, "SOFTWARE\\JavaSoft\\Java Runtime Environment");
-	QList<JavaVersionPtr> JDK32s = this->FindJavaFromRegistryKey(
+	QList<JavaInstallPtr> JDK32s = this->FindJavaFromRegistryKey(
 		KEY_WOW64_32KEY, "SOFTWARE\\JavaSoft\\Java Development Kit");
 
 	java_candidates.append(JRE64s);
@@ -159,7 +159,7 @@ QList<QString> JavaUtils::FindJavaPaths()
 	java_candidates.append(MakeJavaPtr(this->GetDefaultJava()->path));
 
 	QList<QString> candidates;
-	for(JavaVersionPtr java_candidate : java_candidates)
+	for(JavaInstallPtr java_candidate : java_candidates)
 	{
 		if(!candidates.contains(java_candidate->path))
 		{
diff --git a/logic/java/JavaUtils.h b/logic/java/JavaUtils.h
index 78693b37..b671d0a5 100644
--- a/logic/java/JavaUtils.h
+++ b/logic/java/JavaUtils.h
@@ -20,7 +20,7 @@
 
 #include "JavaCheckerJob.h"
 #include "JavaChecker.h"
-#include "JavaVersionList.h"
+#include "JavaInstallList.h"
 
 #ifdef Q_OS_WIN
 #include <windows.h>
@@ -34,11 +34,11 @@ class MULTIMC_LOGIC_EXPORT JavaUtils : public QObject
 public:
 	JavaUtils();
 
-	JavaVersionPtr MakeJavaPtr(QString path, QString id = "unknown", QString arch = "unknown");
+	JavaInstallPtr MakeJavaPtr(QString path, QString id = "unknown", QString arch = "unknown");
 	QList<QString> FindJavaPaths();
-	JavaVersionPtr GetDefaultJava();
+	JavaInstallPtr GetDefaultJava();
 
 #ifdef Q_OS_WIN
-	QList<JavaVersionPtr> FindJavaFromRegistryKey(DWORD keyType, QString keyName);
+	QList<JavaInstallPtr> FindJavaFromRegistryKey(DWORD keyType, QString keyName);
 #endif
 };
diff --git a/logic/java/JavaVersion.cpp b/logic/java/JavaVersion.cpp
new file mode 100644
index 00000000..84fc48a4
--- /dev/null
+++ b/logic/java/JavaVersion.cpp
@@ -0,0 +1,112 @@
+#include "JavaVersion.h"
+#include <MMCStrings.h>
+
+#include <QRegularExpression>
+#include <QString>
+
+JavaVersion & JavaVersion::operator=(const QString & javaVersionString)
+{
+	string = javaVersionString;
+
+	auto getCapturedInteger = [](const QRegularExpressionMatch & match, const QString &what) -> int
+	{
+		auto str = match.captured(what);
+		if(str.isEmpty())
+		{
+			return 0;
+		}
+		return str.toInt();
+	};
+
+	QRegularExpression pattern;
+	if(javaVersionString.startsWith("1."))
+	{
+		pattern = QRegularExpression ("1[.](?<major>[0-9]+)([.](?<minor>[0-9]+))?(_(?<security>[0-9]+)?)?(-(?<prerelease>[a-zA-Z0-9]+))?");
+	}
+	else
+	{
+		pattern = QRegularExpression("(?<major>[0-9]+)([.](?<minor>[0-9]+))?([.](?<security>[0-9]+))?(-(?<prerelease>[a-zA-Z0-9]+))?");
+	}
+
+	auto match = pattern.match(string);
+	parseable = match.hasMatch();
+	major = getCapturedInteger(match, "major");
+	minor = getCapturedInteger(match, "minor");
+	security = getCapturedInteger(match, "security");
+	prerelease = match.captured("prerelease");
+	return *this;
+}
+
+JavaVersion::JavaVersion(const QString &rhs)
+{
+	operator=(rhs);
+}
+
+QString JavaVersion::toString()
+{
+	return string;
+}
+
+bool JavaVersion::requiresPermGen()
+{
+	if(parseable)
+	{
+		return major < 8;
+	}
+	return true;
+}
+
+bool JavaVersion::operator<(const JavaVersion &rhs)
+{
+	if(parseable && rhs.parseable)
+	{
+		if(major < rhs.major)
+			return true;
+		if(major > rhs.major)
+			return false;
+		if(minor < rhs.minor)
+			return true;
+		if(minor > rhs.minor)
+			return false;
+		if(security < rhs.security)
+			return true;
+		if(security > rhs.security)
+			return false;
+
+		// everything else being equal, consider prerelease status
+		bool thisPre = !prerelease.isEmpty();
+		bool rhsPre = !rhs.prerelease.isEmpty();
+		if(thisPre && !rhsPre)
+		{
+			// this is a prerelease and the other one isn't -> lesser
+			return true;
+		}
+		else if(!thisPre && rhsPre)
+		{
+			// this isn't a prerelease and the other one is -> greater
+			return false;
+		}
+		else if(thisPre && rhsPre)
+		{
+			// both are prereleases - use natural compare...
+			return Strings::naturalCompare(prerelease, rhs.prerelease, Qt::CaseSensitive) < 0;
+		}
+		// neither is prerelease, so they are the same -> this cannot be less than rhs
+		return false;
+	}
+	else return Strings::naturalCompare(string, rhs.string, Qt::CaseSensitive) < 0;
+}
+
+bool JavaVersion::operator==(const JavaVersion &rhs)
+{
+	if(parseable && rhs.parseable)
+	{
+		return major == rhs.major && minor == rhs.minor && security == rhs.security && prerelease == rhs.prerelease;
+	}
+	return string == rhs.string;
+}
+
+bool JavaVersion::operator>(const JavaVersion &rhs)
+{
+	return (!operator<(rhs)) && (!operator==(rhs));
+}
diff --git a/logic/java/JavaVersion.h b/logic/java/JavaVersion.h
new file mode 100644
index 00000000..f9a733d3
--- /dev/null
+++ b/logic/java/JavaVersion.h
@@ -0,0 +1,30 @@
+#pragma once
+
+#include "multimc_logic_export.h"
+#include <QString>
+
+class MULTIMC_LOGIC_EXPORT JavaVersion
+{
+	friend class JavaVersionTest;
+public:
+	JavaVersion() {};
+	JavaVersion(const QString & rhs);
+
+	JavaVersion & operator=(const QString & rhs);
+
+	bool operator<(const JavaVersion & rhs);
+	bool operator==(const JavaVersion & rhs);
+	bool operator>(const JavaVersion & rhs);
+
+	bool requiresPermGen();
+
+	QString toString();
+
+private:
+	QString string;
+	int major = 0;
+	int minor = 0;
+	int security = 0;
+	bool parseable = false;
+	QString prerelease;
+};