VFS: Do not overwrite existing files by placeholder

For issue #7557 and #7556

Note: this change the API of the VFS plugin, so the VFS plugin needs small
adaptations

6 files changed, 56 insertions, 24 deletions

diff --git a/src/common/result.h b/src/common/result.h
index 61805982e..579914f5c 100644
--- a/src/common/result.h
+++ b/src/common/result.h
@@ -129,6 +129,18 @@ public:
 };
 
 namespace detail {
+    struct NoResultData{};
+}
+
+template <typename Error>
+class Result<void, Error> : public Result<detail::NoResultData, Error>
+{
+public:
+    using Result<detail::NoResultData, Error>::Result;
+    Result() : Result(detail::NoResultData{}) {}
+};
+
+namespace detail {
 struct OptionalNoErrorData{};
 }
 
diff --git a/src/common/vfs.h b/src/common/vfs.h
index ca28b631b..669e3f5b6 100644
--- a/src/common/vfs.h
+++ b/src/common/vfs.h
@@ -151,20 +151,18 @@ public:
      *
      * If the remote metadata changes, the local placeholder's metadata should possibly
      * change as well.
-     *
-     * Returning false and setting error indicates an error.
      */
-    virtual bool updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId, QString *error) = 0;
+    virtual Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;
 
     /// Create a new dehydrated placeholder. Called from PropagateDownload.
-    virtual void createPlaceholder(const SyncFileItem &item) = 0;
+    virtual Result<void, QString> createPlaceholder(const SyncFileItem &item) = 0;
 
     /** Convert a hydrated placeholder to a dehydrated one. Called from PropagateDownlaod.
      *
      * This is different from delete+create because preserving some file metadata
      * (like pin states) may be essential for some vfs plugins.
      */
-    virtual void dehydratePlaceholder(const SyncFileItem &item) = 0;
+    virtual Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) = 0;
 
     /** Discovery hook: even unchanged files may need UPDATE_METADATA.
      *
@@ -289,9 +287,9 @@ public:
     bool socketApiPinStateActionsShown() const override { return false; }
     bool isHydrating() const override { return false; }
 
-    bool updateMetadata(const QString &, time_t, qint64, const QByteArray &, QString *) override { return true; }
-    void createPlaceholder(const SyncFileItem &) override {}
-    void dehydratePlaceholder(const SyncFileItem &) override {}
+    Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
+    Result<void, QString> createPlaceholder(const SyncFileItem &) override { return {}; }
+    Result<void, QString> dehydratePlaceholder(const SyncFileItem &) override { return {}; }
     void convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override {}
 
     bool needsMetadataUpdate(const SyncFileItem &) override { return false; }
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 7ac691df1..bdd250cee 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -424,7 +424,11 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
         }
 
         qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file;
-        vfs->dehydratePlaceholder(*_item);
+        auto r = vfs->dehydratePlaceholder(*_item);
+        if (!r) {
+            done(SyncFileItem::NormalError, r.error());
+            return;
+        }
         propagator()->_journal->deleteFileRecord(_item->_originalFile);
         updateMetadata(false);
         return;
@@ -435,7 +439,11 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
     }
     if (_item->_type == ItemTypeVirtualFile) {
         qCDebug(lcPropagateDownload) << "creating virtual file" << _item->_file;
-        vfs->createPlaceholder(*_item);
+        auto r = vfs->createPlaceholder(*_item);
+        if (!r) {
+            done(SyncFileItem::NormalError, r.error());
+            return;
+        }
         updateMetadata(false);
         return;
     }
diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index bb2a83a25..a185dd2b0 100644
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -351,10 +351,10 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
 
             // Update on-disk virtual file metadata
             if (item->_type == ItemTypeVirtualFile) {
-                QString error;
-                if (!_syncOptions._vfs->updateMetadata(filePath, item->_modtime, item->_size, item->_fileId, &error)) {
+                auto r = _syncOptions._vfs->updateMetadata(filePath, item->_modtime, item->_size, item->_fileId);
+                if (!r) {
                     item->_instruction = CSYNC_INSTRUCTION_ERROR;
-                    item->_errorString = tr("Could not update virtual file metadata: %1").arg(error);
+                    item->_errorString = tr("Could not update virtual file metadata: %1").arg(r.error());
                     return;
                 }
             }
diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp
index 6827126a9..be64b62a6 100644
--- a/src/libsync/vfs/suffix/vfs_suffix.cpp
+++ b/src/libsync/vfs/suffix/vfs_suffix.cpp
@@ -68,34 +68,47 @@ bool VfsSuffix::isHydrating() const
     return false;
 }
 
-bool VfsSuffix::updateMetadata(const QString &filePath, time_t modtime, qint64, const QByteArray &, QString *)
+Result<void, QString> VfsSuffix::updateMetadata(const QString &filePath, time_t modtime, qint64, const QByteArray &)
 {
     FileSystem::setModTime(filePath, modtime);
-    return true;
+    return {};
 }
 
-void VfsSuffix::createPlaceholder(const SyncFileItem &item)
+Result<void, QString> VfsSuffix::createPlaceholder(const SyncFileItem &item)
 {
     // The concrete shape of the placeholder is also used in isDehydratedPlaceholder() below
     QString fn = _setupParams.filesystemPath + item._file;
     if (!fn.endsWith(fileSuffix())) {
         ASSERT(false, "vfs file isn't ending with suffix");
-        return;
+        return QString("vfs file isn't ending with suffix");
     }
 
     QFile file(fn);
-    file.open(QFile::ReadWrite | QFile::Truncate);
+    if (file.exists() && file.size() > 1
+        && !FileSystem::verifyFileUnchanged(fn, item._size, item._modtime)) {
+        return QString("Cannot create a placeholder because a file with the placeholder name already exist");
+    }
+
+    if (!file.open(QFile::ReadWrite | QFile::Truncate))
+        return file.errorString();
+
     file.write(" ");
     file.close();
     FileSystem::setModTime(fn, item._modtime);
+    return {};
 }
 
-void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
+Result<void, QString> VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
 {
-    QFile::remove(_setupParams.filesystemPath + item._file);
     SyncFileItem virtualItem(item);
     virtualItem._file = item._renameTarget;
-    createPlaceholder(virtualItem);
+    auto r = createPlaceholder(virtualItem);
+    if (!r)
+        return r;
+
+    if (item._file != item._renameTarget) { // can be the same when renaming foo -> foo.owncloud to dehydrate
+        QFile::remove(_setupParams.filesystemPath + item._file);
+    }
 
     // Move the item's pin state
     auto pin = _setupParams.journal->internalPinStates().rawForPath(item._file.toUtf8());
@@ -108,6 +121,7 @@ void VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
     pin = pinState(item._renameTarget);
     if (pin && *pin == PinState::AlwaysLocal)
         setPinState(item._renameTarget, PinState::Unspecified);
+    return {};
 }
 
 void VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h
index 3426c39fe..ce70c2ebb 100644
--- a/src/libsync/vfs/suffix/vfs_suffix.h
+++ b/src/libsync/vfs/suffix/vfs_suffix.h
@@ -38,10 +38,10 @@ public:
     bool socketApiPinStateActionsShown() const override { return true; }
     bool isHydrating() const override;
 
-    bool updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId, QString *error) override;
+    Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) override;
 
-    void createPlaceholder(const SyncFileItem &item) override;
-    void dehydratePlaceholder(const SyncFileItem &item) override;
+    Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
+    Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
     void convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override;
 
     bool needsMetadataUpdate(const SyncFileItem &) override { return false; }