[Pkg-telepathy-commits] [telepathy-mission-control-6] 183/280: Make account deletion async (sort of), and imply a selective commit

Simon McVittie smcv at debian.org
Thu Mar 27 20:07:21 UTC 2014


This is an automated email from the git hooks/post-receive script.

smcv pushed a commit to branch debian
in repository telepathy-mission-control-6.

commit 561206632852eb2cea78ec60dfeb10a88a53cd63
Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Wed Nov 13 19:44:46 2013 +0000

    Make account deletion async (sort of), and imply a selective commit
    
    This means we don't need to commit separately after each deletion,
    and means account plugins don't have to have the concept of flagging
    an account for "delete this later" - much rejoicing.
    
    It also has the incidental benefit that we no longer use the C++
    reserved word 'delete' in a header file.
    
    Bug: https://bugs.freedesktop.org/show_bug.cgi?id=27727
---
 mission-control-plugins/account-storage.c |  97 ++++++++++--------
 mission-control-plugins/account-storage.h |  19 +++-
 src/mcd-account-manager-default.c         | 159 ++++++++++++++----------------
 src/mcd-account.c                         |   2 -
 src/mcd-storage.c                         |  28 +++++-
 tests/twisted/dbus-account-plugin.c       | 145 ++++++++++++++-------------
 tests/twisted/mcp-account-diversion.c     |  41 +++++---
 7 files changed, 275 insertions(+), 216 deletions(-)

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index 53b00be..b87909c 100644
--- a/mission-control-plugins/account-storage.c
+++ b/mission-control-plugins/account-storage.c
@@ -119,13 +119,24 @@ default_get (const McpAccountStorage *storage,
   return FALSE;
 }
 
-static gboolean
-default_delete (const McpAccountStorage *storage,
-    const McpAccountManager *am,
+static void
+default_delete_async (McpAccountStorage *storage,
+    McpAccountManager *am,
     const gchar *account,
-    const gchar *key)
+    GAsyncReadyCallback callback,
+    gpointer user_data)
 {
-  return FALSE;
+  g_task_report_new_error (storage, callback, user_data,
+      default_delete_async, TP_ERROR, TP_ERROR_NOT_IMPLEMENTED,
+      "This storage plugin cannot delete accounts");
+}
+
+static gboolean
+default_delete_finish (McpAccountStorage *storage,
+    GAsyncResult *res,
+    GError **error)
+{
+  return g_task_propagate_boolean (G_TASK (res), error);
 }
 
 static gboolean
