Attachment 88300 Details for Bug 54875 – Default account storage backend: store parameters with types if possible

[patch] Default account storage backend: store parameters with types if possible

0010-Default-account-storage-backend-store-parameters-wit.patch (text/plain), 10.27 KB, created by Simon McVittie on 2013-10-29 14:26:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-10-29 14:26:38 UTC

Size: 10.27 KB

patch

obsolete

>From d50eda67074179873785ec5842203c760a75f6c5 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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 (&params_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 (&params_builder, "{sv}", k, v);
>+    }
>+
>+  g_variant_builder_add (&attrs_builder, "{sv}",
>+      "Parameters", g_variant_builder_end (&params_builder));
>+
>   g_variant_builder_init (&params_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 (&param_iter, v);
>+
>+          while (g_variant_iter_next (&param_iter, "{sv}", &parameter,
>+                &param_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
>

Actions: View | Splinter Review

Attachments on bug 54875: 88294 | 88295 | 88296 | 88297 | 88298 | 88299 | 88300 | 88305

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.