avoid modifying a placeholder (virtual files) when not needed

src/common/vfs.h

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Christian Kamm <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -13,17 +13,17 @@
  */
 #pragma once
 
+#include "ocsynclib.h"
+#include "result.h"
+#include "syncfilestatus.h"
+#include "pinstate.h"
+
 #include <QObject>
 #include <QScopedPointer>
 #include <QSharedPointer>
 
 #include <memory>
 
-#include "ocsynclib.h"
-#include "result.h"
-#include "syncfilestatus.h"
-#include "pinstate.h"
-
 using csync_file_stat_t = struct csync_file_stat_s;
 
 namespace OCC {
@@ -113,6 +113,15 @@
     };
     Q_ENUM(ConvertToPlaceholderResult)
 
+    enum UpdateMetadataType {
+        DatabaseMetadata = 1 << 0,
+        FileMetadata = 1 << 1,
+        AllMetadata = DatabaseMetadata | FileMetadata,
+    };
+
+    Q_DECLARE_FLAGS(UpdateMetadataTypes, UpdateMetadataType)
+    Q_FLAG(UpdateMetadataType)
+
     static QString modeToString(Mode mode);
     static Optional<Mode> modeFromString(const QString &str);
 
@@ -203,10 +212,10 @@
      * new placeholder shall supersede, for rename-replace actions with new downloads,
      * for example.
      */
-    Q_REQUIRED_RESULT virtual Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(
-        const QString &filename,
-        const SyncFileItem &item,
-        const QString &replacesFile = QString()) = 0;
+    Q_REQUIRED_RESULT virtual Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename,
+                                                                                                    const SyncFileItem &item,
+                                                                                                    const QString &replacesFile = {},
+                                                                                                    UpdateMetadataTypes updateType = AllMetadata) = 0;
 
     /// Determine whether the file at the given absolute path is a dehydrated placeholder.
     Q_REQUIRED_RESULT virtual bool isDehydratedPlaceholder(const QString &filePath) = 0;
@@ -311,7 +320,7 @@
     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 {}; }
-    Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return ConvertToPlaceholderResult::Ok; }
+    Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, UpdateMetadataTypes) override { return ConvertToPlaceholderResult::Ok; }
 
     bool needsMetadataUpdate(const SyncFileItem &) override { return false; }
     bool isDehydratedPlaceholder(const QString &) override { return false; }

src/gui/folderwatcher.cpp