@@ -230,7 +241,8 @@ class_init (gpointer klass,
 
   iface->get = default_get;
   iface->create = default_create;
-  iface->delete = default_delete;
+  iface->delete_async = default_delete_async;
+  iface->delete_finish = default_delete_finish;
   iface->commit = default_commit;
   iface->list = default_list;
   iface->ready = default_ready;
@@ -646,61 +658,62 @@ mcp_account_storage_create (const McpAccountStorage *storage,
 }
 
 /**
- * McpAccountStorageDeleteFunc:
+ * mcp_account_storage_delete_async:
  * @storage: an #McpAccountStorage instance
  * @am: an #McpAccountManager instance
  * @account: the unique name of the account
- * @key: (allow-none): the setting whose value we wish to store - either an
- *  attribute like "DisplayName", or "param-" plus a parameter like
- *  "account" - or %NULL to delete the entire account
+ * @callback: called on success or failure
+ * @user_data: data for @callback
  *
- * An implementation of mcp_account_storage_delete().
+ * Delete the account @account, and commit the change,
+ * emitting #McpAccountStorage::deleted afterwards.
  *
- * Returns: %TRUE if the setting or settings are not
- * the plugin's cache after this operation, %FALSE otherwise.
+ * Unlike the 'delete' virtual method in earlier MC versions, this
+ * function is expected to commit the change to long-term storage,
+ * is expected to emit #McpAccountStorage::deleted, and is
+ * not called for the deletion of individual attributes or parameters.
+ *
+ * The default implementation just returns failure (asynchronously),
+ * and is appropriate for read-only storage.
  */
+void
+mcp_account_storage_delete_async (McpAccountStorage *storage,
+    McpAccountManager *am,
+    const gchar *account,
+    GAsyncReadyCallback callback,
+    gpointer user_data)
+{
+  McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
+
+  SDEBUG (storage, "");
+  g_return_if_fail (iface != NULL);
+  g_return_if_fail (iface->delete_async != NULL);
+
+  iface->delete_async (storage, am, account, callback, user_data);
+}
 
 /**
- * mcp_account_storage_delete:
+ * mcp_account_storage_delete_finish:
  * @storage: an #McpAccountStorage instance
- * @am: an #McpAccountManager instance
- * @account: the unique name of the account
- * @key: (allow-none): the setting whose value we wish to store - either an
- *  attribute like "DisplayName", or "param-" plus a parameter like
- *  "account" - or %NULL to delete the entire account
- *
- * The plugin is expected to remove the setting for @key from its
- * internal cache and to remember that its state has changed, so
- * that it can delete said setting from its long term storage if
- * its long term storage method makes this necessary.
+ * @res: the result of mcp_account_storage_delete_async()
+ * @error: used to raise an error if %FALSE is returned
  *
- * If @key is %NULL, the plugin should forget all its settings for
- * @account,and remember to delete the entire account from its storage later.
+ * Process the result of mcp_account_storage_delete_async().
  *
- * The plugin is not expected to update its long term storage at
- * this point.
- *
- * The default implementation just returns %FALSE, and is appropriate for
- * read-only storage.
- *
- * Returns: %TRUE if the setting or settings are not
- * the plugin's cache after this operation, %FALSE otherwise.
- * This is very unlikely to ever be %FALSE, as a plugin is always
- * expected to be able to manipulate its own cache.
+ * Returns: %TRUE on success, %FALSE if the account could not be deleted
  */
 gboolean
-mcp_account_storage_delete (const McpAccountStorage *storage,
-    const McpAccountManager *am,
-    const gchar *account,
-    const gchar *key)
+mcp_account_storage_delete_finish (McpAccountStorage *storage,
+    GAsyncResult *result,
+    GError **error)
 {
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
 
   SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, FALSE);
-  g_return_val_if_fail (iface->delete != NULL, FALSE);
+  g_return_val_if_fail (iface->delete_finish != NULL, FALSE);
 
-  return iface->delete (storage, am, account, key);
+  return iface->delete_finish (storage, result, error);
 }
 
 /**
diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
index 198e835..ff3633e 100644
--- a/mission-control-plugins/account-storage.h
+++ b/mission-control-plugins/account-storage.h
@@ -107,7 +107,14 @@ struct _McpAccountStorageIface
   const gchar *provider;
 
   McpAccountStorageGetFunc get;
-  McpAccountStorageDeleteFunc delete;
+  void (*delete_async) (McpAccountStorage *storage,
+      McpAccountManager *am,
+      const gchar *account,
+      GAsyncReadyCallback callback,
+      gpointer user_data);
+  gboolean (*delete_finish) (McpAccountStorage *storage,
+      GAsyncResult *res,
+      GError **error);
   McpAccountStorageCommitFunc commit;
   McpAccountStorageListFunc list;
   McpAccountStorageReadyFunc ready;
@@ -149,10 +156,14 @@ gchar * mcp_account_storage_create (const McpAccountStorage *storage,
     const gchar *identification,
     GError **error);
 
-gboolean mcp_account_storage_delete (const McpAccountStorage *storage,
-    const McpAccountManager *am,
+void mcp_account_storage_delete_async (McpAccountStorage *storage,
+    McpAccountManager *am,
     const gchar *account,
-    const gchar *key);
+    GAsyncReadyCallback callback,
+    gpointer user_data);
+gboolean mcp_account_storage_delete_finish (McpAccountStorage *storage,
+    GAsyncResult *result,
+    GError **error);
 
 void mcp_account_storage_ready (const McpAccountStorage *storage,
     const McpAccountManager *am);
diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c
index 07f2231..f87aee7 100644
--- a/src/mcd-account-manager-default.c
+++ b/src/mcd-account-manager-default.c
@@ -47,8 +47,6 @@ typedef struct {
     /* owned string, parameter (without "param-") => owned string, value
      * 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;
     /* TRUE if the account doesn't really exist, but is here to stop us
      * loading it from a lower-priority file */
     gboolean absent;
