TableInstanceModel: handle roleName invalidation

roleNames are generally guaranteed to be stable (given that QAIM has no
change signal for them), except that resetting the model is allowed to
invalidate them. TableInstanceModel did so far not take this into account.

Handle this case correctly by snapshotting the current roleNames before
the model is reset. Afterwards, if we detect that roleNames has changed,
we throw the current model set up away and rebuild everything from
scratch – it is unlikely that a more efficient implementation would be
worth it.

Fixes: QTBUG-111987
Pick-to: 6.5 6.2
Change-Id: Id1e3b8e4f983c0f00fc7b30bd4897f1f7fcc3792
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

5 files changed, 91 insertions, 7 deletions

diff --git a/src/qmlmodels/qqmltableinstancemodel.cpp b/src/qmlmodels/qqmltableinstancemodel.cpp
index e4fcae2a44..dcc15f90a5 100644
--- a/src/qmlmodels/qqmltableinstancemodel.cpp
+++ b/src/qmlmodels/qqmltableinstancemodel.cpp
@@ -436,11 +436,15 @@ void QQmlTableInstanceModel::setModel(const QVariant &model)
     // needs to stay in sync with the model. So we need to drain the pool
     // completely when the model changes.
     drainReusableItemsPool(0);
-    if (auto const aim = abstractItemModel())
+    if (auto const aim = abstractItemModel()) {
         disconnect(aim, &QAbstractItemModel::dataChanged, this, &QQmlTableInstanceModel::dataChangedCallback);
+        disconnect(aim, &QAbstractItemModel::modelAboutToBeReset, this, &QQmlTableInstanceModel::modelAboutToBeResetCallback);
+    }
     m_adaptorModel.setModel(model);
-    if (auto const aim = abstractItemModel())
+    if (auto const aim = abstractItemModel()) {
         connect(aim, &QAbstractItemModel::dataChanged, this, &QQmlTableInstanceModel::dataChangedCallback);
+        connect(aim, &QAbstractItemModel::modelAboutToBeReset, this, &QQmlTableInstanceModel::modelAboutToBeResetCallback);
+    }
 }
 
 void QQmlTableInstanceModel::dataChangedCallback(const QModelIndex &begin, const QModelIndex &end, const QVector<int> &roles)
@@ -458,6 +462,21 @@ void QQmlTableInstanceModel::dataChangedCallback(const QModelIndex &begin, const
     }
 }
 
