Sync: Deal with file/folder conflicts #6312

Previously conflicts with a different type on both ends lead to sync
errors. Now they are handled in the expected way: the local item gets
renamed and the remote item gets propagated downwards.

This also adds a unittest for the TYPE_CHANGE case. That one looks like
parts of it might be unified with CONFLICT cases.

6 files changed, 346 insertions, 74 deletions

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index d960b4a2d..e9e4df3ea 100644
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -22,6 +22,7 @@
 #include "propagateremotemove.h"
 #include "propagateremotemkdir.h"
 #include "propagatorjobs.h"
+#include "filesystem.h"
 #include "common/utility.h"
 #include "account.h"
 #include "common/asserts.h"
@@ -368,8 +369,10 @@ PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item)
             return new PropagateRemoteDelete(this, item);
     case CSYNC_INSTRUCTION_NEW:
     case CSYNC_INSTRUCTION_TYPE_CHANGE:
+    case CSYNC_INSTRUCTION_CONFLICT:
         if (item->isDirectory()) {
-            if (item->_direction == SyncFileItem::Down) {
+            // CONFLICT has _direction == None
+            if (item->_direction != SyncFileItem::Up) {
                 auto job = new PropagateLocalMkdir(this, item);
                 job->setDeleteExistingFile(deleteExisting);
                 return job;
@@ -380,7 +383,6 @@ PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item)
             }
         } //fall through
     case CSYNC_INSTRUCTION_SYNC:
-    case CSYNC_INSTRUCTION_CONFLICT:
         if (item->_direction != SyncFileItem::Up) {
             auto job = new PropagateDownloadFile(this, item);
             job->setDeleteExistingFolder(deleteExisting);
@@ -431,6 +433,7 @@ void OwncloudPropagator::start(const SyncFileItemVector &items)
     directories.push(qMakePair(QString(), _rootJob.data()));
     QVector<PropagatorJob *> directoriesToRemove;
     QString removedDirectory;
+    QString maybeConflictDirectory;
     foreach (const SyncFileItemPtr &item, items) {
         if (!removedDirectory.isEmpty() && item->_file.startsWith(removedDirectory)) {
             // this is an item in a directory which is going to be removed.
@@ -464,6 +467,20 @@ void OwncloudPropagator::start(const SyncFileItemVector &items)
             }
         }
 
+        // If a CONFLICT item contains files these can't be processed because
+        // the conflict handling is likely to rename the directory. This can happen
+        // when there's a new local directory at the same time as a remote file.
+        if (!maybeConflictDirectory.isEmpty()) {
+            if (item->destination().startsWith(maybeConflictDirectory)) {
+                qCInfo(lcPropagator) << "Skipping job inside CONFLICT directory"
+                                     << item->_file << item->_instruction;
+                item->_instruction = CSYNC_INSTRUCTION_NONE;
+                continue;
+            } else {
+                maybeConflictDirectory.clear();
+            }
+        }
+
         while (!item->destination().startsWith(directories.top().first)) {
             directories.pop();
         }
@@ -513,6 +530,12 @@ void OwncloudPropagator::start(const SyncFileItemVector &items)
             } else {
                 directories.top().second->appendTask(item);
             }
+
+            if (item->_instruction == CSYNC_INSTRUCTION_CONFLICT) {
+                // This might be a file or a directory on the local side. If it's a
+                // directory we want to skip processing items inside it.
+                maybeConflictDirectory = item->_file + "/";
+            }
         }
     }
 
@@ -702,6 +725,72 @@ OwncloudPropagator::DiskSpaceResult OwncloudPropagator::diskSpaceCheck() const
     return DiskSpaceOk;
 }
 
+bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item,
+    PropagatorCompositeJob *composite, QString *error)
+{
+    QString fn = getFilePath(item->_file);
+
+    QString renameError;
+    auto conflictModTime = FileSystem::getModTime(fn);
+    QString conflictFileName = Utility::makeConflictFileName(
+        item->_file, Utility::qDateTimeFromTime_t(conflictModTime));
+    QString conflictFilePath = getFilePath(conflictFileName);
+
+    emit touchedFile(fn);
+    emit touchedFile(conflictFilePath);
+
+    if (!FileSystem::rename(fn, conflictFilePath, &renameError)) {
+        // If the rename fails, don't replace it.
+
+        // If the file is locked, we want to retry this sync when it
+        // becomes available again.
+        if (FileSystem::isFileLocked(fn)) {
+            emit seenLockedFile(fn);
+        }
+
+        if (error)
+            *error = renameError;
+        return false;
+    }
+    qCInfo(lcPropagator) << "Created conflict file" << fn << "->" << conflictFileName;
+
+    // Create a new conflict record. To get the base etag, we need to read it from the db.
+    ConflictRecord conflictRecord;
+    conflictRecord.path = conflictFileName.toUtf8();
+    conflictRecord.baseModtime = item->_previousModtime;
+
+    SyncJournalFileRecord baseRecord;
+    if (_journal->getFileRecord(item->_originalFile, &baseRecord) && baseRecord.isValid()) {
+        conflictRecord.baseEtag = baseRecord._etag;
+        conflictRecord.baseFileId = baseRecord._fileId;
+    } else {
+        // We might very well end up with no fileid/etag for new/new conflicts
+    }
+
+    _journal->setConflictRecord(conflictRecord);
+
+    // Create a new upload job if the new conflict file should be uploaded
+    if (account()->capabilities().uploadConflictFiles()) {
+        if (composite && !QFileInfo(conflictFilePath).isDir()) {
+            SyncFileItemPtr conflictItem = SyncFileItemPtr(new SyncFileItem);
+            conflictItem->_file = conflictFileName;
+            conflictItem->_type = ItemTypeFile;
+            conflictItem->_direction = SyncFileItem::Up;
+            conflictItem->_instruction = CSYNC_INSTRUCTION_NEW;
+            conflictItem->_modtime = conflictModTime;
+            conflictItem->_size = item->_previousSize;
+            emit newItem(conflictItem);
+            composite->appendTask(conflictItem);
+        } else {
+            // Directories we can't process in one go. The next sync run
+            // will take care of uploading the conflict dir contents.
+            _anotherSyncNeeded = true;
+        }
+    }
+
+    return true;
+}
+
 // ================================================================================
 
 PropagatorJob::PropagatorJob(OwncloudPropagator *propagator)
@@ -741,7 +830,7 @@ void PropagatorCompositeJob::slotSubJobAbortFinished()
 
 void PropagatorCompositeJob::appendJob(PropagatorJob *job)
 {
-    job->setCompositeParent(this);
+    job->setAssociatedComposite(this);
     _jobsToDo.append(job);
 }
 
