Attachment 93318 Details for Bug 71093 – [MC 3/5] _mcd_account_dup_parameters: try to get parameters' types from backend

[patch] [MC 3/5] _mcd_account_dup_parameters: try to get parameters' types from backend

0003-_mcd_account_dup_parameters-try-to-get-parameters-ty.patch (text/plain), 8.94 KB, created by Simon McVittie on 2014-02-03 20:29:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2014-02-03 20:29:52 UTC

Size: 8.94 KB

patch

obsolete

>From 064d2554064090cc08f36d0b3fa69e0c20a99337 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 3 Feb 2014 19:23:26 +0000
>Subject: [PATCH 3/5] _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
>---
> 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 */
>-- 
>1.9.rc1
>

Actions: View | Splinter Review

Attachments on bug 71093: 93126 | 93127 | 93128 | 93129 | 93130 | 93131 | 93132 | 93133 | 93134 | 93137 | 93138 | 93316 | 93317 | 93318 | 93319 | 93320 | 93476 | 93477

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.