Fix failures when replacing folder nodes with file nodes

pbom/domain/__test__/pbonode_test.cpp

@@ -154,8 +154,8 @@ namespace pboman3::domain::test {
 
         QObject::connect(root.get(), &PboNode::childCreated, onChildCreated);
 
-        root->createHierarchy(PboPath("f1/e1"));
-        root->createHierarchy(PboPath("f1/e2"));
+        root->createHierarchy(PboPath("f1/e1"), ConflictResolution::Copy);
+        root->createHierarchy(PboPath("f1/e2"), ConflictResolution::Copy);
 
         ASSERT_EQ(count, 1);
     }
@@ -183,8 +183,8 @@ namespace pboman3::domain::test {
 
         QObject::connect(root.get(), &PboNode::childCreated, onChildCreated);
 
-        root->createHierarchy(PboPath("e1"));
-        root->createHierarchy(PboPath("e2"));
+        root->createHierarchy(PboPath("e1"), ConflictResolution::Copy);
+        root->createHierarchy(PboPath("e2"), ConflictResolution::Copy);
 
         ASSERT_EQ(count, 2);
     }

pbom/domain/pbonode.cpp

@@ -35,7 +35,7 @@ namespace pboman3::domain {
 
         PboNode* p = node->parentNode_;
         assert(p && "Must not be null");
-        p->removeNode(node->sharedFromThis());
+        p->removeNode(node->sharedFromThis(), true);
         p->emitHierarchyChanged();
     }
 
