diff options
| -rw-r--r-- | engine/dconf-engine.c | 84 | ||||
| -rw-r--r-- | tests/engine.c | 162 |
2 files changed, 241 insertions, 5 deletions
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c index cbb2a00..6a1f247 100644 --- a/engine/dconf-engine.c +++ b/engine/dconf-engine.c @@ -24,6 +24,7 @@ #include "../common/dconf-enums.h" #include "../common/dconf-paths.h" +#include "../common/dconf-gvdb-utils.h" #include "../gvdb/gvdb-reader.h" #include <string.h> #include <stdlib.h> @@ -822,6 +823,54 @@ dconf_engine_list (DConfEngine *engine, return list; } +static gboolean +dconf_engine_dir_has_writable_contents (DConfEngine *engine, + const gchar *dir) +{ + DConfChangeset *database; + GHashTable *current_state; + + /* Read the on disk state */ + if (engine->n_sources == 0 || !engine->sources[0]->writable) + // If there are no writable sources, there won't be any pending writes either + return FALSE; + + dconf_engine_acquire_sources (engine); + database = dconf_gvdb_utils_changeset_from_table (engine->sources[0]->values); + dconf_engine_release_sources (engine); + + /* Apply pending and in_flight changes to the on disk state */ + dconf_engine_lock_queue (engine); + + if (engine->in_flight != NULL) + dconf_changeset_change (database, engine->in_flight); + + if (engine->pending != NULL) + { + /** + * We don't want to seal the pending changeset because it may still be changed, + * and sealing the changeset would be a side effect of passing engine->pending + * directly into dconf_changeset_change. + */ + DConfChangeset *changes = dconf_changeset_filter_changes (database, engine->pending); + if (changes != NULL) + { + dconf_changeset_change (database, changes); + dconf_changeset_unref (changes); + } + } + + dconf_engine_unlock_queue (engine); + + /* Check if there are writable contents at the given directory in the current state */ + current_state = dconf_gvdb_utils_table_from_changeset (database); + gboolean result = g_hash_table_contains (current_state, dir); + + g_hash_table_unref (current_state); + dconf_changeset_unref (database); + return result; +} + typedef void (* DConfEngineCallHandleCallback) (DConfEngine *engine, gpointer handle, GVariant *parameter, @@ -1129,6 +1178,34 @@ dconf_engine_prepare_change (DConfEngine *engine, */ static void dconf_engine_manage_queue (DConfEngine *engine); +/** + * a #DConfChangesetPredicate which determines whether the given path and + * value is already present in the given engine. "Already present" means + * that setting that path to that value would have no effect on the + * engine, including for directory resets. + */ +static gboolean +dconf_engine_path_has_value_predicate (const gchar *path, + GVariant *new_value, + gpointer user_data) +{ + DConfEngine *engine = user_data; + + // Path reset are handled specially + if (g_str_has_suffix (path, "/")) + return !dconf_engine_dir_has_writable_contents (engine, path); + + g_autoptr(GVariant) current_value = dconf_engine_read ( + engine, + DCONF_READ_USER_VALUE, + NULL, + path + ); + return ((current_value == NULL && new_value == NULL) || + (current_value != NULL && new_value != NULL && + g_variant_equal (current_value, new_value))); +} + static void dconf_engine_emit_changes (DConfEngine *engine, DConfChangeset *changeset, @@ -1274,6 +1351,10 @@ dconf_engine_change_fast (DConfEngine *engine, if (dconf_changeset_is_empty (changeset)) return TRUE; + gboolean has_no_effect = dconf_changeset_all (changeset, + dconf_engine_path_has_value_predicate, + engine); + if (!dconf_engine_changeset_changes_only_writable_keys (engine, changeset, error)) return FALSE; @@ -1298,7 +1379,8 @@ dconf_engine_change_fast (DConfEngine *engine, dconf_engine_unlock_queue (engine); /* Emit the signal after dropping the lock to avoid deadlock on re-entry. */ - dconf_engine_emit_changes (engine, changeset, origin_tag); + if (!has_no_effect) + dconf_engine_emit_changes (engine, changeset, origin_tag); return TRUE; } diff --git a/tests/engine.c b/tests/engine.c index fd2a348..b0ad884 100644 --- a/tests/engine.c +++ b/tests/engine.c @@ -1466,7 +1466,7 @@ test_watch_sync (void) static void test_change_fast (void) { - DConfChangeset *empty, *good_write, *bad_write, *very_good_write, *slightly_bad_write; + DConfChangeset *empty, *good_write, *good_write2, *bad_write, *very_good_write, *slightly_bad_write; GvdbTable *table, *locks; DConfEngine *engine; gboolean success; @@ -1483,6 +1483,7 @@ test_change_fast (void) empty = dconf_changeset_new (); good_write = dconf_changeset_new_write ("/value", g_variant_new_string ("value")); + good_write2 = dconf_changeset_new_write ("/value2", g_variant_new_string ("value2")); bad_write = dconf_changeset_new_write ("/locked", g_variant_new_string ("value")); very_good_write = dconf_changeset_new_write ("/value", g_variant_new_string ("value")); dconf_changeset_set (very_good_write, "/to-reset", NULL); @@ -1517,6 +1518,15 @@ test_change_fast (void) dconf_mock_dbus_assert_no_async (); g_assert_cmpstr (change_log->str, ==, ""); + /* Verify that value is unset initially */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value"); + g_assert (value == NULL); + + /* Verify that value2 is unset initially */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value2"); + g_assert (value == NULL); + + /* change /value */ success = dconf_engine_change_fast (engine, good_write, NULL, &error); g_assert_no_error (error); g_assert (success); @@ -1530,7 +1540,48 @@ test_change_fast (void) g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value"); g_variant_unref (value); - /* Fail the attempted write. This should cause a warning and a change. */ + /* Repeat the same write for /value (which is already in the in_flight queue) */ + success = dconf_engine_change_fast (engine, good_write, NULL, &error); + g_assert_no_error (error); + g_assert (success); + + /* Verify that /value is (still) set */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value"); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value"); + g_variant_unref (value); + + /* That should not have emitted a synthetic change event, since the (local) value did not change */ + g_assert_cmpstr (change_log->str, ==, ""); + g_string_set_size (change_log, 0); + + /* change /value2 */ + success = dconf_engine_change_fast (engine, good_write2, NULL, &error); + g_assert_no_error (error); + g_assert (success); + + /* That should have emitted a synthetic change event */ + g_assert_cmpstr (change_log->str, ==, "/value2:1::nil;"); + g_string_set_size (change_log, 0); + + /* Verify that /value2 is set */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value2"); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value2"); + g_variant_unref (value); + + /* change /value2 a second time */ + success = dconf_engine_change_fast (engine, good_write2, NULL, &error); + g_assert_no_error (error); + g_assert (success); + + /* That should not have emitted a synthetic change event because the (local) value didn't change */ + g_assert_cmpstr (change_log->str, ==, ""); + + /* Verify that /value2 is still set */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value2"); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value2"); + g_variant_unref (value); + + /* Fail the first attempted write. This should cause a warning and a signal. */ error = g_error_new_literal (G_FILE_ERROR, G_FILE_ERROR_NOENT, "something failed"); dconf_mock_dbus_async_reply (NULL, error); g_clear_error (&error); @@ -1539,8 +1590,25 @@ test_change_fast (void) assert_pop_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something failed"); - /* Verify that the value became unset due to the failure */ - value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value"); + /* Verify that /value is still set (because the second write is in progress) */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value"); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value"); + g_variant_unref (value); + + /* Verify that /value2 is still set because the write is in progress */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value2"); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value2"); + g_variant_unref (value); + + /* Now allow the second set of writes to succeed */ + dconf_mock_dbus_async_reply (g_variant_new ("(s)", "tag"), NULL); + + /* Verify that /value became unset due to the in flight queue clearing */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value"); + g_assert (value == NULL); + + /* Verify that /value2 became unset due to the in flight queue clearing */ + value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value2"); g_assert (value == NULL); /* Now try a successful write */ @@ -1619,6 +1687,91 @@ test_change_fast (void) change_log = NULL; } +/** + * Tests that dconf_engine_change_fast() emits local optimistic change + * notifications in the right circumstances + */ +static void +test_change_fast_redundant (void) +{ + DConfChangeset *change; + DConfEngine *engine; + change_log = g_string_new (NULL); + + // Initialise an empty engine + engine = dconf_engine_new (SRCDIR "/profile/dos", NULL, NULL); + + // Send an empty changeset, which has no effect + change = dconf_changeset_new (); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, ""); + + // Reset the root directory, which has no effect since the database is empty + change = dconf_changeset_new_write ("/", NULL); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, ""); + + // Set apple to NULL, which has no effect because it was already unset + change = dconf_changeset_new_write ("/apple", NULL); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, ""); + + // Set apple to apple + change = dconf_changeset_new_write ("/apple", g_variant_new_string ("apple")); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, "/apple:1::nil;"); + g_string_set_size (change_log, 0); + + // Set apple to apple, which has no effect because it is the same as the old value + change = dconf_changeset_new_write ("/apple", g_variant_new_string ("apple")); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, ""); + g_string_set_size (change_log, 0); + + // Set apple to orange, which has an effect because it is different to the old value + change = dconf_changeset_new_write ("/apple", g_variant_new_string ("orange")); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, "/apple:1::nil;"); + g_string_set_size (change_log, 0); + + // Set apple to NULL, which has an effect because it was previously set + change = dconf_changeset_new_write ("/apple", NULL); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, "/apple:1::nil;"); + g_string_set_size (change_log, 0); + + // Set apple to apple + change = dconf_changeset_new_write ("/apple", g_variant_new_string ("apple")); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, "/apple:1::nil;"); + g_string_set_size (change_log, 0); + + // Reset the root directory, which has an effect since the database is not empty + change = dconf_changeset_new_write ("/", NULL); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, "/:1::nil;"); + g_string_set_size (change_log, 0); + + // Reset the root directory again, which has no effect since the database is empty + change = dconf_changeset_new_write ("/", NULL); + dconf_engine_change_fast (engine, change, NULL, NULL); + dconf_changeset_unref (change); + g_assert_cmpstr (change_log->str, ==, ""); + + dconf_engine_unref (engine); + g_string_free (change_log, TRUE); + change_log = NULL; +} + static GError *change_sync_error; static GVariant *change_sync_result; @@ -2087,6 +2240,7 @@ main (int argc, char **argv) g_test_add_func ("/engine/watch/fast/short_lived", test_watch_fast_short_lived_subscriptions); g_test_add_func ("/engine/watch/sync", test_watch_sync); g_test_add_func ("/engine/change/fast", test_change_fast); + g_test_add_func ("/engine/change/fast_redundant", test_change_fast_redundant); g_test_add_func ("/engine/change/sync", test_change_sync); g_test_add_func ("/engine/signals", test_signals); g_test_add_func ("/engine/sync", test_sync); |