QSortFilterProxyModel: add a protected beginFilterChange

If the filter gets changed and invalidated while there is no mapping
(perhaps because the model had been invalidated first), then we fail
to notice the change and don't emit rowsInserted/Removed. And as the
new filter is already in place by the time invalidateFilter gets
called, we cannot know what the size of the model was before the
change.

The only way to fix that is to introduce a beginFilterChange protected
function that makes sure that we have a mapping from the source model
with the old filter. That is a no-op if a mapping is already in place,
costing only the lookup in a hash table.

By calling this function, custom models with their own filtering logic
can make sure that their model emits the changed-signals as expected.

Add test coverage and documentation and fix the relevant examples
snippet to use that new protected function as recommended, and to
invalidate only the rows filter.

[ChangeLog][Core][QSortFilterProxyModel] Added a new protected
function beginFilterChange() that subclasses overriding
filterAcceptsRow or filterAcceptsColumn should call before the filter
parameter is changed. This makes sure that the signals informing
about rows or columns changing get correctly emitted.

Fixes: QTBUG-115717
Change-Id: Ib73a7119ac9dd9c4bcf220f1274d6b4ed093e7ff
Reviewed-by: David Faure <[email protected]>

examples/widgets/itemviews/customsortfiltermodel/mysortfilterproxymodel.cpp

@@ -15,16 +15,18 @@ MySortFilterProxyModel::MySortFilterProxyModel(QObject *parent)
 //! [1]
 void MySortFilterProxyModel::setFilterMinimumDate(QDate date)
 {
+    beginFilterChange();
     minDate = date;
-    invalidateFilter();
+    invalidateRowsFilter();
 }
 //! [1]
 
 //! [2]
 void MySortFilterProxyModel::setFilterMaximumDate(QDate date)
 {
+    beginFilterChange();
     maxDate = date;
-    invalidateFilter();
+    invalidateRowsFilter();
 }
 //! [2]
 

src/corelib/itemmodels/qsortfilterproxymodel.cpp

@@ -3079,6 +3079,25 @@ void QSortFilterProxyModel::invalidate()
     emit layoutChanged();
 }
 
+/*!
+   \since 6.9
+
+   Prepares a change of the filter.
+
+   This function should be called if you are implementing custom filtering
+   (e.g. filterAcceptsRow()), and your filter parameter is about to be changed.
+
+    \snippet ../widgets/itemviews/customsortfiltermodel/mysortfilterproxymodel.cpp 2
+
+    \sa invalidateFilter(), invalidateColumnsFilter(), invalidateRowsFilter()
+*/
+
+void QSortFilterProxyModel::beginFilterChange()
+{
+    Q_D(QSortFilterProxyModel);
+    d->create_mapping({});
+}
+
 /*!
    \since 4.3
 
@@ -3087,9 +3106,12 @@ void QSortFilterProxyModel::invalidate()
    This function should be called if you are implementing custom filtering
    (e.g. filterAcceptsRow()), and your filter parameters have changed.
 
-   \sa invalidate()
-   \sa invalidateColumnsFilter()
-   \sa invalidateRowsFilter()
+   Before your filter parameters change, call beginFilterChange().
+
+   \snippet ../widgets/itemviews/customsortfiltermodel/mysortfilterproxymodel.cpp 2
+
+   \sa invalidate(), invalidateColumnsFilter(), invalidateRowsFilter(),
+       beginFilterChange()
 */
 void QSortFilterProxyModel::invalidateFilter()
 {
@@ -3109,9 +3131,9 @@ void QSortFilterProxyModel::invalidateFilter()
    instead of invalidateFilter() if you want to hide or show a column where
    the rows don't change.
 
-   \sa invalidate()
-   \sa invalidateFilter()
-   \sa invalidateRowsFilter()
+   Before your filter parameters change, call beginFilterChange().
+
+   \sa invalidate(), invalidateRowsFilter(), beginFilterChange()
 */
 void QSortFilterProxyModel::invalidateColumnsFilter()
 {
@@ -3131,9 +3153,9 @@ void QSortFilterProxyModel::invalidateColumnsFilter()
    instead of invalidateFilter() if you want to hide or show a row where
    the columns don't change.
 
-   \sa invalidate()
-   \sa invalidateFilter()
-   \sa invalidateColumnsFilter()
+   Before your filter parameters change, call beginFilterChange().
+
+   \sa invalidate(), invalidateFilter(), invalidateColumnsFilter()
 */
 void QSortFilterProxyModel::invalidateRowsFilter()
 {

src/corelib/itemmodels/qsortfilterproxymodel.h

@@ -113,6 +113,7 @@ public Q_SLOTS:
     virtual bool filterAcceptsColumn(int source_column, const QModelIndex &source_parent) const;
     virtual bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const;
 
+    void beginFilterChange();
     void invalidateFilter();
     void invalidateRowsFilter();
     void invalidateColumnsFilter();

tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp

@@ -5497,5 +5497,64 @@ void tst_QSortFilterProxyModel::createPersistentOnLayoutAboutToBeChanged() // QT
     QCOMPARE(layoutChangedSpy.size(), 1);
 }
 
+void tst_QSortFilterProxyModel::filterChangeEmitsModelChangedSignals()
+{
+    QStringListModel model({"1", "2", "3", "4", "5"});
+
+    class FilterModel : public QSortFilterProxyModel
+    {
+        QString m_matchString;
+    public:
+        void setFilter(const QString &s)
+        {
+            if (m_matchString == s)
+                return;
+
+            beginFilterChange();
+            m_matchString = s;
+            invalidateFilter();
+        }
+
+        bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override
+        {
+            const auto index = sourceModel()->index(sourceRow, 0, sourceParent);
+            if (!index.isValid())
+                return false;
+
+            return index.data().value<QString>() == m_matchString;
+        }
+    };
+
+    FilterModel filterModel;
+
+     // Reject all source data at the start
+    filterModel.setFilter("X");
+     // Trigger an evaluation
+    filterModel.sort(0, Qt::AscendingOrder);
+    filterModel.setSourceModel(&model);
+    QCOMPARE(filterModel.rowCount(), 0);
+    filterModel.invalidate();
+
+    QSignalSpy rowsInsertedSpy(&filterModel, &QSortFilterProxyModel::rowsInserted);
+    QSignalSpy rowsRemovedSpy(&filterModel, &QSortFilterProxyModel::rowsRemoved);
+
+    filterModel.setFilter("3");
+    QCOMPARE(filterModel.rowCount(), 1);
+    QCOMPARE(rowsInsertedSpy.count(), 1);
+    rowsInsertedSpy.clear();
+
+    filterModel.setFilter("2");
+    QCOMPARE(filterModel.rowCount(), 1);
+    QCOMPARE(rowsInsertedSpy.count(), 1);
+    QCOMPARE(rowsRemovedSpy.count(), 1);
+    rowsInsertedSpy.clear();
+    rowsRemovedSpy.clear();
+
+    filterModel.setFilter("X");
+    QCOMPARE(filterModel.rowCount(), 0);
+    QCOMPARE(rowsInsertedSpy.count(), 0);
+    QCOMPARE(rowsRemovedSpy.count(), 1);
+}
+
 QTEST_MAIN(tst_QSortFilterProxyModel)
 #include "tst_qsortfilterproxymodel.moc"

tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.h

@@ -153,6 +153,8 @@ private slots:
     void filterCaseSensitivityBinding();
     void filterRegularExpressionBinding();
 
+    void filterChangeEmitsModelChangedSignals();
+
 protected:
     void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
     void checkHierarchy(const QStringList &data, const QAbstractItemModel *model);