From c68c2e52c787b7ee3e560c8d69b35684224b2adc Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 12 Nov 2013 16:00:31 +0000 Subject: [PATCH 03/26] Remove mcp_account_storage_set() _set_attribute() and _set_parameter() are now mandatory for writable storage plugins. Note that most of the keyfile escaping code is still needed to help plugins to read their old keyfile values. [adjusted to apply earlier in the branch; left in the code that detects whether mcd_storage_set_parameter() is a no-op for an escaped parameter; took out mcp_account_storage_emit_created documentation fix -smcv] Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71384 Signed-off-by: Simon McVittie --- mission-control-plugins/account-storage.c | 76 ++----------------------------- mission-control-plugins/account-storage.h | 13 ------ src/mcd-account-manager-default.c | 11 ----- src/mcd-storage.c | 28 +++--------- tests/twisted/dbus-account-plugin.c | 13 ------ tests/twisted/mcp-account-diversion.c | 48 ++++++++++++++++--- 6 files changed, 52 insertions(+), 137 deletions(-) diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c index 8f5609c..8318956 100644 --- a/mission-control-plugins/account-storage.c +++ b/mission-control-plugins/account-storage.c @@ -57,7 +57,6 @@ * iface->provider = "org.freedesktop.Telepathy.MissionControl5.FooStorage"; * * iface->get = foo_plugin_get; - * iface->set = foo_plugin_get; * iface->delete = foo_plugin_delete; * iface->commit = foo_plugin_commit; * iface->list = foo_plugin_list; @@ -112,16 +111,6 @@ enum static guint signals[NO_SIGNAL] = { 0 }; static gboolean -default_set (const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *val) -{ - return FALSE; -} - -static gboolean default_set_attribute (McpAccountStorage *storage, McpAccountManager *am, const gchar *account, @@ -164,7 +153,6 @@ class_init (gpointer klass, McpAccountStorageIface *iface = klass; iface->owns = default_owns; - iface->set = default_set; iface->set_attribute = default_set_attribute; iface->set_parameter = default_set_parameter; @@ -423,59 +411,6 @@ mcp_account_storage_get (const McpAccountStorage *storage, } /** - * McpAccountStorageSetFunc: - * @storage: an #McpAccountStorage instance - * @am: an #McpAccountManager instance - * @account: the unique name of the account - * @key: the setting whose value we wish to store: either an attribute - * like "DisplayName", or "param-" plus a parameter like "account" - * @val: a non-%NULL value for @key - * - * An implementation of mcp_account_storage_set(). - * - * Returns: %TRUE if @storage is responsible for @account - */ - -/** - * mcp_account_storage_set: - * @storage: an #McpAccountStorage instance - * @am: an #McpAccountManager instance - * @account: the unique name of the account - * @key: the non-%NULL setting whose value we wish to store: either an - * attribute like "DisplayName", or "param-" plus a parameter like "account" - * @value: a value to associate with @key, escaped as if for a #GKeyFile - * - * The plugin is expected to either quickly and synchronously - * update its internal cache of values with @value, or to - * decline to store the setting. - * - * The plugin is not expected to write to its long term storage - * at this point. It can expect Mission Control to call either - * mcp_account_storage_commit() with either @account or %NULL - * after a short delay. - * - * Plugins that implement mcp_storage_set_attribute() and - * mcp_account_storage_set_parameter() can just return %FALSE here. - * There is a default implementation, which just returns %FALSE. - * - * Returns: %TRUE if the attribute was claimed, %FALSE otherwise - */ -gboolean -mcp_account_storage_set (const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *value) -{ - McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage); - - SDEBUG (storage, ""); - g_return_val_if_fail (iface != NULL, FALSE); - - return iface->set (storage, am, account, key, value); -} - -/** * mcp_account_storage_set_attribute: * @storage: an #McpAccountStorage instance * @am: an #McpAccountManager instance @@ -493,9 +428,8 @@ mcp_account_storage_set (const McpAccountStorage *storage, * The plugin is not expected to write to its long term storage * at this point. * - * There is a default implementation, which just returns %FALSE. - * Mission Control will call mcp_account_storage_set() instead, - * using a keyfile-escaped version of @value. + * There is a default implementation, which just returns %FALSE for read-only + * storage plugins. * * Returns: %TRUE if the attribute was claimed, %FALSE otherwise * @@ -537,10 +471,8 @@ mcp_account_storage_set_attribute (McpAccountStorage *storage, * The plugin is not expected to write to its long term storage * at this point. * - * There is a default implementation, which just returns %FALSE. - * Mission Control will call mcp_account_storage_set() instead, - * using "param-" + @parameter as key and a keyfile-escaped version - * of @value as value. + * There is a default implementation, which just returns %FALSE for read-only + * storage plugins. * * Returns: %TRUE if the parameter was claimed, %FALSE otherwise * diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h index ecc4e26..198e835 100644 --- a/mission-control-plugins/account-storage.h +++ b/mission-control-plugins/account-storage.h @@ -64,12 +64,6 @@ typedef gboolean (*McpAccountStorageGetFunc) ( const McpAccountManager *am, const gchar *account, const gchar *key); -typedef gboolean (*McpAccountStorageSetFunc) ( - const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *val); typedef gchar * (*McpAccountStorageCreate) ( const McpAccountStorage *storage, const McpAccountManager *am, @@ -112,7 +106,6 @@ struct _McpAccountStorageIface const gchar *desc; const gchar *provider; - McpAccountStorageSetFunc set; McpAccountStorageGetFunc get; McpAccountStorageDeleteFunc delete; McpAccountStorageCommitFunc commit; @@ -149,12 +142,6 @@ gboolean mcp_account_storage_get (const McpAccountStorage *storage, const gchar *account, const gchar *key); -gboolean mcp_account_storage_set (const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *value); - gchar * mcp_account_storage_create (const McpAccountStorage *storage, const McpAccountManager *am, const gchar *manager, diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c index 515cdbd..e967b73 100644 --- a/src/mcd-account-manager-default.c +++ b/src/mcd-account-manager-default.c @@ -215,16 +215,6 @@ set_attribute (McpAccountStorage *self, } static gboolean -_set (const McpAccountStorage *self, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *val) -{ - return FALSE; -} - -static gboolean get_parameter (const McpAccountStorage *self, const McpAccountManager *am, const gchar *account, @@ -1010,7 +1000,6 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->priority = PLUGIN_PRIORITY; iface->get = _get; - iface->set = _set; iface->set_attribute = set_attribute; iface->set_parameter = set_parameter; iface->create = _create; diff --git a/src/mcd-storage.c b/src/mcd-storage.c index 5932217..5caafbc 100644 --- a/src/mcd-storage.c +++ b/src/mcd-storage.c @@ -1493,8 +1493,7 @@ static void update_storage (McdStorage *self, const gchar *account, const gchar *key, - GVariant *variant, - const gchar *escaped) + GVariant *variant) { GList *store; gboolean done = FALSE; @@ -1503,7 +1502,7 @@ update_storage (McdStorage *self, /* we're deleting, which is unconditional, no need to check if anyone * * claims this setting for themselves */ - if (escaped == NULL) + if (variant == NULL) done = TRUE; for (store = stores; store != NULL; store = g_list_next (store)) @@ -1511,31 +1510,25 @@ update_storage (McdStorage *self, McpAccountStorage *plugin = store->data; const gchar *pn = mcp_account_storage_name (plugin); - if (done) + 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 (variant != NULL && !parameter && + else 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); } - else if (variant != NULL && parameter && + 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); } - else - { - done = mcp_account_storage_set (plugin, ma, account, key, escaped); - DEBUG ("MCP:%s -> %s %s.%s", - pn, done ? "store" : "ignore", account, key); - } } } @@ -1628,8 +1621,6 @@ mcd_storage_set_attribute (McdStorage *self, if (!mcd_nullable_variant_equal (old_v, new_v)) { - gchar *escaped = NULL; - /* First put it in the attributes hash table. (Watch out, this might * invalidate old_v.) */ if (new_v == NULL) @@ -1638,12 +1629,7 @@ mcd_storage_set_attribute (McdStorage *self, g_hash_table_insert (sa->attributes, g_strdup (attribute), g_variant_ref (new_v)); - /* OK now we have to escape it in a stupid way for plugins */ - if (value != NULL) - escaped = mcd_keyfile_escape_value (value); - - update_storage (self, account, attribute, new_v, escaped); - g_free (escaped); + update_storage (self, account, attribute, new_v); updated = TRUE; } @@ -1714,7 +1700,7 @@ mcd_storage_set_parameter (McdStorage *self, g_variant_ref (new_v)); g_snprintf (key, sizeof (key), "param-%s", parameter); - update_storage (self, account, key, new_v, new_escaped); + update_storage (self, account, key, new_v); return TRUE; } diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c index 0e94b9b..016417a 100644 --- a/tests/twisted/dbus-account-plugin.c +++ b/tests/twisted/dbus-account-plugin.c @@ -1087,18 +1087,6 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage, } static gboolean -test_dbus_account_plugin_set (const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account_name, - const gchar *key, - const gchar *value) -{ - /* Now that we implement set_attribute and set_parameter, this no longer - * needs a real implementation. */ - return FALSE; -} - -static gboolean test_dbus_account_plugin_set_attribute (McpAccountStorage *storage, McpAccountManager *am, const gchar *account_name, @@ -1585,7 +1573,6 @@ account_storage_iface_init (McpAccountStorageIface *iface) iface->priority = MCP_ACCOUNT_STORAGE_PLUGIN_PRIO_NORMAL + 100; iface->get = test_dbus_account_plugin_get; - iface->set = test_dbus_account_plugin_set; iface->set_attribute = test_dbus_account_plugin_set_attribute; iface->set_parameter = test_dbus_account_plugin_set_parameter; iface->list = test_dbus_account_plugin_list; diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c index e36b1ac..44fd4e3 100644 --- a/tests/twisted/mcp-account-diversion.c +++ b/tests/twisted/mcp-account-diversion.c @@ -112,24 +112,57 @@ _create_config (void) } static gboolean -_set (const McpAccountStorage *self, - const McpAccountManager *am, - const gchar *account, - const gchar *key, - const gchar *val) +_set (McpAccountStorage *self, + McpAccountManager *am, + const gchar *account, + const gchar *key, + GVariant *val, + McpParameterFlags flags) { AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self); + gchar *val_str; if (g_str_has_prefix (account, DONT_DIVERT)) return FALSE; adp->save = TRUE; - g_key_file_set_value (adp->keyfile, account, key, val); + + 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; } static gboolean +_set_attribute (McpAccountStorage *self, + McpAccountManager *am, + const gchar *account, + const gchar *attribute, + GVariant *val, + McpAttributeFlags flags) +{ + return _set (self, am, account, attribute, val, flags); +} + +static gboolean +_set_parameter (McpAccountStorage *self, + McpAccountManager *am, + const gchar *account, + const gchar *parameter, + GVariant *val, + McpParameterFlags flags) +{ + gchar *param = g_strdup_printf ("param-%s", parameter); + gboolean ret; + + ret = _set (self, am, account, param, val, flags); + g_free (param); + + return ret; +} + +static gboolean _get (const McpAccountStorage *self, const McpAccountManager *am, const gchar *account, @@ -268,7 +301,8 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->priority = PLUGIN_PRIORITY; iface->get = _get; - iface->set = _set; + iface->set_attribute = _set_attribute; + iface->set_parameter = _set_parameter; iface->delete = _delete; iface->commit = _commit; iface->list = _list; -- 1.8.4.3