@@ -146,6 +146,7 @@ void FolderWatcher::startNotificationTestWhenReady()
     auto path = _testNotificationPath;
     if (QFile::exists(path)) {
         auto mtime = FileSystem::getModTime(path);
+        qCDebug(lcFolderWatcher()) << "setModTime" << path << (mtime + 1);
         FileSystem::setModTime(path, mtime + 1);
     } else {
         QFile f(path);

src/libsync/bulkpropagatorjob.cpp

@@ -479,7 +479,7 @@ void BulkPropagatorJob::adjustLastJobTimeout(AbstractNetworkJob *job, qint64 fil
 void BulkPropagatorJob::finalizeOneFile(const BulkUploadItem &oneFile)
 {
     // Update the database entry
-    const auto result = propagator()->updateMetadata(*oneFile._item);
+    const auto result = propagator()->updateMetadata(*oneFile._item, Vfs::UpdateMetadataType::DatabaseMetadata);
     if (!result) {
         done(oneFile._item, SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()), ErrorCategory::GenericError);
         return;

src/libsync/owncloudpropagator.cpp

@@ -1001,16 +1001,19 @@ QString OwncloudPropagator::adjustRenamedPath(const QString &original) const
     return OCC::adjustRenamedPath(_renamedDirectories, original);
 }
 
-Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::updateMetadata(const SyncFileItem &item)
+Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::updateMetadata(const SyncFileItem &item, Vfs::UpdateMetadataTypes updateType)
 {
-    return OwncloudPropagator::staticUpdateMetadata(item, _localDir, syncOptions()._vfs.data(), _journal);
+    return OwncloudPropagator::staticUpdateMetadata(item, _localDir, syncOptions()._vfs.data(), _journal, updateType);
 }
 
-Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::staticUpdateMetadata(const SyncFileItem &item, const QString localDir,
-                                                                                          Vfs *vfs, SyncJournalDb *const journal)
+Result<Vfs::ConvertToPlaceholderResult, QString> OwncloudPropagator::staticUpdateMetadata(const SyncFileItem &item,
+                                                                                          const QString localDir,
+                                                                                          Vfs *vfs,
+                                                                                          SyncJournalDb *const journal,
+                                                                                          Vfs::UpdateMetadataTypes updateType)
 {
     const QString fsPath = localDir + item.destination();
-    const auto result = vfs->convertToPlaceholder(fsPath, item);
+    const auto result = vfs->convertToPlaceholder(fsPath, item, {}, updateType);
     if (!result) {
         return result.error();
     } else if (*result == Vfs::ConvertToPlaceholderResult::Locked) {
@@ -1375,6 +1378,7 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
                 qCWarning(lcDirectory) << "Error writing to the database for file" << _item->_file;
             }
 
+            qCDebug(lcPropagator()) << "setModTime" << propagator()->fullLocalPath(_item->destination()) << _item->_modtime;
             FileSystem::setModTime(propagator()->fullLocalPath(_item->destination()), _item->_modtime);
         }
 
@@ -1582,7 +1586,7 @@ void CleanupPollsJob::slotPollFinished()
     } else if (job->_item->_status != SyncFileItem::Success) {
         qCWarning(lcCleanupPolls) << "There was an error with file " << job->_item->_file << job->_item->_errorString;
     } else {
-        if (!OwncloudPropagator::staticUpdateMetadata(*job->_item, _localPath, _vfs.data(), _journal)) {
+        if (!OwncloudPropagator::staticUpdateMetadata(*job->_item, _localPath, _vfs.data(), _journal, Vfs::AllMetadata)) {
             qCWarning(lcCleanupPolls) << "database error";
             job->_item->_status = SyncFileItem::FatalError;
             job->_item->_errorString = tr("Error writing metadata to the database");

src/libsync/owncloudpropagator.h

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Olivier Goffart <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -27,13 +27,15 @@
 
 #include "accountfwd.h"
 #include "bandwidthmanager.h"
-#include "common/syncjournaldb.h"
-#include "common/utility.h"
 #include "csync.h"
 #include "progressdispatcher.h"
 #include "syncfileitem.h"
 #include "syncoptions.h"
 
+#include "common/syncjournaldb.h"
+#include "common/utility.h"
+#include "common/vfs.h"
+
 #include <deque>
 
 namespace OCC {
@@ -592,7 +594,7 @@
      *
      * Will also trigger a Vfs::convertToPlaceholder.
      */
-    Result<Vfs::ConvertToPlaceholderResult, QString> updateMetadata(const SyncFileItem &item);
+    Result<Vfs::ConvertToPlaceholderResult, QString> updateMetadata(const SyncFileItem &item, Vfs::UpdateMetadataTypes updateType = Vfs::AllMetadata);
 
     /** Update the database for an item.
      *
@@ -601,8 +603,11 @@
      *
      * Will also trigger a Vfs::convertToPlaceholder.
      */
-    static Result<Vfs::ConvertToPlaceholderResult, QString> staticUpdateMetadata(const SyncFileItem &item, const QString localDir,
-                                                                                 Vfs *vfs, SyncJournalDb * const journal);
+    static Result<Vfs::ConvertToPlaceholderResult, QString> staticUpdateMetadata(const SyncFileItem &item,
+                                                                                 const QString localDir,
+                                                                                 Vfs *vfs,
+                                                                                 SyncJournalDb * const journal,
+                                                                                 Vfs::UpdateMetadataTypes updateType);
 
     Q_REQUIRED_RESULT bool isDelayedUploadItem(const SyncFileItemPtr &item) const;
 

src/libsync/propagatedownload.cpp

@@ -606,6 +606,7 @@ void PropagateDownloadFile::conflictChecksumComputed(const QByteArray &checksumT
         }
         if (_item->_modtime != _item->_previousModtime) {
             Q_ASSERT(_item->_modtime > 0);
+            qCDebug(lcPropagateDownload()) << "setModTime" << fn << _item->_modtime;
             FileSystem::setModTime(fn, _item->_modtime);
             emit propagator()->touchedFile(fn);
         }
@@ -1162,6 +1163,7 @@ void PropagateDownloadFile::downloadFinished()
     if (_item->_modtime <= 0) {
         qCWarning(lcPropagateDownload()) << "invalid modified time" << _item->_file << _item->_modtime;
     }
+    qCDebug(lcPropagateDownload()) << "setModTime" << _tmpFile.fileName() << _item->_modtime;
     FileSystem::setModTime(_tmpFile.fileName(), _item->_modtime);
     // We need to fetch the time again because some file systems such as FAT have worse than a second
     // Accuracy, and we really need the time from the file system. (#3103)

src/libsync/propagateupload.cpp

@@ -800,7 +800,7 @@ void PropagateUploadFileCommon::finalize()
         quotaIt.value() -= _fileToUpload._size;
 
     // Update the database entry
-    const auto result = propagator()->updateMetadata(*_item);
+    const auto result = propagator()->updateMetadata(*_item, Vfs::DatabaseMetadata);
     if (!result) {
         done(SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error()));
         return;

src/libsync/vfs/cfapi/cfapiwrapper.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Kevin Ottens <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -48,6 +48,30 @@
 
 constexpr auto syncRootManagerRegKey = R"(SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\SyncRootManager)";
 
+DWORD sizeToDWORD(size_t size)
+{
+    return OCC::Utility::convertSizeToDWORD(size);
+}
+
+OCC::PinState cfPinStateToPinState(CF_PIN_STATE state)
+{
+    switch (state) {
+    case CF_PIN_STATE_UNSPECIFIED:
+        return OCC::PinState::Unspecified;
+    case CF_PIN_STATE_PINNED:
+        return OCC::PinState::AlwaysLocal;
+    case CF_PIN_STATE_UNPINNED:
+        return OCC::PinState::OnlineOnly;
+    case CF_PIN_STATE_INHERIT:
+        return OCC::PinState::Inherited;
+    case CF_PIN_STATE_EXCLUDED:
+        return OCC::PinState::Excluded;
+    default:
+        Q_UNREACHABLE();
+        return OCC::PinState::Inherited;
+    }
+}
+
 void cfApiSendTransferInfo(const CF_CONNECTION_KEY &connectionKey, const CF_TRANSFER_KEY &transferKey, NTSTATUS status, void *buffer, qint64 offset, qint64 currentBlockLength, qint64 totalLength)
 {
 
@@ -237,6 +261,53 @@
         sendTransferError();
     }
 }
+
+enum class CfApiUpdateMetadataType {
+    OnlyBasicMetadata,
+    AllMetadata,
+};
+
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderState(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath, CfApiUpdateMetadataType updateType)
+{
+    if (updateType == CfApiUpdateMetadataType::AllMetadata && modtime <= 0) {
+            return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(path).arg(modtime)};
+        }
+
+        const auto info = replacesPath.isEmpty() ? OCC::CfApiWrapper::findPlaceholderInfo(path)
+                                                 : OCC::CfApiWrapper::findPlaceholderInfo(replacesPath);
+        if (!info) {
+            return { "Can't update non existing placeholder info" };
+        }
+
+        const auto previousPinState = cfPinStateToPinState(info->PinState);
+        const auto fileIdentity = QString::fromUtf8(fileId).toStdWString();
+        const auto fileIdentitySize = (fileIdentity.length() + 1) * sizeof(wchar_t);
+
+        CF_FS_METADATA metadata;
+        metadata.FileSize.QuadPart = size;
+        OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.CreationTime);
+        OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.LastWriteTime);
+        OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.LastAccessTime);
+        OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.ChangeTime);
+        metadata.BasicInfo.FileAttributes = 0;
+
+        qCInfo(lcCfApiWrapper) << "updatePlaceholderState" << path << modtime;
+        const qint64 result = CfUpdatePlaceholder(OCC::CfApiWrapper::handleForPath(path).get(), updateType == CfApiUpdateMetadataType::AllMetadata ? &metadata : nullptr,
+                                                  fileIdentity.data(), sizeToDWORD(fileIdentitySize),
+                                                  nullptr, 0, CF_UPDATE_FLAG_MARK_IN_SYNC, nullptr, nullptr);
+
+        if (result != S_OK) {
+            qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << replacesPath;
+            return { "Couldn't update placeholder info" };
+        }
+
+               // Pin state tends to be lost on updates, so restore it every time
+        if (!setPinState(path, previousPinState, OCC::CfApiWrapper::NoRecurse)) {
+            return { "Couldn't restore pin state" };
+        }
+
+        return OCC::Vfs::ConvertToPlaceholderResult::Ok;
+}
 }
 
 void CALLBACK cfApiCancelFetchData(const CF_CALLBACK_INFO *callbackInfo, const CF_CALLBACK_PARAMETERS * /*callbackParameters*/)
