Merge branch 'develop' into KDESKTOP-963-Error-Handling-for-Move-to-T…

…rash-Operation-on-Windows

src/libcommon/utility/utility.cpp

@@ -872,6 +872,13 @@ void CommonUtility::clearSignalFile(const AppType appType, const SignalCategory
     }
 }
 
+#ifdef _WIN32
+std::string CommonUtility::toUnsafeStr(const SyncName &name) {
+    std::string unsafeName(name.begin(), name.end());
+    return unsafeName;
+}
+#endif
+
 #ifdef __APPLE__
 bool CommonUtility::isLiteSyncExtEnabled() {
     QProcess *process = new QProcess();

src/libcommon/utility/utility.h

@@ -126,7 +126,11 @@ struct COMMON_EXPORT CommonUtility {
             return utility_base::isLikeFileNotFoundError(ec);
         };
 
+
 #ifdef _WIN32
+        // Converts a std::wstring to std::string assuming that it contains only mono byte chars
+        static std::string toUnsafeStr(const SyncName &name);
+
         static std::wstring getErrorMessage(DWORD errorMessageId) { return utility_base::getErrorMessage(errorMessageId); }
         static std::wstring getLastErrorMessage() { return utility_base::getLastErrorMessage(); };
         static bool isLikeFileNotFoundError(DWORD dwError) noexcept { return utility_base::isLikeFileNotFoundError(dwError); };

src/libcommongui/logger.cpp

@@ -47,14 +47,19 @@ static void kdriveLogCatcher(QtMsgType type, const QMessageLogContext &ctx, cons
     if (qtMsgTypeLevel[type] < logger->minLogLevel()) return;
 
     // Create new context
-    const char *fileName = ctx.file;
-    if (fileName) {
-        const char *lastDirSep = strrchr(fileName, '/');
-        if (lastDirSep) {
-            fileName = lastDirSep + 1;
-        }
+    SyncName fileName;
+    if (ctx.file) {
+        const SyncPath filePath(ctx.file);
+        fileName = filePath.filename();
     }
-    QMessageLogContext ctxNew(fileName, ctx.line, ctx.function, ctx.category);
+#ifdef _WIN32
+    // For performance purposes, assume that the file name contains only mono byte chars
+    std::string unsafeFileName(CommonUtility::toUnsafeStr(fileName));
+    const char *fileNamePtr = unsafeFileName.c_str();
+#else
+    const char *fileNamePtr = fileName.c_str();
+#endif
+    QMessageLogContext ctxNew(fileNamePtr, ctx.line, ctx.function, ctx.category);
 
     if (!logger->isNoop()) {
         logger->doLog(qFormatLogMessage(type, ctxNew, message));

src/libcommonserver/db/db.h

@@ -64,7 +64,7 @@ class COMMONSERVER_EXPORT Db {
         int extendedErrorCode() const;
 
         bool init(const std::string &version);
-        virtual std::string dbType() const { return "Unknown"; };
+        virtual std::string dbType() const { return "Unknown"; }
 
         virtual bool create(bool &retry) = 0;
         virtual bool prepare() = 0;

src/libsyncengine/jobs/network/API_v2/downloadjob.cpp

@@ -437,7 +437,7 @@ bool DownloadJob::createLink(const std::string &mimeType, const std::string &dat
 
         LOGW_DEBUG(_logger,
                    L"Create symlink with target " << Utility::formatSyncPath(targetPath) << L", " << Utility::formatSyncPath(_localpath));
-
+      
         bool isFolder = mimeType == mimeTypeSymlinkFolder;
         IoError ioError = IoError::Success;
         if (!IoHelper::createSymlink(targetPath, _localpath, isFolder, ioError)) {
@@ -479,7 +479,6 @@ bool DownloadJob::createLink(const std::string &mimeType, const std::string &dat
 
         IoError ioError = IoError::Success;
         if (!IoHelper::createAlias(data, _localpath, ioError)) {
-            const std::wstring message = Utility::s2ws(IoHelper::ioError2StdString(ioError));
             LOGW_WARN(_logger, L"Failed to create alias: " << Utility::formatIoError(_localpath, ioError));
 
             return false;

src/libsyncengine/propagation/executor/executorworker.h

@@ -162,7 +162,10 @@ class ExecutorWorker : public OperationProcessor {
         TerminatedJobsQueue _terminatedJobs;
         std::unordered_map<UniqueId, SyncOpPtr> _jobToSyncOpMap;
         std::unordered_map<UniqueId, UniqueId> _syncOpToJobMap;
+
         std::list<UniqueId> _opList;
+        std::recursive_mutex _opListMutex;
+
         std::chrono::steady_clock::time_point _fileProgressTimer = std::chrono::steady_clock::now();
 
         bool _snapshotToInvalidate = false;

src/libsyncengine/reconciliation/syncoperation.h

@@ -43,6 +43,15 @@ class SyncOperation {
         void setOmit(bool newOmit) { _omit = newOmit; }
         [[nodiscard]] const SyncName &newName() const { return _newName; }
         void setNewName(const SyncName &newNewName) { _newName = newNewName; }
+        [[nodiscard]] const SyncPath &localCreationTargetPath() const {
+            assert(_type == OperationType::Create && _targetSide == ReplicaSide::Local);
+            assert(!_localCreationTargetPath.empty());
+            return _localCreationTargetPath;
+        }
+        void setLocalCreationTargetPath(const SyncPath &localCreationTargetPath) {
+            assert(_type == OperationType::Create && _targetSide == ReplicaSide::Local);
+            _localCreationTargetPath = localCreationTargetPath;
+        }
         [[nodiscard]] const std::shared_ptr<Node> &newParentNode() const { return _newParentNode; }
         void setNewParentNode(const std::shared_ptr<Node> &newParentNode) { _newParentNode = newParentNode; }
         [[nodiscard]] bool hasConflict() const { return _conflict.type() != ConflictType::None; }
@@ -67,6 +76,7 @@ class SyncOperation {
         ReplicaSide _targetSide = ReplicaSide::Unknown; // The side on which we will apply the operation
         bool _omit = false; // If true, apply change only in DB
         SyncName _newName; // New name on the replica on which we will apply the operation. Only for create and move operation
+        SyncPath _localCreationTargetPath; // Relative path of the item to create. Only for local create operation.
         std::shared_ptr<Node> _newParentNode =
                 nullptr; // New parent on the replica on which we will apply the operation. Only for move operation
         Conflict _conflict;

src/libsyncengine/requests/serverrequests.cpp

@@ -1496,7 +1496,7 @@ ExitCode ServerRequests::addSync(int driveDbId, const QString &localFolderPath,
     sync.setLocalPath(QStr2Path(localFolderPath));
     sync.setTargetPath(QStr2Path(serverFolderPath));
     sync.setTargetNodeId(serverFolderNodeId.toStdString());
-    sync.setPaused(true);
+    sync.setPaused(false);
 
     // Check vfs support
     QString fsName(KDC::CommonUtility::fileSystemName(SyncName2QStr(sync.localPath().native())));

src/libsyncengine/syncpal/syncpal.cpp

@@ -58,7 +58,7 @@ namespace KDC {
 SyncPal::SyncPal(const SyncPath &syncDbPath, const std::string &version, const bool hasFullyCompleted) :
     _logger(Log::instance()->getLogger()) {
     _syncInfo.syncHasFullyCompleted = hasFullyCompleted;
-    LOGW_SYNCPAL_DEBUG(_logger, L"SyncPal init : " << Utility::formatSyncPath(syncDbPath).c_str());
+    LOGW_SYNCPAL_DEBUG(_logger, L"SyncPal init: " << Utility::formatSyncPath(syncDbPath));
 
     if (!createOrOpenDb(syncDbPath, version)) {
         throw std::runtime_error(SYNCPAL_NEW_ERROR_MSG);
@@ -268,7 +268,7 @@ ExitCode SyncPal::fileSyncing(ReplicaSide side, const SyncPath &path, bool &sync
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table : " << Utility::formatSyncPath(path).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table: " << Utility::formatSyncPath(path));
         return ExitCode::DataError;
     }
 
@@ -282,7 +282,7 @@ ExitCode SyncPal::setFileSyncing(ReplicaSide side, const SyncPath &path, bool sy
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table : " << Utility::formatSyncPath(path).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table: " << Utility::formatSyncPath(path));
         return ExitCode::DataError;
     }
 
@@ -297,7 +297,7 @@ ExitCode SyncPal::path(ReplicaSide side, const NodeId &nodeId, SyncPath &path) {
     }
 
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table : " << Utility::formatSyncPath(path).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table: " << Utility::formatSyncPath(path));
         return ExitCode::DataError;
     }
 
@@ -422,8 +422,7 @@ bool SyncPal::vfsUpdateMetadata(const SyncPath &path, const SyncTime &creationTi
 bool SyncPal::vfsUpdateFetchStatus(const SyncPath &tmpPath, const SyncPath &path, int64_t received, bool &canceled,
                                    bool &finished) {
     if (ParametersCache::isExtendedLogEnabled()) {
-        LOGW_SYNCPAL_DEBUG(_logger,
-                           L"vfsUpdateFetchStatus : " << Utility::formatSyncPath(path).c_str() << L" received=" << received);
+        LOGW_SYNCPAL_DEBUG(_logger, L"vfsUpdateFetchStatus: " << Utility::formatSyncPath(path) << L" received=" << received);
     }
 
     if (!_vfsUpdateFetchStatus) {
@@ -702,7 +701,7 @@ bool SyncPal::setProgress(const SyncPath &relativePath, int64_t current) {
             return false;
         }
         if (item.instruction() != SyncFileInstruction::Get && item.instruction() != SyncFileInstruction::Put) {
-            LOGW_SYNCPAL_WARN(_logger, L"Node not found : " << Utility::formatSyncPath(relativePath).c_str());
+            LOGW_SYNCPAL_WARN(_logger, L"Node not found: " << Utility::formatSyncPath(relativePath));
             return false;
         }
     }
@@ -724,7 +723,7 @@ bool SyncPal::setProgressComplete(const SyncPath &relativeLocalPath, SyncFileSta
     }
     if (!found) {
         // Can happen for a dehydrated placeholder
-        LOGW_SYNCPAL_DEBUG(_logger, L"Node not found : " << Utility::formatSyncPath(relativeLocalPath).c_str());
+        LOGW_SYNCPAL_DEBUG(_logger, L"Node not found: " << Utility::formatSyncPath(relativeLocalPath));
     }
     return true;
 }
@@ -770,7 +769,7 @@ ExitCode SyncPal::addDlDirectJob(const SyncPath &relativePath, const SyncPath &l
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table : " << Utility::formatSyncPath(relativePath).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table: " << Utility::formatSyncPath(relativePath));
         return ExitCode::DataError;
     }
 
@@ -780,7 +779,7 @@ ExitCode SyncPal::addDlDirectJob(const SyncPath &relativePath, const SyncPath &l
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table for localNodeId=" << Utility::s2ws(*localNodeId).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table for localNodeId=" << Utility::s2ws(*localNodeId));
         return ExitCode::DataError;
     }
 
@@ -790,7 +789,7 @@ ExitCode SyncPal::addDlDirectJob(const SyncPath &relativePath, const SyncPath &l
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table for localNodeId=" << Utility::s2ws(*localNodeId).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table for localNodeId=" << Utility::s2ws(*localNodeId));
         return ExitCode::DataError;
     }
 
@@ -1033,7 +1032,7 @@ ExitCode SyncPal::fileRemoteIdFromLocalPath(const SyncPath &path, NodeId &nodeId
         return ExitCode::DbError;
     }
     if (!found) {
-        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table : " << Utility::formatSyncPath(path).c_str());
+        LOGW_SYNCPAL_WARN(_logger, L"Node not found in node table: " << Utility::formatSyncPath(path));
         return ExitCode::DataError;
     }
 
@@ -1167,8 +1166,8 @@ ExitCode SyncPal::fixCorruptedFile(const std::unordered_map<NodeId, SyncPath> &l
         if (ExitCode exitCode = PlatformInconsistencyCheckerUtility::renameLocalFile(
                     localFileInfo.second, PlatformInconsistencyCheckerUtility::SuffixType::Conflict, &destPath);
             exitCode != ExitCode::Ok) {
-            LOGW_SYNCPAL_WARN(_logger, L"Fail to rename " << Path2WStr(localFileInfo.second).c_str() << L" into "
-                                                          << Path2WStr(destPath).c_str());
+            LOGW_SYNCPAL_WARN(_logger, L"Fail to rename " << Utility::formatSyncPath(localFileInfo.second) << L" into "
+                                                          << Utility::formatSyncPath(destPath));
 
             return exitCode;
         }
@@ -1460,24 +1459,26 @@ void SyncPal::removeItemFromTmpBlacklist(const SyncPath &relativePath) {
     _tmpBlacklistManager->removeItemFromTmpBlacklist(relativePath);
 }
 
-ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause) {
+
+ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativeLocalPath, ExitCause cause) {
     std::shared_ptr<Node> dummyNodePtr;
-    return handleAccessDeniedItem(relativePath, dummyNodePtr, dummyNodePtr, cause);
+    return handleAccessDeniedItem(relativeLocalPath, dummyNodePtr, dummyNodePtr, cause);
 }
 
-ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, std::shared_ptr<Node> &localBlacklistedNode,
+ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativeLocalPath, std::shared_ptr<Node> &localBlacklistedNode,
                                          std::shared_ptr<Node> &remoteBlacklistedNode, ExitCause cause) {
-    if (relativePath.empty()) {
+    if (relativeLocalPath.empty()) {
         LOG_SYNCPAL_WARN(_logger, "Access error on root folder");
         return ExitInfo(ExitCode::SystemError, ExitCause::SyncDirAccesError);
     }
-    Error error(syncDbId(), "", "", relativePath.extension() == SyncPath() ? NodeType::Directory : NodeType::File, relativePath,
-                ConflictType::None, InconsistencyType::None, CancelType::None, "", ExitCode::SystemError, cause);
+    Error error(syncDbId(), "", "", relativeLocalPath.extension() == SyncPath() ? NodeType::Directory : NodeType::File,
+                relativeLocalPath, ConflictType::None, InconsistencyType::None, CancelType::None, "", ExitCode::SystemError,
+                cause);
     addError(error);
 
-    NodeId localNodeId = snapshot(ReplicaSide::Local)->itemId(relativePath);
+    NodeId localNodeId = snapshot(ReplicaSide::Local)->itemId(relativeLocalPath);
     if (localNodeId.empty()) {
-        SyncPath absolutePath = localPath() / relativePath;
+        SyncPath absolutePath = localPath() / relativeLocalPath;
         if (!IoHelper::getNodeId(absolutePath, localNodeId)) {
             bool exists = false;
             IoError ioError = IoError::Success;
@@ -1486,18 +1487,19 @@ ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, std::shar
                 return ExitCode::SystemError;
             }
             if (ioError == IoError::AccessDenied) { // A parent of the file does not have sufficient right
-                LOGW_DEBUG(_logger, L"A parent of " << Utility::formatSyncPath(relativePath)
+                LOGW_DEBUG(_logger, L"A parent of " << Utility::formatSyncPath(relativeLocalPath)
                                                     << L"does not have sufficient right, blacklisting the parent item.");
-                return handleAccessDeniedItem(relativePath.parent_path(), localBlacklistedNode, remoteBlacklistedNode, cause);
+                return handleAccessDeniedItem(relativeLocalPath.parent_path(), localBlacklistedNode, remoteBlacklistedNode,
+                                              cause);
             }
         }
     }
 
-
-    LOGW_SYNCPAL_DEBUG(_logger, L"Item " << Utility::formatSyncPath(relativePath) << L" (NodeId: " << Utility::s2ws(localNodeId)
+    LOGW_SYNCPAL_DEBUG(_logger, L"Item " << Utility::formatSyncPath(relativeLocalPath) << L" (NodeId: "
+                                         << Utility::s2ws(localNodeId)
                                          << L" is blacklisted temporarily because of a denied access.");
 
-    NodeId remoteNodeId = snapshot(ReplicaSide::Remote)->itemId(relativePath);
+    NodeId remoteNodeId = snapshot(ReplicaSide::Remote)->itemId(relativeLocalPath);
     if (bool found; remoteNodeId.empty() && !localNodeId.empty() &&
                     !_syncDb->correspondingNodeId(ReplicaSide::Local, localNodeId, remoteNodeId, found)) {
         LOG_SYNCPAL_WARN(_logger, "Error in SyncDb::correspondingNodeId");
@@ -1506,19 +1508,19 @@ ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, std::shar
 
     localBlacklistedNode = updateTree(ReplicaSide::Local)->getNodeById(localNodeId);
     remoteBlacklistedNode = updateTree(ReplicaSide::Remote)->getNodeById(remoteNodeId);
-    
+
     // Blacklist the item
     if (!localNodeId.empty()) {
-        _tmpBlacklistManager->blacklistItem(localNodeId, relativePath, ReplicaSide::Local);
+        _tmpBlacklistManager->blacklistItem(localNodeId, relativeLocalPath, ReplicaSide::Local);
         if (!updateTree(ReplicaSide::Local)->deleteNode(localNodeId)) {
-            LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: " << Utility::formatSyncPath(relativePath));
+            LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: " << Utility::formatSyncPath(relativeLocalPath));
         }
     }
 
     if (!remoteNodeId.empty()) {
-        _tmpBlacklistManager->blacklistItem(remoteNodeId, relativePath, ReplicaSide::Remote);
+        _tmpBlacklistManager->blacklistItem(remoteNodeId, relativeLocalPath, ReplicaSide::Remote);
         if (!updateTree(ReplicaSide::Remote)->deleteNode(remoteNodeId)) {
-            LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: " << Utility::formatSyncPath(relativePath));
+            LOGW_SYNCPAL_WARN(_logger, L"Error in UpdateTree::deleteNode: " << Utility::formatSyncPath(relativeLocalPath));
         }
     }
 

src/libsyncengine/syncpal/syncpal.h

@@ -257,15 +257,22 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this<SyncPal> {
         virtual void refreshTmpBlacklist();
         virtual void removeItemFromTmpBlacklist(const NodeId &nodeId, ReplicaSide side);
         virtual void removeItemFromTmpBlacklist(const SyncPath &relativePath);
-        ExitInfo handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause = ExitCause::FileAccessError);
-        ExitInfo handleAccessDeniedItem(const SyncPath &relativePath, std::shared_ptr<Node> &localBlacklistedNode,
-                                        std::shared_ptr<Node> &remoteBlacklistedNode,
-                                        ExitCause cause);
+        //! Handle an access denied error on an item on the local side.
+        /*!
+         \param relativeLocalPath is the local path of the item.
+         \param cause is an optional exit cause returned by the operation that has failed. Used only to generate an error message
+         tailored to the context.
+         \return The exit info of the function.
+         */
+        ExitInfo handleAccessDeniedItem(const SyncPath &relativeLocalPath, ExitCause cause = ExitCause::FileAccessError);
+        ExitInfo handleAccessDeniedItem(const SyncPath &relativeLocalPath, std::shared_ptr<Node> &localBlacklistedNode,
+                                        std::shared_ptr<Node> &remoteBlacklistedNode, ExitCause cause);
+
         //! Makes copies of real-time snapshots to be used by synchronization workers.
         void copySnapshots();
 
         // Workers
-        std::shared_ptr<ComputeFSOperationWorker> computeFSOperationsWorker() const { return _computeFSOperationsWorker; };
+        std::shared_ptr<ComputeFSOperationWorker> computeFSOperationsWorker() const { return _computeFSOperationsWorker; }
         void setComputeFSOperationsWorker(std::shared_ptr<ComputeFSOperationWorker> worker) {
             _computeFSOperationsWorker = worker;
         }

src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp

@@ -169,9 +169,10 @@ ExitCode ComputeFSOperationWorker::inferChangeFromDbNode(const ReplicaSide side,
             // Check that the file/directory really does not exist on replica
             bool isExcluded = false;
             if (const ExitInfo exitInfo = checkIfOkToDelete(side, dbPath, nodeId, isExcluded); !exitInfo) {
+                // Can happen only on local side
                 if (exitInfo == ExitInfo(ExitCode::SystemError, ExitCause::FileAccessError)) {
                     // Blacklist node
-                    if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(dbPath); !exitInfo) {
+                    if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(localDbPath); !exitInfo) {
                         return exitInfo;
                     }