Improve handling of filtered items when folders are deleted/collapsed

If an expanded folder with filtered children is collapsed or removed,
and the parent-child relationship cannot be determined by parsing the
URLs, this patch makes sure that the filtered children do not reappear
when the filter bar is cleared.

REVIEW: 109455

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 40cd75094b006041771d0dd2a1701ae8019b4c2d..5d1a1968ad7d1fd1e0308b4518ce8c8f81495a69 100644 (file)
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -1,22 +1,23 @@
-/***************************************************************************
- *   Copyright (C) 2011 by Peter Penz <peter.penz19@gmail.com>             *
- *   Copyright (C) 2013 by Frank Reininghaus <frank78ac@googlemail.com>    *
- *                                                                         *
- *   This program is free software; you can redistribute it and/or modify  *
- *   it under the terms of the GNU General Public License as published by  *
- *   the Free Software Foundation; either version 2 of the License, or     *
- *   (at your option) any later version.                                   *
- *                                                                         *
- *   This program is distributed in the hope that it will be useful,       *
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
- *   GNU General Public License for more details.                          *
- *                                                                         *
- *   You should have received a copy of the GNU General Public License     *
- *   along with this program; if not, write to the                         *
- *   Free Software Foundation, Inc.,                                       *
- *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA            *
- ***************************************************************************/
+/*****************************************************************************
+ *   Copyright (C) 2011 by Peter Penz <peter.penz19@gmail.com>               *
+ *   Copyright (C) 2013 by Frank Reininghaus <frank78ac@googlemail.com>      *
+ *   Copyright (C) 2013 by Emmanuel Pescosta <emmanuelpescosta099@gmail.com> *
+ *                                                                           *
+ *   This program is free software; you can redistribute it and/or modify    *
+ *   it under the terms of the GNU General Public License as published by    *
+ *   the Free Software Foundation; either version 2 of the License, or       *
+ *   (at your option) any later version.                                     *
+ *                                                                           *
+ *   This program is distributed in the hope that it will be useful,         *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of          *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the           *
+ *   GNU General Public License for more details.                            *
+ *                                                                           *
+ *   You should have received a copy of the GNU General Public License       *
+ *   along with this program; if not, write to the                           *
+ *   Free Software Foundation, Inc.,                                         *
+ *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA              *
+ *****************************************************************************/

 
 #include "kfileitemmodel.h"
 
 
 #include "kfileitemmodel.h"
 


@@ -421,7 +422,8 @@ bool KFileItemModel::setExpanded(int index, bool expanded)
         return false;
     }
 
         return false;
     }
 

-    const KUrl url = m_itemData.at(index)->item.url();
+    const KFileItem item = m_itemData.at(index)->item;
+    const KUrl url = item.url();

     if (expanded) {
         m_expandedDirs.insert(url);
         m_dirLister->openUrl(url, KDirLister::Keep);
     if (expanded) {
         m_expandedDirs.insert(url);
         m_dirLister->openUrl(url, KDirLister::Keep);


@@ -429,38 +431,10 @@ bool KFileItemModel::setExpanded(int index, bool expanded)
         m_expandedDirs.remove(url);
         m_dirLister->stop(url);
 
         m_expandedDirs.remove(url);
         m_dirLister->stop(url);
 

+        removeFilteredChildren(KFileItemList() << item);

 
 

-        KFileItemList itemsToRemove;
-        const int expandedParentsCount = data(index)["expandedParentsCount"].toInt();
-        ++index;
-        while (index < count() && data(index)["expandedParentsCount"].toInt() > expandedParentsCount) {
-            itemsToRemove.append(m_itemData.at(index)->item);
-            ++index;
-        }
-
-        QSet<KUrl> urlsToRemove;
-        urlsToRemove.reserve(itemsToRemove.count() + 1);
-        urlsToRemove.insert(url);
-        foreach (const KFileItem& item, itemsToRemove) {
-            KUrl url = item.url();
-            url.adjustPath(KUrl::RemoveTrailingSlash);
-            urlsToRemove.insert(url);
-        }
-
-        QHash<KFileItem, ItemData*>::iterator it = m_filteredItems.begin();
-        while (it != m_filteredItems.end()) {
-            const KUrl url = it.key().url();
-            KUrl parentUrl = url.upUrl();
-            parentUrl.adjustPath(KUrl::RemoveTrailingSlash);
-
-            if (urlsToRemove.contains(parentUrl)) {
-                delete it.value();
-                it = m_filteredItems.erase(it);
-            } else {
-                ++it;
-            }
-        }
-
+        const KFileItemList itemsToRemove = childItems(item);
+        removeFilteredChildren(itemsToRemove);

         removeItems(itemsToRemove, DeleteItemData);
     }
 
         removeItems(itemsToRemove, DeleteItemData);
     }
 


