[multimedia/k3b] /: Fix replacement of files from the old session

Albert Astals Cid <[email protected]>
Newsgroups gmane.comp.kde.cvs
Message-ID <[email protected]>
Git commit a8d6dc3c8561e568f0c49c2f2afd86aeac2cbfd8 by Albert Astals Cid, on behalf of Ole Osterhagen.
Committed on 10/08/2026 at 22:15.
Pushed by aacid into branch 'master'.

Fix replacement of files from the old session

When replacing files from an imported session, remove these files before
adding new files so that the call to beginInsertItems/endInsertItems is
not interleaved with calls to beginRemoveItems/endRemoveItems.

When restoring a file from an imported session, remove the reference
from the imported file to the replacement, because the replacement will
soon be deleted.

BUG: 341892

M  +37   -20   libk3b/projects/datacd/k3bdiritem.cpp
M  +1    -0    tests/CMakeLists.txt
M  +88   -2    tests/k3bdataprojectmodeltest.cpp
M  +9    -2    tests/k3bdataprojectmodeltest.h
A  +-    --    tests/testdata/test.iso

https://invent.kde.org/multimedia/k3b/-/commit/a8d6dc3c8561e568f0c49c2f2afd86aeac2cbfd8

diff --git a/libk3b/projects/datacd/k3bdiritem.cpp b/libk3b/projects/datacd/k3bdiritem.cpp
index c06f2fca7..f8a250c10 100644
--- a/libk3b/projects/datacd/k3bdiritem.cpp
+++ b/libk3b/projects/datacd/k3bdiritem.cpp
@@ -17,6 +17,28 @@
 #include <QMimeDatabase>
 
 
+namespace {
+    void handleReplacement( const K3b::DirItem* parent, K3b::DataItem* item )
+    {
+        if( !item->isFile() ) {
+            return;
+        }
+
+        if( K3b::DataItem* oldItem = parent->find( item->k3bName() );
+            oldItem != nullptr &&
+            !oldItem->isDir() &&
+            oldItem->isFromOldSession() )
+        {
+            // in this case we remove this item from it's parent and save it in the new one
+            // to be able to recover it
+            oldItem->take();
+            static_cast<K3b::SessionImportItem*>(oldItem)->setReplaceItem( static_cast<K3b::FileItem*>(item) );
+            static_cast<K3b::FileItem*>(item)->setReplacedItemFromOldSession( oldItem );
+        }
+    }
+} // namespace
+
+
 K3b::DirItem::DirItem(const QString& name, const ItemFlags& flags)
     : K3b::DataItem( flags | DIR ),
       m_size(0),
@@ -88,6 +110,8 @@ K3b::DirItem* K3b::DirItem::addDataItem( K3b::DataItem* item )
         // in DataProjectModel
         item->take();
 
+        handleReplacement( this, item );
+
         // inform the doc
         if( DataDoc* doc = getDoc() ) {
             doc->beginInsertItems( this, m_children.size(), m_children.size() );
@@ -116,6 +140,8 @@ void K3b::DirItem::addDataItems( const Children& items )
             // in DataProjectModel
             item->take();
 
+            handleReplacement( this, item );
+
             newItems.push_back( item );
         }
     }
@@ -204,8 +230,10 @@ K3b::DirItem::Children K3b::DirItem::takeDataItems( int start, int count )
         Q_FOREACH( DataItem* item, takenItems ) {
             if( item->isFile() ) {
                 // restore the item imported from an old session
-                if( DataItem* replaceItem = static_cast<FileItem*>(item)->replaceItemFromOldSession() )
+                if( DataItem* replaceItem = static_cast<FileItem*>(item)->replaceItemFromOldSession() ) {
+                    static_cast<K3b::SessionImportItem*>(replaceItem)->setReplaceItem( nullptr );
                     addDataItem( replaceItem );
+                }
             }
         }
     }
