_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@collabora.co.uk>

4 files changed, 133 insertions, 26 deletions

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index f36ed25c..b04e3bdf 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 aa8de240..79b0cc44 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 1db0717f..3bd21de9 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 c2624a40..16f112bd 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 */