Skip to content

Commit

Permalink
if a virtual file change but bothing changed: set it as in sync
Browse files Browse the repository at this point in the history
some software (at least outlook native software) may fiddle with CfApi
placeholder metadta and set a .msg file to be out of sync state when
opening it

in that case, we will let the sync engine go over it and decide what to
do

it is then possible in that case that we would just put it back in "in
sync" state

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
  • Loading branch information
mgallien authored and backportbot[bot] committed Apr 23, 2024
1 parent 907eda4 commit 01c4d29
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 4 deletions.
8 changes: 7 additions & 1 deletion src/common/vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ class OCSYNC_EXPORT Vfs : public QObject
* If the remote metadata changes, the local placeholder's metadata should possibly
* change as well.
*/
Q_REQUIRED_RESULT virtual Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;
[[nodiscard]] virtual Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) = 0;

[[nodiscard]] virtual Result<Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId) = 0;

[[nodiscard]] virtual bool isPlaceHolderInSync(const QString &filePath) const = 0;

/// Create a new dehydrated placeholder. Called from PropagateDownload.
Q_REQUIRED_RESULT virtual Result<void, QString> createPlaceholder(const SyncFileItem &item) = 0;
Expand Down Expand Up @@ -325,6 +329,8 @@ class OCSYNC_EXPORT VfsOff : public Vfs
[[nodiscard]] bool isHydrating() const override { return false; }

Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
Result<Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId) override {Q_UNUSED(filePath) Q_UNUSED(fileId) return {QString{}};}
[[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override { Q_UNUSED(filePath) return true; }
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 &, const UpdateMetadataTypes) override { return ConvertToPlaceholderResult::Ok; }
Expand Down
1 change: 1 addition & 0 deletions src/csync/csync.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ enum SyncInstructions {
CSYNC_INSTRUCTION_UPDATE_METADATA = 1 << 10, /* If the etag has been updated and need to be writen to the db,
but without any propagation (UPDATE|RECONCILE) */
CSYNC_INSTRUCTION_CASE_CLASH_CONFLICT = 1 << 12, /* The file need to be downloaded because it is a case clash conflict (RECONCILE) */
CSYNC_INSTRUCTION_UPDATE_VFS_METADATA = 1 << 13, /* vfs item metadata are out of sync and we need to tell operating system about it */
};

Q_ENUM_NS(SyncInstructions)
Expand Down
9 changes: 7 additions & 2 deletions src/gui/folder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,13 +621,18 @@ void Folder::slotWatchedPathChanged(const QString &path, ChangeReason reason)
spurious = true;

if (auto pinState = _vfs->pinState(relativePath.toString())) {
if (*pinState == PinState::AlwaysLocal && record.isVirtualFile())
if (*pinState == PinState::AlwaysLocal && record.isVirtualFile()) {
spurious = false;
if (*pinState == PinState::OnlineOnly && record.isFile())
}
if (*pinState == PinState::OnlineOnly && record.isFile()) {
spurious = false;
}
} else {
spurious = false;
}
if (spurious && !_vfs->isPlaceHolderInSync(path)) {
spurious = false;
}
}
if (spurious) {
qCInfo(lcFolder) << "Ignoring spurious notification for file" << relativePath;
Expand Down
6 changes: 6 additions & 0 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1680,6 +1680,12 @@ void ProcessDirectoryJob::processFileFinalize(
}
}

if (_discoveryData->_syncOptions._vfs &&
item->_type == CSyncEnums::ItemTypeFile &&
!_discoveryData->_syncOptions._vfs->isPlaceHolderInSync(_discoveryData->_localDir + path._local)) {
item->_instruction = CSyncEnums::CSYNC_INSTRUCTION_UPDATE_VFS_METADATA;
}

if (path._original != path._target && (item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA || item->_instruction == CSYNC_INSTRUCTION_NONE)) {
ASSERT(_dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_RENAME);
// This is because otherwise subitems are not updated! (ideally renaming a directory could
Expand Down
13 changes: 13 additions & 0 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item)
} else {
return new PropagateLocalRename(this, item);
}
case CSYNC_INSTRUCTION_UPDATE_VFS_METADATA:
return new PropagateVfsUpdateMetadataJob(this, item);
case CSYNC_INSTRUCTION_IGNORE:
case CSYNC_INSTRUCTION_ERROR:
return new PropagateIgnoreJob(this, item);
Expand Down Expand Up @@ -1764,4 +1766,15 @@ void PropagateIgnoreJob::start()
done(status, _item->_errorString, ErrorCategory::NoError);
}

void PropagateVfsUpdateMetadataJob::start()

Check warning on line 1769 in src/libsync/owncloudpropagator.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/owncloudpropagator.cpp:1769:37 [readability-convert-member-functions-to-static]

