Attachment 89210 Details for Bug 27727 – 03/26] Remove mcp_account_storage_set()

[patch] 03/26] Remove mcp_account_storage_set()

0003-Remove-mcp_account_storage_set.patch (text/plain), 13.85 KB, created by Simon McVittie on 2013-11-14 18:12:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-11-14 18:12:16 UTC

Size: 13.85 KB

patch

obsolete

>From c68c2e52c787b7ee3e560c8d69b35684224b2adc Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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 <simon.mcvittie@collabora.co.uk>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 27727: 35148 | 89208 | 89209 | 89210 | 89211 | 89212 | 89213 | 89214 | 89215 | 89216 | 89217 | 89218 | 89219 | 89220 | 89221 | 89222 | 89223 | 89224 | 89225 | 89226 | 89227 | 89228 | 89229 | 89230 | 89231 | 89232 | 89233 | 89237

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.