Fix and test _file and _renameTarget
authorChristian Kamm <mail@ckamm.de>
Thu, 28 Mar 2019 08:10:20 +0000 (09:10 +0100)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:58:44 +0000 (10:58 +0100)
There was a bunch of inconsistency around whether _file was set to
_renameTarget or not. This is now never done, passing on more
information.

src/libsync/owncloudpropagator.cpp
src/libsync/propagatedownload.cpp
src/libsync/propagatorjobs.cpp
test/testsyncmove.cpp
test/testsyncvirtualfiles.cpp

index b8fd817ad8c5d5212a4076fdbfbc6c4c9f5c35ac..7b6d7191df11f1ac6a7c07a455162e1281d26ec8 100644 (file)
@@ -958,14 +958,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
index c0a5e250b3162806b0f0f534a7117e75a2411312..12677cb3d6a575ff79ce1f24e74427b9672f3854 100644 (file)
@@ -428,11 +428,6 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
         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;
     }
index ca69d881975c2211878c61f53380acea36c10b80..a5a864bfdfa1c6687150455aee18eb18eff9204e 100644 (file)
@@ -283,9 +283,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);
index d7e1f7017e4cccee745f2849a511372fb3d91e48..390dd01041641f37eec14ef3c3ce015b25e7daaf 100644 (file)
@@ -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
index 6096c5c924351b2f130dc7dfa81ad412e41438c7..4bd21aabc903f6f02c35161fafc0d784b4bcbecd 100644 (file)
@@ -780,6 +780,8 @@ private slots:
         QVERIFY(hasDehydratedDbEntries("A/a1"));
         QVERIFY(itemInstruction(completeSpy, "A/a1.nextcloud", CSYNC_INSTRUCTION_SYNC));
         QCOMPARE(findItem(completeSpy, "A/a1.nextcloud")->_type, ItemTypeVirtualFileDehydration);
+        QCOMPARE(findItem(completeSpy, "A/a1.nextcloud")->_file, QStringLiteral("A/a1"));
+        QCOMPARE(findItem(completeSpy, "A/a1.nextcloud")->_renameTarget, QStringLiteral("A/a1.nextcloud"));
         QVERIFY(isDehydrated("A/a2"));
         QVERIFY(hasDehydratedDbEntries("A/a2"));
         QVERIFY(itemInstruction(completeSpy, "A/a2.nextcloud", CSYNC_INSTRUCTION_SYNC));