Propagator: Deal with directories becoming files #4302

Note, in particular the revised order of directory deletion jobs.

src/libsync/filesystem.cpp

@@ -543,4 +543,25 @@ QByteArray FileSystem::calcAdler32( const QString& filename )
 }
 #endif
 
+QString FileSystem::makeConflictFileName(const QString &fn, const QDateTime &dt)
+{
+    QString conflictFileName(fn);
+    // Add _conflict-XXXX  before the extension.
+    int dotLocation = conflictFileName.lastIndexOf('.');
+    // If no extension, add it at the end  (take care of cases like foo/.hidden or foo.bar/file)
+    if (dotLocation <= conflictFileName.lastIndexOf('/') + 1) {
+        dotLocation = conflictFileName.size();
+    }
+    QString timeString = dt.toString("yyyyMMdd-hhmmss");
+
+    // Additional marker
+    QByteArray conflictFileUserName = qgetenv("CSYNC_CONFLICT_FILE_USERNAME");
+    if (conflictFileUserName.isEmpty())
+        conflictFileName.insert(dotLocation, "_conflict-" + timeString);
+    else
+        conflictFileName.insert(dotLocation, "_conflict_" + QString::fromUtf8(conflictFileUserName)  + "-" + timeString);
+
+    return conflictFileName;
+}
+
 } // namespace OCC

src/libsync/filesystem.h

@@ -169,6 +169,11 @@ QByteArray OWNCLOUDSYNC_EXPORT calcSha1( const QString& fileName );
 QByteArray OWNCLOUDSYNC_EXPORT calcAdler32( const QString& fileName );
 #endif
 
+/**
+ * Returns a file name based on \a fn that's suitable for a conflict.
+ */
+QString OWNCLOUDSYNC_EXPORT makeConflictFileName(const QString &fn, const QDateTime &dt);
+
 }
 
 /** @} */

src/libsync/owncloudpropagator.cpp

@@ -346,7 +346,7 @@ void OwncloudPropagator::start(const SyncFileItemVector& items)
             if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) {
                 // We do the removal of directories at the end, because there might be moves from
                 // these directories that will happen later.
-                directoriesToRemove.append(dir);
+                directoriesToRemove.prepend(dir);
                 removedDirectory = item->_file + "/";
 
                 // We should not update the etag of parent directories of the removed directory
@@ -362,7 +362,26 @@ void OwncloudPropagator::start(const SyncFileItemVector& items)
             }
             directories.push(qMakePair(item->destination() + "/" , dir));
         } else if (PropagateItemJob* current = createJob(item)) {
-            directories.top().second->append(current);
+            // If the target of a job is currently a directory, we need to remove it!
+            // This can happen when what used to be a directory changed to a file on the
+            // server an a PropagateLocalRename or PropageDownload job wants to run.
+            if (item->_direction == SyncFileItem::Down
+                    && QFileInfo(getFilePath(item->_file)).isDir()) {
+                // The DirectoryConflict job *must* run before the file propagation job
+                // and we also need to make sure other jobs that deal with the files
+                // in the directory (like removes or moves, in particular of other
+                // directories!) run first.
+                // Making it a directory job ensures that moves run first and that the
+                // (potential) directory rename happens before the file propagation.
+                // Prepending all jobs to directoriesToRemove ensures that removals of
+                // subdirectories happen before the directory is renamed.
+                PropagateDirectory *dir = new PropagateDirectory(this, item);
+                dir->_firstJob.reset(new PropagateLocalDirectoryConflict(this, item));
+                dir->append(current);
+                directoriesToRemove.prepend(dir);
+            } else {
+                directories.top().second->append(current);
+            }
         }
     }
 

src/libsync/owncloudpropagator.h

@@ -73,12 +73,16 @@ public:
 
         /** Jobs can be run in parallel to this job */
         FullParallelism,
-        /** This job does not support parallelism, and no other job shall
-            be started until this one has finished */
+
+        /** No other job shall be started until this one has finished.
+            So this job is guaranteed to finish before any jobs below it
+            are executed. */
         WaitForFinished,
 
-        /** This job supports parallelism with other jobs in the same directory, but it should
-             not be parallelized with jobs in other directories  (typically a move operation) */
+        /** A job with this parallelism will allow later jobs to start and
+            run in parallel as long as they aren't PropagateDirectory jobs.
+            When the first directory job is encountered, no further jobs
+            will be started until this one is finished. */
         WaitForFinishedInParentDirectory
     };
 

src/libsync/propagatedownload.cpp

