mcp_account_storage_get: replace with get_attribute, get_parameter

The old API in which plugins poked new values into the McdStorage
was non-obvious, and also fundamentally incompatible with the idea
that each account is owned by at most one plugin: if an account
in a high-priority plugin is masked by one in a low-priority plugin,
the McdStorage can't prevent the low-priority plugin from changing
its effective attribute and parameter values.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=27727

2 files changed, 97 insertions, 137 deletions

diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 7eaccc37..9e0025ae 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -443,8 +443,6 @@ test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self,
 
               g_hash_table_insert (account->attributes,
                   g_strdup (attr), g_variant_ref (value));
-              mcp_account_manager_set_attribute (self->feedback,
-                  account_name, attr, value, flags);
               mcp_account_storage_emit_altered_one (
                   MCP_ACCOUNT_STORAGE (self), account_name, attr);
 
@@ -468,8 +466,6 @@ test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self,
               DEBUG ("%s deleted", attr);
 
               g_hash_table_remove (account->attributes, attr);
-              mcp_account_manager_set_attribute (self->feedback,
-                  account_name, attr, NULL, 0);
               mcp_account_storage_emit_altered_one (
                   MCP_ACCOUNT_STORAGE (self), account_name, attr);
 
@@ -552,8 +548,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
               g_hash_table_insert (account->parameters,
                   g_strdup (param), g_variant_ref (value));
               key = g_strdup_printf ("param-%s", param);
-              mcp_account_manager_set_parameter (self->feedback,
-                  account_name, param, value, flags);
               mcp_account_storage_emit_altered_one (
                   MCP_ACCOUNT_STORAGE (self), account_name, key);
               g_free (key);
@@ -585,8 +579,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
               g_hash_table_insert (account->untyped_parameters,
                   g_strdup (param), g_strdup (escaped));
               key = g_strdup_printf ("param-%s", param);
-              mcp_account_manager_set_value (self->feedback,
-                  account_name, key, escaped);
               mcp_account_storage_emit_altered_one (
                   MCP_ACCOUNT_STORAGE (self), account_name, key);
               g_free (key);
@@ -616,8 +608,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self,
               g_hash_table_remove (account->untyped_parameters,
                   param);
               key = g_strdup_printf ("param-%s", param);
-              mcp_account_manager_set_parameter (self->feedback,
-                  account_name, param, NULL, 0);
               mcp_account_storage_emit_altered_one (
                   MCP_ACCOUNT_STORAGE (self), account_name, key);
               g_free (key);
@@ -964,123 +954,81 @@ test_dbus_account_plugin_delete_finish (McpAccountStorage *storage,
   return g_task_propagate_boolean (G_TASK (res), error);
 }
 
-static gboolean
-test_dbus_account_plugin_get (const McpAccountStorage *storage,
-    const McpAccountManager *am,
+static GVariant *
+test_dbus_account_plugin_get_attribute (McpAccountStorage *storage,
+    McpAccountManager *am,
     const gchar *account_name,
-    const gchar *key)
+    const gchar *attribute,
+    const GVariantType *type,
+    McpAttributeFlags *flags)
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
   Account *account = lookup_account (self, account_name);
+  GVariant *v;
 
-  if (!self->active || account == NULL)
-    return FALSE;
-
-  if (key == NULL)
-    {
-      GHashTableIter iter;
-      gpointer k, v;
-
-      /* get everything */
-      g_dbus_connection_emit_signal (self->bus, NULL,
-          TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
-          "GetAllKeys",
-          g_variant_new_parsed ("(%o,)", account->path), NULL);
-
-      g_hash_table_iter_init (&iter, account->attributes);
+  if (flags != NULL)
+    *flags = 0;
 
-      while (g_hash_table_iter_next (&iter, &k, &v))
-        {
-          gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,
-              v);
-
-          mcp_account_manager_set_value (am, account_name, k, escaped);
-          g_free (escaped);
-        }
+  if (!self->active || account == NULL)
+    return NULL;
 
