telepathy-mission-control: _mcd_account_dup_parameters: try to get parameters' types from backend

Simon McVittie smcv at kemper.freedesktop.org
Tue Feb 4 06:11:11 PST 2014


Module: telepathy-mission-control
Branch: master
Commit: d3fa6dc5b7048f3461755bedb0e666f0d1f50108
URL:    http://cgit.freedesktop.org/telepathy/telepathy-mission-control/commit/?id=d3fa6dc5b7048f3461755bedb0e666f0d1f50108

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Mon Feb  3 19:23:26 2014 +0000

_mcd_account_dup_parameters: try to get parameters' types from backend

Now that I've deleted ExternalAccountStorage support, we have two
uses for this function:

* get the parameters to be passed to RequestConnection

* get the parameters for our own D-Bus API (PropertiesChanged,
  GetAll, etc.)

For the former, we should know the types already, because we should
already have a concrete CM/protocol in mind by the time we get here.

For the latter, ideally we shouldn't need the CM's types at all: if
the backend is storing parameters with types, it's arguably more
correct for Parameters to contain what the user stored, even if that
isn't an exact match for what the CM wants.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71093
Reviewed-by: Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>

---

 mission-control-plugins/account-storage.c |    2 +-
 src/mcd-account.c                         |  100 +++++++++++++++++++++--------
 src/mcd-storage.c                         |   54 ++++++++++++++++
 src/mcd-storage.h                         |    3 +
 4 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index f36ed25..b04e3bd 100644
