Move finishedSignal to AbstractNetworkJobwork/fix_SyncConflictTest

21 files changed, 142 insertions, 200 deletions

diff --git a/src/gui/thumbnailjob.cpp b/src/gui/thumbnailjob.cpp
index 8d3ae5b30..076b4a1ae 100644
--- a/src/gui/thumbnailjob.cpp
+++ b/src/gui/thumbnailjob.cpp
@@ -31,7 +31,7 @@ void ThumbnailJob::start()
     AbstractNetworkJob::start();
 }
 
-bool ThumbnailJob::finished()
+void ThumbnailJob::finished()
 {
     const auto result = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     QPixmap p;
@@ -42,6 +42,5 @@ bool ThumbnailJob::finished()
         }
     }
     emit jobFinished(result, p);
-    return true;
 }
 }
diff --git a/src/gui/thumbnailjob.h b/src/gui/thumbnailjob.h
index 90a67a899..983a14c2a 100644
--- a/src/gui/thumbnailjob.h
+++ b/src/gui/thumbnailjob.h
@@ -44,7 +44,7 @@ signals:
      */
     void jobFinished(int statusCode, QPixmap reply);
 private slots:
-    bool finished() override;
+    void finished() override;
 };
 }
 
diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp
index 2174db3a6..81e01ab4c 100644
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -218,13 +218,11 @@ void AbstractNetworkJob::slotFinished()
             qCWarning(lcNetworkJob) << "Don't retry:" << _reply->url();
         }
     }
-
-    bool discard = finished();
-    if (discard) {
-        Q_EMIT abstractJobFinished();
-        qCDebug(lcNetworkJob) << "Network job finished" << this;
-        deleteLater();
-    }
+    Q_EMIT aboutToFinishedSignal(AbstractNetworkJob::QPrivateSignal());
+    finished();
+    Q_EMIT finishedSignal(AbstractNetworkJob::QPrivateSignal());
+    qCDebug(lcNetworkJob) << "Network job finished" << this;
+    deleteLater();
 }
 
 QByteArray AbstractNetworkJob::responseTimestamp()
diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h
index deaac27db..05801a7de 100644
--- a/src/libsync/abstractnetworkjob.h
+++ b/src/libsync/abstractnetworkjob.h
@@ -134,9 +134,10 @@ signals:
     void networkError(QNetworkReply *reply);
 
     /**
-     * Use complex name to prevent clash with other implementations of a finish signal...
+     * The job is done
      */
-    void abstractJobFinished();
+    void aboutToFinishedSignal(QPrivateSignal);
+    void finishedSignal(QPrivateSignal);
 
 protected:
     /** Initiate a network request, returning a QNetworkReply.
@@ -158,10 +159,8 @@ protected:
     virtual void newReplyHook(QNetworkReply *) {}
 
     /** Called at the end of QNetworkReply::finished processing.
-     *
-     * Returning true triggers a deleteLater() of this job.
      */
-    virtual bool finished() = 0;
+    virtual void finished() = 0;
 
     QByteArray _responseTimestamp;
 
diff --git a/src/libsync/bandwidthmanager.cpp b/src/libsync/bandwidthmanager.cpp
index ac3516a09..63e7b7527 100644
--- a/src/libsync/bandwidthmanager.cpp
+++ b/src/libsync/bandwidthmanager.cpp
@@ -120,10 +120,12 @@ void BandwidthManager::unregisterUploadDevice(QObject *o)
     }
 }
 
-void BandwidthManager::registerDownloadJob(GETJob *j)
+void BandwidthManager::registerDownloadJob(GETFileJob *j)
 {
     _downloadJobList.push_back(j);
-    QObject::connect(j, &QObject::destroyed, this, &BandwidthManager::unregisterDownloadJob);
+    connect(j, &GETFileJob::aboutToFinishedSignal, this, [j, this] {
+        unregisterDownloadJob(j);
+    });
 
     if (usingAbsoluteDownloadLimit()) {
         j->setBandwidthLimited(true);
@@ -137,9 +139,10 @@ void BandwidthManager::registerDownloadJob(GETJob *j)
     }
 }
 
