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 <matthieu.gallien@nextcloud.com>
This commit is contained in:
Matthieu Gallien 2023-11-09 22:38:41 +01:00
parent 369296d6ed
commit b77fc9d4ff
No known key found for this signature in database
GPG key ID: 7D0F74F05C22F553
13 changed files with 130 additions and 87 deletions

View file

@ -113,6 +113,15 @@ public:
};
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 @@ public:
* 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,
Q_REQUIRED_RESULT virtual Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename,
const SyncFileItem &item,
const QString &replacesFile = QString()) = 0;
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 @@ public:
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; }

View file

@ -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;

View file

@ -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) {
@ -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");

View file

@ -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 @@ public:
*
* 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 @@ public:
*
* 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;

View file

@ -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;

View file

@ -48,6 +48,30 @@ constexpr auto syncRootFlagsNoCfApiContextMenu = 2;
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 @@ void CALLBACK cfApiFetchDataCallback(const CF_CALLBACK_INFO *callbackInfo, const
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 cfApiCallbacks[] = {
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 @@ std::wstring pathForHandle(const OCC::CfApiWrapper::FileHandle &handle)
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) {
@ -750,44 +797,7 @@ OCC::Result<void, QString> OCC::CfApiWrapper::createPlaceholderInfo(const QStrin
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 +872,8 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::co
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);
}

View file

@ -97,6 +97,7 @@ NEXTCLOUD_CFAPI_EXPORT Result<void, QString> createPlaceholderInfo(const QString
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());
}

View file

@ -222,7 +222,7 @@ Result<void, QString> VfsCfApi::dehydratePlaceholder(const SyncFileItem &item)
}
}
Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType)
{
const auto localPath = QDir::toNativeSeparators(filename);
@ -238,7 +238,11 @@ Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(
const auto replacesPath = QDir::toNativeSeparators(replacesFile);
if (cfapi::findPlaceholderInfo(localPath)) {
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);
}

View file

@ -45,7 +45,7 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType) override;
bool needsMetadataUpdate(const SyncFileItem &) override;
bool isDehydratedPlaceholder(const QString &filePath) override;

View file

@ -138,7 +138,7 @@ Result<void, QString> VfsSuffix::dehydratePlaceholder(const SyncFileItem &item)
return {};
}
Result<Vfs::ConvertToPlaceholderResult, QString> VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsSuffix::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, UpdateMetadataTypes)
{
// Nothing necessary
return Vfs::ConvertToPlaceholderResult::Ok;

View file

@ -42,7 +42,7 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &) override;
Result<Vfs::ConvertToPlaceholderResult, QString> 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;

View file

@ -122,7 +122,10 @@ Result<void, QString> VfsXAttr::dehydratePlaceholder(const SyncFileItem &item)
return {};
}
Result<Vfs::ConvertToPlaceholderResult, QString> VfsXAttr::convertToPlaceholder(const QString &, const SyncFileItem &, const QString &)
Result<Vfs::ConvertToPlaceholderResult, QString> VfsXAttr::convertToPlaceholder(const QString &,
const SyncFileItem &,
const QString &,
UpdateMetadataTypes)
{
// Nothing necessary
return {ConvertToPlaceholderResult::Ok};

View file

@ -42,7 +42,10 @@ public:
Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile) override;
Result<ConvertToPlaceholderResult, QString> 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;