From 6a37a7a42cfdb63f3cf13e31ad1f156a3dc95a09 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Mon, 29 Jan 2018 13:02:31 +0100 Subject: [PATCH] Placeholders: Fix migration issues Some edgecases weren't covered and didn't have tests yet. --- src/csync/csync_reconcile.cpp | 20 +++++++++++-- src/csync/csync_update.cpp | 13 ++------- test/testsyncplaceholders.cpp | 55 ++++++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp index 72a8afffd..9cd00b393 100644 --- a/src/csync/csync_reconcile.cpp +++ b/src/csync/csync_reconcile.cpp @@ -176,9 +176,12 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) cur->instruction = CSYNC_INSTRUCTION_NEW; break; } - if (cur->type == ItemTypePlaceholder && ctx->current == REMOTE_REPLICA) { - /* Do not remove on the server if the local placeholder is gone: - * instead reestablish the local placeholder */ + /* If the local placeholder is gone it should be reestablished. + * Unless the base file is seen in the local tree now. */ + if (cur->type == ItemTypePlaceholder + && ctx->current == REMOTE_REPLICA + && cur->path.endsWith(ctx->placeholder_suffix) + && !other_tree->findFile(cur->path.left(cur->path.size() - ctx->placeholder_suffix.size()))) { cur->instruction = CSYNC_INSTRUCTION_NEW; break; } @@ -451,6 +454,17 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx) if (cur->instruction == CSYNC_INSTRUCTION_EVAL) cur->instruction = CSYNC_INSTRUCTION_NEW; break; + case CSYNC_INSTRUCTION_NONE: + // NONE/NONE on placeholders might become a REMOVE if the base file + // is found in the local tree. + if (cur->type == ItemTypePlaceholder + && other->instruction == CSYNC_INSTRUCTION_NONE + && ctx->current == LOCAL_REPLICA + && cur->path.endsWith(ctx->placeholder_suffix) + && ctx->local.files.findFile(cur->path.left(cur->path.size() - ctx->placeholder_suffix.size()))) { + cur->instruction = CSYNC_INSTRUCTION_REMOVE; + } + break; default: break; } diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 48adc38b7..cf05290c1 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -47,6 +47,7 @@ #include "common/asserts.h" #include +#include // Needed for PRIu64 on MinGW in C++ mode. #define __STDC_FORMAT_MACROS @@ -790,18 +791,10 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn, && dirent->type == ItemTypeFile && filename.endsWith(ctx->placeholder_suffix)) { QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1); - QByteArray base_uri = db_uri.left(db_uri.size() - ctx->placeholder_suffix.size()); - - // Don't fill the local tree with placeholder data if a real - // file was found. The remote tree will still have the placeholder - // file. - if (ctx->local.files.findFile(base_uri)) - continue; if( ! fill_tree_from_db(ctx, db_uri.constData(), true) ) { - errno = ENOENT; - ctx->status_code = CSYNC_STATUS_OPENDIR_ERROR; - goto error; + qCWarning(lcUpdate) << "Placeholder without db entry for" << filename; + QFile::remove(fullpath); } continue; diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp index a62928c33..454ef45ac 100644 --- a/test/testsyncplaceholders.cpp +++ b/test/testsyncplaceholders.cpp @@ -141,6 +141,26 @@ private slots: QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); QVERIFY(!dbRecord(fakeFolder, "A/a1m.owncloud").isValid()); cleanup(); + + // Edge case: Local placeholder but no db entry for some reason + fakeFolder.remoteModifier().insert("A/a2", 64); + fakeFolder.remoteModifier().insert("A/a3", 64); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(fakeFolder.currentLocalState().find("A/a3.owncloud")); + cleanup(); + + fakeFolder.syncEngine().journal()->deleteFileRecord("A/a2.owncloud"); + fakeFolder.syncEngine().journal()->deleteFileRecord("A/a3.owncloud"); + fakeFolder.remoteModifier().remove("A/a3"); + fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::FilesystemOnly); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(fakeFolder.currentLocalState().find("A/a2.owncloud")); + QVERIFY(itemInstruction(completeSpy, "A/a2.owncloud", CSYNC_INSTRUCTION_NEW)); + QVERIFY(dbRecord(fakeFolder, "A/a2.owncloud").isValid()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a3.owncloud")); + QVERIFY(!dbRecord(fakeFolder, "A/a3.owncloud").isValid()); + cleanup(); } void testPlaceholderConflict() @@ -335,7 +355,7 @@ private slots: } // Check what might happen if an older sync client encounters placeholders - void testOldVersion() + void testOldVersion1() { FakeFolder fakeFolder{ FileInfo() }; SyncOptions syncOptions; @@ -378,6 +398,39 @@ private slots: QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + + // Older versions may leave db entries for foo and foo.owncloud + void testOldVersion2() + { + FakeFolder fakeFolder{ FileInfo() }; + + // Sync a file + fakeFolder.remoteModifier().mkdir("A"); + fakeFolder.remoteModifier().insert("A/a1"); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + // Create the placeholder too + // In the wild, the new version would create the placeholder and the db entry + // while the old version would download the plain file. + fakeFolder.localModifier().insert("A/a1.owncloud"); + auto &db = fakeFolder.syncJournal(); + SyncJournalFileRecord rec; + db.getFileRecord(QByteArray("A/a1"), &rec); + rec._type = ItemTypePlaceholder; + rec._path = "A/a1.owncloud"; + db.setFileRecord(rec); + + SyncOptions syncOptions; + syncOptions._newFilesArePlaceholders = true; + fakeFolder.syncEngine().setSyncOptions(syncOptions); + + // Check that a sync removes the placeholder and its db entry + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud")); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid()); + } }; QTEST_GUILESS_MAIN(TestSyncPlaceholders) -- 2.30.2