From c194b7f3454f470d24be729ef660c5cfe7a9b841 Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Wed, 29 Aug 2012 09:34:41 +0200 Subject: [PATCH] QSqlQueryModel: fix nested beginResetModel/endResetModel Follow-up to 83c9ebbd6692cde99ee692e6549c591100f12545. Consider the case where calls to the reset methods on the same object are nested as in the following sequence: 1. beginResetModel() 2. beginResetModel() 3. endResetModel() 4. endResetModel() In such cases, only the outermost calls, i.e., 1) and 4), should emit signals. After 83c9ebbd6692cde99ee692e6549c591100f12545, 1) and 3) emitted the signals, which is wrong. This is corrected by keeping track of the nesting level. Such sequences can come about when a base class calls the begin/end methods between the calls made by the subclass. QSqlTableModel::select() is an example of this. Test included. Change-Id: Ia62b45cb1abaab00a32bb8357de4a958bcff83e5 Reviewed-by: Andy Shaw Reviewed-by: Olivier Goffart --- src/sql/models/qsqlquerymodel.cpp | 14 ++--- src/sql/models/qsqlquerymodel_p.h | 4 +- .../qsqlquerymodel/tst_qsqlquerymodel.cpp | 57 +++++++++++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/sql/models/qsqlquerymodel.cpp b/src/sql/models/qsqlquerymodel.cpp index dc9ef0d616a..ea3bc7f20b7 100644 --- a/src/sql/models/qsqlquerymodel.cpp +++ b/src/sql/models/qsqlquerymodel.cpp @@ -78,10 +78,10 @@ void QSqlQueryModelPrivate::prefetch(int limit) atEnd = true; // this is the end. } if (newBottom.row() >= 0 && newBottom.row() > bottom.row()) { - if (!resetting) + if (!nestedResetLevel) q->beginInsertRows(QModelIndex(), bottom.row() + 1, newBottom.row()); bottom = newBottom; - if (!resetting) + if (!nestedResetLevel) q->endInsertRows(); } else { bottom = newBottom; @@ -215,10 +215,9 @@ bool QSqlQueryModel::canFetchMore(const QModelIndex &parent) const void QSqlQueryModel::beginResetModel() { Q_D(QSqlQueryModel); - if (!d->resetting) { + if (!d->nestedResetLevel) QAbstractTableModel::beginResetModel(); - d->resetting = true; - } + ++d->nestedResetLevel; } /*! \internal @@ -226,10 +225,9 @@ void QSqlQueryModel::beginResetModel() void QSqlQueryModel::endResetModel() { Q_D(QSqlQueryModel); - if (d->resetting) { - d->resetting = false; + --d->nestedResetLevel; + if (!d->nestedResetLevel) QAbstractTableModel::endResetModel(); - } } /*! \fn int QSqlQueryModel::rowCount(const QModelIndex &parent) const diff --git a/src/sql/models/qsqlquerymodel_p.h b/src/sql/models/qsqlquerymodel_p.h index 3288d311a4f..70f72f393f9 100644 --- a/src/sql/models/qsqlquerymodel_p.h +++ b/src/sql/models/qsqlquerymodel_p.h @@ -67,7 +67,7 @@ class QSqlQueryModelPrivate: public QAbstractItemModelPrivate { Q_DECLARE_PUBLIC(QSqlQueryModel) public: - QSqlQueryModelPrivate() : atEnd(false), resetting(false) {} + QSqlQueryModelPrivate() : atEnd(false), nestedResetLevel(0) {} ~QSqlQueryModelPrivate(); void prefetch(int); @@ -80,7 +80,7 @@ public: uint atEnd : 1; QVector > headers; QVarLengthArray colOffsets; // used to calculate indexInQuery of columns - bool resetting; + int nestedResetLevel; }; // helpers for building SQL expressions diff --git a/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp b/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp index 1c1319630e3..582a76b1191 100644 --- a/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp +++ b/tests/auto/sql/models/qsqlquerymodel/tst_qsqlquerymodel.cpp @@ -91,6 +91,8 @@ private slots: void setQuerySignalEmission(); void setQueryWithNoRowsInResultSet_data() { generic_data(); } void setQueryWithNoRowsInResultSet(); + void nestedResets_data() { generic_data(); } + void nestedResets(); void task_180617(); void task_180617_data() { generic_data(); } @@ -585,6 +587,61 @@ void tst_QSqlQueryModel::setQueryWithNoRowsInResultSet() QCOMPARE(modelRowsInsertedSpy.count(), 0); } +class NestedResetsTest: public QSqlQueryModel +{ + Q_OBJECT + +public: + NestedResetsTest(QObject* parent = 0) : QSqlQueryModel(parent), gotAboutToBeReset(false), gotReset(false) + { + connect(this, SIGNAL(modelAboutToBeReset()), this, SLOT(modelAboutToBeResetSlot())); + connect(this, SIGNAL(modelReset()), this, SLOT(modelResetSlot())); + } + + void testme() + { + // Only the outermost beginResetModel/endResetModel should + // emit signals. + gotAboutToBeReset = gotReset = false; + beginResetModel(); + QCOMPARE(gotAboutToBeReset, true); + QCOMPARE(gotReset, false); + + gotAboutToBeReset = gotReset = false; + beginResetModel(); + QCOMPARE(gotAboutToBeReset, false); + QCOMPARE(gotReset, false); + + gotAboutToBeReset = gotReset = false; + endResetModel(); + QCOMPARE(gotAboutToBeReset, false); + QCOMPARE(gotReset, false); + + gotAboutToBeReset = gotReset = false; + endResetModel(); + QCOMPARE(gotAboutToBeReset, false); + QCOMPARE(gotReset, true); + } + +private slots: + void modelAboutToBeResetSlot() { gotAboutToBeReset = true; } + void modelResetSlot() { gotReset = true; } + +private: + bool gotAboutToBeReset; + bool gotReset; +}; + +void tst_QSqlQueryModel::nestedResets() +{ + QFETCH(QString, dbName); + QSqlDatabase db = QSqlDatabase::database(dbName); + CHECK_DATABASE(db); + + NestedResetsTest t; + t.testme(); +} + // For task 180617 // According to the task, several specific duplicate SQL queries would cause // multiple empty grid lines to be visible in the view