Placeholders: Safe migration to older client versions

Now the db entries for placeholders will have the full placeholder
paths. That way older clients will, on remote discovery, delete the
placeholders and download the real files.

diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h
index 2ba22035884b1010366d1564712e6478de8eb45c..780dfb66d7edcabe73f99234135a79d700532d37 100644 (file)
--- a/src/csync/csync_private.h
+++ b/src/csync/csync_private.h
@@ -212,6 +212,11 @@ struct OCSYNC_EXPORT csync_s {
    */
   bool new_files_are_placeholders = false;
 
+  /**
+   * The suffix to use for placeholder files.
+   */
+  QByteArray placeholder_suffix = ".owncloud";
+
   csync_s(const char *localUri, OCC::SyncJournalDb *statedb);
   ~csync_s();
   int reinitialize();

diff --git a/src/csync/csync_reconcile.cpp b/src/csync/csync_reconcile.cpp
index dcd385ccddc0ce8b309f6f06384fd25a69f457de..72a8afffdc59a4283ed423d36080e1bfb1789ee4 100644 (file)
--- a/src/csync/csync_reconcile.cpp
+++ b/src/csync/csync_reconcile.cpp
@@ -111,6 +111,7 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx)
     }
 
     csync_file_stat_t *other = other_tree->findFile(cur->path);