@@ -79,7 +77,6 @@ ensure_stored_account (McdAccountManagerDefault *self,
       g_hash_table_insert (self->accounts, g_strdup (account), sa);
     }
 
-  sa->pending_deletion = FALSE;
   sa->absent = FALSE;
   return sa;
 }
@@ -346,38 +343,96 @@ _create (const McpAccountStorage *self,
   return unique_name;
 }
 
-static gboolean
-_delete (const McpAccountStorage *self,
-      const McpAccountManager *am,
-      const gchar *account,
-      const gchar *key)
+static void
+delete_async (McpAccountStorage *self,
+    McpAccountManager *am,
+    const gchar *account,
+    GAsyncReadyCallback callback,
+    gpointer user_data)
 {
   McdAccountManagerDefault *amd = MCD_ACCOUNT_MANAGER_DEFAULT (self);
   McdDefaultStoredAccount *sa = lookup_stored_account (amd, account);
+  GTask *task;
+  gchar *filename = NULL;
+  const gchar * const *iter;
+
+  task = g_task_new (amd, NULL, callback, user_data);
 
   if (sa == NULL || sa->absent)
     {
       /* Apparently we never had this account anyway. The plugin API
        * considers this to be "success". */
-      return TRUE;
+      g_task_return_boolean (task, TRUE);
+      goto finally;
     }
 
-  if (key == NULL)
+  filename = account_file_in (g_get_user_data_dir (), account);
+
+  DEBUG ("Deleting account %s from %s", account, filename);
+
+  if (g_unlink (filename) != 0)
     {
-      amd->save = TRUE;
+      int e = errno;
 
-      /* 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);
+      /* ENOENT is OK, anything else is more upsetting */
+      if (e != ENOENT)
+        {
+          WARNING ("Unable to delete %s: %s", filename,
+              g_strerror (e));
+          g_task_return_new_error (task, G_IO_ERROR, g_io_error_from_errno (e),
+              "Unable to delete %s: %s", filename, g_strerror (e));
+          goto finally;
+        }
     }
-  else
+
+  for (iter = g_get_system_data_dirs ();
+      iter != NULL && *iter != NULL;
+      iter++)
     {
-      g_assert_not_reached ();
+      gchar *other = account_file_in (*iter, account);
+      gboolean other_exists = g_file_test (other, G_FILE_TEST_EXISTS);
+
+      g_free (other);
+
+      if (other_exists)
+        {
+          GError *error = NULL;
+
+          /* There is a lower-priority file that would provide this
+           * account. We can't delete a file from XDG_DATA_DIRS which
+           * are conceptually read-only, but we can mask it with an
+           * empty file (prior art: systemd) */
+          if (!g_file_set_contents (filename, "", 0, &error))
+            {
+              g_prefix_error (&error,
+                  "Unable to save empty account file to %s: ", filename);
+              WARNING ("%s", error->message);
+              g_task_return_error (task, error);
+              g_free (filename);
+              goto finally;
+            }
+
+          break;
+        }
     }
 
-  return TRUE;
+  /* clean up the mess */
+  g_hash_table_remove (amd->accounts, account);
+  mcp_account_storage_emit_deleted (self, account);
+
+  g_task_return_boolean (task, TRUE);
+
+finally:
+  g_free (filename);
+  g_object_unref (task);
+}
+
+static gboolean
+delete_finish (McpAccountStorage *storage,
+    GAsyncResult *res,
+    GError **error)
+{
+  return g_task_propagate_boolean (G_TASK (res), error);
 }
 
 static gboolean