@@ -449,27 +477,16 @@ bool K3b::DirItem::canAddDataItem( DataItem* item ) const
 void K3b::DirItem::addDataItemImpl( DataItem* item )
 {
     if( item->isFile() ) {
-        // do we replace an old item?
         QString name = item->k3bName();
         int cnt = 1;
-        while( DataItem* oldItem = find( name ) ) {
-            if( !oldItem->isDir() && oldItem->isFromOldSession() ) {
-                // in this case we remove this item from it's parent and save it in the new one
-                // to be able to recover it
-                oldItem->take();
-                static_cast<SessionImportItem*>(oldItem)->setReplaceItem( static_cast<FileItem*>(item) );
-                static_cast<FileItem*>(item)->setReplacedItemFromOldSession( oldItem );
-                break;
-            }
-            else {
-                //
-                // add a counter to the filename
-                //
-                if( item->k3bName()[item->k3bName().length()-4] == '.' )
-                    name = item->k3bName().left( item->k3bName().length()-4 ) + QString::number(cnt++) + item->k3bName().right(4);
-                else
-                    name = item->k3bName() + QString::number(cnt++);
-            }
+        while( find( name ) ) {
+            //
+            // add a counter to the filename
+            //
+            if( item->k3bName()[item->k3bName().length()-4] == '.' )
+                name = item->k3bName().left( item->k3bName().length()-4 ) + QString::number(cnt++) + item->k3bName().right(4);
+            else
+                name = item->k3bName() + QString::number(cnt++);
         }
         item->setK3bName( name );
     }
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 8fa65acc1..2e9115ee6 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -2,6 +2,7 @@
 add_executable(k3bdataprojectmodeltest
     k3bdataprojectmodeltest.cpp
     k3btestutils.cpp
+    ${CMAKE_SOURCE_DIR}/libk3b/projects/datacd/k3bsessionimportitem.cpp
     ${CMAKE_SOURCE_DIR}/src/projects/k3bdataprojectmodel.cpp)
 target_include_directories(k3bdataprojectmodeltest PRIVATE
     ${CMAKE_BINARY_DIR}/libk3bdevice
diff --git a/tests/k3bdataprojectmodeltest.cpp b/tests/k3bdataprojectmodeltest.cpp
index e4c414adb..325d411f1 100644
--- a/tests/k3bdataprojectmodeltest.cpp
+++ b/tests/k3bdataprojectmodeltest.cpp
@@ -10,6 +10,9 @@
 #include "k3bdataprojectmodel.h"
 #include "k3bdataitem.h"
 #include "k3bdiritem.h"
+#include "k3bfileitem.h"
+#include "k3biso9660.h"
+#include "k3bsessionimportitem.h"
 #include "k3bspecialdataitem.h"
 #include "k3btestutils.h"
 
@@ -20,14 +23,41 @@ QTEST_GUILESS_MAIN( DataProjectModelTest )
 
 Q_DECLARE_METATYPE( QModelIndex )
 
+
+namespace {
+    void verifyFile( const K3b::DataItem* item,
+                     const char expectedFilename[],
+                     const K3b::DataItem* expectedItemFromOldSession )
+    {
+        QCOMPARE( item->k3bName(), expectedFilename );
+        QCOMPARE( static_cast<const K3b::FileItem*>(item)->replaceItemFromOldSession(),
+                  expectedItemFromOldSession );
+    }
+} // namespace
+
+
 DataProjectModelTest::DataProjectModelTest()
 {
     qRegisterMetaType<QModelIndex>();
+
+    // create test.iso:
+    // touch file2 && genisoimage -no-pad -rock -o test.iso file2 && rm file2
+    m_iso = new K3b::Iso9660( QFINDTESTDATA( "testdata/test.iso" ) );
+    m_iso->open();
+}
+
+
+DataProjectModelTest::~DataProjectModelTest()
+{
+    delete m_iso;
 }
 
 
 void DataProjectModelTest::init()
 {
+    const K3b::Iso9660File* isoFile = static_cast<const K3b::Iso9660File*>
+        ( m_iso->firstIsoDirEntry()->entry("file2") );
+
     m_doc = new K3b::DataDoc;
     m_doc->newDocument();
     m_doc->root()->addDataItem( new K3b::DirItem( "First directory" ) ); // index 0
@@ -36,13 +66,13 @@ void DataProjectModelTest::init()
     K3b::DirItem* secondDirectory = new K3b::DirItem( "Second directory" );
     m_doc->root()->addDataItem( secondDirectory ); // index 3
     secondDirectory->addDataItem( new K3b::SpecialDataItem( 1024, "file1" ) ); // index 3 -> 0
-    secondDirectory->addDataItem( new K3b::SpecialDataItem( 512, "file2" ) ); // index 3 -> 1
+    secondDirectory->addDataItem( new K3b::SessionImportItem( isoFile ) ); // index 3 -> 1
     m_doc->root()->addDataItem( new K3b::SpecialDataItem( 300, "file3" ) ); // index 4
     m_doc->root()->addDataItem( new K3b::SpecialDataItem( 400, "file4" ) ); // index 5
 }
 
 
-void DataProjectModelTest::cleanp()
+void DataProjectModelTest::cleanup()
 {
     m_doc->deleteLater();
 }
@@ -86,4 +116,60 @@ void DataProjectModelTest::testRemove()
     spy.check( model.indexForItem( m_doc->root() ), 3 );
 }
 
+
+void DataProjectModelTest::testReplace()
+{
+    K3b::DataProjectModel model( m_doc );
+    K3b::DirItem* secondDirectory = m_doc->root()->find( "Second directory" )->getDirItem();
+    const K3b::DataItem* itemFromOldSession = secondDirectory->find( "file2" );
+
+    TestUtils::InsertRemoveModelSpy removeSpy( &model,
+                                               SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)),
+                                               SIGNAL(rowsRemoved(QModelIndex,int,int)) );
+    TestUtils::InsertRemoveModelSpy insertSpy( &model,
+                                               SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)),
+                                               SIGNAL(rowsInserted(QModelIndex,int,int)) );
+
+    secondDirectory->addDataItem( new K3b::FileItem( nullptr, nullptr, "file2", *m_doc ) );
+
+    const K3b::DirItem::Children& children = secondDirectory->children();
+    QCOMPARE( children.size(), 2 );
+    verifyFile( children.at( 0 ), "file1", nullptr );
+    verifyFile( children.at( 1 ), "file2", itemFromOldSession );
+
+    removeSpy.check( model.indexForItem( secondDirectory ), 1 );
+    insertSpy.check( model.indexForItem( secondDirectory ), 1 );
+}
+
+
+void DataProjectModelTest::testAddAndReplace()
+{
+    K3b::DataProjectModel model( m_doc );
+    K3b::DirItem* secondDirectory = m_doc->root()->find( "Second directory" )->getDirItem();
+    const K3b::DataItem* itemFromOldSession = secondDirectory->find( "file2" );
+
+    TestUtils::InsertRemoveModelSpy removeSpy( &model,
+                                               SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)),
+                                               SIGNAL(rowsRemoved(QModelIndex,int,int)) );
+    TestUtils::InsertRemoveModelSpy insertSpy( &model,
+                                               SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)),
+                                               SIGNAL(rowsInserted(QModelIndex,int,int)) );
+
+    secondDirectory->addDataItems( {
+        new K3b::FileItem( nullptr, nullptr, "file1", *m_doc ),
+        new K3b::FileItem( nullptr, nullptr, "file2", *m_doc ),
+        new K3b::FileItem( nullptr, nullptr, "file3", *m_doc ),
+    } );
+
+    const K3b::DirItem::Children& children = secondDirectory->children();
+    QCOMPARE( children.size(), 4 );
+    verifyFile( children.at( 0 ), "file1",  nullptr );
+    verifyFile( children.at( 1 ), "file11", nullptr );
+    verifyFile( children.at( 2 ), "file2",  itemFromOldSession );
+    verifyFile( children.at( 3 ), "file3",  nullptr );
+
+    removeSpy.check( model.indexForItem( secondDirectory ), 1, 1 );
+    insertSpy.check( model.indexForItem( secondDirectory ), 1, 3 );
+}
+
 #include "moc_k3bdataprojectmodeltest.cpp"
diff --git a/tests/k3bdataprojectmodeltest.h b/tests/k3bdataprojectmodeltest.h
index b5cf9d29f..4f0b72148 100644
--- a/tests/k3bdataprojectmodeltest.h
+++ b/tests/k3bdataprojectmodeltest.h
@@ -11,7 +11,10 @@
 #include <QObject>
 #include <QPointer>
 
-namespace K3b { class DataDoc; }
+namespace K3b {
+    class DataDoc;
+    class Iso9660;
+} // namespace K3b
 
 class DataProjectModelTest : public QObject
 {
@@ -19,16 +22,20 @@ class DataProjectModelTest : public QObject
 
 public:
     DataProjectModelTest();
+    ~DataProjectModelTest() override;
 
 private slots:
     void init(); // executed before each test function
-    void cleanp(); // executed after each test function
+    void cleanup(); // executed after each test function
     void testCreate();
     void testAdd();
     void testRemove();
+    void testReplace();
+    void testAddAndReplace();
 
 private:
     QPointer<K3b::DataDoc> m_doc;
+    K3b::Iso9660* m_iso;
 };
 
 #endif // K3B_DATA_PROJECT_MODEL_TEST_H
diff --git a/tests/testdata/test.iso b/tests/testdata/test.iso
new file mode 100644
index 000000000..294772c67
Binary files /dev/null and b/tests/testdata/test.iso differ
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.