[rhythmbox] rhythmdb: merge change lists from subsequent commits (bug #527898)

[Jonathan Matthew <jmatthew src gnome org>]

commit 754a72759885d63275b1a656d8c6f3f7c29c7e9d
Author: Jonathan Matthew <jonathan d14n org>
Date:   Mon Mar 15 22:47:22 2010 +1000

    rhythmdb: merge change lists from subsequent commits (bug #527898)
    
    When a second commit occurs before the changes from the first are
    emitted (in an idle handler), and both commits contain changes to the
    same entry, we need to combine the changes in the change map used to
    prepare for signal emission.
    
    Previously, the changes from the first commit were being overwritten,
    with the result that property models could get out of sync with the
    entries in the backing model, which would eventually lead to an
    assertion failure when trying to update the property model.

 rhythmdb/rhythmdb.c   |   18 +++++++++++++++++-
 tests/test-rhythmdb.c |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletions(-)
---
diff --git a/rhythmdb/rhythmdb.c b/rhythmdb/rhythmdb.c
index 1b6f120..d459a66 100644
--- a/rhythmdb/rhythmdb.c
+++ b/rhythmdb/rhythmdb.c
@@ -1416,6 +1416,7 @@ process_changed_entries_cb (RhythmDBEntry *entry,
 			    GSList *changes,
 			    RhythmDB *db)
 {
+	GSList *existing;
 	if (db->priv->changed_entries_to_emit == NULL) {
 		db->priv->changed_entries_to_emit = g_hash_table_new_full (NULL,
 									   NULL,
@@ -1423,7 +1424,22 @@ process_changed_entries_cb (RhythmDBEntry *entry,
 									   (GDestroyNotify) free_entry_changes);
 	}
 
-	g_hash_table_insert (db->priv->changed_entries_to_emit, rhythmdb_entry_ref (entry), changes);
+	/* if the entry is already in the change map from a previous commit, add the
+	 * new changes to the end of the existing list.
+	 */
+	existing = g_hash_table_lookup (db->priv->changed_entries_to_emit, entry);
+	if (existing != NULL) {
+		changes = g_slist_concat (existing, changes);
+
+		/* steal the hash entry so it doesn't free the changes; also means we
+		 * don't need to add a reference on the entry.
+		 */
+		g_hash_table_steal (db->priv->changed_entries_to_emit, entry);
+	} else {
+		rhythmdb_entry_ref (entry);
+	}
+
+	g_hash_table_insert (db->priv->changed_entries_to_emit, entry, changes);
 	return TRUE;
 }
 
diff --git a/tests/test-rhythmdb.c b/tests/test-rhythmdb.c
index b495297..77425f6 100644
--- a/tests/test-rhythmdb.c
+++ b/tests/test-rhythmdb.c
@@ -471,6 +471,42 @@ START_TEST (test_rhythmdb_modify_after_delete)
 }
 END_TEST
 
+static void
+commit_change_merge_cb (RhythmDB *db, RhythmDBEntry *entry, GSList *changes, gpointer ok)
+{
+	int expected = GPOINTER_TO_INT (ok);
+	fail_unless (g_slist_length (changes) == expected, "commit change lists merged");
+}
+
+START_TEST (test_rhythmdb_commit_change_merging)
+{
+	RhythmDBEntry *entry;
+	GValue val = {0,};
+
+	entry = rhythmdb_entry_new (db, RHYTHMDB_ENTRY_TYPE_IGNORE, "file:///whee.ogg");
+	fail_unless (entry != NULL, "failed to create entry");
+
+	rhythmdb_commit (db);
+
+	g_value_init (&val, G_TYPE_STRING);
+	g_value_set_static_string (&val, "Anything");
+	rhythmdb_entry_set (db, entry, RHYTHMDB_PROP_GENRE, &val);
+	g_value_unset (&val);
+
+	rhythmdb_commit (db);
+
+	g_value_init (&val, G_TYPE_STRING);
+	g_value_set_static_string (&val, "Nothing");
+	rhythmdb_entry_set (db, entry, RHYTHMDB_PROP_ARTIST, &val);
+	g_value_unset (&val);
+
+	g_signal_connect (G_OBJECT (db), "entry-changed", G_CALLBACK (commit_change_merge_cb), GINT_TO_POINTER (2));
+	set_waiting_signal (G_OBJECT (db), "entry-changed");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+}
+END_TEST
+
 static Suite *
 rhythmdb_suite (void)
 {
@@ -500,6 +536,7 @@ rhythmdb_suite (void)
 	/* tests for breakable bug fixes */
 	tcase_add_test (tc_chain, test_rhythmdb_podcast_upgrade);
 	tcase_add_test (tc_chain, test_rhythmdb_modify_after_delete);
+	tcase_add_test (tc_chain, test_rhythmdb_commit_change_merging);
 
 	return s;
 }