@@ -99,18 +99,18 @@ namespace pboman3::domain {
         while (it != last) {
             PboNode* folder = node->getChild(*it).get();
             if (!folder) {
-                folder = node->createNode(*it, PboNodeType::Folder);
+                folder = node->createNode(*it, PboNodeType::Folder, emitEvents);
             } else if (folder->nodeType_ == PboNodeType::File) {
                 switch (onConflict) {
                     case ConflictResolution::Replace: {
                         const auto p = folder->parentNode_;
-                        p->removeNode(folder->sharedFromThis());
-                        folder = p->createNode(*it, PboNodeType::Folder);
+                        p->removeNode(folder->sharedFromThis(), emitEvents);
+                        folder = p->createNode(*it, PboNodeType::Folder, emitEvents);
                         break;
                     }
                     case ConflictResolution::Copy: {
                         const QString folderTitle = node->pickFolderTitle(*it);
-                        folder = node->createNode(folderTitle, PboNodeType::Folder);
+                        folder = node->createNode(folderTitle, PboNodeType::Folder, emitEvents);
                         break;
                     }
                     default:
@@ -123,21 +123,21 @@ namespace pboman3::domain {
 
         PboNode* file = node->getChild(*last).get();
         if (!file) {
-            file = node->createNode(*last, PboNodeType::File);
+            file = node->createNode(*last, PboNodeType::File, emitEvents);
             if (emitEvents)
                 emitHierarchyChanged();
         } else {
             switch (onConflict) {
                 case ConflictResolution::Replace: {
-                    node->removeNode(file->sharedFromThis());
-                    file = node->createNode(*last, PboNodeType::File);
+                    node->removeNode(file->sharedFromThis(), emitEvents);
+                    file = node->createNode(*last, PboNodeType::File, emitEvents);
                     if (emitEvents)
                         emitHierarchyChanged();
                     break;
                 }
                 case ConflictResolution::Copy: {
                     const QString fileTitle = node->pickFileTitle(*last);
-                    file = node->createNode(fileTitle, PboNodeType::File);
+                    file = node->createNode(fileTitle, PboNodeType::File, emitEvents);
                     if (emitEvents)
                         emitHierarchyChanged();
                     break;
@@ -170,16 +170,20 @@ namespace pboman3::domain {
         return attemptTitle;
     }
 
-    PboNode* PboNode::createNode(const QString& title, PboNodeType nodeType) {
+    PboNode* PboNode::createNode(const QString& title, PboNodeType nodeType, bool emitEvents) {
         const QSharedPointer<PboNode> child(new PboNode(title, nodeType, this));
         const qsizetype insertedIndex = addChild(child);
-        emit childCreated(child.get(), insertedIndex);
+
+        if (emitEvents)
+            emit childCreated(child.get(), insertedIndex);
+
         return child.get();
     }
 
-    void PboNode::removeNode(const QSharedPointer<PboNode>& node) {
+    void PboNode::removeNode(const QSharedPointer<PboNode>& node, bool emitEvents) {
         const qsizetype index = removeChild(node);
-        emit childRemoved(index);
+        if (emitEvents)
+            emit childRemoved(index);
     }
 
     void PboNode::emitHierarchyChanged() {

pbom/domain/pbonode.h

@@ -61,9 +61,9 @@ namespace pboman3::domain {
 
         QString pickFileTitle(const QString& expectedTitle) const;
 
-        PboNode* createNode(const QString& title, PboNodeType nodeType);
+        PboNode* createNode(const QString& title, PboNodeType nodeType, bool emitEvents);
 
-        void removeNode(const QSharedPointer<PboNode>& node);
+        void removeNode(const QSharedPointer<PboNode>& node, bool emitEvents);
 
         void emitHierarchyChanged();
 

pbom/io/bb/__test__/tempbackend_test.cpp

@@ -83,7 +83,7 @@ namespace pboman3::io::test {
         ASSERT_EQ(QDir(dir.path()).entryList().count(), 0);
     }
 
-    TEST(TempBackendTest, Clear_Cleans_Up_If_Needed) {
+    TEST(TempBackendTest, Clear_Cleans_Up_File) {
         //dummy files
         QTemporaryFile f1;
         f1.open();
@@ -105,7 +105,33 @@ namespace pboman3::io::test {
         be.clear(e1);
 
         //check
-        ASSERT_FALSE(QFile::exists(sync.at(0).path()));
+        ASSERT_FALSE(QFile::exists(sync.at(0).toLocalFile()));
+    }
+
+    TEST(TempBackendTest, Clear_Cleans_Up_Folder) {
+        //dummy files
+        QTemporaryFile f1;
+        f1.open();
+        f1.write(QByteArray("some text data 1"));
+        f1.close();
+
+        //nodes to sync
+        const auto root = QSharedPointer<PboNode>::create(QString("root"), PboNodeType::Container, nullptr);
+        PboNode* e1 = root->createHierarchy(PboPath("folder1/file1.txt"));
+
+        e1->binarySource = QSharedPointer<BinarySource>(new FsRawBinarySource(f1.fileName()));
+
+        //the object tested
+        const QTemporaryDir dir;
+        const TempBackend be(QDir(dir.path()));
+        const QList<QUrl> sync = be.hddSync(QList({e1}), []() { return false; });
+
+        //call the method
+        const PboNode* fl1 = root->get(PboPath{"folder1"});
+        be.clear(fl1);
+
+        //check
+        ASSERT_FALSE(QFile::exists(sync.at(0).toLocalFile()));
     }
 
     TEST(TempBackendTest, Clear_Does_Not_Clean_Up) {

pbom/io/bb/binarybackend.cpp

@@ -49,7 +49,8 @@ namespace pboman3::io {
 
     void BinaryBackend::clear(const PboNode* node) const {
         tempBackend_->clear(node);
-        execBackend_->clear(node);
+        if (node->nodeType() == PboNodeType::File)
+            execBackend_->clear(node);
     }
 
     QDir BinaryBackend::getTempDir() const {

pbom/io/bb/tempbackend.cpp

@@ -29,15 +29,18 @@ namespace pboman3::io {
 
 
     void TempBackend::clear(const PboNode* node) const {
-        assert(node->nodeType() == PboNodeType::File);
-
         const QString path = nodeFileSystem_->composeAbsolutePath(node);
         if (QFileInfo(path).exists()) {
-            if (!QFile::remove(path)) {
+            if (node->nodeType() == PboNodeType::File && !QFile::remove(path)) {
                 throw DiskAccessException(
                     "Could not remove the file. Check you have enough permissions and the file is not locked by another process.",
                     path);
             }
+            if (node->nodeType() == PboNodeType::Folder && !QDir(path).removeRecursively()) {
+                throw DiskAccessException(
+                    "Could not remove the directory. Check you have enough permissions and the file is not locked by another process.",
+                    path);
+            }
         }
     }
 

pbom/model/pbomodel.cpp

@@ -89,9 +89,11 @@ namespace pboman3::model {
             LOG(debug, "The conflict resolution for the descriptor was set to:", static_cast<qint32>(resolution))
 
             if (resolution != ConflictResolution::Skip) {
+                const PboNode* existing = parent->get(descriptor.path());
+                binaryBackend_->clear(existing);
+
                 PboNode* created = parent->createHierarchy(descriptor.path(), resolution);
                 created->binarySource = descriptor.binarySource();
-                binaryBackend_->clear(created);
             }
         }
     }

pbom/ui/treewidget/__test__/treewidgetbase__test.cpp

@@ -66,18 +66,16 @@ namespace pboman3::ui::test {
     TEST(TreeWidgetBaseTest, GetSelectedHierarchies_Returns_Correct_Results_When_Multiple_Items_Selected) {
         QTAPP
 
-        //build the mock tree
-        const auto iconMgr = QSharedPointer<IconMgr>(new MockTreeWidgetBaseIconMgr);
-        const auto node = QScopedPointer(new PboNode("file.pbo", PboNodeType::Container, nullptr));
-        const auto item = new TreeWidgetItem(node.get(), iconMgr);
-
         //create tree nodes
+        const auto node = QSharedPointer<PboNode>::create(QString("file.pbo"), PboNodeType::Container, nullptr);
         node->createHierarchy(PboPath("e1/e2/f1.txt"));
         node->createHierarchy(PboPath("e1/e2/f2.txt"));
         node->createHierarchy(PboPath("e1/e3/f3.txt"));
         node->createHierarchy(PboPath("e1/e3/f4.txt"));
 
-        //att tree to the widget
+        //build the mock tree
+        const auto iconMgr = QSharedPointer<MockTreeWidgetBaseIconMgr>::create();
+        const auto item = new TreeWidgetItem(node.get(), iconMgr);
         MockTreeWidgetBase tree;
         tree.addTopLevelItem(item);
 
@@ -103,7 +101,7 @@ namespace pboman3::ui::test {
     TEST(TreeWidgetBaseTest, GetSelectionRoot_Returns_When_Single_Node_Selected) {
         QTAPP
 
-        const auto node = QScopedPointer(new PboNode("file.pbo", PboNodeType::Container, nullptr));
+        const auto node = QSharedPointer<PboNode>::create(QString("file.pbo"), PboNodeType::Container, nullptr);
         const auto item = new TreeWidgetItem(node.get());
 
         MockTreeWidgetBase tree;
@@ -117,16 +115,14 @@ namespace pboman3::ui::test {
     TEST(TreeWidgetBaseTest, GetSelectionRoot_Returns_Correct_Results_When_Multiple_Items_Selected) {
         QTAPP
 
-        //build the mock tree
-        const auto iconMgr = QSharedPointer<IconMgr>(new MockTreeWidgetBaseIconMgr);
-        const auto node = QScopedPointer(new PboNode("file.pbo", PboNodeType::Container, nullptr));
-        const auto item = new TreeWidgetItem(node.get(), iconMgr);
-
         //create tree nodes
+        const auto node = QSharedPointer<PboNode>::create(QString("file.pbo"), PboNodeType::Container, nullptr);
         node->createHierarchy(PboPath("e1/e2/f1.txt"));
         node->createHierarchy(PboPath("e1/e2/f2.txt"));
 
-        //att tree to the widget
+        //build the mock tree
+        const auto iconMgr = QSharedPointer<MockTreeWidgetBaseIconMgr>::create();
+        const auto item = new TreeWidgetItem(node.get(), iconMgr);
         MockTreeWidgetBase tree;
         tree.addTopLevelItem(item);
 

pbom/ui/treewidget/treewidget.cpp

@@ -367,13 +367,23 @@ namespace pboman3::ui {
             InsertDialog dialog(this, InsertDialog::Mode::InternalFiles, &descriptors, &conflicts);
             if (dialog.exec() == QDialog::DialogCode::Accepted) {
                 LOG(info, "The user accepted the conflicts dialog")
-                model_->createNodeSet(target, descriptors, conflicts);
-                delete_.commit();
+                try {
+                    model_->createNodeSet(target, descriptors, conflicts);
+                    delete_.commit();
+                } catch (const DiskAccessException& ex) {
+                    LOG(info, "Error when creating the node set - show error modal:", ex)
+                    UI_HANDLE_ERROR_RET(ex)
+                }
             }
         } else {
             LOG(info, "There were no conflicts - adding the nodes as is")
-            model_->createNodeSet(target, descriptors, conflicts);
-            delete_.commit();
+            try {
+                model_->createNodeSet(target, descriptors, conflicts);
+                delete_.commit();
+            } catch (const DiskAccessException& ex) {
+                LOG(info, "Error when creating the node set - show error modal:", ex)
+                UI_HANDLE_ERROR_RET(ex)
+            }
         }
     }
 
@@ -412,6 +422,11 @@ namespace pboman3::ui {
             UI_HANDLE_ERROR_RET(ex)
         }
 
+        if (parcel.files->empty()) {
+            LOG(debug, "Collected no files - aborting")
+            return;
+        }
+
         LOG(debug, "Collected descriptors:", parcel.files)
         LOG(info, "Selected node is:", *parcel.target)
 
@@ -421,7 +436,12 @@ namespace pboman3::ui {
         InsertDialog dialog(this, InsertDialog::Mode::ExternalFiles, parcel.files.get(), &conflicts);
         if (dialog.exec() == QDialog::DialogCode::Accepted) {
             LOG(info, "The user accepted the file insert dialog")
-            model_->createNodeSet(parcel.target, *parcel.files, conflicts);
+            try {
+                model_->createNodeSet(parcel.target, *parcel.files, conflicts);
+            } catch (const DiskAccessException& ex) {
+                LOG(info, "Error when creating the node set - show error modal:", ex)
+                UI_HANDLE_ERROR_RET(ex)
+            }
         }
     }
 }