Parallel chunk upload: Fix a off-by-one while saving the done chunk in the DB

We should assume that the chunk currently uploading is NOT done (hence the -1)

il task 405

cherry pick from commit 3dd8ce08b8

src/mirall/bandwidthmanager.cpp

@@ -117,6 +117,7 @@ void BandwidthManager::unregisterUploadDevice(UploadDevice* p)
     _relativeUploadDeviceList.removeAll(p);
     if (p == _relativeLimitCurrentMeasuredDevice) {
         _relativeLimitCurrentMeasuredDevice = 0;
+        _relativeUploadLimitProgressAtMeasuringRestart = 0;
     }
 }
 
@@ -141,6 +142,10 @@ void BandwidthManager::registerDownloadJob(GETFileJob* j)
 void BandwidthManager::unregisterDownloadJob(GETFileJob* j)
 {
     _downloadJobList.removeAll(j);
+    if (_relativeLimitCurrentMeasuredJob == j) {
+        _relativeLimitCurrentMeasuredJob = 0;
+        _relativeDownloadLimitProgressAtMeasuringRestart = 0;
+    }
 }
 
 void BandwidthManager::unregisterDownloadJob(QObject* o)

src/mirall/networkjobs.cpp

@@ -31,6 +31,7 @@
 
 #include "mirall/networkjobs.h"
 #include "mirall/account.h"
+#include "mirall/owncloudpropagator.h"
 
 #include "creds/credentialsfactory.h"
 #include "creds/abstractcredentials.h"
