From 07b7597b9d6dec67df0eb78097a53ef6e9de150c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Fri, 10 May 2024 13:36:48 +0200 Subject: [PATCH 01/12] KDESKTOP-391 - Ignore file_delete actions --- src/libsyncengine/jobs/network/networkjobsparams.h | 4 ++-- .../file_system_observer/remotefilesystemobserverworker.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsyncengine/jobs/network/networkjobsparams.h b/src/libsyncengine/jobs/network/networkjobsparams.h index 94f711236..50eeee005 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.h +++ b/src/libsyncengine/jobs/network/networkjobsparams.h @@ -122,8 +122,8 @@ static const std::string createAction = "file_create"; static const std::string renameAction = "file_rename"; static const std::string editAction = "file_update"; static const std::string accessAction = "file_access"; -static const std::string trashAction = "file_trash"; -static const std::string deleteAction = "file_delete"; +static const std::string trashAction = "file_trash"; // The file has been put into the trash +static const std::string deleteAction = "file_delete"; // The file has been completely deleted from the trash static const std::string moveInAction = "file_move"; static const std::string moveOutAction = "file_move_out"; static const std::string restoreAction = "file_restore"; diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 0a03b3047..af35f0156 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -626,7 +626,7 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L") edited at:" << modtime); } - } else if (action == trashAction || action == moveOutAction || action == deleteAction || rightsRemoved) { + } else if (action == trashAction || action == moveOutAction || rightsRemoved) { if (action == moveOutAction && movedItems.find(id) != movedItems.end()) { // Ignore move out action continue; @@ -645,7 +645,7 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a return ExitCodeBackError; } } else if (action == accessAction || action == restoreFileShareCreate || action == restoreFileShareDelete || - action == restoreShareLinkCreate || action == restoreShareLinkDelete) { + action == restoreShareLinkCreate || action == restoreShareLinkDelete || action == deleteAction) { // Ignore these actions } else { LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(name).c_str() << L" (" From 9197deba0e207002420f8eb012eda74078a84f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Fri, 10 May 2024 15:30:09 +0200 Subject: [PATCH 02/12] KDESKTOP-823 - Refactor remote FSO with switch statement --- .../jobs/network/networkjobsparams.cpp | 38 ++- .../jobs/network/networkjobsparams.h | 57 ++-- .../remotefilesystemobserverworker.cpp | 311 +++++++++--------- .../remotefilesystemobserverworker.h | 5 +- 4 files changed, 228 insertions(+), 183 deletions(-) diff --git a/src/libsyncengine/jobs/network/networkjobsparams.cpp b/src/libsyncengine/jobs/network/networkjobsparams.cpp index 384d13bb9..e5a62f833 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.cpp +++ b/src/libsyncengine/jobs/network/networkjobsparams.cpp @@ -21,6 +21,41 @@ #include namespace KDC { + +ActionCode getActionCode(const std::string &action) noexcept { + static const std::unordered_map actionMap = { + {"file_create", ActionCode::actionCodeCreate}, + {"file_rename", ActionCode::actionCodeRename}, + {"file_update", ActionCode::actionCodeEdit}, + {"file_access", ActionCode::actionCodeAccess}, + {"file_trash", ActionCode::actionCodeTrash}, + {"file_delete", ActionCode::actionCodeDelete}, + {"file_move", ActionCode::actionCodeMoveIn}, + {"file_move_out", ActionCode::actionCodeMoveOut}, + {"file_restore", ActionCode::actionCodeRestore}, + {"file_share_create", ActionCode::actionCodeRestoreFileShareCreate}, + {"file_share_delete", ActionCode::actionCodeRestoreFileShareDelete}, + {"share_link_create", ActionCode::actionCodeRestoreShareLinkCreate}, + {"share_link_delete", ActionCode::actionCodeRestoreShareLinkDelete}, + {"acl_insert", ActionCode::actionCodeAccessRightInsert}, + {"acl_update", ActionCode::actionCodeAccessRightUpdate}, + {"acl_remove", ActionCode::actionCodeAccessRightRemove}, + {"acl_user_insert", ActionCode::actionCodeAccessRightUserInsert}, + {"acl_user_update", ActionCode::actionCodeAccessRightUserUpdate}, + {"acl_user_remove", ActionCode::actionCodeAccessRightUserRemove}, + {"acl_team_insert", ActionCode::actionCodeAccessRightTeamInsert}, + {"acl_team_update", ActionCode::actionCodeAccessRightTeamUpdate}, + {"acl_team_remove", ActionCode::actionCodeAccessRightTeamRemove}, + {"acl_main_users_insert", ActionCode::actionCodeAccessRightMainUsersInsert}, + {"acl_main_users_update", ActionCode::actionCodeAccessRightMainUsersUpdate}, + {"acl_main_users_remove", ActionCode::actionCodeAccessRightMainUsersRemove} + }; + + if (const auto it = actionMap.find(action); it != actionMap.cend()) return it->second; + + return ActionCode::actionCodeUnknown; +}; + NetworkErrorCode getNetworkErrorCode(const std::string &errorCode) noexcept { static const std::map> errorCodeMap = { {"forbidden_error", NetworkErrorCode::forbiddenError}, @@ -50,5 +85,6 @@ NetworkErrorReason getNetworkErrorReason(const std::string &errorReason) noexcep if (const auto it = errorReasonMap.find(errorReason); it != errorReasonMap.cend()) return it->second; return NetworkErrorReason::unknownReason; -}; +} + } // namespace KDC diff --git a/src/libsyncengine/jobs/network/networkjobsparams.h b/src/libsyncengine/jobs/network/networkjobsparams.h index 50eeee005..42c1e276c 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.h +++ b/src/libsyncengine/jobs/network/networkjobsparams.h @@ -118,33 +118,34 @@ static const std::string urlKey = "url"; static const std::string symbolicLinkKey = "symbolic_link"; /// Action type -static const std::string createAction = "file_create"; -static const std::string renameAction = "file_rename"; -static const std::string editAction = "file_update"; -static const std::string accessAction = "file_access"; -static const std::string trashAction = "file_trash"; // The file has been put into the trash -static const std::string deleteAction = "file_delete"; // The file has been completely deleted from the trash -static const std::string moveInAction = "file_move"; -static const std::string moveOutAction = "file_move_out"; -static const std::string restoreAction = "file_restore"; -static const std::string restoreFileShareCreate = "file_share_create"; -static const std::string restoreFileShareDelete = "file_share_delete"; -static const std::string restoreShareLinkCreate = "share_link_create"; -static const std::string restoreShareLinkDelete = "share_link_delete"; -//// Rights -/// TODO : implement all rights -static const std::string accessRightInsert = "acl_insert"; -static const std::string accessRightUpdate = "acl_update"; -static const std::string accessRightRemove = "acl_remove"; -static const std::string accessRightUserInsert = "acl_user_insert"; -static const std::string accessRightUserUpdate = "acl_user_update"; -static const std::string accessRightUserRemove = "acl_user_remove"; -static const std::string accessRightTeamInsert = "acl_team_insert"; -static const std::string accessRightTeamUpdate = "acl_team_update"; -static const std::string accessRightTeamRemove = "acl_team_remove"; -static const std::string accessRightMainUsersInsert = "acl_main_users_insert"; -static const std::string accessRightMainUsersUpdate = "acl_main_users_update"; -static const std::string accessRightMainUsersRemove = "acl_main_users_remove"; +enum class ActionCode { + actionCodeCreate, + actionCodeRename, + actionCodeEdit, + actionCodeAccess, + actionCodeTrash, // The file has been put into the trash + actionCodeDelete, // The file has been completely deleted from the trash + actionCodeMoveIn, + actionCodeMoveOut, + actionCodeRestore, + actionCodeRestoreFileShareCreate, + actionCodeRestoreFileShareDelete, + actionCodeRestoreShareLinkCreate, + actionCodeRestoreShareLinkDelete, + actionCodeAccessRightInsert, + actionCodeAccessRightUpdate, + actionCodeAccessRightRemove, + actionCodeAccessRightUserInsert, + actionCodeAccessRightUserUpdate, + actionCodeAccessRightUserRemove, + actionCodeAccessRightTeamInsert, + actionCodeAccessRightTeamUpdate, + actionCodeAccessRightTeamRemove, + actionCodeAccessRightMainUsersInsert, + actionCodeAccessRightMainUsersUpdate, + actionCodeAccessRightMainUsersRemove, + actionCodeUnknown +}; /// Visibility static const std::string isRootKey = "is_root"; @@ -160,7 +161,6 @@ static const std::string codeKey = "code"; static const std::string descriptionKey = "description"; static const std::string contextKey = "context"; /// Error codes - enum class NetworkErrorCode { forbiddenError, notAuthorized, @@ -186,6 +186,7 @@ enum class NetworkErrorReason { unknownReason // None of the handled reasons }; +ActionCode getActionCode(const std::string &action) noexcept; NetworkErrorCode getNetworkErrorCode(const std::string &errorCode) noexcept; NetworkErrorReason getNetworkErrorReason(const std::string &errorCode) noexcept; diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index af35f0156..97d7dcad4 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -22,7 +22,6 @@ #include "jobs/network/getfileinfojob.h" #include "jobs/network/longpolljob.h" #include "jobs/network/continuefilelistwithcursorjob.h" -#include "jobs/network/networkjobsparams.h" #ifdef _WIN32 #include "reconciliation/platform_inconsistency_checker/platforminconsistencycheckerutility.h" #endif @@ -141,7 +140,7 @@ ExitCode RemoteFileSystemObserverWorker::processEvents() { } // Get last listing cursor used - int64_t timestamp; + int64_t timestamp = 0; ExitCode exitCode = _syncPal->listingCursor(_cursor, timestamp); if (exitCode != ExitCodeOk) { LOG_SYNCPAL_WARN(_logger, "Error in SyncPal::listingCursor"); @@ -474,135 +473,134 @@ ExitCode RemoteFileSystemObserverWorker::sendLongPoll(bool &changes) { } ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr actionArray) { - if (actionArray) { - std::unordered_set movedItems; + if (!actionArray) return ExitCodeOk; - for (auto it = actionArray->begin(); it != actionArray->end(); ++it) { - if (stopAsked()) { - return ExitCodeOk; - } + std::unordered_set movedItems; - // Check new actions - std::string action; - Poco::JSON::Object::Ptr actionObj = it->extract(); - if (!JsonParserUtility::extractValue(actionObj, actionKey, action)) { - return ExitCodeBackError; - } + for (auto it = actionArray->begin(); it != actionArray->end(); ++it) { + if (stopAsked()) { + return ExitCodeOk; + } - int idInt = 0; - if (!JsonParserUtility::extractValue(actionObj, fileIdKey, idInt)) { - return ExitCodeBackError; - } - NodeId id = std::to_string(idInt); + // Check new actions + std::string actionStr; + Poco::JSON::Object::Ptr actionObj = it->extract(); + if (!JsonParserUtility::extractValue(actionObj, actionKey, actionStr)) { + return ExitCodeBackError; + } + ActionCode actionCode = getActionCode(actionStr); - int parentIdInt = 0; - if (!JsonParserUtility::extractValue(actionObj, parentIdKey, parentIdInt)) { - return ExitCodeBackError; - } - NodeId parentId = std::to_string(parentIdInt); + int idInt = 0; + if (!JsonParserUtility::extractValue(actionObj, fileIdKey, idInt)) { + return ExitCodeBackError; + } + NodeId id = std::to_string(idInt); - SyncName path; - if (!JsonParserUtility::extractValue(actionObj, pathKey, path)) { - return ExitCodeBackError; - } - SyncName name = path.substr(path.find_last_of('/') + 1); // +1 to ignore the last "/" + int parentIdInt = 0; + if (!JsonParserUtility::extractValue(actionObj, parentIdKey, parentIdInt)) { + return ExitCodeBackError; + } + NodeId parentId = std::to_string(parentIdInt); - SyncName destPath; - if (!JsonParserUtility::extractValue(actionObj, destinationKey, destPath, false)) { - return ExitCodeBackError; - } - SyncName destName = destPath.substr(destPath.find_last_of('/') + 1); // +1 to ignore the last "/" + SyncName path; + if (!JsonParserUtility::extractValue(actionObj, pathKey, path)) { + return ExitCodeBackError; + } + SyncName name = path.substr(path.find_last_of('/') + 1); // +1 to ignore the last "/" - SyncTime createdAt = 0; - if (!JsonParserUtility::extractValue(actionObj, createdAtKey, createdAt, false)) { - return ExitCodeBackError; - } + SyncName destPath; + if (!JsonParserUtility::extractValue(actionObj, destinationKey, destPath, false)) { + return ExitCodeBackError; + } + SyncName destName = destPath.substr(destPath.find_last_of('/') + 1); // +1 to ignore the last "/" - SyncTime modtime = 0; - if (!JsonParserUtility::extractValue(actionObj, lastModifiedAtKey, modtime, false)) { - return ExitCodeBackError; - } + SyncTime createdAt = 0; + if (!JsonParserUtility::extractValue(actionObj, createdAtKey, createdAt, false)) { + return ExitCodeBackError; + } - std::string tmp; - if (!JsonParserUtility::extractValue(actionObj, fileTypeKey, tmp)) { - return ExitCodeBackError; - } - NodeType type = tmp == fileKey ? NodeTypeFile : NodeTypeDirectory; + SyncTime modtime = 0; + if (!JsonParserUtility::extractValue(actionObj, lastModifiedAtKey, modtime, false)) { + return ExitCodeBackError; + } - int64_t size = 0; - if (type == NodeTypeFile) { - if (!JsonParserUtility::extractValue(actionObj, sizeKey, size, false)) { - return ExitCodeBackError; - } - } + std::string tmp; + if (!JsonParserUtility::extractValue(actionObj, fileTypeKey, tmp)) { + return ExitCodeBackError; + } + NodeType type = tmp == fileKey ? NodeTypeFile : NodeTypeDirectory; - // Check unsupported characters - if (hasUnsupportedCharacters(name, id, type)) { - continue; + int64_t size = 0; + if (type == NodeTypeFile) { + if (!JsonParserUtility::extractValue(actionObj, sizeKey, size, false)) { + return ExitCodeBackError; } + } - bool canWrite = true; - Poco::JSON::Object::Ptr capabilitiesObj = actionObj->getObject(capabilitiesKey); - if (capabilitiesObj) { - if (!JsonParserUtility::extractValue(capabilitiesObj, canWriteKey, canWrite)) { - return ExitCodeBackError; - } - } + // Check unsupported characters + if (hasUnsupportedCharacters(name, id, type)) { + continue; + } - // Check access rights - bool rightsAdded = false; - if (accessRightsAdded(action, id, rightsAdded, createdAt, modtime) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L")"); - invalidateSnapshot(); - return ExitCodeBackError; - } - bool rightsRemoved = false; - if (accessRightsRemoved(action, id, rightsRemoved) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L")"); - invalidateSnapshot(); + bool canWrite = true; + Poco::JSON::Object::Ptr capabilitiesObj = actionObj->getObject(capabilitiesKey); + if (capabilitiesObj) { + if (!JsonParserUtility::extractValue(capabilitiesObj, canWriteKey, canWrite)) { return ExitCodeBackError; } + } - SyncName usedName = name; - if (!destName.empty()) { - usedName = destName; - } - SnapshotItem item(id, parentId, usedName, createdAt, modtime, type, size, canWrite); - - bool isWarning = false; - if (ExclusionTemplateCache::instance()->isExcludedTemplate(item.name(), isWarning)) { - if (isWarning) { - Error error(_syncPal->syncDbId(), "", id, type, path, ConflictTypeNone, InconsistencyTypeNone, - CancelTypeExcludedByTemplate); - _syncPal->addError(error); - } - // Remove it from snapshot - _snapshot->removeItem(id); - continue; + SyncName usedName = name; + if (!destName.empty()) { + usedName = destName; + } + SnapshotItem item(id, parentId, usedName, createdAt, modtime, type, size, canWrite); + + bool isWarning = false; + if (ExclusionTemplateCache::instance()->isExcludedTemplate(item.name(), isWarning)) { + if (isWarning) { + Error error(_syncPal->syncDbId(), "", id, type, path, ConflictTypeNone, InconsistencyTypeNone, + CancelTypeExcludedByTemplate); + _syncPal->addError(error); } + // Remove it from snapshot + _snapshot->removeItem(id); + continue; + } #ifdef _WIN32 - SyncName newName; - if (PlatformInconsistencyCheckerUtility::instance()->fixNameWithBackslash(usedName, newName)) { - usedName = newName; - } + SyncName newName; + if (PlatformInconsistencyCheckerUtility::instance()->fixNameWithBackslash(usedName, newName)) { + usedName = newName; + } #endif - // Process action - if (action == createAction || action == restoreAction || action == moveInAction || rightsAdded) { - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" - << Utility::s2ws(id).c_str() << L")" - << L" with action: " << Utility::s2ws(action).c_str()); - } - - if (action == moveInAction) { - // Keep track of moved items - movedItems.insert(id); + bool rightsAdded = false; + + // Process action + switch (actionCode) { + // Item added + case ActionCode::actionCodeAccessRightInsert: + case ActionCode::actionCodeAccessRightUpdate: + case ActionCode::actionCodeAccessRightUserInsert: + case ActionCode::actionCodeAccessRightUserUpdate: + case ActionCode::actionCodeAccessRightTeamInsert: + case ActionCode::actionCodeAccessRightTeamUpdate: + case ActionCode::actionCodeAccessRightMainUsersInsert: + case ActionCode::actionCodeAccessRightMainUsersUpdate: + { + bool hasRights = false; + if (hasAccessRights(id, hasRights, createdAt, modtime) != ExitCodeOk) { + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() + << L")"); + invalidateSnapshot(); + return ExitCodeBackError; } - - if (type == NodeTypeDirectory && (action == moveInAction || action == restoreAction || rightsAdded)) { + if (!hasRights) break; // Current user does not have the right to access this item, ignore action. + } + case ActionCode::actionCodeMoveIn: + case ActionCode::actionCodeRestore: + if (type == NodeTypeDirectory) { // Retrieve all children ExitCode exitCode = exploreDirectory(id); ExitCause exitCause = this->exitCause(); @@ -615,23 +613,57 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a return exitCode; } } - } else if (action == renameAction) { + if (actionCode == ActionCode::actionCodeMoveIn) { + // Keep track of moved items + movedItems.insert(id); + } + case ActionCode::actionCodeCreate: + if (_snapshot->updateItem(item)) { + LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" + << Utility::s2ws(id).c_str() << L")" + << L" with action: " << Utility::s2ws(actionStr).c_str()); + } + break; + + // Item renamed + case ActionCode::actionCodeRename: _syncPal->removeItemFromTmpBlacklist(id, ReplicaSideRemote); if (_snapshot->updateItem(item)) { LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L") renamed"); } - } else if (action == editAction) { + break; + + // Item edited + case ActionCode::actionCodeEdit: if (_snapshot->updateItem(item)) { LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L") edited at:" << modtime); } - } else if (action == trashAction || action == moveOutAction || rightsRemoved) { - if (action == moveOutAction && movedItems.find(id) != movedItems.end()) { - // Ignore move out action - continue; - } + break; + // Item removed + case ActionCode::actionCodeAccessRightRemove: + case ActionCode::actionCodeAccessRightUserRemove: + case ActionCode::actionCodeAccessRightTeamRemove: + case ActionCode::actionCodeAccessRightMainUsersRemove: + { + bool hasRights = false; + if (hasAccessRights(id, hasRights, createdAt, modtime) != ExitCodeOk) { + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() + << L")"); + invalidateSnapshot(); + return ExitCodeBackError; + } + if (hasRights) break; // Current user still have the right to access this item, ignore action. + } + case ActionCode::actionCodeMoveOut: + if (movedItems.find(id) != movedItems.end()) { + // Ignore move out action if destination is inside the synced folder. + break; + } + case ActionCode::actionCodeTrash: _syncPal->removeItemFromTmpBlacklist(id, ReplicaSideRemote); if (_snapshot->removeItem(id)) { if (ParametersCache::instance()->parameters().extendedLog()) { @@ -644,50 +676,27 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a invalidateSnapshot(); return ExitCodeBackError; } - } else if (action == accessAction || action == restoreFileShareCreate || action == restoreFileShareDelete || - action == restoreShareLinkCreate || action == restoreShareLinkDelete || action == deleteAction) { + break; + + // Ignored actions + case ActionCode::actionCodeAccess: + case ActionCode::actionCodeDelete: + case ActionCode::actionCodeRestoreFileShareCreate: + case ActionCode::actionCodeRestoreFileShareDelete: + case ActionCode::actionCodeRestoreShareLinkCreate: + case ActionCode::actionCodeRestoreShareLinkDelete: // Ignore these actions - } else { + break; + + default: LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() << L")"); - } } } return ExitCodeOk; } -ExitCode RemoteFileSystemObserverWorker::accessRightsAdded(const std::string &action, const NodeId &nodeId, bool &rightsAdded, - SyncTime &createdAt, SyncTime &modtime) { - if (action == accessRightInsert || action == accessRightUpdate || action == accessRightUserInsert || - action == accessRightUserUpdate || action == accessRightTeamInsert || action == accessRightTeamUpdate || - action == accessRightMainUsersInsert || action == accessRightMainUsersUpdate) { - bool hasRights = false; - ExitCode exitCode = hasAccessRights(nodeId, hasRights, createdAt, modtime); - rightsAdded = hasRights; - return exitCode; - } - - rightsAdded = false; - return ExitCodeOk; -} - -ExitCode RemoteFileSystemObserverWorker::accessRightsRemoved(const std::string &action, const NodeId &nodeId, - bool &rightsRemoved) { - if (action == accessRightRemove || action == accessRightUserRemove || action == accessRightTeamRemove || - action == accessRightMainUsersRemove) { - bool hasRights = false; - SyncTime createdAt = 0; - SyncTime modtime = 0; - ExitCode exitCode = hasAccessRights(nodeId, hasRights, createdAt, modtime); - rightsRemoved = !hasRights; - return exitCode; - } - - rightsRemoved = false; - return ExitCodeOk; -} - ExitCode RemoteFileSystemObserverWorker::hasAccessRights(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime) { GetFileInfoJob job(_syncPal->driveDbId(), nodeId); diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index 3e8a86cf0..3c78d3911 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -20,6 +20,8 @@ #include "filesystemobserverworker.h" +#include "jobs/network/networkjobsparams.h" + #include namespace KDC { @@ -45,9 +47,6 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { ExitCode processActions(Poco::JSON::Array::Ptr filesArray); - ExitCode accessRightsAdded(const std::string &action, const NodeId &nodeId, bool &rightsAdded, SyncTime &createdAt, - SyncTime &modtime); - ExitCode accessRightsRemoved(const std::string &action, const NodeId &nodeId, bool &rightsRemoved); ExitCode hasAccessRights(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime); bool hasUnsupportedCharacters(const SyncName &name, const NodeId &nodeId, NodeType type); From 8fe5d0bcd6c79423af9216531358821b35a5e3c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Fri, 10 May 2024 16:56:46 +0200 Subject: [PATCH 03/12] KDESKTOP-823 - Refactor file action info extraction --- .../remotefilesystemobserverworker.cpp | 191 +++++++++--------- .../remotefilesystemobserverworker.h | 14 ++ 2 files changed, 113 insertions(+), 92 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 97d7dcad4..74f0931c0 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -482,89 +482,33 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a return ExitCodeOk; } - // Check new actions - std::string actionStr; Poco::JSON::Object::Ptr actionObj = it->extract(); - if (!JsonParserUtility::extractValue(actionObj, actionKey, actionStr)) { - return ExitCodeBackError; - } - ActionCode actionCode = getActionCode(actionStr); - - int idInt = 0; - if (!JsonParserUtility::extractValue(actionObj, fileIdKey, idInt)) { - return ExitCodeBackError; - } - NodeId id = std::to_string(idInt); - - int parentIdInt = 0; - if (!JsonParserUtility::extractValue(actionObj, parentIdKey, parentIdInt)) { - return ExitCodeBackError; - } - NodeId parentId = std::to_string(parentIdInt); - - SyncName path; - if (!JsonParserUtility::extractValue(actionObj, pathKey, path)) { - return ExitCodeBackError; - } - SyncName name = path.substr(path.find_last_of('/') + 1); // +1 to ignore the last "/" - - SyncName destPath; - if (!JsonParserUtility::extractValue(actionObj, destinationKey, destPath, false)) { - return ExitCodeBackError; - } - SyncName destName = destPath.substr(destPath.find_last_of('/') + 1); // +1 to ignore the last "/" - - SyncTime createdAt = 0; - if (!JsonParserUtility::extractValue(actionObj, createdAtKey, createdAt, false)) { - return ExitCodeBackError; - } - - SyncTime modtime = 0; - if (!JsonParserUtility::extractValue(actionObj, lastModifiedAtKey, modtime, false)) { - return ExitCodeBackError; - } - - std::string tmp; - if (!JsonParserUtility::extractValue(actionObj, fileTypeKey, tmp)) { - return ExitCodeBackError; - } - NodeType type = tmp == fileKey ? NodeTypeFile : NodeTypeDirectory; - - int64_t size = 0; - if (type == NodeTypeFile) { - if (!JsonParserUtility::extractValue(actionObj, sizeKey, size, false)) { - return ExitCodeBackError; - } + ActionInfo actionInfo; + if (ExitCode exitCode = extractActionInfo(actionObj, actionInfo); exitCode != ExitCodeOk) { + return exitCode; } // Check unsupported characters - if (hasUnsupportedCharacters(name, id, type)) { + if (hasUnsupportedCharacters(actionInfo.name, actionInfo.nodeId, actionInfo.type)) { continue; } - bool canWrite = true; - Poco::JSON::Object::Ptr capabilitiesObj = actionObj->getObject(capabilitiesKey); - if (capabilitiesObj) { - if (!JsonParserUtility::extractValue(capabilitiesObj, canWriteKey, canWrite)) { - return ExitCodeBackError; - } - } - - SyncName usedName = name; - if (!destName.empty()) { - usedName = destName; + SyncName usedName = actionInfo.name; + if (!actionInfo.destName.empty()) { + usedName = actionInfo.destName; } - SnapshotItem item(id, parentId, usedName, createdAt, modtime, type, size, canWrite); + SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime + , actionInfo.type, actionInfo.size, actionInfo.canWrite); bool isWarning = false; if (ExclusionTemplateCache::instance()->isExcludedTemplate(item.name(), isWarning)) { if (isWarning) { - Error error(_syncPal->syncDbId(), "", id, type, path, ConflictTypeNone, InconsistencyTypeNone, + Error error(_syncPal->syncDbId(), "", actionInfo.nodeId, actionInfo.type, actionInfo.path, ConflictTypeNone, InconsistencyTypeNone, CancelTypeExcludedByTemplate); _syncPal->addError(error); } // Remove it from snapshot - _snapshot->removeItem(id); + _snapshot->removeItem(actionInfo.nodeId); continue; } @@ -577,7 +521,7 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a bool rightsAdded = false; // Process action - switch (actionCode) { + switch (actionInfo.actionCode) { // Item added case ActionCode::actionCodeAccessRightInsert: case ActionCode::actionCodeAccessRightUpdate: @@ -589,20 +533,25 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a case ActionCode::actionCodeAccessRightMainUsersUpdate: { bool hasRights = false; - if (hasAccessRights(id, hasRights, createdAt, modtime) != ExitCodeOk) { + int64_t createdAt = 0; + int64_t modtime = 0; + if (hasAccessRights(actionInfo.nodeId, hasRights, createdAt, modtime) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() + << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); return ExitCodeBackError; } if (!hasRights) break; // Current user does not have the right to access this item, ignore action. + + item.setCreatedAt(createdAt); + item.setLastModified(modtime); } case ActionCode::actionCodeMoveIn: case ActionCode::actionCodeRestore: - if (type == NodeTypeDirectory) { + if (actionInfo.type == NodeTypeDirectory) { // Retrieve all children - ExitCode exitCode = exploreDirectory(id); + ExitCode exitCode = exploreDirectory(actionInfo.nodeId); ExitCause exitCause = this->exitCause(); if (exitCode == ExitCodeNetworkError && exitCause == ExitCauseNetworkTimeout) { @@ -613,32 +562,32 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a return exitCode; } } - if (actionCode == ActionCode::actionCodeMoveIn) { + if (actionInfo.actionCode == ActionCode::actionCodeMoveIn) { // Keep track of moved items - movedItems.insert(id); + movedItems.insert(actionInfo.nodeId); } case ActionCode::actionCodeCreate: if (_snapshot->updateItem(item)) { LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" - << Utility::s2ws(id).c_str() << L")" - << L" with action: " << Utility::s2ws(actionStr).c_str()); + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); } break; // Item renamed case ActionCode::actionCodeRename: - _syncPal->removeItemFromTmpBlacklist(id, ReplicaSideRemote); + _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(name).c_str() << L" (" - << Utility::s2ws(id).c_str() << L") renamed"); + LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L") renamed"); } break; // Item edited case ActionCode::actionCodeEdit: if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(name).c_str() << L" (" - << Utility::s2ws(id).c_str() << L") edited at:" << modtime); + LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() + << L") edited at:" << actionInfo.modtime); } break; @@ -649,9 +598,9 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a case ActionCode::actionCodeAccessRightMainUsersRemove: { bool hasRights = false; - if (hasAccessRights(id, hasRights, createdAt, modtime) != ExitCodeOk) { + if (hasAccessRights(actionInfo.nodeId, hasRights, actionInfo.createdAt, actionInfo.modtime) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(name).c_str() << L" (" << Utility::s2ws(id).c_str() + << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); return ExitCodeBackError; @@ -659,20 +608,20 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a if (hasRights) break; // Current user still have the right to access this item, ignore action. } case ActionCode::actionCodeMoveOut: - if (movedItems.find(id) != movedItems.end()) { + if (movedItems.find(actionInfo.nodeId) != movedItems.end()) { // Ignore move out action if destination is inside the synced folder. break; } case ActionCode::actionCodeTrash: - _syncPal->removeItemFromTmpBlacklist(id, ReplicaSideRemote); - if (_snapshot->removeItem(id)) { + _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); + if (_snapshot->removeItem(actionInfo.nodeId)) { if (ParametersCache::instance()->parameters().extendedLog()) { - LOGW_SYNCPAL_DEBUG(_logger, L"Item removed from remote snapshot: " << SyncName2WStr(name).c_str() << L" (" - << Utility::s2ws(id).c_str() << L")"); + LOGW_SYNCPAL_DEBUG(_logger, L"Item removed from remote snapshot: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); } } else { - LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(name).c_str() << L" (" - << Utility::s2ws(id).c_str() << L")"); + LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); return ExitCodeBackError; } @@ -689,8 +638,66 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a break; default: - LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(name).c_str() << L" (" - << Utility::s2ws(id).c_str() << L")"); + LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + } + } + + return ExitCodeOk; +} + +ExitCode RemoteFileSystemObserverWorker::extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo) { + std::string tmpStr; + if (!JsonParserUtility::extractValue(actionObj, actionKey, tmpStr)) { + return ExitCodeBackError; + } + actionInfo.actionCode = getActionCode(tmpStr); + + int64_t tmpInt = 0; + if (!JsonParserUtility::extractValue(actionObj, fileIdKey, tmpInt)) { + return ExitCodeBackError; + } + actionInfo.nodeId = std::to_string(tmpInt); + + if (!JsonParserUtility::extractValue(actionObj, parentIdKey, tmpInt)) { + return ExitCodeBackError; + } + actionInfo.parentNodeId = std::to_string(tmpInt); + + if (!JsonParserUtility::extractValue(actionObj, pathKey, actionInfo.path)) { + return ExitCodeBackError; + } + actionInfo.name = actionInfo.path.substr(actionInfo.path.find_last_of('/') + 1); // +1 to ignore the last "/" + + SyncName tmpSyncName; + if (!JsonParserUtility::extractValue(actionObj, destinationKey, tmpSyncName, false)) { + return ExitCodeBackError; + } + actionInfo.destName = tmpSyncName.substr(tmpSyncName.find_last_of('/') + 1); // +1 to ignore the last "/" + + if (!JsonParserUtility::extractValue(actionObj, createdAtKey, actionInfo.createdAt, false)) { + return ExitCodeBackError; + } + + if (!JsonParserUtility::extractValue(actionObj, lastModifiedAtKey, actionInfo.modtime, false)) { + return ExitCodeBackError; + } + + if (!JsonParserUtility::extractValue(actionObj, fileTypeKey, tmpStr)) { + return ExitCodeBackError; + } + actionInfo.type = tmpStr == fileKey ? NodeTypeFile : NodeTypeDirectory; + + if (actionInfo.type == NodeTypeFile) { + if (!JsonParserUtility::extractValue(actionObj, sizeKey, actionInfo.size, false)) { + return ExitCodeBackError; + } + } + + Poco::JSON::Object::Ptr capabilitiesObj = actionObj->getObject(capabilitiesKey); + if (capabilitiesObj) { + if (!JsonParserUtility::extractValue(capabilitiesObj, canWriteKey, actionInfo.canWrite)) { + return ExitCodeBackError; } } diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index 3c78d3911..f8be986b8 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -45,7 +45,21 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { ExitCode sendLongPoll(bool &changes); + struct ActionInfo { + ActionCode actionCode {ActionCode::actionCodeUnknown}; + NodeId nodeId; + NodeId parentNodeId; + SyncName name; + SyncName path; + SyncName destName; + SyncTime createdAt {0}; + SyncTime modtime {0}; + NodeType type {NodeTypeUnknown}; + int64_t size {0}; + bool canWrite {true}; + }; ExitCode processActions(Poco::JSON::Array::Ptr filesArray); + ExitCode extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo); ExitCode hasAccessRights(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime); From 5f37d20840ff3188405a025211522952f2ab7e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Mon, 13 May 2024 11:32:23 +0200 Subject: [PATCH 04/12] KDESKTOP-823 - Fix typos --- src/libsyncengine/syncpal/operationprocessor.cpp | 2 +- .../computefsoperationworker.cpp | 12 ++++++------ .../localfilesystemobserverworker.cpp | 4 ++-- .../file_system_observer/snapshot/snapshot.cpp | 4 ++-- .../file_system_observer/snapshot/snapshot.h | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libsyncengine/syncpal/operationprocessor.cpp b/src/libsyncengine/syncpal/operationprocessor.cpp index 62fded425..a5c67a69a 100644 --- a/src/libsyncengine/syncpal/operationprocessor.cpp +++ b/src/libsyncengine/syncpal/operationprocessor.cpp @@ -56,7 +56,7 @@ bool OperationProcessor::isPseudoConflict(std::shared_ptr node, std::share bool useContentChecksum = !snapshot->contentChecksum(*node->id()).empty() && !otherSnapshot->contentChecksum(*correspondingNode->id()).empty(); - bool sameSizeAndDate = snapshot->lastModifed(*node->id()) == otherSnapshot->lastModifed(*correspondingNode->id()) && + bool sameSizeAndDate = snapshot->lastModified(*node->id()) == otherSnapshot->lastModified(*correspondingNode->id()) && snapshot->size(*node->id()) == otherSnapshot->size(*correspondingNode->id()); if (node->type() == NodeType::NodeTypeFile && correspondingNode->type() == node->type() && node->hasChangeEvent((OperationTypeCreate | OperationTypeEdit)) && diff --git a/src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp b/src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp index 93e02d413..22a2ed280 100644 --- a/src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp @@ -351,7 +351,7 @@ ExitCode ComputeFSOperationWorker::exploreDbTree(std::unordered_set &loc } } - const SyncTime snapshotLastModified = snapshot->lastModifed(nodeId); + const SyncTime snapshotLastModified = snapshot->lastModified(nodeId); if (snapshotLastModified != dbLastModified && dbNode.type() == NodeType::NodeTypeFile) { // Edit operation FSOpPtr fsOp = std::make_shared(OperationType::OperationTypeEdit, nodeId, NodeType::NodeTypeFile, @@ -454,7 +454,7 @@ ExitCode ComputeFSOperationWorker::exploreSnapshotTree(ReplicaSide side, const s NodeType type = snapshot->type(*snapIdIt); if (checkOnlyDir && type != NodeTypeDirectory) { - // In first loop, we check only directory + // In first loop, we check only directories snapIdIt++; continue; } @@ -466,7 +466,7 @@ ExitCode ComputeFSOperationWorker::exploreSnapshotTree(ReplicaSide side, const s if (snapshot->isOrphan(snapshot->parentId(nodeId))) { // Ignore orphans if (ParametersCache::instance()->parameters().extendedLog()) { - LOGW_SYNCPAL_DEBUG(_logger, L"Ignoring ophan node " << SyncName2WStr(snapshot->name(nodeId)).c_str() << L" (" + LOGW_SYNCPAL_DEBUG(_logger, L"Ignoring orphan node " << SyncName2WStr(snapshot->name(nodeId)).c_str() << L" (" << Utility::s2ws(nodeId).c_str() << L")"); } continue; @@ -510,7 +510,7 @@ ExitCode ComputeFSOperationWorker::exploreSnapshotTree(ReplicaSide side, const s // Create operation FSOpPtr fsOp = std::make_shared(OperationType::OperationTypeCreate, nodeId, type, snapshot->createdAt(nodeId), - snapshot->lastModifed(nodeId), snapshotSize, snapPath); + snapshot->lastModified(nodeId), snapshotSize, snapPath); opSet->insertOp(fsOp); logOperationGeneration(snapshot->side(), fsOp); } @@ -562,8 +562,8 @@ ExitCode ComputeFSOperationWorker::checkFileIntegrity(const DbNode &dbNode) { int64_t localSnapshotSize = _syncPal->_localSnapshot->size(dbNode.nodeIdLocal().value()); int64_t remoteSnapshotSize = _syncPal->_remoteSnapshot->size(dbNode.nodeIdRemote().value()); - SyncTime localSnapshotLastModified = _syncPal->_localSnapshot->lastModifed(dbNode.nodeIdLocal().value()); - SyncTime remoteSnapshotLastModified = _syncPal->_remoteSnapshot->lastModifed(dbNode.nodeIdRemote().value()); + SyncTime localSnapshotLastModified = _syncPal->_localSnapshot->lastModified(dbNode.nodeIdLocal().value()); + SyncTime remoteSnapshotLastModified = _syncPal->_remoteSnapshot->lastModified(dbNode.nodeIdRemote().value()); // A mismatch is detected if all timestamps are equal but the sizes in snapshots differ. if (localSnapshotSize != remoteSnapshotSize && localSnapshotLastModified == dbNode.lastModifiedLocal().value() && diff --git a/src/libsyncengine/update_detection/file_system_observer/localfilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/localfilesystemobserverworker.cpp index 03c648bc6..738c2f929 100644 --- a/src/libsyncengine/update_detection/file_system_observer/localfilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/localfilesystemobserverworker.cpp @@ -206,7 +206,7 @@ void LocalFileSystemObserverWorker::changesDetected(const std::listsize(nodeId), - _snapshot->lastModifed(nodeId), changed, ioError); + _snapshot->lastModified(nodeId), changed, ioError); if (!success) { LOGW_SYNCPAL_WARN( _logger, L"Error in IoHelper::checkIfFileChanged: " << Utility::formatIoError(absolutePath, ioError).c_str()); @@ -344,7 +344,7 @@ void LocalFileSystemObserverWorker::changesDetected(const std::list _snapshot->lastModifed(nodeId)) { + if (fileStat.modtime > _snapshot->lastModified(nodeId)) { // This is an edit operation #ifdef __APPLE__ if (_syncPal->_vfsMode == VirtualFileModeMac) { 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 2d84caf8d..807d2a8a0 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp @@ -253,7 +253,7 @@ bool Snapshot::setCreatedAt(const NodeId &itemId, SyncTime newTime) { return false; } -SyncTime Snapshot::lastModifed(const NodeId &itemId) { +SyncTime Snapshot::lastModified(const NodeId &itemId) { const std::lock_guard lock(_mutex); SyncTime ret = 0; auto it = _items.find(itemId); @@ -263,7 +263,7 @@ SyncTime Snapshot::lastModifed(const NodeId &itemId) { return ret; } -bool Snapshot::setLastModifed(const NodeId &itemId, SyncTime newTime) { +bool Snapshot::setLastModified(const NodeId &itemId, SyncTime newTime) { const std::lock_guard lock(_mutex); auto it = _items.find(itemId); if (it != _items.end()) { diff --git a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.h b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.h index 91dac0f10..0624437aa 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.h +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.h @@ -46,8 +46,8 @@ class Snapshot : public SharedObject { bool setName(const NodeId &itemId, const SyncName &newName); SyncTime createdAt(const NodeId &itemId); bool setCreatedAt(const NodeId &itemId, SyncTime newTime); - SyncTime lastModifed(const NodeId &itemId); - bool setLastModifed(const NodeId &itemId, SyncTime newTime); + SyncTime lastModified(const NodeId &itemId); + bool setLastModified(const NodeId &itemId, SyncTime newTime); NodeType type(const NodeId &itemId); int64_t size(const NodeId &itemId); std::string contentChecksum(const NodeId &itemId); From e93a1318a9d2b7a1fc959e639d821616b9a4b226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Mon, 13 May 2024 11:35:54 +0200 Subject: [PATCH 05/12] KDESKTOP-823 - fix missing item in snapshot --- .../jobs/network/getfileinfojob.cpp | 10 +++++ .../jobs/network/getfileinfojob.h | 10 +++-- .../remotefilesystemobserverworker.cpp | 42 ++++++++++++------- .../remotefilesystemobserverworker.h | 2 +- 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/libsyncengine/jobs/network/getfileinfojob.cpp b/src/libsyncengine/jobs/network/getfileinfojob.cpp index 33db70f37..61b738128 100644 --- a/src/libsyncengine/jobs/network/getfileinfojob.cpp +++ b/src/libsyncengine/jobs/network/getfileinfojob.cpp @@ -51,6 +51,16 @@ bool GetFileInfoJob::handleResponse(std::istream &is) { if (!JsonParserUtility::extractValue(dataObj, lastModifiedAtKey, _modtime)) { return false; } + std::string tmp; + if (!JsonParserUtility::extractValue(dataObj, typeKey, tmp)) { + return false; + } + bool isDir = tmp == dirKey; + if (!isDir) { + if (!JsonParserUtility::extractValue(dataObj, sizeKey, _size)) { + return false; + } + } std::string symbolicLink; if (JsonParserUtility::extractValue(dataObj, symbolicLinkKey, symbolicLink, false)) { diff --git a/src/libsyncengine/jobs/network/getfileinfojob.h b/src/libsyncengine/jobs/network/getfileinfojob.h index b33a4e578..d7e8448db 100644 --- a/src/libsyncengine/jobs/network/getfileinfojob.h +++ b/src/libsyncengine/jobs/network/getfileinfojob.h @@ -33,6 +33,7 @@ class GetFileInfoJob : public AbstractTokenNetworkJob { inline SyncTime creationTime() const { return _creationTime; } inline SyncTime modtime() const { return _modtime; } inline bool isLink() const { return _isLink; } + inline int64_t size() const { return _size; } inline void setWithPath(bool val) { _withPath = val; } inline SyncPath path() const { return _path; } @@ -49,10 +50,11 @@ class GetFileInfoJob : public AbstractTokenNetworkJob { NodeId _nodeId; NodeId _parentNodeId; std::string _name; - SyncTime _creationTime = 0; - SyncTime _modtime = 0; - bool _isLink = false; - bool _withPath = false; + SyncTime _creationTime {0}; + SyncTime _modtime {0}; + bool _isLink {false}; + bool _withPath {false}; + int64_t _size {-1}; SyncPath _path; }; diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 74f0931c0..7625f9330 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -328,11 +328,14 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons bool error = false; bool ignore = false; std::unordered_set existingFiles; + uint64_t itemCount = 0; while (job->getItem(item, error, ignore)) { if (ignore) { continue; } + itemCount++; + if (error) { LOG_SYNCPAL_WARN(_logger, "Failed to parse CSV reply"); setExitCause(ExitCauseUnknown); @@ -389,6 +392,11 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons auto nodeIdIt = nodeIds.begin(); while (nodeIdIt != nodeIds.end()) { if (_snapshot->isOrphan(*nodeIdIt)) { + LOG_SYNCPAL_DEBUG(_logger, + L"Node " << SyncName2WStr(_snapshot->name(*nodeIdIt)).c_str() + << L" (" << Utility::s2ws(*nodeIdIt).c_str() + << L") is orphan. Removing it from " + << Utility::s2ws(Utility::side2Str(_snapshot->side())).c_str() << L" snapshot."); _snapshot->removeItem(*nodeIdIt); } nodeIdIt++; @@ -396,7 +404,7 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons std::chrono::duration elapsed_seconds = std::chrono::steady_clock::now() - start; LOG_SYNCPAL_DEBUG(_logger, - "End reply parsing in " << elapsed_seconds.count() << "s for " << _snapshot->nbItems() << " items"); + "End reply parsing in " << elapsed_seconds.count() << "s for " << itemCount << " items"); return ExitCodeOk; } @@ -533,9 +541,10 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a case ActionCode::actionCodeAccessRightMainUsersUpdate: { bool hasRights = false; - int64_t createdAt = 0; - int64_t modtime = 0; - if (hasAccessRights(actionInfo.nodeId, hasRights, createdAt, modtime) != ExitCodeOk) { + SyncTime createdAt = 0; + SyncTime modtime = 0; + int64_t size = 0; + if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); @@ -546,10 +555,17 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a item.setCreatedAt(createdAt); item.setLastModified(modtime); + item.setSize(size); + [[fallthrough]]; } case ActionCode::actionCodeMoveIn: case ActionCode::actionCodeRestore: - if (actionInfo.type == NodeTypeDirectory) { + case ActionCode::actionCodeCreate: + if (_snapshot->updateItem(item)) { + LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + } + if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { // Retrieve all children ExitCode exitCode = exploreDirectory(actionInfo.nodeId); ExitCause exitCause = this->exitCause(); @@ -566,11 +582,6 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a // Keep track of moved items movedItems.insert(actionInfo.nodeId); } - case ActionCode::actionCodeCreate: - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - } break; // Item renamed @@ -598,7 +609,7 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a case ActionCode::actionCodeAccessRightMainUsersRemove: { bool hasRights = false; - if (hasAccessRights(actionInfo.nodeId, hasRights, actionInfo.createdAt, actionInfo.modtime) != ExitCodeOk) { + if (getFileInfo(actionInfo.nodeId, hasRights, actionInfo.createdAt, actionInfo.modtime, actionInfo.size) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); @@ -606,12 +617,14 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a return ExitCodeBackError; } if (hasRights) break; // Current user still have the right to access this item, ignore action. + [[fallthrough]]; } case ActionCode::actionCodeMoveOut: if (movedItems.find(actionInfo.nodeId) != movedItems.end()) { // Ignore move out action if destination is inside the synced folder. break; } + [[fallthrough]]; case ActionCode::actionCodeTrash: _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); if (_snapshot->removeItem(actionInfo.nodeId)) { @@ -704,11 +717,11 @@ ExitCode RemoteFileSystemObserverWorker::extractActionInfo(const Poco::JSON::Obj return ExitCodeOk; } -ExitCode RemoteFileSystemObserverWorker::hasAccessRights(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, - SyncTime &modtime) { +ExitCode RemoteFileSystemObserverWorker::getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, + SyncTime &modtime, int64_t &size) { GetFileInfoJob job(_syncPal->driveDbId(), nodeId); job.runSynchronously(); - if (job.hasHttpError()) { + if (job.hasHttpError() || job.exitCode() != ExitCodeOk) { if (job.getStatusCode() == Poco::Net::HTTPResponse::HTTP_FORBIDDEN || job.getStatusCode() == Poco::Net::HTTPResponse::HTTP_NOT_FOUND) { hasRights = false; @@ -720,6 +733,7 @@ ExitCode RemoteFileSystemObserverWorker::hasAccessRights(const NodeId &nodeId, b createdAt = job.creationTime(); modtime = job.modtime(); + size = job.size(); hasRights = true; return ExitCodeOk; } diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index f8be986b8..52f7a4282 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -61,7 +61,7 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { ExitCode processActions(Poco::JSON::Array::Ptr filesArray); ExitCode extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo); - ExitCode hasAccessRights(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime); + ExitCode getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime, int64_t &size); bool hasUnsupportedCharacters(const SyncName &name, const NodeId &nodeId, NodeType type); From 8f7fe08c6d81d7479c0ba83418eecd28c0d75be8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Mon, 13 May 2024 15:54:13 +0200 Subject: [PATCH 06/12] KDESKTOP-823 - Fix SonarCloud issues --- .../jobs/network/getfileinfojob.cpp | 74 +++-- .../jobs/network/networkjobsparams.cpp | 65 +++-- .../jobs/network/networkjobsparams.h | 2 +- .../remotefilesystemobserverworker.cpp | 273 +++++++++--------- .../remotefilesystemobserverworker.h | 1 + 5 files changed, 218 insertions(+), 197 deletions(-) diff --git a/src/libsyncengine/jobs/network/getfileinfojob.cpp b/src/libsyncengine/jobs/network/getfileinfojob.cpp index 61b738128..578413223 100644 --- a/src/libsyncengine/jobs/network/getfileinfojob.cpp +++ b/src/libsyncengine/jobs/network/getfileinfojob.cpp @@ -35,49 +35,47 @@ GetFileInfoJob::GetFileInfoJob(int driveDbId, const NodeId &nodeId) } bool GetFileInfoJob::handleResponse(std::istream &is) { - if (!AbstractTokenNetworkJob::handleResponse(is)) { + if (!AbstractTokenNetworkJob::handleResponse(is)) return false; + + if (!jsonRes()) return true; + + Poco::JSON::Object::Ptr dataObj = jsonRes()->getObject(dataKey); + if (!dataObj) return true; + + if (!JsonParserUtility::extractValue(dataObj, parentIdKey, _parentNodeId)) { return false; } + if (!JsonParserUtility::extractValue(dataObj, createdAtKey, _creationTime)) { + return false; + } + if (!JsonParserUtility::extractValue(dataObj, lastModifiedAtKey, _modtime)) { + return false; + } + std::string tmp; + if (!JsonParserUtility::extractValue(dataObj, typeKey, tmp)) { + return false; + } + bool isDir = tmp == dirKey; + if (!isDir) { + if (!JsonParserUtility::extractValue(dataObj, sizeKey, _size)) { + return false; + } + } - if (jsonRes()) { - Poco::JSON::Object::Ptr dataObj = jsonRes()->getObject(dataKey); - if (dataObj) { - if (!JsonParserUtility::extractValue(dataObj, parentIdKey, _parentNodeId)) { - return false; - } - if (!JsonParserUtility::extractValue(dataObj, createdAtKey, _creationTime)) { - return false; - } - if (!JsonParserUtility::extractValue(dataObj, lastModifiedAtKey, _modtime)) { - return false; - } - std::string tmp; - if (!JsonParserUtility::extractValue(dataObj, typeKey, tmp)) { - return false; - } - bool isDir = tmp == dirKey; - if (!isDir) { - if (!JsonParserUtility::extractValue(dataObj, sizeKey, _size)) { - return false; - } - } - - std::string symbolicLink; - if (JsonParserUtility::extractValue(dataObj, symbolicLinkKey, symbolicLink, false)) { - _isLink = !symbolicLink.empty(); - } + std::string symbolicLink; + if (JsonParserUtility::extractValue(dataObj, symbolicLinkKey, symbolicLink, false)) { + _isLink = !symbolicLink.empty(); + } - if (_withPath) { - std::string str; - if (!JsonParserUtility::extractValue(dataObj, pathKey, str)) { - return false; - } - if (Utility::startsWith(str, "/")) { - str.erase(0, 1); - } - _path = str; - } + if (_withPath) { + std::string str; + if (!JsonParserUtility::extractValue(dataObj, pathKey, str)) { + return false; + } + if (Utility::startsWith(str, "/")) { + str.erase(0, 1); } + _path = str; } return true; diff --git a/src/libsyncengine/jobs/network/networkjobsparams.cpp b/src/libsyncengine/jobs/network/networkjobsparams.cpp index e5a62f833..098d6283f 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.cpp +++ b/src/libsyncengine/jobs/network/networkjobsparams.cpp @@ -22,38 +22,49 @@ namespace KDC { +using enum ActionCode; + +struct StringHash { + using is_transparent = void; // Enables heterogeneous operations. + + std::size_t operator()(std::string_view sv) const { + std::hash hasher; + return hasher(sv); + } +}; + ActionCode getActionCode(const std::string &action) noexcept { - static const std::unordered_map actionMap = { - {"file_create", ActionCode::actionCodeCreate}, - {"file_rename", ActionCode::actionCodeRename}, - {"file_update", ActionCode::actionCodeEdit}, - {"file_access", ActionCode::actionCodeAccess}, - {"file_trash", ActionCode::actionCodeTrash}, - {"file_delete", ActionCode::actionCodeDelete}, - {"file_move", ActionCode::actionCodeMoveIn}, - {"file_move_out", ActionCode::actionCodeMoveOut}, - {"file_restore", ActionCode::actionCodeRestore}, - {"file_share_create", ActionCode::actionCodeRestoreFileShareCreate}, - {"file_share_delete", ActionCode::actionCodeRestoreFileShareDelete}, - {"share_link_create", ActionCode::actionCodeRestoreShareLinkCreate}, - {"share_link_delete", ActionCode::actionCodeRestoreShareLinkDelete}, - {"acl_insert", ActionCode::actionCodeAccessRightInsert}, - {"acl_update", ActionCode::actionCodeAccessRightUpdate}, - {"acl_remove", ActionCode::actionCodeAccessRightRemove}, - {"acl_user_insert", ActionCode::actionCodeAccessRightUserInsert}, - {"acl_user_update", ActionCode::actionCodeAccessRightUserUpdate}, - {"acl_user_remove", ActionCode::actionCodeAccessRightUserRemove}, - {"acl_team_insert", ActionCode::actionCodeAccessRightTeamInsert}, - {"acl_team_update", ActionCode::actionCodeAccessRightTeamUpdate}, - {"acl_team_remove", ActionCode::actionCodeAccessRightTeamRemove}, - {"acl_main_users_insert", ActionCode::actionCodeAccessRightMainUsersInsert}, - {"acl_main_users_update", ActionCode::actionCodeAccessRightMainUsersUpdate}, - {"acl_main_users_remove", ActionCode::actionCodeAccessRightMainUsersRemove} + static const std::unordered_map> actionMap = { + {"file_create", actionCodeCreate}, + {"file_rename", actionCodeRename}, + {"file_update", actionCodeEdit}, + {"file_access", actionCodeAccess}, + {"file_trash", actionCodeTrash}, + {"file_delete", actionCodeDelete}, + {"file_move", actionCodeMoveIn}, + {"file_move_out", actionCodeMoveOut}, + {"file_restore", actionCodeRestore}, + {"file_share_create", actionCodeRestoreFileShareCreate}, + {"file_share_delete", actionCodeRestoreFileShareDelete}, + {"share_link_create", actionCodeRestoreShareLinkCreate}, + {"share_link_delete", actionCodeRestoreShareLinkDelete}, + {"acl_insert", actionCodeAccessRightInsert}, + {"acl_update", actionCodeAccessRightUpdate}, + {"acl_remove", actionCodeAccessRightRemove}, + {"acl_user_insert", actionCodeAccessRightUserInsert}, + {"acl_user_update", actionCodeAccessRightUserUpdate}, + {"acl_user_remove", actionCodeAccessRightUserRemove}, + {"acl_team_insert", actionCodeAccessRightTeamInsert}, + {"acl_team_update", actionCodeAccessRightTeamUpdate}, + {"acl_team_remove", actionCodeAccessRightTeamRemove}, + {"acl_main_users_insert", actionCodeAccessRightMainUsersInsert}, + {"acl_main_users_update", actionCodeAccessRightMainUsersUpdate}, + {"acl_main_users_remove", actionCodeAccessRightMainUsersRemove} }; if (const auto it = actionMap.find(action); it != actionMap.cend()) return it->second; - return ActionCode::actionCodeUnknown; + return actionCodeUnknown; }; NetworkErrorCode getNetworkErrorCode(const std::string &errorCode) noexcept { diff --git a/src/libsyncengine/jobs/network/networkjobsparams.h b/src/libsyncengine/jobs/network/networkjobsparams.h index 42c1e276c..174904f09 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.h +++ b/src/libsyncengine/jobs/network/networkjobsparams.h @@ -26,7 +26,7 @@ static const uint64_t chunkMinSize = 10 * 1024 * 1024; // 10MB static const uint64_t chunkMaxSize = 100 * 1024 * 1024; // 100MB static const uint64_t useUploadSessionThreshold = 100 * 1024 * 1024; // if file size > 100MB -> start upload session static const uint64_t optimalTotalChunks = 200; -static const uint64_t maxTotalChunks = 10000; // Theorical max. file size 10'000 * 100MB = 1TB +static const uint64_t maxTotalChunks = 10000; // Theoretical max. file size 10'000 * 100MB = 1TB /* * Static string diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 7625f9330..7a9727091 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -44,6 +44,8 @@ namespace KDC { +using enum ActionCode; + RemoteFileSystemObserverWorker::RemoteFileSystemObserverWorker(std::shared_ptr syncPal, const std::string &name, const std::string &shortName) : FileSystemObserverWorker(syncPal, name, shortName, ReplicaSideRemote), _driveDbId(syncPal->_driveDbId) {} @@ -505,14 +507,12 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a if (!actionInfo.destName.empty()) { usedName = actionInfo.destName; } - SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime - , actionInfo.type, actionInfo.size, actionInfo.canWrite); bool isWarning = false; - if (ExclusionTemplateCache::instance()->isExcludedTemplate(item.name(), isWarning)) { + if (ExclusionTemplateCache::instance()->isExcludedTemplate(usedName, isWarning)) { if (isWarning) { - Error error(_syncPal->syncDbId(), "", actionInfo.nodeId, actionInfo.type, actionInfo.path, ConflictTypeNone, InconsistencyTypeNone, - CancelTypeExcludedByTemplate); + Error error(_syncPal->syncDbId(), "", actionInfo.nodeId, actionInfo.type, actionInfo.path, ConflictTypeNone, + InconsistencyTypeNone, CancelTypeExcludedByTemplate); _syncPal->addError(error); } // Remove it from snapshot @@ -526,133 +526,9 @@ ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr a usedName = newName; } #endif - bool rightsAdded = false; - - // Process action - switch (actionInfo.actionCode) { - // Item added - case ActionCode::actionCodeAccessRightInsert: - case ActionCode::actionCodeAccessRightUpdate: - case ActionCode::actionCodeAccessRightUserInsert: - case ActionCode::actionCodeAccessRightUserUpdate: - case ActionCode::actionCodeAccessRightTeamInsert: - case ActionCode::actionCodeAccessRightTeamUpdate: - case ActionCode::actionCodeAccessRightMainUsersInsert: - case ActionCode::actionCodeAccessRightMainUsersUpdate: - { - bool hasRights = false; - SyncTime createdAt = 0; - SyncTime modtime = 0; - int64_t size = 0; - if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() - << L")"); - invalidateSnapshot(); - return ExitCodeBackError; - } - if (!hasRights) break; // Current user does not have the right to access this item, ignore action. - - item.setCreatedAt(createdAt); - item.setLastModified(modtime); - item.setSize(size); - [[fallthrough]]; - } - case ActionCode::actionCodeMoveIn: - case ActionCode::actionCodeRestore: - case ActionCode::actionCodeCreate: - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - } - if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { - // Retrieve all children - ExitCode exitCode = exploreDirectory(actionInfo.nodeId); - ExitCause exitCause = this->exitCause(); - - if (exitCode == ExitCodeNetworkError && exitCause == ExitCauseNetworkTimeout) { - _syncPal->addError(Error(ERRID, exitCode, exitCause)); - } - - if (exitCode != ExitCodeOk) { - return exitCode; - } - } - if (actionInfo.actionCode == ActionCode::actionCodeMoveIn) { - // Keep track of moved items - movedItems.insert(actionInfo.nodeId); - } - break; - - // Item renamed - case ActionCode::actionCodeRename: - _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L") renamed"); - } - break; - - // Item edited - case ActionCode::actionCodeEdit: - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() - << L") edited at:" << actionInfo.modtime); - } - break; - // Item removed - case ActionCode::actionCodeAccessRightRemove: - case ActionCode::actionCodeAccessRightUserRemove: - case ActionCode::actionCodeAccessRightTeamRemove: - case ActionCode::actionCodeAccessRightMainUsersRemove: - { - bool hasRights = false; - if (getFileInfo(actionInfo.nodeId, hasRights, actionInfo.createdAt, actionInfo.modtime, actionInfo.size) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() - << L")"); - invalidateSnapshot(); - return ExitCodeBackError; - } - if (hasRights) break; // Current user still have the right to access this item, ignore action. - [[fallthrough]]; - } - case ActionCode::actionCodeMoveOut: - if (movedItems.find(actionInfo.nodeId) != movedItems.end()) { - // Ignore move out action if destination is inside the synced folder. - break; - } - [[fallthrough]]; - case ActionCode::actionCodeTrash: - _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); - if (_snapshot->removeItem(actionInfo.nodeId)) { - if (ParametersCache::instance()->parameters().extendedLog()) { - LOGW_SYNCPAL_DEBUG(_logger, L"Item removed from remote snapshot: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - } - } else { - LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - invalidateSnapshot(); - return ExitCodeBackError; - } - break; - - // Ignored actions - case ActionCode::actionCodeAccess: - case ActionCode::actionCodeDelete: - case ActionCode::actionCodeRestoreFileShareCreate: - case ActionCode::actionCodeRestoreFileShareDelete: - case ActionCode::actionCodeRestoreShareLinkCreate: - case ActionCode::actionCodeRestoreShareLinkDelete: - // Ignore these actions - break; - - default: - LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + if (ExitCode exitCode = processAction(usedName, actionInfo, movedItems); exitCode != ExitCodeOk) { + return exitCode; } } @@ -717,6 +593,141 @@ ExitCode RemoteFileSystemObserverWorker::extractActionInfo(const Poco::JSON::Obj return ExitCodeOk; } +ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, const ActionInfo &actionInfo + , std::unordered_set &movedItems) { + SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime + , actionInfo.type, actionInfo.size, actionInfo.canWrite); + + // Process action + switch (actionInfo.actionCode) { + // Item added + case actionCodeAccessRightInsert: + case actionCodeAccessRightUpdate: + case actionCodeAccessRightUserInsert: + case actionCodeAccessRightUserUpdate: + case actionCodeAccessRightTeamInsert: + case actionCodeAccessRightTeamUpdate: + case actionCodeAccessRightMainUsersInsert: + case actionCodeAccessRightMainUsersUpdate: + { + bool hasRights = false; + SyncTime createdAt = 0; + SyncTime modtime = 0; + int64_t size = 0; + if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() + << L")"); + invalidateSnapshot(); + return ExitCodeBackError; + } + if (!hasRights) break; // Current user does not have the right to access this item, ignore action. + + item.setCreatedAt(createdAt); + item.setLastModified(modtime); + item.setSize(size); + [[fallthrough]]; + } + case actionCodeMoveIn: + case actionCodeRestore: + case actionCodeCreate: + if (_snapshot->updateItem(item)) { + LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + } + if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { + // Retrieve all children + ExitCode exitCode = exploreDirectory(actionInfo.nodeId); + ExitCause exitCause = this->exitCause(); + + if (exitCode == ExitCodeNetworkError && exitCause == ExitCauseNetworkTimeout) { + _syncPal->addError(Error(ERRID, exitCode, exitCause)); + } + + if (exitCode != ExitCodeOk) return exitCode; + } + if (actionInfo.actionCode == ActionCode::actionCodeMoveIn) { + // Keep track of moved items + movedItems.insert(actionInfo.nodeId); + } + break; + + // Item renamed + case actionCodeRename: + _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); + if (_snapshot->updateItem(item)) { + LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L") renamed"); + } + break; + + // Item edited + case actionCodeEdit: + if (_snapshot->updateItem(item)) { + LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() + << L") edited at:" << actionInfo.modtime); + } + break; + + // Item removed + case actionCodeAccessRightRemove: + case actionCodeAccessRightUserRemove: + case actionCodeAccessRightTeamRemove: + case actionCodeAccessRightMainUsersRemove: + { + bool hasRights = false; + SyncTime createdAt = 0; + SyncTime modtime = 0; + int64_t size = 0; + if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() + << L")"); + invalidateSnapshot(); + return ExitCodeBackError; + } + if (hasRights) break; // Current user still have the right to access this item, ignore action. + [[fallthrough]]; + } + case actionCodeMoveOut: + // Ignore move out action if destination is inside the synced folder. + if (movedItems.find(actionInfo.nodeId) != movedItems.end()) break; + [[fallthrough]]; + case actionCodeTrash: + _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); + if (_snapshot->removeItem(actionInfo.nodeId)) { + if (ParametersCache::instance()->parameters().extendedLog()) { + LOGW_SYNCPAL_DEBUG(_logger, L"Item removed from remote snapshot: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + } + } else { + LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + invalidateSnapshot(); + return ExitCodeBackError; + } + break; + + // Ignored actions + case actionCodeAccess: + case actionCodeDelete: + case actionCodeRestoreFileShareCreate: + case actionCodeRestoreFileShareDelete: + case actionCodeRestoreShareLinkCreate: + case actionCodeRestoreShareLinkDelete: + // Ignore these actions + break; + + default: + LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + } + + + return ExitCodeOk; +} + ExitCode RemoteFileSystemObserverWorker::getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime, int64_t &size) { GetFileInfoJob job(_syncPal->driveDbId(), nodeId); diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index 52f7a4282..f36fce6bc 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -60,6 +60,7 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { }; ExitCode processActions(Poco::JSON::Array::Ptr filesArray); ExitCode extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo); + ExitCode processAction(const SyncName &usedName, const ActionInfo &actionInfo, std::unordered_set &movedItems); ExitCode getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime, int64_t &size); From e22b64ced5242bb41aa3ce572da0a1c77ab2ee8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Mon, 13 May 2024 16:36:25 +0200 Subject: [PATCH 07/12] KDESKTOP-823 - Fix SonarCloud issues again --- .../remotefilesystemobserverworker.cpp | 64 +++++++------------ .../remotefilesystemobserverworker.h | 2 +- .../snapshot/snapshot.cpp | 12 ++++ .../snapshot/snapshotitem.h | 8 +-- 4 files changed, 40 insertions(+), 46 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 7a9727091..90515fa3c 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -394,19 +394,16 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons auto nodeIdIt = nodeIds.begin(); while (nodeIdIt != nodeIds.end()) { if (_snapshot->isOrphan(*nodeIdIt)) { - LOG_SYNCPAL_DEBUG(_logger, - L"Node " << SyncName2WStr(_snapshot->name(*nodeIdIt)).c_str() - << L" (" << Utility::s2ws(*nodeIdIt).c_str() - << L") is orphan. Removing it from " - << Utility::s2ws(Utility::side2Str(_snapshot->side())).c_str() << L" snapshot."); + LOG_SYNCPAL_DEBUG(_logger, L"Node " << SyncName2WStr(_snapshot->name(*nodeIdIt)).c_str() << L" (" + << Utility::s2ws(*nodeIdIt).c_str() << L") is orphan. Removing it from " + << Utility::s2ws(Utility::side2Str(_snapshot->side())).c_str() << L" snapshot."); _snapshot->removeItem(*nodeIdIt); } nodeIdIt++; } std::chrono::duration elapsed_seconds = std::chrono::steady_clock::now() - start; - LOG_SYNCPAL_DEBUG(_logger, - "End reply parsing in " << elapsed_seconds.count() << "s for " << itemCount << " items"); + LOG_SYNCPAL_DEBUG(_logger, "End reply parsing in " << elapsed_seconds.count() << "s for " << itemCount << " items"); return ExitCodeOk; } @@ -485,7 +482,7 @@ ExitCode RemoteFileSystemObserverWorker::sendLongPoll(bool &changes) { ExitCode RemoteFileSystemObserverWorker::processActions(Poco::JSON::Array::Ptr actionArray) { if (!actionArray) return ExitCodeOk; - std::unordered_set movedItems; + std::set> movedItems; for (auto it = actionArray->begin(); it != actionArray->end(); ++it) { if (stopAsked()) { @@ -593,10 +590,10 @@ ExitCode RemoteFileSystemObserverWorker::extractActionInfo(const Poco::JSON::Obj return ExitCodeOk; } -ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, const ActionInfo &actionInfo - , std::unordered_set &movedItems) { - SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime - , actionInfo.type, actionInfo.size, actionInfo.canWrite); +ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, const ActionInfo &actionInfo, + std::set> &movedItems) { + SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime, + actionInfo.type, actionInfo.size, actionInfo.canWrite); // Process action switch (actionInfo.actionCode) { @@ -608,16 +605,15 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case actionCodeAccessRightTeamInsert: case actionCodeAccessRightTeamUpdate: case actionCodeAccessRightMainUsersInsert: - case actionCodeAccessRightMainUsersUpdate: - { + case actionCodeAccessRightMainUsersUpdate: { bool hasRights = false; SyncTime createdAt = 0; SyncTime modtime = 0; int64_t size = 0; if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() - << L")"); + << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); return ExitCodeBackError; } @@ -631,10 +627,8 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case actionCodeMoveIn: case actionCodeRestore: case actionCodeCreate: - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory updated: " << SyncName2WStr(usedName).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - } + _snapshot->updateItem(item); + if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { // Retrieve all children ExitCode exitCode = exploreDirectory(actionInfo.nodeId); @@ -655,35 +649,27 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, // Item renamed case actionCodeRename: _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L") renamed"); - } + _snapshot->updateItem(item); break; // Item edited case actionCodeEdit: - if (_snapshot->updateItem(item)) { - LOGW_SYNCPAL_INFO(_logger, L"File/directory: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() - << L") edited at:" << actionInfo.modtime); - } + _snapshot->updateItem(item); break; // Item removed case actionCodeAccessRightRemove: case actionCodeAccessRightUserRemove: case actionCodeAccessRightTeamRemove: - case actionCodeAccessRightMainUsersRemove: - { + case actionCodeAccessRightMainUsersRemove: { bool hasRights = false; SyncTime createdAt = 0; SyncTime modtime = 0; int64_t size = 0; if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() - << L")"); + << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); return ExitCodeBackError; } @@ -696,12 +682,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, [[fallthrough]]; case actionCodeTrash: _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); - if (_snapshot->removeItem(actionInfo.nodeId)) { - if (ParametersCache::instance()->parameters().extendedLog()) { - LOGW_SYNCPAL_DEBUG(_logger, L"Item removed from remote snapshot: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - } - } else { + if (!_snapshot->removeItem(actionInfo.nodeId)) { LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); @@ -721,7 +702,8 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, default: LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + << Utility::s2ws(actionInfo.nodeId).c_str() + << L")"); } @@ -729,7 +711,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, } ExitCode RemoteFileSystemObserverWorker::getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, - SyncTime &modtime, int64_t &size) { + SyncTime &modtime, int64_t &size) { GetFileInfoJob job(_syncPal->driveDbId(), nodeId); job.runSynchronously(); if (job.hasHttpError() || job.exitCode() != ExitCodeOk) { diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index f36fce6bc..25fbde9a5 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -60,7 +60,7 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { }; ExitCode processActions(Poco::JSON::Array::Ptr filesArray); ExitCode extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo); - ExitCode processAction(const SyncName &usedName, const ActionInfo &actionInfo, std::unordered_set &movedItems); + ExitCode processAction(const SyncName &usedName, const ActionInfo &actionInfo, std::set> &movedItems); ExitCode getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime, int64_t &size); 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 807d2a8a0..b3958c6ed 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp @@ -18,6 +18,7 @@ #include "snapshot.h" #include "libcommonserver/log/log.h" +#include "requests/parameterscache.h" #include #include @@ -84,6 +85,12 @@ bool Snapshot::updateItem(const SnapshotItem &newItem) { startUpdate(); } + if (ParametersCache::instance()->parameters().extendedLog()) { + LOGW_DEBUG(Log::instance()->getLogger(), L"File/directory: " << SyncName2WStr(newItem.name()).c_str() << L" (" + << Utility::s2ws(newItem.id()).c_str() << L") updated at:" + << newItem.lastModified()); + } + return true; } @@ -113,6 +120,11 @@ bool Snapshot::removeItem(const NodeId &id) { } _items.erase(id); + + if (ParametersCache::instance()->parameters().extendedLog()) { + LOG_DEBUG(Log::instance()->getLogger(), "Item " << id.c_str() << "removed from remote snapshot."); + } + return true; } diff --git a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshotitem.h b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshotitem.h index 1045cf488..a4018e95a 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshotitem.h +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshotitem.h @@ -35,13 +35,13 @@ class SnapshotItem { bool canShare = true); SnapshotItem(const SnapshotItem &other); - inline NodeId id() const { return _id; } + inline const NodeId & id() const { return _id; } inline void setId(const NodeId &id) { _id = id; } - inline NodeId parentId() const { return _parentId; } + inline const NodeId & parentId() const { return _parentId; } inline void setParentId(const NodeId &newParentId) { _parentId = newParentId; } inline const std::unordered_set &childrenIds() const { return _childrenIds; } inline void setChildrenIds(const std::unordered_set &newChildrenIds) { _childrenIds = newChildrenIds; } - inline SyncName name() const { return _name; } + inline const SyncName & name() const { return _name; } inline void setName(const SyncName &newName) { _name = newName; } inline SyncTime createdAt() const { return _createdAt; } inline void setCreatedAt(SyncTime newCreatedAt) { _createdAt = newCreatedAt; } @@ -51,7 +51,7 @@ class SnapshotItem { inline void setType(NodeType type) { _type = type; } inline int64_t size() const { return _size; } inline void setSize(uint64_t newSize) { _size = newSize; } - inline std::string contentChecksum() const { return _contentChecksum; } + inline const std::string & contentChecksum() const { return _contentChecksum; } inline void setContentChecksum(const std::string &newChecksum) { _contentChecksum = newChecksum; } inline bool canWrite() const { return _canWrite; } inline void setCanWrite(bool canWrite) { _canWrite = canWrite; } From edaaaad70e21257625d957874967839948d3dab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Mon, 13 May 2024 16:49:16 +0200 Subject: [PATCH 08/12] KDESKTOP-823 - Fix CI build --- .../jobs/network/networkjobsparams.cpp | 55 +++++++++---------- .../remotefilesystemobserverworker.cpp | 52 +++++++++--------- 2 files changed, 52 insertions(+), 55 deletions(-) diff --git a/src/libsyncengine/jobs/network/networkjobsparams.cpp b/src/libsyncengine/jobs/network/networkjobsparams.cpp index 098d6283f..551dcde17 100644 --- a/src/libsyncengine/jobs/network/networkjobsparams.cpp +++ b/src/libsyncengine/jobs/network/networkjobsparams.cpp @@ -19,11 +19,10 @@ #include "networkjobsparams.h" #include +#include namespace KDC { -using enum ActionCode; - struct StringHash { using is_transparent = void; // Enables heterogeneous operations. @@ -35,36 +34,36 @@ struct StringHash { ActionCode getActionCode(const std::string &action) noexcept { static const std::unordered_map> actionMap = { - {"file_create", actionCodeCreate}, - {"file_rename", actionCodeRename}, - {"file_update", actionCodeEdit}, - {"file_access", actionCodeAccess}, - {"file_trash", actionCodeTrash}, - {"file_delete", actionCodeDelete}, - {"file_move", actionCodeMoveIn}, - {"file_move_out", actionCodeMoveOut}, - {"file_restore", actionCodeRestore}, - {"file_share_create", actionCodeRestoreFileShareCreate}, - {"file_share_delete", actionCodeRestoreFileShareDelete}, - {"share_link_create", actionCodeRestoreShareLinkCreate}, - {"share_link_delete", actionCodeRestoreShareLinkDelete}, - {"acl_insert", actionCodeAccessRightInsert}, - {"acl_update", actionCodeAccessRightUpdate}, - {"acl_remove", actionCodeAccessRightRemove}, - {"acl_user_insert", actionCodeAccessRightUserInsert}, - {"acl_user_update", actionCodeAccessRightUserUpdate}, - {"acl_user_remove", actionCodeAccessRightUserRemove}, - {"acl_team_insert", actionCodeAccessRightTeamInsert}, - {"acl_team_update", actionCodeAccessRightTeamUpdate}, - {"acl_team_remove", actionCodeAccessRightTeamRemove}, - {"acl_main_users_insert", actionCodeAccessRightMainUsersInsert}, - {"acl_main_users_update", actionCodeAccessRightMainUsersUpdate}, - {"acl_main_users_remove", actionCodeAccessRightMainUsersRemove} + {"file_create", ActionCode::actionCodeCreate}, + {"file_rename", ActionCode::actionCodeRename}, + {"file_update", ActionCode::actionCodeEdit}, + {"file_access", ActionCode::actionCodeAccess}, + {"file_trash", ActionCode::actionCodeTrash}, + {"file_delete", ActionCode::actionCodeDelete}, + {"file_move", ActionCode::actionCodeMoveIn}, + {"file_move_out", ActionCode::actionCodeMoveOut}, + {"file_restore", ActionCode::actionCodeRestore}, + {"file_share_create", ActionCode::actionCodeRestoreFileShareCreate}, + {"file_share_delete", ActionCode::actionCodeRestoreFileShareDelete}, + {"share_link_create", ActionCode::actionCodeRestoreShareLinkCreate}, + {"share_link_delete", ActionCode::actionCodeRestoreShareLinkDelete}, + {"acl_insert", ActionCode::actionCodeAccessRightInsert}, + {"acl_update", ActionCode::actionCodeAccessRightUpdate}, + {"acl_remove", ActionCode::actionCodeAccessRightRemove}, + {"acl_user_insert", ActionCode::actionCodeAccessRightUserInsert}, + {"acl_user_update", ActionCode::actionCodeAccessRightUserUpdate}, + {"acl_user_remove", ActionCode::actionCodeAccessRightUserRemove}, + {"acl_team_insert", ActionCode::actionCodeAccessRightTeamInsert}, + {"acl_team_update", ActionCode::actionCodeAccessRightTeamUpdate}, + {"acl_team_remove", ActionCode::actionCodeAccessRightTeamRemove}, + {"acl_main_users_insert", ActionCode::actionCodeAccessRightMainUsersInsert}, + {"acl_main_users_update", ActionCode::actionCodeAccessRightMainUsersUpdate}, + {"acl_main_users_remove", ActionCode::actionCodeAccessRightMainUsersRemove} }; if (const auto it = actionMap.find(action); it != actionMap.cend()) return it->second; - return actionCodeUnknown; + return ActionCode::actionCodeUnknown; }; NetworkErrorCode getNetworkErrorCode(const std::string &errorCode) noexcept { diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 90515fa3c..5cc8bb7f9 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -44,8 +44,6 @@ namespace KDC { -using enum ActionCode; - RemoteFileSystemObserverWorker::RemoteFileSystemObserverWorker(std::shared_ptr syncPal, const std::string &name, const std::string &shortName) : FileSystemObserverWorker(syncPal, name, shortName, ReplicaSideRemote), _driveDbId(syncPal->_driveDbId) {} @@ -598,14 +596,14 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, // Process action switch (actionInfo.actionCode) { // Item added - case actionCodeAccessRightInsert: - case actionCodeAccessRightUpdate: - case actionCodeAccessRightUserInsert: - case actionCodeAccessRightUserUpdate: - case actionCodeAccessRightTeamInsert: - case actionCodeAccessRightTeamUpdate: - case actionCodeAccessRightMainUsersInsert: - case actionCodeAccessRightMainUsersUpdate: { + case ActionCode::actionCodeAccessRightInsert: + case ActionCode::actionCodeAccessRightUpdate: + case ActionCode::actionCodeAccessRightUserInsert: + case ActionCode::actionCodeAccessRightUserUpdate: + case ActionCode::actionCodeAccessRightTeamInsert: + case ActionCode::actionCodeAccessRightTeamUpdate: + case ActionCode::actionCodeAccessRightMainUsersInsert: + case ActionCode::actionCodeAccessRightMainUsersUpdate: { bool hasRights = false; SyncTime createdAt = 0; SyncTime modtime = 0; @@ -624,9 +622,9 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, item.setSize(size); [[fallthrough]]; } - case actionCodeMoveIn: - case actionCodeRestore: - case actionCodeCreate: + case ActionCode::actionCodeMoveIn: + case ActionCode::actionCodeRestore: + case ActionCode::actionCodeCreate: _snapshot->updateItem(item); if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { @@ -647,21 +645,21 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, break; // Item renamed - case actionCodeRename: + case ActionCode::actionCodeRename: _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); _snapshot->updateItem(item); break; // Item edited - case actionCodeEdit: + case ActionCode::actionCodeEdit: _snapshot->updateItem(item); break; // Item removed - case actionCodeAccessRightRemove: - case actionCodeAccessRightUserRemove: - case actionCodeAccessRightTeamRemove: - case actionCodeAccessRightMainUsersRemove: { + case ActionCode::actionCodeAccessRightRemove: + case ActionCode::actionCodeAccessRightUserRemove: + case ActionCode::actionCodeAccessRightTeamRemove: + case ActionCode::actionCodeAccessRightMainUsersRemove: { bool hasRights = false; SyncTime createdAt = 0; SyncTime modtime = 0; @@ -676,11 +674,11 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, if (hasRights) break; // Current user still have the right to access this item, ignore action. [[fallthrough]]; } - case actionCodeMoveOut: + case ActionCode::actionCodeMoveOut: // Ignore move out action if destination is inside the synced folder. if (movedItems.find(actionInfo.nodeId) != movedItems.end()) break; [[fallthrough]]; - case actionCodeTrash: + case ActionCode::actionCodeTrash: _syncPal->removeItemFromTmpBlacklist(actionInfo.nodeId, ReplicaSideRemote); if (!_snapshot->removeItem(actionInfo.nodeId)) { LOGW_SYNCPAL_WARN(_logger, L"Fail to remove item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" @@ -691,12 +689,12 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, break; // Ignored actions - case actionCodeAccess: - case actionCodeDelete: - case actionCodeRestoreFileShareCreate: - case actionCodeRestoreFileShareDelete: - case actionCodeRestoreShareLinkCreate: - case actionCodeRestoreShareLinkDelete: + case ActionCode::actionCodeAccess: + case ActionCode::actionCodeDelete: + case ActionCode::actionCodeRestoreFileShareCreate: + case ActionCode::actionCodeRestoreFileShareDelete: + case ActionCode::actionCodeRestoreShareLinkCreate: + case ActionCode::actionCodeRestoreShareLinkDelete: // Ignore these actions break; From 996e03cbb372e563e23eb9d4df245dfba1da9ccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Kunz?= Date: Tue, 14 May 2024 11:24:58 +0200 Subject: [PATCH 09/12] KDESKTOP-823 - Apply PR suggestions --- src/libsyncengine/jobs/network/getfileinfojob.cpp | 2 +- .../file_system_observer/remotefilesystemobserverworker.cpp | 6 +++--- .../file_system_observer/snapshot/snapshot.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libsyncengine/jobs/network/getfileinfojob.cpp b/src/libsyncengine/jobs/network/getfileinfojob.cpp index 578413223..392e29c4c 100644 --- a/src/libsyncengine/jobs/network/getfileinfojob.cpp +++ b/src/libsyncengine/jobs/network/getfileinfojob.cpp @@ -55,7 +55,7 @@ bool GetFileInfoJob::handleResponse(std::istream &is) { if (!JsonParserUtility::extractValue(dataObj, typeKey, tmp)) { return false; } - bool isDir = tmp == dirKey; + const bool isDir = tmp == dirKey; if (!isDir) { if (!JsonParserUtility::extractValue(dataObj, sizeKey, _size)) { return false; diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 5cc8bb7f9..821bb4acb 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -359,7 +359,7 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons auto insertInfo = existingFiles.insert(Str2SyncName(item.parentId()) + item.name()); if (!insertInfo.second) { // Item with exact same name already exist in parent folder - LOG_SYNCPAL_DEBUG(Log::instance()->getLogger(), + LOGW_SYNCPAL_DEBUG(Log::instance()->getLogger(), L"Item \"" << SyncName2WStr(item.name()).c_str() << L"\" already exist in directory \"" << SyncName2WStr(_snapshot->name(item.parentId())).c_str() << L"\""); @@ -392,7 +392,7 @@ ExitCode RemoteFileSystemObserverWorker::getItemsInDir(const NodeId &dirId, cons auto nodeIdIt = nodeIds.begin(); while (nodeIdIt != nodeIds.end()) { if (_snapshot->isOrphan(*nodeIdIt)) { - LOG_SYNCPAL_DEBUG(_logger, L"Node " << SyncName2WStr(_snapshot->name(*nodeIdIt)).c_str() << L" (" + LOGW_SYNCPAL_DEBUG(_logger, L"Node '" << SyncName2WStr(_snapshot->name(*nodeIdIt)).c_str() << L"' (" << Utility::s2ws(*nodeIdIt).c_str() << L") is orphan. Removing it from " << Utility::s2ws(Utility::side2Str(_snapshot->side())).c_str() << L" snapshot."); _snapshot->removeItem(*nodeIdIt); @@ -699,7 +699,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, break; default: - LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on file: " << SyncName2WStr(actionInfo.name).c_str() << L" (" + LOGW_SYNCPAL_DEBUG(_logger, L"Unknown operation received on item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); } 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 b3958c6ed..bad6fbdaf 100644 --- a/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp @@ -86,7 +86,7 @@ bool Snapshot::updateItem(const SnapshotItem &newItem) { } if (ParametersCache::instance()->parameters().extendedLog()) { - LOGW_DEBUG(Log::instance()->getLogger(), L"File/directory: " << SyncName2WStr(newItem.name()).c_str() << L" (" + LOGW_DEBUG(Log::instance()->getLogger(), L"Item: " << SyncName2WStr(newItem.name()).c_str() << L" (" << Utility::s2ws(newItem.id()).c_str() << L") updated at:" << newItem.lastModified()); } From 68c03d8dba5b1fdb5a9c684e008cb78e901109eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Tue, 14 May 2024 15:48:12 +0200 Subject: [PATCH 10/12] KDESKTOP-823 - remove code duplication --- .../remotefilesystemobserverworker.cpp | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 5cc8bb7f9..27873b791 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -593,6 +593,18 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime, actionInfo.type, actionInfo.size, actionInfo.canWrite); + const auto hasRights = [=](const NodeId &nodeId, bool &rightsOk + , SyncTime &createdAt, SyncTime &modtime, int64_t &size) -> ExitCode { + ExitCode exitCode = ExitCodeOk; + if (exitCode = getFileInfo(nodeId, rightsOk, createdAt, modtime, size); exitCode != ExitCodeOk) { + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(actionInfo.name).c_str() << L" (" + << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); + invalidateSnapshot(); + } + return exitCode; + }; + // Process action switch (actionInfo.actionCode) { // Item added @@ -604,18 +616,15 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightTeamUpdate: case ActionCode::actionCodeAccessRightMainUsersInsert: case ActionCode::actionCodeAccessRightMainUsersUpdate: { - bool hasRights = false; + bool rightsOk = false; SyncTime createdAt = 0; SyncTime modtime = 0; int64_t size = 0; - if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - invalidateSnapshot(); - return ExitCodeBackError; + if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, createdAt, modtime, size) + ; exitCode != ExitCodeOk) { + return exitCode; } - if (!hasRights) break; // Current user does not have the right to access this item, ignore action. + if (!rightsOk) break; // Current user does not have the right to access this item, ignore action. item.setCreatedAt(createdAt); item.setLastModified(modtime); @@ -660,18 +669,15 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightUserRemove: case ActionCode::actionCodeAccessRightTeamRemove: case ActionCode::actionCodeAccessRightMainUsersRemove: { - bool hasRights = false; + bool rightsOk = false; SyncTime createdAt = 0; SyncTime modtime = 0; int64_t size = 0; - if (getFileInfo(actionInfo.nodeId, hasRights, createdAt, modtime, size) != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - invalidateSnapshot(); - return ExitCodeBackError; + if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, createdAt, modtime, size) + ; exitCode != ExitCodeOk) { + return exitCode; } - if (hasRights) break; // Current user still have the right to access this item, ignore action. + if (rightsOk) break; // Current user still have the right to access this item, ignore action. [[fallthrough]]; } case ActionCode::actionCodeMoveOut: From 2678b3facb8f9940f21043edc70ca7ea5d5ec78f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Wed, 15 May 2024 10:43:25 +0200 Subject: [PATCH 11/12] KDESKTOP-823 - Remove unused variables --- .../remotefilesystemobserverworker.cpp | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 428c7f1ca..94549b1fd 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -594,14 +594,20 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, actionInfo.type, actionInfo.size, actionInfo.canWrite); const auto hasRights = [=](const NodeId &nodeId, bool &rightsOk - , SyncTime &createdAt, SyncTime &modtime, int64_t &size) -> ExitCode { + , SnapshotItem &item) -> ExitCode { ExitCode exitCode = ExitCodeOk; + SyncTime createdAt = 0; + SyncTime modtime = 0; + int64_t size = 0; if (exitCode = getFileInfo(nodeId, rightsOk, createdAt, modtime, size); exitCode != ExitCodeOk) { LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " << SyncName2WStr(actionInfo.name).c_str() << L" (" << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); invalidateSnapshot(); } + item.setCreatedAt(createdAt); + item.setLastModified(modtime); + item.setSize(size); return exitCode; }; @@ -617,18 +623,11 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightMainUsersInsert: case ActionCode::actionCodeAccessRightMainUsersUpdate: { bool rightsOk = false; - SyncTime createdAt = 0; - SyncTime modtime = 0; - int64_t size = 0; - if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, createdAt, modtime, size) + if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, item) ; exitCode != ExitCodeOk) { return exitCode; } if (!rightsOk) break; // Current user does not have the right to access this item, ignore action. - - item.setCreatedAt(createdAt); - item.setLastModified(modtime); - item.setSize(size); [[fallthrough]]; } case ActionCode::actionCodeMoveIn: @@ -670,10 +669,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightTeamRemove: case ActionCode::actionCodeAccessRightMainUsersRemove: { bool rightsOk = false; - SyncTime createdAt = 0; - SyncTime modtime = 0; - int64_t size = 0; - if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, createdAt, modtime, size) + if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, item) ; exitCode != ExitCodeOk) { return exitCode; } From bb7f69e2489abe6870eef2f5e4c3903c905b4602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cle=CC=81ment=20Kunz?= Date: Wed, 15 May 2024 12:05:18 +0200 Subject: [PATCH 12/12] KDESKTOP-823 - Addresses Sonar Cloud's warnings --- .../remotefilesystemobserverworker.cpp | 38 +++++-------------- .../remotefilesystemobserverworker.h | 2 +- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp index 94549b1fd..ea0113e16 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp @@ -593,24 +593,6 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, SnapshotItem item(actionInfo.nodeId, actionInfo.parentNodeId, usedName, actionInfo.createdAt, actionInfo.modtime, actionInfo.type, actionInfo.size, actionInfo.canWrite); - const auto hasRights = [=](const NodeId &nodeId, bool &rightsOk - , SnapshotItem &item) -> ExitCode { - ExitCode exitCode = ExitCodeOk; - SyncTime createdAt = 0; - SyncTime modtime = 0; - int64_t size = 0; - if (exitCode = getFileInfo(nodeId, rightsOk, createdAt, modtime, size); exitCode != ExitCodeOk) { - LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " - << SyncName2WStr(actionInfo.name).c_str() << L" (" - << Utility::s2ws(actionInfo.nodeId).c_str() << L")"); - invalidateSnapshot(); - } - item.setCreatedAt(createdAt); - item.setLastModified(modtime); - item.setSize(size); - return exitCode; - }; - // Process action switch (actionInfo.actionCode) { // Item added @@ -623,8 +605,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightMainUsersInsert: case ActionCode::actionCodeAccessRightMainUsersUpdate: { bool rightsOk = false; - if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, item) - ; exitCode != ExitCodeOk) { + if (ExitCode exitCode = checkRightsAndUpdateItem(actionInfo.nodeId, rightsOk, item); exitCode != ExitCodeOk) { return exitCode; } if (!rightsOk) break; // Current user does not have the right to access this item, ignore action. @@ -634,7 +615,6 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeRestore: case ActionCode::actionCodeCreate: _snapshot->updateItem(item); - if (actionInfo.type == NodeTypeDirectory && actionInfo.actionCode != ActionCode::actionCodeCreate) { // Retrieve all children ExitCode exitCode = exploreDirectory(actionInfo.nodeId); @@ -669,8 +649,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, case ActionCode::actionCodeAccessRightTeamRemove: case ActionCode::actionCodeAccessRightMainUsersRemove: { bool rightsOk = false; - if (ExitCode exitCode = hasRights(actionInfo.nodeId, rightsOk, item) - ; exitCode != ExitCodeOk) { + if (ExitCode exitCode = checkRightsAndUpdateItem(actionInfo.nodeId, rightsOk, item); exitCode != ExitCodeOk) { return exitCode; } if (rightsOk) break; // Current user still have the right to access this item, ignore action. @@ -710,8 +689,7 @@ ExitCode RemoteFileSystemObserverWorker::processAction(const SyncName &usedName, return ExitCodeOk; } -ExitCode RemoteFileSystemObserverWorker::getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, - SyncTime &modtime, int64_t &size) { +ExitCode RemoteFileSystemObserverWorker::checkRightsAndUpdateItem(const NodeId &nodeId, bool &hasRights, SnapshotItem &snapshotItem) { GetFileInfoJob job(_syncPal->driveDbId(), nodeId); job.runSynchronously(); if (job.hasHttpError() || job.exitCode() != ExitCodeOk) { @@ -721,12 +699,16 @@ ExitCode RemoteFileSystemObserverWorker::getFileInfo(const NodeId &nodeId, bool return ExitCodeOk; } + LOGW_SYNCPAL_WARN(_logger, L"Error while determining access rights on item: " + << SyncName2WStr(snapshotItem.name()).c_str() << L" (" + << Utility::s2ws(snapshotItem.id()).c_str() << L")"); + invalidateSnapshot(); return ExitCodeBackError; } - createdAt = job.creationTime(); - modtime = job.modtime(); - size = job.size(); + snapshotItem.setCreatedAt(job.creationTime()); + snapshotItem.setLastModified(job.modtime()); + snapshotItem.setSize(job.size()); hasRights = true; return ExitCodeOk; } diff --git a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h index 25fbde9a5..56e77d738 100644 --- a/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h +++ b/src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.h @@ -62,7 +62,7 @@ class RemoteFileSystemObserverWorker : public FileSystemObserverWorker { ExitCode extractActionInfo(const Poco::JSON::Object::Ptr actionObj, ActionInfo &actionInfo); ExitCode processAction(const SyncName &usedName, const ActionInfo &actionInfo, std::set> &movedItems); - ExitCode getFileInfo(const NodeId &nodeId, bool &hasRights, SyncTime &createdAt, SyncTime &modtime, int64_t &size); + ExitCode checkRightsAndUpdateItem(const NodeId &nodeId, bool &hasRights, SnapshotItem &snapshotItem); bool hasUnsupportedCharacters(const SyncName &name, const NodeId &nodeId, NodeType type);