From 7b6d26990469f26dfbed6d55af25240ef0618df4 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 22 Jul 2022 00:30:27 -0300 Subject: [PATCH] refactor: make NetJob inherit from ConcurrentTask as well! Avoids lots of code duplication Signed-off-by: flow --- launcher/net/NetAction.h | 2 + launcher/net/NetJob.cpp | 202 ++++-------------- launcher/net/NetJob.h | 48 ++--- launcher/ui/pages/modplatform/ModModel.cpp | 5 +- .../modplatform/modrinth/ModrinthModel.cpp | 5 +- 5 files changed, 64 insertions(+), 198 deletions(-) diff --git a/launcher/net/NetAction.h b/launcher/net/NetAction.h index 729d4132..d9c4fadc 100644 --- a/launcher/net/NetAction.h +++ b/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 network) { m_network = network; } + protected slots: virtual void downloadProgress(qint64 bytesReceived, qint64 bytesTotal) = 0; virtual void downloadError(QNetworkReply::NetworkError error) = 0; diff --git a/launcher/net/NetJob.cpp b/launcher/net/NetJob.cpp index bab35fa5..20d75976 100644 --- a/launcher/net/NetJob.cpp +++ b/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(); - m_downloads.append(action); - part_info pi; - m_parts_progress.append(pi); + action->m_index_within_job = m_queue.size(); + m_queue.append(action); - partProgress(m_parts_progress.count() - 1, action->getProgress(), action->getTotalProgress()); - - 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); - } + action->setNetwork(m_network); 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) { - auto part = m_downloads[index]; + for (auto part : m_queue) canFullyAbort &= part->canAbort(); - } + // can abort the active downloads? - for (auto index : m_doing) { - auto part = m_downloads[index]; + for (auto part : m_doing) 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) - QSet todoSet(m_todo.begin(), m_todo.end()); - m_failed.unite(todoSet); -#else - m_failed.unite(m_todo.toSet()); -#endif - m_todo.clear(); + for (auto task : m_queue) + m_failed.insert(task.get(), task); + m_queue.clear(); // abort active downloads auto toKill = m_doing.values(); - for (auto index : toKill) { - auto part = m_downloads[index]; + for (auto part : toKill) { fullyAborted &= part->abort(); } return fullyAborted; } -void NetJob::partSucceeded(int index) +auto NetJob::getFailedActions() -> QList { - // do progress. all slots are 1 in size at least - auto& slot = m_parts_progress[index]; - partProgress(index, slot.total_progress, slot.total_progress); - - m_doing.remove(index); - m_done.insert(index); - m_downloads[index].get()->disconnect(this); - - startMoreParts(); + QList failed; + for (auto index : m_failed) { + failed.push_back(dynamic_cast(index.get())); + } + return failed; } -void NetJob::partFailed(int index) +auto NetJob::getFailedFiles() -> QList { - m_doing.remove(index); - - auto& slot = m_parts_progress[index]; - // Can try 3 times before failing by definitive - if (slot.failures == 3) { - m_failed.insert(index); - } else { - slot.failures++; - m_todo.enqueue(index); + QList failed; + for (auto index : m_failed) { + failed.append(static_cast(index.get())->url().toString()); } - - m_downloads[index].get()->disconnect(this); - - startMoreParts(); + return failed; } -void NetJob::partAborted(int index) +void NetJob::updateState() { - m_aborted = true; - - m_doing.remove(index); - 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); - } + emit progress(m_done.count(), m_total_size); + setStatus(tr("Executing %1 task(s) (%2 out of %3 are done)") + .arg(QString::number(m_doing.count()), QString::number(m_done.count()), QString::number(m_total_size))); } diff --git a/launcher/net/NetJob.h b/launcher/net/NetJob.h index 63c1cf51..cd5d5e48 100644 --- a/launcher/net/NetJob.h +++ b/launcher/net/NetJob.h @@ -39,64 +39,40 @@ #include #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; - explicit NetJob(QString job_name, shared_qobject_ptr network) : Task(), m_network(network) - { - setObjectName(job_name); - } - virtual ~NetJob() = default; + explicit NetJob(QString job_name, shared_qobject_ptr network) : ConcurrentTask(nullptr, job_name), m_network(network) {} + ~NetJob() override = 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 at(int index) -> const NetAction::Ptr { return m_downloads.at(index); } - 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; + auto getFailedActions() -> QList; + auto getFailedFiles() -> QList; public slots: // Qt can't handle auto at the start for some reason? bool abort() override; - private slots: - void startMoreParts(); - - void partProgress(int index, qint64 bytesReceived, qint64 bytesTotal); - void partSucceeded(int index); - void partFailed(int index); - void partAborted(int index); + protected: + void updateState() override; private: shared_qobject_ptr m_network; - struct part_info { - qint64 current_progress = 0; - qint64 total_progress = 1; - int failures = 0; - }; - - QList m_downloads; - QList m_parts_progress; - QQueue m_todo; - QSet m_doing; - QSet m_done; - QSet m_failed; - qint64 m_current_progress = 0; - bool m_aborted = false; + int m_try = 1; }; diff --git a/launcher/ui/pages/modplatform/ModModel.cpp b/launcher/ui/pages/modplatform/ModModel.cpp index 94b1f099..06a6f6b8 100644 --- a/launcher/ui/pages/modplatform/ModModel.cpp +++ b/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 diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp index 3633d575..614be434 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp +++ b/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