Fix and test _file and _renameTarget

There was a bunch of inconsistency around whether _file was set to
_renameTarget or not. This is now never done, passing on more
information.

5 files changed, 15 insertions, 15 deletions

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index 1981fd7e7..f50201020 100644
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -967,14 +967,11 @@ void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status)
 void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
 {
     if (!_item->isEmpty() && status == SyncFileItem::Success) {
-        if (!_item->_renameTarget.isEmpty()) {
-            if (_item->_instruction == CSYNC_INSTRUCTION_RENAME
-                && _item->_originalFile != _item->_renameTarget) {
-                // Remove the stale entries from the database.
-                propagator()->_journal->deleteFileRecord(_item->_originalFile, true);
-            }
-
-            _item->_file = _item->_renameTarget;
+        // If a directory is renamed, recursively delete any stale items
+        // that may still exist below the old path.
+        if (_item->_instruction == CSYNC_INSTRUCTION_RENAME
+            && _item->_originalFile != _item->_renameTarget) {
+            propagator()->_journal->deleteFileRecord(_item->_originalFile, true);
         }
 
         // For new directories we always want to update the etag once
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 60c43ef88..1260a2183 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -374,11 +374,6 @@ void PropagateDownloadFile::start()
         qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file;
         vfs->dehydratePlaceholder(*_item);
         propagator()->_journal->deleteFileRecord(_item->_originalFile);
-        // NOTE: This is only done because other rename-like ops also adjust _file, even though
-        // updateMetadata() will store at destination() anyway. Doing this may not be necessary
-        // but maybe it has an effect on reporting (destination() and moves aren't handled
-        // consistently everywhere)
-        _item->_file = _item->destination();
         updateMetadata(false);
         return;
     }
diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index c988ea0a3..f6f840382 100644
--- a/src/libsync/propagatorjobs.cpp
+++ b/src/libsync/propagatorjobs.cpp
@@ -234,9 +234,7 @@ void PropagateLocalRename::start()
     propagator()->_journal->getFileRecord(_item->_originalFile, &oldRecord);
     propagator()->_journal->deleteFileRecord(_item->_originalFile);
 
-    // store the rename file name in the item.
     const auto oldFile = _item->_file;
-    _item->_file = _item->_renameTarget;
 
     if (!_item->isDirectory()) { // Directories are saved at the end
         SyncFileItem newItem(*_item);
diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
index 4635d3d37..fb07420a5 100644
--- a/test/testsyncmove.cpp
+++ b/test/testsyncmove.cpp
@@ -356,6 +356,10 @@ private slots:
             QCOMPARE(counter.nDELETE, 0);
             QVERIFY(itemSuccessfulMove(completeSpy, "A/a1m"));
             QVERIFY(itemSuccessfulMove(completeSpy, "B/b1m"));
+            QCOMPARE(findItem(completeSpy, "A/a1m")->_file, QStringLiteral("A/a1"));
+            QCOMPARE(findItem(completeSpy, "A/a1m")->_renameTarget, QStringLiteral("A/a1m"));
+            QCOMPARE(findItem(completeSpy, "B/b1m")->_file, QStringLiteral("B/b1"));
+            QCOMPARE(findItem(completeSpy, "B/b1m")->_renameTarget, QStringLiteral("B/b1m"));
         }
 
         // Touch+Move on same side
@@ -485,6 +489,10 @@ private slots:
             QCOMPARE(counter.nDELETE, 0);
             QVERIFY(itemSuccessfulMove(completeSpy, "AM"));
             QVERIFY(itemSuccessfulMove(completeSpy, "BM"));
+            QCOMPARE(findItem(completeSpy, "AM")->_file, QStringLiteral("A"));
+            QCOMPARE(findItem(completeSpy, "AM")->_renameTarget, QStringLiteral("AM"));
+            QCOMPARE(findItem(completeSpy, "BM")->_file, QStringLiteral("B"));
+            QCOMPARE(findItem(completeSpy, "BM")->_renameTarget, QStringLiteral("BM"));
         }
 
         // Folder move with contents touched on the same side
diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index 6b99e21bb..7e1ba9516 100644
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -780,6 +780,8 @@ private slots:
         QVERIFY(hasDehydratedDbEntries("A/a1"));
         QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_SYNC));
         QCOMPARE(findItem(completeSpy, "A/a1.owncloud")->_type, ItemTypeVirtualFileDehydration);
+        QCOMPARE(findItem(completeSpy, "A/a1.owncloud")->_file, QStringLiteral("A/a1"));
+        QCOMPARE(findItem(completeSpy, "A/a1.owncloud")->_renameTarget, QStringLiteral("A/a1.owncloud"));
         QVERIFY(isDehydrated("A/a2"));
         QVERIFY(hasDehydratedDbEntries("A/a2"));
         QVERIFY(itemInstruction(completeSpy, "A/a2.owncloud", CSYNC_INSTRUCTION_SYNC));