-void BandwidthManager::unregisterDownloadJob(QObject *o)
+void BandwidthManager::unregisterDownloadJob(GETFileJob *j)
 {
-    GETJob *j = reinterpret_cast<GETJob *>(o); // note, we might already be in the ~QObject
+    j->setChoked(false);
+    j->setBandwidthLimited(false);
     _downloadJobList.remove(j);
     if (_relativeLimitCurrentMeasuredJob == j) {
         _relativeLimitCurrentMeasuredJob = nullptr;
diff --git a/src/libsync/bandwidthmanager.h b/src/libsync/bandwidthmanager.h
index a75ea973a..2f97607a6 100644
--- a/src/libsync/bandwidthmanager.h
+++ b/src/libsync/bandwidthmanager.h
@@ -24,7 +24,7 @@
 namespace OCC {
 
 class UploadDevice;
-class GETJob;
+class GETFileJob;
 class OwncloudPropagator;
 
 /**
@@ -48,8 +48,8 @@ public slots:
     void registerUploadDevice(UploadDevice *);
     void unregisterUploadDevice(QObject *);
 
-    void registerDownloadJob(GETJob *);
-    void unregisterDownloadJob(QObject *);
+    void registerDownloadJob(GETFileJob *);
+    void unregisterDownloadJob(GETFileJob *);
 
     void absoluteLimitTimerExpired();
     void switchingTimerExpired();
@@ -87,14 +87,14 @@ private:
     qint64 _relativeUploadLimitProgressAtMeasuringRestart;
     qint64 _currentUploadLimit;
 
-    std::list<GETJob *> _downloadJobList;
+    std::list<GETFileJob *> _downloadJobList;
     QTimer _relativeDownloadMeasuringTimer;
 
     // for relative bw limiting, we need to wait this amount before measuring again
     QTimer _relativeDownloadDelayTimer;
 
     // the device measured
-    GETJob *_relativeLimitCurrentMeasuredJob;
+    GETFileJob *_relativeLimitCurrentMeasuredJob;
 
     // for measuring how much progress we made at start
     qint64 _relativeDownloadLimitProgressAtMeasuringRestart;
diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp
index d718fdebe..5c63d41e6 100644
--- a/src/libsync/networkjobs.cpp
+++ b/src/libsync/networkjobs.cpp
@@ -95,7 +95,7 @@ void RequestEtagJob::start()
     AbstractNetworkJob::start();
 }
 
-bool RequestEtagJob::finished()
+void RequestEtagJob::finished()
 {
     qCInfo(lcEtagJob) << "Request Etag of" << reply()->request().url() << "FINISHED WITH STATUS"
                       <<  replyStatusString();
@@ -126,7 +126,6 @@ bool RequestEtagJob::finished()
     } else {
         emit finishedWithResult(HttpError{ httpCode, errorString() });
     }
-    return true;
 }
 
 /*********************************************************************************************/
@@ -152,7 +151,7 @@ void MkColJob::start()
     AbstractNetworkJob::start();
 }
 
-bool MkColJob::finished()
+void MkColJob::finished()
 {
     qCInfo(lcMkColJob) << "MKCOL of" << reply()->request().url() << "FINISHED WITH STATUS"
                        << replyStatusString();
@@ -162,7 +161,6 @@ bool MkColJob::finished()
     } else {
         Q_EMIT finishedWithoutError();
     }
-    return true;
 }
 
 /*********************************************************************************************/
@@ -326,7 +324,7 @@ void LsColJob::start()
 // TODO: Instead of doing all in this slot, we should iteratively parse in readyRead(). This
 // would allow us to be more asynchronous in processing while data is coming from the network,
 // not all in one big blob at the end.
-bool LsColJob::finished()
+void LsColJob::finished()
 {
     qCInfo(lcLsColJob) << "LSCOL of" << reply()->request().url() << "FINISHED WITH STATUS"
                        << replyStatusString();
@@ -356,8 +354,6 @@ bool LsColJob::finished()
         // wrong HTTP code or any other network error
         emit finishedWithError(reply());
     }
-
-    return true;
 }
 
 void LsColJob::startImpl(const QNetworkRequest &req)
@@ -449,7 +445,7 @@ QPixmap AvatarJob::makeCircularAvatar(const QPixmap &baseAvatar)
     return avatar;
 }
 
-bool AvatarJob::finished()
+void AvatarJob::finished()
 {
     int http_result_code = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
@@ -464,7 +460,6 @@ bool AvatarJob::finished()
         }
     }
     emit avatarPixmap(avImage);
-    return true;
 }
 #endif
 
@@ -481,10 +476,9 @@ void EntityExistsJob::start()
     AbstractNetworkJob::start();
 }
 
-bool EntityExistsJob::finished()
+void EntityExistsJob::finished()
 {
     emit exists(reply());
-    return true;
 }
 
 /*********************************************************************************************/
