Skip to content

Commit

Permalink
avoid modifying a placeholder (virtual files) when not needed
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu Gallien <[email protected]>
  • Loading branch information
mgallien committed Nov 9, 2023
1 parent b643773 commit 78efb88
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 21 deletions.
19 changes: 14 additions & 5 deletions src/common/vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Mode> modeFromString(const QString &str);

Expand Down Expand Up @@ -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<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;
Expand Down Expand Up @@ -311,7 +320,7 @@ class OCSYNC_EXPORT VfsOff : public Vfs
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; }
Expand Down
13 changes: 8 additions & 5 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1001,13 +1001,16 @@ 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);
Expand Down Expand Up @@ -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");
Expand Down
15 changes: 10 additions & 5 deletions src/libsync/owncloudpropagator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -592,7 +594,7 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject
*
* 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.
*
Expand All @@ -601,8 +603,11 @@ class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject
*
* 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;

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,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;
Expand Down
8 changes: 6 additions & 2 deletions src/libsync/vfs/cfapi/vfs_cfapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -238,7 +238,11 @@ Result<Vfs::ConvertToPlaceholderResult, QString> 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 Vfs::ConvertToPlaceholderResult::Ok;
}
} else {
return cfapi::convertToPlaceholder(localPath, item._modtime, item._size, item._fileId, replacesPath);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/cfapi/vfs_cfapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class VfsCfApi : public Vfs

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;
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/suffix/vfs_suffix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 updateType)
{
// Nothing necessary
return Vfs::ConvertToPlaceholderResult::Ok;
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/suffix/vfs_suffix.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class VfsSuffix : public Vfs

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;
Expand Down

0 comments on commit 78efb88

Please sign in to comment.