refactor: make NetJob inherit from ConcurrentTask as well!

Avoids lots of code duplication

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

launcher/net/NetAction.h

@@ -54,6 +54,8 @@ class NetAction : public Task {
     QUrl url() { return m_url; }
     auto index() -> int { return m_index_within_job; }
 
+    void setNetwork(shared_qobject_ptr<QNetworkAccessManager> network) { m_network = network; }
+
    protected slots:
     virtual void downloadProgress(qint64 bytesReceived, qint64 bytesTotal) = 0;
     virtual void downloadError(QNetworkReply::NetworkError error) = 0;

launcher/net/NetJob.cpp

@@ -35,204 +35,90 @@
  */
 
 #include "NetJob.h"
-#include "Download.h"
 
 auto NetJob::addNetAction(NetAction::Ptr action) -> bool
 {
-    action->m_index_within_job = m_downloads.size();
+    action->m_index_within_job = m_queue.size();
-    m_downloads.append(action);
+    m_queue.append(action);
-    part_info pi;
-    m_parts_progress.append(pi);
 
-    partProgress(m_parts_progress.count() - 1, action->getProgress(), action->getTotalProgress());
+    action->setNetwork(m_network);
-
-    if (action->isRunning()) {
-        connect(action.get(), &NetAction::succeeded, [this, action]{ partSucceeded(action->index()); });
-        connect(action.get(), &NetAction::failed, [this, action](QString){ partFailed(action->index()); });
-        connect(action.get(), &NetAction::aborted, [this, action](){ partAborted(action->index()); });
-        connect(action.get(), &NetAction::progress, [this, action](qint64 done, qint64 total) { partProgress(action->index(), done, total); });
-        connect(action.get(), &NetAction::status, this, &NetJob::status);
-    } else {
-        m_todo.append(m_parts_progress.size() - 1);
-    }
 
     return true;
 }
 
+void NetJob::startNext()
+{
+    if (m_queue.isEmpty() && m_doing.isEmpty()) {
+        // We're finished, check for failures and retry if we can (up to 3 times)
+        if (!m_failed.isEmpty() && m_try < 3) {
+            m_try += 1;
+            while (!m_failed.isEmpty())
+                m_queue.enqueue(m_failed.take(*m_failed.keyBegin()));
+        }
+    }
+
+    ConcurrentTask::startNext();
+}
+
+auto NetJob::size() const -> int
+{
+    return m_queue.size() + m_doing.size() + m_done.size();
+}
+
 auto NetJob::canAbort() const -> bool
 {
     bool canFullyAbort = true;
 
     // can abort the downloads on the queue?
-    for (auto index : m_todo) {
+    for (auto part : m_queue)
-        auto part = m_downloads[index];
         canFullyAbort &= part->canAbort();
-    }
+
     // can abort the active downloads?
-    for (auto index : m_doing) {
+    for (auto part : m_doing)
-        auto part = m_downloads[index];
         canFullyAbort &= part->canAbort();
-    }
 
     return canFullyAbort;
 }
 
-void NetJob::executeTask()
-{
-    // hack that delays early failures so they can be caught easier
-    QMetaObject::invokeMethod(this, "startMoreParts", Qt::QueuedConnection);
-}
-
-auto NetJob::getFailedFiles() -> QStringList
-{
-    QStringList failed;
-    for (auto index : m_failed) {
-        failed.push_back(m_downloads[index]->url().toString());
-    }
-    failed.sort();
-    return failed;
-}
-
 auto NetJob::abort() -> bool
 {
     bool fullyAborted = true;
 
     // fail all downloads on the queue
-#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
+    for (auto task : m_queue)
-    QSet<int> todoSet(m_todo.begin(), m_todo.end());
+        m_failed.insert(task.get(), task);
-    m_failed.unite(todoSet);
+    m_queue.clear();
-#else
-    m_failed.unite(m_todo.toSet());
-#endif
-    m_todo.clear();
 
     // abort active downloads
     auto toKill = m_doing.values();
-    for (auto index : toKill) {
+    for (auto part : toKill) {
-        auto part = m_downloads[index];
         fullyAborted &= part->abort();
     }
 
     return fullyAborted;
 }
 
-void NetJob::partSucceeded(int index)
+auto NetJob::getFailedActions() -> QList<NetAction*>
 {
-    // do progress. all slots are 1 in size at least
+    QList<NetAction*> failed;
-    auto& slot = m_parts_progress[index];
+    for (auto index : m_failed) {
-    partProgress(index, slot.total_progress, slot.total_progress);
+        failed.push_back(dynamic_cast<NetAction*>(index.get()));
-
+    }
-    m_doing.remove(index);
+    return failed;
-    m_done.insert(index);
-    m_downloads[index].get()->disconnect(this);
-
-    startMoreParts();
 }
 
-void NetJob::partFailed(int index)
+auto NetJob::getFailedFiles() -> QList<QString>
 {
-    m_doing.remove(index);
+    QList<QString> failed;
-
+    for (auto index : m_failed) {
-    auto& slot = m_parts_progress[index];
+        failed.append(static_cast<NetAction*>(index.get())->url().toString());
-    // Can try 3 times before failing by definitive
+    }
-    if (slot.failures == 3) {
+    return failed;
-        m_failed.insert(index);
-    } else {
-        slot.failures++;
-        m_todo.enqueue(index);
 }
 
-    m_downloads[index].get()->disconnect(this);
+void NetJob::updateState()
-
-    startMoreParts();
-}
-
-void NetJob::partAborted(int index)
 {
-    m_aborted = true;
+    emit progress(m_done.count(), m_total_size);
-
+    setStatus(tr("Executing %1 task(s) (%2 out of %3 are done)")
-    m_doing.remove(index);
+                  .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(m_total_size)));
-    m_failed.insert(index);
-    m_downloads[index].get()->disconnect(this);
-
-    startMoreParts();
-}
-
-void NetJob::partProgress(int index, qint64 bytesReceived, qint64 bytesTotal)
-{
-    auto& slot = m_parts_progress[index];
-    slot.current_progress = bytesReceived;
-    slot.total_progress = bytesTotal;
-
-    int done = m_done.size();
-    int doing = m_doing.size();
-    int all = m_parts_progress.size();
-
-    qint64 bytesAll = 0;
-    qint64 bytesTotalAll = 0;
-    for (auto& partIdx : m_doing) {
-        auto part = m_parts_progress[partIdx];
-        // do not count parts with unknown/nonsensical total size
-        if (part.total_progress <= 0) {
-            continue;
-        }
-        bytesAll += part.current_progress;
-        bytesTotalAll += part.total_progress;
-    }
-
-    qint64 inprogress = (bytesTotalAll == 0) ? 0 : (bytesAll * 1000) / bytesTotalAll;
-    auto current = done * 1000 + doing * inprogress;
-    auto current_total = all * 1000;
-    // HACK: make sure it never jumps backwards.
-    // FAIL: This breaks if the size is not known (or is it something else?) and jumps to 1000, so if it is 1000 reset it to inprogress
-    if (m_current_progress == 1000) {
-        m_current_progress = inprogress;
-    }
-    if (m_current_progress > current) {
-        current = m_current_progress;
-    }
-    m_current_progress = current;
-    setProgress(current, current_total);
-}
-
-void NetJob::startMoreParts()
-{
-    if (!isRunning()) {
-        // this actually makes sense. You can put running m_downloads into a NetJob and then not start it until much later.
-        return;
-    }
-
-    // OK. We are actively processing tasks, proceed.
-    // Check for final conditions if there's nothing in the queue.
-    if (!m_todo.size()) {
-        if (!m_doing.size()) {
-            if (!m_failed.size()) {
-                emitSucceeded();
-            } else if (m_aborted) {
-                emitAborted();
-            } else {
-                emitFailed(tr("Job '%1' failed to process:\n%2").arg(objectName()).arg(getFailedFiles().join("\n")));
-            }
-        }
-        return;
-    }
-
-    // There's work to do, try to start more parts, to a maximum of 6 concurrent ones.
-    while (m_doing.size() < 6) {
-        if (m_todo.size() == 0)
-            return;
-        int doThis = m_todo.dequeue();
-        m_doing.insert(doThis);
-
-        auto part = m_downloads[doThis];
-
-        // connect signals :D
-        connect(part.get(), &NetAction::succeeded, this, [this, part]{ partSucceeded(part->index()); });
-        connect(part.get(), &NetAction::failed, this, [this, part](QString){ partFailed(part->index()); });
-        connect(part.get(), &NetAction::aborted, this, [this, part]{ partAborted(part->index()); });
-        connect(part.get(), &NetAction::progress, this, [this, part](qint64 done, qint64 total) { partProgress(part->index(), done, total); });
-        connect(part.get(), &NetAction::status, this, &NetJob::status);
-
-        part->startAction(m_network);
-    }
 }

launcher/net/NetJob.h

@@ -39,64 +39,40 @@
 
 #include <QObject>
 #include "NetAction.h"
-#include "tasks/Task.h"
+#include "tasks/ConcurrentTask.h"
 
 // Those are included so that they are also included by anyone using NetJob
 #include "net/Download.h"
 #include "net/HttpMetaCache.h"
 
-class NetJob : public Task {
+class NetJob : public ConcurrentTask {
     Q_OBJECT
 
    public:
     using Ptr = shared_qobject_ptr<NetJob>;
 
-    explicit NetJob(QString job_name, shared_qobject_ptr<QNetworkAccessManager> network) : Task(), m_network(network)
+    explicit NetJob(QString job_name, shared_qobject_ptr<QNetworkAccessManager> network) : ConcurrentTask(nullptr, job_name), m_network(network) {}
-    {
+    ~NetJob() override = default;
-        setObjectName(job_name);
-    }
-    virtual ~NetJob() = default;
 
-    void executeTask() override;
+    void startNext() override;
+
+    auto size() const -> int;
 
     auto canAbort() const -> bool override;
-
     auto addNetAction(NetAction::Ptr action) -> bool;
 
-    auto operator[](int index) -> NetAction::Ptr { return m_downloads[index]; }
+    auto getFailedActions() -> QList<NetAction*>;
-    auto at(int index) -> const NetAction::Ptr { return m_downloads.at(index); }
+    auto getFailedFiles() -> QList<QString>;
-    auto size() const -> int { return m_downloads.size(); }
-    auto first() -> NetAction::Ptr { return m_downloads.size() != 0 ? m_downloads[0] : NetAction::Ptr{}; }
-
-    auto getFailedFiles() -> QStringList;
 
    public slots:
     // Qt can't handle auto at the start for some reason?
     bool abort() override;
 
-   private slots:
+   protected:
-    void startMoreParts();
+    void updateState() override;
-
-    void partProgress(int index, qint64 bytesReceived, qint64 bytesTotal);
-    void partSucceeded(int index);
-    void partFailed(int index);
-    void partAborted(int index);
 
    private:
     shared_qobject_ptr<QNetworkAccessManager> m_network;
 
-    struct part_info {
+    int m_try = 1;
-        qint64 current_progress = 0;
-        qint64 total_progress = 1;
-        int failures = 0;
-    };
-
-    QList<NetAction::Ptr> m_downloads;
-    QList<part_info> m_parts_progress;
-    QQueue<int> m_todo;
-    QSet<int> m_doing;
-    QSet<int> m_done;
-    QSet<int> m_failed;
-    qint64 m_current_progress = 0;
-    bool m_aborted = false;
 };

launcher/ui/pages/modplatform/ModModel.cpp

@@ -230,10 +230,11 @@ void ListModel::searchRequestFinished(QJsonDocument& doc)
 
 void ListModel::searchRequestFailed(QString reason)
 {
-    if (!jobPtr->first()->m_reply) {
+    auto failed_action = jobPtr->getFailedActions().at(0);
+    if (!failed_action->m_reply) {
         // Network error
         QMessageBox::critical(nullptr, tr("Error"), tr("A network error occurred. Could not load mods."));
-    } else if (jobPtr->first()->m_reply && jobPtr->first()->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 409) {
+    } else if (failed_action->m_reply && failed_action->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 409) {
         // 409 Gone, notify user to update
         QMessageBox::critical(nullptr, tr("Error"),
                               //: %1 refers to the launcher itself

launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp

@@ -301,10 +301,11 @@ void ModpackListModel::searchRequestFinished(QJsonDocument& doc_all)
 
 void ModpackListModel::searchRequestFailed(QString reason)
 {
-    if (!jobPtr->first()->m_reply) {
+    auto failed_action = jobPtr->getFailedActions().at(0);
+    if (!failed_action->m_reply) {
         // Network error
         QMessageBox::critical(nullptr, tr("Error"), tr("A network error occurred. Could not load modpacks."));
-    } else if (jobPtr->first()->m_reply && jobPtr->first()->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 409) {
+    } else if (failed_action->m_reply && failed_action->m_reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 409) {
         // 409 Gone, notify user to update
         QMessageBox::critical(nullptr, tr("Error"),
                               //: %1 refers to the launcher itself