@@ -551,28 +551,6 @@ void PropagateDownloadFileQNAM::slotChecksumFail( const QString& errMsg )
     done(SyncFileItem::SoftError, errMsg ); // tr("The file downloaded with a broken checksum, will be redownloaded."));
 }
 
-QString makeConflictFileName(const QString &fn, const QDateTime &dt)
-{
-    QString conflictFileName(fn);
-    // Add _conflict-XXXX  before the extension.
-    int dotLocation = conflictFileName.lastIndexOf('.');
-    // If no extension, add it at the end  (take care of cases like foo/.hidden or foo.bar/file)
-    if (dotLocation <= conflictFileName.lastIndexOf('/') + 1) {
-        dotLocation = conflictFileName.size();
-    }
-    QString timeString = dt.toString("yyyyMMdd-hhmmss");
-
-    // Additional marker
-    QByteArray conflictFileUserName = qgetenv("CSYNC_CONFLICT_FILE_USERNAME");
-    if (conflictFileUserName.isEmpty())
-        conflictFileName.insert(dotLocation, "_conflict-" + timeString);
-    else
-        conflictFileName.insert(dotLocation, "_conflict_" + QString::fromUtf8(conflictFileUserName)  + "-" + timeString);
-
-    return conflictFileName;
-}
-
-
 namespace { // Anonymous namespace for the recall feature
 static QString makeRecallFileName(const QString &fn)
 {
@@ -636,7 +614,7 @@ void PropagateDownloadFileQNAM::downloadFinished()
             && !FileSystem::fileEquals(fn, _tmpFile.fileName());
     if (isConflict) {
         QString renameError;
-        QString conflictFileName = makeConflictFileName(fn, Utility::qDateTimeFromTime_t(_item->_modtime));
+        QString conflictFileName = FileSystem::makeConflictFileName(fn, Utility::qDateTimeFromTime_t(_item->_modtime));
         if (!FileSystem::rename(fn, conflictFileName, &renameError)) {
             //If the rename fails, don't replace it.
             done(SyncFileItem::SoftError, renameError);

src/libsync/propagatorjobs.cpp

@@ -256,4 +256,39 @@ void PropagateLocalRename::start()
     done(SyncFileItem::Success);
 }
 
+void PropagateLocalDirectoryConflict::start()
+{
+    if (_propagator->_abortRequested.fetchAndAddRelaxed(0))
+        return;
+
+    QString existingDir = _propagator->getFilePath(_item->_file);
+    if (!QFileInfo(existingDir).isDir()) {
+        done(SyncFileItem::Success);
+        return;
+    }
+
+    // Delete the directory if it is empty!
+    QDir dir(existingDir);
+    if (dir.entryList(QDir::NoDotAndDotDot|QDir::AllEntries).count() == 0) {
+        if (dir.rmdir(existingDir)) {
+            done(SyncFileItem::Success);
+            return;
+        }
+        // on error, just try to move it away...
+    }
+
+    QString conflictDir = FileSystem::makeConflictFileName(
+            existingDir, Utility::qDateTimeFromTime_t(_item->_modtime));
+
+    _propagator->addTouchedFile(existingDir);
+    _propagator->addTouchedFile(conflictDir);
+    QString renameError;
+    if (!FileSystem::rename(existingDir, conflictDir, &renameError)) {
+        done(SyncFileItem::NormalError, renameError);
+        return;
+    }
+
+    done(SyncFileItem::Success);
+}
+
 }

src/libsync/propagatorjobs.h

@@ -82,5 +82,24 @@ public:
     JobParallelism parallelism() Q_DECL_OVERRIDE { return WaitForFinishedInParentDirectory; }
 };
 
+/**
+ * Moves away a local directory when it should become a file.
+ *
+ * Example: Locally there's directory foo/ with three files in it,
+ *   one of them ignored, while the server has a file foo.
+ *   In this case, foo/ fill be moved to foo-conflict.../ and the
+ *   file will be downloaded to foo.
+ *
+ * If the directory is empty, it will be removed instead.
+ *
+ * @ingroup libsync
+ */
+class PropagateLocalDirectoryConflict : public PropagateItemJob {
+    Q_OBJECT
+public:
+    PropagateLocalDirectoryConflict (OwncloudPropagator* propagator, const SyncFileItemPtr& item) : PropagateItemJob(propagator, item) {}
+    void start() Q_DECL_OVERRIDE;
+    JobParallelism parallelism() Q_DECL_OVERRIDE { return WaitForFinishedInParentDirectory; }
+};
 
 }