--- a/mission-control-plugins/account-storage.c
+++ b/mission-control-plugins/account-storage.c
@@ -536,7 +536,7 @@ mcp_account_storage_get_parameter (McpAccountStorage *storage,
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
 
   if (type == NULL)
-    SDEBUG (storage, "%s.%s (if type is stored", account, parameter);
+    SDEBUG (storage, "%s.%s (if type is stored)", account, parameter);
   else
     SDEBUG (storage, "%s.%s (type '%.*s')", account, parameter,
         (int) g_variant_type_get_string_length (type),
diff --git a/src/mcd-account.c b/src/mcd-account.c
index aa8de24..79b0cc4 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -3577,20 +3577,14 @@ mcd_account_get_object_path (McdAccount *account)
     return account->priv->object_path;
 }
 
-/**
- * _mcd_account_dup_parameters:
- * @account: the #McdAccount.
- *
- * Get the parameters set for this account. The resulting #GHashTable will be
- * newly allocated and must be g_hash_table_unref()'d after use.
- *
- * Returns: @account's current parameters, or %NULL if they could not be
- *          retrieved.
+/*
+ * Like _mcd_account_dup_parameters(), but return the parameters as they
+ * would be passed to RequestConnection for the given protocol.
  */
-GHashTable *
-_mcd_account_dup_parameters (McdAccount *account)
+static GHashTable *
+mcd_account_coerce_parameters (McdAccount *account,
+                               TpProtocol *protocol)
 {
-    TpProtocol *protocol;
     GList *protocol_params;
     GList *iter;
     GHashTable *params;
@@ -3599,16 +3593,6 @@ _mcd_account_dup_parameters (McdAccount *account)
 
     DEBUG ("called");
 
-    /* FIXME: this is ridiculous. MC stores the parameters for the account, so
-     * it should be able to expose them on D-Bus even if the CM is uninstalled.
-     * It shouldn't need to iterate across the parameters supported by the CM.
-     * But it does, because MC doesn't store the types of parameters. So it
-     * needs the CM (or .manager file) to be around to tell it whether "true"
-     * is a string or a boolean…
-     */
-
-    protocol = mcd_account_dup_protocol (account);
-
     params = g_hash_table_new_full (g_str_hash, g_str_equal,
                                     g_free,
                                     (GDestroyNotify) tp_g_value_slice_free);
@@ -3631,11 +3615,67 @@ _mcd_account_dup_parameters (McdAccount *account)
 
     g_list_free_full (protocol_params,
                       (GDestroyNotify) tp_connection_manager_param_free);
-    g_object_unref (protocol);
     return params;
 }
 
 /**
+ * _mcd_account_dup_parameters:
+ * @account: the #McdAccount.
+ *
+ * Get the parameters set for this account. The resulting #GHashTable will be
+ * newly allocated and must be g_hash_table_unref()'d after use.
+ *
+ * Returns: @account's current parameters, or %NULL if they could not be
+ *          retrieved.
+ */
+GHashTable *
+_mcd_account_dup_parameters (McdAccount *self)
+{
+  McpAccountManager *api;
+  gchar **untyped_parameters;
+  GHashTable *params = NULL;
+  TpProtocol *protocol;
+
+  g_return_val_if_fail (MCD_IS_ACCOUNT (self), NULL);
+
+  DEBUG ("called");
+
+  /* Maybe our storage plugin knows the types of the parameters? */
+
+  api = MCP_ACCOUNT_MANAGER (self->priv->storage);
+  untyped_parameters = mcp_account_storage_list_untyped_parameters (
+      self->priv->storage_plugin, api, self->priv->unique_name);
+
+  if (untyped_parameters == NULL || *untyped_parameters == NULL)
+    {
+      /* Happy path: there are no parameters that lack types. */
+      params = mcd_storage_dup_typed_parameters (self->priv->storage,
+          self->priv->unique_name);
+      goto finally;
+    }
+
+  /* MC didn't always know parameters' types, so it might need the CM
+   * (or .manager file) to be around to tell it whether "true"
+   * is a string or a boolean… this is ridiculous, but backwards-compatible.
+   */
+  protocol = mcd_account_dup_protocol (self);
+
+  if (protocol != NULL)
+    {
+      params = mcd_account_coerce_parameters (self, protocol);
+      g_object_unref (protocol);
+
+      if (params != NULL)
+        goto finally;
+    }
+
+finally:
+  g_strfreev (untyped_parameters);
+
+  return params;
+}
+
+/**
  * mcd_account_request_presence:
  * @account: the #McdAccount.
  * @presence: a #TpConnectionPresenceType.
@@ -4927,6 +4967,7 @@ _mcd_account_connection_begin (McdAccount *account,
                                gboolean user_initiated)
 {
     McdAccountConnectionContext *ctx;
+    TpProtocol *protocol;
 
     /* check whether a connection process is already ongoing */
     if (account->priv->connection_context != NULL)
@@ -4942,10 +4983,19 @@ _mcd_account_connection_begin (McdAccount *account,
     ctx->user_initiated = user_initiated;
 
     /* If we get this far, the account should be valid, so getting the
-     * parameters should succeed.
+     * protocol should succeed.
+     *
+     * (FIXME: as far as I can see, this is not necessarily true when
+     * _mcd_account_reconnect is called by McdAccountManager? But older
+     * MC would have asserted in that situation too, so this is at least
+     * not a regression.)
      */
-    ctx->params = _mcd_account_dup_parameters (account);
+    protocol = mcd_account_dup_protocol (account);
+    g_assert (protocol != NULL);
+
+    ctx->params = mcd_account_coerce_parameters (account, protocol);
     g_assert (ctx->params != NULL);
+    g_object_unref (protocol);
 
     _mcd_account_set_connection_status (account,
                                         TP_CONNECTION_STATUS_CONNECTING,
diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index 1db0717..3bd21de 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -2006,3 +2006,57 @@ mcd_storage_add_account_from_plugin (McdStorage *self,
 
   return TRUE;
 }
+
+GHashTable *
+mcd_storage_dup_typed_parameters (McdStorage *self,
+    const gchar *account_name)
+{
+  McpAccountStorage *plugin;
+  McpAccountManager *api = (McpAccountManager *) self;
+  gsize i;
+  gchar **typed_parameters;
+  GHashTable *params;
+
+  g_return_val_if_fail (MCD_IS_STORAGE (self), NULL);
+
+  plugin = g_hash_table_lookup (self->accounts, account_name);
+  g_return_val_if_fail (plugin != NULL, NULL);
+
+  typed_parameters = mcp_account_storage_list_typed_parameters (plugin, api,
+      account_name);
+
+  params = g_hash_table_new_full (g_str_hash, g_str_equal,
+      g_free, (GDestroyNotify) tp_g_value_slice_free);
+
+  for (i = 0;
+       typed_parameters != NULL && typed_parameters[i] != NULL;
+       i++)
+    {
+      GVariant *v = mcp_account_storage_get_parameter (plugin, api,
+          account_name, typed_parameters[i], NULL, NULL);
+      GValue *value;
+
+      if (v == NULL)
+        {
+          CRITICAL ("%s was in list_typed_parameters() but could not be "
+              "retrieved", typed_parameters[i]);
+          continue;
+        }
+
+      value = g_slice_new0 (GValue);
+      dbus_g_value_parse_g_variant (v, value);
+
+      if (!G_IS_VALUE (value))
+        {
+          CRITICAL ("could not turn %s into a GValue", typed_parameters[i]);
+          g_slice_free (GValue, value);
+          continue;
+        }
+
+      g_hash_table_insert (params, g_strdup (typed_parameters[i]),
+          value);
+      g_variant_unref (v);
+    }
+
+  return params;
+}
diff --git a/src/mcd-storage.h b/src/mcd-storage.h
index c2624a4..16f112b 100644
--- a/src/mcd-storage.h
+++ b/src/mcd-storage.h
@@ -163,6 +163,9 @@ gboolean mcd_storage_init_value_for_attribute (GValue *value,
     const gchar *attribute,
     const GVariantType **variant_type);
 
+GHashTable *mcd_storage_dup_typed_parameters (McdStorage *self,
+    const gchar *account);
+
 G_END_DECLS
 
 #endif /* MCD_STORAGE_H */



More information about the telepathy-commits mailing list