From 7aa9936304aa850826dcabd9e919ad5144db27c8 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 9 Nov 2023 22:38:41 +0100 Subject: [PATCH] avoid modifying a placeholder (virtual files) when not needed acoid modifying some metadata of the placeholder when this placeholder has just been uploaded to the server (will avoid truncating the timestamps) Close #6190 Signed-off-by: Matthieu Gallien --- src/common/vfs.h | 19 ++++++++++++----- src/libsync/owncloudpropagator.cpp | 15 +++++++------ src/libsync/owncloudpropagator.h | 15 ++++++++----- src/libsync/propagateupload.cpp | 2 +- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 29 ++++++++++++++++++++++++++ src/libsync/vfs/cfapi/cfapiwrapper.h | 1 + src/libsync/vfs/cfapi/vfs_cfapi.cpp | 8 +++++-- src/libsync/vfs/cfapi/vfs_cfapi.h | 2 +- src/libsync/vfs/suffix/vfs_suffix.cpp | 2 +- src/libsync/vfs/suffix/vfs_suffix.h | 2 +- src/libsync/vfs/xattr/vfs_xattr.cpp | 5 ++++- src/libsync/vfs/xattr/vfs_xattr.h | 5 ++++- 12 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/common/vfs.h b/src/common/vfs.h index 17a4c48e7aeb7..079bccb46a381 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -113,6 +113,15 @@ class OCSYNC_EXPORT Vfs : public QObject }; Q_ENUM(ConvertToPlaceholderResult) + enum UpdateMetadataType { + DatabaseMetadata = 0 >> 1, + FileMetadata = 0 >> 2, + AllMetadata = DatabaseMetadata | FileMetadata, + }; + + Q_DECLARE_FLAGS(UpdateMetadataTypes, UpdateMetadataType) + Q_FLAG(UpdateMetadataType) + static QString modeToString(Mode mode); static Optional modeFromString(const QString &str); @@ -203,10 +212,10 @@ class OCSYNC_EXPORT Vfs : public QObject * new placeholder shall supersede, for rename-replace actions with new downloads, * for example. */ - Q_REQUIRED_RESULT virtual Result convertToPlaceholder( - const QString &filename, - const SyncFileItem &item, - const QString &replacesFile = QString()) = 0; + Q_REQUIRED_RESULT virtual Result 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 @@ class OCSYNC_EXPORT VfsOff : public Vfs Result updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; } Result createPlaceholder(const SyncFileItem &) override { return {}; } Result dehydratePlaceholder(const SyncFileItem &) override { return {}; } - Result convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) override { return ConvertToPlaceholderResult::Ok; } + Result 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; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 488a83a3f0ff5..b2a058088c303 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1001,16 +1001,19 @@ QString OwncloudPropagator::adjustRenamedPath(const QString &original) const return OCC::adjustRenamedPath(_renamedDirectories, original); } -Result OwncloudPropagator::updateMetadata(const SyncFileItem &item) +Result 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 OwncloudPropagator::staticUpdateMetadata(const SyncFileItem &item, const QString localDir, - Vfs *vfs, SyncJournalDb *const journal) +Result 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) { @@ -1582,7 +1585,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"); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 46f0ea4e5ec2e..440b98551e51f 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -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 namespace OCC { @@ -592,7 +594,7 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject * * Will also trigger a Vfs::convertToPlaceholder. */ - Result updateMetadata(const SyncFileItem &item); + Result updateMetadata(const SyncFileItem &item, Vfs::UpdateMetadataTypes updateType = Vfs::AllMetadata); /** Update the database for an item. * @@ -601,8 +603,11 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject * * Will also trigger a Vfs::convertToPlaceholder. */ - static Result staticUpdateMetadata(const SyncFileItem &item, const QString localDir, - Vfs *vfs, SyncJournalDb * const journal); + static Result staticUpdateMetadata(const SyncFileItem &item, + const QString localDir, + Vfs *vfs, + SyncJournalDb * const journal, + Vfs::UpdateMetadataTypes updateType); Q_REQUIRED_RESULT bool isDelayedUploadItem(const SyncFileItemPtr &item) const; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 2a7a7bbaabc89..f74727ec7bb80 100644 --- a/src/libsync/propagateupload.cpp +++ b/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; diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index ee729896300a5..e16a5c079c824 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -862,3 +862,32 @@ OCC::Result OCC::CfApiWrapper::co return stateResult; } } + +OCC::Result OCC::CfApiWrapper::updatePlaceholderMarkInSync(const QString &path, const QByteArray &fileId, const QString &replacesPath) +{ + 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); + + const qint64 result = CfUpdatePlaceholder(handleForPath(path).get(), 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, NoRecurse)) { + return { "Couldn't restore pin state" }; + } + + return OCC::Vfs::ConvertToPlaceholderResult::Ok; +} diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.h b/src/libsync/vfs/cfapi/cfapiwrapper.h index 10cb51aaa82bb..94ebff914cbe6 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.h +++ b/src/libsync/vfs/cfapi/cfapiwrapper.h @@ -97,6 +97,7 @@ NEXTCLOUD_CFAPI_EXPORT Result createPlaceholderInfo(const QString NEXTCLOUD_CFAPI_EXPORT Result updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString()); NEXTCLOUD_CFAPI_EXPORT Result convertToPlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath); NEXTCLOUD_CFAPI_EXPORT Result dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId); +NEXTCLOUD_CFAPI_EXPORT Result updatePlaceholderMarkInSync(const QString &path, const QByteArray &fileId, const QString &replacesPath = QString()); } diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 595cc52159185..e7905fe6ca7c0 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -222,7 +222,7 @@ Result VfsCfApi::dehydratePlaceholder(const SyncFileItem &item) } } -Result VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) +Result VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType) { const auto localPath = QDir::toNativeSeparators(filename); @@ -238,7 +238,11 @@ Result VfsCfApi::convertToPlaceholder( const auto replacesPath = QDir::toNativeSeparators(replacesFile); if (cfapi::findPlaceholderInfo(localPath)) { - return cfapi::updatePlaceholderInfo(localPath, item._modtime, item._size, item._fileId, replacesPath); + if (updateType & Vfs::UpdateMetadataType::FileMetadata) { + return cfapi::updatePlaceholderInfo(localPath, item._modtime, item._size, item._fileId, replacesPath); + } else { + return cfapi::updatePlaceholderMarkInSync(localPath, item._fileId, replacesPath); + } } else { return cfapi::convertToPlaceholder(localPath, item._modtime, item._size, item._fileId, replacesPath); } diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.h b/src/libsync/vfs/cfapi/vfs_cfapi.h index b675ae389f0e7..ce0d97ea0ccb2 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.h +++ b/src/libsync/vfs/cfapi/vfs_cfapi.h @@ -45,7 +45,7 @@ class VfsCfApi : public Vfs Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; + Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType) override; bool needsMetadataUpdate(const SyncFileItem &) override; bool isDehydratedPlaceholder(const QString &filePath) override; diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp index 6fcb1d2f51da2..f4d5e9a05d72b 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.cpp +++ b/src/libsync/vfs/suffix/vfs_suffix.cpp @@ -138,7 +138,7 @@ Result VfsSuffix::dehydratePlaceholder(const SyncFileItem &item) return {}; } -Result VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) +Result VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, UpdateMetadataTypes) { // Nothing necessary return Vfs::ConvertToPlaceholderResult::Ok; diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h index 6169290ba3224..3164802d9c83d 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.h +++ b/src/libsync/vfs/suffix/vfs_suffix.h @@ -42,7 +42,7 @@ class VfsSuffix : public Vfs Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override; + Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &, UpdateMetadataTypes updateType) override; bool needsMetadataUpdate(const SyncFileItem &) override { return false; } bool isDehydratedPlaceholder(const QString &filePath) override; diff --git a/src/libsync/vfs/xattr/vfs_xattr.cpp b/src/libsync/vfs/xattr/vfs_xattr.cpp index 4300f6d30ad24..f6ba09f1e365d 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.cpp +++ b/src/libsync/vfs/xattr/vfs_xattr.cpp @@ -122,7 +122,10 @@ Result VfsXAttr::dehydratePlaceholder(const SyncFileItem &item) return {}; } -Result VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &) +Result VfsXAttr::convertToPlaceholder(const QString &, + const SyncFileItem &, + const QString &, + UpdateMetadataTypes) { // Nothing necessary return {ConvertToPlaceholderResult::Ok}; diff --git a/src/libsync/vfs/xattr/vfs_xattr.h b/src/libsync/vfs/xattr/vfs_xattr.h index 118d4d2864f9b..b9fc9548c4707 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.h +++ b/src/libsync/vfs/xattr/vfs_xattr.h @@ -42,7 +42,10 @@ class VfsXAttr : public Vfs Result createPlaceholder(const SyncFileItem &item) override; Result dehydratePlaceholder(const SyncFileItem &item) override; - Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override; + Result convertToPlaceholder(const QString &filename, + const SyncFileItem &item, + const QString &replacesFile, + UpdateMetadataTypes updateType) override; bool needsMetadataUpdate(const SyncFileItem &item) override; bool isDehydratedPlaceholder(const QString &filePath) override;