From c7d3e405f2ab91990818af8a104a78aca54ba908 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 9 Oct 2024 15:20:50 +0200 Subject: [PATCH 01/12] Remove needrestart parameter in ConvertToPlaceholder functions --- src/libcommonserver/vfs.h | 4 ++-- .../propagation/executor/executorworker.cpp | 4 ++-- src/libsyncengine/syncpal/syncpal.cpp | 4 ++-- src/libsyncengine/syncpal/syncpal.h | 7 +++---- src/server/appserver.cpp | 11 ++++------- src/server/appserver.h | 2 +- src/server/vfs/mac/vfs_mac.cpp | 4 +--- src/server/vfs/mac/vfs_mac.h | 2 +- src/server/vfs/win/vfs_win.cpp | 2 +- src/server/vfs/win/vfs_win.h | 2 +- 10 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/libcommonserver/vfs.h b/src/libcommonserver/vfs.h index e3afa0886..60ba687b2 100644 --- a/src/libcommonserver/vfs.h +++ b/src/libcommonserver/vfs.h @@ -142,7 +142,7 @@ class Vfs : public QObject { * new placeholder shall supersede, for rename-replace actions with new downloads, * for example. */ - virtual bool convertToPlaceholder(const QString &path, const KDC::SyncFileItem &item, bool &needRestart) = 0; + virtual bool convertToPlaceholder(const QString &path, const KDC::SyncFileItem &item) = 0; virtual bool updateFetchStatus(const QString &tmpPath, const QString &path, qint64 received, bool &canceled, bool &finished) = 0; @@ -272,7 +272,7 @@ class VfsOff : public Vfs { bool updateMetadata(const QString &, time_t, time_t, qint64, const QByteArray &, QString *) override { return true; } bool createPlaceholder(const KDC::SyncPath &, const KDC::SyncFileItem &) override { return true; } bool dehydratePlaceholder(const QString &) override { return true; } - bool convertToPlaceholder(const QString &, const KDC::SyncFileItem &, bool &) override { return true; } + bool convertToPlaceholder(const QString &, const KDC::SyncFileItem &) override { return true; } bool updateFetchStatus(const QString &, const QString &, qint64, bool &, bool &) override { return true; } bool forceStatus(const QString &path, bool isSyncing, int progress, bool isHydrated = false) override; diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 71a4f2843..214b5aa51 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -866,8 +866,8 @@ bool ExecutorWorker::convertToPlaceholder(const SyncPath &relativeLocalPath, boo syncItem.setLocalNodeId(std::to_string(fileStat.inode)); - if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem, - needRestart)) { // TODO : should not use SyncFileItem + if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem)) { // TODO : should not use SyncFileItem + LOGW_WARN(_logger, L"Error in vfsConvertToPlaceholder: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); return false; } diff --git a/src/libsyncengine/syncpal/syncpal.cpp b/src/libsyncengine/syncpal/syncpal.cpp index dd21cefb3..6d77046c2 100644 --- a/src/libsyncengine/syncpal/syncpal.cpp +++ b/src/libsyncengine/syncpal/syncpal.cpp @@ -405,12 +405,12 @@ bool SyncPal::vfsCreatePlaceholder(const SyncPath &relativeLocalPath, const Sync return _vfsCreatePlaceholder(syncDbId(), relativeLocalPath, item); } -bool SyncPal::vfsConvertToPlaceholder(const SyncPath &path, const SyncFileItem &item, bool &needRestart) { +bool SyncPal::vfsConvertToPlaceholder(const SyncPath &path, const SyncFileItem &item) { if (!_vfsConvertToPlaceholder) { return false; } - return _vfsConvertToPlaceholder(syncDbId(), path, item, needRestart); + return _vfsConvertToPlaceholder(syncDbId(), path, item); } bool SyncPal::vfsUpdateMetadata(const SyncPath &path, const SyncTime &creationTime, const SyncTime &modtime, const int64_t size, diff --git a/src/libsyncengine/syncpal/syncpal.h b/src/libsyncengine/syncpal/syncpal.h index 014e1faad..125f00f0b 100644 --- a/src/libsyncengine/syncpal/syncpal.h +++ b/src/libsyncengine/syncpal/syncpal.h @@ -128,7 +128,7 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this { _vfsCreatePlaceholder = vfsCreatePlaceholder; } inline void setVfsConvertToPlaceholderCallback(bool (*vfsConvertToPlaceholder)(int, const SyncPath &, - const SyncFileItem &, bool &)) { + const SyncFileItem &)) { _vfsConvertToPlaceholder = vfsConvertToPlaceholder; } inline void setVfsUpdateMetadataCallback(bool (*vfsUpdateMetadata)(int, const SyncPath &, const SyncTime &, @@ -217,7 +217,7 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this { bool vfsSetPinState(const SyncPath &itemPath, PinState pinState); bool vfsStatus(const SyncPath &itemPath, bool &isPlaceholder, bool &isHydrated, bool &isSyncing, int &progress); bool vfsCreatePlaceholder(const SyncPath &relativeLocalPath, const SyncFileItem &item); - bool vfsConvertToPlaceholder(const SyncPath &path, const SyncFileItem &item, bool &needRestart); + bool vfsConvertToPlaceholder(const SyncPath &path, const SyncFileItem &item); bool vfsUpdateMetadata(const SyncPath &path, const SyncTime &creationTime, const SyncTime &modtime, const int64_t size, const NodeId &id, std::string &error); bool vfsUpdateFetchStatus(const SyncPath &tmpPath, const SyncPath &path, int64_t received, bool &canceled, @@ -287,8 +287,7 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this { bool (*_vfsStatus)(int syncDbId, const SyncPath &itemPath, bool &isPlaceholder, bool &isHydrated, bool &isSyncing, int &progress){nullptr}; bool (*_vfsCreatePlaceholder)(int syncDbId, const SyncPath &relativeLocalPath, const SyncFileItem &item){nullptr}; - bool (*_vfsConvertToPlaceholder)(int syncDbId, const SyncPath &path, const SyncFileItem &item, - bool &needRestart){nullptr}; + bool (*_vfsConvertToPlaceholder)(int syncDbId, const SyncPath &path, const SyncFileItem &item){nullptr}; bool (*_vfsUpdateMetadata)(int syncDbId, const SyncPath &path, const SyncTime &creationTime, const SyncTime &modtime, const int64_t size, const NodeId &id, std::string &error){nullptr}; bool (*_vfsUpdateFetchStatus)(int syncDbId, const SyncPath &tmpPath, const SyncPath &path, int64_t received, diff --git a/src/server/appserver.cpp b/src/server/appserver.cpp index a690eba90..efb47fde4 100644 --- a/src/server/appserver.cpp +++ b/src/server/appserver.cpp @@ -2421,18 +2421,15 @@ bool AppServer::vfsCreatePlaceholder(int syncDbId, const SyncPath &relativeLocal return true; } -bool AppServer::vfsConvertToPlaceholder(int syncDbId, const SyncPath &path, const SyncFileItem &item, bool &needRestart) { +bool AppServer::vfsConvertToPlaceholder(int syncDbId, const SyncPath &path, const SyncFileItem &item) { if (_vfsMap.find(syncDbId) == _vfsMap.end()) { LOG_WARN(Log::instance()->getLogger(), "Vfs not found in vfsMap for syncDbId=" << syncDbId); return false; } - if (!_vfsMap[syncDbId]->convertToPlaceholder(SyncName2QStr(path.native()), item, needRestart)) { - if (!needRestart) { - LOGW_WARN(Log::instance()->getLogger(), L"Error in Vfs::convertToPlaceholder for syncDbId=" - << syncDbId << L" and path=" << Path2WStr(item.path()).c_str()); - } - + if (!_vfsMap[syncDbId]->convertToPlaceholder(SyncName2QStr(path.native()), item)) { + LOGW_WARN(Log::instance()->getLogger(), L"Error in Vfs::convertToPlaceholder for syncDbId=" + << syncDbId << L" and path=" << Path2WStr(item.path()).c_str()); return false; } diff --git a/src/server/appserver.h b/src/server/appserver.h index 5edc79052..28e8c29a7 100644 --- a/src/server/appserver.h +++ b/src/server/appserver.h @@ -202,7 +202,7 @@ class AppServer : public SharedTools::QtSingleApplication { static bool vfsStatus(int syncDbId, const SyncPath &itemPath, bool &isPlaceholder, bool &isHydrated, bool &isSyncing, int &progress); static bool vfsCreatePlaceholder(int syncDbIdconst, const SyncPath &relativeLocalPath, const SyncFileItem &item); - static bool vfsConvertToPlaceholder(int syncDbId, const SyncPath &path, const SyncFileItem &item, bool &needRestart); + static bool vfsConvertToPlaceholder(int syncDbId, const SyncPath &path, const SyncFileItem &item); static bool vfsUpdateMetadata(int syncDbId, const SyncPath &path, const SyncTime &creationTime, const SyncTime &modtime, const int64_t size, const NodeId &id, std::string &error); static bool vfsUpdateFetchStatus(int syncDbId, const SyncPath &tmpPath, const SyncPath &path, int64_t received, diff --git a/src/server/vfs/mac/vfs_mac.cpp b/src/server/vfs/mac/vfs_mac.cpp index 1e9d312eb..1161dfbd9 100644 --- a/src/server/vfs/mac/vfs_mac.cpp +++ b/src/server/vfs/mac/vfs_mac.cpp @@ -340,9 +340,7 @@ bool VfsMac::dehydratePlaceholder(const QString &path) { return true; } -bool VfsMac::convertToPlaceholder(const QString &path, const SyncFileItem &item, bool &needRestart) { - needRestart = false; - +bool VfsMac::convertToPlaceholder(const QString &path, const SyncFileItem &item) { if (extendedLog()) { LOGW_DEBUG(logger(), L"convertToPlaceholder - " << Utility::formatPath(path).c_str()); } diff --git a/src/server/vfs/mac/vfs_mac.h b/src/server/vfs/mac/vfs_mac.h index 9a96a08ab..435248850 100644 --- a/src/server/vfs/mac/vfs_mac.h +++ b/src/server/vfs/mac/vfs_mac.h @@ -67,7 +67,7 @@ class VfsMac : public Vfs { bool createPlaceholder(const SyncPath &relativeLocalPath, const SyncFileItem &item) override; bool dehydratePlaceholder(const QString &path) override; - bool convertToPlaceholder(const QString &path, const SyncFileItem &item, bool &needRestart) override; + bool convertToPlaceholder(const QString &path, const SyncFileItem &item) override; bool updateFetchStatus(const QString &tmpPath, const QString &path, qint64 received, bool &canceled, bool &finished) override; void cancelHydrate(const QString &filePath) override; diff --git a/src/server/vfs/win/vfs_win.cpp b/src/server/vfs/win/vfs_win.cpp index d6cb46e0b..81b86cc90 100644 --- a/src/server/vfs/win/vfs_win.cpp +++ b/src/server/vfs/win/vfs_win.cpp @@ -340,7 +340,7 @@ bool VfsWin::dehydratePlaceholder(const QString &path) { return true; } -bool VfsWin::convertToPlaceholder(const QString &path, const SyncFileItem &item, bool &needRestart) { +bool VfsWin::convertToPlaceholder(const QString &path, const SyncFileItem &item) { LOGW_DEBUG(logger(), L"convertToPlaceholder: " << Utility::formatSyncPath(QStr2Path(path)).c_str()); if (path.isEmpty()) { diff --git a/src/server/vfs/win/vfs_win.h b/src/server/vfs/win/vfs_win.h index ec0692863..c55d8afa0 100644 --- a/src/server/vfs/win/vfs_win.h +++ b/src/server/vfs/win/vfs_win.h @@ -74,7 +74,7 @@ class VfsWin : public Vfs { bool createPlaceholder(const SyncPath &relativeLocalPath, const SyncFileItem &item) override; bool dehydratePlaceholder(const QString &path) override; - bool convertToPlaceholder(const QString &path, const SyncFileItem &item, bool &needRestart) override; + bool convertToPlaceholder(const QString &path, const SyncFileItem &item) override; void convertDirContentToPlaceholder(const QString &filePath, bool isHydratedIn) override; virtual void clearFileAttributes(const QString &path) override; From 225e24976e8a0b3d4e118980dc08d72f1ff3de65 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Fri, 11 Oct 2024 14:06:39 +0200 Subject: [PATCH 02/12] Intermediate commit --- .../propagation/executor/executorworker.cpp | 483 ++++++++++-------- .../propagation/executor/executorworker.h | 23 +- 2 files changed, 294 insertions(+), 212 deletions(-) diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 214b5aa51..b7f3a0e2b 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -123,21 +123,23 @@ void ExecutorWorker::execute() { changesCounter++; std::shared_ptr job = nullptr; + bool ignored = false; + bool bypassProgressComplete = false; switch (syncOp->type()) { case OperationType::Create: { - handleCreateOp(syncOp, job, hasError); + handleCreateOp(syncOp, job, hasError, ignored); break; } case OperationType::Edit: { - handleEditOp(syncOp, job, hasError); + handleEditOp(syncOp, job, hasError, ignored); break; } case OperationType::Move: { - handleMoveOp(syncOp, hasError); + handleMoveOp(syncOp, hasError, ignored, bypassProgressComplete); break; } case OperationType::Delete: { - handleDeleteOp(syncOp, hasError); + handleDeleteOp(syncOp, hasError, ignored, bypassProgressComplete); break; } default: { @@ -147,12 +149,14 @@ void ExecutorWorker::execute() { _executorExitCode = ExitCode::DataError; _executorExitCause = ExitCause::Unknown; hasError = true; - cancelAllOngoingJobs(); - break; } } if (hasError) { + if (!bypassProgressComplete) { + setProgressComplete(syncOp, SyncFileStatus::Error); + } + increaseErrorCount(syncOp); cancelAllOngoingJobs(); break; @@ -166,6 +170,14 @@ void ExecutorWorker::execute() { _ongoingJobs.insert({job->jobId(), job}); _jobToSyncOpMap.insert({job->jobId(), syncOp}); } else { + if (!bypassProgressComplete) { + if (ignored) { + setProgressComplete(syncOp, SyncFileStatus::Ignored); + } else { + setProgressComplete(syncOp, SyncFileStatus::Success); + } + } + if (syncOp->affectedNode()->id().has_value()) { std::unordered_set whiteList; SyncNodeCache::instance()->syncNodes(_syncPal->syncDbId(), SyncNodeType::WhiteList, whiteList); @@ -278,19 +290,36 @@ void ExecutorWorker::logCorrespondingNodeErrorMsg(const SyncOpPtr syncOp) { } } -void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError) { +void ExecutorWorker::setProgressComplete(const SyncOpPtr syncOp, SyncFileStatus status) { + SyncPath relativeLocalFilePath; + if (syncOp->type() == OperationType::Create || syncOp->type() == OperationType::Edit) { + relativeLocalFilePath = syncOp->nodePath(ReplicaSide::Local); + } else { + relativeLocalFilePath = syncOp->affectedNode()->getPath(); + } + + _syncPal->setProgressComplete(relativeLocalFilePath, status); +} + +// !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! +void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored) { // The execution of the create operation consists of three steps: // 1. If omit-flag is False, propagate the file or directory to target replica, because the object is missing there. // 2. Insert a new entry into the database, to avoid that the object is detected again by compute_ops() on the next sync // iteration. // 3. Update the update tree structures to ensure that follow-up operations can execute correctly, as they are based on the // information in these structures. + hasError = false; + ignored = false; + SyncPath relativeLocalFilePath = syncOp->nodePath(ReplicaSide::Local); assert(!relativeLocalFilePath.empty()); SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalFilePath; if (isLiteSyncActivated() && !syncOp->omit()) { bool isDehydratedPlaceholder = false; if (!checkLiteSyncInfoForCreate(syncOp, absoluteLocalFilePath, isDehydratedPlaceholder)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in checkLiteSyncInfoForCreate"); + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -304,12 +333,16 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptrdeleteNode(syncOp->affectedNode())) { LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: affectedNode name=" << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; hasError = true; return; } if (!targetUpdateTree(syncOp)->deleteNode(syncOp->correspondingNode())) { logCorrespondingNodeErrorMsg(syncOp); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; hasError = true; return; } @@ -326,43 +359,33 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraffectedNode()->lastmodified(), node)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } - - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Success); } else { if (!isLiteSyncActivated() && !enoughLocalSpace(syncOp)) { + _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); _executorExitCode = ExitCode::SystemError; _executorExitCause = ExitCause::NotEnoughDiskSpace; - _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); hasError = true; return; } bool exists = false; if (!hasRight(syncOp, exists)) { - if (syncOp->targetSide() == ReplicaSide::Local) { - // Ignore operation - _syncPal->setProgressComplete(relativeLocalFilePath, - SyncFileStatus::Ignored); // TODO : do not exclude item from sync, rollback - } else if (syncOp->targetSide() == ReplicaSide::Remote) { + if (syncOp->targetSide() == ReplicaSide::Remote) { if (!exists) { - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); _executorExitCode = ExitCode::DataError; _executorExitCause = ExitCause::UnexpectedFileSystemEvent; hasError = true; return; } - bool needRestart = false; - if (!convertToPlaceholder(relativeLocalFilePath, true, needRestart)) { - LOGW_SYNCPAL_WARN(_logger, L"Failed to create placeholder for: " + _executorExitCode = convertToPlaceholder(relativeLocalFilePath, true, _executorExitCause); + if (_executorExitCode != ExitCode::Unknown) { + LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - _executorExitCode = needRestart ? ExitCode::DataError : ExitCode::SystemError; - _executorExitCause = needRestart ? ExitCause::UnexpectedFileSystemEvent : ExitCause::Unknown; hasError = true; return; } @@ -378,7 +401,6 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraddError(err); } - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Ignored); std::shared_ptr sourceUpdateTree = affectedUpdateTree(syncOp); if (!sourceUpdateTree->deleteNode(syncOp->affectedNode())) { LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: node name=" @@ -390,10 +412,12 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraffectedNode()->type() == NodeType::Directory) { // Propagate the directory creation immediately in order to avoid blocking other dependant job creation if (!runCreateDirJob(syncOp, job)) { - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - std::shared_ptr createDirJob = std::dynamic_pointer_cast(job); if (createDirJob && (createDirJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_BAD_REQUEST || createDirJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_FORBIDDEN)) { checkAlreadyExcluded(absoluteLocalFilePath, createDirJob->parentDirId()); } - hasError = true; - if (syncOp->targetSide() == ReplicaSide::Local) { _executorExitCode = job->exitCode() == ExitCode::NeedRestart ? ExitCode::DataError : job->exitCode(); _executorExitCause = @@ -421,28 +441,20 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptrexitCode() == ExitCode::NeedRestart ? ExitCause::FileAlreadyExist : job->exitCause(); } + hasError = true; return; } - bool needRestart = false; - if (!convertToPlaceholder(relativeLocalFilePath, syncOp->targetSide() == ReplicaSide::Remote, needRestart)) { + _executorExitCode = + convertToPlaceholder(relativeLocalFilePath, syncOp->targetSide() == ReplicaSide::Remote, _executorExitCause); + if (_executorExitCode != ExitCode::Unknown) { LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - if (needRestart) { - _executorExitCode = ExitCode::DataError; - _executorExitCause = ExitCause::UnexpectedFileSystemEvent; - } else { - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::Unknown; - } - hasError = true; return; } job.reset(); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Success); } } } @@ -487,6 +499,7 @@ void ExecutorWorker::checkAlreadyExcluded(const SyncPath &absolutePath, const No _executorExitCause = ExitCause::FileAlreadyExist; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptr &job) noexcept { // 1. If omit-flag is False, propagate the file or directory to replica Y, because the object is missing there. std::shared_ptr newCorrespondingParentNode = nullptr; @@ -495,7 +508,6 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptrsetProgressComplete(syncOp->affectedNode()->getPath(), SyncFileStatus::Error); _executorExitCode = ExitCode::DataError; _executorExitCause = ExitCause::Unknown; return false; @@ -506,7 +518,6 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - _syncPal->setProgressComplete(syncOp->affectedNode()->getPath(), SyncFileStatus::Error); _executorExitCode = ExitCode::DataError; _executorExitCause = ExitCause::Unknown; return false; @@ -524,51 +535,10 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - - bool parentExists = false; - IoError ioError = IoError::Success; - if (!IoHelper::checkIfPathExists(absoluteLocalFilePath.parent_path(), parentExists, ioError)) { - LOGW_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " - << Utility::formatIoError(absoluteLocalFilePath.parent_path(), ioError).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::FileAccessError; - return false; - } - - if (ioError == IoError::AccessDenied) { - LOGW_WARN(_logger, L"Item misses search permission: " - << Utility::formatSyncPath(absoluteLocalFilePath.parent_path()).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::NoSearchPermission; - return false; - } - - bool exists = false; - if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { - LOGW_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " - << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::FileAccessError; - return false; - } - - if (ioError == IoError::AccessDenied) { - LOGW_WARN(_logger, - L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::NoSearchPermission; - return false; - } - - bool restart = !parentExists // Parent path does not exist on local replica. Rebuild the snapshots. - || exists; // Item already exist. Rebuild the snapshots. - - _executorExitCode = restart ? ExitCode::DataError : ExitCode::SystemError; - _executorExitCause = restart ? ExitCause::InvalidSnapshot : ExitCause::FileAccessError; return false; } @@ -577,7 +547,6 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptrsetProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); _executorExitCode = ExitCode::SystemError; _executorExitCause = ExitCause::Unknown; return false; @@ -600,7 +569,7 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); + // _executorExitCode and _executorExitCause are set by the above function return false; } @@ -671,18 +640,11 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptrnodePath(ReplicaSide::Local); SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalFilePath; if (syncOp->affectedNode()->type() == NodeType::Directory) { - bool needRestart = false; - if (!convertToPlaceholder(relativeLocalFilePath, syncOp->targetSide() == ReplicaSide::Remote, needRestart)) { - if (needRestart) { - LOGW_SYNCPAL_DEBUG(_logger, SyncName2WStr(syncOp->affectedNode()->name()).c_str() - << L" does not exist anymore. Aborting jobs and restarting sync."); - } else { - LOGW_SYNCPAL_WARN(_logger, L"Failed to create placeholder for: " - << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - } - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - _executorExitCode = needRestart ? ExitCode::Ok : ExitCode::SystemError; - _executorExitCause = ExitCause::Unknown; + _executorExitCode = + convertToPlaceholder(relativeLocalFilePath, syncOp->targetSide() == ReplicaSide::Remote, _executorExitCause); + if (_executorExitCode != ExitCode::Unknown) { + LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " + << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); _syncPal->setRestart(true); if (!_syncPal->updateTree(ReplicaSide::Local)->deleteNode(syncOp->affectedNode())) { @@ -706,23 +668,19 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); - _executorExitCode = needRestart - ? ExitCode::DataError - : ExitCode::SystemError; // TODO : data error here leads to a rebuild of the snapshots - // while just running the sync again should be enough - _executorExitCause = needRestart ? ExitCause::UnexpectedFileSystemEvent : ExitCause::Unknown; + _executorExitCode = convertToPlaceholder(relativeLocalFilePath, true, _executorExitCause); + if (_executorExitCode != ExitCode::Unknown) { + LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " + << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); return false; } uint64_t filesize = 0; if (!getFileSize(absoluteLocalFilePath, filesize)) { - LOGW_WARN(_logger, - L"Error in ExecutorWorker::getFileSize for " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + LOGW_SYNCPAL_WARN(_logger, L"Error in ExecutorWorker::getFileSize for " + << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -788,6 +746,7 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptrgetSyncFileItem(relativeLocalPath, syncItem)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to retrieve SyncFileItem associated to item: " << Utility::formatSyncPath(relativeLocalPath).c_str()); - _executorExitCode = ExitCode::DataError; - _executorExitCause = ExitCause::Unknown; - return false; + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; } - return _syncPal->vfsCreatePlaceholder(relativeLocalPath, syncItem); // TODO : should not use SyncFileItem + if (!_syncPal->vfsCreatePlaceholder(relativeLocalPath, syncItem)) { + // Check if the item already exists on local replica + SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; + bool exists = false; + IoError ioError = IoError::Success; + if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " + << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); + return ExitCode::SystemError; + } + + if (ioError == IoError::AccessDenied) { + LOGW_SYNCPAL_WARN(_logger, + L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::NoSearchPermission; + return ExitCode::SystemError; + } + + if (exists) { + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; + } else { + // Check if the parent folder exists on local replica + bool parentExists = false; + if (!IoHelper::checkIfPathExists(absoluteLocalFilePath.parent_path(), parentExists, ioError)) { + LOGW_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " + << Utility::formatIoError(absoluteLocalFilePath.parent_path(), ioError).c_str()); + return ExitCode::SystemError; + } + + if (ioError == IoError::AccessDenied) { + LOGW_WARN(_logger, L"Item misses search permission: " + << Utility::formatSyncPath(absoluteLocalFilePath.parent_path()).c_str()); + exitCause = ExitCause::NoSearchPermission; + return ExitCode::SystemError; + } + + if (!parentExists) { + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; + } + } + + exitCause = ExitCause::FileAccessError; + return ExitCode::SystemError; + } + + return ExitCode::Ok; } -bool ExecutorWorker::convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, bool &needRestart) { +ExitCode ExecutorWorker::convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, ExitCause &exitCause) { + exitCause = ExitCause::Unknown; + SyncFileItem syncItem; if (!_syncPal->getSyncFileItem(relativeLocalPath, syncItem)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to retrieve SyncFileItem associated to item: " << Utility::formatSyncPath(relativeLocalPath).c_str()); - _executorExitCode = ExitCode::DataError; - _executorExitCause = ExitCause::Unknown; - return false; + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; } - syncItem.setDehydrated(!hydrated); - SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; - // Update the local ID +#ifdef __APPLE__ + // VfsMac::convertToPlaceholder needs only SyncFileItem::_dehydrated + syncItem.setDehydrated(!hydrated); +#elif __WIN32 + // VfsWin::convertToPlaceholder needs only SyncFileItem::_localNodeId FileStat fileStat; IoError ioError = IoError::Success; if (!IoHelper::getFileStat(absoluteLocalFilePath, &fileStat, ioError)) { - LOGW_WARN(_logger, L"Error in IoHelper::getFileStat: " << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::Unknown; - return false; + LOGW_SYNCPAL_WARN(_logger, + L"Error in IoHelper::getFileStat: " << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); + return ExitCode::SystemError; } if (ioError == IoError::NoSuchFileOrDirectory) { - LOGW_WARN(_logger, L"Item does not exist anymore: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - _executorExitCode = ExitCode::DataError; - _executorExitCause = ExitCause::InvalidSnapshot; - return false; + LOGW_SYNCPAL_WARN(_logger, L"Item does not exist anymore: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; } else if (ioError == IoError::AccessDenied) { - LOGW_WARN(_logger, L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - _executorExitCode = ExitCode::SystemError; - _executorExitCause = ExitCause::NoSearchPermission; - return false; + LOGW_SYNCPAL_WARN(_logger, L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::NoSearchPermission; + return ExitCode::SystemError; } syncItem.setLocalNodeId(std::to_string(fileStat.inode)); +#endif - if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem)) { // TODO : should not use SyncFileItem - LOGW_WARN(_logger, L"Error in vfsConvertToPlaceholder: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - return false; + if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem)) { + LOGW_SYNCPAL_WARN(_logger, + L"Error in vfsConvertToPlaceholder: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::FileAccessError; + return ExitCode::SystemError; } if (!_syncPal->vfsSetPinState(absoluteLocalFilePath, hydrated ? PinState::AlwaysLocal : PinState::OnlineOnly)) { - LOGW_WARN(_logger, L"Error in vfsSetPinState: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - return false; + LOGW_SYNCPAL_WARN(_logger, L"Error in vfsSetPinState: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::FileAccessError; + return ExitCode::SystemError; } - return true; + return ExitCode::Ok; } -void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError) { +// !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! +void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored) { // The execution of the edit operation consists of three steps: // 1. If omit-flag is False, propagate the file to replicaY, replacing the existing one. // 2. Insert a new entry into the database, to avoid that the object is detected again by compute_ops() on the next sync // iteration. // 3. If the omit flag is False, update the updatetreeY structure to ensure that follow-up operations can execute correctly, // as they are based on the information in this structure. + hasError = false; + ignored = false; + SyncPath relativeLocalFilePath = syncOp->nodePath(ReplicaSide::Local); if (relativeLocalFilePath.empty()) { @@ -900,11 +915,18 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr bool ignoreItem = false; bool isSyncing = false; if (!checkLiteSyncInfoForEdit(syncOp, absoluteLocalFilePath, ignoreItem, isSyncing)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in checkLiteSyncInfoForEdit"); + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } - if (ignoreItem || isSyncing) { - _syncPal->setProgressComplete(relativeLocalFilePath, ignoreItem ? SyncFileStatus::Ignored : SyncFileStatus::Syncing); + + if (ignoreItem) { + ignored = true; + return; + } + + if (isSyncing) { return; } } @@ -917,36 +939,34 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr syncOp->affectedNode()->lastmodified(), node)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativeLocalFilePath, SyncFileStatus::Error); + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } } else { if (!enoughLocalSpace(syncOp)) { + _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); _executorExitCode = ExitCode::SystemError; _executorExitCause = ExitCause::NotEnoughDiskSpace; - _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); hasError = true; return; } bool exists = false; if (!hasRight(syncOp, exists)) { - // Ignore operation - _syncPal->setProgressComplete(relativeLocalFilePath, - SyncFileStatus::Ignored); // TODO: we do not want to propagate the changes, but they - // must be kept on local replica. - // Rename the file and exclude the renamed file + ignored = true; return; } if (!generateEditJob(syncOp, job)) { + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } } } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::generateEditJob(SyncOpPtr syncOp, std::shared_ptr &job) { // 1. If omit-flag is False, propagate the file to replicaY, replacing the existing one. if (syncOp->targetSide() == ReplicaSide::Local) { @@ -997,6 +1017,8 @@ bool ExecutorWorker::generateEditJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->id().has_value() ? syncOp->affectedNode()->id().value() : ""; LOGW_SYNCPAL_DEBUG(_logger, L"Do not upload dehydrated placeholders: " << Utility::formatSyncPath(absolutePath).c_str() @@ -1091,6 +1114,7 @@ bool ExecutorWorker::fixModificationDate(SyncOpPtr syncOp, const SyncPath &absol return true; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::checkLiteSyncInfoForEdit(SyncOpPtr syncOp, const SyncPath &absolutePath, bool &ignoreItem, bool &isSyncing) { ignoreItem = false; @@ -1109,6 +1133,7 @@ bool ExecutorWorker::checkLiteSyncInfoForEdit(SyncOpPtr syncOp, const SyncPath & if (isPlaceholder && !isHydrated) { ignoreItem = true; return fixModificationDate(syncOp, absolutePath); + // _executorExitCode and _executorExitCause are set by the above function } } else { if (isPlaceholder) { @@ -1162,21 +1187,24 @@ bool ExecutorWorker::checkLiteSyncInfoForEdit(SyncOpPtr syncOp, const SyncPath & return true; } -void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError) { +// !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! +void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete) { // The three execution steps are as follows: // 1. If omit-flag is False, move the object on replica Y (where it still needs to be moved) from uY to vY, changing the name // to nameX. // 2. Update the database entry, to avoid detecting the move operation again. // 3. If the omit flag is False, update the updatetreeY structure to ensure that follow-up operations can execute correctly, // as they are based on the information in this structure. - - SyncPath relativePath = syncOp->affectedNode()->getPath(); + hasError = false; + ignored = false; + bypassProgressComplete = false; if (syncOp->omit()) { // Do not generate job, only push changes in DB and update tree if (syncOp->hasConflict()) { bool propagateChange = true; hasError = propagateConflictToDbAndTree(syncOp, propagateChange); + // _executorExitCode and _executorExitCause are set by the above function Error err(_syncPal->syncDbId(), syncOp->conflict().localNode() != nullptr @@ -1202,27 +1230,29 @@ void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError) { if (!propagateMoveToDbAndTree(syncOp)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativePath, SyncFileStatus::Error); + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } } else { bool exists = false; if (!hasRight(syncOp, exists)) { - // Ignore operation - _syncPal->setProgressComplete(relativePath, - SyncFileStatus::Ignored); // TODO : do not exclude the item from sync, rollback changes + ignored = true; return; } - if (!generateMoveJob(syncOp)) { + if (!generateMoveJob(syncOp, ignored, bypassProgressComplete)) { + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } } } -bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { +// !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! +bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete) { + bypassProgressComplete = false; + // 1. If omit-flag is False, move the object on replica Y (where it still needs to be moved) from uY to vY, changing the name // to nameX. std::shared_ptr job = nullptr; @@ -1237,6 +1267,8 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { if (!correspondingNode) { LOGW_SYNCPAL_WARN(_logger, L"Corresponding node not found for item with " << Utility::formatSyncPath(syncOp->affectedNode()->getPath()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -1246,6 +1278,8 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { if (!parentNode) { LOGW_SYNCPAL_WARN(_logger, L"Parent node not found for item with " << Utility::formatSyncPath(correspondingNode->getPath()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -1261,6 +1295,8 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { if (!correspondingNode) { LOGW_SYNCPAL_WARN(_logger, L"Corresponding node not found for item " << Utility::formatSyncPath(syncOp->affectedNode()->getPath()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -1270,6 +1306,8 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { if (!parentNode) { LOGW_SYNCPAL_WARN(_logger, L"Parent node not found for item " << Utility::formatSyncPath(correspondingNode->getPath()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -1296,6 +1334,8 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { : correspondingNodeInOtherTree(parentNode); if (!remoteParentNode) { LOGW_SYNCPAL_WARN(_logger, L"Parent node not found for item " << Path2WStr(parentNode->getPath()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } job = std::make_shared(_syncPal->driveDbId(), absoluteDestLocalFilePath, @@ -1337,8 +1377,7 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { // Propagate changes to DB and update trees std::shared_ptr newNode = nullptr; if (!propagateChangeToDbAndTree(syncOp, job, newNode)) { - cancelAllOngoingJobs(); - _syncPal->setProgressComplete(relativeDestLocalFilePath, SyncFileStatus::Error); + // _executorExitCode and _executorExitCause are set by the above function return false; } @@ -1359,21 +1398,25 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) { _syncPal->addError(err); } - _syncPal->setProgressComplete(relativeOriginLocalFilePath, SyncFileStatus::Success); return true; } - return handleFinishedJob(job, syncOp, syncOp->affectedNode()->getPath()); + return handleFinishedJob(job, syncOp, syncOp->affectedNode()->getPath(), ignored, bypassProgressComplete); + // _executorExitCode and _executorExitCause are set by the above function } -void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError) { +// !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! +void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete) { // The three execution steps are as follows: // 1. If omit-flag is False, delete the file or directory on replicaY, because the objects till exists there // 2. Remove the entry from the database. If nX is a directory node, also remove all entries for each node n ∈ S. This avoids // that the object(s) are detected again by compute_ops() on the next sync iteration // 3. Update the update tree structures to ensure that follow-up operations can execute correctly, as they are based on the // information in these structures - SyncPath relativePath = syncOp->affectedNode()->getPath(); + hasError = false; + ignored = false; + bypassProgressComplete = false; + if (syncOp->omit()) { // Do not generate job, only push changes in DB and update tree if (syncOp->hasConflict() && @@ -1381,6 +1424,7 @@ void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError) { ConflictType::EditDelete) { // Error message handled with move operation in case Edit-Delete conflict bool propagateChange = true; hasError = propagateConflictToDbAndTree(syncOp, propagateChange); + // _executorExitCode and _executorExitCause are set by the above function Error err(_syncPal->syncDbId(), syncOp->conflict().localNode() != nullptr @@ -1404,29 +1448,31 @@ void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError) { } if (!propagateDeleteToDbAndTree(syncOp)) { + // _executorExitCode and _executorExitCause are set by the above function LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - _syncPal->setProgressComplete(relativePath, SyncFileStatus::Error); hasError = true; return; } } else { bool exists = false; if (!hasRight(syncOp, exists)) { - // Ignore operation - _syncPal->setProgressComplete(relativePath, - SyncFileStatus::Ignored); // TODO : do not exclude item from sync, rollback + ignored = true; return; } - if (!generateDeleteJob(syncOp)) { + if (!generateDeleteJob(syncOp, ignored, bypassProgressComplete)) { + // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } } } -bool ExecutorWorker::generateDeleteJob(SyncOpPtr syncOp) { +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! +bool ExecutorWorker::generateDeleteJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete) { + bypassProgressComplete = false; + // 1. If omit-flag is False, delete the file or directory on replicaY, because the objects till exists there std::shared_ptr job = nullptr; SyncPath relativeLocalFilePath = syncOp->nodePath(ReplicaSide::Local); @@ -1477,7 +1523,8 @@ bool ExecutorWorker::generateDeleteJob(SyncOpPtr syncOp) { job->setAffectedFilePath(relativeLocalFilePath); job->runSynchronously(); - return handleFinishedJob(job, syncOp, relativeLocalFilePath); + return handleFinishedJob(job, syncOp, relativeLocalFilePath, ignored, bypassProgressComplete); + // _executorExitCode and _executorExitCause are set by the above function } bool ExecutorWorker::hasRight(SyncOpPtr syncOp, bool &exists) { @@ -1684,12 +1731,20 @@ bool ExecutorWorker::deleteFinishedAsyncJobs() { SyncOpPtr syncOp = jobToSyncOpIt->second; SyncPath relativeLocalPath = syncOp->nodePath(ReplicaSide::Local); - if (!handleFinishedJob(job, syncOp, relativeLocalPath)) { + bool ignored = false; + bool bypassProgressComplete = false; + if (!handleFinishedJob(job, syncOp, relativeLocalPath, ignored, bypassProgressComplete)) { increaseErrorCount(syncOp); hasError = true; } if (!hasError) { + if (ignored) { + setProgressComplete(syncOp, SyncFileStatus::Ignored); + } else { + setProgressComplete(syncOp, SyncFileStatus::Success); + } + if (syncOp->affectedNode()->id().has_value()) { std::unordered_set whiteList; SyncNodeCache::instance()->syncNodes(_syncPal->syncDbId(), SyncNodeType::WhiteList, whiteList); @@ -1710,6 +1765,7 @@ bool ExecutorWorker::deleteFinishedAsyncJobs() { return !hasError; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::handleManagedBackError(ExitCause jobExitCause, SyncOpPtr syncOp, bool isInconsistencyIssue, bool downloadImpossible) { _executorExitCode = ExitCode::Ok; @@ -1717,6 +1773,7 @@ bool ExecutorWorker::handleManagedBackError(ExitCause jobExitCause, SyncOpPtr sy if (jobExitCause == ExitCause::NotFound && !downloadImpossible) { // The operation failed because the destination does not exist anymore _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; LOG_SYNCPAL_DEBUG(_logger, "Destination does not exist anymore, restarting sync."); return false; } @@ -1732,6 +1789,8 @@ bool ExecutorWorker::handleManagedBackError(ExitCause jobExitCause, SyncOpPtr sy if (!affectedUpdateTree(syncOp)->deleteNode(syncOp->affectedNode())) { LOGW_SYNCPAL_WARN( _logger, L"Error in UpdateTree::deleteNode: node name=" << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -1739,6 +1798,8 @@ bool ExecutorWorker::handleManagedBackError(ExitCause jobExitCause, SyncOpPtr sy if (!targetUpdateTree(syncOp)->deleteNode(syncOp->correspondingNode())) { LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: node name=" << SyncName2WStr(syncOp->correspondingNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } } @@ -1777,16 +1838,16 @@ bool isManagedBackError(ExitCause exitCause) { } } // namespace details -bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpPtr syncOp, const SyncPath &relativeLocalPath) { +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! +bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpPtr syncOp, const SyncPath &relativeLocalPath, + bool &ignored, bool &bypassProgressComplete) { + ignored = false; + bypassProgressComplete = false; + if (job->exitCode() == ExitCode::NeedRestart) { cancelAllOngoingJobs(); _syncPal->setRestart(true); - _syncPal->setProgressComplete( - relativeLocalPath, - SyncFileStatus::Success); // Not really success but the file should not appear in error in Finder - _executorExitCode = ExitCode::Ok; - - return false; + return true; } NodeId locaNodeId; @@ -1804,12 +1865,13 @@ bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpP job->exitCode() == ExitCode::BackError && details::isManagedBackError(job->exitCause())) { return handleManagedBackError(job->exitCause(), syncOp, isInconsistencyIssue, networkJob && networkJob->isDownloadImpossible()); + // _executorExitCode and _executorExitCause are set by the above function } if (job->exitCode() != ExitCode::Ok) { if (networkJob && (networkJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_FORBIDDEN || networkJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_CONFLICT)) { - handleForbiddenAction(syncOp, relativeLocalPath); + handleForbiddenAction(syncOp, relativeLocalPath, ignored); } else if (job->exitCode() == ExitCode::SystemError && (job->exitCause() == ExitCause::FileAccessError || job->exitCause() == ExitCause::MoveToTrashFailed)) { LOGW_DEBUG(_logger, L"Item: " << Utility::formatSyncPath(relativeLocalPath).c_str() @@ -1840,20 +1902,19 @@ bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpP } } else { // Cancel all queued jobs - _executorExitCode = job->exitCode(); - _executorExitCause = job->exitCause(); LOGW_SYNCPAL_WARN(_logger, L"Cancelling jobs. exit code: " << _executorExitCode << L" exit cause: " << _executorExitCause); cancelAllOngoingJobs(); - _syncPal->setProgressComplete(relativeLocalPath, SyncFileStatus::Error); + _executorExitCode = job->exitCode(); + _executorExitCause = job->exitCause(); return false; } } else { // Propagate changes to DB and update trees std::shared_ptr newNode; if (!propagateChangeToDbAndTree(syncOp, job, newNode)) { + // _executorExitCode and _executorExitCause are set by the above function cancelAllOngoingJobs(); - _syncPal->setProgressComplete(relativeLocalPath, SyncFileStatus::Error); return false; } @@ -1875,31 +1936,24 @@ bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpP } } - const bool bypassProgressComplete = - syncOp->affectedNode()->hasChangeEvent(OperationType::Create) && - syncOp->affectedNode()->hasChangeEvent( - OperationType::Delete); // TODO : If node has both create and delete events, bypass - // progress complete. But this should be refactored - // alongside UpdateTreeWorker::getOrCreateNodeFromPath - - if (!bypassProgressComplete) { - _syncPal->setProgressComplete(relativeLocalPath, status); - } + bypassProgressComplete = syncOp->affectedNode()->hasChangeEvent(OperationType::Create) && + syncOp->affectedNode()->hasChangeEvent(OperationType::Delete); } return true; } -void ExecutorWorker::handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath) { +void ExecutorWorker::handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath, bool &ignored) { + ignored = false; + const SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; bool removeFromDb = true; - SyncFileStatus status = SyncFileStatus::Success; CancelType cancelType = CancelType::None; switch (syncOp->type()) { case OperationType::Create: { cancelType = CancelType::Create; - status = SyncFileStatus::Ignored; + ignored = true; removeFromDb = false; PlatformInconsistencyCheckerUtility::renameLocalFile(absoluteLocalFilePath, PlatformInconsistencyCheckerUtility::SuffixTypeBlacklisted); @@ -1951,7 +2005,6 @@ void ExecutorWorker::handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &rel syncOp->affectedNode()->moveOrigin().has_value() ? relativeLocalPath : ""); _syncPal->addError(err); } - _syncPal->setProgressComplete(relativeLocalPath, status); if (removeFromDb) { // Remove the node from DB and tree so it will be re-created at its original location on next sync @@ -1976,6 +2029,7 @@ void ExecutorWorker::sendProgress() { } } +// !!! When returning true (hasError), _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propagateChange) { propagateChange = true; @@ -1998,6 +2052,7 @@ bool ExecutorWorker::propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propag if (localNodeFoundInDb) { // Remove local node from DB if (!deleteFromDb(syncOp->conflict().localNode())) { + // _executorExitCode and _executorExitCause are set by the above function propagateChange = false; return true; } @@ -2054,10 +2109,12 @@ bool ExecutorWorker::propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propag return false; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateChangeToDbAndTree(SyncOpPtr syncOp, std::shared_ptr job, std::shared_ptr &node) { if (syncOp->hasConflict()) { bool propagateChange = true; bool hasError = propagateConflictToDbAndTree(syncOp, propagateChange); + // _executorExitCode and _executorExitCause are set by the above function if (!propagateChange || hasError) { return !hasError; } @@ -2098,25 +2155,31 @@ bool ExecutorWorker::propagateChangeToDbAndTree(SyncOpPtr syncOp, std::shared_pt if (syncOp->type() == OperationType::Create) { return propagateCreateToDbAndTree(syncOp, nodeId, modtime, node); + // _executorExitCode and _executorExitCause are set by the above function } else { return propagateEditToDbAndTree(syncOp, nodeId, modtime, node); + // _executorExitCode and _executorExitCause are set by the above function } } case OperationType::Move: { return propagateMoveToDbAndTree(syncOp); + // _executorExitCode and _executorExitCause are set by the above function } case OperationType::Delete: { return propagateDeleteToDbAndTree(syncOp); + // _executorExitCode and _executorExitCause are set by the above function } default: { LOGW_SYNCPAL_WARN(_logger, L"Unknown operation type " << syncOp->type() << L" on file " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); + _executorExitCode = ExitCode::SystemError; + _executorExitCause = ExitCause::Unknown; + return false; } } - - return false; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateCreateToDbAndTree(SyncOpPtr syncOp, const NodeId &newNodeId, std::optional newLastModTime, std::shared_ptr &node) { std::shared_ptr newCorrespondingParentNode = nullptr; @@ -2258,6 +2321,8 @@ bool ExecutorWorker::propagateCreateToDbAndTree(SyncOpPtr syncOp, const NodeId & LOGW_SYNCPAL_WARN(_logger, L"Error in Node::insertChildren: node name=" << SyncName2WStr(node->name()).c_str() << L" parent node name=" << SyncName2WStr(newCorrespondingParentNode->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -2268,6 +2333,7 @@ bool ExecutorWorker::propagateCreateToDbAndTree(SyncOpPtr syncOp, const NodeId & return true; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateEditToDbAndTree(SyncOpPtr syncOp, const NodeId &newNodeId, std::optional newLastModTime, std::shared_ptr &node) { DbNode dbNode; @@ -2355,6 +2421,7 @@ bool ExecutorWorker::propagateEditToDbAndTree(SyncOpPtr syncOp, const NodeId &ne return true; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateMoveToDbAndTree(SyncOpPtr syncOp) { std::shared_ptr correspondingNode = syncOp->correspondingNode() ? syncOp->correspondingNode() : syncOp->affectedNode(); // No corresponding node => rename @@ -2362,6 +2429,7 @@ bool ExecutorWorker::propagateMoveToDbAndTree(SyncOpPtr syncOp) { if (!correspondingNode || !correspondingNode->idb().has_value()) { LOG_SYNCPAL_WARN(_logger, "Invalid corresponding node"); _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -2463,6 +2531,8 @@ bool ExecutorWorker::propagateMoveToDbAndTree(SyncOpPtr syncOp) { LOGW_SYNCPAL_WARN(_logger, L"Error in Node::setParentNode: node name=" << SyncName2WStr(parentNode->name()).c_str() << L" parent node name=" << SyncName2WStr(correspondingNode->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } @@ -2470,6 +2540,8 @@ bool ExecutorWorker::propagateMoveToDbAndTree(SyncOpPtr syncOp) { LOGW_SYNCPAL_WARN(_logger, L"Error in Node::insertChildren: node name=" << SyncName2WStr(correspondingNode->name()).c_str() << L" parent node name=" << SyncName2WStr(correspondingNode->parentNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } } @@ -2477,10 +2549,12 @@ bool ExecutorWorker::propagateMoveToDbAndTree(SyncOpPtr syncOp) { return true; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::propagateDeleteToDbAndTree(SyncOpPtr syncOp) { // 2. Remove the entry from the database. If nX is a directory node, also remove all entries for each node n ∈ S. This // avoids that the object(s) are detected again by compute_ops() on the next sync iteration if (!deleteFromDb(syncOp->affectedNode())) { + // _executorExitCode and _executorExitCause are set by the above function return false; } @@ -2488,18 +2562,23 @@ bool ExecutorWorker::propagateDeleteToDbAndTree(SyncOpPtr syncOp) { if (!affectedUpdateTree(syncOp)->deleteNode(syncOp->affectedNode())) { LOGW_SYNCPAL_WARN( _logger, L"Error in UpdateTree::deleteNode: node name=" << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } if (!targetUpdateTree(syncOp)->deleteNode(syncOp->correspondingNode())) { LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: node name=" << SyncName2WStr(syncOp->correspondingNode()->name()).c_str()); + _executorExitCode = ExitCode::DataError; + _executorExitCause = ExitCause::Unknown; return false; } return true; } +// !!! When returning false, _executorExitCode and _executorExitCause must be set !!! bool ExecutorWorker::deleteFromDb(std::shared_ptr node) { if (!node->idb().has_value()) { LOGW_SYNCPAL_WARN(_logger, L"Node " << SyncName2WStr(node->name()).c_str() << L" does not have a DB ID"); diff --git a/src/libsyncengine/propagation/executor/executorworker.h b/src/libsyncengine/propagation/executor/executorworker.h index 3f6cbc390..47990e86b 100644 --- a/src/libsyncengine/propagation/executor/executorworker.h +++ b/src/libsyncengine/propagation/executor/executorworker.h @@ -69,14 +69,14 @@ class ExecutorWorker : public OperationProcessor { void initProgressManager(); bool initSyncFileItem(SyncOpPtr syncOp, SyncFileItem &syncItem); - void handleCreateOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError); + void handleCreateOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored); void checkAlreadyExcluded(const SyncPath &absolutePath, const NodeId &parentId); bool generateCreateJob(SyncOpPtr syncOp, std::shared_ptr &job) noexcept; bool checkLiteSyncInfoForCreate(SyncOpPtr syncOp, const SyncPath &path, bool &isDehydratedPlaceholder); - bool createPlaceholder(const SyncPath &relativeLocalPath); - bool convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, bool &needRestart); + ExitCode createPlaceholder(const SyncPath &relativeLocalPath, ExitCause &exitCause); + ExitCode convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, ExitCause &exitCause); - void handleEditOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError); + void handleEditOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored); bool generateEditJob(SyncOpPtr syncOp, std::shared_ptr &job); /** @@ -91,11 +91,11 @@ class ExecutorWorker : public OperationProcessor { bool &isSyncing); // TODO : is called "check..." but perform some actions. Wording not // good, function probably does too much - void handleMoveOp(SyncOpPtr syncOp, bool &hasError); - bool generateMoveJob(SyncOpPtr syncOp); + void handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete); + bool generateMoveJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete); - void handleDeleteOp(SyncOpPtr syncOp, bool &hasError); - bool generateDeleteJob(SyncOpPtr syncOp); + void handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete); + bool generateDeleteJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete); bool hasRight(SyncOpPtr syncOp, bool &exists); bool enoughLocalSpace(SyncOpPtr syncOp); @@ -103,8 +103,9 @@ class ExecutorWorker : public OperationProcessor { void waitForAllJobsToFinish(bool &hasError); bool deleteFinishedAsyncJobs(); bool handleManagedBackError(ExitCause jobExitCause, SyncOpPtr syncOp, bool isInconsistencyIssue, bool downloadImpossible); - bool handleFinishedJob(std::shared_ptr job, SyncOpPtr syncOp, const SyncPath &relativeLocalPath); - void handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath); + bool handleFinishedJob(std::shared_ptr job, SyncOpPtr syncOp, const SyncPath &relativeLocalPath, + bool &ignored, bool &bypassProgressComplete); + void handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath, bool &ignored); void sendProgress(); bool propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propagateChange); @@ -137,6 +138,8 @@ class ExecutorWorker : public OperationProcessor { bool getFileSize(const SyncPath &path, uint64_t &size); void logCorrespondingNodeErrorMsg(const SyncOpPtr syncOp); + void setProgressComplete(const SyncOpPtr syncOp, SyncFileStatus status); + std::unordered_map> _ongoingJobs; TerminatedJobsQueue _terminatedJobs; std::unordered_map _jobToSyncOpMap; From 3e46fa47c5208aaf0a063d77720dfe5b4e0ef269 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Fri, 11 Oct 2024 17:20:08 +0200 Subject: [PATCH 03/12] Manage Inconsistency status --- .../propagation/executor/executorworker.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index b7f3a0e2b..9bec2f5e2 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -173,6 +173,8 @@ void ExecutorWorker::execute() { if (!bypassProgressComplete) { if (ignored) { setProgressComplete(syncOp, SyncFileStatus::Ignored); + } else if (syncOp->affectedNode() && syncOp->affectedNode()->inconsistencyType() != InconsistencyType::None) { + setProgressComplete(syncOp, SyncFileStatus::Inconsistency); } else { setProgressComplete(syncOp, SyncFileStatus::Success); } @@ -383,7 +385,7 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); hasError = true; @@ -447,7 +449,7 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptrtargetSide() == ReplicaSide::Remote, _executorExitCause); - if (_executorExitCode != ExitCode::Unknown) { + if (_executorExitCode != ExitCode::Ok) { LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); hasError = true; @@ -536,7 +538,7 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); return false; @@ -573,15 +575,11 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->inconsistencyType() != InconsistencyType::None) { // Notify user that the file has been renamed SyncFileItem syncItem; if (_syncPal->getSyncFileItem(relativeLocalFilePath, syncItem)) { - status = SyncFileStatus::Inconsistency; - Error err(_syncPal->syncDbId(), syncItem.localNodeId().has_value() ? syncItem.localNodeId().value() : "", syncItem.remoteNodeId().has_value() ? syncItem.remoteNodeId().value() : "", syncItem.type(), syncItem.newPath().has_value() ? syncItem.newPath().value() : syncItem.path(), syncItem.conflict(), @@ -589,8 +587,6 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraddError(err); } } - - _syncPal->setProgressComplete(relativeLocalFilePath, status); } else { if (syncOp->affectedNode()->type() == NodeType::Directory) { job = std::make_shared(absoluteLocalFilePath); @@ -642,7 +638,7 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->type() == NodeType::Directory) { _executorExitCode = convertToPlaceholder(relativeLocalFilePath, syncOp->targetSide() == ReplicaSide::Remote, _executorExitCause); - if (_executorExitCode != ExitCode::Unknown) { + if (_executorExitCode != ExitCode::Ok) { LOGW_SYNCPAL_WARN(_logger, L"Failed to convert to placeholder for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); _syncPal->setRestart(true); @@ -669,7 +665,7 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); return false; From 177a0fe05f86e20d9e7ef2243120415da8b626bd Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Mon, 14 Oct 2024 10:45:37 +0200 Subject: [PATCH 04/12] Optimization --- .../snapshot/snapshot.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp index ec45fb13a..4c6d08264 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp @@ -488,22 +488,20 @@ bool Snapshot::checkIntegrityRecursively() { bool Snapshot::checkIntegrityRecursively(const NodeId &parentId) { // Check that we do not have the same file twice in the same folder - const auto &parrentItem = _items[parentId]; - for (auto child = parrentItem.childrenIds().begin(), end = parrentItem.childrenIds().end(); child != end; child++) { - if (!checkIntegrityRecursively(*child)) { + const auto &parentItem = _items[parentId]; + std::set names; + for (auto childId = parentItem.childrenIds().begin(), end = parentItem.childrenIds().end(); childId != end; childId++) { + if (!checkIntegrityRecursively(*childId)) { return false; } - for (auto child2 = child; child2 != end; ++child2) { - if (*child != *child2 && _items[*child].name() == _items[*child2].name()) { - LOG_ERROR(Log::instance()->getLogger(), "Snapshot integrity check failed, the folder named: \"" - << SyncName2Str(parrentItem.name()).c_str() << "\"(" - << parrentItem.id().c_str() << ") contains: \"" - << SyncName2Str(_items[*child].name()).c_str() - << "\" twice with two differents NodeId (" << child->c_str() - << " and " << child2->c_str() << ")"); - return false; - } + auto result = names.insert(_items[*childId].name()); + if (!result.second) { + LOG_ERROR(Log::instance()->getLogger(), + "Snapshot integrity check failed, the folder named: \"" + << SyncName2Str(parentItem.name()).c_str() << "\"(" << parentItem.id().c_str() << ") contains: \"" + << SyncName2Str(_items[*childId].name()).c_str() << "\" twice with two differents NodeId"); + return false; } } return true; From 7ff7f579b2aa121c3f6792ed6a121c820153c289 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Tue, 15 Oct 2024 16:23:06 +0200 Subject: [PATCH 05/12] Intermediate commit --- src/libcommon/theme/theme.cpp | 1 - .../propagation/executor/executorworker.cpp | 34 +- .../propagation/executor/executorworker.h | 1 + src/libsyncengine/syncpal/syncpal.cpp | 8 +- src/libsyncengine/syncpal/syncpal.h | 2 + src/server/vfs/mac/litesyncextconnector.mm | 26 +- test/libsyncengine/db/testsyncdb.cpp | 4 +- .../jobs/network/testnetworkjobs.cpp | 2 +- test/libsyncengine/jobs/testjobmanager.cpp | 2 +- .../executor/testexecutorworker.cpp | 7 +- .../propagation/executor/testintegration.cpp | 4 +- .../testoperationsorterworker.cpp | 6 +- .../testconflictfinderworker.cpp | 8 +- .../testconflictresolverworker.cpp | 4 +- .../testoperationgeneratorworker.cpp | 4 +- ...testplatforminconsistencycheckerworker.cpp | 4 +- .../requests/testexclusiontemplatecache.cpp | 2 +- test/libsyncengine/syncpal/testsyncpal.cpp | 4 +- .../testcomputefsoperationworker.cpp | 4 +- .../testlocalfilesystemobserverworker.cpp | 4 +- .../testremotefilesystemobserverworker.cpp | 4 +- .../update_detector/testupdatetreeworker.cpp | 2 +- test/server/CMakeLists.txt | 25 +- test/server/test.cpp | 5 + test/server/updater/testabstractupdater.cpp | 2 +- test/server/updater/testupdater.h | 2 - test/server/workers/testworkers.cpp | 301 ++++++++++++++++++ test/server/workers/testworkers.h | 65 ++++ test/test_utility/testhelpers.h | 1 + 29 files changed, 478 insertions(+), 60 deletions(-) create mode 100644 test/server/workers/testworkers.cpp create mode 100644 test/server/workers/testworkers.h diff --git a/src/libcommon/theme/theme.cpp b/src/libcommon/theme/theme.cpp index 0d8d4e119..75f9bebe8 100644 --- a/src/libcommon/theme/theme.cpp +++ b/src/libcommon/theme/theme.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include namespace KDC { diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 9bec2f5e2..21474a91a 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -384,14 +384,6 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - hasError = true; - return; - } - // Ignore operation SyncFileItem syncItem; if (_syncPal->getSyncFileItem(relativeLocalFilePath, syncItem)) { @@ -782,6 +774,7 @@ ExitCode ExecutorWorker::createPlaceholder(const SyncPath &relativeLocalPath, Ex return ExitCode::DataError; } if (!_syncPal->vfsCreatePlaceholder(relativeLocalPath, syncItem)) { + // TODO: vfs functions should output an ioError parameter // Check if the item already exists on local replica SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; bool exists = false; @@ -871,8 +864,29 @@ ExitCode ExecutorWorker::convertToPlaceholder(const SyncPath &relativeLocalPath, #endif if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem)) { - LOGW_SYNCPAL_WARN(_logger, - L"Error in vfsConvertToPlaceholder: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + // TODO: vfs functions should output an ioError parameter + // Check that the item exists on local replica + SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; + bool exists = false; + IoError ioError = IoError::Success; + if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " + << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); + return ExitCode::SystemError; + } + + if (ioError == IoError::AccessDenied) { + LOGW_SYNCPAL_WARN(_logger, + L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::NoSearchPermission; + return ExitCode::SystemError; + } + + if (!exists) { + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; + } + exitCause = ExitCause::FileAccessError; return ExitCode::SystemError; } diff --git a/src/libsyncengine/propagation/executor/executorworker.h b/src/libsyncengine/propagation/executor/executorworker.h index 47990e86b..26db3131b 100644 --- a/src/libsyncengine/propagation/executor/executorworker.h +++ b/src/libsyncengine/propagation/executor/executorworker.h @@ -154,6 +154,7 @@ class ExecutorWorker : public OperationProcessor { bool _snapshotToInvalidate = false; friend class TestExecutorWorker; + friend class TestWorkers; }; } // namespace KDC diff --git a/src/libsyncengine/syncpal/syncpal.cpp b/src/libsyncengine/syncpal/syncpal.cpp index 6d77046c2..e1c26a7ea 100644 --- a/src/libsyncengine/syncpal/syncpal.cpp +++ b/src/libsyncengine/syncpal/syncpal.cpp @@ -382,7 +382,7 @@ bool SyncPal::vfsPinState(const SyncPath &itemPath, PinState &pinState) { } bool SyncPal::vfsSetPinState(const SyncPath &itemPath, PinState pinState) { - if (!_vfsPinState) { + if (!_vfsSetPinState) { return false; } @@ -998,6 +998,10 @@ std::shared_ptr SyncPal::updateTree(ReplicaSide side) const { return (side == ReplicaSide::Local ? _localUpdateTree : _remoteUpdateTree); } +void SyncPal::createProgressInfo() { + _progressInfo = std::shared_ptr(new ProgressInfo(shared_from_this())); +} + ExitCode SyncPal::fileRemoteIdFromLocalPath(const SyncPath &path, NodeId &nodeId) const { DbNodeId dbNodeId = -1; bool found = false; @@ -1176,7 +1180,7 @@ void SyncPal::start() { SyncNodeCache::instance()->update(syncDbId(), SyncNodeType::TmpLocalBlacklist, std::unordered_set()); // Create ProgressInfo - _progressInfo = std::shared_ptr(new ProgressInfo(shared_from_this())); + createProgressInfo(); // Create workers createWorkers(); diff --git a/src/libsyncengine/syncpal/syncpal.h b/src/libsyncengine/syncpal/syncpal.h index 125f00f0b..3ab11a6b0 100644 --- a/src/libsyncengine/syncpal/syncpal.h +++ b/src/libsyncengine/syncpal/syncpal.h @@ -353,6 +353,7 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this { std::shared_ptr updateTree(ReplicaSide side) const; // Progress info management + void createProgressInfo(); void resetEstimateUpdates(); void startEstimateUpdates(); void stopEstimateUpdates(); @@ -399,6 +400,7 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this { friend class TestSnapshot; friend class TestLocalJobs; friend class TestIntegration; + friend class TestWorkers; }; } // namespace KDC diff --git a/src/server/vfs/mac/litesyncextconnector.mm b/src/server/vfs/mac/litesyncextconnector.mm index 419eb15aa..4a8fac94c 100644 --- a/src/server/vfs/mac/litesyncextconnector.mm +++ b/src/server/vfs/mac/litesyncextconnector.mm @@ -1001,13 +1001,27 @@ bool checkIoErrorAndLogIfNeeded(IoError ioError, const std::string &itemType, co } QString path = localSyncPath + "/" + relativePath; - std::string stdPath = path.toStdString(); - if (fileStat->st_mode == S_IFDIR || fileStat->st_mode == S_IFREG) { - int flags = fileStat->st_mode == S_IFDIR ? O_DIRECTORY | O_CREAT // Directory - : O_CREAT | FWRITE | O_NONBLOCK; // File - int mode = fileStat->st_mode == S_IFDIR ? S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH // Rights rwxr-xr-x - : S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH; // Rights rw-r--r-- + if (fileStat->st_mode == S_IFDIR) { + SyncPath dirPath{QStr2Path(path)}; + IoError ioError = IoError::Success; + if (!IoHelper::createDirectory(dirPath, ioError)) { + LOGW_WARN(_logger, + L"Call to IoHelper::createDirectory failed: " + << Utility::formatSyncPath(dirPath).c_str()); + return false; + } + + if (ioError != IoError::Success) { + LOGW_WARN(_logger, + L"Failed to create directory: " + << Utility::formatIoError(dirPath, ioError).c_str()); + return false; + } + } else if (fileStat->st_mode == S_IFREG) { + std::string stdPath = QStr2Str(path); + int flags = O_CREAT | FWRITE | O_NONBLOCK; + int mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH; // Rights rw-r--r-- int fd = open(stdPath.c_str(), flags, mode); if (fd == -1) { diff --git a/test/libsyncengine/db/testsyncdb.cpp b/test/libsyncengine/db/testsyncdb.cpp index 9eb21ec12..b9ebd2c9b 100644 --- a/test/libsyncengine/db/testsyncdb.cpp +++ b/test/libsyncengine/db/testsyncdb.cpp @@ -88,8 +88,8 @@ void TestSyncDb::setUp() { std::filesystem::remove(syncDbPath); // Create DB - _testObj = new SyncDbMock(syncDbPath.string(), "3.4.0"); - _testObj->init("3.4.0"); + _testObj = new SyncDbMock(syncDbPath.string(), KDRIVE_VERSION_STRING); + _testObj->init(KDRIVE_VERSION_STRING); _testObj->setAutoDelete(true); } diff --git a/test/libsyncengine/jobs/network/testnetworkjobs.cpp b/test/libsyncengine/jobs/network/testnetworkjobs.cpp index d097fa04b..4de05dec1 100644 --- a/test/libsyncengine/jobs/network/testnetworkjobs.cpp +++ b/test/libsyncengine/jobs/network/testnetworkjobs.cpp @@ -89,7 +89,7 @@ void TestNetworkJobs::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ParametersCache::instance()->parameters().setExtendedLog(true); // Insert user, account & drive diff --git a/test/libsyncengine/jobs/testjobmanager.cpp b/test/libsyncengine/jobs/testjobmanager.cpp index 59e049801..0fc091de9 100644 --- a/test/libsyncengine/jobs/testjobmanager.cpp +++ b/test/libsyncengine/jobs/testjobmanager.cpp @@ -59,7 +59,7 @@ void KDC::TestJobManager::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ParametersCache::instance()->parameters().setExtendedLog(true); // Insert user, account & drive diff --git a/test/libsyncengine/propagation/executor/testexecutorworker.cpp b/test/libsyncengine/propagation/executor/testexecutorworker.cpp index b329ae298..48c75c995 100644 --- a/test/libsyncengine/propagation/executor/testexecutorworker.cpp +++ b/test/libsyncengine/propagation/executor/testexecutorworker.cpp @@ -18,7 +18,6 @@ #include "testexecutorworker.h" -#include #include "propagation/executor/executorworker.h" #include "io/filestat.h" #include "io/iohelper.h" @@ -26,6 +25,8 @@ #include "network/proxy.h" #include "test_utility/testhelpers.h" +#include + namespace KDC { void TestExecutorWorker::setUp() { @@ -42,7 +43,7 @@ void TestExecutorWorker::setUp() { bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); std::filesystem::remove(parmsDbPath); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); // Insert user, account, drive & sync int userId(12321); @@ -67,7 +68,7 @@ void TestExecutorWorker::setUp() { Proxy::instance(parameters.proxyConfig()); } - _syncPal = std::make_shared(_sync.dbId(), "3.4.0"); + _syncPal = std::make_shared(_sync.dbId(), KDRIVE_VERSION_STRING); _syncPal->createWorkers(); _syncPal->syncDb()->setAutoDelete(true); } diff --git a/test/libsyncengine/propagation/executor/testintegration.cpp b/test/libsyncengine/propagation/executor/testintegration.cpp index b9cc08d3f..8b52fda31 100644 --- a/test/libsyncengine/propagation/executor/testintegration.cpp +++ b/test/libsyncengine/propagation/executor/testintegration.cpp @@ -80,7 +80,7 @@ void TestIntegration::setUp() { // Create parmsDb bool alreadyExists; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); // Insert user, account, drive & sync int userId(12321); @@ -108,7 +108,7 @@ void TestIntegration::setUp() { Proxy::instance(parameters.proxyConfig()); } - _syncPal = std::make_shared(sync.dbId(), "3.4.0"); + _syncPal = std::make_shared(sync.dbId(), KDRIVE_VERSION_STRING); // Insert items to blacklist SyncNodeCache::instance()->update(_syncPal->syncDbId(), SyncNodeType::BlackList, {test_beaucoupRemoteId}); diff --git a/test/libsyncengine/propagation/operation_sorter/testoperationsorterworker.cpp b/test/libsyncengine/propagation/operation_sorter/testoperationsorterworker.cpp index 517670dc6..74b3fe99a 100644 --- a/test/libsyncengine/propagation/operation_sorter/testoperationsorterworker.cpp +++ b/test/libsyncengine/propagation/operation_sorter/testoperationsorterworker.cpp @@ -18,6 +18,8 @@ #include "testoperationsorterworker.h" +#include "test_utility/testhelpers.h" + #include using namespace CppUnit; @@ -34,11 +36,11 @@ void TestOperationSorterWorker::setUp() { // Create SyncPal bool alreadyExists; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); SyncPath syncDbPath = Db::makeDbName(1, 1, 1, 1, alreadyExists); std::filesystem::remove(syncDbPath); - _syncPal = std::make_shared(syncDbPath, "3.4.0", true); + _syncPal = std::make_shared(syncDbPath, KDRIVE_VERSION_STRING, true); _syncPal->syncDb()->setAutoDelete(true); _syncPal->_operationsSorterWorker = std::make_shared(_syncPal, "Operation Sorter", "OPSO"); diff --git a/test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.cpp b/test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.cpp index c04da0ae2..1e62745b2 100644 --- a/test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.cpp +++ b/test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.cpp @@ -18,21 +18,21 @@ #include "testconflictfinderworker.h" +#include "test_utility/testhelpers.h" + using namespace CppUnit; namespace KDC { -const std::string version = "3.4.0"; - void TestConflictFinderWorker::setUp() { // Create SyncPal bool alreadyExists; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); SyncPath syncDbPath = Db::makeDbName(1, 1, 1, 1, alreadyExists); std::filesystem::remove(syncDbPath); - _syncPal = std::shared_ptr(new SyncPal(syncDbPath, "3.4.0", true)); + _syncPal = std::shared_ptr(new SyncPal(syncDbPath, KDRIVE_VERSION_STRING, true)); _syncPal->syncDb()->setAutoDelete(true); _syncPal->_conflictFinderWorker = diff --git a/test/libsyncengine/reconciliation/conflict_resolver/testconflictresolverworker.cpp b/test/libsyncengine/reconciliation/conflict_resolver/testconflictresolverworker.cpp index e48db9fbb..0237a4970 100644 --- a/test/libsyncengine/reconciliation/conflict_resolver/testconflictresolverworker.cpp +++ b/test/libsyncengine/reconciliation/conflict_resolver/testconflictresolverworker.cpp @@ -29,11 +29,11 @@ void TestConflictResolverWorker::setUp() { // Create SyncPal bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); SyncPath syncDbPath = Db::makeDbName(1, 1, 1, 1, alreadyExists); std::filesystem::remove(syncDbPath); - _syncPal = std::make_shared(syncDbPath, "3.4.0", true); + _syncPal = std::make_shared(syncDbPath, KDRIVE_VERSION_STRING, true); _syncPal->syncDb()->setAutoDelete(true); _syncPal->_conflictResolverWorker = std::make_shared(_syncPal, "Conflict Resolver", "CORE"); diff --git a/test/libsyncengine/reconciliation/operation_generator/testoperationgeneratorworker.cpp b/test/libsyncengine/reconciliation/operation_generator/testoperationgeneratorworker.cpp index 2efd9e019..3a7560d58 100644 --- a/test/libsyncengine/reconciliation/operation_generator/testoperationgeneratorworker.cpp +++ b/test/libsyncengine/reconciliation/operation_generator/testoperationgeneratorworker.cpp @@ -32,11 +32,11 @@ void KDC::TestOperationGeneratorWorker::setUp() { // Create SyncPal bool alreadyExists; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); SyncPath syncDbPath = Db::makeDbName(1, 1, 1, 1, alreadyExists); std::filesystem::remove(syncDbPath); - _syncPal = std::make_shared(syncDbPath, "3.4.0", true); + _syncPal = std::make_shared(syncDbPath, KDRIVE_VERSION_STRING, true); _syncPal->syncDb()->setAutoDelete(true); _syncPal->_operationsGeneratorWorker = std::make_shared(_syncPal, "Operation Generator", "OPGE"); diff --git a/test/libsyncengine/reconciliation/platform_inconsistency_checker/testplatforminconsistencycheckerworker.cpp b/test/libsyncengine/reconciliation/platform_inconsistency_checker/testplatforminconsistencycheckerworker.cpp index f9fa5c600..238e462e6 100644 --- a/test/libsyncengine/reconciliation/platform_inconsistency_checker/testplatforminconsistencycheckerworker.cpp +++ b/test/libsyncengine/reconciliation/platform_inconsistency_checker/testplatforminconsistencycheckerworker.cpp @@ -47,7 +47,7 @@ void TestPlatformInconsistencyCheckerWorker::setUp() { // Create parmsDb bool alreadyExists = false; const auto parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); // Insert user, account, drive & sync const User user(1, 1, "dummy"); @@ -63,7 +63,7 @@ void TestPlatformInconsistencyCheckerWorker::setUp() { ParmsDb::instance()->insertSync(sync); // Create SyncPal - _syncPal = std::make_shared(sync.dbId(), "3.4.0"); + _syncPal = std::make_shared(sync.dbId(), KDRIVE_VERSION_STRING); _syncPal->syncDb()->setAutoDelete(true); _syncPal->_tmpBlacklistManager = std::make_shared(_syncPal); diff --git a/test/libsyncengine/requests/testexclusiontemplatecache.cpp b/test/libsyncengine/requests/testexclusiontemplatecache.cpp index 220f7a6ad..f4867791d 100644 --- a/test/libsyncengine/requests/testexclusiontemplatecache.cpp +++ b/test/libsyncengine/requests/testexclusiontemplatecache.cpp @@ -113,7 +113,7 @@ void TestExclusionTemplateCache::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ExclusionTemplateCache::instance()->update(true, excludedTemplates); } diff --git a/test/libsyncengine/syncpal/testsyncpal.cpp b/test/libsyncengine/syncpal/testsyncpal.cpp index a7244b13b..a0d3ad3a4 100644 --- a/test/libsyncengine/syncpal/testsyncpal.cpp +++ b/test/libsyncengine/syncpal/testsyncpal.cpp @@ -47,7 +47,7 @@ void TestSyncPal::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); // Insert user, account, drive & sync int userId = atoi(testVariables.userId.c_str()); @@ -75,7 +75,7 @@ void TestSyncPal::setUp() { Proxy::instance(parameters.proxyConfig()); } - _syncPal = std::shared_ptr(new SyncPal(sync.dbId(), "3.4.0")); + _syncPal = std::shared_ptr(new SyncPal(sync.dbId(), KDRIVE_VERSION_STRING)); } void TestSyncPal::tearDown() { diff --git a/test/libsyncengine/update_detection/file_system_observer/testcomputefsoperationworker.cpp b/test/libsyncengine/update_detection/file_system_observer/testcomputefsoperationworker.cpp index 53503a717..8f30ca9c2 100644 --- a/test/libsyncengine/update_detection/file_system_observer/testcomputefsoperationworker.cpp +++ b/test/libsyncengine/update_detection/file_system_observer/testcomputefsoperationworker.cpp @@ -56,7 +56,7 @@ void TestComputeFSOperationWorker::setUp() { /// Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); /// Insert user, account, drive & sync int userId = atoi(testVariables.userId.c_str()); @@ -75,7 +75,7 @@ void TestComputeFSOperationWorker::setUp() { Sync sync(1, drive.dbId(), localPathStr, testVariables.remotePath); ParmsDb::instance()->insertSync(sync); - _syncPal = std::make_shared(sync.dbId(), "3.4.0"); + _syncPal = std::make_shared(sync.dbId(), KDRIVE_VERSION_STRING); _syncPal->syncDb()->setAutoDelete(true); /// Insert node "AC" in blacklist diff --git a/test/libsyncengine/update_detection/file_system_observer/testlocalfilesystemobserverworker.cpp b/test/libsyncengine/update_detection/file_system_observer/testlocalfilesystemobserverworker.cpp index b452185b8..846d0fed9 100644 --- a/test/libsyncengine/update_detection/file_system_observer/testlocalfilesystemobserverworker.cpp +++ b/test/libsyncengine/update_detection/file_system_observer/testlocalfilesystemobserverworker.cpp @@ -68,7 +68,7 @@ void TestLocalFileSystemObserverWorker::setUp() { bool alreadyExists = false; const SyncPath parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ParametersCache::instance()->parameters().setExtendedLog(true); ParametersCache::instance()->parameters().setSyncHiddenFiles(true); @@ -79,7 +79,7 @@ void TestLocalFileSystemObserverWorker::setUp() { const SyncPath syncDbPath = Db::makeDbName(1, 1, 1, 1, alreadyExists, true); // Create SyncPal - _syncPal = std::make_shared(syncDbPath, "3.4.0", true); + _syncPal = std::make_shared(syncDbPath, KDRIVE_VERSION_STRING, true); _syncPal->syncDb()->setAutoDelete(true); _syncPal->setLocalPath(_rootFolderPath); diff --git a/test/libsyncengine/update_detection/file_system_observer/testremotefilesystemobserverworker.cpp b/test/libsyncengine/update_detection/file_system_observer/testremotefilesystemobserverworker.cpp index a21263149..bbfed8fbd 100644 --- a/test/libsyncengine/update_detection/file_system_observer/testremotefilesystemobserverworker.cpp +++ b/test/libsyncengine/update_detection/file_system_observer/testremotefilesystemobserverworker.cpp @@ -64,7 +64,7 @@ void TestRemoteFileSystemObserverWorker::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); // Insert user, account, drive & sync const int userId(atoi(testVariables.userId.c_str())); @@ -83,7 +83,7 @@ void TestRemoteFileSystemObserverWorker::setUp() { Sync sync(1, drive.dbId(), "/", "/"); ParmsDb::instance()->insertSync(sync); - _syncPal = std::make_shared(sync.dbId(), "3.4.0"); + _syncPal = std::make_shared(sync.dbId(), KDRIVE_VERSION_STRING); _syncPal->syncDb()->setAutoDelete(true); /// Insert node in blacklist diff --git a/test/libsyncengine/update_detection/update_detector/testupdatetreeworker.cpp b/test/libsyncengine/update_detection/update_detector/testupdatetreeworker.cpp index 4a1735545..9add1a6ed 100644 --- a/test/libsyncengine/update_detection/update_detector/testupdatetreeworker.cpp +++ b/test/libsyncengine/update_detection/update_detector/testupdatetreeworker.cpp @@ -29,7 +29,7 @@ void TestUpdateTreeWorker::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ParametersCache::instance()->parameters().setExtendedLog(true); // Create DB diff --git a/test/server/CMakeLists.txt b/test/server/CMakeLists.txt index 55d8456c0..205aa477d 100644 --- a/test/server/CMakeLists.txt +++ b/test/server/CMakeLists.txt @@ -10,7 +10,8 @@ set(server_srcs_path ${CMAKE_SOURCE_DIR}/src/server) set(testserver_SRCS ../test.cpp - ../test_utility/localtemporarydirectory.cpp + ../test_utility/testhelpers.h ../test_utility/testhelpers.cpp + ../test_utility/localtemporarydirectory.h ../test_utility/localtemporarydirectory.cpp ../../src/server/logarchiver.h ../../src/server/logarchiver.cpp ${CMAKE_SOURCE_DIR}/src/libcommon/updater.h ${CMAKE_SOURCE_DIR}/src/libcommon/updater.cpp ${server_srcs_path}/logarchiver.h ${server_srcs_path}/logarchiver.cpp @@ -22,6 +23,7 @@ set(testserver_SRCS logarchiver/testlogarchiver.h logarchiver/testlogarchiver.cpp updater/testupdater.h updater/testupdater.cpp updater/testabstractupdater.h updater/testabstractupdater.cpp + workers/testworkers.h workers/testworkers.cpp ) if(APPLE) @@ -53,7 +55,7 @@ if (WIN32) include_directories("C:/Program Files (x86)/libzip/include") else () include_directories("/usr/local/include") -endif () +endif() add_executable(${testserver_NAME} ${testserver_SRCS}) @@ -74,7 +76,7 @@ elseif (APPLE) else () target_link_libraries(${testserver_NAME} "/usr/local/lib/liblog4cplusU.so") -endif () +endif() if (WIN32) target_link_libraries(${testserver_NAME} @@ -88,7 +90,7 @@ elseif (APPLE) else () target_link_libraries(${testserver_NAME} "/usr/local/lib/libcppunit.so") -endif () +endif() # Loaded after liblog4cplus to avoid an initialization crash @@ -97,8 +99,17 @@ if (APPLE) "${libsyncengine_NAME}_vfs_mac" "${updater_DEPS}") else() target_link_libraries(${testserver_NAME} - ${libsyncengine_NAME}) -endif () + ${libsyncengine_NAME}) +endif() + +# Install + +# VFS plugin +set(vfs_installdir "${BIN_OUTPUT_DIRECTORY}") +install(TARGETS "${libsyncengine_NAME}_vfs_mac" + LIBRARY DESTINATION "${vfs_installdir}" + RUNTIME DESTINATION "${vfs_installdir}" +) if (APPLE) # Default sync exclude list @@ -111,4 +122,4 @@ if (APPLE) message(STATUS \"Fixing library paths for ${testserver_NAME}...\") execute_process(COMMAND \"install_name_tool\" -change libxxhash.0.dylib @rpath/libxxhash.0.8.2.dylib ${BIN_OUTPUT_DIRECTORY}/${testserver_NAME}) " COMPONENT RUNTIME) -endif () +endif() diff --git a/test/server/test.cpp b/test/server/test.cpp index da0a7e578..53e4f2a54 100644 --- a/test/server/test.cpp +++ b/test/server/test.cpp @@ -24,12 +24,17 @@ #endif #include "logarchiver/testlogarchiver.h" #include "updater/testupdater.h" +#include "workers/testworkers.h" + namespace KDC { + #ifdef __APPLE__ CPPUNIT_TEST_SUITE_REGISTRATION(TestLiteSyncExtConnector); #endif CPPUNIT_TEST_SUITE_REGISTRATION(TestLogArchiver); CPPUNIT_TEST_SUITE_REGISTRATION(TestUpdater); +CPPUNIT_TEST_SUITE_REGISTRATION(TestWorkers); + } // namespace KDC int main(int, char **) { diff --git a/test/server/updater/testabstractupdater.cpp b/test/server/updater/testabstractupdater.cpp index b49052b0c..0a13f5385 100644 --- a/test/server/updater/testabstractupdater.cpp +++ b/test/server/updater/testabstractupdater.cpp @@ -44,7 +44,7 @@ void TestAbstractUpdater::setUp() { // Create parmsDb bool alreadyExists = false; std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); - ParmsDb::instance(parmsDbPath, "3.4.0", true, true); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); ParametersCache::instance()->parameters().setExtendedLog(true); } diff --git a/test/server/updater/testupdater.h b/test/server/updater/testupdater.h index d8736b53d..5d92304e0 100644 --- a/test/server/updater/testupdater.h +++ b/test/server/updater/testupdater.h @@ -18,8 +18,6 @@ #include "testincludes.h" -#include -#include #include "server/updater/kdcupdater.h" #ifdef __APPLE__ #include "server/updater/sparkleupdater.h" diff --git a/test/server/workers/testworkers.cpp b/test/server/workers/testworkers.cpp new file mode 100644 index 000000000..9deb7c0e4 --- /dev/null +++ b/test/server/workers/testworkers.cpp @@ -0,0 +1,301 @@ +/* + * Infomaniak kDrive - Desktop + * Copyright (C) 2023-2024 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "testworkers.h" + +#include "propagation/executor/executorworker.h" +#include "keychainmanager/keychainmanager.h" +#include "network/proxy.h" +#include "io/iohelper.h" +#include "test_utility/testhelpers.h" + +namespace KDC { + +#if defined(__APPLE__) +std::unique_ptr TestWorkers::_vfsPtr = nullptr; +#elif defined(__WIN32) +std::unique_ptr TestWorkers::_vfsPtr = nullptr; +#else +std::unique_ptr TestWorkers::_vfsPtr = nullptr; +#endif + +bool TestWorkers::createPlaceholder(int syncDbId, const SyncPath &relativeLocalPath, const SyncFileItem &item) { + (void) syncDbId; + + if (_vfsPtr && !_vfsPtr->createPlaceholder(relativeLocalPath, item)) { + return false; + } + return true; +} + +bool TestWorkers::convertToPlaceholder(int syncDbId, const SyncPath &relativeLocalPath, const SyncFileItem &item) { + (void) syncDbId; + + if (_vfsPtr && !_vfsPtr->convertToPlaceholder(Path2QStr(relativeLocalPath), item)) { + return false; + } + return true; +} + +bool TestWorkers::setPinState(int syncDbId, const SyncPath &relativeLocalPath, PinState pinState) { + (void) syncDbId; + + if (_vfsPtr && !_vfsPtr->setPinState(Path2QStr(relativeLocalPath), pinState)) { + return false; + } + return true; +} + +void TestWorkers::setUp() { + _logger = Log::instance()->getLogger(); + + const testhelpers::TestVariables testVariables; + + const std::string localPathStr = _localTempDir.path().string(); + + // Insert api token into keystore + std::string keychainKey("123"); + KeyChainManager::instance(true); + KeyChainManager::instance()->writeToken(keychainKey, testVariables.apiToken); + + // Create parmsDb + bool alreadyExists = false; + std::filesystem::path parmsDbPath = Db::makeDbName(alreadyExists, true); + std::filesystem::remove(parmsDbPath); + ParmsDb::instance(parmsDbPath, KDRIVE_VERSION_STRING, true, true); + + // Insert user, account, drive & sync + int userId(12321); + User user(1, userId, keychainKey); + ParmsDb::instance()->insertUser(user); + + int accountId(atoi(testVariables.accountId.c_str())); + Account account(1, accountId, user.dbId()); + ParmsDb::instance()->insertAccount(account); + + int driveDbId = 1; + int driveId = atoi(testVariables.driveId.c_str()); + Drive drive(driveDbId, driveId, account.dbId(), std::string(), 0, std::string()); + ParmsDb::instance()->insertDrive(drive); + + _sync = Sync(1, drive.dbId(), localPathStr, testVariables.remotePath); +#if defined(__APPLE__) + _sync.setVirtualFileMode(VirtualFileMode::Mac); +#elif defined(__WIN32) + _sync.setVirtualFileMode(VirtualFileMode::Win); +#else + _sync.setVirtualFileMode(VirtualFileMode::Off); +#endif + + ParmsDb::instance()->insertSync(_sync); + + // Setup proxy + Parameters parameters; + if (bool found = false; ParmsDb::instance()->selectParameters(parameters, found) && found) { + Proxy::instance(parameters.proxyConfig()); + } + + // Create VFS instance + VfsSetupParams vfsSetupParams; + vfsSetupParams._syncDbId = _sync.dbId(); +#ifdef __WIN32 + vfsSetupParams._driveId = drive.driveId(); + vfsSetupParams._userId = user.userId(); +#endif + vfsSetupParams._localPath = _sync.localPath(); + vfsSetupParams._targetPath = _sync.targetPath(); + vfsSetupParams._logger = _logger; + +#if defined(__APPLE__) + _vfsPtr = std::unique_ptr(new VfsMac(vfsSetupParams)); +#elif defined(__WIN32) + _vfsPtr = std::unique_ptr(new VfsWin(vfsSetupParams)); +#else + _vfsPtr = std::unique_ptr(new VfsOff(vfsSetupParams)); +#endif + + // Setup SyncPal + _syncPal = std::make_shared(_sync.dbId(), KDRIVE_VERSION_STRING); + _syncPal->createWorkers(); + _syncPal->syncDb()->setAutoDelete(true); + _syncPal->createProgressInfo(); + _syncPal->setVfsCreatePlaceholderCallback(createPlaceholder); + _syncPal->setVfsConvertToPlaceholderCallback(convertToPlaceholder); + _syncPal->setVfsSetPinStateCallback(setPinState); +} + +void TestWorkers::tearDown() { + ParmsDb::instance()->close(); + ParmsDb::reset(); + if (_syncPal && _syncPal->syncDb()) { + _syncPal->syncDb()->close(); + } + if (_vfsPtr) { + _vfsPtr = nullptr; + } +} + +void TestWorkers::testCreatePlaceholder() { + _syncPal->resetEstimateUpdates(); + + // Progress not intialized + { + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(SyncPath("dummy"), exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + } + + // Create folder operation + SyncPath relativeFolderPath{"placeholders_folder"}; + { + SyncFileItem syncItem; + syncItem.setPath(relativeFolderPath); + syncItem.setType(NodeType::Directory); + syncItem.setDirection(SyncDirection::Down); + _syncPal->initProgress(syncItem); + + // Folder doesn't exist (normal case) + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); + + // Folder already exists + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + } + + // Create file operation + SyncPath relativeFilePath{relativeFolderPath / "placeholder_file"}; + { + SyncFileItem syncItem; + syncItem.setPath(relativeFilePath); + syncItem.setType(NodeType::File); + syncItem.setDirection(SyncDirection::Down); + _syncPal->initProgress(syncItem); + + // Folder access denied + IoError ioError{IoError::Unknown}; + CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && + ioError == IoError::Success); + + ExitCause exitCause = ExitCause::Unknown; + ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::SystemError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::NoSearchPermission); + + ioError = IoError::Unknown; + CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, true, true, true, ioError) && + ioError == IoError::Success); + + // File doesn't exist (normal case) + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); + + // File already exists + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + } +} + +void TestWorkers::testConvertToPlaceholder() { + _syncPal->resetEstimateUpdates(); + + // Progress not intialized + { + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode = _syncPal->_executorWorker->convertToPlaceholder(SyncPath("dummy"), true, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + } + + // Convert folder operation + SyncPath relativeFolderPath{"placeholders_folder"}; + { + SyncFileItem syncItem; + syncItem.setPath(relativeFolderPath); + syncItem.setType(NodeType::Directory); + syncItem.setDirection(SyncDirection::Down); + _syncPal->initProgress(syncItem); + + // Folder doesn't exist + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFolderPath, true, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + + // Folder already exists (normal case) + std::error_code ec; + CPPUNIT_ASSERT(std::filesystem::create_directory(_syncPal->localPath() / relativeFolderPath, ec) && ec.value() == 0); + + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFolderPath, true, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); + } + + // Convert file operation + SyncPath relativeFilePath{relativeFolderPath / "placeholder_file"}; + { + SyncFileItem syncItem; + syncItem.setPath(relativeFilePath); + syncItem.setType(NodeType::File); + syncItem.setDirection(SyncDirection::Down); + _syncPal->initProgress(syncItem); + + // Folder access denied + IoError ioError{IoError::Unknown}; + CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && + ioError == IoError::Success); + + ExitCause exitCause = ExitCause::Unknown; + ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::SystemError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::NoSearchPermission); + + ioError = IoError::Unknown; + CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, true, true, true, ioError) && + ioError == IoError::Success); + + // File doesn't exist + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFilePath, true, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); + + // File already exists (normal case) + { + std::ofstream ofs(_syncPal->localPath() / relativeFilePath); + ofs << "Some content.\n"; + } + + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFilePath, true, exitCause); + CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); + CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); + } +} + +} // namespace KDC diff --git a/test/server/workers/testworkers.h b/test/server/workers/testworkers.h new file mode 100644 index 000000000..f8b7dc0e5 --- /dev/null +++ b/test/server/workers/testworkers.h @@ -0,0 +1,65 @@ +/* + * Infomaniak kDrive - Desktop + * Copyright (C) 2023-2024 Infomaniak Network SA + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "testincludes.h" + +#if defined(__APPLE__) +#include "server/vfs/mac/vfs_mac.h" +#elif defined(__WIN32) +#include "server/vfs/win/vfs_win.h" +#else +#include "server/vfs/win/vfs.h" +#endif + +#include "propagation/executor/executorworker.h" +#include "test_utility/localtemporarydirectory.h" + +namespace KDC { + +class TestWorkers : public CppUnit::TestFixture { + CPPUNIT_TEST_SUITE(TestWorkers); + CPPUNIT_TEST(testCreatePlaceholder); + CPPUNIT_TEST(testConvertToPlaceholder); + CPPUNIT_TEST_SUITE_END(); + + public: + void setUp(void) final; + void tearDown() override; + void testCreatePlaceholder(); + void testConvertToPlaceholder(); + + protected: + static bool createPlaceholder(int syncDbId, const SyncPath &relativeLocalPath, const SyncFileItem &item); + static bool convertToPlaceholder(int syncDbId, const SyncPath &relativeLocalPath, const SyncFileItem &item); + static bool setPinState(int syncDbId, const SyncPath &relativeLocalPath, PinState pinState); + + log4cplus::Logger _logger; + std::shared_ptr _syncPal; + Sync _sync; + LocalTemporaryDirectory _localTempDir{"TestExecutorWorker"}; + +#if defined(__APPLE__) + static std::unique_ptr _vfsPtr; +#elif defined(__WIN32) + static std::unique_ptr _vfsPtr; +#else + static std::unique_ptr _vfsPtr; +#endif +}; + +} // namespace KDC diff --git a/test/test_utility/testhelpers.h b/test/test_utility/testhelpers.h index 5b597572a..f1cf87e03 100644 --- a/test/test_utility/testhelpers.h +++ b/test/test_utility/testhelpers.h @@ -21,6 +21,7 @@ #include "libcommon/utility/utility.h" #include "utility/types.h" #include "utility/utility.h" +#include "version.h" #include From fea28248700259346a28b191e417e149d318cf0f Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 16 Oct 2024 07:44:53 +0200 Subject: [PATCH 06/12] Fix build --- test/server/CMakeLists.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/server/CMakeLists.txt b/test/server/CMakeLists.txt index 205aa477d..4b35dd826 100644 --- a/test/server/CMakeLists.txt +++ b/test/server/CMakeLists.txt @@ -106,10 +106,12 @@ endif() # VFS plugin set(vfs_installdir "${BIN_OUTPUT_DIRECTORY}") -install(TARGETS "${libsyncengine_NAME}_vfs_mac" - LIBRARY DESTINATION "${vfs_installdir}" - RUNTIME DESTINATION "${vfs_installdir}" -) +if (APPLE) + install(TARGETS "${libsyncengine_NAME}_vfs_mac" + LIBRARY DESTINATION "${vfs_installdir}" + RUNTIME DESTINATION "${vfs_installdir}" + ) +endif() if (APPLE) # Default sync exclude list From 9d0c9269869e52af630dfe3c2ad3bc868ddce526 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 16 Oct 2024 08:47:50 +0200 Subject: [PATCH 07/12] Fix Linux issues --- test/server/CMakeLists.txt | 6 +++-- test/server/workers/testworkers.cpp | 36 +++++++++++++++++++++-------- test/server/workers/testworkers.h | 2 +- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/test/server/CMakeLists.txt b/test/server/CMakeLists.txt index 4b35dd826..dfb3d90a4 100644 --- a/test/server/CMakeLists.txt +++ b/test/server/CMakeLists.txt @@ -12,8 +12,9 @@ set(testserver_SRCS ../test.cpp ../test_utility/testhelpers.h ../test_utility/testhelpers.cpp ../test_utility/localtemporarydirectory.h ../test_utility/localtemporarydirectory.cpp - ../../src/server/logarchiver.h ../../src/server/logarchiver.cpp ${CMAKE_SOURCE_DIR}/src/libcommon/updater.h ${CMAKE_SOURCE_DIR}/src/libcommon/updater.cpp + ${CMAKE_SOURCE_DIR}/src/libcommonserver/plugin.h ${CMAKE_SOURCE_DIR}/src/libcommonserver/plugin.cpp + ${CMAKE_SOURCE_DIR}/src/libcommonserver/vfs.h ${CMAKE_SOURCE_DIR}/src/libcommonserver/vfs.cpp ${server_srcs_path}/logarchiver.h ${server_srcs_path}/logarchiver.cpp ${server_srcs_path}/updater/kdcupdater.h ${server_srcs_path}/updater/kdcupdater.cpp ${server_srcs_path}/updater/updateinfo.h ${server_srcs_path}/updater/updateinfo.cpp @@ -96,7 +97,8 @@ endif() # Loaded after liblog4cplus to avoid an initialization crash if (APPLE) target_link_libraries(${testserver_NAME} - "${libsyncengine_NAME}_vfs_mac" "${updater_DEPS}") + "${libsyncengine_NAME}_vfs_mac" + "${updater_DEPS}") else() target_link_libraries(${testserver_NAME} ${libsyncengine_NAME}) diff --git a/test/server/workers/testworkers.cpp b/test/server/workers/testworkers.cpp index 9deb7c0e4..e18ed7785 100644 --- a/test/server/workers/testworkers.cpp +++ b/test/server/workers/testworkers.cpp @@ -151,12 +151,15 @@ void TestWorkers::tearDown() { } void TestWorkers::testCreatePlaceholder() { + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode{ExitCode::Unknown}; + _syncPal->resetEstimateUpdates(); // Progress not intialized { - ExitCause exitCause{ExitCause::Unknown}; - ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(SyncPath("dummy"), exitCause); + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(SyncPath("dummy"), exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); } @@ -171,16 +174,18 @@ void TestWorkers::testCreatePlaceholder() { _syncPal->initProgress(syncItem); // Folder doesn't exist (normal case) - ExitCause exitCause{ExitCause::Unknown}; - ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); +#if defined(__APPLE__) && defined(__WIN32) // Folder already exists exitCause = ExitCause::Unknown; exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); +#endif } // Create file operation @@ -192,19 +197,21 @@ void TestWorkers::testCreatePlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); +#if defined(__APPLE__) && defined(__WIN32) // Folder access denied IoError ioError{IoError::Unknown}; CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && ioError == IoError::Success); - ExitCause exitCause = ExitCause::Unknown; - ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::SystemError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::NoSearchPermission); ioError = IoError::Unknown; CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, true, true, true, ioError) && ioError == IoError::Success); +#endif // File doesn't exist (normal case) exitCause = ExitCause::Unknown; @@ -212,15 +219,20 @@ void TestWorkers::testCreatePlaceholder() { CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); +#if defined(__APPLE__) && defined(__WIN32) // File already exists exitCause = ExitCause::Unknown; exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); +#endif } } void TestWorkers::testConvertToPlaceholder() { + ExitCause exitCause{ExitCause::Unknown}; + ExitCode exitCode{ExitCode::Unknown}; + _syncPal->resetEstimateUpdates(); // Progress not intialized @@ -240,11 +252,13 @@ void TestWorkers::testConvertToPlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); +#if defined(__APPLE__) && defined(__WIN32) // Folder doesn't exist - ExitCause exitCause{ExitCause::Unknown}; - ExitCode exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFolderPath, true, exitCause); + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFolderPath, true, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); +#endif // Folder already exists (normal case) std::error_code ec; @@ -265,13 +279,14 @@ void TestWorkers::testConvertToPlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); +#if defined(__APPLE__) && defined(__WIN32) // Folder access denied IoError ioError{IoError::Unknown}; CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && ioError == IoError::Success); - ExitCause exitCause = ExitCause::Unknown; - ExitCode exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); + exitCause = ExitCause::Unknown; + exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::SystemError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::NoSearchPermission); @@ -284,6 +299,7 @@ void TestWorkers::testConvertToPlaceholder() { exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFilePath, true, exitCause); CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::DataError); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::InvalidSnapshot); +#endif // File already exists (normal case) { diff --git a/test/server/workers/testworkers.h b/test/server/workers/testworkers.h index f8b7dc0e5..34a43a73f 100644 --- a/test/server/workers/testworkers.h +++ b/test/server/workers/testworkers.h @@ -23,7 +23,7 @@ #elif defined(__WIN32) #include "server/vfs/win/vfs_win.h" #else -#include "server/vfs/win/vfs.h" +#include "libcommonserver/vfs.h" #endif #include "propagation/executor/executorworker.h" From 02120305122d0c5db58955561278e1a038f79093 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 16 Oct 2024 11:54:53 +0200 Subject: [PATCH 08/12] Disable hydration/dehydration workers for testing --- src/server/vfs/mac/vfs_mac.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/server/vfs/mac/vfs_mac.cpp b/src/server/vfs/mac/vfs_mac.cpp index 1161dfbd9..40b705f92 100644 --- a/src/server/vfs/mac/vfs_mac.cpp +++ b/src/server/vfs/mac/vfs_mac.cpp @@ -53,17 +53,21 @@ VfsMac::VfsMac(KDC::VfsSetupParams &vfsSetupParams, QObject *parent) : throw std::runtime_error("Error getting LiteSyncExtConnector instance!"); } - // Start worker threads - for (int i = 0; i < NB_WORKERS; i++) { - for (int j = 0; j < s_nb_threads[i]; j++) { - QThread *workerThread = new QThread(); - _workerInfo[i]._threadList.append(workerThread); - Worker *worker = new Worker(this, i, j, logger()); - worker->moveToThread(workerThread); - connect(workerThread, &QThread::started, worker, &Worker::start); - connect(workerThread, &QThread::finished, worker, &QObject::deleteLater); - connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); - workerThread->start(); + // Start hydration/dehydration workers + // !!! Disabled for testing because no QEventLoop !!! + if (qApp) { + // Start worker threads + for (int i = 0; i < NB_WORKERS; i++) { + for (int j = 0; j < s_nb_threads[i]; j++) { + QThread *workerThread = new QThread(); + _workerInfo[i]._threadList.append(workerThread); + Worker *worker = new Worker(this, i, j, logger()); + worker->moveToThread(workerThread); + connect(workerThread, &QThread::started, worker, &Worker::start); + connect(workerThread, &QThread::finished, worker, &QObject::deleteLater); + connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater); + workerThread->start(); + } } } } From fc1f2d626e46a0c5b465b54d6e51d985a7b8f845 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 16 Oct 2024 13:48:40 +0200 Subject: [PATCH 09/12] Address comments --- src/libsyncengine/propagation/executor/executorworker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 21474a91a..0f84f1eb5 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -367,9 +367,9 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraddError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); _executorExitCode = ExitCode::SystemError; _executorExitCause = ExitCause::NotEnoughDiskSpace; + _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); hasError = true; return; } @@ -955,9 +955,9 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr } } else { if (!enoughLocalSpace(syncOp)) { - _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); _executorExitCode = ExitCode::SystemError; _executorExitCause = ExitCause::NotEnoughDiskSpace; + _syncPal->addError(Error(_syncPal->syncDbId(), name(), _executorExitCode, _executorExitCause)); hasError = true; return; } From 5ef7fed096a0fc919cd065e98cda905756199404 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Wed, 16 Oct 2024 16:29:35 +0200 Subject: [PATCH 10/12] Address comments --- test/server/workers/testworkers.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/server/workers/testworkers.cpp b/test/server/workers/testworkers.cpp index e18ed7785..b341a6715 100644 --- a/test/server/workers/testworkers.cpp +++ b/test/server/workers/testworkers.cpp @@ -179,7 +179,7 @@ void TestWorkers::testCreatePlaceholder() { CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); -#if defined(__APPLE__) && defined(__WIN32) +#if defined(__APPLE__) || defined(__WIN32) // Folder already exists exitCause = ExitCause::Unknown; exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFolderPath, exitCause); @@ -197,7 +197,7 @@ void TestWorkers::testCreatePlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); -#if defined(__APPLE__) && defined(__WIN32) +#if defined(__APPLE__) || defined(__WIN32) // Folder access denied IoError ioError{IoError::Unknown}; CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && @@ -219,7 +219,7 @@ void TestWorkers::testCreatePlaceholder() { CPPUNIT_ASSERT_EQUAL(exitCode, ExitCode::Ok); CPPUNIT_ASSERT_EQUAL(exitCause, ExitCause::Unknown); -#if defined(__APPLE__) && defined(__WIN32) +#if defined(__APPLE__) || defined(__WIN32) // File already exists exitCause = ExitCause::Unknown; exitCode = _syncPal->_executorWorker->createPlaceholder(relativeFilePath, exitCause); @@ -252,7 +252,7 @@ void TestWorkers::testConvertToPlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); -#if defined(__APPLE__) && defined(__WIN32) +#if defined(__APPLE__) || defined(__WIN32) // Folder doesn't exist exitCause = ExitCause::Unknown; exitCode = _syncPal->_executorWorker->convertToPlaceholder(relativeFolderPath, true, exitCause); @@ -279,7 +279,7 @@ void TestWorkers::testConvertToPlaceholder() { syncItem.setDirection(SyncDirection::Down); _syncPal->initProgress(syncItem); -#if defined(__APPLE__) && defined(__WIN32) +#if defined(__APPLE__) || defined(__WIN32) // Folder access denied IoError ioError{IoError::Unknown}; CPPUNIT_ASSERT(IoHelper::setRights(_syncPal->localPath() / relativeFolderPath, false, false, false, ioError) && From ebf3bd7dc7e55b8ce36771f133e01a4d9ec33555 Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Thu, 17 Oct 2024 08:37:10 +0200 Subject: [PATCH 11/12] Address comments --- .../propagation/executor/executorworker.cpp | 111 ++++++++++-------- .../propagation/executor/executorworker.h | 63 ++++++++-- .../snapshot/snapshot.cpp | 9 +- 3 files changed, 115 insertions(+), 68 deletions(-) diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 0f84f1eb5..6863efd9a 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -321,7 +321,6 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptraffectedNode()->lastmodified(), node)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -411,7 +409,6 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptr createDirJob = std::dynamic_pointer_cast(job); if (createDirJob && (createDirJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_BAD_REQUEST || createDirJob->getStatusCode() == Poco::Net::HTTPResponse::HTTP_FORBIDDEN)) { - checkAlreadyExcluded(absoluteLocalFilePath, createDirJob->parentDirId()); + if (!checkAlreadyExcluded(absoluteLocalFilePath, createDirJob->parentDirId())) { + LOG_SYNCPAL_WARN(_logger, "Error in ExecutorWorker::checkAlreadyExcluded"); + hasError = true; + return; + } } if (syncOp->targetSide() == ReplicaSide::Local) { @@ -453,44 +454,72 @@ void ExecutorWorker::handleCreateOp(SyncOpPtr syncOp, std::shared_ptrdriveDbId(), parentId); - job.runSynchronously(); - - try { - Poco::JSON::Object::Ptr resObj = job.jsonRes(); - Poco::JSON::Array::Ptr dataArray = resObj->getArray(dataKey); - for (Poco::JSON::Array::ConstIterator it = dataArray->begin(); it != dataArray->end(); ++it) { - Poco::JSON::Object::Ptr obj = it->extract(); - if (obj) { - std::string name; - if (!JsonParserUtility::extractValue(obj, nameKey, name)) { - return; - } + ExitCode exitCode = job.runSynchronously(); + if (exitCode != ExitCode::Ok) { + LOGW_SYNCPAL_WARN(_logger, "Error in GetFileListJob::runSynchronously for driveDbId=" + << _syncPal->driveDbId() << " nodeId=" << parentId.c_str() << " : " << exitCode); + _executorExitCode = exitCode; + _executorExitCause = ExitCause::Unknown; + return false; + } - if (name == absolutePath.filename().string()) { - alreadyExist = true; - } - } + Poco::JSON::Object::Ptr resObj = job.jsonRes(); + if (!resObj) { + LOG_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + _executorExitCode = ExitCode::BackError; + _executorExitCause = ExitCause::ApiErr; + return false; + } + + Poco::JSON::Array::Ptr dataArray = resObj->getArray(dataKey); + if (!dataArray) { + LOG_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + _executorExitCode = ExitCode::BackError; + _executorExitCause = ExitCause::ApiErr; + return false; + } + + for (Poco::JSON::Array::ConstIterator it = dataArray->begin(); it != dataArray->end(); ++it) { + Poco::JSON::Object::Ptr obj = it->extract(); + std::string name; + if (!JsonParserUtility::extractValue(obj, nameKey, name)) { + LOG_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + _executorExitCode = ExitCode::BackError; + _executorExitCause = ExitCause::ApiErr; + return false; + } + + if (name == absolutePath.filename().string()) { + alreadyExist = true; + break; } - } catch (...) { - LOGW_SYNCPAL_WARN(_logger, - L"Failed to check if file: " << Utility::formatSyncPath(absolutePath).c_str() << L" already exist."); } if (!alreadyExist) { - return; + return true; } - // The item already exist, exclude it - PlatformInconsistencyCheckerUtility::renameLocalFile(absolutePath, - PlatformInconsistencyCheckerUtility::SuffixTypeBlacklisted); + // The item already exists, exclude it + exitCode = PlatformInconsistencyCheckerUtility::renameLocalFile(absolutePath, + PlatformInconsistencyCheckerUtility::SuffixTypeBlacklisted); + if (exitCode != ExitCode::Ok) { + LOGW_SYNCPAL_WARN(_logger, L"Failed to rename file: " << Utility::formatSyncPath(absolutePath).c_str()); + _executorExitCode = exitCode; + _executorExitCause = ExitCause::Unknown; + return false; + } _executorExitCode = ExitCode::DataError; _executorExitCause = ExitCause::FileAlreadyExist; + return false; } // !!! When returning false, _executorExitCode and _executorExitCause must be set !!! @@ -563,7 +592,6 @@ bool ExecutorWorker::generateCreateJob(SyncOpPtr syncOp, std::shared_ptraffectedNode()->name()).c_str()); - // _executorExitCode and _executorExitCause are set by the above function return false; } @@ -926,7 +954,6 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr bool isSyncing = false; if (!checkLiteSyncInfoForEdit(syncOp, absoluteLocalFilePath, ignoreItem, isSyncing)) { LOGW_SYNCPAL_WARN(_logger, L"Error in checkLiteSyncInfoForEdit"); - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -949,7 +976,6 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr syncOp->affectedNode()->lastmodified(), node)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -969,7 +995,6 @@ void ExecutorWorker::handleEditOp(SyncOpPtr syncOp, std::shared_ptr } if (!generateEditJob(syncOp, job)) { - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -1143,7 +1168,6 @@ bool ExecutorWorker::checkLiteSyncInfoForEdit(SyncOpPtr syncOp, const SyncPath & if (isPlaceholder && !isHydrated) { ignoreItem = true; return fixModificationDate(syncOp, absolutePath); - // _executorExitCode and _executorExitCause are set by the above function } } else { if (isPlaceholder) { @@ -1214,7 +1238,6 @@ void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignore if (syncOp->hasConflict()) { bool propagateChange = true; hasError = propagateConflictToDbAndTree(syncOp, propagateChange); - // _executorExitCode and _executorExitCause are set by the above function Error err(_syncPal->syncDbId(), syncOp->conflict().localNode() != nullptr @@ -1240,7 +1263,6 @@ void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignore if (!propagateMoveToDbAndTree(syncOp)) { LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -1252,7 +1274,6 @@ void ExecutorWorker::handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignore } if (!generateMoveJob(syncOp, ignored, bypassProgressComplete)) { - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -1387,7 +1408,6 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp, bool &ignored, bool &bypa // Propagate changes to DB and update trees std::shared_ptr newNode = nullptr; if (!propagateChangeToDbAndTree(syncOp, job, newNode)) { - // _executorExitCode and _executorExitCause are set by the above function return false; } @@ -1412,7 +1432,6 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp, bool &ignored, bool &bypa } return handleFinishedJob(job, syncOp, syncOp->affectedNode()->getPath(), ignored, bypassProgressComplete); - // _executorExitCode and _executorExitCause are set by the above function } // !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! @@ -1434,7 +1453,6 @@ void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &igno ConflictType::EditDelete) { // Error message handled with move operation in case Edit-Delete conflict bool propagateChange = true; hasError = propagateConflictToDbAndTree(syncOp, propagateChange); - // _executorExitCode and _executorExitCause are set by the above function Error err(_syncPal->syncDbId(), syncOp->conflict().localNode() != nullptr @@ -1458,7 +1476,6 @@ void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &igno } if (!propagateDeleteToDbAndTree(syncOp)) { - // _executorExitCode and _executorExitCause are set by the above function LOGW_SYNCPAL_WARN(_logger, L"Failed to propagate changes in DB or update tree for: " << SyncName2WStr(syncOp->affectedNode()->name()).c_str()); hasError = true; @@ -1472,7 +1489,6 @@ void ExecutorWorker::handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &igno } if (!generateDeleteJob(syncOp, ignored, bypassProgressComplete)) { - // _executorExitCode and _executorExitCause are set by the above function hasError = true; return; } @@ -1534,7 +1550,6 @@ bool ExecutorWorker::generateDeleteJob(SyncOpPtr syncOp, bool &ignored, bool &by job->setAffectedFilePath(relativeLocalFilePath); job->runSynchronously(); return handleFinishedJob(job, syncOp, relativeLocalFilePath, ignored, bypassProgressComplete); - // _executorExitCode and _executorExitCause are set by the above function } bool ExecutorWorker::hasRight(SyncOpPtr syncOp, bool &exists) { @@ -1688,6 +1703,8 @@ bool ExecutorWorker::enoughLocalSpace(SyncOpPtr syncOp) { } void ExecutorWorker::waitForAllJobsToFinish(bool &hasError) { + hasError = false; + while (!_ongoingJobs.empty()) { if (stopAsked()) { cancelAllOngoingJobs(); @@ -1719,6 +1736,7 @@ void ExecutorWorker::waitForAllJobsToFinish(bool &hasError) { bool ExecutorWorker::deleteFinishedAsyncJobs() { bool hasError = false; + while (!_terminatedJobs.empty()) { // Delete all terminated jobs if (!hasError && _ongoingJobs.find(_terminatedJobs.front()) != _ongoingJobs.end()) { @@ -1875,7 +1893,6 @@ bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpP job->exitCode() == ExitCode::BackError && details::isManagedBackError(job->exitCause())) { return handleManagedBackError(job->exitCause(), syncOp, isInconsistencyIssue, networkJob && networkJob->isDownloadImpossible()); - // _executorExitCode and _executorExitCause are set by the above function } if (job->exitCode() != ExitCode::Ok) { @@ -1923,7 +1940,6 @@ bool ExecutorWorker::handleFinishedJob(std::shared_ptr job, SyncOpP // Propagate changes to DB and update trees std::shared_ptr newNode; if (!propagateChangeToDbAndTree(syncOp, job, newNode)) { - // _executorExitCode and _executorExitCause are set by the above function cancelAllOngoingJobs(); return false; } @@ -2062,7 +2078,6 @@ bool ExecutorWorker::propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propag if (localNodeFoundInDb) { // Remove local node from DB if (!deleteFromDb(syncOp->conflict().localNode())) { - // _executorExitCode and _executorExitCause are set by the above function propagateChange = false; return true; } @@ -2124,7 +2139,6 @@ bool ExecutorWorker::propagateChangeToDbAndTree(SyncOpPtr syncOp, std::shared_pt if (syncOp->hasConflict()) { bool propagateChange = true; bool hasError = propagateConflictToDbAndTree(syncOp, propagateChange); - // _executorExitCode and _executorExitCause are set by the above function if (!propagateChange || hasError) { return !hasError; } @@ -2165,19 +2179,15 @@ bool ExecutorWorker::propagateChangeToDbAndTree(SyncOpPtr syncOp, std::shared_pt if (syncOp->type() == OperationType::Create) { return propagateCreateToDbAndTree(syncOp, nodeId, modtime, node); - // _executorExitCode and _executorExitCause are set by the above function } else { return propagateEditToDbAndTree(syncOp, nodeId, modtime, node); - // _executorExitCode and _executorExitCause are set by the above function } } case OperationType::Move: { return propagateMoveToDbAndTree(syncOp); - // _executorExitCode and _executorExitCause are set by the above function } case OperationType::Delete: { return propagateDeleteToDbAndTree(syncOp); - // _executorExitCode and _executorExitCause are set by the above function } default: { LOGW_SYNCPAL_WARN(_logger, L"Unknown operation type " << syncOp->type() << L" on file " @@ -2564,7 +2574,6 @@ bool ExecutorWorker::propagateDeleteToDbAndTree(SyncOpPtr syncOp) { // 2. Remove the entry from the database. If nX is a directory node, also remove all entries for each node n ∈ S. This // avoids that the object(s) are detected again by compute_ops() on the next sync iteration if (!deleteFromDb(syncOp->affectedNode())) { - // _executorExitCode and _executorExitCause are set by the above function return false; } diff --git a/src/libsyncengine/propagation/executor/executorworker.h b/src/libsyncengine/propagation/executor/executorworker.h index 26db3131b..aa9e4352d 100644 --- a/src/libsyncengine/propagation/executor/executorworker.h +++ b/src/libsyncengine/propagation/executor/executorworker.h @@ -63,20 +63,26 @@ class ExecutorWorker : public OperationProcessor { void executorCallback(UniqueId jobId); protected: + /// @note _executorExitCode and _executorExitCause must be set when the function returns void execute() override; private: - void initProgressManager(); - bool initSyncFileItem(SyncOpPtr syncOp, SyncFileItem &syncItem); - + /// @note _executorExitCode and _executorExitCause must be set when the function returns with hasError == true void handleCreateOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored); - void checkAlreadyExcluded(const SyncPath &absolutePath, const NodeId &parentId); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false + bool checkAlreadyExcluded(const SyncPath &absolutePath, const NodeId &parentId); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool generateCreateJob(SyncOpPtr syncOp, std::shared_ptr &job) noexcept; + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool checkLiteSyncInfoForCreate(SyncOpPtr syncOp, const SyncPath &path, bool &isDehydratedPlaceholder); - ExitCode createPlaceholder(const SyncPath &relativeLocalPath, ExitCause &exitCause); - ExitCode convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, ExitCause &exitCause); + /// @note _executorExitCode and _executorExitCause must be set when the function returns with hasError == true void handleEditOp(SyncOpPtr syncOp, std::shared_ptr &job, bool &hasError, bool &ignored); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool generateEditJob(SyncOpPtr syncOp, std::shared_ptr &job); /** @@ -85,43 +91,74 @@ class ExecutorWorker : public OperationProcessor { * @param syncOp : the operation to propagate. * @param absolutePath : absolute local path of the affected file. * @return `true` if the date is modified successfully. + * @note _executorExitCode and _executorExitCause must be set when the function returns false */ bool fixModificationDate(SyncOpPtr syncOp, const SyncPath &absolutePath); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool checkLiteSyncInfoForEdit(SyncOpPtr syncOp, const SyncPath &absolutePath, bool &ignoreItem, bool &isSyncing); // TODO : is called "check..." but perform some actions. Wording not // good, function probably does too much + /// @note _executorExitCode and _executorExitCause must be set when the function returns with hasError == true void handleMoveOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool generateMoveJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete); + /// @note _executorExitCode and _executorExitCause must be set when the function returns with hasError == true void handleDeleteOp(SyncOpPtr syncOp, bool &hasError, bool &ignored, bool &bypassProgressComplete); - bool generateDeleteJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete); - bool hasRight(SyncOpPtr syncOp, bool &exists); - bool enoughLocalSpace(SyncOpPtr syncOp); + /// @note _executorExitCode and _executorExitCause must be set when the function returns false + bool generateDeleteJob(SyncOpPtr syncOp, bool &ignored, bool &bypassProgressComplete); + /// @note _executorExitCode and _executorExitCause must be set when the function returns with hasError == true void waitForAllJobsToFinish(bool &hasError); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool deleteFinishedAsyncJobs(); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool handleManagedBackError(ExitCause jobExitCause, SyncOpPtr syncOp, bool isInconsistencyIssue, bool downloadImpossible); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool handleFinishedJob(std::shared_ptr job, SyncOpPtr syncOp, const SyncPath &relativeLocalPath, bool &ignored, bool &bypassProgressComplete); - void handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath, bool &ignored); - void sendProgress(); + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateConflictToDbAndTree(SyncOpPtr syncOp, bool &propagateChange); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateChangeToDbAndTree(SyncOpPtr syncOp, std::shared_ptr job, std::shared_ptr &node); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateCreateToDbAndTree(SyncOpPtr syncOp, const NodeId &newNodeId, std::optional newLastModTime, std::shared_ptr &node); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateEditToDbAndTree(SyncOpPtr syncOp, const NodeId &newNodeId, std::optional newLastModTime, std::shared_ptr &node); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateMoveToDbAndTree(SyncOpPtr syncOp); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool propagateDeleteToDbAndTree(SyncOpPtr syncOp); + + /// @note _executorExitCode and _executorExitCause must be set when the function returns false bool deleteFromDb(std::shared_ptr node); - bool runCreateDirJob(SyncOpPtr syncOp, std::shared_ptr job); + ExitCode createPlaceholder(const SyncPath &relativeLocalPath, ExitCause &exitCause); + ExitCode convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, ExitCause &exitCause); + void initProgressManager(); + bool initSyncFileItem(SyncOpPtr syncOp, SyncFileItem &syncItem); + void handleForbiddenAction(SyncOpPtr syncOp, const SyncPath &relativeLocalPath, bool &ignored); + void sendProgress(); + bool hasRight(SyncOpPtr syncOp, bool &exists); + bool enoughLocalSpace(SyncOpPtr syncOp); + bool runCreateDirJob(SyncOpPtr syncOp, std::shared_ptr job); void cancelAllOngoingJobs(bool reschedule = false); - void manageJobDependencies(SyncOpPtr syncOp, std::shared_ptr job); inline bool isLiteSyncActivated() { return _syncPal->vfsMode() != VirtualFileMode::Off; } diff --git a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp index 4c6d08264..23509e9e2 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp @@ -497,10 +497,11 @@ bool Snapshot::checkIntegrityRecursively(const NodeId &parentId) { auto result = names.insert(_items[*childId].name()); if (!result.second) { - LOG_ERROR(Log::instance()->getLogger(), - "Snapshot integrity check failed, the folder named: \"" - << SyncName2Str(parentItem.name()).c_str() << "\"(" << parentItem.id().c_str() << ") contains: \"" - << SyncName2Str(_items[*childId].name()).c_str() << "\" twice with two differents NodeId"); + LOGW_WARN(Log::instance()->getLogger(), L"Snapshot integrity check failed, the folder named: \"" + << SyncName2WStr(parentItem.name()).c_str() << L"\"(" + << Utility::s2ws(parentItem.id()).c_str() << L") contains: \"" + << SyncName2WStr(_items[*childId].name()).c_str() + << L"\" twice with two differents NodeId"); return false; } } From 4596e22c750cbaf0fc39dd2da2f627a5db78a58b Mon Sep 17 00:00:00 2001 From: Christophe Larchier Date: Thu, 17 Oct 2024 09:07:59 +0200 Subject: [PATCH 12/12] Address comments --- .../propagation/executor/executorworker.cpp | 127 ++++++++---------- .../propagation/executor/executorworker.h | 1 + 2 files changed, 58 insertions(+), 70 deletions(-) diff --git a/src/libsyncengine/propagation/executor/executorworker.cpp b/src/libsyncengine/propagation/executor/executorworker.cpp index 6863efd9a..561d695a9 100644 --- a/src/libsyncengine/propagation/executor/executorworker.cpp +++ b/src/libsyncengine/propagation/executor/executorworker.cpp @@ -461,8 +461,8 @@ bool ExecutorWorker::checkAlreadyExcluded(const SyncPath &absolutePath, const No GetFileListJob job(_syncPal->driveDbId(), parentId); ExitCode exitCode = job.runSynchronously(); if (exitCode != ExitCode::Ok) { - LOGW_SYNCPAL_WARN(_logger, "Error in GetFileListJob::runSynchronously for driveDbId=" - << _syncPal->driveDbId() << " nodeId=" << parentId.c_str() << " : " << exitCode); + LOG_SYNCPAL_WARN(_logger, "Error in GetFileListJob::runSynchronously for driveDbId=" + << _syncPal->driveDbId() << " nodeId=" << parentId.c_str() << " : " << exitCode); _executorExitCode = exitCode; _executorExitCause = ExitCause::Unknown; return false; @@ -470,8 +470,8 @@ bool ExecutorWorker::checkAlreadyExcluded(const SyncPath &absolutePath, const No Poco::JSON::Object::Ptr resObj = job.jsonRes(); if (!resObj) { - LOG_WARN(Log::instance()->getLogger(), - "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + LOG_SYNCPAL_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); _executorExitCode = ExitCode::BackError; _executorExitCause = ExitCause::ApiErr; return false; @@ -479,8 +479,8 @@ bool ExecutorWorker::checkAlreadyExcluded(const SyncPath &absolutePath, const No Poco::JSON::Array::Ptr dataArray = resObj->getArray(dataKey); if (!dataArray) { - LOG_WARN(Log::instance()->getLogger(), - "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + LOG_SYNCPAL_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); _executorExitCode = ExitCode::BackError; _executorExitCause = ExitCause::ApiErr; return false; @@ -490,8 +490,8 @@ bool ExecutorWorker::checkAlreadyExcluded(const SyncPath &absolutePath, const No Poco::JSON::Object::Ptr obj = it->extract(); std::string name; if (!JsonParserUtility::extractValue(obj, nameKey, name)) { - LOG_WARN(Log::instance()->getLogger(), - "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); + LOG_SYNCPAL_WARN(Log::instance()->getLogger(), + "GetFileListJob failed for driveDbId=" << _syncPal->driveDbId() << " nodeId=" << parentId.c_str()); _executorExitCode = ExitCode::BackError; _executorExitCause = ExitCause::ApiErr; return false; @@ -804,49 +804,7 @@ ExitCode ExecutorWorker::createPlaceholder(const SyncPath &relativeLocalPath, Ex if (!_syncPal->vfsCreatePlaceholder(relativeLocalPath, syncItem)) { // TODO: vfs functions should output an ioError parameter // Check if the item already exists on local replica - SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; - bool exists = false; - IoError ioError = IoError::Success; - if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { - LOGW_SYNCPAL_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " - << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); - return ExitCode::SystemError; - } - - if (ioError == IoError::AccessDenied) { - LOGW_SYNCPAL_WARN(_logger, - L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - exitCause = ExitCause::NoSearchPermission; - return ExitCode::SystemError; - } - - if (exists) { - exitCause = ExitCause::InvalidSnapshot; - return ExitCode::DataError; - } else { - // Check if the parent folder exists on local replica - bool parentExists = false; - if (!IoHelper::checkIfPathExists(absoluteLocalFilePath.parent_path(), parentExists, ioError)) { - LOGW_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " - << Utility::formatIoError(absoluteLocalFilePath.parent_path(), ioError).c_str()); - return ExitCode::SystemError; - } - - if (ioError == IoError::AccessDenied) { - LOGW_WARN(_logger, L"Item misses search permission: " - << Utility::formatSyncPath(absoluteLocalFilePath.parent_path()).c_str()); - exitCause = ExitCause::NoSearchPermission; - return ExitCode::SystemError; - } - - if (!parentExists) { - exitCause = ExitCause::InvalidSnapshot; - return ExitCode::DataError; - } - } - - exitCause = ExitCause::FileAccessError; - return ExitCode::SystemError; + return processCreateOrConvertToPlaceholderError(relativeLocalPath, true, exitCause); } return ExitCode::Ok; @@ -894,38 +852,67 @@ ExitCode ExecutorWorker::convertToPlaceholder(const SyncPath &relativeLocalPath, if (!_syncPal->vfsConvertToPlaceholder(absoluteLocalFilePath, syncItem)) { // TODO: vfs functions should output an ioError parameter // Check that the item exists on local replica - SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; - bool exists = false; - IoError ioError = IoError::Success; - if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { - LOGW_SYNCPAL_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " - << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); + return processCreateOrConvertToPlaceholderError(relativeLocalPath, false, exitCause); + } + + if (!_syncPal->vfsSetPinState(absoluteLocalFilePath, hydrated ? PinState::AlwaysLocal : PinState::OnlineOnly)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in vfsSetPinState: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::FileAccessError; + return ExitCode::SystemError; + } + + return ExitCode::Ok; +} + +ExitCode ExecutorWorker::processCreateOrConvertToPlaceholderError(const SyncPath &relativeLocalPath, bool create, + ExitCause &exitCause) { + // TODO: Simplify/remove this function when vfs functions will output an ioError parameter + exitCause = ExitCause::Unknown; + + SyncPath absoluteLocalFilePath = _syncPal->localPath() / relativeLocalPath; + bool exists = false; + IoError ioError = IoError::Success; + if (!IoHelper::checkIfPathExists(absoluteLocalFilePath, exists, ioError)) { + LOGW_SYNCPAL_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " + << Utility::formatIoError(absoluteLocalFilePath, ioError).c_str()); + return ExitCode::SystemError; + } + + if (ioError == IoError::AccessDenied) { + LOGW_SYNCPAL_WARN(_logger, L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + exitCause = ExitCause::NoSearchPermission; + return ExitCode::SystemError; + } + + if ((create && exists) || (!create && !exists)) { + exitCause = ExitCause::InvalidSnapshot; + return ExitCode::DataError; + } + + if (create) { + // Check if the parent folder exists on local replica + bool parentExists = false; + if (!IoHelper::checkIfPathExists(absoluteLocalFilePath.parent_path(), parentExists, ioError)) { + LOGW_WARN(_logger, L"Error in IoHelper::checkIfPathExists: " + << Utility::formatIoError(absoluteLocalFilePath.parent_path(), ioError).c_str()); return ExitCode::SystemError; } if (ioError == IoError::AccessDenied) { - LOGW_SYNCPAL_WARN(_logger, - L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); + LOGW_WARN(_logger, + L"Item misses search permission: " << Utility::formatSyncPath(absoluteLocalFilePath.parent_path()).c_str()); exitCause = ExitCause::NoSearchPermission; return ExitCode::SystemError; } - if (!exists) { + if (!parentExists) { exitCause = ExitCause::InvalidSnapshot; return ExitCode::DataError; } - - exitCause = ExitCause::FileAccessError; - return ExitCode::SystemError; - } - - if (!_syncPal->vfsSetPinState(absoluteLocalFilePath, hydrated ? PinState::AlwaysLocal : PinState::OnlineOnly)) { - LOGW_SYNCPAL_WARN(_logger, L"Error in vfsSetPinState: " << Utility::formatSyncPath(absoluteLocalFilePath).c_str()); - exitCause = ExitCause::FileAccessError; - return ExitCode::SystemError; } - return ExitCode::Ok; + exitCause = ExitCause::FileAccessError; + return ExitCode::SystemError; } // !!! When returning with hasError == true, _executorExitCode and _executorExitCause must be set !!! diff --git a/src/libsyncengine/propagation/executor/executorworker.h b/src/libsyncengine/propagation/executor/executorworker.h index aa9e4352d..e68aeca31 100644 --- a/src/libsyncengine/propagation/executor/executorworker.h +++ b/src/libsyncengine/propagation/executor/executorworker.h @@ -150,6 +150,7 @@ class ExecutorWorker : public OperationProcessor { ExitCode createPlaceholder(const SyncPath &relativeLocalPath, ExitCause &exitCause); ExitCode convertToPlaceholder(const SyncPath &relativeLocalPath, bool hydrated, ExitCause &exitCause); + ExitCode processCreateOrConvertToPlaceholderError(const SyncPath &relativeLocalPath, bool create, ExitCause &exitCause); void initProgressManager(); bool initSyncFileItem(SyncOpPtr syncOp, SyncFileItem &syncItem);