@@ -510,7 +504,7 @@ void DetermineAuthTypeJob::start()
     AbstractNetworkJob::start();
 }
 
-bool DetermineAuthTypeJob::finished()
+void DetermineAuthTypeJob::finished()
 {
     auto authChallenge = reply()->rawHeader("WWW-Authenticate").toLower();
     auto result = AuthType::Basic;
@@ -521,7 +515,6 @@ bool DetermineAuthTypeJob::finished()
     }
     qCInfo(lcDetermineAuthTypeJob) << "Auth type for" << _account->davUrl() << "is" << result;
     emit this->authType(result);
-    return true;
 }
 
 SimpleNetworkJob::SimpleNetworkJob(AccountPtr account, const QUrl &rootUrl, const QString &path, const QByteArray &verb, const QNetworkRequest &req, QObject *parent)
@@ -588,13 +581,11 @@ void SimpleNetworkJob::addNewReplyHook(std::function<void(QNetworkReply *)> &&ho
     _replyHooks.push_back(hook);
 }
 
-bool SimpleNetworkJob::finished()
+void SimpleNetworkJob::finished()
 {
     if (_device) {
         _device->close();
     }
-    emit finishedSignal();
-    return true;
 }
 
 void SimpleNetworkJob::newReplyHook(QNetworkReply *reply)
diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h
index 4ecfbc07d..e612fecd5 100644
--- a/src/libsync/networkjobs.h
+++ b/src/libsync/networkjobs.h
@@ -53,7 +53,7 @@ signals:
     void exists(QNetworkReply *);
 
 private slots:
