From 75f3b587f56e4552853de5db5600a0d117631738 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 13 Nov 2013 19:36:44 +0000 Subject: [PATCH 15/26] Call set_attribute, set_parameter to delete them It really doesn't make a great deal of sense to use the same callback to delete individual keys, and to delete accounts. McdAccountManagerDefault already dealt with that case, but the two test plugins didn't. --- mission-control-plugins/account-storage.c | 6 ++-- src/mcd-account-manager-default.c | 23 +------------ src/mcd-storage.c | 19 +++-------- tests/twisted/dbus-account-plugin.c | 54 +++++++++++++++++-------------- tests/twisted/mcp-account-diversion.c | 39 +++++++++++++--------- 5 files changed, 63 insertions(+), 78 deletions(-) diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c index 5ad31b6..53b00be 100644 --- a/mission-control-plugins/account-storage.c +++ b/mission-control-plugins/account-storage.c @@ -504,7 +504,8 @@ mcp_account_storage_get (const McpAccountStorage *storage, * @am: an #McpAccountManager instance * @account: the unique name of the account * @attribute: the name of an attribute, e.g. "DisplayName" - * @value: a value to associate with @attribute + * @value: (allow-none): a value to associate with @attribute, + * or %NULL to delete * @flags: flags influencing how the attribute is to be stored * * Store an attribute. @@ -547,7 +548,8 @@ mcp_account_storage_set_attribute (McpAccountStorage *storage, * @account: the unique name of the account * @parameter: the name of a parameter, e.g. "account" (note that there * is no "param-" prefix here) - * @value: a value to associate with @parameter + * @value: (allow-none): a value to associate with @parameter, + * or %NULL to delete * @flags: flags influencing how the parameter is to be stored * * Store a parameter. diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c index e967b73..e277fe5 100644 --- a/src/mcd-account-manager-default.c +++ b/src/mcd-account-manager-default.c @@ -374,28 +374,7 @@ _delete (const McpAccountStorage *self, } else { - if (g_str_has_prefix (key, "param-")) - { - if (g_hash_table_remove (sa->parameters, key + 6)) - amd->save = TRUE; - - if (g_hash_table_remove (sa->untyped_parameters, key + 6)) - amd->save = TRUE; - } - else - { - if (g_hash_table_remove (sa->attributes, key)) - amd->save = TRUE; - } - - /* if that was the last attribute or parameter, the account is gone - * too */ - if (g_hash_table_size (sa->attributes) == 0 && - g_hash_table_size (sa->untyped_parameters) == 0 && - g_hash_table_size (sa->parameters) == 0) - { - sa->pending_deletion = TRUE; - } + g_assert_not_reached (); } return TRUE; diff --git a/src/mcd-storage.c b/src/mcd-storage.c index 37d6700..dbc04c3 100644 --- a/src/mcd-storage.c +++ b/src/mcd-storage.c @@ -1379,38 +1379,29 @@ update_storage (McdStorage *self, GVariant *variant) { GList *store; - gboolean done = FALSE; gboolean parameter = g_str_has_prefix (key, "param-"); McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); - /* we're deleting, which is unconditional, no need to check if anyone * - * claims this setting for themselves */ - if (variant == NULL) - done = TRUE; - for (store = stores; store != NULL; store = g_list_next (store)) { McpAccountStorage *plugin = store->data; const gchar *pn = mcp_account_storage_name (plugin); - if (done) /* in particular, if variant == NULL */ - { - DEBUG ("MCP:%s -> delete %s.%s", pn, account, key); - mcp_account_storage_delete (plugin, ma, account, key); - } - else if (!parameter && + if (!parameter && mcp_account_storage_set_attribute (plugin, ma, account, key, variant, MCP_ATTRIBUTE_FLAG_NONE)) { - done = TRUE; DEBUG ("MCP:%s -> store attribute %s.%s", pn, account, key); + /* set it to NULL in all lower-priority stores */ + variant = NULL; } else if (parameter && mcp_account_storage_set_parameter (plugin, ma, account, key + 6, variant, MCP_PARAMETER_FLAG_NONE)) { - done = TRUE; DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key); + /* set it to NULL in all lower-priority stores */ + variant = NULL; } } } diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c index 016417a..3ea5df9 100644 --- a/tests/twisted/dbus-account-plugin.c +++ b/tests/twisted/dbus-account-plugin.c @@ -940,28 +940,9 @@ test_dbus_account_plugin_delete (const McpAccountStorage *storage, "DeferringDelete", g_variant_new_parsed ("(%o,)", account->path), NULL); } - else if (g_str_has_prefix (key, "param-")) - { - g_hash_table_remove (account->parameters, key + 6); - g_hash_table_remove (account->untyped_parameters, key + 6); - g_hash_table_remove (account->parameter_flags, key + 6); - g_hash_table_add (account->uncommitted_parameters, g_strdup (key + 6)); - - g_dbus_connection_emit_signal (self->bus, NULL, - TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, - "DeferringDeleteParameter", - g_variant_new_parsed ("(%o, %s)", account->path, key + 6), NULL); - } else { - g_hash_table_remove (account->attributes, key); - g_hash_table_remove (account->attribute_flags, key); - g_hash_table_add (account->uncommitted_attributes, g_strdup (key)); - - g_dbus_connection_emit_signal (self->bus, NULL, - TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, - "DeferringDeleteAttribute", - g_variant_new_parsed ("(%o, %s)", account->path, key), NULL); + g_assert_not_reached (); } return TRUE; @@ -1099,8 +1080,6 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage, g_return_val_if_fail (account_name != NULL, FALSE); g_return_val_if_fail (attribute != NULL, FALSE); - /* for deletions, MC would call delete() instead */ - g_return_val_if_fail (value != NULL, FALSE); DEBUG ("%s of %s", attribute, account_name); @@ -1108,6 +1087,20 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage, (account->flags & UNCOMMITTED_DELETION)) return FALSE; + if (value == NULL) + { + g_hash_table_remove (account->attributes, attribute); + g_hash_table_remove (account->attribute_flags, attribute); + g_hash_table_add (account->uncommitted_attributes, g_strdup (attribute)); + + g_dbus_connection_emit_signal (self->bus, NULL, + TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, + "DeferringDeleteAttribute", + g_variant_new_parsed ("(%o, %s)", account->path, attribute), NULL); + + return TRUE; + } + g_hash_table_insert (account->attributes, g_strdup (attribute), g_variant_ref (value)); g_hash_table_insert (account->attribute_flags, g_strdup (attribute), @@ -1136,8 +1129,6 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage, g_return_val_if_fail (account_name != NULL, FALSE); g_return_val_if_fail (parameter != NULL, FALSE); - /* for deletions, MC would call delete() instead */ - g_return_val_if_fail (value != NULL, FALSE); DEBUG ("%s of %s", parameter, account_name); @@ -1145,6 +1136,21 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage, (account->flags & UNCOMMITTED_DELETION)) return FALSE; + if (value == NULL) + { + g_hash_table_remove (account->parameters, parameter); + g_hash_table_remove (account->untyped_parameters, parameter); + g_hash_table_remove (account->parameter_flags, parameter); + g_hash_table_add (account->uncommitted_parameters, g_strdup (parameter)); + + g_dbus_connection_emit_signal (self->bus, NULL, + TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, + "DeferringDeleteParameter", + g_variant_new_parsed ("(%o, %s)", account->path, parameter), NULL); + + return TRUE; + } + g_hash_table_remove (account->untyped_parameters, parameter); g_hash_table_insert (account->parameters, g_strdup (parameter), g_variant_ref (value)); diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c index 44fd4e3..9639faf 100644 --- a/tests/twisted/mcp-account-diversion.c +++ b/tests/twisted/mcp-account-diversion.c @@ -125,11 +125,29 @@ _set (McpAccountStorage *self, if (g_str_has_prefix (account, DONT_DIVERT)) return FALSE; - adp->save = TRUE; + if (val == NULL) + { + gsize n; + GStrv keys; + + if (g_key_file_remove_key (adp->keyfile, account, key, NULL)) + adp->save = TRUE; + + keys = g_key_file_get_keys (adp->keyfile, account, &n, NULL); - val_str = mcp_account_manager_escape_variant_for_keyfile (am, val); - g_key_file_set_value (adp->keyfile, account, key, val_str); - g_free (val_str); + if (keys == NULL || n == 0) + g_key_file_remove_group (adp->keyfile, account, NULL); + + g_strfreev (keys); + } + else + { + adp->save = TRUE; + + val_str = mcp_account_manager_escape_variant_for_keyfile (am, val); + g_key_file_set_value (adp->keyfile, account, key, val_str); + g_free (val_str); + } return TRUE; } @@ -220,18 +238,7 @@ _delete (const McpAccountStorage *self, } else { - gsize n; - GStrv keys; - - if (g_key_file_remove_key (adp->keyfile, account, key, NULL)) - adp->save = TRUE; - - keys = g_key_file_get_keys (adp->keyfile, account, &n, NULL); - - if (keys == NULL || n == 0) - g_key_file_remove_group (adp->keyfile, account, NULL); - - g_strfreev (keys); + g_assert_not_reached (); } return TRUE; -- 1.8.4.3