@@ -598,6 +572,30 @@ void KFileItemModel::applyFilters()
     insertItems(newVisibleItems);
 }
 
     insertItems(newVisibleItems);
 }
 

+void KFileItemModel::removeFilteredChildren(const KFileItemList& parentsList)
+{
+    if (m_filteredItems.isEmpty()) {
+        return;
+    }
+
+    // First, we put the parent items into a set to provide fast lookup
+    // while iterating over m_filteredItems and prevent quadratic
+    // complexity if there are N parents and N filtered items.
+    const QSet<KFileItem> parents = parentsList.toSet();
+
+    QHash<KFileItem, ItemData*>::iterator it = m_filteredItems.begin();
+    while (it != m_filteredItems.end()) {
+        const ItemData* parent = it.value()->parent;
+
+        if (parent && parents.contains(parent->item)) {
+            delete it.value();
+            it = m_filteredItems.erase(it);
+        } else {
+            ++it;
+        }
+    }
+}
+

 QList<KFileItemModel::RoleInfo> KFileItemModel::rolesInformation()
 {
     static QList<RoleInfo> rolesInfo;
 QList<KFileItemModel::RoleInfo> KFileItemModel::rolesInformation()
 {
     static QList<RoleInfo> rolesInfo;


@@ -826,35 +824,7 @@ void KFileItemModel::slotItemsDeleted(const KFileItemList& items)
         }
 
         if (m_requestRole[ExpandedParentsCountRole]) {
         }
 
         if (m_requestRole[ExpandedParentsCountRole]) {

-            // Remove all filtered children of deleted items. First, we put the
-            // deleted URLs into a set to provide fast lookup while iterating
-            // over m_filteredItems and prevent quadratic complexity if there
-            // are N removed items and N filtered items.
-            //
-            // TODO: This does currently *not* work if the parent-child
-            // relationships can not be determined just by using KUrl::upUrl().
-            // This is the case, e.g., when browsing smb:/.
-            QSet<KUrl> urlsToRemove;
-            urlsToRemove.reserve(itemsToRemove.count());
-            foreach (const KFileItem& item, itemsToRemove) {
-                KUrl url = item.url();
-                url.adjustPath(KUrl::RemoveTrailingSlash);
-                urlsToRemove.insert(url);
-            }
-
-            QHash<KFileItem, ItemData*>::iterator it = m_filteredItems.begin();
-            while (it != m_filteredItems.end()) {
-                const KUrl url = it.key().url();
-                KUrl parentUrl = url.upUrl();
-                parentUrl.adjustPath(KUrl::RemoveTrailingSlash);
-
-                if (urlsToRemove.contains(parentUrl)) {
-                    delete it.value();
-                    it = m_filteredItems.erase(it);
-                } else {
-                    ++it;
-                }
-            }
+            removeFilteredChildren(itemsToRemove);

         }
     }
 
         }
     }
 

diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index 85a8d0c0d35506afe8991b45f0934a30742d2171..1d837cb2a5c38938201d9cb914e233e869ac9254 100644 (file)
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -396,6 +396,12 @@ private:
      */
     void applyFilters();
 
      */
     void applyFilters();
 