+void QQmlTableInstanceModel::modelAboutToBeResetCallback()
+{
+    // When the model is reset, we can no longer rely on any of the data it has
+    // provided us so far. Normally it's enough for the view to recreate all the
+    // delegate items in that case, except if the model roles has changed as well
+    // (since those are cached by QQmlAdaptorModel / Accessors). For the latter case, we
+    // simply set the model once more in the delegate model to rebuild everything.
+    auto const aim = abstractItemModel();
+    auto oldRoleNames = aim->roleNames();
+    QObject::connect(aim, &QAbstractItemModel::modelReset, this, [this, aim, oldRoleNames](){
+        if (oldRoleNames != aim->roleNames())
+            setModel(model());
+    }, Qt::SingleShotConnection);
+}
+
 QQmlComponent *QQmlTableInstanceModel::delegate() const
 {
     return m_delegate;
diff --git a/src/qmlmodels/qqmltableinstancemodel_p.h b/src/qmlmodels/qqmltableinstancemodel_p.h
index 6d3f455adc..c7e7caf593 100644
--- a/src/qmlmodels/qqmltableinstancemodel_p.h
+++ b/src/qmlmodels/qqmltableinstancemodel_p.h
@@ -115,6 +115,7 @@ private:
     void destroyModelItem(QQmlDelegateModelItem *modelItem, DestructionMode mode);
 
     void dataChangedCallback(const QModelIndex &begin, const QModelIndex &end, const QVector<int> &roles);
+    void modelAboutToBeResetCallback();
 
     static bool isDoneIncubating(QQmlDelegateModelItem *modelItem);
     static void deleteModelItemLater(QQmlDelegateModelItem *modelItem);
diff --git a/tests/auto/quick/qquicktableview/data/resetModelData.qml b/tests/auto/quick/qquicktableview/data/resetModelData.qml
new file mode 100644
index 0000000000..f7b3ec3009
--- /dev/null
+++ b/tests/auto/quick/qquicktableview/data/resetModelData.qml
@@ -0,0 +1,25 @@
+import QtQuick
+
+Item {
+    width: 640
+    height: 450
+
+    property alias tableView: tableView
+
+    TableView {
+        id: tableView
+        anchors.fill: parent
+        height: 300
+        width: 200
+
+        property bool success: false
+
+        delegate: Rectangle {
+            required property var model
+            implicitWidth: 100
+            implicitHeight: 50
+            property var mydata: model?.custom ?? model.display
+            onMydataChanged: tableView.success = mydata === 42
+        }
+    }
+}
diff --git a/tests/auto/quick/qquicktableview/testmodel.h b/tests/auto/quick/qquicktableview/testmodel.h
index 02a3478bdd..71d58d6602 100644
--- a/tests/auto/quick/qquicktableview/testmodel.h
+++ b/tests/auto/quick/qquicktableview/testmodel.h
@@ -36,13 +36,27 @@ public:
 
     QVariant data(const QModelIndex &index, int role) const override
     {
-        if (!index.isValid() || role != Qt::DisplayRole)
+        if (!index.isValid())
             return QVariant();
 
-        int serializedIndex = index.row() + (index.column() * m_columns);
-        if (modelData.contains(serializedIndex))
-            return modelData.value(serializedIndex);
-        return QStringLiteral("%1").arg(index.row());
+        QVariant ret;
+
+        switch (role) {
+        case Qt::UserRole:
+            ret = 42;
+            break;
+        case Qt::DisplayRole: {
+            int serializedIndex = index.row() + (index.column() * m_columns);
+            if (modelData.contains(serializedIndex))
+                ret = modelData.value(serializedIndex);
+            else
+                ret = QStringLiteral("%1").arg(index.row()); }
+            break;
+        default:
+            break;
+        }
+
+        return ret;
     }
 
     Q_INVOKABLE QVariant dataFromSerializedIndex(int index) const
@@ -54,9 +68,18 @@ public:
 
     QHash<int, QByteArray> roleNames() const override
     {
+        if (m_useCustomRoleNames)
+            return { { Qt::UserRole, "custom"} };
         return { {Qt::DisplayRole, "display"} };
     }
 
+    Q_INVOKABLE void useCustomRoleNames(bool use)
+    {
+        beginResetModel();
+        m_useCustomRoleNames = use;
+        endResetModel();
+    }
+
     Q_INVOKABLE void setModelData(const QPoint &cell, const QSize &span, const QString &string)
     {
         for (int c = 0; c < span.width(); ++c) {
@@ -178,6 +201,7 @@ private:
     int m_rows = 0;
     int m_columns = 0;
     bool m_dataCanBeFetched = false;
+    bool m_useCustomRoleNames = false;
     QHash<int, QString> modelData;
     Qt::ItemFlags m_flags = Qt::ItemIsSelectable | Qt::ItemIsEnabled | Qt::ItemIsEditable;
 };
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index 1869539c36..84cfe3b62d 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -269,6 +269,7 @@ private slots:
     void editWarning_nonEditableModelItem();
     void attachedPropertiesOnEditDelegate();
     void requiredPropertiesOnEditDelegate();
+    void resettingRolesRespected();
 };
 
 tst_QQuickTableView::tst_QQuickTableView()
@@ -7295,6 +7296,20 @@ void tst_QQuickTableView::requiredPropertiesOnEditDelegate()
     QCOMPARE(textInput->property("current").toBool(), false);
 }
 
+void tst_QQuickTableView::resettingRolesRespected()
+{
+    LOAD_TABLEVIEW("resetModelData.qml");
+
+    TestModel model(1, 1);
+    tableView->setModel(QVariant::fromValue(&model));
+
+    WAIT_UNTIL_POLISHED;
+
+    QVERIFY(!tableView->property("success").toBool());
+    model.useCustomRoleNames(true);
+    QTRY_VERIFY(tableView->property("success").toBool());
+}
+
 QTEST_MAIN(tst_QQuickTableView)
 
 #include "tst_qquicktableview.moc"