@@ -858,6 +947,7 @@ PropagateDirectory::PropagateDirectory(OwncloudPropagator *propagator, const Syn
 {
     if (_firstJob) {
         connect(_firstJob.data(), &PropagatorJob::finished, this, &PropagateDirectory::slotFirstJobFinished);
+        _firstJob->setAssociatedComposite(&_subJobs);
     }
     connect(&_subJobs, &PropagatorJob::finished, this, &PropagateDirectory::slotSubJobsFinished);
 }
@@ -901,7 +991,9 @@ void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status)
 {
     _firstJob.take()->deleteLater();
 
-    if (status != SyncFileItem::Success && status != SyncFileItem::Restoration) {
+    if (status != SyncFileItem::Success
+        && status != SyncFileItem::Restoration
+        && status != SyncFileItem::Conflict) {
         if (_state != Finished) {
             // Synchronously abort
             abort(AbortType::Synchronous);
diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h
index 7cd00531d..88bb2f1f7 100644
--- a/src/libsync/owncloudpropagator.h
+++ b/src/libsync/owncloudpropagator.h
@@ -103,11 +103,13 @@ public:
      */
     virtual qint64 committedDiskSpace() const { return 0; }
 
-    /** Set the composite parent job
+    /** Set the associated composite job
      *
-     * Used only from PropagatorCompositeJob itself, when a job is added.
+     * Used only from PropagatorCompositeJob itself, when a job is added
+     * and from PropagateDirectory to associate the subJobs with the first
+     * job.
      */
-    void setCompositeParent(PropagatorCompositeJob *job) { _compositeParent = job; }
+    void setAssociatedComposite(PropagatorCompositeJob *job) { _associatedComposite = job; }
 
 public slots:
     /*
@@ -138,10 +140,13 @@ protected:
 
     /** If this job gets added to a composite job, this will point to the parent.
      *
+     * For the PropagateDirectory::_firstJob it will point to
+     * PropagateDirectory::_subJobs.
+     *
      * That can be useful for jobs that want to spawn follow-up jobs without
      * becoming composite jobs themselves.
      */
-    PropagatorCompositeJob *_compositeParent = nullptr;
+    PropagatorCompositeJob *_associatedComposite = nullptr;
 };
 
 /*
@@ -492,6 +497,18 @@ public:
      */
     DiskSpaceResult diskSpaceCheck() const;
 
+    /** Handles a conflict by renaming the file 'item'.
+     *
+     * Sets up conflict records.
+     *
+     * It also creates a new upload job in composite if the item that's
+     * moved away is a file and conflict uploads are requested.
+     *
+     * Returns true on success, false and error on error.
+     */
+    bool createConflict(const SyncFileItemPtr &item,
+        PropagatorCompositeJob *composite, QString *error);
+
 private slots:
 
     void abortTimeout()
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 2b7bd7640..0601e9fba 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -697,14 +697,9 @@ void PropagateDownloadFile::deleteExistingFolder()
         // on error, just try to move it away...
     }
 
-    QString conflictDir = Utility::makeConflictFileName(
-        existingDir, Utility::qDateTimeFromTime_t(FileSystem::getModTime(existingDir)));
-
-    emit propagator()->touchedFile(existingDir);
-    emit propagator()->touchedFile(conflictDir);
-    QString renameError;
-    if (!FileSystem::rename(existingDir, conflictDir, &renameError)) {
-        done(SyncFileItem::NormalError, renameError);
+    QString error;
+    if (!propagator()->createConflict(_item, _associatedComposite, &error)) {
+        done(SyncFileItem::NormalError, error);
     }
 }
 
@@ -819,58 +814,14 @@ void PropagateDownloadFile::downloadFinished()
         return;
     }
 
-    // In case of conflict, make a backup of the old file
-    // Ignore conflicts where both files are binary equal
     bool isConflict = _item->_instruction == CSYNC_INSTRUCTION_CONFLICT
-        && !FileSystem::fileEquals(fn, _tmpFile.fileName());
+        && (QFileInfo(fn).isDir() || !FileSystem::fileEquals(fn, _tmpFile.fileName()));
     if (isConflict) {
-        QString renameError;
-        auto conflictModTime = FileSystem::getModTime(fn);
-        QString conflictFileName = Utility::makeConflictFileName(
-            _item->_file, Utility::qDateTimeFromTime_t(conflictModTime));
-        QString conflictFilePath = propagator()->getFilePath(conflictFileName);
-        if (!FileSystem::rename(fn, conflictFilePath, &renameError)) {
-            // If the rename fails, don't replace it.
-
-            // If the file is locked, we want to retry this sync when it
-            // becomes available again.
-            if (FileSystem::isFileLocked(fn)) {
-                emit propagator()->seenLockedFile(fn);
-            }
-
-            done(SyncFileItem::SoftError, renameError);
+        QString error;
+        if (!propagator()->createConflict(_item, _associatedComposite, &error)) {
+            done(SyncFileItem::SoftError, error);
             return;
         }
-        qCInfo(lcPropagateDownload) << "Created conflict file" << fn << "->" << conflictFileName;
-
-        // Create a new conflict record. To get the base etag, we need to read it from the db.
-        ConflictRecord conflictRecord;
-        conflictRecord.path = conflictFileName.toUtf8();
-        conflictRecord.baseModtime = _item->_previousModtime;
-
-        SyncJournalFileRecord baseRecord;
-        if (propagator()->_journal->getFileRecord(_item->_originalFile, &baseRecord) && baseRecord.isValid()) {
-            conflictRecord.baseEtag = baseRecord._etag;
-            conflictRecord.baseFileId = baseRecord._fileId;
-        } else {
-            // We might very well end up with no fileid/etag for new/new conflicts
-        }
-
-        propagator()->_journal->setConflictRecord(conflictRecord);
-
-        // Create a new upload job if the new conflict file should be uploaded
-        if (propagator()->account()->capabilities().uploadConflictFiles()) {
-            SyncFileItemPtr conflictItem = SyncFileItemPtr(new SyncFileItem);
-            conflictItem->_file = conflictFileName;
-            conflictItem->_type = ItemTypeFile;
-            conflictItem->_direction = SyncFileItem::Up;
-            conflictItem->_instruction = CSYNC_INSTRUCTION_NEW;
-            conflictItem->_modtime = conflictModTime;
-            conflictItem->_size = _item->_previousSize;
-            ASSERT(_compositeParent);
-            emit propagator()->newItem(conflictItem);
-            _compositeParent->appendTask(conflictItem);
-        }
     }
 
     FileSystem::setModTime(_tmpFile.fileName(), _item->_modtime);
diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index 49ecfe156..66b934c06 100644
--- a/src/libsync/propagatorjobs.cpp
+++ b/src/libsync/propagatorjobs.cpp
@@ -151,13 +151,21 @@ void PropagateLocalMkdir::start()
     // When turning something that used to be a file into a directory
     // we need to delete the file first.
     QFileInfo fi(newDirStr);
-    if (_deleteExistingFile && fi.exists() && fi.isFile()) {
-        QString removeError;
-        if (!FileSystem::remove(newDirStr, &removeError)) {
-            done(SyncFileItem::NormalError,
-                tr("could not delete file %1, error: %2")
-                    .arg(newDirStr, removeError));
-            return;
+    if (fi.exists() && fi.isFile()) {
+        if (_deleteExistingFile) {
+            QString removeError;
+            if (!FileSystem::remove(newDirStr, &removeError)) {
+                done(SyncFileItem::NormalError,
+                    tr("could not delete file %1, error: %2")
+                        .arg(newDirStr, removeError));
+                return;
+            }
+        } else if (_item->_instruction == CSYNC_INSTRUCTION_CONFLICT) {
+            QString error;
+            if (!propagator()->createConflict(_item, _associatedComposite, &error)) {
+                done(SyncFileItem::SoftError, error);
+                return;
+            }
         }
     }
 
@@ -186,7 +194,10 @@ void PropagateLocalMkdir::start()
     }
     propagator()->_journal->commit("localMkdir");
 
-    done(SyncFileItem::Success);
+    auto resultStatus = _item->_instruction == CSYNC_INSTRUCTION_CONFLICT
+        ? SyncFileItem::Conflict
+        : SyncFileItem::Success;
+    done(resultStatus);
 }
 
 void PropagateLocalMkdir::setDeleteExistingFile(bool enabled)
diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index b3bbc5383..cd0cf6ccd 100644
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -432,6 +432,7 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
         item->_modtime = file->modtime;
         item->_size = file->size;
         item->_checksumHeader = file->checksumHeader;
+        item->_type = file->type;
     } else {
         if (instruction != CSYNC_INSTRUCTION_NONE) {
             qCWarning(lcEngine) << "ERROR: Instruction" << item->_instruction << "vs" << instruction << "for" << fileUtf8;
@@ -576,8 +577,6 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other,
         item->_inode = file->inode;
     }
 
-    item->_type = file->type;
-
     SyncFileItem::Direction dir = SyncFileItem::None;
 
     int re = 0;
diff --git a/test/testsyncconflict.cpp b/test/testsyncconflict.cpp
index 0a15bb9e9..daf7eab0d 100644
--- a/test/testsyncconflict.cpp
+++ b/test/testsyncconflict.cpp
@@ -345,6 +345,208 @@ private slots:
         QFETCH(QString, output);
         QCOMPARE(Utility::conflictFileBaseName(input.toUtf8()), output.toUtf8());
     }
+
+    void testLocalDirRemoteFileConflict()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        fakeFolder.syncEngine().account()->setCapabilities({ { "uploadConflictFiles", true } });
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+
+        auto cleanup = [&]() {
+            completeSpy.clear();
+        };
+        cleanup();
+
+        // 1) a NEW/NEW conflict
+        fakeFolder.localModifier().mkdir("Z");
+        fakeFolder.localModifier().mkdir("Z/subdir");
+        fakeFolder.localModifier().insert("Z/foo");
+        fakeFolder.remoteModifier().insert("Z", 63);
+
+        // 2) local file becomes a dir; remote file changes
+        fakeFolder.localModifier().remove("A/a1");
+        fakeFolder.localModifier().mkdir("A/a1");
+        fakeFolder.localModifier().insert("A/a1/bar");
+        fakeFolder.remoteModifier().appendByte("A/a1");
+
+        // 3) local dir gets a new file; remote dir becomes a file
+        fakeFolder.localModifier().insert("B/zzz");
+        fakeFolder.remoteModifier().remove("B");
+        fakeFolder.remoteModifier().insert("B", 31);
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        auto conflicts = findConflicts(fakeFolder.currentLocalState());
+        conflicts += findConflicts(fakeFolder.currentLocalState().children["A"]);
+        QCOMPARE(conflicts.size(), 3);
+        std::sort(conflicts.begin(), conflicts.end());
+
+        auto conflictRecords = fakeFolder.syncJournal().conflictRecordPaths();
+        QCOMPARE(conflictRecords.size(), 3);
+        std::sort(conflictRecords.begin(), conflictRecords.end());
+
+        // 1)
+        QVERIFY(itemConflict(completeSpy, "Z"));
+        QCOMPARE(fakeFolder.currentLocalState().find("Z")->size, 63);
+        QVERIFY(conflicts[2].contains("Z"));
+        QCOMPARE(conflicts[2].toUtf8(), conflictRecords[2]);
+        QVERIFY(QFileInfo(fakeFolder.localPath() + conflicts[2]).isDir());
+        QVERIFY(QFile::exists(fakeFolder.localPath() + conflicts[2] + "/foo"));
+
+        // 2)
+        QVERIFY(itemConflict(completeSpy, "A/a1"));
+        QCOMPARE(fakeFolder.currentLocalState().find("A/a1")->size, 5);
+        QVERIFY(conflicts[0].contains("A/a1"));
+        QCOMPARE(conflicts[0].toUtf8(), conflictRecords[0]);
+        QVERIFY(QFileInfo(fakeFolder.localPath() + conflicts[0]).isDir());
+        QVERIFY(QFile::exists(fakeFolder.localPath() + conflicts[0] + "/bar"));
+
+        // 3)
+        QVERIFY(itemConflict(completeSpy, "B"));
+        QCOMPARE(fakeFolder.currentLocalState().find("B")->size, 31);
+        QVERIFY(conflicts[1].contains("B"));
+        QCOMPARE(conflicts[1].toUtf8(), conflictRecords[1]);
+        QVERIFY(QFileInfo(fakeFolder.localPath() + conflicts[1]).isDir());
+        QVERIFY(QFile::exists(fakeFolder.localPath() + conflicts[1] + "/zzz"));
+
+        // The contents of the conflict directories will only be uploaded after
+        // another sync.
+        QVERIFY(fakeFolder.syncEngine().isAnotherSyncNeeded() == ImmediateFollowUp);
+        cleanup();
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(itemSuccessful(completeSpy, conflicts[0], CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemSuccessful(completeSpy, conflicts[0] + "/bar", CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemSuccessful(completeSpy, conflicts[1], CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemSuccessful(completeSpy, conflicts[1] + "/zzz", CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemSuccessful(completeSpy, conflicts[2], CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemSuccessful(completeSpy, conflicts[2] + "/foo", CSYNC_INSTRUCTION_NEW));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testLocalFileRemoteDirConflict()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        fakeFolder.syncEngine().account()->setCapabilities({ { "uploadConflictFiles", true } });
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+
+        // 1) a NEW/NEW conflict
+        fakeFolder.remoteModifier().mkdir("Z");
+        fakeFolder.remoteModifier().mkdir("Z/subdir");
+        fakeFolder.remoteModifier().insert("Z/foo");
+        fakeFolder.localModifier().insert("Z");
+
+        // 2) local dir becomes file: remote dir adds file
+        fakeFolder.localModifier().remove("A");
+        fakeFolder.localModifier().insert("A", 63);
+        fakeFolder.remoteModifier().insert("A/bar");
+
+        // 3) local file changes; remote file becomes dir
+        fakeFolder.localModifier().appendByte("B/b1");
+        fakeFolder.remoteModifier().remove("B/b1");
+        fakeFolder.remoteModifier().mkdir("B/b1");
+        fakeFolder.remoteModifier().insert("B/b1/zzz");
+
+        QVERIFY(fakeFolder.syncOnce());
+        auto conflicts = findConflicts(fakeFolder.currentLocalState());
+        conflicts += findConflicts(fakeFolder.currentLocalState().children["B"]);
+        QCOMPARE(conflicts.size(), 3);
+        std::sort(conflicts.begin(), conflicts.end());
+
+        auto conflictRecords = fakeFolder.syncJournal().conflictRecordPaths();
+        QCOMPARE(conflictRecords.size(), 3);
+        std::sort(conflictRecords.begin(), conflictRecords.end());
+
+        // 1)
+        QVERIFY(itemConflict(completeSpy, "Z"));
+        QVERIFY(conflicts[2].contains("Z"));
+        QCOMPARE(conflicts[2].toUtf8(), conflictRecords[2]);
+
+        // 2)
+        QVERIFY(itemConflict(completeSpy, "A"));
+        QVERIFY(conflicts[0].contains("A"));
+        QCOMPARE(conflicts[0].toUtf8(), conflictRecords[0]);
+
+        // 3)
+        QVERIFY(itemConflict(completeSpy, "B/b1"));
+        QVERIFY(conflicts[1].contains("B/b1"));
+        QCOMPARE(conflicts[1].toUtf8(), conflictRecords[1]);
+
+        // Also verifies that conflicts were uploaded
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testTypeConflictWithMove()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+
+        // the remote becomes a file, but a file inside the dir has moved away!
+        fakeFolder.remoteModifier().remove("A");
+        fakeFolder.remoteModifier().insert("A");
+        fakeFolder.localModifier().rename("A/a1", "a1");
+
+        // same, but with a new file inside the dir locally
+        fakeFolder.remoteModifier().remove("B");
+        fakeFolder.remoteModifier().insert("B");
+        fakeFolder.localModifier().rename("B/b1", "b1");
+        fakeFolder.localModifier().insert("B/new");
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(itemSuccessful(completeSpy, "A", CSYNC_INSTRUCTION_TYPE_CHANGE));
+        QVERIFY(itemConflict(completeSpy, "B"));
+
+        auto conflicts = findConflicts(fakeFolder.currentLocalState());
+        std::sort(conflicts.begin(), conflicts.end());
+        QVERIFY(conflicts.size() == 2);
+        QVERIFY(conflicts[0].contains("A_conflict"));
+        QVERIFY(conflicts[1].contains("B_conflict"));
+        for (auto conflict : conflicts)
+            QDir(fakeFolder.localPath() + conflict).removeRecursively();
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // Currently a1 and b1 don't get moved, but redownloaded
+    }
+
+    void testTypeChange()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+
+        // dir becomes file
+        fakeFolder.remoteModifier().remove("A");
+        fakeFolder.remoteModifier().insert("A");
+        fakeFolder.localModifier().remove("B");
+        fakeFolder.localModifier().insert("B");
+
+        // file becomes dir
+        fakeFolder.remoteModifier().remove("C/c1");
+        fakeFolder.remoteModifier().mkdir("C/c1");
+        fakeFolder.remoteModifier().insert("C/c1/foo");
+        fakeFolder.localModifier().remove("C/c2");
+        fakeFolder.localModifier().mkdir("C/c2");
+        fakeFolder.localModifier().insert("C/c2/bar");
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        QVERIFY(itemSuccessful(completeSpy, "A", CSYNC_INSTRUCTION_TYPE_CHANGE));
+        QVERIFY(itemSuccessful(completeSpy, "B", CSYNC_INSTRUCTION_TYPE_CHANGE));
+        QVERIFY(itemSuccessful(completeSpy, "C/c1", CSYNC_INSTRUCTION_TYPE_CHANGE));
+        QVERIFY(itemSuccessful(completeSpy, "C/c2", CSYNC_INSTRUCTION_TYPE_CHANGE));
+
+        // A becomes a conflict because we don't delete folders with files
+        // inside of them!
+        auto conflicts = findConflicts(fakeFolder.currentLocalState());
+        QVERIFY(conflicts.size() == 1);
+        QVERIFY(conflicts[0].contains("A_conflict"));
+        for (auto conflict : conflicts)
+            QDir(fakeFolder.localPath() + conflict).removeRecursively();
+
+        QVERIFY(fakeFolder.syncEngine().isAnotherSyncNeeded() == ImmediateFollowUp);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncConflict)