From 754a72759885d63275b1a656d8c6f3f7c29c7e9d Mon Sep 17 00:00:00 2001 From: Jonathan Matthew Date: Mon, 15 Mar 2010 12:47:22 +0000 Subject: 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. --- diff --git a/rhythmdb/rhythmdb.c b/rhythmdb/rhythmdb.c index 1b6f120..d459a66 100644 diff -upr rhythmbox-0.12.6.old/rhythmdb/rhythmdb.c rhythmbox-0.12.6/rhythmdb/rhythmdb.c --- rhythmbox-0.12.6.old/rhythmdb/rhythmdb.c 2009-11-21 00:31:38.000000000 +0000 +++ rhythmbox-0.12.6/rhythmdb/rhythmdb.c 2010-03-15 13:32:37.000000000 +0000 @@ -1398,6 +1398,7 @@ process_changed_entries_cb (RhythmDBEntr 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, @@ -1405,7 +1406,22 @@ process_changed_entries_cb (RhythmDBEntr (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 -upr rhythmbox-0.12.6.old/tests/test-rhythmdb.c rhythmbox-0.12.6/tests/test-rhythmdb.c --- rhythmbox-0.12.6.old/tests/test-rhythmdb.c 2009-11-15 00:54:49.000000000 +0000 +++ rhythmbox-0.12.6/tests/test-rhythmdb.c 2010-03-15 13:32:37.000000000 +0000 @@ -471,6 +471,42 @@ START_TEST (test_rhythmdb_modify_after_d } 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; }