From: Christian Kamm <mail@ckamm.de>
Date: Thu, 28 Mar 2019 08:10:20 +0000 (+0100)
Subject: Fix and test _file and _renameTarget
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~269
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=47f10fbf96a9e273bb66ff3977d98f9abe742cc2;p=nextcloud-desktop.git

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.
---

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index b8fd817ad..7b6d7191d 100644
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -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
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index c0a5e250b..12677cb3d 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -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;
     }
diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index ca69d8819..a5a864bfd 100644
--- a/src/libsync/propagatorjobs.cpp
+++ b/src/libsync/propagatorjobs.cpp
@@ -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);
diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
index d7e1f7017..390dd0104 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 6096c5c92..4bd21aabc 100644
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -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));