@@ -397,58 +452,6 @@ am_default_commit_one (McdAccountManagerDefault *self,
 
   filename = account_file_in (g_get_user_data_dir (), account_name);
 
-  if (sa->pending_deletion)
-    {
-      const gchar * const *iter;
-
-      DEBUG ("Deleting account %s from %s", account_name, filename);
-
-      if (g_unlink (filename) != 0)
-        {
-          int e = errno;
-
-          /* ENOENT is OK, anything else is more upsetting */
-          if (e != ENOENT)
-            {
-              WARNING ("Unable to delete %s: %s", filename,
-                  g_strerror (e));
-              g_free (filename);
-              return FALSE;
-            }
-        }
-
-      for (iter = g_get_system_data_dirs ();
-          iter != NULL && *iter != NULL;
-          iter++)
-        {
-          gchar *other = account_file_in (*iter, account_name);
-          gboolean other_exists = g_file_test (other, G_FILE_TEST_EXISTS);
-
-          g_free (other);
-
-          if (other_exists)
-            {
-              /* There is a lower-priority file that would provide this
-               * account. We can't delete a file from XDG_DATA_DIRS which
-               * are conceptually read-only, but we can mask it with an
-               * empty file (prior art: systemd) */
-              if (!g_file_set_contents (filename, "", 0, &error))
-                {
-                  WARNING ("Unable to save empty account file to %s: %s",
-                      filename, error->message);
-                  g_clear_error (&error);
-                  g_free (filename);
-                  return FALSE;
-                }
-
-              break;
-            }
-        }
-
-      g_free (filename);
-      return TRUE;
-    }
-
   DEBUG ("Saving account %s to %s", account_name, filename);
 
   g_variant_builder_init (&attrs_builder, G_VARIANT_TYPE_VARDICT);
@@ -544,17 +547,6 @@ _commit (const McpAccountStorage *self,
       amd->save = FALSE;
     }
 
-  g_hash_table_iter_init (&outer, amd->accounts);
-
-  /* forget about any entirely removed accounts */
-  while (g_hash_table_iter_next (&outer, NULL, &sa_p))
-    {
-      McdDefaultStoredAccount *sa = sa_p;
-
-      if (sa->pending_deletion)
-        g_hash_table_iter_remove (&outer);
-    }
-
   return all_succeeded;
 }
 
@@ -982,7 +974,8 @@ account_storage_iface_init (McpAccountStorageIface *iface,
   iface->set_attribute = set_attribute;
   iface->set_parameter = set_parameter;
   iface->create = _create;
-  iface->delete = _delete;
+  iface->delete_async = delete_async;
+  iface->delete_finish = delete_finish;
   iface->commit = _commit;
   iface->list = _list;
 
diff --git a/src/mcd-account.c b/src/mcd-account.c
index 6c08d8e..e70eaad 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -765,8 +765,6 @@ mcd_account_delete_async (McdAccount *account,
         g_free (data_dir_str);
     }
 
-    mcd_storage_commit (priv->storage, name);
-
     if (!priv->removed)
     {
         DEBUG ("emitting Account.Removed for %s", name);
diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index dbc04c3..25194b3 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -1942,6 +1942,28 @@ mcd_storage_create_account (McdStorage *self,
   return NULL;
 }
 
+static void
+delete_cb (GObject *source,
+    GAsyncResult *res,
+    gpointer user_data)
+{
+  GError *error = NULL;
+
+  if (mcp_account_storage_delete_finish (MCP_ACCOUNT_STORAGE (source),
+        res, &error))
+    {
+      DEBUG ("deleted account %s", (const gchar *) user_data);
+    }
+  else
+    {
+      DEBUG ("could not delete account %s (but no way to signal that): "
+          "%s #%d: %s", (const gchar *) user_data,
+          g_quark_to_string (error->domain), error->code, error->message);
+      g_error_free (error);
+    }
+
+  g_free (user_data);
+}
 
 /*
  * mcd_storage_delete_account:
@@ -1969,7 +1991,11 @@ mcd_storage_delete_account (McdStorage *self,
     {
       McpAccountStorage *plugin = store->data;
 
-      mcp_account_storage_delete (plugin, ma, account, NULL);
+      /* FIXME: when we know which plugin owns the account, we can stop
+       * ignoring the error (if any), and make this method async
+       * in order to pass the error up to McdAccount */
+      mcp_account_storage_delete_async (plugin, ma, account,
+          delete_cb, g_strdup (account));
     }
 }
 
diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 3ea5df9..01410a1 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -54,7 +54,7 @@ typedef struct {
     GHashTable *parameter_flags;
     /* set of strings */
     GHashTable *uncommitted_parameters;
-    enum { UNCOMMITTED_CREATION, UNCOMMITTED_DELETION } flags;
+    enum { UNCOMMITTED_CREATION = 1 } flags;
     TpStorageRestrictionFlags restrictions;
 } Account;
 
@@ -183,7 +183,6 @@ ensure_account (TestDBusAccountPlugin *self,
       g_hash_table_insert (self->accounts, g_strdup (account_name), account);
     }
 
-  account->flags &= ~UNCOMMITTED_DELETION;
   return account;
 }
 
@@ -211,18 +210,14 @@ service_vanished_cb (GDBusConnection *bus,
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (user_data);
   GHashTableIter iter;
-  gpointer k, v;
+  gpointer k;
 
   self->active = FALSE;
   g_hash_table_iter_init (&iter, self->accounts);
 
-  while (g_hash_table_iter_next (&iter, &k, &v))
+  while (g_hash_table_iter_next (&iter, &k, NULL))
     {
-      Account *account = v;
-
-      if ((account->flags & UNCOMMITTED_DELETION) == 0)
-        mcp_account_storage_emit_deleted (MCP_ACCOUNT_STORAGE (self), k);
-
+      mcp_account_storage_emit_deleted (MCP_ACCOUNT_STORAGE (self), k);
       g_hash_table_iter_remove (&iter);
     }
 
@@ -908,44 +903,64 @@ test_dbus_account_plugin_create (const McpAccountStorage *storage,
   return name;
 }
 
-static gboolean
-test_dbus_account_plugin_delete (const McpAccountStorage *storage,
-    const McpAccountManager *am,
+static void delete_account_cb (GObject *source_object,
+    GAsyncResult *res,
+    gpointer user_data);
+
+static void
+test_dbus_account_plugin_delete_async (McpAccountStorage *storage,
+    McpAccountManager *am,
     const gchar *account_name,
-    const gchar *key)
+    GAsyncReadyCallback callback,
+    gpointer user_data)
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
   Account *account = lookup_account (self, account_name);
+  GTask *task = g_task_new (self, NULL, callback, user_data);
+
+  g_task_set_task_data (task, g_strdup (user_data), g_free);
 
   DEBUG ("called");
 
   if (account == NULL || !self->active)
-    return FALSE;
-
-  if (key == NULL)
     {
-      account->flags |= UNCOMMITTED_DELETION;
-      g_hash_table_remove_all (account->attributes);
-      g_hash_table_remove_all (account->parameters);
-      g_hash_table_remove_all (account->untyped_parameters);
-      g_hash_table_remove_all (account->attribute_flags);
-      g_hash_table_remove_all (account->parameter_flags);
+      /* We were asked to delete an account we don't have. It's
+       * a bit like success. */
+      g_task_return_boolean (task, TRUE);
+      g_object_unref (task);
+      return;
+    }
 
-      account->flags &= ~UNCOMMITTED_CREATION;
-      g_hash_table_remove_all (account->uncommitted_attributes);
-      g_hash_table_remove_all (account->uncommitted_parameters);
+  /* deletion used to be delayed, so the regression tests will expect this
+   * to happen - leave them unmodified for now */
+  g_dbus_connection_emit_signal (self->bus, NULL,
+      TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
+      "DeferringDelete", g_variant_new_parsed ("(%o,)", account->path),
+      NULL);
+  g_dbus_connection_emit_signal (self->bus, NULL,
+      TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
+      "CommittingOne", g_variant_new_parsed ("(%o,)", account->path), NULL);
 
-      g_dbus_connection_emit_signal (self->bus, NULL,
-          TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
-          "DeferringDelete", g_variant_new_parsed ("(%o,)", account->path),
-          NULL);
-    }
-  else
-    {
-      g_assert_not_reached ();
-    }
+  g_dbus_connection_call (self->bus,
+      TEST_DBUS_ACCOUNT_SERVICE,
+      TEST_DBUS_ACCOUNT_SERVICE_PATH,
+      TEST_DBUS_ACCOUNT_SERVICE_IFACE,
+      "DeleteAccount",
+      g_variant_new_parsed ("(%s,)", account_name),
+      G_VARIANT_TYPE_UNIT,
+      G_DBUS_CALL_FLAGS_NONE,
+      -1,
+      NULL, /* no cancellable */
+      delete_account_cb,
+      task);
+}
 
