Update the roles for filtered items if necessary

Since Dolphin 4.11, we store not only KFileItems, but also the
corresponding ItemData struct for filtered items. This is required for
keeping track of the parent-child relationships, and has the nice side
effect that the ItemData need not be re-determined when the items are
shown again.

However, this can become a problem if the visible roles or the sort role
change while some items are filtered.

This is fixed by is fixed by clearing the QHash "values" for the
filtered items if the visible roles change. The hash will be
re-populated with all requested data as soon as the items are shown
again and the data(int) method of the model is called.

Moreover, before the items are inserted into the model after filtering,
we have to make sure that the sort role "Permissions"/"User"/etc. is
present in the hash "values". This is achieved by factoring out the code
that currently does this job for new items in createItemDataList() into
a new function, and calling this in insertItems(), because the same
treatment is required for the previously filtered files.

BUG: 328791
FIXED-IN: 4.12.1
REVIEW: 114266

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 4c8577543fdd3bb897a7d2e1c8034409617d0bcd..739384bf9f0431c8e6b6cd3f1f900247811746c8 100644 (file)
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -413,6 +413,15 @@ void KFileItemModel::setRoles(const QSet<QByteArray>& roles)
         kWarning() << "TODO: Emitting itemsChanged() with no information what has changed!";
         emit itemsChanged(KItemRangeList() << KItemRange(0, count()), QSet<QByteArray>());
     }
         kWarning() << "TODO: Emitting itemsChanged() with no information what has changed!";
         emit itemsChanged(KItemRangeList() << KItemRange(0, count()), QSet<QByteArray>());
     }

+
+    // Clear the 'values' of all filtered items. They will be re-populated with the
+    // correct roles the next time 'values' will be accessed via data(int).
+    QHash<KFileItem, ItemData*>::iterator filteredIt = m_filteredItems.begin();
+    const QHash<KFileItem, ItemData*>::iterator filteredEnd = m_filteredItems.end();
+    while (filteredIt != filteredEnd) {
+        (*filteredIt)->values.clear();
+        ++filteredIt;
+    }

 }
 
 QSet<QByteArray> KFileItemModel::roles() const
 }
 
 QSet<QByteArray> KFileItemModel::roles() const


@@ -1033,6 +1042,7 @@ void KFileItemModel::insertItems(QList<ItemData*>& newItems)
 #endif
 
     m_groups.clear();
 #endif
 
     m_groups.clear();

+    prepareItemsForSorting(newItems);

 
     if (m_sortRole == NameRole && m_naturalSorting) {
         // Natural sorting of items can be very slow. However, it becomes much
 
     if (m_sortRole == NameRole && m_naturalSorting) {
         // Natural sorting of items can be very slow. However, it becomes much


@@ -1196,6 +1206,11 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         itemDataList.append(itemData);
     }
 
         itemDataList.append(itemData);
     }
 

+    return itemDataList;
+}
+
+void KFileItemModel::prepareItemsForSorting(QList<ItemData*>& itemDataList)
+{

     switch (m_sortRole) {
     case PermissionsRole:
     case OwnerRole:
     switch (m_sortRole) {
     case PermissionsRole:
     case OwnerRole:


@@ -1205,16 +1220,20 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         // These roles can be determined with retrieveData, and they have to be stored
         // in the QHash "values" for the sorting.
         foreach (ItemData* itemData, itemDataList) {
         // These roles can be determined with retrieveData, and they have to be stored
         // in the QHash "values" for the sorting.
         foreach (ItemData* itemData, itemDataList) {

-            itemData->values = retrieveData(itemData->item, parentItem);
+            if (itemData->values.isEmpty()) {
+                itemData->values = retrieveData(itemData->item, itemData->parent);
+            }

         }
         break;
 
     case TypeRole:
         // At least store the data including the file type for items with known MIME type.
         foreach (ItemData* itemData, itemDataList) {
         }
         break;
 
     case TypeRole:
         // At least store the data including the file type for items with known MIME type.
         foreach (ItemData* itemData, itemDataList) {

-            const KFileItem item = itemData->item;
-            if (item.isDir() || item.isMimeTypeKnown()) {
-                itemData->values = retrieveData(itemData->item, parentItem);
+            if (itemData->values.isEmpty()) {
+                const KFileItem item = itemData->item;
+                if (item.isDir() || item.isMimeTypeKnown()) {
+                    itemData->values = retrieveData(itemData->item, itemData->parent);
+                }

             }
         }
         break;
             }
         }
         break;


@@ -1223,12 +1242,10 @@ QList<KFileItemModel::ItemData*> KFileItemModel::createItemDataList(const KUrl&
         // The other roles are either resolved by KFileItemModelRolesUpdater
         // (this includes the SizeRole for directories), or they do not need
         // to be stored in the QHash "values" for sorting because the data can
         // The other roles are either resolved by KFileItemModelRolesUpdater
         // (this includes the SizeRole for directories), or they do not need
         // to be stored in the QHash "values" for sorting because the data can

-        // be retrieved directly from the KFileItem (NameRole, SiezRole for files,
+        // be retrieved directly from the KFileItem (NameRole, SizeRole for files,

         // DateRole).
         break;
     }
         // DateRole).
         break;
     }

-
-    return itemDataList;

 }
 
 int KFileItemModel::expandedParentsCount(const ItemData* data)
 }
 
 int KFileItemModel::expandedParentsCount(const ItemData* data)

diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index c57329fc85051b153f0430e327bc2cdc1dccfded..0224290238b6a2d77741b625c0d7212ba080baa9 100644 (file)
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -320,6 +320,13 @@ private:
      */
     QList<ItemData*> createItemDataList(const KUrl& parentUrl, const KFileItemList& items) const;
 
      */
     QList<ItemData*> createItemDataList(const KUrl& parentUrl, const KFileItemList& items) const;
 

+    /**
+     * Prepares the items for sorting. Normally, the hash 'values' in ItemData is filled
+     * lazily to save time and memory, but for some sort roles, it is expected that the
+     * sort role data is stored in 'values'.
+     */
+    void prepareItemsForSorting(QList<ItemData*>& itemDataList);
+

     static int expandedParentsCount(const ItemData* data);
 
     void removeExpandedItems();
     static int expandedParentsCount(const ItemData* data);
 
     void removeExpandedItems();

diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp
index 62ff4fae2d705bb9953a01e0a55c3e4cd1e32075..ede9a3d6e6c720cccfe68d8b739aa2b45fe8998b 100644 (file)
--- a/src/tests/kfileitemmodeltest.cpp
+++ b/src/tests/kfileitemmodeltest.cpp
@@ -90,6 +90,8 @@ private slots:
     void testNameRoleGroups();
     void testNameRoleGroupsWithExpandedItems();
     void testInconsistentModel();
     void testNameRoleGroups();
     void testNameRoleGroupsWithExpandedItems();
     void testInconsistentModel();

+    void testChangeRolesForFilteredItems();
+    void testChangeSortRoleWhileFiltering();

 
 private:
     QStringList itemsInModel() const;
 
 private:
     QStringList itemsInModel() const;


@@ -1462,6 +1464,103 @@ void KFileItemModelTest::testInconsistentModel()
 
 }
 
 
 }
 

+void KFileItemModelTest::testChangeRolesForFilteredItems()
+{
+    QSet<QByteArray> modelRoles = m_model->roles();
+    modelRoles << "owner";
+    m_model->setRoles(modelRoles);
+
+    QStringList files;
+    files << "a.txt" << "aa.txt" << "aaa.txt";
+    m_testDir->createFiles(files);
+
+    m_model->loadDirectory(m_testDir->url());
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text" and "owner" roles, but not "group".
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(m_model->data(index).contains("owner"));
+        QVERIFY(!m_model->data(index).contains("group"));
+    }
+
+    // Add a filter, such that only "aaa.txt" remains in the model.
+    m_model->setNameFilter("aaa");
+    QCOMPARE(itemsInModel(), QStringList() << "aaa.txt");
+
+    // Add the "group" role.
+    modelRoles << "group";
+    m_model->setRoles(modelRoles);
+
+    // Modify the filter, such that "aa.txt" reappears, and verify that all items have the expected roles.
+    m_model->setNameFilter("aa");
+    QCOMPARE(itemsInModel(), QStringList() << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text", "owner", and "group" roles.
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(m_model->data(index).contains("owner"));
+        QVERIFY(m_model->data(index).contains("group"));
+    }
+
+    // Remove the "owner" role.
+    modelRoles.remove("owner");
+    m_model->setRoles(modelRoles);
+
+    // Clear the filter, and verify that all items have the expected roles
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "aa.txt" << "aaa.txt");
+
+    for (int index = 0; index < m_model->count(); ++index) {
+        // All items should have the "text" and "group" roles, but now "owner".
+        QVERIFY(m_model->data(index).contains("text"));
+        QVERIFY(!m_model->data(index).contains("owner"));
+        QVERIFY(m_model->data(index).contains("group"));
+    }
+}
+
+void KFileItemModelTest::testChangeSortRoleWhileFiltering()
+{
+    KFileItemList items;
+
+    KIO::UDSEntry entry;
+    entry.insert(KIO::UDSEntry::UDS_FILE_TYPE, 0100000);    // S_IFREG might not be defined on non-Unix platforms.
+    entry.insert(KIO::UDSEntry::UDS_ACCESS, 07777);
+    entry.insert(KIO::UDSEntry::UDS_SIZE, 0);
+    entry.insert(KIO::UDSEntry::UDS_MODIFICATION_TIME, 0);
+    entry.insert(KIO::UDSEntry::UDS_GROUP, "group");
+    entry.insert(KIO::UDSEntry::UDS_ACCESS_TIME, 0);
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "a.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-b");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "b.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-c");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    entry.insert(KIO::UDSEntry::UDS_NAME, "c.txt");
+    entry.insert(KIO::UDSEntry::UDS_USER, "user-a");
+    items.append(KFileItem(entry, m_testDir->url(), false, true));
+
+    m_model->slotItemsAdded(m_testDir->url(), items);
+    m_model->slotCompleted();
+
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "c.txt");
+
+    // Add a filter.
+    m_model->setNameFilter("a");
+    QCOMPARE(itemsInModel(), QStringList() << "a.txt");
+
+    // Sort by "owner".
+    m_model->setSortRole("owner");
+
+    // Clear the filter, and verify that the items are sorted correctly.
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "c.txt" << "a.txt" << "b.txt");
+}
+

 QStringList KFileItemModelTest::itemsInModel() const
 {
     QStringList items;
 QStringList KFileItemModelTest::itemsInModel() const
 {
     QStringList items;