Virtual files: Wipe virtual after download completes, not before

Otherwise a interrupted or unsuccessful download would mean that the
download-intend was forgotten. The next sync would reestablish the
virtual file instead.

src/libsync/propagatedownload.cpp

@@ -399,17 +399,6 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
         return;
     }
 
-    // If we want to download something that used to be a virtual file,
-    // wipe the virtual file and proceed with a normal download
-    if (_item->_type == ItemTypeVirtualFileDownload) {
-        auto virtualFile = propagator()->addVirtualFileSuffix(_item->_file);
-        auto fn = propagator()->getFilePath(virtualFile);
-        qCDebug(lcPropagateDownload) << "Downloading file that used to be a virtual file" << fn;
-        QFile::remove(fn);
-        propagator()->_journal->deleteFileRecord(virtualFile);
-        _item->_type = ItemTypeFile;
-    }
-
     if (_deleteExisting) {
         deleteExistingFolder();
 
@@ -963,6 +952,17 @@ void PropagateDownloadFile::downloadFinished()
     if (_conflictRecord.isValid())
         propagator()->_journal->setConflictRecord(_conflictRecord);
 
+    // If we downloaded something that used to be a virtual file,
+    // wipe the virtual file and its db entry now that we're done.
+    if (_item->_type == ItemTypeVirtualFileDownload) {
+        auto virtualFile = propagator()->addVirtualFileSuffix(_item->_file);
+        auto fn = propagator()->getFilePath(virtualFile);
+        qCDebug(lcPropagateDownload) << "Download of previous virtual file finished" << fn;
+        QFile::remove(fn);
+        propagator()->_journal->deleteFileRecord(virtualFile);
+        _item->_type = ItemTypeFile;
+    }
+
     updateMetadata(isConflict);
 }
 

test/testsyncvirtualfiles.cpp

@@ -354,6 +354,60 @@ private slots:
         QVERIFY(!dbRecord(fakeFolder, "A/a6.owncloud").isValid());
     }
 
+    void testVirtualFileDownloadResume()
+    {
+        FakeFolder fakeFolder{ FileInfo() };
+        SyncOptions syncOptions;
+        syncOptions._newFilesAreVirtual = true;
+        fakeFolder.syncEngine().setSyncOptions(syncOptions);
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+
+        auto cleanup = [&]() {
+            completeSpy.clear();
+            fakeFolder.syncJournal().wipeErrorBlacklist();
+        };
+        cleanup();
+
+        auto triggerDownload = [&](const QByteArray &path) {
+            auto &journal = fakeFolder.syncJournal();
+            SyncJournalFileRecord record;
+            journal.getFileRecord(path + ".owncloud", &record);
+            if (!record.isValid())
+                return;
+            record._type = ItemTypeVirtualFileDownload;
+            journal.setFileRecord(record);
+            journal.avoidReadFromDbOnNextSync(record._path);
+        };
+
+        // Create a virtual file for remote files
+        fakeFolder.remoteModifier().mkdir("A");
+        fakeFolder.remoteModifier().insert("A/a1");
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
+        cleanup();
+
+        // Download by changing the db entry
+        triggerDownload("A/a1");
+        fakeFolder.serverErrorPaths().append("A/a1", 500);
+        QVERIFY(!fakeFolder.syncOnce());
+        QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NONE));
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
+        QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypeVirtualFileDownload);
+        QVERIFY(!dbRecord(fakeFolder, "A/a1").isValid());
+        cleanup();
+
+        fakeFolder.serverErrorPaths().clear();
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NONE));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile);
+        QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid());
+    }
+
     // Check what might happen if an older sync client encounters virtual files
     void testOldVersion1()
     {