Delete finished propagation jobs in PropagateDirectory #5269 (#5400)

owncloud · Jan 26, 2017 · c3ae512 · c3ae512
1 parent a1a5111
commit c3ae512
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 27 deletions.
diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
@@ -588,17 +588,12 @@ OwncloudPropagator *PropagatorJob::propagator() const
 PropagatorJob::JobParallelism PropagateDirectory::parallelism()
 {
     // If any of the non-finished sub jobs is not parallel, we have to wait
-
-    // FIXME!  we should probably cache this result
-
-    if (_firstJob && _firstJob->_state != Finished) {
-        if (_firstJob->parallelism() != FullParallelism)
-            return WaitForFinished;
+    if (_firstJob && _firstJob->parallelism() != FullParallelism) {
+        return WaitForFinished;
     }
 
-    // FIXME: use the cached value of finished job
     for (int i = 0; i < _subJobs.count(); ++i) {
-        if (_subJobs.at(i)->_state != Finished && _subJobs.at(i)->parallelism() != FullParallelism) {
+        if (_subJobs.at(i)->parallelism() != FullParallelism) {
             return WaitForFinished;
         }
     }
@@ -629,15 +624,8 @@ bool PropagateDirectory::scheduleNextJob()
         return false;
     }
 
-    // cache the value of first unfinished subjob
     bool stopAtDirectory = false;
-    int i = _firstUnfinishedSubJob;
-    int subJobsCount = _subJobs.count();
-    while (i < subJobsCount && _subJobs.at(i)->_state == Finished) {
-      _firstUnfinishedSubJob = ++i;
-    }
-
-    for (int i = _firstUnfinishedSubJob; i < subJobsCount; ++i) {
+    for (int i = 0; i < _subJobs.size(); ++i) {
         if (_subJobs.at(i)->_state == Finished) {
             continue;
         }
@@ -665,8 +653,22 @@ bool PropagateDirectory::scheduleNextJob()
 
 void PropagateDirectory::slotSubJobFinished(SyncFileItem::Status status)
 {
+    PropagatorJob *subJob = static_cast<PropagatorJob *>(sender());
+
+    // Delete the job and remove it from our list of jobs.
+    subJob->deleteLater();
+    bool wasFirstJob = false;
+    if (subJob == _firstJob.data()) {
+        wasFirstJob = true;
+        _firstJob.reset();
+    } else {
+        int i = _subJobs.indexOf(subJob);
+        Q_ASSERT(i >= 0);
+        _subJobs.remove(i);
+    }
+
     if (status == SyncFileItem::FatalError ||
-            (sender() == _firstJob.data() && status != SyncFileItem::Success && status != SyncFileItem::Restoration)) {
+            (wasFirstJob && status != SyncFileItem::Success && status != SyncFileItem::Restoration)) {
         abort();
         _state = Finished;
         emit finished(status);
@@ -675,16 +677,10 @@ void PropagateDirectory::slotSubJobFinished(SyncFileItem::Status status)
         _hasError = status;
     }
     _runningNow--;
-    _jobsFinished++;
-
-    int totalJobs = _subJobs.count();
-    if (_firstJob) {
-        totalJobs++;
-    }
 
     // We finished processing all the jobs
     // check if we finished
-    if (_jobsFinished >= totalJobs) {
+    if (!_firstJob && _subJobs.isEmpty()) {
         Q_ASSERT(!_runningNow); // how can we be finished if there are still jobs running now
         finalize();
     } else {

diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h
@@ -192,14 +192,12 @@ class OWNCLOUDSYNC_EXPORT PropagateDirectory : public PropagatorJob {
 
     SyncFileItemPtr _item;
 
-    int _jobsFinished; // number of jobs that have completed
     int _runningNow; // number of subJobs running right now
     SyncFileItem::Status _hasError;  // NoStatus,  or NormalError / SoftError if there was an error
-    int _firstUnfinishedSubJob;
 
     explicit PropagateDirectory(OwncloudPropagator *propagator, const SyncFileItemPtr &item = SyncFileItemPtr(new SyncFileItem))
         : PropagatorJob(propagator)
-        , _firstJob(0), _item(item),  _jobsFinished(0), _runningNow(0), _hasError(SyncFileItem::NoStatus), _firstUnfinishedSubJob(0)
+        , _item(item), _runningNow(0), _hasError(SyncFileItem::NoStatus)
     { }
 
     virtual ~PropagateDirectory() {

diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
@@ -213,6 +213,17 @@ private slots:
         }
     }
 
+    void abortAfterFailedMkdir() {
+        FakeFolder fakeFolder{FileInfo{}};
+        QSignalSpy finishedSpy(&fakeFolder.syncEngine(), SIGNAL(finished(bool)));
+        fakeFolder.serverErrorPaths().append("NewFolder");
+        fakeFolder.localModifier().mkdir("NewFolder");
+        // This should be aborted and would otherwise fail in FileInfo::create.
+        fakeFolder.localModifier().insert("NewFolder/NewFile");
+        fakeFolder.syncOnce();
+        QCOMPARE(finishedSpy.size(), 1);
+        QCOMPARE(finishedSpy.first().first().toBool(), false);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncEngine)