From: Frank Reininghaus Date: Sat, 14 Dec 2013 10:51:07 +0000 (+0100) Subject: Update the roles for filtered items if necessary X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/8210d5e472a2bff9f1a1f16b0740df25822b5362 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 4c8577543..739384bf9 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -413,6 +413,15 @@ void KFileItemModel::setRoles(const QSet& roles) kWarning() << "TODO: Emitting itemsChanged() with no information what has changed!"; emit itemsChanged(KItemRangeList() << KItemRange(0, count()), QSet()); } + + // 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::iterator filteredIt = m_filteredItems.begin(); + const QHash::iterator filteredEnd = m_filteredItems.end(); + while (filteredIt != filteredEnd) { + (*filteredIt)->values.clear(); + ++filteredIt; + } } QSet KFileItemModel::roles() const @@ -1033,6 +1042,7 @@ void KFileItemModel::insertItems(QList& newItems) #endif m_groups.clear(); + prepareItemsForSorting(newItems); if (m_sortRole == NameRole && m_naturalSorting) { // Natural sorting of items can be very slow. However, it becomes much @@ -1196,6 +1206,11 @@ QList KFileItemModel::createItemDataList(const KUrl& itemDataList.append(itemData); } + return itemDataList; +} + +void KFileItemModel::prepareItemsForSorting(QList& itemDataList) +{ switch (m_sortRole) { case PermissionsRole: case OwnerRole: @@ -1205,16 +1220,20 @@ QList 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) { - 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) { - 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; @@ -1223,12 +1242,10 @@ QList 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 - // be retrieved directly from the KFileItem (NameRole, SiezRole for files, + // be retrieved directly from the KFileItem (NameRole, SizeRole for files, // DateRole). break; } - - return itemDataList; } int KFileItemModel::expandedParentsCount(const ItemData* data) diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h index c57329fc8..022429023 100644 --- a/src/kitemviews/kfileitemmodel.h +++ b/src/kitemviews/kfileitemmodel.h @@ -320,6 +320,13 @@ private: */ QList 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& itemDataList); + static int expandedParentsCount(const ItemData* data); void removeExpandedItems(); diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 62ff4fae2..ede9a3d6e 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -90,6 +90,8 @@ private slots: void testNameRoleGroups(); void testNameRoleGroupsWithExpandedItems(); void testInconsistentModel(); + void testChangeRolesForFilteredItems(); + void testChangeSortRoleWhileFiltering(); private: QStringList itemsInModel() const; @@ -1462,6 +1464,103 @@ void KFileItemModelTest::testInconsistentModel() } +void KFileItemModelTest::testChangeRolesForFilteredItems() +{ + QSet 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;