-  return TRUE;
+static gboolean
+test_dbus_account_plugin_delete_finish (McpAccountStorage *storage,
+    GAsyncResult *res,
+    GError **error)
+{
+  return g_task_propagate_boolean (G_TASK (res), error);
 }
 
 static gboolean
@@ -957,7 +972,7 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
   Account *account = lookup_account (self, account_name);
 
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return FALSE;
 
   if (key == NULL)
@@ -1083,8 +1098,7 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
 
   DEBUG ("%s of %s", attribute, account_name);
 
-  if (!self->active || account == NULL ||
-      (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return FALSE;
 
   if (value == NULL)
@@ -1132,8 +1146,7 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
 
   DEBUG ("%s of %s", parameter, account_name);
 
-  if (!self->active || account == NULL ||
-      (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return FALSE;
 
   if (value == NULL)
@@ -1202,27 +1215,31 @@ delete_account_cb (GObject *source_object,
     GAsyncResult *res,
     gpointer user_data)
 {
-  AsyncData *ad = user_data;
+  GTask *task = user_data;
   GVariant *tuple;
   GError *error = NULL;
+  TestDBusAccountPlugin *self = g_task_get_source_object (task);
+  const gchar *account_name = g_task_get_task_data (task);
 
-  tuple = g_dbus_connection_call_finish (ad->self->bus, res, &error);
+  tuple = g_dbus_connection_call_finish (self->bus, res, &error);
 
   if (tuple != NULL)
     {
-      g_hash_table_remove (ad->self->accounts, ad->account_name);
+      /* we'll emit ::deleted when we see the signal, which probably
+       * already happened */
+      g_hash_table_remove (self->accounts, account_name);
       g_variant_unref (tuple);
+      g_task_return_boolean (task, TRUE);
     }
   else
     {
-      g_warning ("Unable to delete account %s: %s", ad->account_name,
-          error->message);
-      g_clear_error (&error);
-      /* FIXME: we could roll back the deletion by claiming that
-       * the service re-created the account? */
+      g_prefix_error (&error, "Unable to delete account %s: ",
+          account_name);
+      g_warning ("%s", error->message);
+      g_task_return_error (task, error);
     }
 
-  async_data_free (ad);
+  g_object_unref (task);
 }
 
 static void
@@ -1355,25 +1372,6 @@ test_dbus_account_plugin_commit (const McpAccountStorage *storage,
       TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
       "CommittingOne", g_variant_new_parsed ("(%o,)", account->path), NULL);
 
-  if (account->flags & UNCOMMITTED_DELETION)
-    {
-      g_dbus_connection_call (self->bus,
-          TEST_DBUS_ACCOUNT_SERVICE,
-          TEST_DBUS_ACCOUNT_SERVICE_PATH,
-          TEST_DBUS_ACCOUNT_SERVICE_IFACE,
-          "DeleteAccount",
-          g_variant_new_parsed ("(%s,)", account_name),
-          G_VARIANT_TYPE_UNIT,
-          G_DBUS_CALL_FLAGS_NONE,
-          -1,
-          NULL, /* no cancellable */
-          delete_account_cb,
-          async_data_new (self, account_name));
-
-      /* this doesn't mean we succeeded: it means we tried */
-      return TRUE;
-    }
-
   if (account->flags & UNCOMMITTED_CREATION)
     {
       g_dbus_connection_call (self->bus,
@@ -1509,7 +1507,7 @@ test_dbus_account_plugin_get_identifier (const McpAccountStorage *storage,
 
   DEBUG ("%s", account_name);
 
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return;
 
   /* Our "library-specific unique identifier" is just the object-path
@@ -1528,7 +1526,7 @@ test_dbus_account_plugin_get_additional_info (const McpAccountStorage *storage,
 
   DEBUG ("%s", account_name);
 
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return NULL;
 
   ret = g_hash_table_new_full (g_str_hash, g_str_equal,
@@ -1548,7 +1546,7 @@ test_dbus_account_plugin_get_restrictions (const McpAccountStorage *storage,
 
   DEBUG ("%s", account_name);
 
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return 0;
 
   return account->restrictions;
@@ -1564,7 +1562,7 @@ test_dbus_account_plugin_owns (McpAccountStorage *storage,
 
   DEBUG ("%s", account_name);
 
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
+  if (!self->active || account == NULL)
     return FALSE;
 
   return TRUE;
@@ -1583,7 +1581,8 @@ account_storage_iface_init (McpAccountStorageIface *iface)
   iface->set_parameter = test_dbus_account_plugin_set_parameter;
   iface->list = test_dbus_account_plugin_list;
   iface->ready = test_dbus_account_plugin_ready;
-  iface->delete = test_dbus_account_plugin_delete;
+  iface->delete_async = test_dbus_account_plugin_delete_async;
+  iface->delete_finish = test_dbus_account_plugin_delete_finish;
   iface->commit = test_dbus_account_plugin_commit;
   iface->get_identifier = test_dbus_account_plugin_get_identifier;
   iface->get_additional_info = test_dbus_account_plugin_get_additional_info;
diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c
index 9639faf..8e210d6 100644
--- a/tests/twisted/mcp-account-diversion.c
+++ b/tests/twisted/mcp-account-diversion.c
@@ -223,27 +223,45 @@ _get (const McpAccountStorage *self,
   return TRUE;
 }
 
-static gboolean
-_delete (const McpAccountStorage *self,
-      const McpAccountManager *am,
-      const gchar *account,
-      const gchar *key)
+static gboolean _commit (const McpAccountStorage *self,
+    const McpAccountManager *am,
+    const gchar *account_name);
+
+static void
+delete_async (McpAccountStorage *self,
+    McpAccountManager *am,
+    const gchar *account,
+    GAsyncReadyCallback callback,
+    gpointer user_data)
 {
   AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self);
+  GTask *task = g_task_new (adp, NULL, callback, user_data);
 
-  if (key == NULL)
+  if (g_key_file_remove_group (adp->keyfile, account, NULL))
+    adp->save = TRUE;
+
+  if (_commit (self, am, account))
     {
-      if (g_key_file_remove_group (adp->keyfile, account, NULL))
-        adp->save = TRUE;
+      mcp_account_storage_emit_deleted (self, account);
+      g_task_return_boolean (task, TRUE);
     }
   else
     {
-      g_assert_not_reached ();
+      g_task_return_new_error (task, TP_ERROR, TP_ERROR_NOT_AVAILABLE,
+          "_commit()'s error handling is not good enough to know why "
+          "I couldn't commit the deletion of %s", account);
     }
 
-  return TRUE;
+  g_object_unref (task);
 }
 
+static gboolean
+delete_finish (McpAccountStorage *storage,
+    GAsyncResult *res,
+    GError **error)
+{
+  return g_task_propagate_boolean (G_TASK (res), error);
+}
 
 static gboolean
 _commit (const McpAccountStorage *self,
@@ -310,7 +328,8 @@ account_storage_iface_init (McpAccountStorageIface *iface,
   iface->get = _get;
   iface->set_attribute = _set_attribute;
   iface->set_parameter = _set_parameter;
-  iface->delete = _delete;
+  iface->delete_async = delete_async;
+  iface->delete_finish = delete_finish;
   iface->commit = _commit;
   iface->list = _list;
 }

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-telepathy/telepathy-mission-control-6.git



More information about the Pkg-telepathy-commits mailing list