-      g_hash_table_iter_init (&iter, account->untyped_parameters);
+  v = g_hash_table_lookup (account->attributes, attribute);
 
-      while (g_hash_table_iter_next (&iter, &k, &v))
-        {
-          gchar *param_foo;
+  g_dbus_connection_emit_signal (self->bus, NULL,
+      TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
+      "GetAttribute",
+      g_variant_new_parsed ("(%o, %s)", account->path, attribute), NULL);
 
-          param_foo = g_strdup_printf ("param-%s", (const gchar *) k);
-          mcp_account_manager_set_value (am, account_name, param_foo, v);
+  if (v != NULL)
+    {
+      return g_variant_ref (v);
+    }
+  else
+    {
+      return NULL;
+    }
+}
 
-          g_free (param_foo);
-        }
+static GVariant *
+test_dbus_account_plugin_get_parameter (McpAccountStorage *storage,
+    McpAccountManager *am,
+    const gchar *account_name,
+    const gchar *parameter,
+    const GVariantType *type,
+    McpParameterFlags *flags)
+{
+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
+  Account *account = lookup_account (self, account_name);
+  GVariant *v;
+  const gchar *s;
 
-      g_hash_table_iter_init (&iter, account->parameters);
+  if (flags != NULL)
+    *flags = 0;
 
-      while (g_hash_table_iter_next (&iter, &k, &v))
-        {
-          gchar *param_foo;
-          gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,
-              v);
+  if (!self->active || account == NULL)
+    return NULL;
 
-          param_foo = g_strdup_printf ("param-%s", (const gchar *) k);
-          mcp_account_manager_set_value (am, account_name, param_foo, escaped);
-          g_free (escaped);
+  g_dbus_connection_emit_signal (self->bus, NULL,
+      TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
+      "GetParameter",
+      g_variant_new_parsed ("(%o, %s)", account->path, parameter), NULL);
 
-          g_free (param_foo);
-        }
+  v = g_hash_table_lookup (account->parameters, parameter);
+  s = g_hash_table_lookup (account->untyped_parameters, parameter);
 
-      return TRUE;
+  if (v != NULL)
+    {
+      return g_variant_ref (v);
     }
-
-  /* get one parameter */
-
-  if (g_str_has_prefix (key, "param-"))
+  else if (s != NULL)
     {
-      GVariant *v = g_hash_table_lookup (account->parameters, key + 6);
-      const gchar *s = g_hash_table_lookup (account->untyped_parameters, key + 6);
-
-      g_dbus_connection_emit_signal (self->bus, NULL,
-          TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
-          "GetParameter",
-          g_variant_new_parsed ("(%o, %s)", account->path, key + 6), NULL);
-
-      if (v != NULL)
-        {
-          gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,
-              v);
-
-          mcp_account_manager_set_value (am, account_name, key, escaped);
-          g_free (escaped);
-        }
-      else if (s != NULL)
-        {
-          mcp_account_manager_set_value (am, account_name, key, s);
-        }
-      else
-        {
-          return FALSE;
-        }
+      return mcp_account_manager_unescape_variant_from_keyfile (am,
+          s, type, NULL);
     }
   else
     {
-      GVariant *v = g_hash_table_lookup (account->attributes, key);
-
-      g_dbus_connection_emit_signal (self->bus, NULL,
-          TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
-          "GetAttribute",
-          g_variant_new_parsed ("(%o, %s)", account->path, key), NULL);
-
-      if (v != NULL)
-        {
-          gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,
-              v);
-
-          mcp_account_manager_set_value (am, account_name, key, escaped);
-          g_free (escaped);
-        }
-      else
-        {
-          return FALSE;
-        }
+      return NULL;
     }
-
-  return TRUE;
 }
 
 static McpAccountStorageSetResult
