From ad821d58fc7832781a4280ccc256da14f6e78814 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 30 Jan 2014 16:53:31 +0000 Subject: [PATCH 6/9] _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 --- src/mcd-account.c | 129 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 104 insertions(+), 25 deletions(-) diff --git a/src/mcd-account.c b/src/mcd-account.c index aea13e2..a4810e1 100644 --- a/src/mcd-account.c +++ b/src/mcd-account.c @@ -3595,20 +3595,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; @@ -3617,16 +3611,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); @@ -3649,11 +3633,96 @@ _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) +{ + GVariant *vardict; + 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? */ + vardict = mcp_account_storage_get_typed_parameters ( + self->priv->storage_plugin, + (McpAccountManager *) self->priv->storage, + self->priv->unique_name); + untyped_parameters = mcp_account_storage_get_untyped_parameters ( + self->priv->storage_plugin, + (McpAccountManager *) self->priv->storage, + self->priv->unique_name); + + if (vardict != NULL && + !g_variant_is_of_type (vardict, G_VARIANT_TYPE_VARDICT)) + { + CRITICAL ("account storage plugin returned wrong type"); + return NULL; + } + + if (untyped_parameters == NULL || *untyped_parameters == NULL) + { + /* Happy path: there are no parameters that lack types. */ + GVariantIter iter; + gchar *k; + GVariant *v; + + params = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, (GDestroyNotify) tp_g_value_slice_free); + + g_variant_iter_init (&iter, vardict); + + while (g_variant_iter_loop (&iter, "{sv}", &k, &v)) + { + GValue *value = g_slice_new0 (GValue); + + dbus_g_value_parse_g_variant (v, value); + g_hash_table_insert (params, g_strdup (k), value); + } + + 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: + if (vardict != NULL) + g_variant_unref (vardict); + + g_strfreev (untyped_parameters); + + return params; +} + +/** * mcd_account_request_presence: * @account: the #McdAccount. * @presence: a #TpConnectionPresenceType. @@ -4960,6 +5029,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) @@ -4975,10 +5045,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, -- 1.9.rc1