method 'start' can be made static
{
const auto fullFileName = propagator()->fullLocalPath(_item->_file);
const auto result = propagator()->syncOptions()._vfs->updatePlaceholderMarkInSync(fullFileName, _item->_fileId);
emit propagator()->touchedFile(fullFileName);
if (!result) {
qCWarning(lcPropagator()) << "error when updating VFS metadata" << result.error();
}
done(SyncFileItem::Success, {}, {});
}

}
11 changes: 11 additions & 0 deletions src/libsync/owncloudpropagator.h
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,17 @@ class PropagateIgnoreJob : public PropagateItemJob
void start() override;
};

class PropagateVfsUpdateMetadataJob : public PropagateItemJob
{
Q_OBJECT
public:
PropagateVfsUpdateMetadataJob(OwncloudPropagator *propagator, const SyncFileItemPtr &item)
: PropagateItemJob(propagator, item)
{
}
void start() override;
};

class PropagateUploadFileCommon;

class OWNCLOUDSYNC_EXPORT OwncloudPropagator : public QObject
Expand Down
4 changes: 4 additions & 0 deletions src/libsync/progressdispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ QString Progress::asResultString(const SyncFileItem &item)
return QCoreApplication::translate("progress", "Error");
case CSYNC_INSTRUCTION_UPDATE_METADATA:
return QCoreApplication::translate("progress", "Updated local metadata");
case CSYNC_INSTRUCTION_UPDATE_VFS_METADATA:
return QCoreApplication::translate("progress", "Updated local virtual files metadata");
case CSYNC_INSTRUCTION_NONE:
case CSYNC_INSTRUCTION_EVAL:
return QCoreApplication::translate("progress", "Unknown");
Expand Down Expand Up @@ -87,6 +89,8 @@ QString Progress::asActionString(const SyncFileItem &item)
return QCoreApplication::translate("progress", "error");
case CSYNC_INSTRUCTION_UPDATE_METADATA:
return QCoreApplication::translate("progress", "updating local metadata");
case CSYNC_INSTRUCTION_UPDATE_VFS_METADATA:
return QCoreApplication::translate("progress", "updating local virtual files metadata");
case CSYNC_INSTRUCTION_NONE:
case CSYNC_INSTRUCTION_EVAL:
break;
Expand Down
16 changes: 15 additions & 1 deletion src/libsync/vfs/cfapi/cfapiwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,12 @@ enum class CfApiUpdateMetadataType {
AllMetadata,
};

OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderState(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath, CfApiUpdateMetadataType updateType)
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)};
Expand Down Expand Up @@ -900,3 +905,12 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::up
{
return updatePlaceholderState(path, {}, {}, fileId, replacesPath, CfApiUpdateMetadataType::OnlyBasicMetadata);
}

bool OCC::CfApiWrapper::isPlaceHolderInSync(const QString &filePath)
{
if (const auto originalInfo = findPlaceholderInfo(filePath)) {
return originalInfo->InSyncState == CF_IN_SYNC_STATE_IN_SYNC;
}

return true;
}
1 change: 1 addition & 0 deletions src/libsync/vfs/cfapi/cfapiwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> upd
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());
NEXTCLOUD_CFAPI_EXPORT bool isPlaceHolderInSync(const QString &filePath);

}

Expand Down
10 changes: 10 additions & 0 deletions src/libsync/vfs/cfapi/vfs_cfapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,16 @@ Result<void, QString> VfsCfApi::updateMetadata(const QString &filePath, time_t m
}
}

Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId)
{
return cfapi::updatePlaceholderMarkInSync(filePath, fileId, {});
}

bool VfsCfApi::isPlaceHolderInSync(const QString &filePath) const
{
return cfapi::isPlaceHolderInSync(filePath);
}

Result<void, QString> VfsCfApi::createPlaceholder(const SyncFileItem &item)
{
Q_ASSERT(params().filesystemPath.endsWith('/'));
Expand Down
4 changes: 4 additions & 0 deletions src/libsync/vfs/cfapi/vfs_cfapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ class VfsCfApi : public Vfs

Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) override;

Result<Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId) override;

[[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override;

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, UpdateMetadataTypes updateType) override;
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/vfs/suffix/vfs_suffix.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class VfsSuffix : public Vfs
[[nodiscard]] bool isHydrating() const override;

Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) override;
Result<Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId) override {Q_UNUSED(filePath) Q_UNUSED(fileId) return {QString{}};}
[[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override { Q_UNUSED(filePath) return true; }

Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/vfs/xattr/vfs_xattr.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class VfsXAttr : public Vfs
[[nodiscard]] bool isHydrating() const override;

Result<void, QString> updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId) override;
Result<Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderMarkInSync(const QString &filePath, const QByteArray &fileId) override {Q_UNUSED(filePath) Q_UNUSED(fileId) return {QString{}};}
[[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override { Q_UNUSED(filePath) return true; }

Result<void, QString> createPlaceholder(const SyncFileItem &item) override;
Result<void, QString> dehydratePlaceholder(const SyncFileItem &item) override;
Expand Down

0 comments on commit 01c4d29

Please sign in to comment.