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.

6 files changed, 57 insertions, 152 deletions

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index 0d7c9323..a5b89f0c 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;
@@ -111,16 +110,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,
@@ -208,7 +197,6 @@ class_init (gpointer klass,
   GType type = G_TYPE_FROM_CLASS (klass);
   McpAccountStorageIface *iface = klass;
 
-  iface->set = default_set;
   iface->set_attribute = default_set_attribute;
   iface->set_parameter = default_set_parameter;
   iface->create = default_create;
@@ -474,59 +462,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
- * mcp_account_storage_commit() 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);
-  g_return_val_if_fail (iface->set != NULL, FALSE);
-
-  return iface->set (storage, am, account, key, value);
-}
-
-/**
  * mcp_account_storage_set_attribute:
  * @storage: an #McpAccountStorage instance
  * @am: an #McpAccountManager instance
@@ -544,9 +479,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
  *
@@ -588,10 +522,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
  *
@@ -1026,7 +958,7 @@ mcp_account_storage_provider (const McpAccountStorage *storage)
 }
 
 /**
- * mcp_account_storage_emit_create:
+ * mcp_account_storage_emit_created:
  * @storage: an #McpAccountStorage instance
  * @account: the unique name of the created account
  *
diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
index 14ec5770..1a3a012c 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;
   McpAccountStorageListFunc list;
@@ -146,12 +139,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 515cdbd3..e967b73e 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 c31dc2e6..7fc72548 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -1492,8 +1492,7 @@ static void
 update_storage (McdStorage *self,
     const gchar *account,
     const gchar *key,
-    GVariant *variant,
-    const gchar *escaped)
+    GVariant *variant)
 {
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
   gboolean parameter = g_str_has_prefix (key, "param-");
@@ -1504,29 +1503,22 @@ update_storage (McdStorage *self,
   g_return_if_fail (sa != NULL);
 
   pn = mcp_account_storage_name (sa->storage);
-  if (escaped == NULL)
+  if (variant == NULL)
     {
       DEBUG ("MCP:%s -> delete %s.%s", pn, account, key);
       mcp_account_storage_delete (sa->storage, ma, account, key);
     }
-  else if (variant != NULL && !parameter &&
-      mcp_account_storage_set_attribute (sa->storage, ma, account, key, variant,
-          MCP_ATTRIBUTE_FLAG_NONE))
-    {
-      DEBUG ("MCP:%s -> store attribute %s.%s", pn, account, key);
-    }
-  else if (variant != NULL && parameter &&
-      mcp_account_storage_set_parameter (sa->storage, ma, account, key + 6,
-          variant, MCP_PARAMETER_FLAG_NONE))
+  else if (parameter)
     {
       DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key);
+      mcp_account_storage_set_parameter (sa->storage, ma, account, key + 6,
+          variant, MCP_PARAMETER_FLAG_NONE);
     }
   else
     {
-      gboolean done;
-
-      done = mcp_account_storage_set (sa->storage, ma, account, key, escaped);
-      DEBUG ("MCP:%s -> %s %s.%s", pn, done ? "store" : "ignore", account, key);
+      DEBUG ("MCP:%s -> store attribute %s.%s", pn, account, key);
+      mcp_account_storage_set_attribute (sa->storage, ma, account, key,
+          variant, MCP_PARAMETER_FLAG_NONE);
     }
 }
 
@@ -1620,8 +1612,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)
@@ -1630,12 +1620,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;
     }
 
@@ -1667,8 +1652,6 @@ mcd_storage_set_parameter (McdStorage *self,
 {
   GVariant *old_v;
   GVariant *new_v = NULL;
-  const gchar *old_escaped;
-  gchar *new_escaped = NULL;
   McdStorageAccount *sa;
   gboolean updated = FALSE;
 
@@ -1680,18 +1663,12 @@ mcd_storage_set_parameter (McdStorage *self,
   g_return_val_if_fail (sa != NULL, FALSE);
 
   if (value != NULL)
-    {
-      new_escaped = mcd_keyfile_escape_value (value);
-      new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value));
-    }
+    new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value));
 
   old_v = g_hash_table_lookup (sa->parameters, parameter);
-  old_escaped = g_hash_table_lookup (sa->escaped_parameters, parameter);
 
   if (old_v != NULL)
     updated = !mcd_nullable_variant_equal (old_v, new_v);
-  else if (old_escaped != NULL)
-    updated = tp_strdiff (old_escaped, new_escaped);
   else
     updated = (value != NULL);
 
@@ -1707,11 +1684,10 @@ 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;
     }
 
-  g_free (new_escaped);
   tp_clear_pointer (&new_v, g_variant_unref);
   return updated;
 }
diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 9d79dc0a..b2ae25b2 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,
@@ -1566,7 +1554,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 c3fa55a0..8707d069 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,
@@ -266,7 +299,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;