@@ -48,7 +49,7 @@ AbstractNetworkJob::AbstractNetworkJob(Account *account, const QString &path, QO
     , _path(path)
 {
     _timer.setSingleShot(true);
-    _timer.setInterval(10*1000); // default to 10 seconds.
+    _timer.setInterval(OwncloudPropagator::httpTimeout() * 1000); // default to 5 minutes.
     connect(&_timer, SIGNAL(timeout()), this, SLOT(slotTimeout()));
 }
 

src/mirall/owncloudpropagator.h

@@ -46,10 +46,8 @@ class PropagatorJob : public QObject {
 protected:
     OwncloudPropagator *_propagator;
     void emitReady() {
-        bool wasReady = _readySent;
         _readySent = true;
-        if (!wasReady)
-            emit ready();
+        emit ready();
     };
 public:
     bool _readySent;
@@ -191,7 +189,7 @@ public:
     qint64 _read;
     qint64 _size;
     qint64 _start;
-    BandwidthManager* _bandwidthManager;
+    QPointer<BandwidthManager> _bandwidthManager;
 
     qint64 _bandwidthQuota;
     qint64 _readWithProgress;

src/mirall/propagator_qnam.cpp

@@ -56,6 +56,11 @@ static SyncFileItem::Status classifyError(QNetworkReply::NetworkError nerror, in
         return SyncFileItem::SoftError;
     }
 
+    if (httpCode == 423) {
+        // Locked
+        return SyncFileItem::SoftError;
+    }
+
     return SyncFileItem::NormalError;
 }
 
@@ -198,7 +203,9 @@ UploadDevice::UploadDevice(QIODevice *file,  qint64 start, qint64 size, Bandwidt
 
 
 UploadDevice::~UploadDevice() {
-    _bandwidthManager->unregisterUploadDevice(this);
+    if (_bandwidthManager) {
+        _bandwidthManager->unregisterUploadDevice(this);
+    }
 }
 
 qint64 UploadDevice::writeData(const char* , qint64 ) {
@@ -385,14 +392,13 @@ void PropagateUploadFileQNAM::startNextChunk()
     if( isOpen ) {
         PUTFileJob* job = new PUTFileJob(AccountManager::instance()->account(), _propagator->_remoteFolder + path, device, headers, _currentChunk);
         _jobs.append(job);
-        job->setTimeout(_propagator->httpTimeout() * 1000);
+        _currentChunk++;
         connect(job, SIGNAL(finishedSignal()), this, SLOT(slotPutFinished()));
         connect(job, SIGNAL(uploadProgress(qint64,qint64)), this, SLOT(slotUploadProgress(qint64,qint64)));
         connect(job, SIGNAL(uploadProgress(qint64,qint64)), device, SLOT(slotJobUploadProgress(qint64,qint64)));
         connect(job, SIGNAL(destroyed(QObject*)), this, SLOT(slotJobDestroyed(QObject*)));
         job->start();
         _propagator->_activeJobs++;
-        _currentChunk++;
 
         QByteArray env = qgetenv("OWNCLOUD_PARALLEL_CHUNK");
         bool parallelChunkUpload = env=="true" || env =="1";;
@@ -450,7 +456,11 @@ void PropagateUploadFileQNAM::slotPutFinished()
         qDebug() << replyContent; // display the XML error in the debug
         QRegExp rx("<s:message>(.*)</s:message>"); // Issue #1366: display server exception
         if (rx.indexIn(QString::fromUtf8(replyContent)) != -1) {
-            errorString += QLatin1String(" (") + rx.cap(1) + QLatin1Char(')');
+            if (_item._httpErrorCode == 423) {
+                errorString = rx.cap(1); // For "Locked", the library user doesn't want all the stuff except the <s:message> contents
+            } else {
+                errorString += QLatin1String(" (") + rx.cap(1) + QLatin1Char(')');
+            }
         }
 
         if (_item._httpErrorCode == 412) {
@@ -512,7 +522,7 @@ void PropagateUploadFileQNAM::slotPutFinished()
         auto currentChunk = job->_chunk;
         foreach (auto *job, _jobs) {
             // Take the minimum finished one
-            currentChunk = qMin(currentChunk, job->_chunk);
+            currentChunk = qMin(currentChunk, job->_chunk - 1);
         }
         pi._chunk = (currentChunk + _startChunk + 1) % _chunkCount ; // next chunk to start with
         pi._transferid = _transferId;
@@ -575,12 +585,12 @@ void PropagateUploadFileQNAM::slotUploadProgress(qint64 sent, qint64 t)
     quint64 amount = progressChunk * chunkSize();
     sender()->setProperty("byteWritten", sent);
     if (_jobs.count() > 1) {
-        amount += sent;
-    } else {
         amount -= (_jobs.count() -1) * chunkSize();
         foreach (QObject *j, _jobs) {
             amount += j->property("byteWritten").toULongLong();
         }
+    } else {
+        amount += sent;
     }
     emit progress(_item, amount);
 }
@@ -631,7 +641,7 @@ void PropagateUploadFileQNAM::abort()
 
 // DOES NOT take owncership of the device.
 GETFileJob::GETFileJob(Account* account, const QString& path, QFile *device,
-                    const QMap<QByteArray, QByteArray> &headers, QByteArray expectedEtagForResume,
+                    const QMap<QByteArray, QByteArray> &headers, const QByteArray &expectedEtagForResume,
                     quint64 _resumeStart,  QObject* parent)
 : AbstractNetworkJob(account, path, parent),
   _device(device), _headers(headers), _expectedEtagForResume(expectedEtagForResume),
@@ -642,10 +652,10 @@ GETFileJob::GETFileJob(Account* account, const QString& path, QFile *device,
 }
 
 GETFileJob::GETFileJob(Account* account, const QUrl& url, QFile *device,
-                    const QMap<QByteArray, QByteArray> &headers,
-                    QObject* parent)
+                       const QMap<QByteArray, QByteArray> &headers, const QByteArray &expectedEtagForResume,
+                       quint64 resumeStart, QObject* parent)
 : AbstractNetworkJob(account, url.toEncoded(), parent),
-  _device(device), _headers(headers), _resumeStart(0),
+  _device(device), _headers(headers), _expectedEtagForResume(expectedEtagForResume), _resumeStart(resumeStart),
   _errorStatus(SyncFileItem::NoStatus), _directDownloadUrl(url)
 , _bandwidthLimited(false), _bandwidthChoked(false), _bandwidthQuota(0), _bandwidthManager(0)
 , _hasEmittedFinishedSignal(false)
@@ -928,11 +938,10 @@ void PropagateDownloadFileQNAM::start()
         QUrl url = QUrl::fromUserInput(_item._directDownloadUrl);
         _job = new GETFileJob(AccountManager::instance()->account(),
                               url,
-                              &_tmpFile, headers);
+                              &_tmpFile, headers, expectedEtagForResume, _startSize);
         qDebug() << Q_FUNC_INFO << "directDownloadUrl given for " << _item._file << _item._directDownloadUrl << headers["Cookie"];
     }
     _job->setBandwidthManager(&_propagator->_bandwidthManager);
-    _job->setTimeout(_propagator->httpTimeout() * 1000);
     connect(_job, SIGNAL(finishedSignal()), this, SLOT(slotGetFinished()));
     connect(_job, SIGNAL(downloadProgress(qint64,qint64)), this, SLOT(slotDownloadProgress(qint64,qint64)));
     _propagator->_activeJobs ++;

src/mirall/propagator_qnam.h

@@ -147,18 +147,18 @@ class GETFileJob : public AbstractNetworkJob {
     bool _bandwidthLimited; // if _bandwidthQuota will be used
     bool _bandwidthChoked; // if download is paused (won't read on readyRead())
     qint64 _bandwidthQuota;
-    BandwidthManager *_bandwidthManager;
+    QPointer<BandwidthManager> _bandwidthManager;
     bool _hasEmittedFinishedSignal;
 public:
 
     // DOES NOT take owncership of the device.
-    explicit GETFileJob(Account* account, const QString& path, QFile *device,
-                        const QMap<QByteArray, QByteArray> &headers, QByteArray expectedEtagForResume,
-                        quint64 resumeStart, QObject* parent = 0);
+    explicit GETFileJob(Mirall::Account* account, const QString& path, QFile* device,
+                        const QMap<QByteArray, QByteArray> &headers, const QByteArray& expectedEtagForResume,
+                        quint64 _resumeStart, QObject* parent = 0);
     // For directDownloadUrl:
-    explicit GETFileJob(Account* account, const QUrl& url, QFile *device,
-                        const QMap<QByteArray, QByteArray> &headers,
-                        QObject* parent = 0);
+    explicit GETFileJob(Mirall::Account* account, const QUrl& url, QFile* device,
+                        const QMap<QByteArray, QByteArray> &headers, const QByteArray& expectedEtagForResume,
+                        quint64 resumeStart, QObject* parent = 0);
     virtual ~GETFileJob() {
         if (_bandwidthManager) {
             _bandwidthManager->unregisterDownloadJob(this);

src/mirall/propagatorjobs.cpp

@@ -352,7 +352,7 @@ bool PropagateNeonJob::updateErrorFromSession(int neon_code, ne_request* req, in
             if (ignoreHttpCode && httpStatusCode == ignoreHttpCode)
                 return false;
 
-            done(SyncFileItem::NormalError, errorString);
+            done((httpStatusCode != 423) ? SyncFileItem::NormalError : SyncFileItem::SoftError, errorString);
             return true;
         case NE_LOOKUP:  /* Server or proxy hostname lookup failed */
         case NE_AUTH:     /* User authentication failed on server */