refactor+feat: improve code separation in ensure metadata

... and avoid calculating the same hash multiple times

Signed-off-by: flow <flowlnlnln@gmail.com>

launcher/modplatform/EnsureMetadataTask.cpp

@@ -3,89 +3,72 @@
 #include <MurmurHash2.h>
 #include <QDebug>
 
-#include "FileSystem.h"
 #include "Json.h"
+
 #include "minecraft/mod/Mod.h"
 #include "minecraft/mod/tasks/LocalModUpdateTask.h"
+
 #include "modplatform/flame/FlameAPI.h"
 #include "modplatform/flame/FlameModIndex.h"
 #include "modplatform/modrinth/ModrinthAPI.h"
 #include "modplatform/modrinth/ModrinthPackIndex.h"
+
 #include "net/NetJob.h"
-#include "tasks/MultipleOptionsTask.h"
 
 static ModPlatform::ProviderCapabilities ProviderCaps;
 
 static ModrinthAPI modrinth_api;
 static FlameAPI flame_api;
 
-EnsureMetadataTask::EnsureMetadataTask(Mod* mod, QDir dir, ModPlatform::Provider prov) : Task(nullptr), m_index_dir(dir), m_provider(prov)
+EnsureMetadataTask::EnsureMetadataTask(Mod* mod, QDir dir, ModPlatform::Provider prov)
+    : Task(nullptr), m_index_dir(dir), m_provider(prov), m_hashing_task(nullptr), m_current_task(nullptr)
 {
-    auto hash = getHash(mod);
+    auto hash_task = createNewHash(mod);
-    if (hash.isEmpty())
+    connect(hash_task.get(), &Task::succeeded, [this, hash_task, mod] { m_mods.insert(hash_task->getResult(), mod); });
-        emitFail(mod);
+    connect(hash_task.get(), &Task::failed, [this, hash_task, mod] { emitFail(mod, RemoveFromList::No); });
-    else
+    hash_task->start();
-        m_mods.insert(hash, mod);
 }
 
 EnsureMetadataTask::EnsureMetadataTask(QList<Mod*>& mods, QDir dir, ModPlatform::Provider prov)
-    : Task(nullptr), m_index_dir(dir), m_provider(prov)
+    : Task(nullptr), m_index_dir(dir), m_provider(prov), m_current_task(nullptr)
 {
+    m_hashing_task = new ConcurrentTask(this, "MakeHashesTask", 10);
     for (auto* mod : mods) {
-        if (!mod->valid()) {
+        auto hash_task = createNewHash(mod);
-            emitFail(mod);
+        if (!hash_task)
             continue;
-        }
+        connect(hash_task.get(), &Task::succeeded, [this, hash_task, mod] { m_mods.insert(hash_task->getResult(), mod); });
-
+        connect(hash_task.get(), &Task::failed, [this, hash_task, mod] { emitFail(mod, RemoveFromList::No); });
-        auto hash = getHash(mod);
+        m_hashing_task->addTask(hash_task);
-        if (hash.isEmpty()) {
-            emitFail(mod);
-            continue;
-        }
-
-        m_mods.insert(hash, mod);
     }
 }
 
