Virtual files: Wipe virtual after download completes, not before
authorChristian Kamm <mail@ckamm.de>
Wed, 30 May 2018 08:33:32 +0000 (10:33 +0200)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:57:54 +0000 (10:57 +0100)
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
test/testsyncvirtualfiles.cpp

index d12bccc621a97a9476ea8567f8356d7f270778a3..a7b32bc767f631d7b0e2d430a54b940f8eff6a1a 100644 (file)
@@ -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);
 }
 
index ba162836f918759829083fa0ccbcd357afcdcb65..0bcd33c51da72f77a8f482bb8eb2b1154dbf0c35 100644 (file)
@@ -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()
     {