[libdazzle/gbsneto/fix-model-filter] model-filter: Use correct iter when parsing added items

[Georges Basile Stavracas Neto <gbsneto src gnome org>]

commit 788d17dc8e260fd4dbab2387b386616eceb4f9d5
Author: Georges Basile Stavracas Neto <georges stavracas gmail com>
Date:   Sun Sep 9 12:30:58 2018 -0300

    model-filter: Use correct iter when parsing added items
    
    When parsing the added items from the child model, the filter
    model was retrieving the wrong iterator, causing the filter
    model to send 'items-changed' with the wrong values.
    
    This is because it was looking for "position + 1" when parsing
    the added items. Assuming that there is no filter function set,
    and the following sequence:
    
         ⥛  Item0  ⇋  Item1  ⇋  Item2  ⇋  Item3  ⥓
    
    Where each arrow represents an iterator. Now, suppose that
    filter model receives from the child model:
    
        items-changed(position=2, removed=1, added=1).
    
    Using the above example, that means "first, remove the item at
    position=2; then, add Item4 at position=2". Theoretically, the
    filter model should be like this this after those operations:
    
         ⥛  Item0  ⇋  Item1  ⇋  Item4  ⇋  Item3  ⥓
    
    And should emit the following signals:
    
       i. items-changed(position=2, n_removed=1, n_added=0)
      ii. items-changed(position=2, n_removed=0, n_added=1)
    
    That is, effectively, replacing Item2 by Item4. However, filter
    model currently does as following:
    
       i. At position=2, remove N=removed items. This will remove
          Item2:
    
         ⥛  Item0  ⇋  Item1  ⇋  Item3  ⥓
    
      ii. Emit items-changed(position=2, n_removed=1, n_added=0)
    
     iii. At position=3, add N=added items. At position=3, there
          is only the end iterator. Item4 is then added to before
          Item3, as expected.
    
         ⥛  Item0  ⇋  Item1  ⇋  Item4  ⇋  Item3 ⥓
    
      iv. Emit items-changed(position=3, n_removed=0, n_added=1)
    
    That happens because at (iii), filter model used position + 1, and
    the end iterator's position reports that value. In the example, it
    reports 3, and the filter model sends that value in 'items-changed'.
    
    This is a very visible issue when binding the filter model to a
    GtkListBox.
    
    Fix that by not adding 1 to the position at (iii). Additionally, add
    tests to make sure this behavior is maintained.

 src/util/dzl-list-model-filter.c |  2 +-
 tests/test-model-filter.c        | 67 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
---
diff --git a/src/util/dzl-list-model-filter.c b/src/util/dzl-list-model-filter.c
index c5315a9..c64e358 100644
--- a/src/util/dzl-list-model-filter.c
+++ b/src/util/dzl-list-model-filter.c
@@ -200,7 +200,7 @@ add_new_items:
 
   if (n_added > 0)
     {
-      GSequenceIter *iter = g_sequence_get_iter_at_pos (priv->child_seq, position + 1);
+      GSequenceIter *iter = g_sequence_get_iter_at_pos (priv->child_seq, position);
       GSequenceIter *filter_iter = find_next_visible_filter_iter (self, iter);
       guint filter_position = g_sequence_iter_get_position (filter_iter);
       guint count = 0;
diff --git a/tests/test-model-filter.c b/tests/test-model-filter.c
index fe69a40..6840e72 100644
--- a/tests/test-model-filter.c
+++ b/tests/test-model-filter.c
@@ -128,11 +128,78 @@ test_basic (void)
   g_clear_object (&filter);
 }
 
+static guint last_n_added = 0;
+static guint last_n_removed = 0;
+static guint last_changed_position = 0;
+
+static void
+model_items_changed_cb (DzlListModelFilter *filter,
+                        guint               position,
+                        guint               n_removed,
+                        guint               n_added,
+                        GListModel         *model)
+{
+  last_n_added = n_added;
+  last_n_removed = n_removed;
+  last_changed_position = position;
+}
+
+
+static void
+filter_items_changed_cb (DzlListModelFilter *filter,
+                         guint               position,
+                         guint               n_removed,
+                         guint               n_added,
+                         GListModel         *model)
+{
+  g_assert_cmpint (n_added, ==, last_n_added);
+  g_assert_cmpint (n_removed, ==, last_n_removed);
+  g_assert_cmpint (position, ==, last_changed_position);
+}
+
+static void
+test_items_changed (void)
+{
+  DzlListModelFilter *filter;
+  GListStore *model;
+  guint i;
+
+  model = g_list_store_new (TEST_TYPE_ITEM);
+  g_assert (model);
+
+  g_signal_connect (model, "items-changed", G_CALLBACK (model_items_changed_cb), NULL);
+
+  filter = dzl_list_model_filter_new (G_LIST_MODEL (model));
+  g_assert (filter);
+
+  g_signal_connect_after (filter, "items-changed", G_CALLBACK (filter_items_changed_cb), model);
+
+  /* The filter model is not filtered, so it must mirror whatever
+   * the child model does. In this case, test if the position of
+   * items-changed match.
+   */
+  for (i = 0; i < 100; i++)
+    {
+      g_autoptr (TestItem) val = test_item_new (i);
+      g_list_store_append (model, val);
+    }
+
+  g_assert_cmpint (100, ==, g_list_model_get_n_items (G_LIST_MODEL (model)));
+  g_assert_cmpint (100, ==, g_list_model_get_n_items (G_LIST_MODEL (filter)));
+
+  for (i = 0; i < 100; i++)
+    g_list_store_remove (model, 0);
+
+  g_clear_object (&model);
+  g_clear_object (&filter);
+}
+
 gint
 main (gint argc,
       gchar *argv[])
 {
   g_test_init (&argc, &argv, NULL);
   g_test_add_func ("/Dazzle/ListModelFilter/basic", test_basic);
+  g_test_add_func ("/Dazzle/ListModelFilter/items-changed", test_items_changed);
   return g_test_run ();
 }