From a7b953fb6f246f3d61723872ac97a1721fece128 Mon Sep 17 00:00:00 2001 From: Michael Goddard Date: Tue, 2 Nov 2010 12:14:03 +1000 Subject: QSqlTableModel/QSqlQueryModel and insertColumns problem. After inserting a column, fetching data through QSqlTableModel was off by one or more, since it passed the indexInQuery through to QSQM. Also, the headerData would sometimes return a blank string for an inserted column, and sometimes the column number. The autotests have been beefed up a little to check insertRows and insertColumns play nicely. Change-Id: I7399d4c4d94f958884b67ab9b39b5cf2485d8416 Task-number: QTBUG-12626 Reviewed-by: Charles Yin (cherry picked from commit 9c61e9a40e774fe32b16c133a5cdd6b9d9d29d83) --- src/sql/models/qsqlquerymodel.cpp | 6 +- src/sql/models/qsqltablemodel.cpp | 10 +- tests/auto/qsqldatabase/tst_databases.h | 24 ++++ tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp | 31 ++++- tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp | 166 ++++++++++++++++++++++- 5 files changed, 223 insertions(+), 14 deletions(-) diff --git a/src/sql/models/qsqlquerymodel.cpp b/src/sql/models/qsqlquerymodel.cpp index fbb442a..d1051de 100644 --- a/src/sql/models/qsqlquerymodel.cpp +++ b/src/sql/models/qsqlquerymodel.cpp @@ -279,7 +279,11 @@ QVariant QSqlQueryModel::headerData(int section, Qt::Orientation orientation, in val = d->headers.value(section).value(Qt::EditRole); if (val.isValid()) return val; - if (role == Qt::DisplayRole && d->rec.count() > section) + + // See if it's an inserted column (iiq.column() != -1) + QModelIndex dItem = indexInQuery(createIndex(0, section)); + + if (role == Qt::DisplayRole && d->rec.count() > section && dItem.column() != -1) return d->rec.fieldName(section); } return QAbstractItemModel::headerData(section, orientation, role); diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index ca491c4..4df1d63 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -425,6 +425,10 @@ QVariant QSqlTableModel::data(const QModelIndex &index, int role) const if (!index.isValid() || (role != Qt::DisplayRole && role != Qt::EditRole)) return QVariant(); + // Problem.. we need to use QSQM::indexInQuery to handle inserted columns + // but inserted rows we need to handle + // and indexInQuery is not virtual (grrr) so any values we pass to QSQM need + // to handle the insertedRows QModelIndex item = indexInQuery(index); switch (d->strategy) { @@ -452,7 +456,9 @@ QVariant QSqlTableModel::data(const QModelIndex &index, int role) const return var; break; } } - return QSqlQueryModel::data(item, role); + + // We need to handle row mapping here, but not column mapping + return QSqlQueryModel::data(index.sibling(item.row(), index.column()), role); } /*! @@ -1230,7 +1236,7 @@ int QSqlTableModel::rowCount(const QModelIndex &parent) const QModelIndex QSqlTableModel::indexInQuery(const QModelIndex &item) const { Q_D(const QSqlTableModel); - const QModelIndex it = QSqlQueryModel::indexInQuery(item); + const QModelIndex it = QSqlQueryModel::indexInQuery(item); // this adjusts columns only if (d->strategy == OnManualSubmit) { int rowOffset = 0; QSqlTableModelPrivate::CacheMap::ConstIterator i = d->cache.constBegin(); diff --git a/tests/auto/qsqldatabase/tst_databases.h b/tests/auto/qsqldatabase/tst_databases.h index 5dcf754..350e0d0 100644 --- a/tests/auto/qsqldatabase/tst_databases.h +++ b/tests/auto/qsqldatabase/tst_databases.h @@ -51,6 +51,7 @@ #include #include #include +#include #include @@ -166,6 +167,29 @@ public: return count; } + int fillTestTableWithStrategies( const QString& driverPrefix = QString() ) const + { + QTest::addColumn( "dbName" ); + QTest::addColumn("submitpolicy_i"); + int count = 0; + + for ( int i = 0; i < dbNames.count(); ++i ) { + QSqlDatabase db = QSqlDatabase::database( dbNames.at( i ) ); + + if ( !db.isValid() ) + continue; + + if ( driverPrefix.isEmpty() || db.driverName().startsWith( driverPrefix ) ) { + QTest::newRow( QString("%1 [field]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnFieldChange; + QTest::newRow( QString("%1 [row]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnRowChange; + QTest::newRow( QString("%1 [manual]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnManualSubmit; + ++count; + } + } + + return count; + } + void addDb( const QString& driver, const QString& dbName, const QString& user = QString(), const QString& passwd = QString(), const QString& host = QString(), int port = -1, const QString params = QString() ) diff --git a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp index 0ca1417..e876764 100644 --- a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp +++ b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp @@ -318,6 +318,11 @@ void tst_QSqlQueryModel::insertColumn() model.setQuery(QSqlQuery("select * from " + qTableName("test", __FILE__), db)); model.fetchMore(); // necessary??? + bool isToUpper = db.driverName().startsWith("QIBASE") || db.driverName().startsWith("QOCI") || db.driverName().startsWith("QDB2"); + const QString idColumn(isToUpper ? "ID" : "id"); + const QString nameColumn(isToUpper ? "NAME" : "name"); + const QString titleColumn(isToUpper ? "TITLE" : "title"); + QSignalSpy spy(&model, SIGNAL(columnsInserted(QModelIndex, int, int))); QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); @@ -325,6 +330,11 @@ void tst_QSqlQueryModel::insertColumn() QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); QCOMPARE(model.data(model.index(0, 3)), QVariant()); + QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), idColumn); + QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), nameColumn); + QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), titleColumn); + QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), QString("4")); + QVERIFY(model.insertColumn(1)); QCOMPARE(spy.count(), 1); @@ -344,6 +354,12 @@ void tst_QSqlQueryModel::insertColumn() QCOMPARE(model.data(model.index(0, 3)).toInt(), 1); QCOMPARE(model.data(model.index(0, 4)), QVariant()); + QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), idColumn); + QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), QString("2")); + QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), nameColumn); + QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), titleColumn); + QCOMPARE(model.headerData(4, Qt::Horizontal).toString(), QString("5")); + QVERIFY(!model.insertColumn(-1)); QVERIFY(!model.insertColumn(100)); QVERIFY(!model.insertColumn(1, model.index(1, 1))); @@ -378,14 +394,21 @@ void tst_QSqlQueryModel::insertColumn() QCOMPARE(model.indexInQuery(model.index(0, 5)).column(), -1); QCOMPARE(model.indexInQuery(model.index(0, 6)).column(), -1); - bool isToUpper = db.driverName().startsWith("QIBASE") || db.driverName().startsWith("QOCI") || db.driverName().startsWith("QDB2"); QCOMPARE(model.record().field(0).name(), QString()); - QCOMPARE(model.record().field(1).name(), isToUpper ? QString("ID") : QString("id")); + QCOMPARE(model.record().field(1).name(), idColumn); QCOMPARE(model.record().field(2).name(), QString()); - QCOMPARE(model.record().field(3).name(), isToUpper ? QString("NAME") : QString("name")); - QCOMPARE(model.record().field(4).name(), isToUpper ? QString("TITLE") : QString("title")); + QCOMPARE(model.record().field(3).name(), nameColumn); + QCOMPARE(model.record().field(4).name(), titleColumn); QCOMPARE(model.record().field(5).name(), QString()); QCOMPARE(model.record().field(6).name(), QString()); + + QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), QString("1")); + QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), idColumn); + QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), QString("3")); + QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), nameColumn); + QCOMPARE(model.headerData(4, Qt::Horizontal).toString(), titleColumn); + QCOMPARE(model.headerData(5, Qt::Horizontal).toString(), QString("6")); + QCOMPARE(model.headerData(6, Qt::Horizontal).toString(), QString("7")); } void tst_QSqlQueryModel::record() diff --git a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp index 525315f..f7d2180 100644 --- a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp +++ b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp @@ -78,11 +78,13 @@ private slots: void select_data() { generic_data(); } void select(); + void insertColumns_data() { generic_data_with_strategies(); } + void insertColumns(); void submitAll_data() { generic_data(); } void submitAll(); void setRecord_data() { generic_data(); } void setRecord(); - void insertRow_data() { generic_data(); } + void insertRow_data() { generic_data_with_strategies(); } void insertRow(); void insertRecord_data() { generic_data(); } void insertRecord(); @@ -132,6 +134,7 @@ private slots: void insertBeforeDelete(); private: void generic_data(const QString& engine=QString()); + void generic_data_with_strategies(const QString& engine=QString()); }; tst_QSqlTableModel::tst_QSqlTableModel() @@ -229,7 +232,17 @@ void tst_QSqlTableModel::recreateTestTables() void tst_QSqlTableModel::generic_data(const QString &engine) { if ( dbs.fillTestTable(engine) == 0 ) { - if(engine.isEmpty()) + if (engine.isEmpty()) + QSKIP( "No database drivers are available in this Qt configuration", SkipAll ); + else + QSKIP( (QString("No database drivers of type %1 are available in this Qt configuration").arg(engine)).toLocal8Bit(), SkipAll ); + } +} + +void tst_QSqlTableModel::generic_data_with_strategies(const QString &engine) +{ + if ( dbs.fillTestTableWithStrategies(engine) == 0 ) { + if (engine.isEmpty()) QSKIP( "No database drivers are available in this Qt configuration", SkipAll ); else QSKIP( (QString("No database drivers of type %1 are available in this Qt configuration").arg(engine)).toLocal8Bit(), SkipAll ); @@ -291,6 +304,81 @@ void tst_QSqlTableModel::select() QCOMPARE(model.data(model.index(3, 3)), QVariant()); } +void tst_QSqlTableModel::insertColumns() +{ + // Just like the select test, with extra stuff + QFETCH(QString, dbName); + QFETCH(int, submitpolicy_i); + QSqlTableModel::EditStrategy submitpolicy = (QSqlTableModel::EditStrategy) submitpolicy_i; + QSqlDatabase db = QSqlDatabase::database(dbName); + CHECK_DATABASE(db); + + QSqlTableModel model(0, db); + model.setTable(test); + model.setSort(0, Qt::AscendingOrder); + model.setEditStrategy(submitpolicy); + + QVERIFY_SQL(model, select()); + + QCOMPARE(model.rowCount(), 3); + QCOMPARE(model.columnCount(), 3); + + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 3)), QVariant()); + + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 3)), QVariant()); + + QCOMPARE(model.data(model.index(2, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 3)), QVariant()); + + QCOMPARE(model.data(model.index(3, 0)), QVariant()); + QCOMPARE(model.data(model.index(3, 1)), QVariant()); + QCOMPARE(model.data(model.index(3, 2)), QVariant()); + QCOMPARE(model.data(model.index(3, 3)), QVariant()); + + // Now add a column at 0 and 2 + model.insertColumn(0); + model.insertColumn(2); + + QCOMPARE(model.rowCount(), 3); + QCOMPARE(model.columnCount(), 5); + + QCOMPARE(model.data(model.index(0, 0)), QVariant()); + QCOMPARE(model.data(model.index(0, 1)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 2)), QVariant()); + QCOMPARE(model.data(model.index(0, 3)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 4)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 5)), QVariant()); + + QCOMPARE(model.data(model.index(1, 0)), QVariant()); + QCOMPARE(model.data(model.index(1, 1)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 2)), QVariant()); + QCOMPARE(model.data(model.index(1, 3)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 4)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 5)), QVariant()); + + QCOMPARE(model.data(model.index(2, 0)), QVariant()); + QCOMPARE(model.data(model.index(2, 1)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 2)), QVariant()); + QCOMPARE(model.data(model.index(2, 3)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(2, 4)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 5)), QVariant()); + + QCOMPARE(model.data(model.index(3, 0)), QVariant()); + QCOMPARE(model.data(model.index(3, 1)), QVariant()); + QCOMPARE(model.data(model.index(3, 2)), QVariant()); + QCOMPARE(model.data(model.index(3, 3)), QVariant()); + QCOMPARE(model.data(model.index(3, 4)), QVariant()); + QCOMPARE(model.data(model.index(3, 5)), QVariant()); +} + void tst_QSqlTableModel::setRecord() { QFETCH(QString, dbName); @@ -346,26 +434,90 @@ void tst_QSqlTableModel::setRecord() void tst_QSqlTableModel::insertRow() { QFETCH(QString, dbName); + QFETCH(int, submitpolicy_i); + QSqlTableModel::EditStrategy submitpolicy = (QSqlTableModel::EditStrategy) submitpolicy_i; QSqlDatabase db = QSqlDatabase::database(dbName); CHECK_DATABASE(db); QSqlTableModel model(0, db); - model.setEditStrategy(QSqlTableModel::OnRowChange); + model.setEditStrategy(submitpolicy); model.setTable(test); model.setSort(0, Qt::AscendingOrder); QVERIFY_SQL(model, select()); + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(2, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 3); + QVERIFY(model.insertRow(2)); + + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(2, 0)).toInt(), 0); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString()); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 0); + QCOMPARE(model.data(model.index(3, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(3, 2)).toInt(), 3); + QSqlRecord rec = model.record(1); rec.setValue(0, 42); - rec.setValue(1, QString("vohi")); + rec.setValue(1, QString("francis")); + + // FieldChange updates immediately and resorts + // Row/Manual submit does not resort QVERIFY(model.setRecord(2, rec)); - QCOMPARE(model.data(model.index(2, 0)).toInt(), 42); - QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); - QCOMPARE(model.data(model.index(2, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); + + // See comment above setRecord + if (submitpolicy == QSqlTableModel::OnFieldChange) { + QCOMPARE(model.data(model.index(2, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 3); + QCOMPARE(model.data(model.index(3, 0)).toInt(), 42); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis")); + QCOMPARE(model.data(model.index(3, 2)).toInt(), 2); + } else { + QCOMPARE(model.data(model.index(2, 0)).toInt(), 42); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("francis")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(3, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(3, 2)).toInt(), 3); + } QVERIFY(model.submitAll()); + + // After the submit we should have the resorted view + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(2, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 3); + QCOMPARE(model.data(model.index(3, 0)).toInt(), 42); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis")); + QCOMPARE(model.data(model.index(3, 2)).toInt(), 2); + } void tst_QSqlTableModel::insertRecord() -- cgit v0.12