From: Christian Kamm Date: Wed, 17 Jan 2018 09:48:25 +0000 (+0100) Subject: Placeholders: Deal with conflicts when a placeholder exists X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~616 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=0cd83a2c098fdfab45abe716574773ff916912b0;p=nextcloud-desktop.git Placeholders: Deal with conflicts when a placeholder exists So "foo.owncloud" exists but the user adds a new "foo". --- diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index ee0c9b72b..dcd385ccd 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -316,6 +316,16 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) /* file on current replica is changed or new */ case CSYNC_INSTRUCTION_EVAL: case CSYNC_INSTRUCTION_NEW: + // If the db says this is a placeholder, but there is a local item, + // go to "possible conflict" mode by adjusting the remote instruction. + if (ctx->current == LOCAL_REPLICA + && (other->type == ItemTypePlaceholder || other->type == ItemTypePlaceholderDownload) + && cur->type != ItemTypePlaceholder + && other->instruction == CSYNC_INSTRUCTION_NONE) { + other->instruction = CSYNC_INSTRUCTION_EVAL; + other->type = ItemTypePlaceholderDownload; + } + switch (other->instruction) { /* file on other replica is changed or new */ case CSYNC_INSTRUCTION_NEW: diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index d72a0ec51..b470c4dca 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -221,12 +221,24 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr f fs->checksumHeader.constData(), base._checksumHeader.constData(), fs->type, base._type, base._serverHasIgnoredFiles, base._e2eMangledName.constData()); + + // If the db suggests a placeholder should be downloaded, + // treat the file as new on the remote. if (ctx->current == REMOTE_REPLICA && base._type == ItemTypePlaceholderDownload) { fs->instruction = CSYNC_INSTRUCTION_NEW; fs->type = ItemTypePlaceholderDownload; goto out; } + // If what the db thinks is a placeholder is actually a file/dir, + // treat it as new locally. + if (ctx->current == LOCAL_REPLICA + && (base._type == ItemTypePlaceholder || base._type == ItemTypePlaceholderDownload) + && fs->type != ItemTypePlaceholder) { + fs->instruction = CSYNC_INSTRUCTION_EVAL; + goto out; + } + if (ctx->current == REMOTE_REPLICA && fs->etag != base._etag) { fs->instruction = CSYNC_INSTRUCTION_EVAL; @@ -756,6 +768,12 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, && filename.endsWith(".owncloud")) { QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); db_uri = db_uri.left(db_uri.size() - 9); + + // Don't overwrite data that was already retrieved from disk! + // This can happen if foo.owncloud exists and the user adds foo. + if (ctx->local.files.findFile(db_uri)) + continue; + if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { errno = ENOENT; ctx->status_code = CSYNC_STATUS_OPENDIR_ERROR; diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index 038da9a5c..b760354a8 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -131,6 +131,69 @@ private slots: cleanup(); } + void testPlaceholderConflict() + { + FakeFolder fakeFolder{ FileInfo() }; + SyncOptions syncOptions; + syncOptions._usePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + + auto cleanup = [&]() { + completeSpy.clear(); + }; + cleanup(); + + Logger::instance()->setLogDebug(true); + Logger::instance()->setLogFile("-"); + + // Create a placeholder for a new remote file + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1", 64); + fakeFolder.remoteModifier().insert("A/a2", 64); + fakeFolder.remoteModifier().mkdir("B"); + fakeFolder.remoteModifier().insert("B/b1", 64); + fakeFolder.remoteModifier().insert("B/b2", 64); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("B/b2.owncloud")); + cleanup(); + + // A: the correct file and a conflicting file are added, placeholders stay + // B: same setup, but the placeholders are deleted by the user + fakeFolder.localModifier().insert("A/a1", 64); + fakeFolder.localModifier().insert("A/a2", 30); + fakeFolder.localModifier().insert("B/b1", 64); + fakeFolder.localModifier().insert("B/b2", 30); + fakeFolder.localModifier().remove("B/b1.owncloud"); + fakeFolder.localModifier().remove("B/b2.owncloud"); + QVERIFY(fakeFolder.syncOnce()); + + // Everything is CONFLICT since mtimes are different even for a1/b1 + QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "B/b1", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "B/b2", CSYNC_INSTRUCTION_CONFLICT)); + + // no placeholder files should remain + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("B/b1.owncloud")); + QVERIFY(!fakeFolder.currentLocalState().find("B/b2.owncloud")); + + // conflict files should exist + QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 2); + + // nothing should have the placeholder tag + QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "A/a2")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "B/b1")._type, ItemTypeFile); + QCOMPARE(dbRecord(fakeFolder, "B/b2")._type, ItemTypeFile); + + cleanup(); + } + void testWithNormalSync() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; @@ -199,11 +262,15 @@ private slots: fakeFolder.remoteModifier().insert("A/a2"); fakeFolder.remoteModifier().insert("A/a3"); fakeFolder.remoteModifier().insert("A/a4"); + fakeFolder.remoteModifier().insert("A/a5"); + fakeFolder.remoteModifier().insert("A/a6"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a3.owncloud")); QVERIFY(fakeFolder.currentLocalState().find("A/a4.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a5.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a6.owncloud")); cleanup(); // Download by changing the db entry @@ -211,16 +278,29 @@ private slots: triggerDownload("A/a2"); triggerDownload("A/a3"); triggerDownload("A/a4"); + triggerDownload("A/a5"); + triggerDownload("A/a6"); fakeFolder.remoteModifier().appendByte("A/a2"); fakeFolder.remoteModifier().remove("A/a3"); fakeFolder.remoteModifier().rename("A/a4", "A/a4m"); + fakeFolder.localModifier().insert("A/a5"); + fakeFolder.localModifier().insert("A/a6"); + fakeFolder.localModifier().remove("A/a6.owncloud"); QVERIFY(fakeFolder.syncOnce()); QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW)); QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_NEW)); QVERIFY(itemInstruction(completeSpy, "A/a3", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a4", CSYNC_INSTRUCTION_REMOVE)); QVERIFY(itemInstruction(completeSpy, "A/a4m", CSYNC_INSTRUCTION_NEW)); + QVERIFY(itemInstruction(completeSpy, "A/a5", CSYNC_INSTRUCTION_CONFLICT)); + QVERIFY(itemInstruction(completeSpy, "A/a6", CSYNC_INSTRUCTION_CONFLICT)); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + 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); } };