Fix the API for resetting QAbstractItemModels.

This commit deprecates the QAIM::reset() method, and adds beginResetModel()
and endResetModel() methods, addressing Qt issue 247023.

http://www.qtsoftware.com/developer/task-tracker/index_html?method=entry&id=247023

If models and proxies use QAIM::reset() alone, then proxies will
emit modelAboutToBeReset after its source model is reset. This means that mapToSource
will not behave as expected (Will always return an invalid index) in a slot connected
to modelAboutToBeReset.

The usecase for this is maintaining viewstate (which items are selected, expanded)
when the model is reset. See BrowserWidget::modelChanged here:
http://websvn.kde.org/trunk/KDE/kdepim/akonadi/akonadiconsole/browserwidget.cpp?view=markup

Task-number: 247023
Reviewed-by: Olivier Goffart <ogoffart@trolltech.com>
Merge-request: 1072

7 files changed, 290 insertions, 37 deletions

diff --git a/src/corelib/kernel/qabstractitemmodel.cpp b/src/corelib/kernel/qabstractitemmodel.cpp
index 1c9104a313..2175542bbb 100644
--- a/src/corelib/kernel/qabstractitemmodel.cpp
+++ b/src/corelib/kernel/qabstractitemmodel.cpp
@@ -1339,7 +1339,7 @@ void QAbstractItemModelPrivate::columnsRemoved(const QModelIndex &parent,
     \endlist
 
 
-    \sa layoutAboutToBeChanged(), dataChanged(), headerDataChanged(), reset(),
+    \sa layoutAboutToBeChanged(), dataChanged(), headerDataChanged(), modelReset(),
         changePersistentIndex()
 */
 
@@ -2769,7 +2769,25 @@ void QAbstractItemModel::endMoveColumns()
 /*!
     Resets the model to its original state in any attached views.
 
-    The view to which the model is attached to will be reset as well.
+    Use beginResetModel() and endResetModel() instead whenever possible. If usng this method, the modelAboutToBeReset signal will
+    not function as expected through proxy models.
+
+    Use this method only if there is no way to call beginResetModel() before invalidating the model.
+*/
+void QAbstractItemModel::reset()
+{
+    Q_D(QAbstractItemModel);
+    emit modelAboutToBeReset();
+    d->invalidatePersistentIndexes();
+    emit modelReset();
+}
+
+/*!
+    Begins a model reset operation.
+
+    A reset operation resets the model to its current state in any attached views.
+
+    \note Any views attached to this model will be reset as well.
 
     When a model is reset it means that any previous data reported from the
     model is now invalid and has to be queried for again. This also means that
@@ -2779,12 +2797,29 @@ void QAbstractItemModel::endMoveColumns()
     call this function rather than emit dataChanged() to inform other
     components when the underlying data source, or its structure, has changed.
 
-    \sa modelAboutToBeReset(), modelReset()
+    You must call this function before resetting any internal data structures in your model
+    or proxy model.
+
+    \sa modelAboutToBeReset(), modelReset(), endResetModel()
+    \since 4.6
 */
-void QAbstractItemModel::reset()
+void QAbstractItemModel::beginResetModel()
 {
-    Q_D(QAbstractItemModel);
     emit modelAboutToBeReset();
+}
+
+/*!
+    Completes a model reset operation.
+
+    You must call this function after resetting any internal data structure in your model
+    or proxy model.
+
+    \sa beginResetModel()
+    \since 4.6
+*/
+void QAbstractItemModel::endResetModel()
+{
+    Q_D(QAbstractItemModel);
     d->invalidatePersistentIndexes();
     emit modelReset();
 }
@@ -3256,7 +3291,7 @@ bool QAbstractListModel::dropMimeData(const QMimeData *data, Qt::DropAction acti
     This signal is emitted when reset() is called, before the model's internal
     state (e.g. persistent model indexes) has been invalidated.
 
-    \sa reset(), modelReset()
+    \sa beginResetModel(), modelReset()
 */
 
 /*!
@@ -3266,7 +3301,7 @@ bool QAbstractListModel::dropMimeData(const QMimeData *data, Qt::DropAction acti
     This signal is emitted when reset() is called, after the model's internal
     state (e.g. persistent model indexes) has been invalidated.
 
-    \sa reset(), modelAboutToBeReset()
+    \sa endResetModel(), modelAboutToBeReset()
 */
 
 /*!
diff --git a/src/corelib/kernel/qabstractitemmodel.h b/src/corelib/kernel/qabstractitemmodel.h
index b47e4cb5ac..1ca6cbeb3d 100644
--- a/src/corelib/kernel/qabstractitemmodel.h
+++ b/src/corelib/kernel/qabstractitemmodel.h
@@ -293,6 +293,9 @@ protected:
 
     void reset();
 
+    void beginResetModel();
+    void endResetModel();
+
     void changePersistentIndex(const QModelIndex &from, const QModelIndex &to);
     void changePersistentIndexList(const QModelIndexList &from, const QModelIndexList &to);
     QModelIndexList persistentIndexList() const;
diff --git a/src/gui/itemviews/qsortfilterproxymodel.cpp b/src/gui/itemviews/qsortfilterproxymodel.cpp
index d173efe97a..18fbf7b204 100644
--- a/src/gui/itemviews/qsortfilterproxymodel.cpp
+++ b/src/gui/itemviews/qsortfilterproxymodel.cpp
@@ -174,6 +174,7 @@ public:
                            const QModelIndex &source_bottom_right);
     void _q_sourceHeaderDataChanged(Qt::Orientation orientation, int start, int end);
 
+    void _q_sourceAboutToBeReset();
     void _q_sourceReset();
 
     void _q_sourceLayoutAboutToBeChanged();
@@ -1148,11 +1149,17 @@ void QSortFilterProxyModelPrivate::_q_sourceHeaderDataChanged(Qt::Orientation or
     emit q->headerDataChanged(orientation, proxy_start, proxy_end);
 }
 
+void QSortFilterProxyModelPrivate::_q_sourceAboutToBeReset()
+{
+    Q_Q(QSortFilterProxyModel);
+    q->beginResetModel();
+}
+
 void QSortFilterProxyModelPrivate::_q_sourceReset()
 {
     Q_Q(QSortFilterProxyModel);
     // All internal structures are deleted in clear()
-    q->reset();
+    q->endResetModel();
     update_source_sort_column();
     if (dynamic_sortfilter)
         sort();
@@ -1499,6 +1506,7 @@ void QSortFilterProxyModel::setSourceModel(QAbstractItemModel *sourceModel)
     disconnect(d->model, SIGNAL(layoutChanged()),
                this, SLOT(_q_sourceLayoutChanged()));
 
+    disconnect(d->model, SIGNAL(modelAboutToBeReset()), this, SLOT(_q_sourceAboutToBeReset()));
     disconnect(d->model, SIGNAL(modelReset()), this, SLOT(_q_sourceReset()));
 
     QAbstractProxyModel::setSourceModel(sourceModel);
@@ -1539,6 +1547,7 @@ void QSortFilterProxyModel::setSourceModel(QAbstractItemModel *sourceModel)
     connect(d->model, SIGNAL(layoutChanged()),
             this, SLOT(_q_sourceLayoutChanged()));
 
+    connect(d->model, SIGNAL(modelAboutToBeReset()), this, SLOT(_q_sourceAboutToBeReset()));
     connect(d->model, SIGNAL(modelReset()), this, SLOT(_q_sourceReset()));
 
     d->clear_mapping();
diff --git a/src/gui/itemviews/qsortfilterproxymodel.h b/src/gui/itemviews/qsortfilterproxymodel.h
index 12baa19eb4..0314aec8ba 100644
--- a/src/gui/itemviews/qsortfilterproxymodel.h
+++ b/src/gui/itemviews/qsortfilterproxymodel.h
@@ -177,6 +177,7 @@ private:
 
     Q_PRIVATE_SLOT(d_func(), void _q_sourceDataChanged(const QModelIndex &source_top_left, const QModelIndex &source_bottom_right))
     Q_PRIVATE_SLOT(d_func(), void _q_sourceHeaderDataChanged(Qt::Orientation orientation, int start, int end))
+    Q_PRIVATE_SLOT(d_func(), void _q_sourceAboutToBeReset())
     Q_PRIVATE_SLOT(d_func(), void _q_sourceReset())
     Q_PRIVATE_SLOT(d_func(), void _q_sourceLayoutAboutToBeChanged())
     Q_PRIVATE_SLOT(d_func(), void _q_sourceLayoutChanged())
diff --git a/tests/auto/qabstractitemmodel/dynamictreemodel.cpp b/tests/auto/qabstractitemmodel/dynamictreemodel.cpp
index 6c3e0cb88a..374b7db4fa 100644
--- a/tests/auto/qabstractitemmodel/dynamictreemodel.cpp
+++ b/tests/auto/qabstractitemmodel/dynamictreemodel.cpp
@@ -204,42 +204,106 @@ ModelMoveCommand::ModelMoveCommand(DynamicTreeModel *model, QObject *parent)
 {
 
 }
+bool ModelMoveCommand::emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow)
+{
+  return m_model->beginMoveRows(srcParent, srcStart, srcEnd, destParent, destRow);
+}
 
 void ModelMoveCommand::doCommand()
 {
-  QModelIndex srcParent = findIndex(m_rowNumbers);
-  QModelIndex destParent = findIndex(m_destRowNumbers);
-
-  if (!m_model->beginMoveRows(srcParent, m_startRow, m_endRow, destParent, m_destRow))
-  {
-    return;
-  }
-  
-  for (int column = 0; column < m_numCols; ++column)
-  {
-    QList<qint64> l = m_model->m_childItems.value(srcParent.internalId())[column].mid(m_startRow, m_endRow - m_startRow + 1 );
+    QModelIndex srcParent = findIndex(m_rowNumbers);
+    QModelIndex destParent = findIndex(m_destRowNumbers);
 
-    for (int i = m_startRow; i <= m_endRow ; i++)
+    if (!emitPreSignal(srcParent, m_startRow, m_endRow, destParent, m_destRow))
     {
-      m_model->m_childItems[srcParent.internalId()][column].removeAt(m_startRow);
-    }
-    int d;
-    if (m_destRow < m_startRow)
-      d = m_destRow;
-    else
-    {
-      if (srcParent == destParent)
-        d = m_destRow - (m_endRow - m_startRow + 1);
-      else
-        d = m_destRow - (m_endRow - m_startRow) + 1;
+        return;
     }
 
-    foreach(const qint64 id, l)
+    for (int column = 0; column < m_numCols; ++column)
     {
-      m_model->m_childItems[destParent.internalId()][column].insert(d++, id);
+        QList<qint64> l = m_model->m_childItems.value(srcParent.internalId())[column].mid(m_startRow, m_endRow - m_startRow + 1 );
+
+        for (int i = m_startRow; i <= m_endRow ; i++)
+        {
+            m_model->m_childItems[srcParent.internalId()][column].removeAt(m_startRow);
+        }
+        int d;
+        if (m_destRow < m_startRow)
+            d = m_destRow;
+        else
+        {
+            if (srcParent == destParent)
+                d = m_destRow - (m_endRow - m_startRow + 1);
+            else
+                d = m_destRow - (m_endRow - m_startRow) + 1;
+        }
+
+        foreach(const qint64 id, l)
+        {
+            m_model->m_childItems[destParent.internalId()][column].insert(d++, id);
+        }
     }
-  }
 
-   m_model->endMoveRows();
+    emitPostSignal();
+}
+
+void ModelMoveCommand::emitPostSignal()
+{
+    m_model->endMoveRows();
+}
+
+ModelResetCommand::ModelResetCommand(DynamicTreeModel* model, QObject* parent)
+  : ModelMoveCommand(model, parent)
+{
+
+}
+
+ModelResetCommand::~ModelResetCommand()
+{
+
+}
+
+bool ModelResetCommand::emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow)
+{
+    Q_UNUSED(srcParent);
+    Q_UNUSED(srcStart);
+    Q_UNUSED(srcEnd);
+    Q_UNUSED(destParent);
+    Q_UNUSED(destRow);
+
+    return true;
+}
+
+void ModelResetCommand::emitPostSignal()
+{
+    m_model->reset();
+}
+
+ModelResetCommandFixed::ModelResetCommandFixed(DynamicTreeModel* model, QObject* parent)
+  : ModelMoveCommand(model, parent)
+{
+
+}
+
+ModelResetCommandFixed::~ModelResetCommandFixed()
+{
+
+}
+
+bool ModelResetCommandFixed::emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow)
+{
+    Q_UNUSED(srcParent);
+    Q_UNUSED(srcStart);
+    Q_UNUSED(srcEnd);
+    Q_UNUSED(destParent);
+    Q_UNUSED(destRow);
+
+    m_model->beginResetModel();
+    return true;
+}
+
+void ModelResetCommandFixed::emitPostSignal()
+{
+    m_model->endResetModel();
 }
 
diff --git a/tests/auto/qabstractitemmodel/dynamictreemodel.h b/tests/auto/qabstractitemmodel/dynamictreemodel.h
index 88e293c7a7..c19ed9da8b 100644
--- a/tests/auto/qabstractitemmodel/dynamictreemodel.h
+++ b/tests/auto/qabstractitemmodel/dynamictreemodel.h
@@ -70,6 +70,8 @@ private:
 
   friend class ModelInsertCommand;
   friend class ModelMoveCommand;
+  friend class ModelResetCommand;
+  friend class ModelResetCommandFixed;
 
 };
 
@@ -118,6 +120,7 @@ public:
   virtual void doCommand();
 };
 
+
 class ModelMoveCommand : public ModelChangeCommand
 {
   Q_OBJECT
@@ -126,8 +129,12 @@ public:
 
   virtual ~ModelMoveCommand() {}
 
+  virtual bool emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow);
+
   virtual void doCommand();
 
+  virtual void emitPostSignal();
+
   void setDestAncestors( QList<int> rows ) { m_destRowNumbers = rows; }
 
   void setDestRow(int row) { m_destRow = row; }
@@ -137,5 +144,37 @@ protected:
   int m_destRow;
 };
 
+/**
+  A command which does a move and emits a reset signal.
+*/
+class ModelResetCommand : public ModelMoveCommand
+{
+  Q_OBJECT
+public:
+  ModelResetCommand(DynamicTreeModel* model, QObject* parent = 0);
+
+  virtual ~ModelResetCommand();
+
+  virtual bool emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow);
+  virtual void emitPostSignal();
+
+};
+
+/**
+  A command which does a move and emits a beginResetModel and endResetModel signals.
+*/
+class ModelResetCommandFixed : public ModelMoveCommand
+{
+  Q_OBJECT
+public:
+  ModelResetCommandFixed(DynamicTreeModel* model, QObject* parent = 0);
+
+  virtual ~ModelResetCommandFixed();
+
+  virtual bool emitPreSignal(const QModelIndex &srcParent, int srcStart, int srcEnd, const QModelIndex &destParent, int destRow);
+  virtual void emitPostSignal();
+
+};
+
 
 #endif
diff --git a/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp b/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
index 9c83474e42..61eeae7346 100644
--- a/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
+++ b/tests/auto/qabstractitemmodel/tst_qabstractitemmodel.cpp
@@ -43,6 +43,8 @@
 #include <QtTest/QtTest>
 #include <QtCore/QtCore>
 
+#include <QSortFilterProxyModel>
+
 //TESTED_CLASS=QAbstractListModel QAbstractTableModel
 //TESTED_FILES=
 
@@ -110,6 +112,8 @@ private slots:
     void testMoveWithinOwnRange_data();
     void testMoveWithinOwnRange();
 
+    void testReset();
+
 
 private:
     DynamicTreeModel *m_model;
@@ -814,7 +818,7 @@ void tst_QAbstractItemModel::complexChangesWithPersistent()
 
     //remove a bunch of columns
     model.removeColumns(2, 4);
-    
+
     QVERIFY(a == model.index(1, 1, QModelIndex()));
     QVERIFY(b == model.index(9, 3, QModelIndex()));
     QVERIFY(c == model.index(5, 2, QModelIndex()));
@@ -825,7 +829,7 @@ void tst_QAbstractItemModel::complexChangesWithPersistent()
         QVERIFY(!e[i].isValid());
     for (int i=6; i <10 ; i++)
         QVERIFY(e[i] == model.index(2, i-4 , QModelIndex()));
-    
+
     //move some indexes around
     model.setPersistent(model.index(1, 1 , QModelIndex()), model.index(9, 3 , QModelIndex()));
     model.setPersistent(model.index(9, 3 , QModelIndex()), model.index(8, 4 , QModelIndex()));
@@ -1652,6 +1656,104 @@ void tst_QAbstractItemModel::testMoveWithinOwnRange()
 
 }
 