-QString EnsureMetadataTask::getHash(Mod* mod)
+Hashing::Hasher::Ptr EnsureMetadataTask::createNewHash(Mod* mod)
 {
-    /* Here we create a mapping hash -> mod, because we need that relationship to parse the API routes */
+    if (!mod->valid() || mod->type() == Mod::MOD_FOLDER)
-    if (mod->type() == Mod::MOD_FOLDER)
+        return nullptr;
-        return {};
 
-    QString result;
+    return Hashing::createHasher(mod->fileinfo().absoluteFilePath(), m_provider);
-    switch (m_provider) {
-        case ModPlatform::Provider::MODRINTH: {
-            QFile file(mod->fileinfo().absoluteFilePath());
-
-            try {
-                file.open(QFile::ReadOnly);
-            } catch (FS::FileSystemException& e) {
-                qCritical() << QString("Failed to open JAR file of %1").arg(mod->name());
-                qCritical() << QString("Reason: ") << e.cause();
-
-                return {};
 }
 
-            auto hash_type = ProviderCaps.hashType(ModPlatform::Provider::MODRINTH).first();
+QString EnsureMetadataTask::getExistingHash(Mod* mod)
-            result = ProviderCaps.hash(ModPlatform::Provider::MODRINTH, &file, hash_type);
+{
-
+    // Check for already computed hashes
-            file.close();
+    // (linear on the number of mods vs. linear on the size of the mod's JAR)
-
+    auto it = m_mods.keyValueBegin();
+    while (it != m_mods.keyValueEnd()) {
+        if ((*it).second == mod)
             break;
-        }
+        it++;
-        case ModPlatform::Provider::FLAME: {
-            auto should_filter_out = [](char c) {
-                // CF-specific
-                return (c == 9 || c == 10 || c == 13 || c == 32);
-            };
-
-            std::ifstream file_stream(mod->fileinfo().absoluteFilePath().toStdString(), std::ifstream::binary);
-            result = QString::number(MurmurHash2(std::move(file_stream), 4096, should_filter_out));
-        }
     }
 
-    return result;
+    // We already have the hash computed
+    if (it != m_mods.keyValueEnd()) {
+        return (*it).first;
+    }
+
+    // No existing hash
+    return {};
 }
 
 bool EnsureMetadataTask::abort()
@@ -185,20 +168,22 @@ void EnsureMetadataTask::executeTask()
     version_task->start();
 }
 
-void EnsureMetadataTask::emitReady(Mod* m)
+void EnsureMetadataTask::emitReady(Mod* m, RemoveFromList remove)
 {
     qDebug() << QString("Generated metadata for %1").arg(m->name());
     emit metadataReady(m);
 
-    m_mods.remove(getHash(m));
+    if (remove == RemoveFromList::Yes)
+        m_mods.remove(getExistingHash(m));
 }
 
-void EnsureMetadataTask::emitFail(Mod* m)
+void EnsureMetadataTask::emitFail(Mod* m, RemoveFromList remove)
 {
     qDebug() << QString("Failed to generate metadata for %1").arg(m->name());
     emit metadataFailed(m);
 
-    m_mods.remove(getHash(m));
+    if (remove == RemoveFromList::Yes)
+        m_mods.remove(getExistingHash(m));
 }
 
 // Modrinth

launcher/modplatform/EnsureMetadataTask.h

@@ -1,12 +1,14 @@
 #pragma once
 
 #include "ModIndex.h"
-#include "tasks/SequentialTask.h"
 #include "net/NetJob.h"
 
+#include "modplatform/helpers/HashUtils.h"
+
+#include "tasks/ConcurrentTask.h"
+
 class Mod;
 class QDir;
-class MultipleOptionsTask;
 
 class EnsureMetadataTask : public Task {
     Q_OBJECT
@@ -17,6 +19,8 @@ class EnsureMetadataTask : public Task {
 
     ~EnsureMetadataTask() = default;
 
+    Task::Ptr getHashingTask() { return m_hashing_task; }
+
    public slots:
     bool abort() override;
    protected slots:
@@ -31,10 +35,16 @@ class EnsureMetadataTask : public Task {
     auto flameProjectsTask() -> NetJob::Ptr;
 
     // Helpers
-    void emitReady(Mod*);
+    enum class RemoveFromList {
-    void emitFail(Mod*);
+        Yes,
+        No
+    };
+    void emitReady(Mod*, RemoveFromList = RemoveFromList::Yes);
+    void emitFail(Mod*, RemoveFromList = RemoveFromList::Yes);
 
-    auto getHash(Mod*) -> QString;
+    // Hashes and stuff
+    auto createNewHash(Mod*) -> Hashing::Hasher::Ptr;
+    auto getExistingHash(Mod*) -> QString;
 
    private slots:
     void modrinthCallback(ModPlatform::IndexedPack& pack, ModPlatform::IndexedVersion& ver, Mod*);
@@ -50,5 +60,6 @@ class EnsureMetadataTask : public Task {
     ModPlatform::Provider m_provider;
 
     QHash<QString, ModPlatform::IndexedVersion> m_temp_versions;
+    ConcurrentTask* m_hashing_task;
     NetJob* m_current_task;
 };