From d50eda67074179873785ec5842203c760a75f6c5 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 Oct 2013 13:51:06 +0000 Subject: [PATCH 10/12] Default account storage backend: store parameters with types if possible --- src/mcd-account-manager-default.c | 101 +++++++++++++++++---- .../account-storage/default-keyring-storage.py | 20 +++- 2 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c index 531b55e..65f3fb2 100644 --- a/src/mcd-account-manager-default.c +++ b/src/mcd-account-manager-default.c @@ -41,8 +41,11 @@ typedef struct { /* owned string, attribute => owned GVariant, value * attributes to be stored in the variant-file */ GHashTable *attributes; + /* owned string, parameter (without "param-") => owned GVariant, value + * parameters of known type to be stored in the variant-file */ + GHashTable *parameters; /* owned string, parameter (without "param-") => owned string, value - * parameters to be stored in the variant-file */ + * parameters of unknwn type to be stored in the variant-file */ GHashTable *untyped_parameters; /* TRUE if the entire account is pending deletion */ gboolean pending_deletion; @@ -69,6 +72,8 @@ ensure_stored_account (McdAccountManagerDefault *self, sa = g_slice_new0 (McdDefaultStoredAccount); sa->attributes = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_variant_unref); + sa->parameters = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, (GDestroyNotify) g_variant_unref); sa->untyped_parameters = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); g_hash_table_insert (self->accounts, g_strdup (account), sa); @@ -85,6 +90,7 @@ stored_account_free (gpointer p) McdDefaultStoredAccount *sa = p; g_hash_table_unref (sa->attributes); + g_hash_table_unref (sa->parameters); g_hash_table_unref (sa->untyped_parameters); g_slice_free (McdDefaultStoredAccount, sa); } @@ -160,14 +166,13 @@ mcd_account_manager_default_class_init (McdAccountManagerDefaultClass *cls) DEBUG ("mcd_account_manager_default_class_init"); } -/* The value is escaped as if for a keyfile */ static gboolean -set_parameter (const McpAccountStorage *self, - const McpAccountManager *am, +set_parameter (McpAccountStorage *self, + McpAccountManager *am, const gchar *account, - const gchar *prefixed, const gchar *parameter, - const gchar *val) + GVariant *val, + McpParameterFlags flags) { McdAccountManagerDefault *amd = MCD_ACCOUNT_MANAGER_DEFAULT (self); McdDefaultStoredAccount *sa; @@ -175,11 +180,14 @@ set_parameter (const McpAccountStorage *self, sa = ensure_stored_account (amd, account); amd->save = TRUE; + /* remove it from all sets, then re-add it to the right one if + * non-null */ + g_hash_table_remove (sa->parameters, parameter); + g_hash_table_remove (sa->untyped_parameters, parameter); + if (val != NULL) - g_hash_table_insert (sa->untyped_parameters, g_strdup (parameter), - g_strdup (val)); - else - g_hash_table_remove (sa->untyped_parameters, parameter); + g_hash_table_insert (sa->parameters, g_strdup (parameter), + g_variant_ref (val)); return TRUE; } @@ -213,15 +221,7 @@ _set (const McpAccountStorage *self, const gchar *key, const gchar *val) { - if (g_str_has_prefix (key, "param-")) - { - return set_parameter (self, am, account, key, key + 6, val); - } - else - { - /* we implement set_attribute(), so MC shouldn't call this */ - g_assert_not_reached (); - } + return FALSE; } static gboolean @@ -237,10 +237,20 @@ get_parameter (const McpAccountStorage *self, if (parameter != NULL) { gchar *v = NULL; + GVariant *variant = NULL; if (sa == NULL || sa->absent) return FALSE; + variant = g_hash_table_lookup (sa->parameters, parameter); + + if (variant != NULL) + { + mcp_account_manager_set_parameter (am, account, parameter, + variant, MCP_PARAMETER_FLAG_NONE); + return TRUE; + } + v = g_hash_table_lookup (sa->untyped_parameters, parameter); if (v == NULL) @@ -299,6 +309,15 @@ _get (const McpAccountStorage *self, v, MCP_ATTRIBUTE_FLAG_NONE); } + g_hash_table_iter_init (&iter, sa->parameters); + + while (g_hash_table_iter_next (&iter, &k, &v)) + { + if (v != NULL) + mcp_account_manager_set_parameter (am, account, k, v, + MCP_PARAMETER_FLAG_NONE); + } + g_hash_table_iter_init (&iter, sa->untyped_parameters); while (g_hash_table_iter_next (&iter, &k, &v)) @@ -360,12 +379,16 @@ _delete (const McpAccountStorage *self, /* flag the whole account as purged */ sa->pending_deletion = TRUE; g_hash_table_remove_all (sa->attributes); + g_hash_table_remove_all (sa->parameters); g_hash_table_remove_all (sa->untyped_parameters); } else { if (g_str_has_prefix (key, "param-")) { + if (g_hash_table_remove (sa->parameters, key + 6)) + amd->save = TRUE; + if (g_hash_table_remove (sa->untyped_parameters, key + 6)) amd->save = TRUE; } @@ -378,7 +401,8 @@ _delete (const McpAccountStorage *self, /* if that was the last attribute or parameter, the account is gone * too */ if (g_hash_table_size (sa->attributes) == 0 && - g_hash_table_size (sa->untyped_parameters) == 0) + g_hash_table_size (sa->untyped_parameters) == 0 && + g_hash_table_size (sa->parameters) == 0) { sa->pending_deletion = TRUE; } @@ -464,6 +488,17 @@ am_default_commit_one (McdAccountManagerDefault *self, g_variant_builder_add (&attrs_builder, "{sv}", k, v); } + g_variant_builder_init (¶ms_builder, G_VARIANT_TYPE ("a{sv}")); + g_hash_table_iter_init (&inner, sa->parameters); + + while (g_hash_table_iter_next (&inner, &k, &v)) + { + g_variant_builder_add (¶ms_builder, "{sv}", k, v); + } + + g_variant_builder_add (&attrs_builder, "{sv}", + "Parameters", g_variant_builder_end (¶ms_builder)); + g_variant_builder_init (¶ms_builder, G_VARIANT_TYPE ("a{ss}")); g_hash_table_iter_init (&inner, sa->untyped_parameters); @@ -733,6 +768,31 @@ am_default_load_variant_file (McdAccountManagerDefault *self, param_value); } } + else if (!tp_strdiff (k, "Parameters")) + { + GVariantIter param_iter; + gchar *parameter; + GVariant *param_value; + + if (!g_variant_is_of_type (v, G_VARIANT_TYPE ("a{sv}"))) + { + gchar *repr = g_variant_print (v, TRUE); + + WARNING ("invalid Parameters found in %s, " + "ignoring: %s", full_name, repr); + g_free (repr); + continue; + } + + g_variant_iter_init (¶m_iter, v); + + while (g_variant_iter_next (¶m_iter, "{sv}", ¶meter, + ¶m_value)) + { + /* steals parameter, param_value */ + g_hash_table_insert (sa->parameters, parameter, param_value); + } + } else { /* an ordinary attribute */ @@ -949,6 +1009,7 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->get = _get; iface->set = _set; iface->set_attribute = set_attribute; + iface->set_parameter = set_parameter; iface->create = _create; iface->delete = _delete; iface->commit_one = _commit; diff --git a/tests/twisted/account-storage/default-keyring-storage.py b/tests/twisted/account-storage/default-keyring-storage.py index 6ed673e..27b45e5 100644 --- a/tests/twisted/account-storage/default-keyring-storage.py +++ b/tests/twisted/account-storage/default-keyring-storage.py @@ -76,7 +76,8 @@ def test(q, bus, mc): properties.get('InvalidAccounts') params = dbus.Dictionary({"account": "dontdivert@example.com", - "password": "secrecy"}, signature='sv') + "password": "secrecy", + "snakes": dbus.UInt32(23)}, signature='sv') (simulated_cm, account) = create_fakecm_account(q, bus, mc, params) account_path = account.__dbus_object_path__ @@ -122,9 +123,11 @@ def test(q, bus, mc): 'ConnectAutomatically')) assertEquals("(uint32 4, 'xa', 'never online')", account_store('get', 'variant-file', 'AutomaticPresence')) - assertEquals("keyfile-escaped 'dontdivert@example.com'", + assertEquals("'dontdivert@example.com'", account_store('get', 'variant-file', 'param-account')) - assertEquals("keyfile-escaped 'secrecy'", + assertEquals("uint32 23", + account_store('get', 'variant-file', 'param-snakes')) + assertEquals("'secrecy'", account_store('get', 'variant-file', 'param-password')) # Reactivate MC @@ -132,6 +135,9 @@ def test(q, bus, mc): account = get_fakecm_account(bus, mc, account_path) account_iface = dbus.Interface(account, cs.ACCOUNT) + assertEquals({'password': 'secrecy', 'account': 'dontdivert@example.com', + 'snakes': 23}, account.Properties.Get(cs.ACCOUNT, 'Parameters')) + # Delete the account assert account_iface.Remove() is None account_event, account_manager_event = q.expect_many( @@ -172,7 +178,8 @@ def test(q, bus, mc): 'AutomaticPresence': <(uint32 2, 'available', '')>, 'KeyFileParameters': <{ 'account': 'dontdivert@example.com', - 'password': 'password_in_variant_file' + 'password': 'password_in_variant_file', + 'snakes': '42' }> } """) @@ -219,6 +226,11 @@ def test(q, bus, mc): account = get_fakecm_account(bus, mc, account_path) account_iface = dbus.Interface(account, cs.ACCOUNT) + assertEquals(42, + account.Properties.Get(cs.ACCOUNT, 'Parameters')['snakes']) + assertEquals(dbus.UInt32, + type(account.Properties.Get(cs.ACCOUNT, 'Parameters')['snakes'])) + # Files in lower-priority XDG locations aren't copied until something # actually changes, and they aren't deleted. assert not os.path.exists(new_variant_file_name) -- 1.8.4.rc3