+
     if (!other) {
         if (ctx->current == REMOTE_REPLICA) {
             // The file was not found and the other tree is the local one
@@ -131,6 +132,31 @@ static void _csync_merge_algorithm_visitor(csync_file_stat_t *cur, CSYNC * ctx)
         /* If it is ignored, other->instruction will be  IGNORE so this one will also be ignored */
     }
 
+    // If the user adds a file locally check whether a placeholder for that name exists.
+    // If so, go to "potential conflict" mode by switching the remote entry to be a
+    // real file.
+    if (!other
+        && ctx->current == LOCAL_REPLICA
+        && cur->instruction == CSYNC_INSTRUCTION_NEW
+        && cur->type != ItemTypePlaceholder) {
+        // Check if we have a placeholder entry in the remote tree
+        auto placeholderPath = cur->path;
+        placeholderPath.append(ctx->placeholder_suffix);
+        other = other_tree->findFile(placeholderPath);
+        if (!other) {
+            /* Check the renamed path as well. */
+            other = other_tree->findFile(csync_rename_adjust_parent_path(ctx, placeholderPath));
+        }
+        if (other && other->type == ItemTypePlaceholder) {
+            qCInfo(lcReconcile) << "Found placeholder for local" << cur->path << "in remote tree";
+            other->path = cur->path;
+            other->type = ItemTypePlaceholderDownload;
+            other->instruction = CSYNC_INSTRUCTION_EVAL;
+        } else {
+            other = nullptr;
+        }
+    }
+
     /* file only found on current replica */
     if (!other) {
         switch(cur->instruction) {
@@ -316,16 +342,6 @@ 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 b470c4dca8f3bb3d2522a6f335dad297649561fc..48adc38b733b63d285c0ec1f5452ee688468ed06 100644 (file)
--- a/src/csync/csync_update.cpp
+++ b/src/csync/csync_update.cpp
@@ -188,7 +188,6 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
       return -1;
   }
 
-
   /*
    * When file is encrypted it's phash (path hash) will not match the local file phash,
    * we could match the e2eMangledName but that might be slow wihout index, and it's
@@ -201,6 +200,22 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
       }
   }
 
+  // The db entry might be for a placeholder, so look for that on the
+  // remote side. If we find one, change the current fs to look like a
+  // placeholder too, because that's what one would see if the remote
+  // db was filled from the database.
+  if (ctx->current == REMOTE_REPLICA && !base.isValid() && fs->type == ItemTypeFile) {
+      auto placeholderPath = fs->path;
+      placeholderPath.append(ctx->placeholder_suffix);
+      ctx->statedb->getFileRecord(placeholderPath, &base);
+      if (base.isValid() && base._type == ItemTypePlaceholder) {
+          fs->type = ItemTypePlaceholder;
+          fs->path = placeholderPath;
+      } else {
+          base = OCC::SyncJournalFileRecord();
+      }
+  }
+
   if(base.isValid()) { /* there is an entry in the database */
       // When the file is loaded from the file system it misses
       // the e2e mangled name and e2e encryption status
@@ -242,10 +257,9 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
       if (ctx->current == REMOTE_REPLICA && fs->etag != base._etag) {
           fs->instruction = CSYNC_INSTRUCTION_EVAL;
 
-          if (base._type == ItemTypePlaceholder && fs->type == ItemTypeFile) {
+          if (fs->type == ItemTypePlaceholder) {
               // If the local thing is a placeholder, we just update the metadata
               fs->instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
-              fs->type = ItemTypePlaceholder; // retain the PLACEHOLDER type in the db
           } else if (base._type != fs->type) {
               // Preserve the EVAL flag later on if the type has changed.
               fs->child_modified = true;
@@ -394,6 +408,14 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
                   return;
               }
 
+              // Now we know there is a sane rename candidate.
+
+              // Rename of a placeholder
+              if (base._type == ItemTypePlaceholder && fs->type == ItemTypeFile) {
+                  fs->type = ItemTypePlaceholder;
+                  fs->path.append(ctx->placeholder_suffix);
+              }
+
               // Record directory renames
               if (fs->type == ItemTypeDirectory) {
                   // If the same folder was already renamed by a different entry,
@@ -434,11 +456,12 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
               }
           }
 
-          // Potentially turn new remote files into placeholders
+          // Turn new remote files into placeholders if the option is enabled.
           if (ctx->new_files_are_placeholders
               && fs->instruction == CSYNC_INSTRUCTION_NEW
               && fs->type == ItemTypeFile) {
               fs->type = ItemTypePlaceholder;
+              fs->path.append(ctx->placeholder_suffix);
           }
 
           goto out;
@@ -764,14 +787,15 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
     // When encountering placeholder files, read the relevant
     // entry from the db instead.
     if (ctx->current == LOCAL_REPLICA
-            && dirent->type == ItemTypeFile
-            && filename.endsWith(".owncloud")) {
+        && dirent->type == ItemTypeFile
+        && filename.endsWith(ctx->placeholder_suffix)) {
         QByteArray db_uri = fullpath.mid(strlen(ctx->local.uri) + 1);
-        db_uri = db_uri.left(db_uri.size() - 9);
+        QByteArray base_uri = db_uri.left(db_uri.size() - ctx->placeholder_suffix.size());
 
-        // 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))
+        // 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) ) {

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 2ed886550374cc82e22ee619beec23bbdce76cfb..d408a9d32d4ec4236555427f663449a87d5b80b3 100644 (file)
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -714,7 +714,7 @@ void Folder::setSyncOptions()
     opt._newBigFolderSizeLimit = newFolderLimit.first ? newFolderLimit.second * 1000LL * 1000LL : -1; // convert from MB to B
     opt._confirmExternalStorage = cfgFile.confirmExternalStorage();
     opt._moveFilesToTrash = cfgFile.moveToTrash();
-    opt._usePlaceholders = _definition.usePlaceholders;
+    opt._newFilesArePlaceholders = _definition.usePlaceholders;
 
     QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE");
     if (!chunkSizeEnv.isEmpty()) {

diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index bf907b2ee390e70a23ae63a9206db7eabcaa7123..a674e7365373b2e0c7f133f582e121ce0b3ba262 100644 (file)
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -389,7 +389,7 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
 
     // For placeholder files just create the file and be done
     if (_item->_type == ItemTypePlaceholder) {
-        auto fn = propagator()->placeholderFilePath(_item->_file);
+        auto fn = propagator()->getFilePath(_item->_file);
         qCDebug(lcPropagateDownload) << "creating placeholder file" << fn;
         QFile file(fn);
         file.open(QFile::ReadWrite);
@@ -405,6 +405,7 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
         auto fn = propagator()->placeholderFilePath(_item->_file);
         qCDebug(lcPropagateDownload) << "Downloading file that used to be a placeholder" << fn;
         QFile::remove(fn);
+        propagator()->_journal->deleteFileRecord(_item->_file + ".owncloud");
         _item->_type = ItemTypeFile;
     }
 

diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp
index 0b1aa6194736c429f485a2034415268dfb101579..f540446cc1835d36060de6a48f374248f68c405b 100644 (file)
--- a/src/libsync/propagatorjobs.cpp
+++ b/src/libsync/propagatorjobs.cpp
@@ -96,9 +96,6 @@ void PropagateLocalRemove::start()
         return;
 
     QString filename = propagator()->_localDir + _item->_file;
-    if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload)
-        filename = propagator()->placeholderFilePath(_item->_file);
-
     qCDebug(lcPropagateLocalRemove) << filename;
 
     if (propagator()->localFileNameClash(_item->_file)) {
@@ -253,11 +250,6 @@ void PropagateLocalRename::start()
     QString existingFile = propagator()->getFilePath(_item->_file);
     QString targetFile = propagator()->getFilePath(_item->_renameTarget);
 
-    if (_item->_type == ItemTypePlaceholder || _item->_type == ItemTypePlaceholderDownload) {
-        existingFile = propagator()->placeholderFilePath(_item->_file);
-        targetFile = propagator()->placeholderFilePath(_item->_renameTarget);
-    }
-
     // if the file is a file underneath a moved dir, the _item->file is equal
     // to _item->renameTarget and the file is not moved as a result.
     if (_item->_file != _item->_renameTarget) {

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index 69e7f7d25d6ee91b79c5a3593fb9d67f370b1703..d4e2b090ff487c60927e8bf87b25ac0f0074ae87 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -858,7 +858,7 @@ void SyncEngine::startSync()
         return shouldDiscoverLocally(path);
     };
 
-    _csync_ctx->new_files_are_placeholders = _syncOptions._usePlaceholders;
+    _csync_ctx->new_files_are_placeholders = _syncOptions._newFilesArePlaceholders;
 
     // If needed, make sure we have up to date E2E information before the
     // discovery phase, otherwise we start right away

diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h
index e9e97efec3bda25a2cf8bd218f0c9360b5c3f8ec..f442c380bb1cdbc09004b1be5c2762a00cb507cb 100644 (file)
--- a/src/libsync/syncoptions.h
+++ b/src/libsync/syncoptions.h
@@ -37,7 +37,7 @@ struct SyncOptions
     bool _moveFilesToTrash = false;
 
     /** Create a placeholder for new files instead of downloading */
-    bool _usePlaceholders = false;
+    bool _newFilesArePlaceholders = false;
 
     /** The initial un-adjusted chunk size in bytes for chunked uploads, both
      * for old and new chunking algorithm, which classifies the item to be chunked

diff --git a/test/testsyncplaceholders.cpp b/test/testsyncplaceholders.cpp
index b760354a846900c6cd0fa9266a43dee46e1b5430..a62928c33bbbd94b2a8d2b279c27c3b1850c80f3 100644 (file)
--- a/test/testsyncplaceholders.cpp
+++ b/test/testsyncplaceholders.cpp
@@ -53,7 +53,7 @@ private slots:
 
         FakeFolder fakeFolder{FileInfo()};
         SyncOptions syncOptions;
-        syncOptions._usePlaceholders = true;
+        syncOptions._newFilesArePlaceholders = true;
         fakeFolder.syncEngine().setSyncOptions(syncOptions);
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
@@ -72,8 +72,8 @@ private slots:
         QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
-        QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW));
-        QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder);
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NEW));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder);
         cleanup();
 
         // Another sync doesn't actually lead to changes
@@ -81,6 +81,17 @@ private slots:
         QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder);
+        QVERIFY(completeSpy.isEmpty());
+        cleanup();
+
+        // Not even when the remote is rediscovered
+        fakeFolder.syncJournal().forceRemoteDiscoveryNextSync();
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
+        QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder);
         QVERIFY(completeSpy.isEmpty());
         cleanup();
 
@@ -90,9 +101,9 @@ private slots:
         QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
-        QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_UPDATE_METADATA));
-        QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder);
-        QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65);
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_UPDATE_METADATA));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder);
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._fileSize, 65);
         cleanup();
 
         // If the local placeholder file is removed, it'll just be recreated
@@ -103,9 +114,9 @@ private slots:
         QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
-        QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW));
-        QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypePlaceholder);
-        QCOMPARE(dbRecord(fakeFolder, "A/a1")._fileSize, 65);
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_NEW));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._type, ItemTypePlaceholder);
+        QCOMPARE(dbRecord(fakeFolder, "A/a1.owncloud")._fileSize, 65);
         cleanup();
 
         // Remote rename is propagated
@@ -117,8 +128,8 @@ private slots:
         QVERIFY(fakeFolder.currentLocalState().find("A/a1m.owncloud"));
         QVERIFY(!fakeFolder.currentRemoteState().find("A/a1"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/a1m"));
-        QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_RENAME));
-        QCOMPARE(dbRecord(fakeFolder, "A/a1m")._type, ItemTypePlaceholder);
+        QVERIFY(itemInstruction(completeSpy, "A/a1m.owncloud", CSYNC_INSTRUCTION_RENAME));
+        QCOMPARE(dbRecord(fakeFolder, "A/a1m.owncloud")._type, ItemTypePlaceholder);
         cleanup();
 
         // Remote remove is propagated
@@ -126,8 +137,9 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(!fakeFolder.currentLocalState().find("A/a1m.owncloud"));
         QVERIFY(!fakeFolder.currentRemoteState().find("A/a1m"));
-        QVERIFY(itemInstruction(completeSpy, "A/a1m", CSYNC_INSTRUCTION_REMOVE));
-        QVERIFY(!dbRecord(fakeFolder, "A/a1m").isValid());
+        QVERIFY(itemInstruction(completeSpy, "A/a1m.owncloud", CSYNC_INSTRUCTION_REMOVE));
+        QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a1m.owncloud").isValid());
         cleanup();
     }
 
@@ -135,7 +147,7 @@ private slots:
     {
         FakeFolder fakeFolder{ FileInfo() };
         SyncOptions syncOptions;
-        syncOptions._usePlaceholders = true;
+        syncOptions._newFilesArePlaceholders = true;
         fakeFolder.syncEngine().setSyncOptions(syncOptions);
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
@@ -145,9 +157,6 @@ private slots:
         };
         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);
@@ -155,6 +164,8 @@ private slots:
         fakeFolder.remoteModifier().mkdir("B");
         fakeFolder.remoteModifier().insert("B/b1", 64);
         fakeFolder.remoteModifier().insert("B/b2", 64);
+        fakeFolder.remoteModifier().mkdir("C");
+        fakeFolder.remoteModifier().insert("C/c1", 64);
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
         QVERIFY(fakeFolder.currentLocalState().find("B/b2.owncloud"));
@@ -162,12 +173,15 @@ private slots:
 
         // A: the correct file and a conflicting file are added, placeholders stay
         // B: same setup, but the placeholders are deleted by the user
+        // C: user adds a *directory* locally
         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");
+        fakeFolder.localModifier().mkdir("C/c1");
+        fakeFolder.localModifier().insert("C/c1/foo");
         QVERIFY(fakeFolder.syncOnce());
 
         // Everything is CONFLICT since mtimes are different even for a1/b1
@@ -175,21 +189,29 @@ private slots:
         QVERIFY(itemInstruction(completeSpy, "A/a2", CSYNC_INSTRUCTION_CONFLICT));
         QVERIFY(itemInstruction(completeSpy, "B/b1", CSYNC_INSTRUCTION_CONFLICT));
         QVERIFY(itemInstruction(completeSpy, "B/b2", CSYNC_INSTRUCTION_CONFLICT));
+        QVERIFY(itemInstruction(completeSpy, "C/c1", 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"));
+        QVERIFY(!fakeFolder.currentLocalState().find("C/c1.owncloud"));
 
         // conflict files should exist
-        QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 2);
+        QCOMPARE(fakeFolder.syncJournal().conflictRecordPaths().size(), 3);
 
         // 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);
+        QCOMPARE(dbRecord(fakeFolder, "C/c1")._type, ItemTypeFile);
+        QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a2.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "B/b1.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "B/b2.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "C/c1.owncloud").isValid());
 
         cleanup();
     }
@@ -198,7 +220,7 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         SyncOptions syncOptions;
-        syncOptions._usePlaceholders = true;
+        syncOptions._newFilesArePlaceholders = true;
         fakeFolder.syncEngine().setSyncOptions(syncOptions);
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
@@ -227,8 +249,8 @@ private slots:
         QVERIFY(!fakeFolder.currentLocalState().find("A/new"));
         QVERIFY(fakeFolder.currentLocalState().find("A/new.owncloud"));
         QVERIFY(fakeFolder.currentRemoteState().find("A/new"));
-        QVERIFY(itemInstruction(completeSpy, "A/new", CSYNC_INSTRUCTION_NEW));
-        QCOMPARE(dbRecord(fakeFolder, "A/new")._type, ItemTypePlaceholder);
+        QVERIFY(itemInstruction(completeSpy, "A/new.owncloud", CSYNC_INSTRUCTION_NEW));
+        QCOMPARE(dbRecord(fakeFolder, "A/new.owncloud")._type, ItemTypePlaceholder);
         cleanup();
     }
 
@@ -236,7 +258,7 @@ private slots:
     {
         FakeFolder fakeFolder{FileInfo()};
         SyncOptions syncOptions;
-        syncOptions._usePlaceholders = true;
+        syncOptions._newFilesArePlaceholders = true;
         fakeFolder.syncEngine().setSyncOptions(syncOptions);
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
@@ -249,7 +271,7 @@ private slots:
         auto triggerDownload = [&](const QByteArray &path) {
             auto &journal = fakeFolder.syncJournal();
             SyncJournalFileRecord record;
-            journal.getFileRecord(path, &record);
+            journal.getFileRecord(path + ".owncloud", &record);
             if (!record.isValid())
                 return;
             record._type = ItemTypePlaceholderDownload;
@@ -288,11 +310,14 @@ private slots:
         fakeFolder.localModifier().remove("A/a6.owncloud");
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(itemInstruction(completeSpy, "A/a1", CSYNC_INSTRUCTION_NEW));
+        QVERIFY(itemInstruction(completeSpy, "A/a1.owncloud", CSYNC_INSTRUCTION_REMOVE));
         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/a2.owncloud", CSYNC_INSTRUCTION_REMOVE));
+        QVERIFY(itemInstruction(completeSpy, "A/a3.owncloud", CSYNC_INSTRUCTION_REMOVE));
+        QVERIFY(itemInstruction(completeSpy, "A/a4.owncloud", CSYNC_INSTRUCTION_REMOVE));
         QVERIFY(itemInstruction(completeSpy, "A/a4m", CSYNC_INSTRUCTION_NEW));
         QVERIFY(itemInstruction(completeSpy, "A/a5", CSYNC_INSTRUCTION_CONFLICT));
+        QVERIFY(itemInstruction(completeSpy, "A/a5.owncloud", CSYNC_INSTRUCTION_REMOVE));
         QVERIFY(itemInstruction(completeSpy, "A/a6", CSYNC_INSTRUCTION_CONFLICT));
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         QCOMPARE(dbRecord(fakeFolder, "A/a1")._type, ItemTypeFile);
@@ -301,6 +326,57 @@ private slots:
         QCOMPARE(dbRecord(fakeFolder, "A/a4m")._type, ItemTypeFile);
         QCOMPARE(dbRecord(fakeFolder, "A/a5")._type, ItemTypeFile);
         QCOMPARE(dbRecord(fakeFolder, "A/a6")._type, ItemTypeFile);
+        QVERIFY(!dbRecord(fakeFolder, "A/a1.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a2.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a3.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a4.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a5.owncloud").isValid());
+        QVERIFY(!dbRecord(fakeFolder, "A/a6.owncloud").isValid());
+    }
+
+    // Check what might happen if an older sync client encounters placeholders
+    void testOldVersion()
+    {
+        FakeFolder fakeFolder{ FileInfo() };
+        SyncOptions syncOptions;
+        syncOptions._newFilesArePlaceholders = true;
+        fakeFolder.syncEngine().setSyncOptions(syncOptions);
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // Create a placeholder
+        fakeFolder.remoteModifier().mkdir("A");
+        fakeFolder.remoteModifier().insert("A/a1");
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
+
+        // Simulate an old client by switching the type of all ItemTypePlaceholder
+        // entries in the db to an invalid type.
+        auto &db = fakeFolder.syncJournal();
+        SyncJournalFileRecord rec;
+        db.getFileRecord(QByteArray("A/a1.owncloud"), &rec);
+        QVERIFY(rec.isValid());
+        QCOMPARE(rec._type, ItemTypePlaceholder);
+        rec._type = static_cast<ItemType>(-1);
+        db.setFileRecord(rec);
+
+        // Also switch off new files becoming placeholders
+        syncOptions._newFilesArePlaceholders = false;
+        fakeFolder.syncEngine().setSyncOptions(syncOptions);
+
+        // A sync that doesn't do remote discovery has no effect
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1.owncloud"));
+        QVERIFY(!fakeFolder.currentLocalState().find("A/a1"));
+        QVERIFY(fakeFolder.currentRemoteState().find("A/a1"));
+        QVERIFY(!fakeFolder.currentRemoteState().find("A/a1.owncloud"));
+
+        // But with a remote discovery the placeholders will be removed and
+        // the remote files will be downloaded.
+        db.forceRemoteDiscoveryNextSync();
+        QVERIFY(fakeFolder.syncOnce());
+        QVERIFY(fakeFolder.currentLocalState().find("A/a1"));
+        QVERIFY(!fakeFolder.currentLocalState().find("A/a1.owncloud"));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
 };