Attachment 87680 Details for Bug 34640 – Don't give storage plugins the full parameter set, just 'account'

[patch] Don't give storage plugins the full parameter set, just 'account'

0002-Don-t-give-storage-plugins-the-full-parameter-set-ju.patch (text/plain), 13.33 KB, created by Simon McVittie on 2013-10-15 17:14:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-10-15 17:14:48 UTC

Size: 13.33 KB

patch

obsolete

>From 59fa74e4b07cb261bf4c555bb948d6c5d59c585d Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 15 Oct 2013 18:05:45 +0100
>Subject: [PATCH 2/4] Don't give storage plugins the full parameter set, just
> 'account'
>
>We broke plugin API since the last release anyway, so this isn't
>a new API break.
>
>The doc-comments all claim that the string is the result of
>IdentifyAccount, because that's about to be true. :-)
>---
> mission-control-plugins/account-storage.c | 13 +++++++------
> mission-control-plugins/account-storage.h |  4 ++--
> mission-control-plugins/account.c         | 10 +++++++---
> mission-control-plugins/account.h         |  2 +-
> mission-control-plugins/implementation.h  |  2 +-
> src/mcd-account-manager-default.c         |  5 +++--
> src/mcd-account-manager-sso.c             |  6 +++++-
> src/mcd-account-manager.c                 |  8 +++++++-
> src/mcd-storage.c                         | 20 +++++++-------------
> src/mcd-storage.h                         |  2 +-
> tests/twisted/dbus-account-plugin.c       |  4 ++--
> 11 files changed, 43 insertions(+), 33 deletions(-)
>
>diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
>index 159a9f4..b51c126 100644
>--- a/mission-control-plugins/account-storage.c
>+++ b/mission-control-plugins/account-storage.c
>@@ -586,13 +586,14 @@ mcp_account_storage_set_parameter (McpAccountStorage *storage,
>  * @am: an object which can be used to call back into the account manager
>  * @manager: the name of the manager
>  * @protocol: the name of the protocol
>- * @params: A gchar * / GValue * hash table of account parameters
>+ * @identification: a normalized form of the account name, or "account"
>+ *  if nothing is suitable (e.g. for telepathy-salut)
>  * @error: a GError to fill
>  *
>  * Inform the plugin that a new account is being created. @manager, @protocol
>- * and @params are given to help determining the account's unique name, but does
>- * not need to be stored on the account yet, mcp_account_storage_set() and
>- * mcp_account_storage_commit() will be called later.
>+ * and @identification are given to help determining the account's unique name,
>+ * but does not need to be stored on the account yet, mcp_account_storage_set()
>+ * and mcp_account_storage_commit() will be called later.
>  *
>  * It is recommended to use mcp_account_manager_get_unique_name() to create the
>  * unique name, but it's not mandatory. One could base the unique name on an
>@@ -611,7 +612,7 @@ mcp_account_storage_create (const McpAccountStorage *storage,
>     const McpAccountManager *am,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error)
> {
>   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
>@@ -625,7 +626,7 @@ mcp_account_storage_create (const McpAccountStorage *storage,
>       return NULL;
>     }
> 
>-  return iface->create (storage, am, manager, protocol, params, error);
>+  return iface->create (storage, am, manager, protocol, identification, error);
> }
> 
> /**
>diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
>index e1af506..5c11102 100644
>--- a/mission-control-plugins/account-storage.h
>+++ b/mission-control-plugins/account-storage.h
>@@ -75,7 +75,7 @@ typedef gchar * (*McpAccountStorageCreate) (
>     const McpAccountManager *am,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error);
> typedef gboolean (*McpAccountStorageDeleteFunc) (
>     const McpAccountStorage *storage,
>@@ -163,7 +163,7 @@ gchar * mcp_account_storage_create (const McpAccountStorage *storage,
>     const McpAccountManager *am,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error);
> 
> gboolean mcp_account_storage_delete (const McpAccountStorage *storage,
>diff --git a/mission-control-plugins/account.c b/mission-control-plugins/account.c
>index e962857..62318d4 100644
>--- a/mission-control-plugins/account.c
>+++ b/mission-control-plugins/account.c
>@@ -279,7 +279,7 @@ mcp_account_manager_parameter_make_secret (const McpAccountManager *mcpa,
>  * @mcpa: an #McpAccountManager instance
>  * @manager: the name of the manager
>  * @protocol: the name of the protocol
>- * @params: A gchar * / GValue * hash table of account parameters.
>+ * @identification: the result of calling IdentifyAccount for this account
>  *
>  * Generate and return the canonical unique name of this [new] account.
>  * Should not be called for accounts which have already had a name
>@@ -287,6 +287,10 @@ mcp_account_manager_parameter_make_secret (const McpAccountManager *mcpa,
>  * MC has not previously seen before (ie one created by a 3rd party
>  * in the back-end that the plugin in question provides an interface to).
>  *
>+ * Changed in 5.17: instead of a map from string to GValue, the last
>+ * argument is the result of calling IdentifyAccount on the parameters,
>+ * which normalizes the account's name in a protocol-dependent way.
>+ *
>  * Returns: the newly allocated account name, which should be freed
>  * once the caller is done with it.
>  */
>@@ -294,14 +298,14 @@ gchar *
> mcp_account_manager_get_unique_name (McpAccountManager *mcpa,
>     const gchar *manager,
>     const gchar *protocol,
>-    const GHashTable *params)
>+    const gchar *identification)
> {
>   McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa);
> 
>   g_return_val_if_fail (iface != NULL, NULL);
>   g_return_val_if_fail (iface->unique_name != NULL, NULL);
> 
>-  return iface->unique_name (mcpa, manager, protocol, params);
>+  return iface->unique_name (mcpa, manager, protocol, identification);
> }
> 
> /**
>diff --git a/mission-control-plugins/account.h b/mission-control-plugins/account.h
>index 69a33d5..a476342 100644
>--- a/mission-control-plugins/account.h
>+++ b/mission-control-plugins/account.h
>@@ -77,7 +77,7 @@ void mcp_account_manager_parameter_make_secret (const McpAccountManager *mcpa,
> gchar * mcp_account_manager_get_unique_name (McpAccountManager *mcpa,
>     const gchar *manager,
>     const gchar *protocol,
>-    const GHashTable *params);
>+    const gchar *identification);
> 
> GStrv mcp_account_manager_list_keys (const McpAccountManager *mcpa,
>     const gchar *account);
>diff --git a/mission-control-plugins/implementation.h b/mission-control-plugins/implementation.h
>index eaed4f7..29d2937 100644
>--- a/mission-control-plugins/implementation.h
>+++ b/mission-control-plugins/implementation.h
>@@ -97,7 +97,7 @@ struct _McpAccountManagerIface {
>   gchar * (* unique_name) (const McpAccountManager *ma,
>       const gchar *manager,
>       const gchar *protocol,
>-      const GHashTable *params);
>+      const gchar *identification);
> 
>   GStrv (* list_keys) (const McpAccountManager *ma,
>       const gchar *acct);
>diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c
>index 0a89fe1..66ce891 100644
>--- a/src/mcd-account-manager-default.c
>+++ b/src/mcd-account-manager-default.c
>@@ -161,7 +161,7 @@ _create (const McpAccountStorage *self,
>     const McpAccountManager *am,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error)
> {
>   gchar *unique_name;
>@@ -169,7 +169,8 @@ _create (const McpAccountStorage *self,
>   /* See comment in plugin-account.c::_storage_create_account() before changing
>    * this implementation, it's more subtle than it looks */
>   unique_name = mcp_account_manager_get_unique_name (MCP_ACCOUNT_MANAGER (am),
>-                                                     manager, protocol, params);
>+                                                     manager, protocol,
>+                                                     identification);
>   g_return_val_if_fail (unique_name != NULL, NULL);
> 
>   return unique_name;
>diff --git a/src/mcd-account-manager-sso.c b/src/mcd-account-manager-sso.c
>index 186c636..9226bbb 100644
>--- a/src/mcd-account-manager-sso.c
>+++ b/src/mcd-account-manager-sso.c
>@@ -970,7 +970,11 @@ _ag_accountid_to_mc_key (McdAccountManagerSso *sso,
>       /* we want this to override any other settings for uid generation */
>       g_hash_table_insert (params, g_strdup (MC_ACCOUNT_KEY), &value);
> 
>-      name = mcp_account_manager_get_unique_name (am, cman, proto, params);
>+      /* FIXME: We should call IdentifyAccount(params) really. But this
>+       * plugin probably doesn't even compile any more, so, whatever;
>+       * just use the "account name". */
>+      name = mcp_account_manager_get_unique_name (am, cman, proto,
>+          g_value_get_string (&value));
> 
>     cleanup:
>       ag_account_select_service (account, service);
>diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c
>index 55da9cb..0a19de1 100644
>--- a/src/mcd-account-manager.c
>+++ b/src/mcd-account-manager.c
>@@ -799,6 +799,7 @@ _mcd_account_manager_create_account (McdAccountManager *account_manager,
>     gchar *unique_name = NULL;
>     const gchar *provider;
>     GError *e = NULL;
>+    const gchar *id;
> 
>     DEBUG ("called");
>     if (G_UNLIKELY (manager == NULL || manager[0] == 0 ||
>@@ -815,8 +816,13 @@ _mcd_account_manager_create_account (McdAccountManager *account_manager,
>     provider = tp_asv_get_string (properties,
>                                   TP_PROP_ACCOUNT_INTERFACE_STORAGE_STORAGE_PROVIDER);
> 
>+    id = tp_asv_get_string (params, "account");
>+
>+    if (id == NULL)
>+        id = "account";
>+
>     unique_name = mcd_storage_create_account (storage, provider,
>-                                              manager, protocol, params,
>+                                              manager, protocol, id,
>                                               &e);
> 
>     if (unique_name == NULL)
>diff --git a/src/mcd-storage.c b/src/mcd-storage.c
>index fec00a1..dfe972d 100644
>--- a/src/mcd-storage.c
>+++ b/src/mcd-storage.c
>@@ -578,23 +578,17 @@ static gchar *
> unique_name (const McpAccountManager *ma,
>     const gchar *manager,
>     const gchar *protocol,
>-    const GHashTable *params)
>+    const gchar *identification)
> {
>   McdStorage *self = MCD_STORAGE (ma);
>-  const gchar *base = NULL;
>   gchar *esc_manager, *esc_protocol, *esc_base;
>   guint i;
>   gsize base_len = strlen (TP_ACCOUNT_OBJECT_PATH_BASE);
>   DBusGConnection *connection = tp_proxy_get_dbus_connection (self->dbusd);
> 
>-  base = tp_asv_get_string (params, "account");
>-
>-  if (base == NULL)
>-    base = "account";
>-
>   esc_manager = tp_escape_as_identifier (manager);
>   esc_protocol = g_strdelimit (g_strdup (protocol), "-", '_');
>-  esc_base = tp_escape_as_identifier (base);
>+  esc_base = tp_escape_as_identifier (identification);
> 
>   for (i = 0; i < G_MAXUINT; i++)
>     {
>@@ -1832,7 +1826,7 @@ mcd_keyfile_set_value (GKeyFile *keyfile,
>  * @provider: the desired storage provider, or %NULL
>  * @manager: the name of the manager
>  * @protocol: the name of the protocol
>- * @params: A gchar * / GValue * hash table of account parameters
>+ * @identification: the result of IdentifyAccount
>  * @error: a #GError to fill when returning %NULL
>  *
>  * Create a new account in storage. This should not store any
>@@ -1847,7 +1841,7 @@ mcd_storage_create_account (McdStorage *self,
>     const gchar *provider,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error)
> {
>   GList *store;
>@@ -1867,7 +1861,7 @@ mcd_storage_create_account (McdStorage *self,
>           if (!tp_strdiff (mcp_account_storage_provider (plugin), provider))
>             {
>               return mcp_account_storage_create (plugin, ma, manager,
>-                  protocol, params, error);
>+                  protocol, identification, error);
>             }
>         }
> 
>@@ -1918,8 +1912,8 @@ mcd_storage_create_account (McdStorage *self,
>       McpAccountStorage *plugin = store->data;
>       gchar *ret;
> 
>-      ret = mcp_account_storage_create (plugin, ma, manager, protocol, params,
>-          error);
>+      ret = mcp_account_storage_create (plugin, ma, manager, protocol,
>+          identification, error);
> 
>       if (ret != NULL)
>         return ret;
>diff --git a/src/mcd-storage.h b/src/mcd-storage.h
>index 893a0af..2cb2c03 100644
>--- a/src/mcd-storage.h
>+++ b/src/mcd-storage.h
>@@ -95,7 +95,7 @@ gchar *mcd_storage_create_account (McdStorage *storage,
>     const gchar *provider,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identification,
>     GError **error);
> 
> void mcd_storage_delete_account (McdStorage *storage, const gchar *account);
>diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
>index e44e32c..eb727a3 100644
>--- a/tests/twisted/dbus-account-plugin.c
>+++ b/tests/twisted/dbus-account-plugin.c
>@@ -882,7 +882,7 @@ test_dbus_account_plugin_create (const McpAccountStorage *storage,
>     const McpAccountManager *am,
>     const gchar *manager,
>     const gchar *protocol,
>-    GHashTable *params,
>+    const gchar *identifier,
>     GError **error)
> {
>   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
>@@ -893,7 +893,7 @@ test_dbus_account_plugin_create (const McpAccountStorage *storage,
>     return FALSE;
> 
>   name = mcp_account_manager_get_unique_name ((McpAccountManager *) am,
>-      manager, protocol, params);
>+      manager, protocol, identifier);
>   account = ensure_account (self, name);
>   g_dbus_connection_emit_signal (self->bus, NULL,
>       TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
>-- 
>1.8.4.rc3
>

Actions: View | Splinter Review

Attachments on bug 34640: 87679 | 87680 | 87681 | 87682

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.