@@ -1586,7 +1534,8 @@ account_storage_iface_init (McpAccountStorageIface *iface)
   /* this should be higher priority than the diverted-keyfile one */
   iface->priority = MCP_ACCOUNT_STORAGE_PLUGIN_PRIO_NORMAL + 100;
 
-  iface->get = test_dbus_account_plugin_get;
+  iface->get_attribute = test_dbus_account_plugin_get_attribute;
+  iface->get_parameter = test_dbus_account_plugin_get_parameter;
   iface->set_attribute = test_dbus_account_plugin_set_attribute;
   iface->set_parameter = test_dbus_account_plugin_set_parameter;
   iface->list = test_dbus_account_plugin_list;
diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c
index 7bba5d8f..11c7caa9 100644
--- a/tests/twisted/mcp-account-diversion.c
+++ b/tests/twisted/mcp-account-diversion.c
@@ -197,47 +197,57 @@ _set_parameter (McpAccountStorage *self,
   return ret;
 }
 
-static gboolean
-_get (const McpAccountStorage *self,
-    const McpAccountManager *am,
+static GVariant *
+_get_attribute (McpAccountStorage *self,
+    McpAccountManager *am,
     const gchar *account,
-    const gchar *key)
+    const gchar *attribute,
+    const GVariantType *type,
+    McpAttributeFlags *flags)
 {
   AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self);
+  gchar *v;
+  GVariant *ret;
 
-  if (key != NULL)
-    {
-      gchar *v = g_key_file_get_value (adp->keyfile, account, key, NULL);
+  if (flags != NULL)
+    *flags = 0;
 
-      if (v == NULL)
-        return FALSE;
+  v = g_key_file_get_value (adp->keyfile, account, attribute, NULL);
 
-      mcp_account_manager_set_value (am, account, key, v);
-      g_free (v);
-    }
-  else
-    {
-      gsize i;
-      gsize n;
-      GStrv keys = g_key_file_get_keys (adp->keyfile, account, &n, NULL);
+  if (v == NULL)
+    return NULL;
 
-      if (keys == NULL)
-        n = 0;
+  ret = mcp_account_manager_unescape_variant_from_keyfile (am, v, type, NULL);
+  g_free (v);
+  return ret;
+}
 
-      for (i = 0; i < n; i++)
-        {
-          gchar *v = g_key_file_get_value (adp->keyfile, account, keys[i], NULL);
+static GVariant *
+_get_parameter (McpAccountStorage *self,
+    McpAccountManager *am,
+    const gchar *account,
+    const gchar *parameter,
+    const GVariantType *type,
+    McpParameterFlags *flags)
+{
+  AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self);
+  gchar *key;
+  gchar *v;
+  GVariant *ret;
 
-          if (v != NULL)
-            mcp_account_manager_set_value (am, account, keys[i], v);
+  if (flags != NULL)
+    *flags = 0;
 
-          g_free (v);
-        }
+  key = g_strdup_printf ("param-%s", parameter);
+  v = g_key_file_get_value (adp->keyfile, account, key, NULL);
+  g_free (key);
 
-      g_strfreev (keys);
-    }
+  if (v == NULL)
+    return NULL;
 
-  return TRUE;
+  ret = mcp_account_manager_unescape_variant_from_keyfile (am, v, type, NULL);
+  g_free (v);
+  return ret;
 }
 
 static gboolean _commit (const McpAccountStorage *self,
@@ -371,7 +381,8 @@ account_storage_iface_init (McpAccountStorageIface *iface,
   iface->desc = PLUGIN_DESCRIPTION;
   iface->priority = PLUGIN_PRIORITY;
 
-  iface->get = _get;
+  iface->get_attribute = _get_attribute;
+  iface->get_parameter = _get_parameter;
   iface->set_attribute = _set_attribute;
   iface->set_parameter = _set_parameter;
   iface->delete_async = delete_async;