+class ListenerObject : public QObject
+{
+  Q_OBJECT
+public:
+    ListenerObject(QAbstractProxyModel *parent);
+
+protected:
+    void fillIndexStores(const QModelIndex &parent);
+
+public slots:
+    void slotAboutToBeReset();
+    void slotReset();
+
+private:
+    QAbstractProxyModel *m_model;
+    QList<QPersistentModelIndex> m_persistentIndexes;
+    QModelIndexList m_nonPersistentIndexes;
+};
+
+
+ListenerObject::ListenerObject(QAbstractProxyModel *parent)
+    : QObject(parent), m_model(parent)
+{
+    connect(m_model, SIGNAL(modelAboutToBeReset()), SLOT(slotAboutToBeReset()));
+    connect(m_model, SIGNAL(modelReset()), SLOT(slotReset()));
+
+    fillIndexStores(QModelIndex());
+}
+
+void ListenerObject::fillIndexStores(const QModelIndex &parent)
+{
+    const int column = 0;
+    int row = 0;
+    QModelIndex idx = m_model->index(row, column, parent);
+    while (idx.isValid())
+    {
+        m_persistentIndexes << QPersistentModelIndex(idx);
+        m_nonPersistentIndexes << idx;
+        if (m_model->hasChildren(idx))
+        {
+            fillIndexStores(idx);
+        }
+        ++row;
+        idx = m_model->index(row, column, parent);
+    }
+}
+
+void ListenerObject::slotAboutToBeReset()
+{
+    // Nothing has been changed yet. All indexes should be the same.
+    for (int i = 0; i < m_persistentIndexes.size(); ++i)
+    {
+        QModelIndex idx = m_persistentIndexes.at(i);
+        QVERIFY(idx == m_nonPersistentIndexes.at(i));
+        QVERIFY(m_model->mapToSource(idx).isValid());
+    }
+}
+
+void ListenerObject::slotReset()
+{
+    foreach(const QModelIndex &idx, m_persistentIndexes)
+    {
+        QVERIFY(!idx.isValid());
+    }
+}
+
+
+void tst_QAbstractItemModel::testReset()
+{
+    QSignalSpy beforeResetSpy(m_model, SIGNAL(modelAboutToBeReset()));
+    QSignalSpy afterResetSpy(m_model, SIGNAL(modelReset()));
+
+
+    QSortFilterProxyModel *nullProxy = new QSortFilterProxyModel(this);
+    nullProxy->setSourceModel(m_model);
+
+    // Makes sure the model and proxy are in a consistent state. before and after reset.
+    new ListenerObject(nullProxy);
+
+    ModelResetCommandFixed *resetCommand = new ModelResetCommandFixed(m_model, this);
+
+    resetCommand->setNumCols(4);
+    resetCommand->setStartRow(0);
+    resetCommand->setEndRow(0);
+    resetCommand->setDestRow(0);
+    resetCommand->setDestAncestors(QList<int>() << 5);
+    resetCommand->doCommand();
+
+    // Verify that the correct signals were emitted
+    QVERIFY(beforeResetSpy.size() == 1);
+    QVERIFY(afterResetSpy.size() == 1);
+
+    // Verify that the move actually happened.
+    QVERIFY(m_model->rowCount() == 9);
+    QModelIndex destIndex = m_model->index(4, 0);
+    QVERIFY(m_model->rowCount(destIndex) == 11);
+
+}
 
 
 QTEST_MAIN(tst_QAbstractItemModel)