From 31329749a1eaa196c583eca7b4d325cc2bbcaaef Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 6 Aug 2025 10:59:35 +0200 Subject: [PATCH 01/13] chore: modernize and clean bulk download job code try to use const and auto as much as possible remove useless constructor argument that is always using the default value makes code cleaner Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 57 +++++++++++------------ src/libsync/bulkpropagatordownloadjob.h | 11 ++--- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index 991280dbe3..d35717dc84 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -5,7 +5,6 @@ #include "bulkpropagatordownloadjob.h" -#include "owncloudpropagator_p.h" #include "syncfileitem.h" #include "syncengine.h" #include "common/syncjournaldb.h" @@ -13,7 +12,6 @@ #include "propagatorjobs.h" #include "filesystem.h" #include "account.h" -#include "networkjobs.h" #include "propagatedownloadencrypted.h" #include @@ -23,11 +21,10 @@ namespace OCC { Q_LOGGING_CATEGORY(lcBulkPropagatorDownloadJob, "nextcloud.sync.propagator.bulkdownload", QtInfoMsg) BulkPropagatorDownloadJob::BulkPropagatorDownloadJob(OwncloudPropagator *propagator, - PropagateDirectory *parentDirJob, - const std::vector &items) - : PropagatorJob(propagator) - , _filesToDownload(items) - , _parentDirJob(parentDirJob) + PropagateDirectory *parentDirJob) + : PropagatorJob{propagator} + , _filesToDownload{} + , _parentDirJob{parentDirJob} { } @@ -35,15 +32,15 @@ namespace { static QString makeRecallFileName(const QString &fn) { - QString recallFileName(fn); + auto recallFileName(fn); // Add _recall-XXXX before the extension. - int dotLocation = recallFileName.lastIndexOf('.'); + auto dotLocation = recallFileName.lastIndexOf('.'); // If no extension, add it at the end (take care of cases like foo/.hidden or foo.bar/file) if (dotLocation <= recallFileName.lastIndexOf('/') + 1) { dotLocation = recallFileName.size(); } - QString timeString = QDateTime::currentDateTimeUtc().toString("yyyyMMdd-hhmmss"); + const auto &timeString = QDateTime::currentDateTimeUtc().toString("yyyyMMdd-hhmmss"); recallFileName.insert(dotLocation, "_.sys.admin#recall#-" + timeString); return recallFileName; @@ -55,28 +52,28 @@ void handleRecallFile(const QString &filePath, const QString &folderPath, SyncJo FileSystem::setFileHidden(filePath, true); - QFile file(filePath); + auto file = QFile{filePath}; if (!file.open(QIODevice::ReadOnly)) { qCWarning(lcBulkPropagatorDownloadJob) << "Could not open recall file" << file.errorString(); return; } - QFileInfo existingFile(filePath); - QDir baseDir = existingFile.dir(); + const auto existingFile = QFileInfo{filePath}; + const auto &baseDir = existingFile.dir(); while (!file.atEnd()) { - QByteArray line = file.readLine(); + auto line = file.readLine(); line.chop(1); // remove trailing \n - QString recalledFile = QDir::cleanPath(baseDir.filePath(line)); + const auto &recalledFile = QDir::cleanPath(baseDir.filePath(line)); if (!recalledFile.startsWith(folderPath) || !recalledFile.startsWith(baseDir.path())) { qCWarning(lcBulkPropagatorDownloadJob) << "Ignoring recall of " << recalledFile; continue; } // Path of the recalled file in the local folder - QString localRecalledFile = recalledFile.mid(folderPath.size()); + const auto &localRecalledFile = recalledFile.mid(folderPath.size()); - SyncJournalFileRecord record; + auto record = SyncJournalFileRecord{}; if (!journal.getFileRecord(localRecalledFile, &record) || !record.isValid()) { qCWarning(lcBulkPropagatorDownloadJob) << "No db entry for recall of" << localRecalledFile; continue; @@ -84,7 +81,7 @@ void handleRecallFile(const QString &filePath, const QString &folderPath, SyncJo qCInfo(lcBulkPropagatorDownloadJob) << "Recalling" << localRecalledFile << "Checksum:" << record._checksumHeader; - QString targetPath = makeRecallFileName(recalledFile); + const auto &targetPath = makeRecallFileName(recalledFile); qCDebug(lcBulkPropagatorDownloadJob) << "Copy recall file: " << recalledFile << " -> " << targetPath; // Remove the target first, QFile::copy will not overwrite it. @@ -112,7 +109,7 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() _state = Running; - for (const auto &fileToDownload : _filesToDownload) { + for (const auto &fileToDownload : std::as_const(_filesToDownload)) { qCDebug(lcBulkPropagatorDownloadJob) << "Scheduling bulk propagator job:" << this << "and starting download of item" << "with file:" << fileToDownload->_file << "with size:" << fileToDownload->_size; _filesDownloading.push_back(fileToDownload); @@ -219,11 +216,11 @@ void BulkPropagatorDownloadJob::start(const SyncFileItemPtr &item) qCDebug(lcBulkPropagatorDownloadJob) << item->_file << propagator()->_activeJobList.count(); - const auto path = item->_file; + const auto &path = item->_file; const auto slashPosition = path.lastIndexOf('/'); - const auto parentPath = slashPosition >= 0 ? path.left(slashPosition) : QString(); + const auto &parentPath = slashPosition >= 0 ? path.left(slashPosition) : QString(); - SyncJournalFileRecord parentRec; + auto parentRec = SyncJournalFileRecord{}; if (!propagator()->_journal->getFileRecord(parentPath, &parentRec)) { qCWarning(lcBulkPropagatorDownloadJob) << "could not get file from local DB" << parentPath; abortWithError(item, SyncFileItem::NormalError, tr("Could not get file %1 from local DB").arg(parentPath)); @@ -234,10 +231,10 @@ void BulkPropagatorDownloadJob::start(const SyncFileItemPtr &item) startAfterIsEncryptedIsChecked(item); } else { _downloadEncryptedHelper = new PropagateDownloadEncrypted(propagator(), parentPath, item, this); - connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::fileMetadataFound, [this, &item] { + connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::fileMetadataFound, this, [this, &item] { startAfterIsEncryptedIsChecked(item); }); - connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::failed, [this, &item] { + connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::failed, this, [this, &item] { abortWithError( item, SyncFileItem::NormalError, @@ -249,7 +246,7 @@ void BulkPropagatorDownloadJob::start(const SyncFileItemPtr &item) bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) { - const auto fn = propagator()->fullLocalPath(item->_file); + const auto fullFileName = propagator()->fullLocalPath(item->_file); const auto result = propagator()->updateMetadata(*item); if (!result) { abortWithError(item, SyncFileItem::FatalError, tr("Error updating metadata: %1").arg(result.error())); @@ -264,7 +261,7 @@ bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) // handle the special recall file if (!item->_remotePerm.hasPermission(RemotePermissions::IsShared) && (item->_file == QLatin1String(".sys.admin#recall#") || item->_file.endsWith(QLatin1String("/.sys.admin#recall#")))) { - handleRecallFile(fn, propagator()->localPath(), *propagator()->_journal); + handleRecallFile(fullFileName, propagator()->localPath(), *propagator()->_journal); } const auto isLockOwnedByCurrentUser = item->_lockOwnerId == propagator()->account()->davUser(); @@ -273,13 +270,13 @@ bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) const auto isTokenLockOwnedByCurrentUser = (item->_lockOwnerType == SyncFileItem::LockOwnerType::TokenLock && isLockOwnedByCurrentUser); if (item->_locked == SyncFileItem::LockStatus::LockedItem && !isUserLockOwnedByCurrentUser && !isTokenLockOwnedByCurrentUser) { - qCDebug(lcBulkPropagatorDownloadJob()) << fn << "file is locked: making it read only"; - FileSystem::setFileReadOnly(fn, true); + qCDebug(lcBulkPropagatorDownloadJob()) << fullFileName << "file is locked: making it read only"; + FileSystem::setFileReadOnly(fullFileName, true); } else { - qCDebug(lcBulkPropagatorDownloadJob()) << fn << "file is not locked: making it" << ((!item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) + qCDebug(lcBulkPropagatorDownloadJob()) << fullFileName << "file is not locked: making it" << ((!item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) ? "read only" : "read write"); - FileSystem::setFileReadOnlyWeak(fn, (!item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite))); + FileSystem::setFileReadOnlyWeak(fullFileName, (!item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite))); } return true; } diff --git a/src/libsync/bulkpropagatordownloadjob.h b/src/libsync/bulkpropagatordownloadjob.h index ec0bfedf1a..71336ee18c 100644 --- a/src/libsync/bulkpropagatordownloadjob.h +++ b/src/libsync/bulkpropagatordownloadjob.h @@ -8,21 +8,18 @@ #include "owncloudpropagator.h" #include "abstractnetworkjob.h" -#include -#include +#include namespace OCC { class PropagateDownloadEncrypted; -Q_DECLARE_LOGGING_CATEGORY(lcBulkPropagatorDownloadJob) - class BulkPropagatorDownloadJob : public PropagatorJob { Q_OBJECT public: - explicit BulkPropagatorDownloadJob(OwncloudPropagator *propagator, PropagateDirectory *parentDirJob, const std::vector &items = {}); + explicit BulkPropagatorDownloadJob(OwncloudPropagator *propagator, PropagateDirectory *parentDirJob); bool scheduleSelfOrChild() override; @@ -47,9 +44,9 @@ private: void checkPropagationIsDone(); - std::vector _filesToDownload; + QList _filesToDownload; - std::vector _filesDownloading; + QList _filesDownloading; PropagateDownloadEncrypted *_downloadEncryptedHelper = nullptr; From 82b059a7443985b5b1d955f83e0c58280ac5d900 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 6 Aug 2025 16:18:03 +0200 Subject: [PATCH 02/13] fix: remove bulk virtual file job code to dehydrate files this code does not make sense I tested manually and this code is never called I doubt this code is needed or useful Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index d35717dc84..a903dccf9c 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -140,28 +140,7 @@ void BulkPropagatorDownloadJob::startAfterIsEncryptedIsChecked(const SyncFileIte return; } - // For virtual files just dehydrate or create the placeholder and be done - if (item->_type == ItemTypeVirtualFileDehydration) { - const auto fsPath = propagator()->fullLocalPath(item->_file); - if (!FileSystem::verifyFileUnchanged(fsPath, item->_previousSize, item->_previousModtime)) { - propagator()->_anotherSyncNeeded = true; - item->_errorString = tr("File has changed since discovery"); - abortWithError(item, SyncFileItem::SoftError, tr("File has changed since discovery")); - return; - } - qCDebug(lcBulkPropagatorDownloadJob) << "dehydrating file" << item->_file; - const auto r = vfs->dehydratePlaceholder(*item); - if (!r) { - qCCritical(lcBulkPropagatorDownloadJob) << "Could not dehydrate a file" << QDir::toNativeSeparators(item->_file) << ":" << r.error(); - abortWithError(item, SyncFileItem::NormalError, r.error()); - return; - } - if (!propagator()->_journal->deleteFileRecord(item->_originalFile)) { - qCWarning(lcBulkPropagatorDownloadJob) << "could not delete file from local DB" << item->_originalFile; - abortWithError(item, SyncFileItem::NormalError, tr("Could not delete file record %1 from local DB").arg(item->_originalFile)); - return; - } - } else if (item->_type == ItemTypeVirtualFile) { + if (item->_type == ItemTypeVirtualFile) { qCDebug(lcBulkPropagatorDownloadJob) << "creating virtual file" << item->_file; const auto r = vfs->createPlaceholder(*item); if (!r) { From 42446f25ee93f7bc350dd23b5c8c6f57b1c07647 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 7 Aug 2025 13:06:47 +0200 Subject: [PATCH 03/13] chore: remove deprectaed use of Q_REQUIRED_RESULT use the c++ standard way of requiring caller to care about the returned value Signed-off-by: Matthieu Gallien --- src/common/vfs.h | 18 ++++++++-------- src/gui/sharemanager.h | 2 +- src/gui/systray.h | 10 ++++----- src/gui/userstatusselectormodel.h | 34 +++++++++++++++---------------- src/libsync/owncloudpropagator.h | 8 ++++---- src/libsync/userstatusconnector.h | 14 ++++++------- 6 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/common/vfs.h b/src/common/vfs.h index d0f4e28fc0..4707b013cc 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -186,21 +186,21 @@ public: [[nodiscard]] virtual bool isPlaceHolderInSync(const QString &filePath) const = 0; /// Create a new dehydrated placeholder. Called from PropagateDownload. - Q_REQUIRED_RESULT virtual Result createPlaceholder(const SyncFileItem &item) = 0; + [[nodiscard]] virtual Result createPlaceholder(const SyncFileItem &item) = 0; /** Convert a hydrated placeholder to a dehydrated one. Called from PropagateDownlaod. * * This is different from delete+create because preserving some file metadata * (like pin states) may be essential for some vfs plugins. */ - Q_REQUIRED_RESULT virtual Result dehydratePlaceholder(const SyncFileItem &item) = 0; + [[nodiscard]] virtual Result dehydratePlaceholder(const SyncFileItem &item) = 0; /** Discovery hook: even unchanged files may need UPDATE_METADATA. * * For instance cfapi vfs wants local hydrated non-placeholder files to * become hydrated placeholder files. */ - Q_REQUIRED_RESULT virtual bool needsMetadataUpdate(const SyncFileItem &item) = 0; + [[nodiscard]] virtual bool needsMetadataUpdate(const SyncFileItem &item) = 0; /** Convert a new file to a hydrated placeholder. * @@ -215,13 +215,13 @@ public: * new placeholder shall supersede, for rename-replace actions with new downloads, * for example. */ - Q_REQUIRED_RESULT virtual Result convertToPlaceholder(const QString &filename, + [[nodiscard]] 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; + [[nodiscard]] virtual bool isDehydratedPlaceholder(const QString &filePath) = 0; /** Similar to isDehydratedPlaceholder() but used from sync discovery. * @@ -230,7 +230,7 @@ public: * * Returning true means that type was fully determined. */ - Q_REQUIRED_RESULT virtual bool statTypeVirtualFile(csync_file_stat_t *stat, void *stat_data) = 0; + [[nodiscard]] virtual bool statTypeVirtualFile(csync_file_stat_t *stat, void *stat_data) = 0; /** Sets the pin state for the item at a path. * @@ -241,7 +241,7 @@ public: * * folderPath is relative to the sync folder. Can be "" for root folder. */ - Q_REQUIRED_RESULT virtual bool setPinState(const QString &folderPath, PinState state) = 0; + [[nodiscard]] virtual bool setPinState(const QString &folderPath, PinState state) = 0; /** Returns the pin state of an item at a path. * @@ -252,7 +252,7 @@ public: * * Returns none on retrieval error. */ - Q_REQUIRED_RESULT virtual Optional pinState(const QString &folderPath) = 0; + [[nodiscard]] virtual Optional pinState(const QString &folderPath) = 0; /** Returns availability status of an item at a path. * @@ -261,7 +261,7 @@ public: * * folderPath is relative to the sync folder. Can be "" for root folder. */ - Q_REQUIRED_RESULT virtual AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) = 0; + [[nodiscard]] virtual AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) = 0; public slots: /** Update in-sync state based on SyncFileStatusTracker signal. diff --git a/src/gui/sharemanager.h b/src/gui/sharemanager.h index 6a34d921ab..e25f10768b 100644 --- a/src/gui/sharemanager.h +++ b/src/gui/sharemanager.h @@ -120,7 +120,7 @@ public: /* * Get whether the share has a password set */ - [[nodiscard]] Q_REQUIRED_RESULT bool isPasswordSet() const; + [[nodiscard]] [[nodiscard]] bool isPasswordSet() const; /* * Is it a share with a user or group (local or remote) diff --git a/src/gui/systray.h b/src/gui/systray.h index 3d2337b226..807b7498b4 100644 --- a/src/gui/systray.h +++ b/src/gui/systray.h @@ -81,12 +81,12 @@ public: enum class FileDetailsPage { Activity, Sharing }; Q_ENUM(FileDetailsPage); - Q_REQUIRED_RESULT QString windowTitle() const; - Q_REQUIRED_RESULT bool useNormalWindow() const; + [[nodiscard]] QString windowTitle() const; + [[nodiscard]] bool useNormalWindow() const; - Q_REQUIRED_RESULT bool syncIsPaused() const; - Q_REQUIRED_RESULT bool anySyncFolders() const; - Q_REQUIRED_RESULT bool isOpen() const; + [[nodiscard]] bool syncIsPaused() const; + [[nodiscard]] bool anySyncFolders() const; + [[nodiscard]] bool isOpen() const; [[nodiscard]] bool enableAddAccount() const; diff --git a/src/gui/userstatusselectormodel.h b/src/gui/userstatusselectormodel.h index c9e56f07f5..424925f555 100644 --- a/src/gui/userstatusselectormodel.h +++ b/src/gui/userstatusselectormodel.h @@ -67,30 +67,30 @@ public: explicit UserStatusSelectorModel(const UserStatus &userStatus, QObject *parent = nullptr); - Q_REQUIRED_RESULT int userIndex() const; + [[nodiscard]] int userIndex() const; - Q_REQUIRED_RESULT UserStatus::OnlineStatus onlineStatus() const; + [[nodiscard]] UserStatus::OnlineStatus onlineStatus() const; void setOnlineStatus(UserStatus::OnlineStatus status); - Q_REQUIRED_RESULT QUrl onlineIcon() const; - Q_REQUIRED_RESULT QUrl awayIcon() const; - Q_REQUIRED_RESULT QUrl dndIcon() const; - Q_REQUIRED_RESULT QUrl busyIcon() const; - Q_REQUIRED_RESULT QUrl invisibleIcon() const; + [[nodiscard]] QUrl onlineIcon() const; + [[nodiscard]] QUrl awayIcon() const; + [[nodiscard]] QUrl dndIcon() const; + [[nodiscard]] QUrl busyIcon() const; + [[nodiscard]] QUrl invisibleIcon() const; - Q_REQUIRED_RESULT QString userStatusMessage() const; + [[nodiscard]] QString userStatusMessage() const; void setUserStatusMessage(const QString &message); - Q_REQUIRED_RESULT QString userStatusEmoji() const; + [[nodiscard]] QString userStatusEmoji() const; void setUserStatusEmoji(const QString &emoji); [[nodiscard]] QVector predefinedStatuses() const; - Q_REQUIRED_RESULT QVariantList clearStageTypes() const; - Q_REQUIRED_RESULT QString clearAtDisplayString() const; + [[nodiscard]] QVariantList clearStageTypes() const; + [[nodiscard]] QString clearAtDisplayString() const; [[nodiscard]] Q_INVOKABLE QString clearAtReadable(const OCC::UserStatus &status) const; - Q_REQUIRED_RESULT QString errorMessage() const; - Q_REQUIRED_RESULT bool busyStatusSupported() const; + [[nodiscard]] QString errorMessage() const; + [[nodiscard]] bool busyStatusSupported() const; public slots: void setUserIndex(const int userIndex); @@ -117,10 +117,10 @@ private: void onMessageCleared(); void onError(UserStatusConnector::Error error); - Q_REQUIRED_RESULT QString clearAtReadable(const Optional &clearAt) const; - Q_REQUIRED_RESULT QString clearAtStageToString(ClearStageType stage) const; - Q_REQUIRED_RESULT QString timeDifferenceToString(int differenceSecs) const; - Q_REQUIRED_RESULT Optional clearStageTypeToDateTime(ClearStageType type) const; + [[nodiscard]] QString clearAtReadable(const Optional &clearAt) const; + [[nodiscard]] QString clearAtStageToString(ClearStageType stage) const; + [[nodiscard]] QString timeDifferenceToString(int differenceSecs) const; + [[nodiscard]] Optional clearStageTypeToDateTime(ClearStageType type) const; void setError(const QString &reason); void clearError(); diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h index 1bb177dbfa..e131b858bc 100644 --- a/src/libsync/owncloudpropagator.h +++ b/src/libsync/owncloudpropagator.h @@ -523,14 +523,14 @@ public: */ bool hasCaseClashAccessibilityProblem(const QString &relfile); - Q_REQUIRED_RESULT QString fullLocalPath(const QString &tmp_file_name) const; + [[nodiscard]] QString fullLocalPath(const QString &tmp_file_name) const; [[nodiscard]] QString localPath() const; /** * Returns the full remote path including the folder root of a * folder sync path. */ - Q_REQUIRED_RESULT QString fullRemotePath(const QString &tmp_file_name) const; + [[nodiscard]] QString fullRemotePath(const QString &tmp_file_name) const; [[nodiscard]] QString remotePath() const; [[nodiscard]] QString fulllRemotePathToPathInSyncJournalDb(const QString &fullRemotePath) const; @@ -623,9 +623,9 @@ public: SyncJournalDb * const journal, Vfs::UpdateMetadataTypes updateType); - Q_REQUIRED_RESULT bool isDelayedUploadItem(const SyncFileItemPtr &item) const; + [[nodiscard]] bool isDelayedUploadItem(const SyncFileItemPtr &item) const; - Q_REQUIRED_RESULT const std::deque& delayedTasks() const + [[nodiscard]] const std::deque& delayedTasks() const { return _delayedTasks; } diff --git a/src/libsync/userstatusconnector.h b/src/libsync/userstatusconnector.h index ebd4ff37a8..4fe0b5732e 100644 --- a/src/libsync/userstatusconnector.h +++ b/src/libsync/userstatusconnector.h @@ -62,11 +62,11 @@ public: UserStatus(const QString &id, const QString &message, const QString &icon, OnlineStatus state, bool messagePredefined, const Optional &clearAt = {}); - Q_REQUIRED_RESULT QString id() const; - Q_REQUIRED_RESULT QString message() const; - Q_REQUIRED_RESULT QString icon() const; - Q_REQUIRED_RESULT OnlineStatus state() const; - Q_REQUIRED_RESULT Optional clearAt() const; + [[nodiscard]] QString id() const; + [[nodiscard]] QString message() const; + [[nodiscard]] QString icon() const; + [[nodiscard]] OnlineStatus state() const; + [[nodiscard]] Optional clearAt() const; [[nodiscard]] QString clearAtDisplayString() const; @@ -77,9 +77,9 @@ public: void setMessagePredefined(bool value); void setClearAt(const Optional &dateTime); - Q_REQUIRED_RESULT bool messagePredefined() const; + [[nodiscard]] bool messagePredefined() const; - Q_REQUIRED_RESULT QUrl stateIcon() const; + [[nodiscard]] QUrl stateIcon() const; private: QString _id; From 0687dfac3416cf9000a52d8cda154c7dd55178e9 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 7 Aug 2025 15:25:03 +0200 Subject: [PATCH 04/13] chore: fix warning about implicit capture of this in a lambda Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 8b3757ef58..5d181cd68f 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -534,7 +534,7 @@ VfsCfApi::HydratationAndPinStates VfsCfApi::computeRecursiveHydrationAndPinState const auto dir = QDir(info.absoluteFilePath()); Q_ASSERT(dir.exists()); const auto children = dir.entryList(); - return std::accumulate(std::cbegin(children), std::cend(children), dirState, [=](const HydratationAndPinStates ¤tState, const QString &name) { + return std::accumulate(std::cbegin(children), std::cend(children), dirState, [=, this](const HydratationAndPinStates ¤tState, const QString &name) { if (name == QStringLiteral("..") || name == QStringLiteral(".")) { return currentState; } From a57f63fa02316fa6665872a4b7a48f04e0b9b072 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 7 Aug 2025 15:25:30 +0200 Subject: [PATCH 05/13] feat(vfs): add a new API to create a list of placeholders at once should allow to speed API calls to Windows CfApi by reducing their numbers by asking for a list of new placeholders via a single call instead of one for each new item Signed-off-by: Matthieu Gallien --- src/common/vfs.h | 7 +++++++ src/libsync/owncloudpropagator.cpp | 3 ++- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 15 +++++++++++++++ src/libsync/vfs/cfapi/vfs_cfapi.h | 2 ++ src/libsync/vfs/suffix/vfs_suffix.cpp | 15 +++++++++++++++ src/libsync/vfs/suffix/vfs_suffix.h | 2 ++ src/libsync/vfs/xattr/vfs_xattr.cpp | 15 +++++++++++++++ src/libsync/vfs/xattr/vfs_xattr.h | 2 ++ test/testsynccfapi.cpp | 21 +++++++++++++++++++++ 9 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/common/vfs.h b/src/common/vfs.h index 4707b013cc..ae34d9e2d9 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -25,6 +25,7 @@ using AccountPtr = QSharedPointer; class SyncJournalDb; class VfsPrivate; class SyncFileItem; +using SyncFileItemPtr = QSharedPointer; /** Collection of parameters for initializing a Vfs instance. */ struct OCSYNC_EXPORT VfsSetupParams @@ -188,6 +189,9 @@ public: /// Create a new dehydrated placeholder. Called from PropagateDownload. [[nodiscard]] virtual Result createPlaceholder(const SyncFileItem &item) = 0; + /// Create a new dehydrated list of placeholders + [[nodiscard]] virtual Result createPlaceholders(const QList &items) = 0; + /** Convert a hydrated placeholder to a dehydrated one. Called from PropagateDownlaod. * * This is different from delete+create because preserving some file metadata @@ -323,7 +327,10 @@ public: OCC::Result updateMetadata(const SyncFileItem &, const QString &, const QString &) override { return {OCC::Vfs::ConvertToPlaceholderResult::Ok}; } Result 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 createPlaceholder(const SyncFileItem &) override { return {}; } + Result createPlaceholders(const QList &) override { return {}; } + Result dehydratePlaceholder(const SyncFileItem &) override { return {}; } Result convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, const UpdateMetadataTypes) override { return ConvertToPlaceholderResult::Ok; } diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index d8b16f877d..cafdf82205 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -690,7 +690,8 @@ void OwncloudPropagator::startFilePropagation(const SyncFileItemPtr &item, const auto isDownload = item->_direction == SyncFileItem::Down && (item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC); const auto isVirtualFile = item->_type == ItemTypeVirtualFile; - const auto shouldAddBulkPropagateDownloadItem = isDownload && isVirtualFile && isVfsCfApi; + const auto isEncrypted = item->_e2eEncryptionStatus != SyncFileItem::EncryptionStatus::NotEncrypted; + const auto shouldAddBulkPropagateDownloadItem = isDownload && isVirtualFile && isVfsCfApi && !isEncrypted; if (shouldAddBulkPropagateDownloadItem) { addBulkPropagateDownloadItem(item, directories); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 5d181cd68f..3a23d6eb34 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -211,6 +211,21 @@ Result VfsCfApi::createPlaceholder(const SyncFileItem &item) return result; } +Result VfsCfApi::createPlaceholders(const QList &items) +{ + auto result = Result{}; + + for (const auto &oneItem : items) { + const auto itemResult = createPlaceholder(*oneItem); + if (!itemResult) { + result = itemResult; + break; + } + } + + return result; +} + Result VfsCfApi::dehydratePlaceholder(const SyncFileItem &item) { const auto localPath = QDir::toNativeSeparators(_setupParams.filesystemPath + item._file); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.h b/src/libsync/vfs/cfapi/vfs_cfapi.h index 91cdabe87e..0eef29251e 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.h +++ b/src/libsync/vfs/cfapi/vfs_cfapi.h @@ -39,6 +39,8 @@ public: [[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override; Result createPlaceholder(const SyncFileItem &item) override; + Result createPlaceholders(const QList &items) override; + Result dehydratePlaceholder(const SyncFileItem &item) override; Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType) override; diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp index 7f18412bfb..7cb5a43185 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.cpp +++ b/src/libsync/vfs/suffix/vfs_suffix.cpp @@ -108,6 +108,21 @@ Result VfsSuffix::createPlaceholder(const SyncFileItem &item) return {}; } +Result VfsSuffix::createPlaceholders(const QList &items) +{ + auto result = Result{}; + + for (const auto &oneItem : items) { + const auto itemResult = createPlaceholder(*oneItem); + if (!itemResult) { + result = itemResult; + break; + } + } + + return result; +} + Result VfsSuffix::dehydratePlaceholder(const SyncFileItem &item) { SyncFileItem virtualItem(item); diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h index 0c0e5de7e4..803338ec65 100644 --- a/src/libsync/vfs/suffix/vfs_suffix.h +++ b/src/libsync/vfs/suffix/vfs_suffix.h @@ -35,6 +35,8 @@ public: [[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override { Q_UNUSED(filePath) return true; } Result createPlaceholder(const SyncFileItem &item) override; + Result createPlaceholders(const QList &items) override; + Result dehydratePlaceholder(const SyncFileItem &item) override; Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &, UpdateMetadataTypes updateType) override; diff --git a/src/libsync/vfs/xattr/vfs_xattr.cpp b/src/libsync/vfs/xattr/vfs_xattr.cpp index c6315b82ac..e5bd5e213c 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.cpp +++ b/src/libsync/vfs/xattr/vfs_xattr.cpp @@ -97,6 +97,21 @@ Result VfsXAttr::createPlaceholder(const SyncFileItem &item) return xattr::addNextcloudPlaceholderAttributes(path); } +Result VfsXAttr::createPlaceholders(const QList &items) +{ + auto result = Result{}; + + for (const auto &oneItem : items) { + const auto itemResult = createPlaceholder(*oneItem); + if (!itemResult) { + result = itemResult; + break; + } + } + + return result; +} + Result VfsXAttr::dehydratePlaceholder(const SyncFileItem &item) { const auto path = QString(_setupParams.filesystemPath + item._file); diff --git a/src/libsync/vfs/xattr/vfs_xattr.h b/src/libsync/vfs/xattr/vfs_xattr.h index 382f4d1d81..1922c44370 100644 --- a/src/libsync/vfs/xattr/vfs_xattr.h +++ b/src/libsync/vfs/xattr/vfs_xattr.h @@ -34,6 +34,8 @@ public: [[nodiscard]] bool isPlaceHolderInSync(const QString &filePath) const override { Q_UNUSED(filePath) return true; } Result createPlaceholder(const SyncFileItem &item) override; + Result createPlaceholders(const QList &items) override; + Result dehydratePlaceholder(const SyncFileItem &item) override; Result convertToPlaceholder(const QString &filename, const SyncFileItem &item, diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp index 60678ad829..d16832470b 100644 --- a/test/testsynccfapi.cpp +++ b/test/testsynccfapi.cpp @@ -1517,6 +1517,27 @@ private slots: QVERIFY(fakeFolder.currentRemoteState().find("S/A/a2m")); QVERIFY(fakeFolder.currentRemoteState().find("S/B/b2m")); } + + void createFolderAndFiles() + { + FakeFolder fakeFolder {FileInfo{}}; + auto vfs = setupVfs(fakeFolder); + + fakeFolder.remoteModifier().mkdir("first folder"); + + QVERIFY(fakeFolder.syncOnce()); + + fakeFolder.remoteModifier().insert("first folder/file1"); + fakeFolder.remoteModifier().insert("first folder/file2"); + fakeFolder.remoteModifier().insert("first folder/file3"); + fakeFolder.remoteModifier().mkdir("first folder/second folder"); + fakeFolder.remoteModifier().insert("first folder/second folder/second file1"); + fakeFolder.remoteModifier().insert("first folder/second folder/second file2"); + fakeFolder.remoteModifier().insert("first folder/second folder/second file3"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentRemoteState(), fakeFolder.currentRemoteState()); + } }; QTEST_GUILESS_MAIN(TestSyncCfApi) From 39f1cb1ef591c3b5601dd6e6a674d55e147bd3b1 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 12 Aug 2025 12:15:07 +0200 Subject: [PATCH 06/13] feat(windows/vfs): use the new batch create API for placeholder files use the proper VFS plug-in API to request all files to be cerated as a single batch will now need a proper implementation using the batch Windows CfApi function Signed-off-by: Matthieu Gallien --- src/common/vfs.h | 1 + src/libsync/bulkpropagatordownloadjob.cpp | 51 ++++++++++++++--------- src/libsync/bulkpropagatordownloadjob.h | 2 - 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/common/vfs.h b/src/common/vfs.h index ae34d9e2d9..8f875332b2 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -14,6 +14,7 @@ #include #include +#include #include using csync_file_stat_t = struct csync_file_stat_s; diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index a903dccf9c..e95d18212a 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -110,13 +110,33 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() _state = Running; for (const auto &fileToDownload : std::as_const(_filesToDownload)) { - qCDebug(lcBulkPropagatorDownloadJob) << "Scheduling bulk propagator job:" << this << "and starting download of item" - << "with file:" << fileToDownload->_file << "with size:" << fileToDownload->_size; - _filesDownloading.push_back(fileToDownload); - start(fileToDownload); + Q_ASSERT(fileToDownload->_type == ItemTypeVirtualFileDehydration || fileToDownload->_type == ItemTypeVirtualFile); + + if (propagator()->localFileNameClash(fileToDownload->_file)) { + _parentDirJob->appendTask(fileToDownload); + finalizeOneFile(fileToDownload); + qCCritical(lcBulkPropagatorDownloadJob) << "File" << QDir::toNativeSeparators(fileToDownload->_file) << "can not be downloaded because it is non virtual!"; + abortWithError(fileToDownload, SyncFileItem::NormalError, tr("File %1 cannot be downloaded because it is non virtual!").arg(QDir::toNativeSeparators(fileToDownload->_file))); + return false; + } } - _filesToDownload.clear(); + const auto &vfs = propagator()->syncOptions()._vfs; + Q_ASSERT(vfs && vfs->mode() == Vfs::WindowsCfApi); + + const auto r = vfs->createPlaceholders(_filesToDownload); + + for (const auto &fileToDownload : std::as_const(_filesToDownload)) { + if (!updateMetadata(fileToDownload)) { + return false; + } + + if (!fileToDownload->_remotePerm.isNull() && !fileToDownload->_remotePerm.hasPermission(RemotePermissions::CanWrite)) { + // make sure ReadOnly flag is preserved for placeholder, similarly to regular files + FileSystem::setFileReadOnly(propagator()->fullLocalPath(fileToDownload->_file), true); + } + finalizeOneFile(fileToDownload); + } checkPropagationIsDone(); @@ -168,19 +188,19 @@ void BulkPropagatorDownloadJob::startAfterIsEncryptedIsChecked(const SyncFileIte void BulkPropagatorDownloadJob::finalizeOneFile(const SyncFileItemPtr &file) { - const auto foundIt = std::find_if(std::cbegin(_filesDownloading), std::cend(_filesDownloading), [&file](const auto &fileDownloading) { + const auto foundIt = std::find_if(std::cbegin(_filesToDownload), std::cend(_filesToDownload), [&file](const auto &fileDownloading) { return fileDownloading == file; }); - if (foundIt != std::cend(_filesDownloading)) { + if (foundIt != std::cend(_filesToDownload)) { emit propagator()->itemCompleted(file, ErrorCategory::GenericError); - _filesDownloading.erase(foundIt); + _filesToDownload.erase(foundIt); } checkPropagationIsDone(); } void BulkPropagatorDownloadJob::checkPropagationIsDone() { - if (_filesToDownload.empty() && _filesDownloading.empty()) { + if (_filesToDownload.empty() && _filesToDownload.empty()) { qCInfo(lcBulkPropagatorDownloadJob) << "finished with status" << SyncFileItem::Status::Success; emit finished(SyncFileItem::Status::Success); propagator()->scheduleNextJob(); @@ -209,17 +229,8 @@ void BulkPropagatorDownloadJob::start(const SyncFileItemPtr &item) if (!propagator()->account()->capabilities().clientSideEncryptionAvailable() || !parentRec.isValid() || !parentRec.isE2eEncrypted()) { startAfterIsEncryptedIsChecked(item); } else { - _downloadEncryptedHelper = new PropagateDownloadEncrypted(propagator(), parentPath, item, this); - connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::fileMetadataFound, this, [this, &item] { - startAfterIsEncryptedIsChecked(item); - }); - connect(_downloadEncryptedHelper, &PropagateDownloadEncrypted::failed, this, [this, &item] { - abortWithError( - item, - SyncFileItem::NormalError, - tr("File %1 cannot be downloaded because encryption information is missing.").arg(QDir::toNativeSeparators(item->_file))); - }); - _downloadEncryptedHelper->start(); + Q_ASSERT(false); + qCCritical(lcBulkPropagatorDownloadJob()) << "no encrypted fiels should be created via bulk download"; } } diff --git a/src/libsync/bulkpropagatordownloadjob.h b/src/libsync/bulkpropagatordownloadjob.h index 71336ee18c..62b64ecbaa 100644 --- a/src/libsync/bulkpropagatordownloadjob.h +++ b/src/libsync/bulkpropagatordownloadjob.h @@ -46,8 +46,6 @@ private: QList _filesToDownload; - QList _filesDownloading; - PropagateDownloadEncrypted *_downloadEncryptedHelper = nullptr; PropagateDirectory *_parentDirJob = nullptr; From 8351bb56f5562daf11bd6e89492d4994f2c29780 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 12 Aug 2025 16:12:26 +0200 Subject: [PATCH 07/13] feat(vfs/windows): use a single call to CfCreatePlaceholders for all Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 59 ++++++++++++++++++++++++++ src/libsync/vfs/cfapi/cfapiwrapper.h | 13 ++++++ src/libsync/vfs/cfapi/vfs_cfapi.cpp | 13 +++--- 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index e61525c29e..ad3e7359b1 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -906,6 +906,65 @@ OCC::Result OCC::CfApiWrapper::createPlaceholderInfo(const QStrin return {}; } +OCC::Result OCC::CfApiWrapper::createPlaceholdersInfo(const QString &localBasePath, const QList &itemsInfo) +{ + const auto stdWStringBasePath = localBasePath.toStdWString(); + auto cloudEntry = std::make_unique(itemsInfo.size()); + + for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { + const auto &placeholderInfo = itemsInfo[itemIndice]; + + if (placeholderInfo.modtime <= 0) { + return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(placeholderInfo.relativePath).arg(placeholderInfo.modtime)}; + } + + cloudEntry[itemIndice].FileIdentity = placeholderInfo.fileId.data(); + cloudEntry[itemIndice].FileIdentityLength = static_cast(placeholderInfo.fileId.length()); + + cloudEntry[itemIndice].RelativeFileName = placeholderInfo.platformNativeRelativePath.data(); + cloudEntry[itemIndice].Flags = CF_PLACEHOLDER_CREATE_FLAG_MARK_IN_SYNC; + cloudEntry[itemIndice].FsMetadata.FileSize.QuadPart = placeholderInfo.size; + cloudEntry[itemIndice].FsMetadata.BasicInfo.FileAttributes = FILE_ATTRIBUTE_NORMAL; + OCC::Utility::UnixTimeToLargeIntegerFiletime(placeholderInfo.modtime, &cloudEntry[itemIndice].FsMetadata.BasicInfo.CreationTime); + OCC::Utility::UnixTimeToLargeIntegerFiletime(placeholderInfo.modtime, &cloudEntry[itemIndice].FsMetadata.BasicInfo.LastWriteTime); + OCC::Utility::UnixTimeToLargeIntegerFiletime(placeholderInfo.modtime, &cloudEntry[itemIndice].FsMetadata.BasicInfo.LastAccessTime); + OCC::Utility::UnixTimeToLargeIntegerFiletime(placeholderInfo.modtime, &cloudEntry[itemIndice].FsMetadata.BasicInfo.ChangeTime); + + if (placeholderInfo.fileInfo.isDir()) { + cloudEntry[itemIndice].Flags |= CF_PLACEHOLDER_CREATE_FLAG_DISABLE_ON_DEMAND_POPULATION; + cloudEntry[itemIndice].FsMetadata.BasicInfo.FileAttributes = FILE_ATTRIBUTE_DIRECTORY; + cloudEntry[itemIndice].FsMetadata.FileSize.QuadPart = 0; + } + + qCDebug(lcCfApiWrapper) << "CfCreatePlaceholders" << stdWStringBasePath << placeholderInfo.platformNativeRelativePath << placeholderInfo.modtime << placeholderInfo.size; + qCDebug(lcCfApiWrapper) << QString::fromStdWString(cloudEntry[itemIndice].RelativeFileName); + } + + auto numberOfCreatedPlaceholders = 0ul; + const qint64 result = CfCreatePlaceholders(stdWStringBasePath.data(), cloudEntry.get(), itemsInfo.size(), CF_CREATE_FLAG_NONE, &numberOfCreatedPlaceholders); + if (result != S_OK) { + qCWarning(lcCfApiWrapper) << "Couldn't create placeholders info" << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << "number of placeholders created:" << numberOfCreatedPlaceholders; + + for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { + qCDebug(lcCfApiWrapper) << QString::fromStdWString(cloudEntry[itemIndice].RelativeFileName) << QString::fromWCharArray(_com_error(cloudEntry[itemIndice].Result).ErrorMessage()); + } + + return { "Couldn't create placeholder info" }; + } + + for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { + const auto &placeholderInfo = itemsInfo[itemIndice]; + const auto parentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(localBasePath + placeholderInfo.relativePath).absolutePath())); + const auto state = parentInfo && parentInfo->PinState == CF_PIN_STATE_UNPINNED ? CF_PIN_STATE_UNPINNED : CF_PIN_STATE_INHERIT; + + if (!setPinState(placeholderInfo.relativePath, cfPinStateToPinState(state), NoRecurse)) { + return { "Couldn't set the default inherit pin state" }; + } + } + + return {}; +} + OCC::Result OCC::CfApiWrapper::updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath) { return updatePlaceholderState(path, modtime, size, fileId, replacesPath, CfApiUpdateMetadataType::AllMetadata); diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.h b/src/libsync/vfs/cfapi/cfapiwrapper.h index 329a8ca6fa..b2627356e6 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.h +++ b/src/libsync/vfs/cfapi/cfapiwrapper.h @@ -15,6 +15,8 @@ #include "common/result.h" #include "common/vfs.h" +#include + #include struct CF_PLACEHOLDER_BASIC_INFO; @@ -90,6 +92,17 @@ enum SetPinRecurseMode { NEXTCLOUD_CFAPI_EXPORT Result setPinState(const QString &path, PinState state, SetPinRecurseMode mode); NEXTCLOUD_CFAPI_EXPORT Result createPlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId); + +struct PlaceholdersInfo { + QFileInfo fileInfo; + QString relativePath; + std::wstring platformNativeRelativePath; + QByteArray fileId; + time_t modtime; + qint64 size; +}; + +NEXTCLOUD_CFAPI_EXPORT Result createPlaceholdersInfo(const QString &localBasePath, const QList &itemsInfo); 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); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 3a23d6eb34..3103881a4c 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -213,16 +213,17 @@ Result VfsCfApi::createPlaceholder(const SyncFileItem &item) Result VfsCfApi::createPlaceholders(const QList &items) { - auto result = Result{}; + const auto fileInfo = QFileInfo(_setupParams.filesystemPath + items[0]->_file); + const auto localPath = QDir::toNativeSeparators(fileInfo.absolutePath()); + auto allPlaceholdersInfo = QList{}; for (const auto &oneItem : items) { - const auto itemResult = createPlaceholder(*oneItem); - if (!itemResult) { - result = itemResult; - break; - } + auto fileInfo = QFileInfo(_setupParams.filesystemPath + oneItem->_file); + allPlaceholdersInfo.emplace_back(std::move(fileInfo), fileInfo.fileName(), QDir::toNativeSeparators(fileInfo.fileName()).toStdWString(), oneItem->_fileId, oneItem->_modtime, oneItem->_size); } + const auto result = cfapi::createPlaceholdersInfo(localPath, {allPlaceholdersInfo}); + return result; } From 3bd6717e004b8ef3fc628a0e9aba16e95355a295 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 25 Aug 2025 13:20:50 +0200 Subject: [PATCH 08/13] fix(nodiscard): remove duplicated nodiscard annotation Signed-off-by: Matthieu Gallien --- src/gui/sharemanager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/sharemanager.h b/src/gui/sharemanager.h index e25f10768b..0653f2d5ff 100644 --- a/src/gui/sharemanager.h +++ b/src/gui/sharemanager.h @@ -120,7 +120,7 @@ public: /* * Get whether the share has a password set */ - [[nodiscard]] [[nodiscard]] bool isPasswordSet() const; + [[nodiscard]] bool isPasswordSet() const; /* * Is it a share with a user or group (local or remote) From 4c33454ddade134ecb89624a888e2422b1af8e7b Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 25 Aug 2025 13:31:15 +0200 Subject: [PATCH 09/13] fix: handle errors when createPlaceholders report errors Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 136 +++++++--------------- src/libsync/bulkpropagatordownloadjob.h | 4 +- 2 files changed, 45 insertions(+), 95 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index e95d18212a..3d7e3009ad 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -109,6 +109,41 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() _state = Running; + return start(); +} + +PropagatorJob::JobParallelism BulkPropagatorDownloadJob::parallelism() const +{ + return PropagatorJob::JobParallelism::FullParallelism; +} + +void BulkPropagatorDownloadJob::finalizeOneFile(const SyncFileItemPtr &file) +{ + const auto foundIt = std::find_if(std::cbegin(_filesToDownload), std::cend(_filesToDownload), [&file](const auto &fileDownloading) { + return fileDownloading == file; + }); + if (foundIt != std::cend(_filesToDownload)) { + emit propagator()->itemCompleted(file, ErrorCategory::GenericError); + _filesToDownload.erase(foundIt); + } + checkPropagationIsDone(); +} + +void BulkPropagatorDownloadJob::checkPropagationIsDone() +{ + if (_filesToDownload.empty()) { + qCInfo(lcBulkPropagatorDownloadJob) << "finished with status" << SyncFileItem::Status::Success; + emit finished(SyncFileItem::Status::Success); + propagator()->scheduleNextJob(); + } +} + +bool BulkPropagatorDownloadJob::start() +{ + if (propagator()->_abortRequested) { + return false; + } + for (const auto &fileToDownload : std::as_const(_filesToDownload)) { Q_ASSERT(fileToDownload->_type == ItemTypeVirtualFileDehydration || fileToDownload->_type == ItemTypeVirtualFile); @@ -126,6 +161,14 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() const auto r = vfs->createPlaceholders(_filesToDownload); + if (!r) { + qCCritical(lcBulkPropagatorDownloadJob) << "Could not create placholders:" << r.error(); + for (const auto &fileToDownload : std::as_const(_filesToDownload)) { + abortWithError(fileToDownload, SyncFileItem::NormalError, r.error()); + } + return false; + } + for (const auto &fileToDownload : std::as_const(_filesToDownload)) { if (!updateMetadata(fileToDownload)) { return false; @@ -140,98 +183,7 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() checkPropagationIsDone(); - return true; -} - -PropagatorJob::JobParallelism BulkPropagatorDownloadJob::parallelism() const -{ - return PropagatorJob::JobParallelism::FullParallelism; -} - -void BulkPropagatorDownloadJob::startAfterIsEncryptedIsChecked(const SyncFileItemPtr &item) -{ - const auto &vfs = propagator()->syncOptions()._vfs; - Q_ASSERT(vfs && vfs->mode() == Vfs::WindowsCfApi); - Q_ASSERT(item->_type == ItemTypeVirtualFileDehydration || item->_type == ItemTypeVirtualFile); - - if (propagator()->localFileNameClash(item->_file)) { - _parentDirJob->appendTask(item); - finalizeOneFile(item); - return; - } - - if (item->_type == ItemTypeVirtualFile) { - qCDebug(lcBulkPropagatorDownloadJob) << "creating virtual file" << item->_file; - const auto r = vfs->createPlaceholder(*item); - if (!r) { - qCCritical(lcBulkPropagatorDownloadJob) << "Could not create a placholder for a file" << QDir::toNativeSeparators(item->_file) << ":" << r.error(); - abortWithError(item, SyncFileItem::NormalError, r.error()); - return; - } - } else { - // we should never get here, as BulkPropagatorDownloadJob must only ever be instantiated and only contain virtual files - qCCritical(lcBulkPropagatorDownloadJob) << "File" << QDir::toNativeSeparators(item->_file) << "can not be downloaded because it is non virtual!"; - abortWithError(item, SyncFileItem::NormalError, tr("File %1 cannot be downloaded because it is non virtual!").arg(QDir::toNativeSeparators(item->_file))); - return; - } - - if (!updateMetadata(item)) { - return; - } - - if (!item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) { - // make sure ReadOnly flag is preserved for placeholder, similarly to regular files - FileSystem::setFileReadOnly(propagator()->fullLocalPath(item->_file), true); - } - finalizeOneFile(item); -} - -void BulkPropagatorDownloadJob::finalizeOneFile(const SyncFileItemPtr &file) -{ - const auto foundIt = std::find_if(std::cbegin(_filesToDownload), std::cend(_filesToDownload), [&file](const auto &fileDownloading) { - return fileDownloading == file; - }); - if (foundIt != std::cend(_filesToDownload)) { - emit propagator()->itemCompleted(file, ErrorCategory::GenericError); - _filesToDownload.erase(foundIt); - } - checkPropagationIsDone(); -} - -void BulkPropagatorDownloadJob::checkPropagationIsDone() -{ - if (_filesToDownload.empty() && _filesToDownload.empty()) { - qCInfo(lcBulkPropagatorDownloadJob) << "finished with status" << SyncFileItem::Status::Success; - emit finished(SyncFileItem::Status::Success); - propagator()->scheduleNextJob(); - } -} - -void BulkPropagatorDownloadJob::start(const SyncFileItemPtr &item) -{ - if (propagator()->_abortRequested) { - return; - } - - qCDebug(lcBulkPropagatorDownloadJob) << item->_file << propagator()->_activeJobList.count(); - - const auto &path = item->_file; - const auto slashPosition = path.lastIndexOf('/'); - const auto &parentPath = slashPosition >= 0 ? path.left(slashPosition) : QString(); - - auto parentRec = SyncJournalFileRecord{}; - if (!propagator()->_journal->getFileRecord(parentPath, &parentRec)) { - qCWarning(lcBulkPropagatorDownloadJob) << "could not get file from local DB" << parentPath; - abortWithError(item, SyncFileItem::NormalError, tr("Could not get file %1 from local DB").arg(parentPath)); - return; - } - - if (!propagator()->account()->capabilities().clientSideEncryptionAvailable() || !parentRec.isValid() || !parentRec.isE2eEncrypted()) { - startAfterIsEncryptedIsChecked(item); - } else { - Q_ASSERT(false); - qCCritical(lcBulkPropagatorDownloadJob()) << "no encrypted fiels should be created via bulk download"; - } + return !_filesToDownload.empty(); } bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) diff --git a/src/libsync/bulkpropagatordownloadjob.h b/src/libsync/bulkpropagatordownloadjob.h index 62b64ecbaa..8097c21b46 100644 --- a/src/libsync/bulkpropagatordownloadjob.h +++ b/src/libsync/bulkpropagatordownloadjob.h @@ -28,11 +28,9 @@ public: public slots: void addDownloadItem(const OCC::SyncFileItemPtr &item); - void start(const OCC::SyncFileItemPtr &item); + [[nodiscard]] bool start(); private slots: - void startAfterIsEncryptedIsChecked(const OCC::SyncFileItemPtr &item); - void finalizeOneFile(const OCC::SyncFileItemPtr &file); void done(const OCC::SyncFileItem::Status status); From 1b6c2e68056fb6b74e6f6a90ec00ecf4cbdfbc0d Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 25 Aug 2025 15:52:19 +0200 Subject: [PATCH 10/13] fix(vfs/cfapi): skip inserting new placeholder items with invalid mtime filter new placeholder items by invalid modtime and only proceed with creating only the valid ones Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 2 +- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 36 +++++++++++++---------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index 3d7e3009ad..35896f8e2c 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -183,7 +183,7 @@ bool BulkPropagatorDownloadJob::start() checkPropagationIsDone(); - return !_filesToDownload.empty(); + return false; } bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index ad3e7359b1..59dbbfb874 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -908,16 +908,23 @@ OCC::Result OCC::CfApiWrapper::createPlaceholderInfo(const QStrin OCC::Result OCC::CfApiWrapper::createPlaceholdersInfo(const QString &localBasePath, const QList &itemsInfo) { - const auto stdWStringBasePath = localBasePath.toStdWString(); - auto cloudEntry = std::make_unique(itemsInfo.size()); + auto filteredItemsInfo = QList{}; + filteredItemsInfo.reserve(itemsInfo.size()); - for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { - const auto &placeholderInfo = itemsInfo[itemIndice]; - - if (placeholderInfo.modtime <= 0) { - return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(placeholderInfo.relativePath).arg(placeholderInfo.modtime)}; + std::copy_if(itemsInfo.begin(), itemsInfo.end(), std::back_inserter(filteredItemsInfo), [] (const auto &onePlaceholderInfo) -> bool { + if (onePlaceholderInfo.modtime <= 0) { + qCWarning(lcCfApiWrapper()) << "Skip invalid modtime file: " << onePlaceholderInfo.relativePath << "modtime:" << onePlaceholderInfo.modtime; + return false; } + return true; + }); + const auto stdWStringBasePath = localBasePath.toStdWString(); + auto cloudEntry = std::make_unique(filteredItemsInfo.size()); + + for(auto itemIndice = 0; itemIndice < filteredItemsInfo.size(); ++itemIndice) { + const auto &placeholderInfo = filteredItemsInfo[itemIndice]; + cloudEntry[itemIndice].FileIdentity = placeholderInfo.fileId.data(); cloudEntry[itemIndice].FileIdentityLength = static_cast(placeholderInfo.fileId.length()); @@ -935,29 +942,26 @@ OCC::Result OCC::CfApiWrapper::createPlaceholdersInfo(const QStri cloudEntry[itemIndice].FsMetadata.BasicInfo.FileAttributes = FILE_ATTRIBUTE_DIRECTORY; cloudEntry[itemIndice].FsMetadata.FileSize.QuadPart = 0; } - - qCDebug(lcCfApiWrapper) << "CfCreatePlaceholders" << stdWStringBasePath << placeholderInfo.platformNativeRelativePath << placeholderInfo.modtime << placeholderInfo.size; - qCDebug(lcCfApiWrapper) << QString::fromStdWString(cloudEntry[itemIndice].RelativeFileName); } auto numberOfCreatedPlaceholders = 0ul; - const qint64 result = CfCreatePlaceholders(stdWStringBasePath.data(), cloudEntry.get(), itemsInfo.size(), CF_CREATE_FLAG_NONE, &numberOfCreatedPlaceholders); + const qint64 result = CfCreatePlaceholders(stdWStringBasePath.data(), cloudEntry.get(), filteredItemsInfo.size(), CF_CREATE_FLAG_NONE, &numberOfCreatedPlaceholders); if (result != S_OK) { qCWarning(lcCfApiWrapper) << "Couldn't create placeholders info" << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << "number of placeholders created:" << numberOfCreatedPlaceholders; - for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { + for(auto itemIndice = 0; itemIndice < filteredItemsInfo.size(); ++itemIndice) { qCDebug(lcCfApiWrapper) << QString::fromStdWString(cloudEntry[itemIndice].RelativeFileName) << QString::fromWCharArray(_com_error(cloudEntry[itemIndice].Result).ErrorMessage()); } return { "Couldn't create placeholder info" }; } - for(auto itemIndice = 0; itemIndice < itemsInfo.size(); ++itemIndice) { - const auto &placeholderInfo = itemsInfo[itemIndice]; - const auto parentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(localBasePath + placeholderInfo.relativePath).absolutePath())); + for(auto itemIndice = 0; itemIndice < filteredItemsInfo.size(); ++itemIndice) { + const auto &placeholderInfo = filteredItemsInfo[itemIndice]; + const auto parentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(localBasePath + QDir::separator() + placeholderInfo.relativePath).absolutePath())); const auto state = parentInfo && parentInfo->PinState == CF_PIN_STATE_UNPINNED ? CF_PIN_STATE_UNPINNED : CF_PIN_STATE_INHERIT; - if (!setPinState(placeholderInfo.relativePath, cfPinStateToPinState(state), NoRecurse)) { + if (!setPinState(QDir::toNativeSeparators(QFileInfo(localBasePath + QDir::separator() + placeholderInfo.relativePath).absoluteFilePath()), cfPinStateToPinState(state), NoRecurse)) { return { "Couldn't set the default inherit pin state" }; } } From 571ee0c3bf4a97b82dbe3ce02506037feb751d75 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 29 Aug 2025 10:12:25 +0200 Subject: [PATCH 11/13] fix: fix small issues from review on bulk download job Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index 35896f8e2c..7b5e3a9766 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -126,7 +126,6 @@ void BulkPropagatorDownloadJob::finalizeOneFile(const SyncFileItemPtr &file) emit propagator()->itemCompleted(file, ErrorCategory::GenericError); _filesToDownload.erase(foundIt); } - checkPropagationIsDone(); } void BulkPropagatorDownloadJob::checkPropagationIsDone() @@ -145,13 +144,12 @@ bool BulkPropagatorDownloadJob::start() } for (const auto &fileToDownload : std::as_const(_filesToDownload)) { - Q_ASSERT(fileToDownload->_type == ItemTypeVirtualFileDehydration || fileToDownload->_type == ItemTypeVirtualFile); + Q_ASSERT(fileToDownload->_type == ItemTypeVirtualFile); if (propagator()->localFileNameClash(fileToDownload->_file)) { - _parentDirJob->appendTask(fileToDownload); finalizeOneFile(fileToDownload); - qCCritical(lcBulkPropagatorDownloadJob) << "File" << QDir::toNativeSeparators(fileToDownload->_file) << "can not be downloaded because it is non virtual!"; - abortWithError(fileToDownload, SyncFileItem::NormalError, tr("File %1 cannot be downloaded because it is non virtual!").arg(QDir::toNativeSeparators(fileToDownload->_file))); + qCWarning(lcBulkPropagatorDownloadJob) << "File" << QDir::toNativeSeparators(fileToDownload->_file) << "can not be downloaded because of a local file name clash!"; + abortWithError(fileToDownload, SyncFileItem::FileNameClash, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(fileToDownload->_file))); return false; } } From b3f575d74f6acca7f8211e304f6e78ef8fedd244 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 29 Aug 2025 10:13:11 +0200 Subject: [PATCH 12/13] fix: remove duplicated useless code in single item download job Signed-off-by: Matthieu Gallien --- src/libsync/propagatedownload.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 94ba15a09c..78019533f4 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -533,11 +533,6 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked() const auto fsPath = propagator()->fullLocalPath(_item->_file); makeParentFolderModifiable(fsPath); - // do a klaas' case clash check. - if (propagator()->localFileNameClash(_item->_file)) { - done(SyncFileItem::FileNameClash, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)), ErrorCategory::GenericError); - return; - } auto r = vfs->createPlaceholder(*_item); if (!r) { done(SyncFileItem::NormalError, r.error(), ErrorCategory::GenericError); From 1f0f5b0ac34b1fd9e514f28a5f23bc192fc53ce4 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 29 Aug 2025 13:03:43 +0200 Subject: [PATCH 13/13] fix: bulk download job is a child job, update state accordingly we should return false from BulkPropagatorDownloadJob::scheduleSelfOrChild() because we have no child jobs that are not done and waiting to be started ensure we update the job status only once (even in case of errors) ensure we update the state of the items once and only once Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatordownloadjob.cpp | 34 ++++++++++------------- src/libsync/bulkpropagatordownloadjob.h | 4 +-- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/libsync/bulkpropagatordownloadjob.cpp b/src/libsync/bulkpropagatordownloadjob.cpp index 7b5e3a9766..dd9ca34299 100644 --- a/src/libsync/bulkpropagatordownloadjob.cpp +++ b/src/libsync/bulkpropagatordownloadjob.cpp @@ -109,7 +109,11 @@ bool BulkPropagatorDownloadJob::scheduleSelfOrChild() _state = Running; - return start(); + start(); + + _state = Finished; + + return false; } PropagatorJob::JobParallelism BulkPropagatorDownloadJob::parallelism() const @@ -128,19 +132,11 @@ void BulkPropagatorDownloadJob::finalizeOneFile(const SyncFileItemPtr &file) } } -void BulkPropagatorDownloadJob::checkPropagationIsDone() -{ - if (_filesToDownload.empty()) { - qCInfo(lcBulkPropagatorDownloadJob) << "finished with status" << SyncFileItem::Status::Success; - emit finished(SyncFileItem::Status::Success); - propagator()->scheduleNextJob(); - } -} - -bool BulkPropagatorDownloadJob::start() +void BulkPropagatorDownloadJob::start() { if (propagator()->_abortRequested) { - return false; + abortWithError({}, SyncFileItem::NormalError, {}); + return; } for (const auto &fileToDownload : std::as_const(_filesToDownload)) { @@ -150,7 +146,7 @@ bool BulkPropagatorDownloadJob::start() finalizeOneFile(fileToDownload); qCWarning(lcBulkPropagatorDownloadJob) << "File" << QDir::toNativeSeparators(fileToDownload->_file) << "can not be downloaded because of a local file name clash!"; abortWithError(fileToDownload, SyncFileItem::FileNameClash, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(fileToDownload->_file))); - return false; + return; } } @@ -162,14 +158,16 @@ bool BulkPropagatorDownloadJob::start() if (!r) { qCCritical(lcBulkPropagatorDownloadJob) << "Could not create placholders:" << r.error(); for (const auto &fileToDownload : std::as_const(_filesToDownload)) { - abortWithError(fileToDownload, SyncFileItem::NormalError, r.error()); + finalizeOneFile(fileToDownload); } - return false; + abortWithError({}, SyncFileItem::NormalError, r.error()); + return; } for (const auto &fileToDownload : std::as_const(_filesToDownload)) { if (!updateMetadata(fileToDownload)) { - return false; + abortWithError(fileToDownload, SyncFileItem::NormalError, tr("Unable to update metadata of new file %1.", "error with update metadata of new Win VFS file").arg(fileToDownload->_file)); + return; } if (!fileToDownload->_remotePerm.isNull() && !fileToDownload->_remotePerm.hasPermission(RemotePermissions::CanWrite)) { @@ -179,9 +177,7 @@ bool BulkPropagatorDownloadJob::start() finalizeOneFile(fileToDownload); } - checkPropagationIsDone(); - - return false; + done(SyncFileItem::Success); } bool BulkPropagatorDownloadJob::updateMetadata(const SyncFileItemPtr &item) diff --git a/src/libsync/bulkpropagatordownloadjob.h b/src/libsync/bulkpropagatordownloadjob.h index 8097c21b46..4611e094da 100644 --- a/src/libsync/bulkpropagatordownloadjob.h +++ b/src/libsync/bulkpropagatordownloadjob.h @@ -28,7 +28,7 @@ public: public slots: void addDownloadItem(const OCC::SyncFileItemPtr &item); - [[nodiscard]] bool start(); + void start(); private slots: void finalizeOneFile(const OCC::SyncFileItemPtr &file); @@ -40,8 +40,6 @@ private slots: private: bool updateMetadata(const SyncFileItemPtr &item); - void checkPropagationIsDone(); - QList _filesToDownload; PropagateDownloadEncrypted *_downloadEncryptedHelper = nullptr;