-    bool finished() override;
+    void finished() override;
 };
 
 /**
@@ -102,7 +102,7 @@ signals:
     void finishedWithoutError();
 
 private slots:
-    bool finished() override;
+    void finished() override;
 
 protected:
     void startImpl(const QNetworkRequest &req);
@@ -167,7 +167,7 @@ signals:
     void avatarPixmap(const QPixmap &);
 
 private slots:
-    bool finished() override;
+    void finished() override;
 };
 #endif
 
@@ -190,7 +190,7 @@ signals:
     void finishedWithoutError();
 
 private:
-    bool finished() override;
+    void finished() override;
 };
 
 /**
@@ -208,7 +208,7 @@ signals:
     void finishedWithResult(const HttpResult<QByteArray> &etag);
 
 private slots:
-    bool finished() override;
+    void finished() override;
 };
 
 /**
@@ -232,7 +232,7 @@ signals:
     void authType(AuthType);
 
 protected Q_SLOTS:
-    bool finished() override;
+    void finished() override;
 };
 
 /**
@@ -259,11 +259,8 @@ public:
 
     void addNewReplyHook(std::function<void(QNetworkReply *)> &&hook);
 
-signals:
-    void finishedSignal();
-
 protected:
-    bool finished() override;
+    void finished() override;
     void newReplyHook(QNetworkReply *) override;
 
     QNetworkRequest _request;
diff --git a/src/libsync/networkjobs/jsonjob.cpp b/src/libsync/networkjobs/jsonjob.cpp
index 47d767f44..703b4edb4 100644
--- a/src/libsync/networkjobs/jsonjob.cpp
+++ b/src/libsync/networkjobs/jsonjob.cpp
@@ -28,7 +28,7 @@ JsonJob::~JsonJob()
 {
 }
 
-bool JsonJob::finished()
+void JsonJob::finished()
 {
     qCInfo(lcJsonApiJob) << "JsonJob of" << reply()->request().url() << "FINISHED WITH STATUS"
                          << replyStatusString();
@@ -38,7 +38,7 @@ bool JsonJob::finished()
     } else {
         parse(reply()->readAll());
     }
-    return SimpleNetworkJob::finished();
+    SimpleNetworkJob::finished();
 }
 
 void JsonJob::parse(const QByteArray &data)
diff --git a/src/libsync/networkjobs/jsonjob.h b/src/libsync/networkjobs/jsonjob.h
index b6e338a06..147a89ed8 100644
--- a/src/libsync/networkjobs/jsonjob.h
+++ b/src/libsync/networkjobs/jsonjob.h
@@ -33,7 +33,7 @@ public:
     const QJsonParseError &parseError() const;
 
 protected:
-    bool finished() override;
+    void finished() override;
 
     virtual void parse(const QByteArray &data);
 
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 3d3314ff8..fa4d04c12 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -75,15 +75,22 @@ QString OWNCLOUDSYNC_EXPORT createDownloadTmpFileName(const QString &previous)
 GETFileJob::GETFileJob(AccountPtr account, const QUrl &url, const QString &path, QIODevice *device,
     const QMap<QByteArray, QByteArray> &headers, const QByteArray &expectedEtagForResume,
     qint64 resumeStart, QObject *parent)
-    : GETJob(account, url, path, parent)
+    : AbstractNetworkJob(account, url, path, parent)
     , _device(device)
     , _headers(headers)
     , _expectedEtagForResume(expectedEtagForResume)
     , _expectedContentLength(-1)
     , _contentLength(-1)
     , _resumeStart(resumeStart)
-    , _hasEmittedFinishedSignal(false)
 {
+    connect(this, &GETFileJob::networkError, this, [this] {
+        if (timedOut()) {
+            qCWarning(lcGetJob) << this << "timeout";
+            _errorString = tr("Connection Timeout");
+            _errorStatus = SyncFileItem::FatalError;
+        }
+    });
+
     // Long downloads must not block non-propagation jobs.
     setPriority(QNetworkRequest::LowPriority);
 }
@@ -110,12 +117,23 @@ void GETFileJob::start()
     AbstractNetworkJob::start();
 }
 
+void GETFileJob::finished()
+{
+    if (_bandwidthManager) {
+        _bandwidthManager->unregisterDownloadJob(this);
+    }
+    if (reply()->bytesAvailable() && _httpOk) {
+        // we where throttled, write out the remaining data
+        slotReadyRead();
+        Q_ASSERT(!reply()->bytesAvailable());
+    }
+}
+
 void GETFileJob::newReplyHook(QNetworkReply *reply)
 {
     reply->setReadBufferSize(16 * 1024); // keep low so we can easier limit the bandwidth
 
     connect(reply, &QNetworkReply::metaDataChanged, this, &GETFileJob::slotMetaDataChanged);
-    connect(reply, &QIODevice::readyRead, this, &GETFileJob::slotReadyRead);
     connect(reply, &QNetworkReply::finished, this, &GETFileJob::slotReadyRead);
     connect(reply, &QNetworkReply::downloadProgress, this, &GETFileJob::downloadProgress);
 }
@@ -179,11 +197,12 @@ void GETFileJob::slotMetaDataChanged()
     }
 
     qint64 start = 0;
-    QString ranges = QString::fromUtf8(reply()->rawHeader("Content-Range"));
+    const QString ranges = QString::fromUtf8(reply()->rawHeader("Content-Range"));
     if (!ranges.isEmpty()) {
-        QRegExp rx(QStringLiteral("bytes (\\d+)-"));
-        if (rx.indexIn(ranges) >= 0) {
-            start = rx.cap(1).toLongLong();
+        static QRegularExpression rx(QStringLiteral("bytes (\\d+)-"));
+        const auto match = rx.match(ranges);
+        if (match.hasMatch()) {
+            start = match.captured(1).toLongLong();
         }
     }
     if (start != _resumeStart) {
@@ -210,32 +229,32 @@ void GETFileJob::slotMetaDataChanged()
     if (!lastModified.isNull()) {
         _lastModified = Utility::qDateTimeToTime_t(lastModified.toDateTime());
     }
-
-    _saveBodyToFile = true;
+    _httpOk = true;
+    connect(reply(), &QIODevice::readyRead, this, &GETFileJob::slotReadyRead);
 }
 
-void GETJob::setBandwidthManager(BandwidthManager *bwm)
+void GETFileJob::setBandwidthManager(BandwidthManager *bwm)
 {
     _bandwidthManager = bwm;
 }
 
-void GETJob::setChoked(bool c)
+void GETFileJob::setChoked(bool c)
 {
     _bandwidthChoked = c;
-    QMetaObject::invokeMethod(this, "slotReadyRead", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(this, &GETFileJob::slotReadyRead, Qt::QueuedConnection);
 }
 
-void GETJob::setBandwidthLimited(bool b)
+void GETFileJob::setBandwidthLimited(bool b)
 {
     _bandwidthLimited = b;
-    QMetaObject::invokeMethod(this, "slotReadyRead", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(this, &GETFileJob::slotReadyRead, Qt::QueuedConnection);
 }
 
-void GETJob::giveBandwidthQuota(qint64 q)
+void GETFileJob::giveBandwidthQuota(qint64 q)
 {
     _bandwidthQuota = q;
     qCDebug(lcGetJob) << "Got" << q << "bytes";
-    QMetaObject::invokeMethod(this, "slotReadyRead", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(this, &GETFileJob::slotReadyRead, Qt::QueuedConnection);
 }
 
 qint64 GETFileJob::currentDownloadPosition()
@@ -248,28 +267,31 @@ qint64 GETFileJob::currentDownloadPosition()
 
 void GETFileJob::slotReadyRead()
 {
-    if (!reply())
+    Q_ASSERT(reply());
+    if (!_httpOk) {
         return;
-    int bufferSize = qMin(1024 * 8ll, reply()->bytesAvailable());
+    }
+
+    const qint64 bufferSize = std::min<qint64>(1024 * 8, reply()->bytesAvailable());
     QByteArray buffer(bufferSize, Qt::Uninitialized);
 
-    while (reply()->bytesAvailable() > 0 && _saveBodyToFile) {
+    while (reply()->bytesAvailable() > 0) {
         if (_bandwidthChoked) {
             qCWarning(lcGetJob) << "Download choked";
             break;
         }
         qint64 toRead = bufferSize;
         if (_bandwidthLimited) {
-            toRead = qMin(qint64(bufferSize), _bandwidthQuota);
+            toRead = std::min<qint64>(bufferSize, _bandwidthQuota);
             if (toRead == 0) {
-                qCWarning(lcGetJob) << "Out of quota";
+                qCWarning(lcGetJob) << "Out of badnwidth quota";
                 break;
             }
             _bandwidthQuota -= toRead;
         }
 
-        qint64 r = reply()->read(buffer.data(), toRead);
-        if (r < 0) {
+        const qint64 read = reply()->read(buffer.data(), toRead);
+        if (read < 0) {
             _errorString = networkReplyErrorString(*reply());
             _errorStatus = SyncFileItem::NormalError;
             qCWarning(lcGetJob) << "Error while reading from device: " << _errorString;
@@ -277,52 +299,26 @@ void GETFileJob::slotReadyRead()
             return;
         }
 
-        qint64 w = _device->write(buffer.constData(), r);
-        if (w != r) {
+        const qint64 written = _device->write(buffer.constData(), read);
+        if (written != read) {
             _errorString = _device->errorString();
             _errorStatus = SyncFileItem::NormalError;
-            qCWarning(lcGetJob) << "Error while writing to file" << w << r << _errorString;
+            qCWarning(lcGetJob) << "Error while writing to file" << written << read << _errorString;
             reply()->abort();
             return;
         }
     }
-
-    if (reply()->isFinished() && (reply()->bytesAvailable() == 0 || !_saveBodyToFile)) {
-        qCDebug(lcGetJob) << "Actually finished!";
-        if (_bandwidthManager) {
-            _bandwidthManager->unregisterDownloadJob(this);
-        }
-        if (!_hasEmittedFinishedSignal) {
-            qCInfo(lcGetJob) << "GET of" << reply()->request().url().toString() << "FINISHED WITH STATUS"
-                             << replyStatusString()
-                             << reply()->rawHeader("Content-Range") << reply()->rawHeader("Content-Length");
-
-            emit finishedSignal();
-        }
-        _hasEmittedFinishedSignal = true;
-        deleteLater();
-    }
 }
 
-GETJob::GETJob(AccountPtr account, const QUrl &rootUrl, const QString &path, QObject *parent)
-    : AbstractNetworkJob(account, rootUrl, path, parent)
-{
-    connect(this, &GETJob::networkError, this, [this] {
-        if (timedOut()) {
-            qCWarning(lcGetJob) << this << "timeout";
-            _errorString = tr("Connection Timeout");
-            _errorStatus = SyncFileItem::FatalError;
-        }
-    });
-}
-GETJob::~GETJob()
+
+GETFileJob::~GETFileJob()
 {
     if (_bandwidthManager) {
         _bandwidthManager->unregisterDownloadJob(this);
     }
 }
 
-QString GETJob::errorString() const
+QString GETFileJob::errorString() const
 {
     if (!_errorString.isEmpty()) {
         return _errorString;
@@ -553,7 +549,7 @@ void PropagateDownloadFile::startFullDownload()
             &_tmpFile, headers, _expectedEtagForResume, _resumeStart, this);
     }
     _job->setBandwidthManager(&propagator()->_bandwidthManager);
-    connect(_job.data(), &GETJob::finishedSignal, this, &PropagateDownloadFile::slotGetFinished);
+    connect(_job.data(), &GETFileJob::finishedSignal, this, &PropagateDownloadFile::slotGetFinished);
     connect(qobject_cast<GETFileJob *>(_job.data()), &GETFileJob::downloadProgress,
         this, &PropagateDownloadFile::slotDownloadProgress);
     propagator()->_activeJobList.append(this);
@@ -578,7 +574,7 @@ void PropagateDownloadFile::slotGetFinished()
 {
     propagator()->_activeJobList.removeOne(this);
 
-    GETJob *job = _job;
+    GETFileJob *job = _job;
     OC_ASSERT(job);
 
     _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
@@ -694,8 +690,8 @@ void PropagateDownloadFile::slotGetFinished()
         return;
     }
 
-    if (bodySize > 0 && bodySize != _tmpFile.size() - job->resumeStart()) {
-        qCDebug(lcPropagateDownload) << bodySize << _tmpFile.size() << job->resumeStart();
+    if (bodySize > 0 && (bodySize != (_tmpFile.size() - job->resumeStart()))) {
+        qCDebug(lcPropagateDownload) << bodySize << "!=" << (_tmpFile.size() - job->resumeStart()) << _tmpFile.size() << job->resumeStart();
         propagator()->_anotherSyncNeeded = true;
         done(SyncFileItem::SoftError, tr("The file could not be downloaded completely."));
         return;
diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h
index 939fd5f2e..eb587c7b0 100644
--- a/src/libsync/propagatedownload.h
+++ b/src/libsync/propagatedownload.h
@@ -21,48 +21,11 @@
 
 namespace OCC {
 
-class OWNCLOUDSYNC_EXPORT GETJob : public AbstractNetworkJob
-{
-    Q_OBJECT
-protected:
-    QByteArray _etag;
-    time_t _lastModified = 0;
-    QString _errorString;
-    SyncFileItem::Status _errorStatus = SyncFileItem::NoStatus;
-    bool _bandwidthLimited = false; // if _bandwidthQuota will be used
-    bool _bandwidthChoked = false; // if download is paused (won't read on readyRead())
-    qint64 _bandwidthQuota = 0;
-    QPointer<BandwidthManager> _bandwidthManager = nullptr;
-
-public:
-    GETJob(AccountPtr account, const QUrl &rootUrl, const QString &path, QObject *parent = nullptr);
-
-    ~GETJob() override;
-
-    virtual qint64 currentDownloadPosition() = 0;
-    virtual qint64 resumeStart() { return 0; }
-
-    QByteArray &etag() { return _etag; }
-    time_t lastModified() { return _lastModified; }
-
-    void setErrorString(const QString &s) { _errorString = s; }
-    QString errorString() const;
-    SyncFileItem::Status errorStatus() { return _errorStatus; }
-    void setErrorStatus(const SyncFileItem::Status &s) { _errorStatus = s; }
-    void setBandwidthManager(BandwidthManager *bwm);
-    void setChoked(bool c);
-    void setBandwidthLimited(bool b);
-    void giveBandwidthQuota(qint64 q);
-
-signals:
-    void finishedSignal();
-};
-
 /**
  * @brief Downloads the remote file via GET
  * @ingroup libsync
  */
