From 4bab93b24674a2e268778aff43e72dcaa8b554cd Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Tue, 2 Apr 2019 11:51:47 +0200 Subject: [PATCH] Vfs: Better handling and more tests for suffix file renames Previously removing the vfs suffix of a file always triggered a conflict. Now it may just cause a file download. This was done because users expected symmetry in the rename actions and renaming foo -> foo.owncloud already triggers the "make the file virtual" action. Now foo.owncloud -> foo triggers the "download the contents" action. --- src/libsync/discovery.cpp | 24 ++++++++++++++++--- test/testsyncvirtualfiles.cpp | 45 ++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 91c8b6895..41fd96c7b 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -718,11 +718,29 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; item->_direction = SyncFileItem::Down; // Does not matter } + } else if (!typeChange && isVfsWithSuffix() + && dbEntry.isVirtualFile() && !localEntry.isVirtualFile + && dbEntry._inode == localEntry.inode + && dbEntry._modtime == localEntry.modtime + && localEntry.size == 1) { + // A suffix vfs file can be downloaded by renaming it to remove the suffix. + // This check leaks some details of VfsSuffix, particularly the size of placeholders. + item->_direction = SyncFileItem::Down; + if (noServerEntry) { + item->_instruction = CSYNC_INSTRUCTION_REMOVE; + item->_type = ItemTypeFile; + } else { + item->_instruction = CSYNC_INSTRUCTION_SYNC; + item->_type = ItemTypeVirtualFileDownload; + item->_previousSize = 1; + } } else if (serverModified - // If a suffix-file changes we prefer to go into conflict mode - but in-place - // placeholders could be replaced by real files and should be a regular SYNC - // if there's no server change. || (isVfsWithSuffix() && dbEntry.isVirtualFile())) { + // There's a local change and a server change: Conflict! + // Alternatively, this might be a suffix-file that's virtual in the db but + // not locally. These also become conflicts. For in-place placeholders that's + // not necessary: they could be replaced by real files and should then trigger + // a regular SYNC upwards when there's no server change. processFileConflict(item, path, localEntry, serverEntry, dbEntry); } else if (typeChange) { item->_instruction = CSYNC_INSTRUCTION_TYPE_CHANGE; diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp index 69c5a4038..b32889505 100644 --- a/test/testsyncvirtualfiles.cpp +++ b/test/testsyncvirtualfiles.cpp @@ -333,6 +333,11 @@ private slots: fakeFolder.remoteModifier().insert("A/a4"); fakeFolder.remoteModifier().insert("A/a5"); fakeFolder.remoteModifier().insert("A/a6"); + fakeFolder.remoteModifier().insert("A/a7"); + fakeFolder.remoteModifier().insert("A/b1"); + fakeFolder.remoteModifier().insert("A/b2"); + fakeFolder.remoteModifier().insert("A/b3"); + fakeFolder.remoteModifier().insert("A/b4"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(fakeFolder.currentLocalState().find("A/a1.nextcloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a2.nextcloud")); @@ -340,6 +345,11 @@ private slots: QVERIFY(fakeFolder.currentLocalState().find("A/a4.nextcloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a5.nextcloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a6.nextcloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a7.nextcloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/b1.nextcloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/b2.nextcloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/b3.nextcloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/b4.nextcloud")); cleanup(); // Download by changing the db entry @@ -349,12 +359,25 @@ private slots: triggerDownload(fakeFolder, "A/a4"); triggerDownload(fakeFolder, "A/a5"); triggerDownload(fakeFolder, "A/a6"); + triggerDownload(fakeFolder, "A/a7"); + // Download by renaming locally + fakeFolder.localModifier().rename("A/b1.nextcloud", "A/b1"); + fakeFolder.localModifier().rename("A/b2.nextcloud", "A/b2"); + fakeFolder.localModifier().rename("A/b3.nextcloud", "A/b3"); + fakeFolder.localModifier().rename("A/b4.nextcloud", "A/b4"); + // Remote complications fakeFolder.remoteModifier().appendByte("A/a2"); fakeFolder.remoteModifier().remove("A/a3"); fakeFolder.remoteModifier().rename("A/a4", "A/a4m"); + fakeFolder.remoteModifier().appendByte("A/b2"); + fakeFolder.remoteModifier().remove("A/b3"); + fakeFolder.remoteModifier().rename("A/b4", "A/b4m"); + // Local complications fakeFolder.localModifier().insert("A/a5"); fakeFolder.localModifier().insert("A/a6"); fakeFolder.localModifier().remove("A/a6.nextcloud"); + fakeFolder.localModifier().rename("A/a7.nextcloud", "A/a7"); + QVERIFY(fakeFolder.syncOnce()); QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_SYNC)); QCOMPARE(findItem(completeSpy, "A/a1")->_type, ItemTypeVirtualFileDownload); @@ -368,19 +391,39 @@ private slots: QVERIFY(itemInstruction(completeSpy, "A/a5", CSYNC_INSTRUCTION_CONFLICT)); QVERIFY(itemInstruction(completeSpy, "A/a5.nextcloud", CSYNC_INSTRUCTION_NONE)); QVERIFY(itemInstruction(completeSpy, "A/a6", CSYNC_INSTRUCTION_CONFLICT)); - QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QVERIFY(itemInstruction(completeSpy, "A/a7", CSYNC_INSTRUCTION_SYNC)); + QVERIFY(itemInstruction(completeSpy, "A/b1", CSYNC_INSTRUCTION_SYNC)); + QVERIFY(itemInstruction(completeSpy, "A/b2", CSYNC_INSTRUCTION_SYNC)); + QVERIFY(itemInstruction(completeSpy, "A/b3", CSYNC_INSTRUCTION_REMOVE)); + QVERIFY(itemInstruction(completeSpy, "A/b4m.nextcloud", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/b4", CSYNC_INSTRUCTION_REMOVE)); QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a2")._type, ItemTypeFile); QVERIFY(!dbRecord(fakeFolder, "A/a3").isValid()); QCOMPARE(dbRecord(fakeFolder, "A/a4m")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a5")._type, ItemTypeFile); QCOMPARE(dbRecord(fakeFolder, "A/a6")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a7")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/b1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/b2")._type, ItemTypeFile); + QVERIFY(!dbRecord(fakeFolder, "A/b3").isValid()); + QCOMPARE(dbRecord(fakeFolder, "A/b4m.nextcloud")._type, ItemTypeVirtualFile); QVERIFY(!dbRecord(fakeFolder, "A/a1.nextcloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a2.nextcloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a3.nextcloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a4.nextcloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a5.nextcloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a6.nextcloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/a7.nextcloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/b1.nextcloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/b2.nextcloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/b3.nextcloud").isValid()); + QVERIFY(!dbRecord(fakeFolder, "A/b4.nextcloud").isValid()); + + triggerDownload(fakeFolder, "A/b4m"); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } void testVirtualFileDownloadResume() -- 2.30.2