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 ee0c9b72bba2298581a725e447764b86cb90bf9a..dcd385ccddc0ce8b309f6f06384fd25a69f457de 100644 (file)
--- 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 d72a0ec5159511845dd38061c9bb2653852316b2..b470c4dca8f3bb3d2522a6f335dad297649561fc 100644 (file)
--- 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<csync_file_stat_t> 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 038da9a5c512d2a724deed6c9765376ba2dcec81..b760354a846900c6cd0fa9266a43dee46e1b5430 100644 (file)
--- 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);
     }
 };