From 89d3a66658ebdb16582a4d7a2cab57cfd6906393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Mon, 26 Jun 2017 01:14:32 +0200 Subject: NOISSUE some safe refactors and changes of the task subsystem Possibly also some bug fixes. --- api/logic/CMakeLists.txt | 2 - api/logic/java/JavaCheckerJob.cpp | 3 +- api/logic/java/JavaCheckerJob.h | 5 -- api/logic/launch/LaunchTask.cpp | 2 +- api/logic/launch/steps/Update.cpp | 2 +- api/logic/minecraft/launch/ModMinecraftJar.cpp | 2 +- api/logic/net/NetAction.h | 67 ++++++++++++++------------ api/logic/net/NetJob.cpp | 45 ++++++++++++++--- api/logic/net/NetJob.h | 31 +----------- api/logic/tasks/Task.cpp | 2 +- api/logic/tasks/Task.h | 19 +++----- api/logic/tasks/ThreadTask.cpp | 41 ---------------- api/logic/tasks/ThreadTask.h | 26 ---------- api/logic/updater/DownloadTask.cpp | 4 +- 14 files changed, 89 insertions(+), 162 deletions(-) delete mode 100644 api/logic/tasks/ThreadTask.cpp delete mode 100644 api/logic/tasks/ThreadTask.h (limited to 'api/logic') diff --git a/api/logic/CMakeLists.txt b/api/logic/CMakeLists.txt index 2eda34fe..bb999619 100644 --- a/api/logic/CMakeLists.txt +++ b/api/logic/CMakeLists.txt @@ -350,8 +350,6 @@ set(TASKS_SOURCES # Tasks tasks/Task.h tasks/Task.cpp - tasks/ThreadTask.h - tasks/ThreadTask.cpp tasks/SequentialTask.h tasks/SequentialTask.cpp ) diff --git a/api/logic/java/JavaCheckerJob.cpp b/api/logic/java/JavaCheckerJob.cpp index 01b5b28d..a111af20 100644 --- a/api/logic/java/JavaCheckerJob.cpp +++ b/api/logic/java/JavaCheckerJob.cpp @@ -22,7 +22,7 @@ void JavaCheckerJob::partFinished(JavaCheckResult result) num_finished++; qDebug() << m_job_name.toLocal8Bit() << "progress:" << num_finished << "/" << javacheckers.size(); - emit progress(num_finished, javacheckers.size()); + setProgress(num_finished, javacheckers.size()); javaresults.replace(result.id, result); @@ -35,7 +35,6 @@ void JavaCheckerJob::partFinished(JavaCheckResult result) void JavaCheckerJob::executeTask() { qDebug() << m_job_name.toLocal8Bit() << " started."; - m_running = true; for (auto iter : javacheckers) { javaresults.append(JavaCheckResult()); diff --git a/api/logic/java/JavaCheckerJob.h b/api/logic/java/JavaCheckerJob.h index 58a98190..c27a50c3 100644 --- a/api/logic/java/JavaCheckerJob.h +++ b/api/logic/java/JavaCheckerJob.h @@ -58,10 +58,6 @@ public: { return javacheckers.size(); } - virtual bool isRunning() const override - { - return m_running; - } signals: void started(); @@ -80,5 +76,4 @@ private: qint64 current_progress = 0; qint64 total_progress = 0; int num_finished = 0; - bool m_running = false; }; diff --git a/api/logic/launch/LaunchTask.cpp b/api/logic/launch/LaunchTask.cpp index 23c28f50..e1794cae 100644 --- a/api/logic/launch/LaunchTask.cpp +++ b/api/logic/launch/LaunchTask.cpp @@ -83,7 +83,7 @@ void LaunchTask::onStepFinished() } auto step = m_steps[currentStep]; - if(step->successful()) + if(step->wasSuccessful()) { // end? if(currentStep == m_steps.size() - 1) diff --git a/api/logic/launch/steps/Update.cpp b/api/logic/launch/steps/Update.cpp index 956230f4..f06c39eb 100644 --- a/api/logic/launch/steps/Update.cpp +++ b/api/logic/launch/steps/Update.cpp @@ -42,7 +42,7 @@ void Update::proceed() void Update::updateFinished() { - if(m_updateTask->successful()) + if(m_updateTask->wasSuccessful()) { m_updateTask.reset(); emitSucceeded(); diff --git a/api/logic/minecraft/launch/ModMinecraftJar.cpp b/api/logic/minecraft/launch/ModMinecraftJar.cpp index dad41a98..b56f6fca 100644 --- a/api/logic/minecraft/launch/ModMinecraftJar.cpp +++ b/api/logic/minecraft/launch/ModMinecraftJar.cpp @@ -31,7 +31,7 @@ void ModMinecraftJar::executeTask() void ModMinecraftJar::jarModdingFinished() { - if(m_jarModTask->successful()) + if(m_jarModTask->wasSuccessful()) { emitSucceeded(); } diff --git a/api/logic/net/NetAction.h b/api/logic/net/NetAction.h index a533c317..f164dda3 100644 --- a/api/logic/net/NetAction.h +++ b/api/logic/net/NetAction.h @@ -43,18 +43,26 @@ protected: public: virtual ~NetAction() {}; -public: - virtual qint64 totalProgress() const + bool isRunning() const { - return m_total_progress; + return m_status == Job_InProgress; } - virtual qint64 currentProgress() const + bool isFinished() const { - return m_progress; + return m_status >= Job_Finished; } - virtual qint64 numberOfFailures() const + bool wasSuccessful() const { - return m_failures; + return m_status == Job_Finished || m_status == Job_Failed_Proceed; + } + + qint64 totalProgress() const + { + return m_total_progress; + } + qint64 currentProgress() const + { + return m_progress; } virtual bool abort() { @@ -64,25 +72,10 @@ public: { return false; } - -public: - /// the network reply - unique_qobject_ptr m_reply; - - /// source URL - QUrl m_url; - - /// The file's status - JobStatus m_status = Job_NotStarted; - - /// index within the parent job - int m_index_within_job = 0; - - qint64 m_progress = 0; - qint64 m_total_progress = 1; - - /// number of failures up to this point - int m_failures = 0; + QUrl url() + { + return m_url; + } signals: void started(int index); @@ -91,14 +84,28 @@ signals: void failed(int index); void aborted(int index); -protected -slots: +protected slots: virtual void downloadProgress(qint64 bytesReceived, qint64 bytesTotal) = 0; virtual void downloadError(QNetworkReply::NetworkError error) = 0; virtual void downloadFinished() = 0; virtual void downloadReadyRead() = 0; -public -slots: +public slots: virtual void start() = 0; + +public: + /// index within the parent job, FIXME: nuke + int m_index_within_job = 0; + + /// the network reply + unique_qobject_ptr m_reply; + + /// source URL + QUrl m_url; + + qint64 m_progress = 0; + qint64 m_total_progress = 1; + +protected: + JobStatus m_status = Job_NotStarted; }; diff --git a/api/logic/net/NetJob.cpp b/api/logic/net/NetJob.cpp index 275da749..693a0934 100644 --- a/api/logic/net/NetJob.cpp +++ b/api/logic/net/NetJob.cpp @@ -73,18 +73,19 @@ void NetJob::partProgress(int index, qint64 bytesReceived, qint64 bytesTotal) void NetJob::executeTask() { qDebug() << m_job_name.toLocal8Bit() << " started."; - m_running = true; - for (int i = 0; i < downloads.size(); i++) - { - m_todo.enqueue(i); - } // hack that delays early failures so they can be caught easier QMetaObject::invokeMethod(this, "startMoreParts", Qt::QueuedConnection); } void NetJob::startMoreParts() { - // check for final conditions if there's nothing in the queue + if(!isRunning()) + { + // this actually makes sense. You can put running 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()) @@ -107,7 +108,7 @@ void NetJob::startMoreParts() } return; } - // otherwise try to start more parts + // There's work to do, try to start more parts. while (m_doing.size() < 6) { if(!m_todo.size()) @@ -131,7 +132,7 @@ QStringList NetJob::getFailedFiles() QStringList failed; for (auto index: m_failed) { - failed.push_back(downloads[index]->m_url.toString()); + failed.push_back(downloads[index]->url().toString()); } failed.sort(); return failed; @@ -170,3 +171,31 @@ bool NetJob::abort() } return fullyAborted; } + +bool NetJob::addNetAction(NetActionPtr action) +{ + action->m_index_within_job = downloads.size(); + downloads.append(action); + part_info pi; + { + pi.current_progress = action->currentProgress(); + pi.total_progress = action->totalProgress(); + pi.failures = 0; + } + parts_progress.append(pi); + total_progress += pi.total_progress; + + // FIXME: detect if the action is already running, put it in m_doing if it is! + setProgress(current_progress, total_progress); + if(action->isRunning()) + { + connect(action.get(), SIGNAL(succeeded(int)), SLOT(partSucceeded(int))); + connect(action.get(), SIGNAL(failed(int)), SLOT(partFailed(int))); + connect(action.get(), SIGNAL(netActionProgress(int, qint64, qint64)), SLOT(partProgress(int, qint64, qint64))); + } + else + { + m_todo.append(parts_progress.size() - 1); + } + return true; +} diff --git a/api/logic/net/NetJob.h b/api/logic/net/NetJob.h index ca4f5df1..cd576664 100644 --- a/api/logic/net/NetJob.h +++ b/api/logic/net/NetJob.h @@ -32,30 +32,8 @@ class MULTIMC_LOGIC_EXPORT NetJob : public Task public: explicit NetJob(QString job_name) : Task(), m_job_name(job_name) {} virtual ~NetJob() {} - bool addNetAction(NetActionPtr action) - { - action->m_index_within_job = downloads.size(); - downloads.append(action); - part_info pi; - { - pi.current_progress = action->currentProgress(); - pi.total_progress = action->totalProgress(); - pi.failures = action->numberOfFailures(); - } - parts_progress.append(pi); - total_progress += pi.total_progress; - // if this is already running, the action needs to be started right away! - if (isRunning()) - { - setProgress(current_progress, total_progress); - connect(action.get(), SIGNAL(succeeded(int)), SLOT(partSucceeded(int))); - connect(action.get(), SIGNAL(failed(int)), SLOT(partFailed(int))); - connect(action.get(), SIGNAL(netActionProgress(int, qint64, qint64)), - SLOT(partProgress(int, qint64, qint64))); - action->start(); - } - return true; - } + + bool addNetAction(NetActionPtr action); NetActionPtr operator[](int index) { @@ -75,10 +53,6 @@ public: { return downloads.size(); } - virtual bool isRunning() const override - { - return m_running; - } QStringList getFailedFiles(); bool canAbort() const override; @@ -113,6 +87,5 @@ private: QSet m_failed; qint64 current_progress = 0; qint64 total_progress = 0; - bool m_running = false; bool m_aborted = false; }; diff --git a/api/logic/tasks/Task.cpp b/api/logic/tasks/Task.cpp index 23ee08e4..94a4d428 100644 --- a/api/logic/tasks/Task.cpp +++ b/api/logic/tasks/Task.cpp @@ -76,7 +76,7 @@ bool Task::isFinished() const return m_finished; } -bool Task::successful() const +bool Task::wasSuccessful() const { return m_succeeded; } diff --git a/api/logic/tasks/Task.h b/api/logic/tasks/Task.h index 47c4a13e..3654ed24 100644 --- a/api/logic/tasks/Task.h +++ b/api/logic/tasks/Task.h @@ -27,21 +27,15 @@ public: explicit Task(QObject *parent = 0); virtual ~Task() {}; - virtual bool isRunning() const; - - virtual bool isFinished() const; - - /*! - * True if this task was successful. - * If the task failed or is still running, returns false. - */ - virtual bool successful() const; + bool isRunning() const; + bool isFinished() const; + bool wasSuccessful() const; /*! * Returns the string that was passed to emitFailed as the error message when the task failed. * If the task hasn't failed, returns an empty string. */ - virtual QString failReason() const; + QString failReason() const; virtual bool canAbort() const { return false; } @@ -68,8 +62,7 @@ signals: void failed(QString reason); void status(QString status); -public -slots: +public slots: virtual void start(); virtual bool abort() { return false; }; @@ -84,7 +77,7 @@ public slots: void setStatus(const QString &status); void setProgress(qint64 current, qint64 total); -protected: +private: bool m_running = false; bool m_finished = false; bool m_succeeded = false; diff --git a/api/logic/tasks/ThreadTask.cpp b/api/logic/tasks/ThreadTask.cpp deleted file mode 100644 index ddd1dee5..00000000 --- a/api/logic/tasks/ThreadTask.cpp +++ /dev/null @@ -1,41 +0,0 @@ -#include "ThreadTask.h" -#include -ThreadTask::ThreadTask(Task * internal, QObject *parent) : Task(parent), m_internal(internal) -{ -} - -void ThreadTask::start() -{ - connect(m_internal, SIGNAL(failed(QString)), SLOT(iternal_failed(QString))); - connect(m_internal, SIGNAL(progress(qint64,qint64)), SLOT(iternal_progress(qint64,qint64))); - connect(m_internal, SIGNAL(started()), SLOT(iternal_started())); - connect(m_internal, SIGNAL(status(QString)), SLOT(iternal_status(QString))); - connect(m_internal, SIGNAL(succeeded()), SLOT(iternal_succeeded())); - m_running = true; - QtConcurrent::run(m_internal, &Task::start); -} - -void ThreadTask::iternal_failed(QString reason) -{ - emitFailed(reason); -} - -void ThreadTask::iternal_progress(qint64 current, qint64 total) -{ - progress(current, total); -} - -void ThreadTask::iternal_started() -{ - emit started(); -} - -void ThreadTask::iternal_status(QString status) -{ - setStatus(status); -} - -void ThreadTask::iternal_succeeded() -{ - emitSucceeded(); -} diff --git a/api/logic/tasks/ThreadTask.h b/api/logic/tasks/ThreadTask.h deleted file mode 100644 index 46ce3a36..00000000 --- a/api/logic/tasks/ThreadTask.h +++ /dev/null @@ -1,26 +0,0 @@ -#pragma once - -#include "Task.h" -#include "multimc_logic_export.h" - -class MULTIMC_LOGIC_EXPORT ThreadTask : public Task -{ - Q_OBJECT -public: - explicit ThreadTask(Task * internal, QObject * parent = nullptr); - -protected: - void executeTask() {}; - -public slots: - virtual void start(); - -private slots: - void iternal_started(); - void iternal_progress(qint64 current, qint64 total); - void iternal_succeeded(); - void iternal_failed(QString reason); - void iternal_status(QString status); -private: - Task * m_internal; -}; \ No newline at end of file diff --git a/api/logic/updater/DownloadTask.cpp b/api/logic/updater/DownloadTask.cpp index 0d40f78a..f08839b5 100644 --- a/api/logic/updater/DownloadTask.cpp +++ b/api/logic/updater/DownloadTask.cpp @@ -70,7 +70,7 @@ void DownloadTask::vinfoDownloadFailed() { // Something failed. We really need the second download (current version info), so parse // downloads anyways as long as the first one succeeded. - if (m_newVersionFileListDownload->m_status != Job_Failed) + if (m_newVersionFileListDownload->wasSuccessful()) { processDownloadedVersionInfo(); return; @@ -97,7 +97,7 @@ void DownloadTask::processDownloadedVersionInfo() } // if we have the current version info, use it. - if (m_currentVersionFileListDownload && m_currentVersionFileListDownload->m_status != Job_Failed) + if (m_currentVersionFileListDownload && m_currentVersionFileListDownload->wasSuccessful()) { setStatus(tr("Reading file list for current version...")); qDebug() << "Reading file list for current version..."; -- cgit v1.2.3