From 4fa3e2a7144c532af62520c9da53f423d6f002ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= <peterix@gmail.com>
Date: Mon, 24 Apr 2017 01:30:51 +0200
Subject: GH-1856 Fix metadata version and list loading

Shouldn't crash anymore, shouldn't overwrite data in some bad way anymore either.
---
 api/logic/meta/BaseEntity.cpp  | 11 +++++++++++
 api/logic/meta/BaseEntity.h    | 11 ++---------
 api/logic/meta/Version.cpp     | 10 ++++++++--
 api/logic/meta/Version.h       |  3 ++-
 api/logic/meta/VersionList.cpp | 45 ++++++++++++++++++++----------------------
 5 files changed, 44 insertions(+), 36 deletions(-)

(limited to 'api/logic/meta')

diff --git a/api/logic/meta/BaseEntity.cpp b/api/logic/meta/BaseEntity.cpp
index 633afab2..439256b5 100644
--- a/api/logic/meta/BaseEntity.cpp
+++ b/api/logic/meta/BaseEntity.cpp
@@ -141,6 +141,17 @@ void Meta::BaseEntity::load()
 	m_updateTask->start();
 }
 
+bool Meta::BaseEntity::isLoaded() const
+{
+	return m_loadStatus > LoadStatus::NotLoaded;
+}
+
+bool Meta::BaseEntity::shouldStartRemoteUpdate() const
+{
+	// TODO: version-locks and offline mode?
+	return m_updateStatus != UpdateStatus::InProgress;
+}
+
 shared_qobject_ptr<Task> Meta::BaseEntity::getCurrentTask()
 {
 	if(m_updateStatus == UpdateStatus::InProgress)
diff --git a/api/logic/meta/BaseEntity.h b/api/logic/meta/BaseEntity.h
index 85c25e48..4483beab 100644
--- a/api/logic/meta/BaseEntity.h
+++ b/api/logic/meta/BaseEntity.h
@@ -51,15 +51,8 @@ public:
 	virtual QString localFilename() const = 0;
 	virtual QUrl url() const;
 
-	bool isLoaded() const
-	{
-		return m_loadStatus > LoadStatus::NotLoaded;
-	}
-	bool shouldStartRemoteUpdate() const
-	{
-		// TODO: version-locks and offline mode?
-		return m_updateStatus != UpdateStatus::InProgress;
-	}
+	bool isLoaded() const;
+	bool shouldStartRemoteUpdate() const;
 
 	void load();
 	shared_qobject_ptr<Task> getCurrentTask();
diff --git a/api/logic/meta/Version.cpp b/api/logic/meta/Version.cpp
index 338e180b..b00a29e7 100644
--- a/api/logic/meta/Version.cpp
+++ b/api/logic/meta/Version.cpp
@@ -25,6 +25,10 @@ Meta::Version::Version(const QString &uid, const QString &version)
 {
 }
 
+Meta::Version::~Version()
+{
+}
+
 QString Meta::Version::descriptor()
 {
 	return m_version;
@@ -76,8 +80,10 @@ void Meta::Version::merge(const std::shared_ptr<BaseEntity> &other)
 	{
 		setParentUid(version->m_parentUid);
 	}
-
-	setData(version->m_data);
+	if(version->m_data)
+	{
+		setData(version->m_data);
+	}
 }
 
 QString Meta::Version::localFilename() const
diff --git a/api/logic/meta/Version.h b/api/logic/meta/Version.h
index f132b861..2f92ee9f 100644
--- a/api/logic/meta/Version.h
+++ b/api/logic/meta/Version.h
@@ -38,6 +38,7 @@ class MULTIMC_LOGIC_EXPORT Version : public QObject, public BaseVersion, public
 
 public: /* con/des */
 	explicit Version(const QString &uid, const QString &version);
+	virtual ~Version();
 
 	QString descriptor() override;
 	QString name() override;
@@ -104,7 +105,7 @@ private:
 	QString m_parentUid;
 	QString m_version;
 	QString m_type;
-	qint64 m_time;
+	qint64 m_time = 0;
 	QHash<QString, QString> m_requires;
 	VersionFilePtr m_data;
 };
diff --git a/api/logic/meta/VersionList.cpp b/api/logic/meta/VersionList.cpp
index 0f1404ba..44687d3c 100644
--- a/api/logic/meta/VersionList.cpp
+++ b/api/logic/meta/VersionList.cpp
@@ -182,38 +182,35 @@ void VersionList::merge(const BaseEntity::Ptr &other)
 		setParentUid(list->m_parentUid);
 	}
 
-	if (m_versions.isEmpty())
-	{
-		setVersions(list->m_versions);
-	}
-	else
+	// TODO: do not reset the whole model. maybe?
+	beginResetModel();
+	m_versions.clear();
+	for (const VersionPtr &version : list->m_versions)
 	{
-		for (const VersionPtr &version : list->m_versions)
+		// we already have the version. merge the contents
+		if (m_lookup.contains(version->version()))
+		{
+			m_lookup.value(version->version())->merge(version);
+		}
+		else
+		{
+			m_lookup.insert(version->uid(), version);
+		}
+		// connect it.
+		setupAddedVersion(m_versions.size(), version);
+		m_versions.append(version);
+		if (!m_recommended || (version->type() == "release" && version->rawTime() > m_recommended->rawTime()))
 		{
-			if (m_lookup.contains(version->version()))
-			{
-				m_lookup.value(version->version())->merge(version);
-			}
-			else
-			{
-				beginInsertRows(QModelIndex(), m_versions.size(), m_versions.size());
-				setupAddedVersion(m_versions.size(), version);
-				m_versions.append(version);
-				m_lookup.insert(version->uid(), version);
-				endInsertRows();
-
-				if (!m_recommended || (version->type() == "release" && version->rawTime() > m_recommended->rawTime()))
-				{
-					m_recommended = version;
-					emit dataChanged(index(0), index(m_versions.size() - 1), QVector<int>() << RecommendedRole);
-				}
-			}
+			m_recommended = version;
 		}
 	}
+	endResetModel();
 }
 
 void VersionList::setupAddedVersion(const int row, const VersionPtr &version)
 {
+	// FIXME: do not disconnect from everythin, disconnect only the lambdas here
+	version->disconnect();
 	connect(version.get(), &Version::requiresChanged, this, [this, row]() { emit dataChanged(index(row), index(row), QVector<int>() << RequiresRole); });
 	connect(version.get(), &Version::timeChanged, this, [this, row]() { emit dataChanged(index(row), index(row), QVector<int>() << TimeRole << SortRole); });
 	connect(version.get(), &Version::typeChanged, this, [this, row]() { emit dataChanged(index(row), index(row), QVector<int>() << TypeRole); });
-- 
cgit v1.2.3