Add tests for the VisualDataGroup onChanged signal.

And other things.  Fixes onChanged being emitted with an empty change
list, an out of bounds array access in groups property and model parts
group filter not being correctly reset.

Change-Id: If2f27b303a141634d106b7c5164984e5817aff4e
Reviewed-by: Martin Jones <martin.jones@nokia.com>

5 files changed, 269 insertions, 9 deletions

diff --git a/src/declarative/items/qquickvisualdatamodel.cpp b/src/declarative/items/qquickvisualdatamodel.cpp
index 8852dce3cc..5cf2d6c9b3 100644
--- a/src/declarative/items/qquickvisualdatamodel.cpp
+++ b/src/declarative/items/qquickvisualdatamodel.cpp
@@ -766,7 +766,7 @@ QQuickVisualDataGroup *QQuickVisualDataModelPrivate::group_at(
 {
     QQuickVisualDataModelPrivate *d = static_cast<QQuickVisualDataModelPrivate *>(property->data);
     return index >= 0 && index < d->m_groupCount - 1
-            ? d->m_groups[index - 1]
+            ? d->m_groups[index + 1]
             : 0;
 }
 
@@ -1865,7 +1865,7 @@ void QQuickVisualDataGroupPrivate::emitChanges(QV8Engine *engine)
 {
     Q_Q(QQuickVisualDataGroup);
     static int idx = signalIndex("changed(QDeclarativeV8Handle,QDeclarativeV8Handle)");
-    if (isSignalConnected(idx)) {
+    if (isSignalConnected(idx) && !changeSet.isEmpty()) {
         v8::HandleScope handleScope;
         v8::Context::Scope contextScope(engine->context());
         v8::Local<v8::Array> removed  = QQuickVisualDataModelPrivate::buildChangeList(changeSet.removes());
@@ -2372,8 +2372,11 @@ void QQuickVisualPartsModel::updateFilterGroup()
     if (!model->m_cacheMetaType)
         return;
 
-    if (m_inheritGroup)
-        return;
+    if (m_inheritGroup) {
+        if (m_filterGroup == model->m_filterGroup)
+            return;
+        m_filterGroup = model->m_filterGroup;
+    }
 
     QDeclarativeListCompositor::Group previousGroup = m_compositorGroup;
     m_compositorGroup = Compositor::Default;
diff --git a/tests/auto/declarative/qquickvisualdatamodel/data/create.qml b/tests/auto/declarative/qquickvisualdatamodel/data/create.qml
index 36ea3baf76..3475a0dace 100644
--- a/tests/auto/declarative/qquickvisualdatamodel/data/create.qml
+++ b/tests/auto/declarative/qquickvisualdatamodel/data/create.qml
@@ -7,6 +7,8 @@ ListView {
     model: VisualDataModel {
         id: visualModel
 
+        persistedItems.includeByDefault: true
+
         model: myModel
         delegate: Item {
             id: delegate
@@ -16,6 +18,7 @@ ListView {
 
             property bool destroyed: false
 
+
             Component.onDestruction: destroyed = true
         }
     }
diff --git a/tests/auto/declarative/qquickvisualdatamodel/data/groups-invalid.qml b/tests/auto/declarative/qquickvisualdatamodel/data/groups-invalid.qml
new file mode 100644
index 0000000000..70c6f9f995
--- /dev/null
+++ b/tests/auto/declarative/qquickvisualdatamodel/data/groups-invalid.qml
@@ -0,0 +1,14 @@
+import QtQuick 2.0
+
+VisualDataModel {
+    id: visualModel
+
+    objectName: "visualModel"
+
+    groups: [
+        VisualDataGroup { id: visibleItems; objectName: "visibleItems"; name: "visible"; includeByDefault: true },
+        VisualDataGroup { id: selectedItems; objectName: "selectedItems"; name: "selected" },
+        VisualDataGroup { id: unnamed; objectName: "unnamed" },
+        VisualDataGroup { id: capitalised; objectName: "capitalised"; name: "Capitalised" }
+    ]
+}
diff --git a/tests/auto/declarative/qquickvisualdatamodel/data/onChanged.qml b/tests/auto/declarative/qquickvisualdatamodel/data/onChanged.qml
new file mode 100644
index 0000000000..71dc7d72d7
--- /dev/null
+++ b/tests/auto/declarative/qquickvisualdatamodel/data/onChanged.qml
@@ -0,0 +1,87 @@
+import QtQuick 2.0
+
+VisualDataModel {
+    id: vm
+
+    property var inserted
+    property var removed
+
+    Component.onCompleted: {
+        vm.inserted = []
+        vm.removed = []
+        vi.inserted = []
+        vi.removed = []
+        si.inserted = []
+        si.removed = []
+    }
+
+    function verify(changes, indexes, counts, moveIds) {
+        if (changes.length != indexes.length
+                || changes.length != counts.length
+                || changes.length != moveIds.length) {
+            console.log("invalid length", changes.length, indexes.length, counts.length, moveIds.length)
+            return false
+        }
+
+        var valid = true;
+        for (var i = 0; i < changes.length; ++i) {
+            if (changes[i].index != indexes[i]) {
+                console.log(i, "incorrect index. actual:", changes[i].index, "expected:", indexes[i])
+                valid = false;
+            }
+            if (changes[i].count != counts[i]) {
+                console.log(i, "incorrect count. actual:", changes[i].count, "expected:", counts[i])
+                valid = false;
+            }
+            if (changes[i].moveId != moveIds[i]) {
+                console.log(i, "incorrect moveId. actual:", changes[i].moveId, "expected:", moveIds[i])
+                valid = false;
+            }
+        }
+        return valid
+    }
+
+    groups: [
+        VisualDataGroup {
+            id: vi;
+
+            property var inserted
+            property var removed
+
+            name: "visible"
+            includeByDefault: true
+
+            onChanged: {
+                vi.inserted = inserted
+                vi.removed = removed
+            }
+        },
+        VisualDataGroup {
+            id: si;
+
+            property var inserted
+            property var removed
+
+            name: "selected"
+            onChanged: {
+                si.inserted = inserted
+                si.removed = removed
+            }
+        }
+    ]
+
+    model: ListModel {
+        id: listModel
+        ListElement { number: "one" }
+        ListElement { number: "two" }
+        ListElement { number: "three" }
+        ListElement { number: "four" }
+    }
+
+    delegate: Item {}
+
+    items.onChanged: {
+        vm.inserted = inserted
+        vm.removed = removed
+    }
+}
diff --git a/tests/auto/declarative/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp b/tests/auto/declarative/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
index c997ec8c20..706c20fa15 100644
--- a/tests/auto/declarative/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
+++ b/tests/auto/declarative/qquickvisualdatamodel/tst_qquickvisualdatamodel.cpp
@@ -139,7 +139,10 @@ private slots:
     void move();
     void groups_data();
     void groups();
+    void invalidGroups();
     void get();
+    void onChanged_data();
+    void onChanged();
     void create();
 
 private:
@@ -1208,25 +1211,25 @@ void tst_qquickvisualdatamodel::groups()
         QCOMPARE(selectedItems->count(), 2);
     } {
         evaluate<void>(visualModel, part + "filterOnGroup = \"visible\"");
-        qDebug() << "listview->count()" << listview->count();
         QCOMPARE(listview->count(), 9);
         QCOMPARE(visualModel->items()->count(), 12);
         QCOMPARE(visibleItems->count(), 9);
         QCOMPARE(selectedItems->count(), 2);
+        QCOMPARE(evaluate<QString>(visualModel, part + "filterOnGroup"), QString("visible"));
     } {
         evaluate<void>(visualModel, part + "filterOnGroup = \"selected\"");
-        qDebug() << "listview->count()" << listview->count();
         QCOMPARE(listview->count(), 2);
         QCOMPARE(visualModel->items()->count(), 12);
         QCOMPARE(visibleItems->count(), 9);
         QCOMPARE(selectedItems->count(), 2);
+        QCOMPARE(evaluate<QString>(visualModel, part + "filterOnGroup"), QString("selected"));
     } {
-        evaluate<void>(visualModel, part + "filterOnGroup = \"items\"");
-        qDebug() << "listview->count()" << listview->count();
+        evaluate<void>(visualModel, part + "filterOnGroup = undefined");
         QCOMPARE(listview->count(), 12);
         QCOMPARE(visualModel->items()->count(), 12);
         QCOMPARE(visibleItems->count(), 9);
         QCOMPARE(selectedItems->count(), 2);
+        QCOMPARE(evaluate<QString>(visualModel, part + "filterOnGroup"), QString("items"));
     } {
         QQuickItem *delegate = findItem<QQuickItem>(contentItem, "delegate", 5);
         QVERIFY(delegate);
@@ -1490,6 +1493,144 @@ void tst_qquickvisualdatamodel::get()
     }
 }
 
+void tst_qquickvisualdatamodel::invalidGroups()
+{
+    QUrl source = QUrl::fromLocalFile(TESTDATA("groups-invalid.qml"));
+    QTest::ignoreMessage(QtWarningMsg, (source.toString() + ":12:9: QML VisualDataGroup: " + QQuickVisualDataGroup::tr("Group names must start with a lower case letter")).toUtf8());
+
+    QDeclarativeComponent component(&engine, source);
+    QScopedPointer<QObject> object(component.create());
+    QVERIFY(object);
+
+    QCOMPARE(evaluate<int>(object.data(), "groups.length"), 4);
+    QCOMPARE(evaluate<QString>(object.data(), "groups[0].name"), QString("items"));
+    QCOMPARE(evaluate<QString>(object.data(), "groups[1].name"), QString("persistedItems"));
+    QCOMPARE(evaluate<QString>(object.data(), "groups[2].name"), QString("visible"));
+    QCOMPARE(evaluate<QString>(object.data(), "groups[3].name"), QString("selected"));
+}
+
+void tst_qquickvisualdatamodel::onChanged_data()
+{
+    QTest::addColumn<QString>("expression");
+    QTest::addColumn<QStringList>("tests");
+
+    QTest::newRow("item appended")
+            << QString("listModel.append({\"number\": \"five\"})")
+            << (QStringList()
+                << "verify(vm.removed, [], [], [])"
+                << "verify(vm.inserted, [4], [1], [undefined])"
+                << "verify(vi.removed, [], [], [])"
+                << "verify(vi.inserted, [4], [1], [undefined])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+    QTest::newRow("item prepended")
+            << QString("listModel.insert(0, {\"number\": \"five\"})")
+            << (QStringList()
+                << "verify(vm.removed, [], [], [])"
+                << "verify(vm.inserted, [0], [1], [undefined])"
+                << "verify(vi.removed, [], [], [])"
+                << "verify(vi.inserted, [0], [1], [undefined])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+    QTest::newRow("item inserted")
+            << QString("listModel.insert(2, {\"number\": \"five\"})")
+            << (QStringList()
+                << "verify(vm.removed, [], [], [])"
+                << "verify(vm.inserted, [2], [1], [undefined])"
+                << "verify(vi.removed, [], [], [])"
+                << "verify(vi.inserted, [2], [1], [undefined])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+
+    QTest::newRow("item removed tail")
+            << QString("listModel.remove(3)")
+            << (QStringList()
+                << "verify(vm.removed, [3], [1], [undefined])"
+                << "verify(vm.inserted, [], [], [])"
+                << "verify(vi.removed, [3], [1], [undefined])"
+                << "verify(vi.inserted, [], [], [])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+    QTest::newRow("item removed head")
+            << QString("listModel.remove(0)")
+            << (QStringList()
+                << "verify(vm.removed, [0], [1], [undefined])"
+                << "verify(vm.inserted, [], [], [])"
+                << "verify(vi.removed, [0], [1], [undefined])"
+                << "verify(vi.inserted, [], [], [])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+    QTest::newRow("item removed middle")
+            << QString("listModel.remove(1)")
+            << (QStringList()
+                << "verify(vm.removed, [1], [1], [undefined])"
+                << "verify(vm.inserted, [], [], [])"
+                << "verify(vi.removed, [1], [1], [undefined])"
+                << "verify(vi.inserted, [], [], [])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+
+
+    QTest::newRow("item moved from tail")
+            << QString("listModel.move(3, 0, 1)")
+            << (QStringList()
+                << "verify(vm.removed, [3], [1], [vm.inserted[0].moveId])"
+                << "verify(vm.inserted, [0], [1], [vm.removed[0].moveId])"
+                << "verify(vi.removed, [3], [1], [vi.inserted[0].moveId])"
+                << "verify(vi.inserted, [0], [1], [vi.removed[0].moveId])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+    QTest::newRow("item moved from head")
+            << QString("listModel.move(0, 2, 2)")
+            << (QStringList()
+                << "verify(vm.removed, [0], [2], [vm.inserted[0].moveId])"
+                << "verify(vm.inserted, [2], [2], [vm.removed[0].moveId])"
+                << "verify(vi.removed, [0], [2], [vi.inserted[0].moveId])"
+                << "verify(vi.inserted, [2], [2], [vi.removed[0].moveId])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+
+    QTest::newRow("groups changed")
+            << QString("items.setGroups(1, 2, [\"items\", \"selected\"])")
+            << (QStringList()
+                << "verify(vm.inserted, [], [], [])"
+                << "verify(vm.removed, [], [], [])"
+                << "verify(vi.removed, [1], [2], [undefined])"
+                << "verify(vi.inserted, [], [], [])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [0], [2], [undefined])");
+
+    QTest::newRow("multiple removes")
+            << QString("{ vi.remove(1, 1); "
+                       "vi.removeGroups(0, 2, \"items\") }")
+            << (QStringList()
+                << "verify(vm.removed, [0, 1], [1, 1], [undefined, undefined])"
+                << "verify(vm.inserted, [], [], [])"
+                << "verify(vi.removed, [1], [1], [undefined])"
+                << "verify(vi.inserted, [], [], [])"
+                << "verify(si.removed, [], [], [])"
+                << "verify(si.inserted, [], [], [])");
+}
+
+void tst_qquickvisualdatamodel::onChanged()
+{
+    QFETCH(QString, expression);
+    QFETCH(QStringList, tests);
+
+    QDeclarativeComponent component(&engine, QUrl::fromLocalFile(TESTDATA("onChanged.qml")));
+    QScopedPointer<QObject> object(component.create());
+    QVERIFY(object);
+
+    evaluate<void>(object.data(), expression);
+
+    foreach (const QString &test, tests) {
+        bool passed = evaluate<bool>(object.data(), test);
+        if (!passed)
+            qWarning() << test;
+        QVERIFY(passed);
+    }
+}
+
 void tst_qquickvisualdatamodel::create()
 {
     QQuickView view;
@@ -1535,8 +1676,20 @@ void tst_qquickvisualdatamodel::create()
 
     QQuickItem *delegate;
 
+    // persistedItems.includeByDefault is true, so all items belong to persistedItems initially.
+    QVERIFY(delegate = findItem<QQuickItem>(contentItem, "delegate", 1));
+    QCOMPARE(evaluate<bool>(delegate, "VisualDataModel.inPersistedItems"), true);
+
+    // changing include by default doesn't remove persistance.
+    evaluate<void>(visualModel, "persistedItems.includeByDefault = false");
+    QCOMPARE(evaluate<bool>(delegate, "VisualDataModel.inPersistedItems"), true);
+
+    // removing from persistedItems does.
+    evaluate<void>(visualModel, "persistedItems.remove(0, 20)");
+    QCOMPARE(listview->count(), 20);
+    QCOMPARE(evaluate<bool>(delegate, "VisualDataModel.inPersistedItems"), false);
+
     // Request an item instantiated by the view.
-    QVERIFY(findItem<QQuickItem>(contentItem, "delegate", 1));
     QVERIFY(delegate = qobject_cast<QQuickItem *>(evaluate<QObject *>(visualModel, "items.create(1)")));
     QCOMPARE(delegate, findItem<QQuickItem>(contentItem, "delegate", 1));
     QCOMPARE(evaluate<bool>(delegate, "VisualDataModel.inPersistedItems"), true);