+    /**
+     * Removes filtered items whose expanded parents have been deleted
+     * or collapsed via setExpanded(parentIndex, false).
+     */
+    void removeFilteredChildren(const KFileItemList& parentsList);
+

     /**
      * Maps the QByteArray-roles to RoleTypes and provides translation- and
      * group-contexts.
     /**
      * Maps the QByteArray-roles to RoleTypes and provides translation- and
      * group-contexts.

diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp
index cd2e8d7773bf0fb468f2941a6ffff1b8f87f3ac9..484ddee110ce816bf3146e19ca300c5bdab51d21 100644 (file)
--- a/src/tests/kfileitemmodeltest.cpp
+++ b/src/tests/kfileitemmodeltest.cpp
@@ -81,6 +81,7 @@ private slots:
     void testRemoveHiddenItems();
     void collapseParentOfHiddenItems();
     void removeParentOfHiddenItems();
     void testRemoveHiddenItems();
     void collapseParentOfHiddenItems();
     void removeParentOfHiddenItems();

+    void testGeneralParentChildRelationships();

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


@@ -1004,6 +1005,98 @@ void KFileItemModelTest::removeParentOfHiddenItems()
     QCOMPARE(itemsInModel(), QStringList() << "a" << "1");
 }
 
     QCOMPARE(itemsInModel(), QStringList() << "a" << "1");
 }
 

+/**
+ * Create a tree structure where parent-child relationships can not be
+ * determined by parsing the URLs, and verify that KFileItemModel
+ * handles them correctly.
+ */
+void KFileItemModelTest::testGeneralParentChildRelationships()
+{
+    QSet<QByteArray> modelRoles = m_model->roles();
+    modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount";
+    m_model->setRoles(modelRoles);
+
+    QStringList files;
+    files << "parent1/realChild1/realGrandChild1" << "parent2/realChild2/realGrandChild2";
+    m_testDir->createFiles(files);
+
+    m_model->loadDirectory(m_testDir->url());
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "parent2");
+
+    // Expand all folders.
+    m_model->setExpanded(0, true);
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2");
+
+    m_model->setExpanded(1, true);
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2");
+
+    m_model->setExpanded(3, true);
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2" << "realChild2");
+
+    m_model->setExpanded(4, true);
+    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "parent2" << "realChild2" << "realGrandChild2");
+
+    // Add some more children and grand-children.
+    const KUrl parent1 = m_model->fileItem(0).url();
+    const KUrl parent2 = m_model->fileItem(3).url();
+    const KUrl realChild1 = m_model->fileItem(1).url();
+    const KUrl realChild2 = m_model->fileItem(4).url();
+
+    m_model->slotItemsAdded(parent1, KFileItemList() << KFileItem(KUrl("child1"), QString(), KFileItem::Unknown));
+    m_model->slotCompleted();
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2");
+
+    m_model->slotItemsAdded(parent2, KFileItemList() << KFileItem(KUrl("child2"), QString(), KFileItem::Unknown));
+    m_model->slotCompleted();
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2");
+
+    m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(KUrl("grandChild1"), QString(), KFileItem::Unknown));
+    m_model->slotCompleted();
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2");
+
+    m_model->slotItemsAdded(realChild1, KFileItemList() << KFileItem(KUrl("grandChild1"), QString(), KFileItem::Unknown));
+    m_model->slotCompleted();
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "realGrandChild2" << "child2");
+
+    m_model->slotItemsAdded(realChild2, KFileItemList() << KFileItem(KUrl("grandChild2"), QString(), KFileItem::Unknown));
+    m_model->slotCompleted();
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "grandChild1" << "realGrandChild1" << "child1" << "parent2" << "realChild2" << "grandChild2" << "realGrandChild2" << "child2");
+
+    // Set a name filter that matches nothing -> only expanded folders remain.
+    QSignalSpy itemsRemovedSpy(m_model, SIGNAL(itemsRemoved(KItemRangeList)));
+    m_model->setNameFilter("xyz");
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "realChild1" << "parent2" << "realChild2");
+    QCOMPARE(itemsRemovedSpy.count(), 1);
+    QList<QVariant> arguments = itemsRemovedSpy.takeFirst();
+    KItemRangeList itemRangeList = arguments.at(0).value<KItemRangeList>();
+    QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(2, 3) << KItemRange(7, 3));
+
+    // Collapse "parent1".
+    m_model->setExpanded(0, false);
+    QCOMPARE(itemsInModel(), QStringList() << "parent1" << "parent2" << "realChild2");
+    QCOMPARE(itemsRemovedSpy.count(), 1);
+    arguments = itemsRemovedSpy.takeFirst();
+    itemRangeList = arguments.at(0).value<KItemRangeList>();
+    QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(1, 1));
+
+    // Remove "parent2".
+    m_model->slotItemsDeleted(KFileItemList() << m_model->fileItem(1));
+    QCOMPARE(itemsInModel(), QStringList() << "parent1");
+    QCOMPARE(itemsRemovedSpy.count(), 1);
+    arguments = itemsRemovedSpy.takeFirst();
+    itemRangeList = arguments.at(0).value<KItemRangeList>();
+    QCOMPARE(itemRangeList, KItemRangeList() << KItemRange(1, 2));
+
+    // Clear filter, verify that no items reappear.
+    m_model->setNameFilter(QString());
+    QCOMPARE(itemsInModel(), QStringList() << "parent1");
+}
+

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