@@ -262,11 +333,6 @@
     CF_CALLBACK_REGISTRATION_END
 };
 
-DWORD sizeToDWORD(size_t size)
-{
-    return OCC::Utility::convertSizeToDWORD(size);
-}
-
 void deletePlaceholderInfo(CF_PLACEHOLDER_BASIC_INFO *info)
 {
     auto byte = reinterpret_cast<char *>(info);
@@ -281,25 +347,6 @@
     return std::wstring(buffer);
 }
 
-OCC::PinState cfPinStateToPinState(CF_PIN_STATE state)
-{
-    switch (state) {
-    case CF_PIN_STATE_UNSPECIFIED:
-        return OCC::PinState::Unspecified;
-    case CF_PIN_STATE_PINNED:
-        return OCC::PinState::AlwaysLocal;
-    case CF_PIN_STATE_UNPINNED:
-        return OCC::PinState::OnlineOnly;
-    case CF_PIN_STATE_INHERIT:
-        return OCC::PinState::Inherited;
-    case CF_PIN_STATE_EXCLUDED:
-        return OCC::PinState::Excluded;
-    default:
-        Q_UNREACHABLE();
-        return OCC::PinState::Inherited;
-    }
-}
-
 CF_PIN_STATE pinStateToCfPinState(OCC::PinState state)
 {
     switch (state) {
@@ -732,6 +779,7 @@
         cloudEntry.FsMetadata.FileSize.QuadPart = 0;
     }
 
+    qCDebug(lcCfApiWrapper) << "CfCreatePlaceholders" << path << modtime;
     const qint64 result = CfCreatePlaceholders(localBasePath.data(), &cloudEntry, 1, CF_CREATE_FLAG_NONE, nullptr);
     if (result != S_OK) {
         qCWarning(lcCfApiWrapper) << "Couldn't create placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
@@ -750,44 +798,7 @@
 
 OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
 {
-
-    if (modtime <= 0) {
-        return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(path).arg(modtime)};
-    }
-
-    const auto info = replacesPath.isEmpty() ? findPlaceholderInfo(path)
-                                             : findPlaceholderInfo(replacesPath);
-    if (!info) {
-        return { "Can't update non existing placeholder info" };
-    }
-
-    const auto previousPinState = cfPinStateToPinState(info->PinState);
-    const auto fileIdentity = QString::fromUtf8(fileId).toStdWString();
-    const auto fileIdentitySize = (fileIdentity.length() + 1) * sizeof(wchar_t);
-
-    CF_FS_METADATA metadata;
-    metadata.FileSize.QuadPart = size;
-    OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.CreationTime);
-    OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.LastWriteTime);
-    OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.LastAccessTime);
-    OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.ChangeTime);
-    metadata.BasicInfo.FileAttributes = 0;
-
-    const qint64 result = CfUpdatePlaceholder(handleForPath(path).get(), &metadata,
-                                              fileIdentity.data(), sizeToDWORD(fileIdentitySize),
-                                              nullptr, 0, CF_UPDATE_FLAG_MARK_IN_SYNC, nullptr, nullptr);
-
-    if (result != S_OK) {
-        qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << replacesPath;
-        return { "Couldn't update placeholder info" };
-    }
-
-    // Pin state tends to be lost on updates, so restore it every time
-    if (!setPinState(path, previousPinState, NoRecurse)) {
-        return { "Couldn't restore pin state" };
-    }
-
-    return OCC::Vfs::ConvertToPlaceholderResult::Ok;
+    return updatePlaceholderState(path, modtime, size, fileId, replacesPath, CfApiUpdateMetadataType::AllMetadata);
 }
 
 OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId)
@@ -862,3 +873,8 @@
         return stateResult;
     }
 }
+
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::updatePlaceholderMarkInSync(const QString &path, const QByteArray &fileId, const QString &replacesPath)
+{
+    return updatePlaceholderState(path, {}, {}, fileId, replacesPath, CfApiUpdateMetadataType::OnlyBasicMetadata);
+}

src/libsync/vfs/cfapi/cfapiwrapper.h

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Kevin Ottens <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -97,6 +97,7 @@
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
 NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId);
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &path, const QByteArray &fileId, const QString &replacesPath = QString());
 
 }