-class OWNCLOUDSYNC_EXPORT GETFileJob : public GETJob
+class OWNCLOUDSYNC_EXPORT GETFileJob : public AbstractNetworkJob
 {
     Q_OBJECT
     QIODevice *_device;
@@ -71,10 +34,6 @@ class OWNCLOUDSYNC_EXPORT GETFileJob : public GETJob
     qint64 _expectedContentLength;
     qint64 _contentLength;
     qint64 _resumeStart;
-    bool _hasEmittedFinishedSignal;
-
-    /// Will be set to true once we've seen a 2xx response header
-    bool _saveBodyToFile = false;
 
 public:
     // DOES NOT take ownership of the device.
@@ -82,26 +41,16 @@ public:
     explicit GETFileJob(AccountPtr account, const QUrl &url, const QString &path, QIODevice *device,
         const QMap<QByteArray, QByteArray> &headers, const QByteArray &expectedEtagForResume,
         qint64 resumeStart, QObject *parent = nullptr);
+    virtual ~GETFileJob();
 
-    qint64 currentDownloadPosition() override;
+    qint64 currentDownloadPosition();
 
     void start() override;
-    bool finished() override
-    {
-        if (_saveBodyToFile && reply()->bytesAvailable()) {
-            return false;
-        } else {
-            if (!_hasEmittedFinishedSignal) {
-                emit finishedSignal();
-            }
-            _hasEmittedFinishedSignal = true;
-            return true; // discard
-        }
-    }
+    void finished() override;
 
     void newReplyHook(QNetworkReply *reply) override;
 
-    qint64 resumeStart() override
+    qint64 resumeStart()
     {
         return _resumeStart;
     }
@@ -110,12 +59,38 @@ public:
     qint64 expectedContentLength() const { return _expectedContentLength; }
     void setExpectedContentLength(qint64 size) { _expectedContentLength = size; }
 
+    void setChoked(bool c);
+    void setBandwidthLimited(bool b);
+    void giveBandwidthQuota(qint64 q);
+    void setBandwidthManager(BandwidthManager *bwm);
+
+    QByteArray &etag() { return _etag; }
+    time_t lastModified() { return _lastModified; }
+
+    void setErrorString(const QString &s) { _errorString = s; }
+    QString errorString() const;
+    SyncFileItem::Status errorStatus() { return _errorStatus; }
+    void setErrorStatus(const SyncFileItem::Status &s) { _errorStatus = s; }
+
 private slots:
     void slotReadyRead();
     void slotMetaDataChanged();
 
 signals:
     void downloadProgress(qint64, qint64);
+
+protected:
+    bool restartDevice();
+
+    QByteArray _etag;
+    time_t _lastModified = 0;
+    QString _errorString;
+    SyncFileItem::Status _errorStatus = SyncFileItem::NoStatus;
+    bool _bandwidthLimited = false; // if _bandwidthQuota will be used
+    bool _bandwidthChoked = false; // if download is paused (won't read on readyRead())
+    qint64 _bandwidthQuota = 0;
+    bool _httpOk = false;
+    QPointer<BandwidthManager> _bandwidthManager = nullptr;
 };
 
 /**
@@ -217,7 +192,7 @@ private:
 
     qint64 _resumeStart;
     qint64 _downloadProgress;
-    QPointer<GETJob> _job;
+    QPointer<GETFileJob> _job;
     QFile _tmpFile;
     bool _deleteExisting;
     ConflictRecord _conflictRecord;
diff --git a/src/libsync/propagateremotedelete.cpp b/src/libsync/propagateremotedelete.cpp
index 55f1e201b..e286a8e8a 100644
--- a/src/libsync/propagateremotedelete.cpp
+++ b/src/libsync/propagateremotedelete.cpp
@@ -35,13 +35,10 @@ void DeleteJob::start()
     AbstractNetworkJob::start();
 }
 
-bool DeleteJob::finished()
+void DeleteJob::finished()
 {
     qCInfo(lcDeleteJob) << "DELETE of" << reply()->request().url() << "FINISHED WITH STATUS"
                        << replyStatusString();
-
-    emit finishedSignal();
-    return true;
 }
 
 void PropagateRemoteDelete::start()
diff --git a/src/libsync/propagateremotedelete.h b/src/libsync/propagateremotedelete.h
index e8379f492..fc8ede65c 100644
--- a/src/libsync/propagateremotedelete.h
+++ b/src/libsync/propagateremotedelete.h
@@ -29,10 +29,7 @@ public:
     explicit DeleteJob(AccountPtr account, const QUrl &url, const QString &path, QObject *parent = nullptr);
 
     void start() override;
-    bool finished() override;
-
-signals:
-    void finishedSignal();
+    void finished() override;
 };
 
 /**
diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp
index 6e697dc2e..e09f7148a 100644
--- a/src/libsync/propagateremotemove.cpp
+++ b/src/libsync/propagateremotemove.cpp
@@ -48,13 +48,10 @@ void MoveJob::start()
 }
 
 
-bool MoveJob::finished()
+void MoveJob::finished()
 {
     qCInfo(lcMoveJob) << "MOVE of" << reply()->request().url() << "FINISHED WITH STATUS"
                       << replyStatusString();
-
-    emit finishedSignal();
-    return true;
 }
 
 void PropagateRemoteMove::start()
diff --git a/src/libsync/propagateremotemove.h b/src/libsync/propagateremotemove.h
index 898efd48a..d80719234 100644
--- a/src/libsync/propagateremotemove.h
+++ b/src/libsync/propagateremotemove.h
@@ -33,10 +33,7 @@ public:
         HeaderMap _extraHeaders, QObject *parent = nullptr);
 
     void start() override;
-    bool finished() override;
-
-signals:
-    void finishedSignal();
+    void finished() override;
 };
 
 /**
diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 952a91a28..59b9ed33b 100644
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -89,7 +89,7 @@ void PUTFileJob::start()
     AbstractNetworkJob::start();
 }
 
-bool PUTFileJob::finished()
+void PUTFileJob::finished()
 {
     _device->close();
 
@@ -97,9 +97,6 @@ bool PUTFileJob::finished()
                      << replyStatusString()
                      << reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt()
                      << reply()->attribute(QNetworkRequest::HttpReasonPhraseAttribute);
-
-    emit finishedSignal();
-    return true;
 }
 
 void PUTFileJob::newReplyHook(QNetworkReply *reply)
@@ -493,7 +490,7 @@ void PropagateUploadFileCommon::addChildJob(AbstractNetworkJob *job)
 {
     _childJobs.insert(job);
     connect(
-        job, &AbstractNetworkJob::abstractJobFinished, this, [job, this] {
+        job, &AbstractNetworkJob::aboutToFinishedSignal, this, [job, this] {
             _childJobs.erase(job);
         },
         Qt::DirectConnection);
@@ -619,7 +616,9 @@ void PropagateUploadFileCommon::abortNetworkJobs(
     };
 
     // Abort all running jobs, except for explicitly excluded ones
-    for (auto *job : _childJobs) {
+    // perform actions on a copy as aborted jobs will be removed from _childJobs
+    const auto children = _childJobs;
+    for (auto *job : children) {
         auto reply = job->reply();
         if (!reply || !reply->isRunning())
             continue;
diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h
index 24cdf5fde..5c5884bb0 100644
--- a/src/libsync/propagateupload.h
+++ b/src/libsync/propagateupload.h
@@ -106,7 +106,7 @@ public:
 
     void start() override;
 
-    bool finished() override;
+    void finished() override;
 
     QIODevice *device()
     {
@@ -127,7 +127,6 @@ protected:
     void newReplyHook(QNetworkReply *reply) override;
 
 signals:
-    void finishedSignal();
     void uploadProgress(qint64, qint64);
 
 };
diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp
index b43211ef6..5bda3a1dd 100644
--- a/src/libsync/propagateuploadng.cpp
+++ b/src/libsync/propagateuploadng.cpp
@@ -27,13 +27,12 @@
 #include "propagateremotedelete.h"
 #include "common/asserts.h"
 
+#include <QCoreApplication>
 #include <QDir>
 #include <QFileInfo>
 #include <QNetworkAccessManager>
 #include <QRandomGenerator>
 
-#include <cmath>
-#include <cstring>
 #include <memory>
 
 namespace OCC {
@@ -221,6 +220,7 @@ void PropagateUploadFileNG::slotPropfindFinished()
         for (auto it = _serverChunks.begin(); it != _serverChunks.end(); ++it) {
             auto job = new DeleteJob(propagator()->account(), propagator()->account()->url(), chunkPath() + QLatin1Char('/') + it->originalName, this);
             addChildJob(job);
+            connect(job, &DeleteJob::finishedSignal, this, &PropagateUploadFileNG::slotDeleteJobFinished);
             job->start();
         }
         _serverChunks.clear();
@@ -336,8 +336,7 @@ void PropagateUploadFileNG::doFinalMove()
     // Still not finished all ranges.
     if (!_rangesToUpload.isEmpty())
         return;
-
-    Q_ASSERT(childJobs().empty(), "MOVE for upload even though jobs are still running");
+    Q_ASSERT_X(childJobs().empty(), Q_FUNC_INFO, "MOVE for upload even though jobs are still running");
 
     _finished = true;
 
diff --git a/test/testjobqueue.cpp b/test/testjobqueue.cpp
index 395797faf..4ca5262a5 100644
--- a/test/testjobqueue.cpp
+++ b/test/testjobqueue.cpp
@@ -45,9 +45,8 @@ public:
     }
 
 protected:
-    bool finished() override
+    void finished() override
     {
-        return true;
     }
 };
 
diff --git a/test/testutils/syncenginetestutils.cpp b/test/testutils/syncenginetestutils.cpp
index 993496f86..8d4b471bc 100644
--- a/test/testutils/syncenginetestutils.cpp
+++ b/test/testutils/syncenginetestutils.cpp
@@ -1011,7 +1011,7 @@ QString FakeFolder::localPath() const
 void FakeFolder::scheduleSync()
 {
     // Have to be done async, else, an error before exec() does not terminate the event loop.
-    QMetaObject::invokeMethod(_syncEngine.get(), "startSync", Qt::QueuedConnection);
+    QMetaObject::invokeMethod(_syncEngine.get(), &OCC::SyncEngine::startSync, Qt::QueuedConnection);
 }
 
 void FakeFolder::execUntilBeforePropagation()