diff options
author | Simon McVittie <simon.mcvittie@collabora.co.uk> | 2014-02-04 13:58:33 +0000 |
---|---|---|
committer | Simon McVittie <simon.mcvittie@collabora.co.uk> | 2014-02-04 13:58:33 +0000 |
commit | 9da576a7d1d6df02632f83e3d2613af1ab11fa17 (patch) | |
tree | 948f4ba4e2480e1ae75c375b0f9e34f6a846fe72 | |
parent | a9eb0c74c190d172cf3d47fa264ab5fb4a30ce62 (diff) |
Opportunistically migrate accounts from untyped to typed Parameters
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71093
Reviewed-by: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
[also depend on new tp-glib for
tp_connection_manager_param_dup_variant_type -smcv]
-rw-r--r-- | configure.ac | 2 | ||||
-rw-r--r-- | src/mcd-account.c | 39 | ||||
-rw-r--r-- | src/mcd-storage.c | 121 | ||||
-rw-r--r-- | src/mcd-storage.h | 4 | ||||
-rw-r--r-- | tests/twisted/account-storage/load-keyfiles.py | 31 | ||||
-rw-r--r-- | tests/twisted/account-storage/storage_helper.py | 8 |
6 files changed, 186 insertions, 19 deletions
diff --git a/configure.ac b/configure.ac index a428ee62..5be3c80c 100644 --- a/configure.ac +++ b/configure.ac @@ -156,7 +156,7 @@ PKG_CHECK_MODULES(DBUS, [dbus-1 >= 0.95, dbus-glib-1 >= 0.82]) AC_SUBST(DBUS_CFLAGS) AC_SUBST(DBUS_LIBS) -PKG_CHECK_MODULES([TELEPATHY], [telepathy-glib >= 0.23.0]) +PKG_CHECK_MODULES([TELEPATHY], [telepathy-glib >= 0.23.1]) AC_DEFINE([TP_VERSION_MIN_REQUIRED], [TP_VERSION_0_24], [Ignore post-0.24 deprecations]) AC_DEFINE([TP_VERSION_MAX_ALLOWED], [TP_VERSION_0_24], diff --git a/src/mcd-account.c b/src/mcd-account.c index 9374751f..0cfcdabe 100644 --- a/src/mcd-account.c +++ b/src/mcd-account.c @@ -194,6 +194,9 @@ _mcd_account_connection_context_free (McdAccountConnectionContext *c) static guint _mcd_account_signals[LAST_SIGNAL] = { 0 }; static GQuark account_ready_quark = 0; +static void mcd_account_changed_property (McdAccount *account, + const gchar *key, const GValue *value); + GQuark mcd_account_error_quark (void) { @@ -466,6 +469,7 @@ static void on_manager_ready (McdManager *manager, const GError *error, { McdAccount *account = MCD_ACCOUNT (user_data); GError *invalid_reason = NULL; + TpProtocol *protocol; if (error) { @@ -481,6 +485,38 @@ static void on_manager_ready (McdManager *manager, const GError *error, g_clear_error (&account->priv->invalid_reason); } + protocol = _mcd_manager_dup_protocol (account->priv->manager, + account->priv->protocol_name); + + if (protocol != NULL) + { + if (mcd_storage_maybe_migrate_parameters ( + account->priv->storage, + account->priv->unique_name, + protocol)) + { + GHashTable *params = _mcd_account_dup_parameters (account); + + if (params != NULL) + { + GValue value = G_VALUE_INIT; + + g_value_init (&value, TP_HASH_TYPE_STRING_VARIANT_MAP); + g_value_take_boxed (&value, params); + mcd_account_changed_property (account, "Parameters", &value); + g_value_unset (&value); + } + else + { + WARNING ("somehow managed to migrate parameters without " + "being able to emit change notification"); + } + } + + + g_object_unref (protocol); + } + mcd_account_loaded (account); } @@ -655,9 +691,6 @@ on_connection_abort (McdConnection *connection, McdAccount *account) _mcd_account_set_connection (account, NULL); } -static void mcd_account_changed_property (McdAccount *account, - const gchar *key, const GValue *value); - static void mcd_account_request_presence_int (McdAccount *account, TpConnectionPresenceType type, diff --git a/src/mcd-storage.c b/src/mcd-storage.c index 3bd21de9..68ccd431 100644 --- a/src/mcd-storage.c +++ b/src/mcd-storage.c @@ -2060,3 +2060,124 @@ mcd_storage_dup_typed_parameters (McdStorage *self, return params; } + +/* See whether we can migrate the parameters from being stored without + * their types, to being stored with their types. + * Commit changes and return TRUE if anything happened. */ +gboolean +mcd_storage_maybe_migrate_parameters (McdStorage *self, + const gchar *account_name, + TpProtocol *protocol) +{ + McpAccountManager *api = MCP_ACCOUNT_MANAGER (self); + McpAccountStorage *plugin; + gchar **untyped_parameters = NULL; + gsize i; + gboolean ret = FALSE; + + plugin = g_hash_table_lookup (self->accounts, account_name); + g_return_val_if_fail (plugin != NULL, FALSE); + + /* If the storage backend can't store typed parameters, there's no point. */ + if (!mcp_account_storage_has_any_flag (plugin, account_name, + MCP_ACCOUNT_STORAGE_FLAG_STORES_TYPES)) + goto finally; + + untyped_parameters = mcp_account_storage_list_untyped_parameters ( + plugin, api, account_name); + + /* If there's nothing to migrate, there's also no point. */ + if (untyped_parameters == NULL || untyped_parameters[0] == NULL) + goto finally; + + DEBUG ("trying to migrate %s", account_name); + + for (i = 0; untyped_parameters[i] != NULL; i++) + { + const gchar *param_name = untyped_parameters[i]; + const TpConnectionManagerParam *param = tp_protocol_get_param (protocol, + param_name); + GError *error = NULL; + GVariantType *type = NULL; + GVariant *value; + McpAccountStorageSetResult res; + + if (param == NULL) + { + DEBUG ("cannot migrate parameter '%s': not supported by %s/%s", + param_name, tp_protocol_get_cm_name (protocol), + tp_protocol_get_name (protocol)); + goto next_param; + } + + type = tp_connection_manager_param_dup_variant_type (param); + + DEBUG ("Migrating parameter '%s' of type '%.*s'", + param_name, + (gint) g_variant_type_get_string_length (type), + g_variant_type_peek_string (type)); + + value = mcp_account_storage_get_parameter (plugin, api, + account_name, param_name, type, NULL); + + if (value == NULL) + { + DEBUG ("cannot migrate parameter '%s': %s #%d: %s", + param_name, g_quark_to_string (error->domain), error->code, + error->message); + g_error_free (error); + goto next_param; + } + + if (!g_variant_is_of_type (value, type)) + { + DEBUG ("trying to convert parameter from type '%s'", + g_variant_get_type_string (value)); + + /* consumes parameter */ + value = tp_variant_convert (value, type); + + if (value == NULL) + { + DEBUG ("could not convert parameter to desired type"); + goto next_param; + } + } + + res = mcp_account_storage_set_parameter (plugin, api, + account_name, param_name, value, MCP_PARAMETER_FLAG_NONE); + + switch (res) + { + case MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED: + /* it really ought to be CHANGED, surely? */ + DEBUG ("Tried to upgrade parameter %s but the " + "storage backend claims not to have changed it? " + "Not sure I really believe that", param_name); + /* fall through to the CHANGED case */ + + case MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED: + ret = TRUE; + break; + + case MCP_ACCOUNT_STORAGE_SET_RESULT_FAILED: + WARNING ("Failed to set parameter %s", param_name); + break; + + default: + WARNING ("set_parameter returned invalid result code %d " + "for parameter %s", res, param_name); + } + +next_param: + if (type != NULL) + g_variant_type_free (type); + } + + if (ret) + mcp_account_storage_commit (plugin, api, account_name); + +finally: + g_strfreev (untyped_parameters); + return ret; +} diff --git a/src/mcd-storage.h b/src/mcd-storage.h index 16f112bd..56732f11 100644 --- a/src/mcd-storage.h +++ b/src/mcd-storage.h @@ -166,6 +166,10 @@ gboolean mcd_storage_init_value_for_attribute (GValue *value, GHashTable *mcd_storage_dup_typed_parameters (McdStorage *self, const gchar *account); +gboolean mcd_storage_maybe_migrate_parameters (McdStorage *self, + const gchar *account_name, + TpProtocol *protocol); + G_END_DECLS #endif /* MCD_STORAGE_H */ diff --git a/tests/twisted/account-storage/load-keyfiles.py b/tests/twisted/account-storage/load-keyfiles.py index dfba53b4..b6c9d2ef 100644 --- a/tests/twisted/account-storage/load-keyfiles.py +++ b/tests/twisted/account-storage/load-keyfiles.py @@ -240,8 +240,22 @@ def test(q, bus, mc): # The masked account is still masked assert open(variant_file_names['masked'], 'r').read() == '' - # Teach the one that knows its CM that the 'password' is a string. - # This results in the higher-priority file being written out. + # Because the CM exists, we can work out the correct types + # for the 'migration' account's parameters. This triggers a commit + # even though nothing has conceptually changed, so we have the type + # for later. The file is copied, not moved, because XDG_DATA_DIRS are, + # conceptually, read-only. + assert not os.path.exists(old_key_file_name) + assert not os.path.exists(newer_key_file_name) + assert os.path.exists(low_prio_variant_file_names['migration']) + assert os.path.exists(variant_file_names['migration']) + assertEquals("'password_in_variant_file'", + account_store('get', 'variant-file', 'param-password', + account=tails['migration'])) + assertEquals("uint32 42", account_store('get', 'variant-file', + 'param-snakes', account=tails['migration'])) + + # Setting the password still does the right thing. account_ifaces['migration'].UpdateParameters({'password': 'hello'}, []) q.expect('dbus-signal', path=account_paths['migration'], @@ -250,21 +264,16 @@ def test(q, bus, mc): predicate=(lambda e: 'Parameters' in e.args[0]), ) - # Check the account has copied (not moved! XDG_DATA_DIRS are, - # conceptually, read-only) 'migration' from the old to the new name assert not os.path.exists(old_key_file_name) assert not os.path.exists(newer_key_file_name) assert os.path.exists(low_prio_variant_file_names['migration']) assert os.path.exists(variant_file_names['migration']) - assertEquals("'hello'", account_store('get', 'variant-file', - 'param-password', account=tails['migration'])) - # Parameters whose types are still unknown are copied too, but their - # types are still unknown - assertEquals("keyfile-escaped '42'", account_store('get', 'variant-file', - 'param-snakes', account=tails['migration'])) + assertEquals("'hello'", + account_store('get', 'variant-file', 'param-password', + account=tails['migration'])) # 'absentcm' is still only in the low-priority location: we can't - # known the types of its parameters + # known the types of its parameters, so it doesn't get migrated. assert not os.path.exists(old_key_file_name) assert not os.path.exists(newer_key_file_name) assert os.path.exists(low_prio_variant_file_names['absentcm']) diff --git a/tests/twisted/account-storage/storage_helper.py b/tests/twisted/account-storage/storage_helper.py index ebbe78e6..0f67df92 100644 --- a/tests/twisted/account-storage/storage_helper.py +++ b/tests/twisted/account-storage/storage_helper.py @@ -141,12 +141,12 @@ AutomaticPresence=2;available;; assertEquals("'Second to none'", account_store('get', 'variant-file', 'DisplayName', account=a2_tail)) - # MC doesn't currently ensure that parameters are stored with their - # proper types. - assertEquals("keyfile-escaped 'dontdivert1@example.com'", + # Because the CM is installed, we can work out the right types + # for the parameters, too. + assertEquals("'dontdivert1@example.com'", account_store('get', 'variant-file', 'param-account', account=a1_tail)) - assertEquals("keyfile-escaped 'dontdivert2@example.com'", + assertEquals("'dontdivert2@example.com'", account_store('get', 'variant-file', 'param-account', account=a2_tail)) |