Attachment 93543 Details for Bug 71384 – If account storage plugins are GAsyncInitable, initialize them before list()

[patch] If account storage plugins are GAsyncInitable, initialize them before list()

0001-If-account-storage-plugins-are-GAsyncInitable-initia.patch (text/plain), 14.41 KB, created by Simon McVittie on 2014-02-06 16:29:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2014-02-06 16:29:42 UTC

Size: 14.41 KB

patch

obsolete

>From 855d286845d72f0da3c0e77c24bce117dcb90af2 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Thu, 6 Feb 2014 14:56:38 +0000
>Subject: [PATCH] If account storage plugins are GAsyncInitable, initialize
> them before list()
>
>This means they don't have to be prepared to block during list().
>---
> src/mcd-account-manager-priv.h      |   3 --
> src/mcd-account-manager.c           |  24 ++++-----
> src/mcd-master.c                    |   2 -
> src/mcd-storage.c                   |  88 +++++++++++++++++++++++++-----
> src/mcd-storage.h                   |   7 +++
> tests/twisted/dbus-account-plugin.c | 103 ++++++++++++++++++++++++++++--------
> tests/twisted/mctest.py             |  31 ++++++-----
> 7 files changed, 191 insertions(+), 67 deletions(-)
>
>diff --git a/src/mcd-account-manager-priv.h b/src/mcd-account-manager-priv.h
>index e89b1d3..b57c89a 100644
>--- a/src/mcd-account-manager-priv.h
>+++ b/src/mcd-account-manager-priv.h
>@@ -30,9 +30,6 @@
> 
> G_BEGIN_DECLS
> 
>-G_GNUC_INTERNAL void _mcd_account_manager_setup
>-    (McdAccountManager *account_manager);
>-
> G_GNUC_INTERNAL GHashTable *_mcd_account_manager_get_accounts
>     (McdAccountManager *account_manager);
> 
>diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c
>index a221910..9b9eb0e 100644
>--- a/src/mcd-account-manager.c
>+++ b/src/mcd-account-manager.c
>@@ -1373,26 +1373,20 @@ migrate_accounts (McdAccountManager *self)
>     }
> }
> 
>-/**
>- * _mcd_account_manager_setup:
>- * @account_manager: the #McdAccountManager.
>- *
>- * This function must be called by the McdMaster; it reads the accounts from
>- * the config file, and it needs a McdMaster instance to be active.
>- */
>-void
>-_mcd_account_manager_setup (McdAccountManager *account_manager)
>+static void
>+storage_loaded_cb (GObject *source_object,
>+    GAsyncResult *res,
>+    gpointer user_data)
> {
>+    McdAccountManager *account_manager = MCD_ACCOUNT_MANAGER (user_data);
>     McdAccountManagerPrivate *priv = account_manager->priv;
>     McdStorage *storage = priv->storage;
>     GHashTable *accounts;
>     GHashTableIter iter;
>     gpointer k, v;
> 
>-    /* for simplicity we don't support re-entrant setup */
>-    g_return_if_fail (priv->setup_lock == 0);
>-
>-    priv->setup_lock = 1; /* will be released at the end of this function */
>+    /* it can't fail at the moment; only check it if assertions are enabled */
>+    g_assert (mcd_storage_load_finish (storage, res, NULL));
> 
>     tp_list_connection_names (priv->dbus_daemon,
>                               list_connection_names_cb, NULL, NULL,
>@@ -1658,7 +1652,9 @@ _mcd_account_manager_constructed (GObject *obj)
>                           NULL);
> 
>     DEBUG ("loading plugins");
>-    mcd_storage_load (priv->storage);
>+    priv->setup_lock = 1;
>+    mcd_storage_load_async (priv->storage, storage_loaded_cb,
>+        g_object_ref (account_manager));
> 
>     /* initializes the interfaces */
>     mcd_dbus_init_interfaces_instances (account_manager);
>diff --git a/src/mcd-master.c b/src/mcd-master.c
>index d4b53cf..2858e20 100644
>--- a/src/mcd-master.c
>+++ b/src/mcd-master.c
>@@ -208,8 +208,6 @@ mcd_master_constructor (GType type, guint n_params,
>     priv->dispatcher = mcd_dispatcher_new (priv->dbus_daemon, master);
>     g_assert (MCD_IS_DISPATCHER (priv->dispatcher));
> 
>-    _mcd_account_manager_setup (priv->account_manager);
>-
>     dbus_connection_set_exit_on_disconnect (
>         dbus_g_connection_get_connection (
>             tp_proxy_get_dbus_connection (TP_PROXY (priv->dbus_daemon))),
>diff --git a/src/mcd-storage.c b/src/mcd-storage.c
>index b2d57dc..c8d35a5 100644
>--- a/src/mcd-storage.c
>+++ b/src/mcd-storage.c
>@@ -614,23 +614,20 @@ reconnect_cb (McpAccountStorage *plugin,
>         account_name);
> }
> 
>-/*
>- * mcd_storage_load:
>- * @storage: An object implementing the #McdStorage interface
>- *
>- * Load the long term account settings storage into our internal cache.
>- * Should only really be called during startup, ie before our DBus names
>- * have been claimed and other people might be relying on responses from us.
>- */
>-void
>-mcd_storage_load (McdStorage *self)
>+static void
>+mcd_storage_load_progress (GTask *task)
> {
>+  McdStorage *self = MCD_STORAGE (g_task_get_source_object (task));
>+  gsize remaining = GPOINTER_TO_SIZE (g_task_get_task_data (task));
>   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
>   GList *store = NULL;
> 
>-  g_return_if_fail (MCD_IS_STORAGE (self));
>-
>-  sort_and_cache_plugins ();
>+  if (remaining > 1)
>+    {
>+      /* wait for the rest */
>+      g_task_set_task_data (task, GSIZE_TO_POINTER (remaining - 1), NULL);
>+      return;
>+    }
> 
>   /* fetch accounts stored in plugins, highest priority first, so that
>    * low priority plugins can be overidden by high priority */
>@@ -680,6 +677,8 @@ mcd_storage_load (McdStorage *self)
>       /* already freed the contents, just need to free the list itself */
>       g_list_free (stored);
>     }
>+
>+  g_task_return_boolean (task, TRUE);
> }
> 
> /*
>@@ -2357,3 +2356,66 @@ finally:
>   g_strfreev (untyped_parameters);
>   return ret;
> }
>+
>+static void
>+mcd_storage_loaded_cb (GObject *source_object,
>+    GAsyncResult *res,
>+    gpointer user_data)
>+{
>+  McpAccountStorage *loaded = MCP_ACCOUNT_STORAGE (source_object);
>+  GError *error = NULL;
>+
>+  if (!g_async_initable_init_finish (G_ASYNC_INITABLE (source_object),
>+        res, &error))
>+    {
>+      /* don't use that one */
>+      WARNING ("%s failed to initialize: %s #%d: %s",
>+          mcp_account_storage_name (loaded),
>+          g_quark_to_string (error->domain), error->code, error->message);
>+      stores = g_list_remove (stores, loaded);
>+    }
>+
>+  /* Whether it was OK or not, we can stop waiting for it. */
>+  mcd_storage_load_progress (G_TASK (user_data));
>+
>+  g_object_unref (user_data);
>+}
>+
>+void
>+mcd_storage_load_async (McdStorage *self,
>+    GAsyncReadyCallback callback,
>+    gpointer user_data)
>+{
>+  GList *store;
>+  GTask *task = g_task_new (self, NULL, callback, user_data);
>+  gsize remaining = 1;
>+
>+  sort_and_cache_plugins ();
>+
>+  g_task_set_task_data (task, GSIZE_TO_POINTER (remaining), NULL);
>+
>+  for (store = stores; store != NULL; store = store->next)
>+    {
>+      if (G_IS_ASYNC_INITABLE (store->data))
>+        {
>+          remaining++;
>+          g_task_set_task_data (task, GSIZE_TO_POINTER (remaining), NULL);
>+          g_async_initable_init_async (store->data, G_PRIORITY_DEFAULT,
>+              NULL, mcd_storage_loaded_cb, g_object_ref (task));
>+        }
>+    }
>+
>+  /* just in case there were no async-initable stores */
>+  mcd_storage_load_progress (task);
>+  g_object_unref (task);
>+}
>+
>+gboolean
>+mcd_storage_load_finish (McdStorage *self,
>+    GAsyncResult *result,
>+    GError **error)
>+{
>+  g_return_val_if_fail (g_task_is_valid (result, self), FALSE);
>+
>+  return g_task_propagate_boolean (G_TASK (result), error);
>+}
>diff --git a/src/mcd-storage.h b/src/mcd-storage.h
>index 56732f1..bbeab73 100644
>--- a/src/mcd-storage.h
>+++ b/src/mcd-storage.h
>@@ -170,6 +170,13 @@ gboolean mcd_storage_maybe_migrate_parameters (McdStorage *self,
>     const gchar *account_name,
>     TpProtocol *protocol);
> 
>+void mcd_storage_load_async (McdStorage *self,
>+    GAsyncReadyCallback callback,
>+    gpointer user_data);
>+gboolean mcd_storage_load_finish (McdStorage *self,
>+    GAsyncResult *result,
>+    GError **error);
>+
> G_END_DECLS
> 
> #endif /* MCD_STORAGE_H */
>diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
>index 79464ed..d62136c 100644
>--- a/tests/twisted/dbus-account-plugin.c
>+++ b/tests/twisted/dbus-account-plugin.c
>@@ -78,6 +78,7 @@ test_dbus_account_free (gpointer p)
> }
> 
> static void account_storage_iface_init (McpAccountStorageIface *);
>+static void async_initable_iface_init (GAsyncInitableIface *);
> 
> struct _TestDBusAccountPlugin {
>   GObject parent;
>@@ -95,6 +96,8 @@ struct _TestDBusAccountPluginClass {
> 
> G_DEFINE_TYPE_WITH_CODE (TestDBusAccountPlugin, test_dbus_account_plugin,
>     G_TYPE_OBJECT,
>+    G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE,
>+        async_initable_iface_init);
>     G_IMPLEMENT_INTERFACE (MCP_TYPE_ACCOUNT_STORAGE,
>         account_storage_iface_init));
> 
>@@ -631,19 +634,19 @@ parameters_changed_cb (GDBusConnection *bus,
>   g_variant_unref (deleted);
> }
> 
>-static GList *
>-test_dbus_account_plugin_list (McpAccountStorage *storage,
>-    McpAccountManager *am)
>+static void get_accounts_cb (GObject *source_object,
>+    GAsyncResult *res,
>+    gpointer user_data);
>+
>+static void
>+test_dbus_account_plugin_init_async (GAsyncInitable *initable,
>+    int io_priority,
>+    GCancellable *cancellable,
>+    GAsyncReadyCallback callback,
>+    gpointer user_data)
> {
>-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
>-  GError *error = NULL;
>-  GVariant *tuple, *accounts;
>-  GVariant *attributes, *attribute_flags;
>-  GVariant *parameters, *untyped_parameters, *param_flags;
>-  GVariantIter account_iter;
>-  const gchar *account_name;
>-  GList *ret = NULL;
>-  guint32 restrictions;
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (initable);
>+  GTask *task;
> 
>   DEBUG ("called");
> 
>@@ -695,8 +698,9 @@ test_dbus_account_plugin_list (McpAccountStorage *storage,
>       g_object_ref (self),
>       g_object_unref);
> 
>-  /* list is allowed to block */
>-  tuple = g_dbus_connection_call_sync (self->bus,
>+  task = g_task_new (self, cancellable, callback, user_data);
>+
>+  g_dbus_connection_call (self->bus,
>       TEST_DBUS_ACCOUNT_SERVICE,
>       TEST_DBUS_ACCOUNT_SERVICE_PATH,
>       TEST_DBUS_ACCOUNT_SERVICE_IFACE,
>@@ -705,15 +709,35 @@ test_dbus_account_plugin_list (McpAccountStorage *storage,
>       G_VARIANT_TYPE ("(a{s(a{sv}a{su}a{sv}a{ss}a{su}u)})"),
>       G_DBUS_CALL_FLAGS_NONE,
>       -1,
>-      NULL, /* no cancellable */
>-      &error);
>+      cancellable,
>+      get_accounts_cb,
>+      task);
>+}
>+
>+static void
>+get_accounts_cb (GObject *source_object,
>+    GAsyncResult *res,
>+    gpointer user_data)
>+{
>+  GTask *task = G_TASK (user_data);
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (
>+      g_task_get_source_object (task));
>+  GError *error = NULL;
>+  GVariant *tuple, *accounts;
>+  GVariant *attributes, *attribute_flags;
>+  GVariant *parameters, *untyped_parameters, *param_flags;
>+  GVariantIter account_iter;
>+  const gchar *account_name;
>+  guint32 restrictions;
>+
>+  tuple = g_dbus_connection_call_finish (self->bus, res, &error);
> 
>   if (g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_NAME_HAS_NO_OWNER) ||
>       g_error_matches (error, G_DBUS_ERROR, G_DBUS_ERROR_SERVICE_UNKNOWN))
>     {
>-      /* this regression test isn't using the fake accounts service */
>-      g_clear_error (&error);
>-      return NULL;
>+      g_task_return_error (task, error);
>+      g_object_unref (task);
>+      return;
>     }
> 
>   self->active = TRUE;
>@@ -732,12 +756,42 @@ test_dbus_account_plugin_list (McpAccountStorage *storage,
>       test_dbus_account_plugin_add_account (self, account_name,
>           attributes, attribute_flags, parameters, untyped_parameters,
>           param_flags, restrictions);
>-
>-      ret = g_list_prepend (ret, g_strdup (account_name));
>     }
> 
>   g_variant_unref (accounts);
>   g_variant_unref (tuple);
>+
>+  g_task_return_boolean (task, TRUE);
>+  g_object_unref (task);
>+}
>+
>+static gboolean
>+test_dbus_account_plugin_init_finish (GAsyncInitable *initable,
>+    GAsyncResult *res,
>+    GError **error)
>+{
>+  return g_task_propagate_boolean (G_TASK (res), error);
>+}
>+
>+static GList *
>+test_dbus_account_plugin_list (McpAccountStorage *storage,
>+    McpAccountManager *am)
>+{
>+  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
>+  GList *ret = NULL;
>+  GHashTableIter iter;
>+  gpointer k;
>+
>+  DEBUG ("called");
>+
>+  /* in tests where we're not using this plugin, this shouldn't be called */
>+  g_return_val_if_fail (self->active, NULL);
>+
>+  g_hash_table_iter_init (&iter, self->accounts);
>+
>+  while (g_hash_table_iter_next (&iter, &k, NULL))
>+    ret = g_list_prepend (ret, g_strdup (k));
>+
>   return ret;
> }
> 
>@@ -1445,3 +1499,10 @@ account_storage_iface_init (McpAccountStorageIface *iface)
>   iface->get_restrictions = test_dbus_account_plugin_get_restrictions;
>   iface->create = test_dbus_account_plugin_create;
> }
>+
>+static void
>+async_initable_iface_init (GAsyncInitableIface *iface)
>+{
>+  iface->init_async = test_dbus_account_plugin_init_async;
>+  iface->init_finish = test_dbus_account_plugin_init_finish;
>+}
>diff --git a/tests/twisted/mctest.py b/tests/twisted/mctest.py
>index 549a742..f8e4f4f 100644
>--- a/tests/twisted/mctest.py
>+++ b/tests/twisted/mctest.py
>@@ -61,7 +61,8 @@ def install_colourer():
>     return sys.stdout
> 
> class MC(dbus.proxies.ProxyObject):
>-    def __init__(self, queue, bus, wait_for_names=True, initially_online=True):
>+    def __init__(self, queue, bus, wait_for_names=True, initially_online=True,
>+            wait_for_fake_accounts_service=False):
>         """
>         Arguments:
> 
>@@ -84,7 +85,13 @@ class MC(dbus.proxies.ProxyObject):
>         self.bus = bus
> 
>         if wait_for_names:
>-            self.wait_for_names()
>+            if wait_for_fake_accounts_service:
>+                self.wait_for_names(servicetest.EventPattern('dbus-signal',
>+                        path=cs.TEST_DBUS_ACCOUNT_PLUGIN_PATH,
>+                        interface=cs.TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
>+                        signal='Active'))
>+            else:
>+                self.wait_for_names()
> 
>     def wait_for_names(self, *also_expect):
>         """
>@@ -121,9 +128,16 @@ def exec_test_deferred (fun, params, protocol=None, timeout=None,
>     queue.attach_to_bus(bus)
>     error = None
> 
>+    if use_fake_accounts_service:
>+        fake_accounts_service = FakeAccountsService(queue, bus)
>+    else:
>+        fake_accounts_service = None
>+
>     if preload_mc:
>         try:
>-            mc = MC(queue, bus, initially_online=initially_online)
>+            mc = MC(queue, bus, initially_online=initially_online,
>+                    wait_for_fake_accounts_service=use_fake_accounts_service)
>+
>         except Exception, e:
>             import traceback
>             traceback.print_exc()
>@@ -131,17 +145,6 @@ def exec_test_deferred (fun, params, protocol=None, timeout=None,
>     else:
>         mc = None
> 
>-    if use_fake_accounts_service:
>-        fake_accounts_service = FakeAccountsService(queue, bus)
>-
>-        if preload_mc:
>-            queue.expect('dbus-signal',
>-                    path=cs.TEST_DBUS_ACCOUNT_PLUGIN_PATH,
>-                    interface=cs.TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
>-                    signal='Active')
>-    else:
>-        fake_accounts_service = None
>-
>     if pass_kwargs:
>         kwargs=dict(fake_accounts_service=fake_accounts_service)
>     else:
>-- 
>1.9.rc1
>

Actions: View | Splinter Review

Attachments on bug 71384: 88886 | 93543 | 93548 | 93549

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.