From 85aefa4232c44ba88bc8ac485e512b249f383a0a Mon Sep 17 00:00:00 2001
From: Hannah von Reth <hannah.vonreth@owncloud.com>
Date: Fri, 20 Mar 2020 17:39:10 +0100
Subject: [PATCH] Sync: Fix handling of virtual files in error state

Issue: #7799
---
 src/libsync/discoveryphase.cpp | 24 +++++++++++++++++-------
 test/testsyncmove.cpp          | 24 +++++++++++++++++++-----
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index 37c01c04a..32135b388 100644
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -159,14 +159,24 @@ QPair<bool, QByteArray> DiscoveryPhase::findAndCancelDeletedJob(const QString &o
     QByteArray oldEtag;
     auto it = _deletedItem.find(originalPath);
     if (it != _deletedItem.end()) {
-        ENFORCE((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE
+        const csync_instructions_e instruction = (*it)->_instruction;
+        if (instruction == CSYNC_INSTRUCTION_IGNORE && (*it)->_type == ItemTypeVirtualFile) {
             // re-creation of virtual files count as a delete
-             || ((*it)->_type == ItemTypeVirtualFile && (*it)->_instruction == CSYNC_INSTRUCTION_NEW)
-             || ((*it)->_isRestoration && (*it)->_instruction == CSYNC_INSTRUCTION_NEW)
-               );
-        (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
-        result = true;
-        oldEtag = (*it)->_etag;
+            // a file might be in an error state and thus gets marked as CSYNC_INSTRUCTION_IGNORE
+            // after it was initially marked as CSYNC_INSTRUCTION_REMOVE
+            // return true, to not trigger any additional actions on that file that could elad to dataloss
+            result = true;
+            oldEtag = (*it)->_etag;
+        } else {
+            ENFORCE(instruction == CSYNC_INSTRUCTION_REMOVE
+                // re-creation of virtual files count as a delete
+                || ((*it)->_type == ItemTypeVirtualFile && instruction == CSYNC_INSTRUCTION_NEW)
+                || ((*it)->_isRestoration && instruction == CSYNC_INSTRUCTION_NEW)
+                );
+            (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
+            result = true;
+            oldEtag = (*it)->_etag;
+        }
         _deletedItem.erase(it);
     }
     if (auto *otherJob = _queuedDeletedDirectories.take(originalPath)) {
diff --git a/test/testsyncmove.cpp b/test/testsyncmove.cpp
index 540d313ed..bf43fa12e 100644
--- a/test/testsyncmove.cpp
+++ b/test/testsyncmove.cpp
@@ -880,7 +880,8 @@ private slots:
             }
             return s;
         };
-        const QByteArray testPath = "folder/folderA/file.txt";
+        const QString src = "folder/folderA/file.txt";
+        const QString dest = "folder/folderB/file.txt";
         FakeFolder fakeFolder{ FileInfo{ QString(), { FileInfo{ QStringLiteral("folder"), { FileInfo{ QStringLiteral("folderA"), { { QStringLiteral("file.txt"), 400 } } }, QStringLiteral("folderB") } } } } };
 
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
@@ -896,8 +897,13 @@ private slots:
             fakeFolder.syncOnce();
         }
 
-        fakeFolder.serverErrorPaths().append(testPath, 403);
-        fakeFolder.localModifier().rename(getName(testPath), getName("folder/folderB/file.txt"));
+
+        fakeFolder.serverErrorPaths().append(src, 403);
+        fakeFolder.localModifier().rename(getName(src), getName(dest));
+        QVERIFY(!fakeFolder.currentLocalState().find(getName(src)));
+        QVERIFY(fakeFolder.currentLocalState().find(getName(dest)));
+        QVERIFY(fakeFolder.currentRemoteState().find(src));
+        QVERIFY(!fakeFolder.currentRemoteState().find(dest));
 
         // sync1 file gets detected as error, instruction is still NEW_FILE
         fakeFolder.syncOnce();
@@ -910,8 +916,16 @@ private slots:
             fakeFolder.syncJournal().internalPinStates().setForPath("", PinState::AlwaysLocal);
             fakeFolder.syncOnce();
         }
-        // the sync must have failed as we have a file in the error state
-        QVERIFY(fakeFolder.currentLocalState() != fakeFolder.currentRemoteState());
+
+        QVERIFY(!fakeFolder.currentLocalState().find(src));
+        QVERIFY(fakeFolder.currentLocalState().find(getName(dest)));
+        if (vfsMode == Vfs::WithSuffix)
+        {
+            // the placeholder was not restored as it is still in error state
+            QVERIFY(!fakeFolder.currentLocalState().find(dest));
+        }
+        QVERIFY(fakeFolder.currentRemoteState().find(src));
+        QVERIFY(!fakeFolder.currentRemoteState().find(dest));
     }
 
 };