Kdesktop 772 mode delete conflict resolution on directory is broken

src/libsyncengine/propagation/executor/executorworker.cpp

@@ -1240,7 +1240,7 @@ bool ExecutorWorker::generateMoveJob(SyncOpPtr syncOp) {
     job->runSynchronously();
 
     if (job->exitCode() == ExitCodeOk && syncOp->conflict().type() != ConflictTypeNone) {
-        // Conflict fixing job finished succesfully
+        // Conflict fixing job finished successfully
         // Propagate changes to DB and update trees
         std::shared_ptr<Node> newNode = nullptr;
         if (!propagateChangeToDbAndTree(syncOp, job, newNode)) {
@@ -2344,7 +2344,7 @@ bool ExecutorWorker::deleteFromDb(std::shared_ptr<Node> node) {
         return false;
     }
 
-    // Remove item (and childs by cascade) from DB
+    // Remove item (and children by cascade) from DB
     bool found = false;
     if (!_syncPal->_syncDb->deleteNode(*node->idb(), found)) {
         LOG_SYNCPAL_WARN(_logger, "Failed to remove node " << *node->idb() << " from DB");

src/libsyncengine/reconciliation/conflict_finder/conflictfinderworker.cpp

@@ -44,7 +44,7 @@ void ConflictFinderWorker::findConflicts() {
     std::vector<std::shared_ptr<Node>> localMoveDirNodes;
     findConflictsInTree(_syncPal->_localUpdateTree, _syncPal->_remoteUpdateTree, localMoveDirNodes, remoteMoveDirNodes);
 
-    // Move Move Cycle
+    // Move-Move Cycle
     std::optional<std::vector<Conflict>> moveMoveCycleList =
         determineMoveMoveCycleConflicts(localMoveDirNodes, remoteMoveDirNodes);
     if (moveMoveCycleList) {
@@ -222,7 +222,7 @@ void ConflictFinderWorker::findConflictsInTree(std::shared_ptr<UpdateTree> local
         // remove it from queue
         queue.pop();
 
-        // visit childrens
+        // visit children
         for (auto &childElt : node->children()) {
             auto child = childElt.second;
             if (!visited[child]) {
@@ -448,7 +448,7 @@ std::optional<std::vector<std::shared_ptr<Node>>> ConflictFinderWorker::findChan
         // remove it from queue
         queue.pop_front();
 
-        // visit childrens
+        // visit children
         for (auto child : node->children()) {
             if (!visited[child.second]) {
                 visited[child.second] = true;

src/libsyncengine/syncpal/isyncworker.cpp

@@ -155,7 +155,7 @@ void ISyncWorker::setDone(ExitCode code) {
     LOG_SYNCPAL_DEBUG(_logger, "Worker " << _name.c_str() << " has finished with code=" << code << " and cause=" << _exitCause);
 
     if (code != ExitCodeOk) {
-        _syncPal->addError(Error(_syncPal->syncDbId(), _shortName, code, _exitCause));
+        _syncPal->addError(Error(_syncPal->syncDbId(), _shortName, code, _exitCause));  // TODO : we should not have syncpal as attribute just to add an error. The caller should add the error.
     }
 
     _isRunning = false;

src/libsyncengine/update_detection/update_detector/updatetree.cpp

@@ -49,7 +49,7 @@ void UpdateTree::deleteNode(std::shared_ptr<Node> node) {
         return;
     }
 
-    // Recusively remove all children
+    // Recursively remove all children
     while (node->children().size() > 0) {
         deleteNode((node->children().begin())->second);
     }

src/libsyncengine/update_detection/update_detector/updatetreeworker.h

@@ -111,13 +111,24 @@ class UpdateTreeWorker : public ISyncWorker {
 
         ExitCode getNewPathAfterMove(const SyncPath &path, SyncPath &newPath);
         ExitCode updateNodeWithDb(const std::shared_ptr<Node> parentNode);
+        ExitCode updateTmpNode(const std::shared_ptr<Node> tmpNode);
         ExitCode getOriginPath(const std::shared_ptr<Node> node, SyncPath &path);
         ExitCode updateNameFromDbForMoveOp(const std::shared_ptr<Node> node, FSOpPtr moveOp);
         std::shared_ptr<Node> getOrCreateNodeFromPath(const SyncPath &path);
         void mergingTempNodeToRealNode(std::shared_ptr<Node> tmpNode, std::shared_ptr<Node> realNode);
 
+        /**
+         * Check that there is no temporary node remaining in the update tree
+         * @return true if no temporary node is found
+         */
         bool integrityCheck();
 
+        /**
+         * Draw the update tree in the log file for debugging purpose
+         */
+        void drawUpdateTree();
+        void drawUpdateTreeRow(const std::shared_ptr<Node> node, SyncName &treeStr, uint64_t depth = 0);
+
         friend class TestUpdateTreeWorker;
 };
 

test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.cpp

@@ -346,7 +346,7 @@ void TestConflictFinderWorker::testEditDelete() {
     CPPUNIT_ASSERT(confTest->type() == ConflictType::ConflictTypeEditDelete);
 }
 
-void TestConflictFinderWorker::testMoveDelete() {
+void TestConflictFinderWorker::testMoveDeleteFile() {
     setUpTreesAndDb();
     _syncPal->_localUpdateTree->getNodeByPath("Dir 4/Dir 4.1/Dir 4.1.1/File 4.1.1.1")->setChangeEvents(OperationTypeMove);
     _syncPal->_remoteUpdateTree->getNodeByPath("Dir 4/Dir 4.1/Dir 4.1.1/File 4.1.1.1")->setChangeEvents(OperationTypeDelete);
@@ -359,6 +359,11 @@ void TestConflictFinderWorker::testMoveDelete() {
     CPPUNIT_ASSERT(confTest->type() == ConflictType::ConflictTypeMoveDelete);
 }
 
+void TestConflictFinderWorker::testMoveDeleteDir() {
+    // TODO : to be implemented but this class needs to be refactored first.
+    // We shouldn't have to setup a SyncPal completely to test one worker.
+}
+
 void TestConflictFinderWorker::testMoveParentDelete() {
     setUpTreesAndDb();
     _syncPal->_localUpdateTree->getNodeByPath("Dir 4/Dir 4.1/Dir 4.1.1")->setChangeEvents(OperationTypeDelete);
@@ -563,6 +568,7 @@ void TestConflictFinderWorker::testCase55c() {
     _syncPal->_conflictQueue->pop();
     CPPUNIT_ASSERT(_syncPal->_conflictQueue->top().type() == ConflictTypeMoveCreate);
 }
+
 /* Move-ParentDelete > Move-Move (Source) */
 void TestConflictFinderWorker::testCase57() {
     // cf p96 figure 5.7 (b)

test/libsyncengine/reconciliation/conflict_finder/testconflictfinderworker.h

@@ -29,7 +29,8 @@ class TestConflictFinderWorker : public CppUnit::TestFixture {
         CPPUNIT_TEST(testEditEdit);
         CPPUNIT_TEST(testMoveCreate);
         CPPUNIT_TEST(testEditDelete);
-        CPPUNIT_TEST(testMoveDelete);
+        CPPUNIT_TEST(testMoveDeleteFile);
+        CPPUNIT_TEST(testMoveDeleteDir);
         CPPUNIT_TEST(testMoveParentDelete);
         CPPUNIT_TEST(testCreateParentDelete);
         CPPUNIT_TEST(testMoveMoveSrc);
@@ -55,7 +56,8 @@ class TestConflictFinderWorker : public CppUnit::TestFixture {
         void testEditEdit();
         void testMoveCreate();
         void testEditDelete();
-        void testMoveDelete();
+        void testMoveDeleteFile();
+        void testMoveDeleteDir();
         void testMoveParentDelete();
         void testCreateParentDelete();
         void testMoveMoveSrc();