Simplify a bit storage API

This is an API break, but we already did some since last release.

This removes mcp_account_storage_commit() because it is redundant with
mcp_account_storage_commit_one (plugin, am, NULL);

This removes mcp_account_storage_owns() because an account is now
owned by one and only one storage plugin and MC now keeps track of
which storage plugin each account comes from.

Finally this adds default implementation on most iface methods to
make read-only plugins easier to implement. Only _get() and _list()
and mandatory.

7 files changed, 229 insertions, 342 deletions

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index b51c1264..28ca4cfa 100644
--- a/mission-control-plugins/account-storage.c
+++ b/mission-control-plugins/account-storage.c
@@ -59,7 +59,6 @@
  *   iface->get = foo_plugin_get;
  *   iface->set = foo_plugin_get;
  *   iface->delete = foo_plugin_delete;
- *   iface->commit = foo_plugin_commit;
  *   iface->commit_one = foo_plugin_commit_one;
  *   iface->list = foo_plugin_list;
  *   iface->ready = foo_plugin_ready;
@@ -67,7 +66,6 @@
  *   iface->get_additional_info = foo_plugin_get_additional_info;
  *   iface->get_restrictions = foo_plugin_get_restrictions;
  *   iface->create = foo_plugin_create;
- *   iface->owns = foo_plugin_owns;
  *   iface->set_attribute = foo_plugin_set_attribute;
  *   iface->set_parameter = foo_plugin_set_parameter;
  * }
@@ -144,17 +142,63 @@ default_set_parameter (McpAccountStorage *storage,
   return FALSE;
 }
 
+static gchar *
+default_create (const McpAccountStorage *storage,
+    const McpAccountManager *am,
+    const gchar *manager,
+    const gchar *protocol,
+    const gchar *identification,
+    GError **error)
+{
+  g_set_error (error, TP_ERROR, TP_ERROR_NOT_IMPLEMENTED,
+      "This storage does not implement create function");
+  return NULL;
+}
+
 static gboolean
-default_owns (McpAccountStorage *storage,
-    McpAccountManager *am,
+default_delete (const McpAccountStorage *storage,
+    const McpAccountManager *am,
+    const gchar *account,
+    const gchar *key)
+{
+  return FALSE;
+}
+
+static gboolean
+default_commit_one (const McpAccountStorage *storage,
+    const McpAccountManager *am,
     const gchar *account)
 {
-  /* This has the side-effect of pushing the "manager" key back into @am,
-   * but that should be a no-op in practice: we always call this
-   * method in priority order and stop at the first one that says "yes",
-   * and @am's idea of what "manager" is should have come from that same
-   * plugin anyway. */
-  return mcp_account_storage_get (storage, am, account, "manager");
+  return FALSE;
+}
+
+static void
+default_ready (const McpAccountStorage *storage,
+    const McpAccountManager *am)
+{
+}
+
+static void
+default_get_identifier (const McpAccountStorage *storage,
+    const gchar *account,
+    GValue *identifier)
+{
+  g_value_init (identifier, G_TYPE_STRING);
+  g_value_set_string (identifier, account);
+}
+
+static GHashTable *
+default_get_additional_info (const McpAccountStorage *storage,
+    const gchar *account)
+{
+  return g_hash_table_new (g_str_hash, g_str_equal);
+}
+
+static TpStorageRestrictionFlags
+default_get_restrictions (const McpAccountStorage *storage,
+    const gchar *account)
+{
+  return 0;
 }
 
 static void
@@ -164,10 +208,16 @@ class_init (gpointer klass,
   GType type = G_TYPE_FROM_CLASS (klass);
   McpAccountStorageIface *iface = klass;
 
-  iface->owns = default_owns;
   iface->set = default_set;
   iface->set_attribute = default_set_attribute;
   iface->set_parameter = default_set_parameter;
+  iface->create = default_create;
+  iface->delete = default_delete;
+  iface->commit_one = default_commit_one;
+  iface->ready = default_ready;
+  iface->get_identifier = default_get_identifier;
+  iface->get_additional_info = default_get_additional_info;
+  iface->get_restrictions = default_get_restrictions;
 
   if (signals[CREATED] != 0)
     {
@@ -313,7 +363,6 @@ mcp_account_storage_get_type (void)
  * @set: implementation of mcp_account_storage_set()
  * @get: implementation of mcp_account_storage_get()
  * @delete: implementation of mcp_account_storage_delete()
- * @commit: implementation of mcp_account_storage_commit()
  * @list: implementation of mcp_account_storage_list()
  * @ready: implementation of mcp_account_storage_ready()
  * @commit_one: implementation of mcp_account_storage_commit_one()
@@ -453,9 +502,8 @@ mcp_account_storage_get (const McpAccountStorage *storage,
  * decline to store the setting.
  *
  * The plugin is not expected to write to its long term storage
- * at this point. It can expect Mission Control to call either
- * mcp_account_storage_commit() or mcp_account_storage_commit_one()
- * after a short delay.
+ * at this point. It can expect Mission Control to call
+ * mcp_account_storage_commit_one() after a short delay.
  *
  * Plugins that implement mcp_storage_set_attribute() and
  * mcp_account_storage_set_parameter() can just return %FALSE here.
@@ -474,6 +522,7 @@ mcp_account_storage_set (const McpAccountStorage *storage,
 
   SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, FALSE);
+  g_return_val_if_fail (iface->set != NULL, FALSE);
 
   return iface->set (storage, am, account, key, value);
 }
@@ -593,7 +642,7 @@ mcp_account_storage_set_parameter (McpAccountStorage *storage,
  * Inform the plugin that a new account is being created. @manager, @protocol
  * and @identification are given to help determining the account's unique name,
  * but does not need to be stored on the account yet, mcp_account_storage_set()
- * and mcp_account_storage_commit() will be called later.
+ * and mcp_account_storage_commit_one() will be called later.
  *
  * It is recommended to use mcp_account_manager_get_unique_name() to create the
  * unique name, but it's not mandatory. One could base the unique name on an
@@ -601,7 +650,10 @@ mcp_account_storage_set_parameter (McpAccountStorage *storage,
  * (e.g. goa__1234).
  *
  * #McpAccountStorage::created signal should not be emitted for this account,
- * not even when mcp_account_storage_commit() will be called.
+ * not even when mcp_account_storage_commit_one() will be called.
+ *
+ * There is a default implementation, which just returns %NULL and raise an
+ * error.
  *
  * Returns: (transfer full): the newly allocated account name, which should
  *  be freed once the caller is done with it, or %NULL if that couldn't
@@ -617,14 +669,9 @@ mcp_account_storage_create (const McpAccountStorage *storage,
 {
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
 
+  SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, NULL);
-
-  if (iface->create == NULL)
-    {
-      g_set_error (error, TP_ERROR, TP_ERROR_NOT_IMPLEMENTED,
-          "This storage does not implement create function");
-      return NULL;
-    }
+  g_return_val_if_fail (iface->create != NULL, NULL);
 
   return iface->create (storage, am, manager, protocol, identification, error);
 }
@@ -664,6 +711,9 @@ mcp_account_storage_create (const McpAccountStorage *storage,
  * The plugin is not expected to update its long term storage at
  * this point.
  *
+ * There is a default implementation, which just returns %FALSE, for read-only
+ * plugins.
+ *
  * 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
@@ -679,24 +729,29 @@ mcp_account_storage_delete (const McpAccountStorage *storage,
 
   SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, FALSE);
+  g_return_val_if_fail (iface->delete != NULL, FALSE);
 
   return iface->delete (storage, am, account, key);
 }
 
 /**
- * McpAccountStorageCommitFunc:
+ * McpAccountStorageCommitOneFunc:
  * @storage: an #McpAccountStorage instance
  * @am: an #McpAccountManager instance
+ * @account: (allow-none): the unique suffix of an account's object path,
+ *  or %NULL
  *
- * An implementation of mcp_account_storage_commit().
+ * An implementation of mcp_account_storage_commit_one().
  *
  * Returns: %TRUE if the commit process was started successfully
  */
 
 /**
- * mcp_account_storage_commit:
+ * mcp_account_storage_commit_one:
  * @storage: an #McpAccountStorage instance
  * @am: an #McpAccountManager instance
+ * @account: (allow-none): the unique suffix of an account's object path,
+ *  or %NULL if all accounts are to be committed
  *
  * The plugin is expected to write its cache to long term storage,
  * deleting, adding or updating entries in said storage as needed.
@@ -705,67 +760,11 @@ mcp_account_storage_delete (const McpAccountStorage *storage,
  * not required to have finished its commit operation when it returns,
  * merely to have started the operation.
  *
- * If the @commit_one method is implemented, it will be called preferentially
- * if only one account is to be committed. If the @commit_one method is
- * implemented but @commit is not, @commit_one will be called with
- * @account_name = %NULL to commit all accounts.
+ * If @account = %NULL it means that it should commit all accounts owned by the
+ * storage plugin.
  *
- * Returns: %TRUE if the commit process was started (but not necessarily
- * completed) successfully; %FALSE if there was a problem that was immediately
- * obvious.
- */
-gboolean
-mcp_account_storage_commit (const McpAccountStorage *storage,
-    const McpAccountManager *am)
-{
-  McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
-
-  SDEBUG (storage, "committing all accounts");
-  g_return_val_if_fail (iface != NULL, FALSE);
-
-  if (iface->commit != NULL)
-    {
-      return iface->commit (storage, am);
-    }
-  else if (iface->commit_one != NULL)
-    {
-      return iface->commit_one (storage, am, NULL);
-    }
-  else
-    {
-      SDEBUG (storage,
-          "neither commit nor commit_one is implemented; cannot save accounts");
-      return FALSE;
-    }
-}
-
-/**
- * McpAccountStorageCommitOneFunc:
- * @storage: an #McpAccountStorage instance
- * @am: an #McpAccountManager instance
- * @account: (allow-none): the unique suffix of an account's object path,
- *  or %NULL
- *
- * An implementation of mcp_account_storage_commit_one().
- *
- * Returns: %TRUE if the commit process was started successfully
- */
-
-/**
- * mcp_account_storage_commit_one:
- * @storage: an #McpAccountStorage instance
- * @am: an #McpAccountManager instance
- * @account: (allow-none): the unique suffix of an account's object path,
- *  or %NULL if all accounts are to be committed and
- *  mcp_account_storage_commit() is unimplemented
- *
- * The same as mcp_account_storage_commit(), but only commit the given
- * account. This is optional to implement; the default implementation
- * is to call @commit.
- *
- * If both mcp_account_storage_commit_one() and mcp_account_storage_commit()
- * are implemented, Mission Control will never pass @account = %NULL to
- * this method.
+ * A default implementation that simply returns %FALSE is provided for read-only
+ * plugins.
  *
  * Returns: %TRUE if the commit process was started (but not necessarily
  * completed) successfully; %FALSE if there was a problem that was immediately
@@ -780,12 +779,9 @@ mcp_account_storage_commit_one (const McpAccountStorage *storage,
 
   SDEBUG (storage, "called for %s", account ? account : "<all accounts>");
   g_return_val_if_fail (iface != NULL, FALSE);
+  g_return_val_if_fail (iface->commit_one != NULL, FALSE);
 
-  if (iface->commit_one != NULL)
-    return iface->commit_one (storage, am, account);
-  else
-    /* Fall back to plain ->commit() */
-    return mcp_account_storage_commit (storage, am);
+  return iface->commit_one (storage, am, account);
 }
 
 /**
@@ -822,6 +818,7 @@ mcp_account_storage_list (const McpAccountStorage *storage,
 
   SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, NULL);
+  g_return_val_if_fail (iface->list != NULL, NULL);
 
   return iface->list (storage, am);
 }
@@ -842,6 +839,9 @@ mcp_account_storage_list (const McpAccountStorage *storage,
  * Informs the plugin that it is now permitted to create new accounts,
  * ie it can now fire its "created", "altered-one", "toggled" and "deleted"
  * signals.
+ *
+ * There is a default implementation for plugins that can't create accounts from
+ * external sources, as they can never fire the async account change signals.
  */
 void
 mcp_account_storage_ready (const McpAccountStorage *storage,
@@ -850,12 +850,9 @@ mcp_account_storage_ready (const McpAccountStorage *storage,
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
 
   g_return_if_fail (iface != NULL);
+  g_return_if_fail (iface->ready != NULL);
 
-  /* plugins that can't create accounts from external sources don't  *
-   * need to implement this method, as they can never fire the async *
-   * account change signals:                                         */
-  if (iface->ready != NULL)
-    iface->ready (storage, am);
+  iface->ready (storage, am);
 }
 
 /**
@@ -880,6 +877,8 @@ mcp_account_storage_ready (const McpAccountStorage *storage,
  *
  * This method will only be called for the storage plugin that "owns"
  * the account.
+ *
+ * There is default implementation that sets @identifier to @account string.
  */
 void
 mcp_account_storage_get_identifier (const McpAccountStorage *storage,
@@ -890,18 +889,11 @@ mcp_account_storage_get_identifier (const McpAccountStorage *storage,
 
   SDEBUG (storage, "");
   g_return_if_fail (iface != NULL);
+  g_return_if_fail (iface->get_identifier != NULL);
   g_return_if_fail (identifier != NULL);
   g_return_if_fail (!G_IS_VALUE (identifier));
 
-  if (iface->get_identifier == NULL)
-    {
-      g_value_init (identifier, G_TYPE_STRING);
-      g_value_set_string (identifier, account);
-    }
-  else
-    {
-      iface->get_identifier (storage, account, identifier);
-    }
+  iface->get_identifier (storage, account, identifier);
 }
 
 /**
@@ -926,6 +918,8 @@ mcp_account_storage_get_identifier (const McpAccountStorage *storage,
  * This method will only be called for the storage plugin that "owns"
  * the account.
  *
+ * There is a default implementation that return an empty table.
+ *
  * Returns: (transfer container) (element-type utf8 GObject.Value): additional
  *  storage-specific information
  */
@@ -934,18 +928,12 @@ mcp_account_storage_get_additional_info (const McpAccountStorage *storage,
     const gchar *account)
 {
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
-  GHashTable *ret = NULL;
 
   SDEBUG (storage, "");
   g_return_val_if_fail (iface != NULL, FALSE);
+  g_return_val_if_fail (iface->get_additional_info != NULL, FALSE);
 
-  if (iface->get_additional_info != NULL)
-    ret = iface->get_additional_info (storage, account);
-
-  if (ret == NULL)
-    ret = g_hash_table_new (g_str_hash, g_str_equal);
-
-  return ret;
+  return iface->get_additional_info (storage, account);
 }
 
 /**
@@ -966,6 +954,8 @@ mcp_account_storage_get_additional_info (const McpAccountStorage *storage,
  * This method will only be called for the storage plugin that "owns"
  * the account.
  *
+ * There is a default implementation that just return 0 (no restrictions).
+ *
  * Returns: a bitmask of %TpStorageRestrictionFlags with the restrictions to
  *  account storage.
  */
@@ -976,11 +966,9 @@ mcp_account_storage_get_restrictions (const McpAccountStorage *storage,
   McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
 
   g_return_val_if_fail (iface != NULL, 0);
+  g_return_val_if_fail (iface->get_restrictions != NULL, 0);
 
-  if (iface->get_restrictions == NULL)
-    return 0;
-  else
-    return iface->get_restrictions (storage, account);
+  return iface->get_restrictions (storage, account);
 }
 
 /**
@@ -1112,33 +1100,3 @@ mcp_account_storage_emit_reconnect (McpAccountStorage *storage,
 {
   g_signal_emit (storage, signals[RECONNECT], 0, account);
 }
-
-/**
- * mcp_account_storage_owns:
- * @storage: an #McpAccountStorage instance
- * @am: an #McpAccountManager instance
- * @account: the unique name (object-path tail) of an account
- *
- * Check whether @account is stored in @storage. The highest-priority
- * plugin for which this function returns %TRUE is considered to be
- * responsible for @account.
- *
- * There is a default implementation, which calls mcp_account_storage_get()
- * for the well-known key "manager".
- *
- * Returns: %TRUE if @account is stored in @storage
- *
- * Since: 5.15.0
- */
-gboolean
-mcp_account_storage_owns (McpAccountStorage *storage,
-    McpAccountManager *am,
-    const gchar *account)
-{
-  McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage);
-
-  g_return_val_if_fail (iface != NULL, FALSE);
-  g_return_val_if_fail (iface->owns != NULL, FALSE);
-
-  return iface->owns (storage, am, account);
-}
diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
index 5c111025..fc0cc8f3 100644
--- a/mission-control-plugins/account-storage.h
+++ b/mission-control-plugins/account-storage.h
@@ -85,9 +85,6 @@ typedef gboolean (*McpAccountStorageDeleteFunc) (
 typedef GList * (*McpAccountStorageListFunc) (
     const McpAccountStorage *storage,
     const McpAccountManager *am);
-typedef gboolean (*McpAccountStorageCommitFunc) (
-    const McpAccountStorage *storage,
-    const McpAccountManager *am);
 typedef gboolean (*McpAccountStorageCommitOneFunc) (
     const McpAccountStorage *storage,
     const McpAccountManager *am,
@@ -118,7 +115,6 @@ struct _McpAccountStorageIface
   McpAccountStorageSetFunc set;
   McpAccountStorageGetFunc get;
   McpAccountStorageDeleteFunc delete;
-  McpAccountStorageCommitFunc commit;
   McpAccountStorageListFunc list;
   McpAccountStorageReadyFunc ready;
   McpAccountStorageCommitOneFunc commit_one;
@@ -128,9 +124,6 @@ struct _McpAccountStorageIface
   McpAccountStorageCreate create;
 
   /* Since 5.15.0 */
-  gboolean (*owns) (McpAccountStorage *storage,
-      McpAccountManager *am,
-      const gchar *account);
   gboolean (*set_attribute) (McpAccountStorage *storage,
       McpAccountManager *am,
       const gchar *account,
@@ -175,10 +168,6 @@ void mcp_account_storage_ready (const McpAccountStorage *storage,
     const McpAccountManager *am);
 
 gboolean
-mcp_account_storage_commit (const McpAccountStorage *storage,
-    const McpAccountManager *am);
-
-gboolean
 mcp_account_storage_commit_one (const McpAccountStorage *storage,
     const McpAccountManager *am,
     const gchar *account);
@@ -203,10 +192,6 @@ const gchar *mcp_account_storage_name (const McpAccountStorage *storage);
 const gchar *mcp_account_storage_description (const McpAccountStorage *storage);
 const gchar *mcp_account_storage_provider (const McpAccountStorage *storage);
 
-gboolean mcp_account_storage_owns (McpAccountStorage *storage,
-    McpAccountManager *am,
-    const gchar *account);
-
 gboolean mcp_account_storage_set_attribute (McpAccountStorage *storage,
     McpAccountManager *am,
     const gchar *account,
diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c
index 806f38f9..822058a3 100644
--- a/src/mcd-account-manager.c
+++ b/src/mcd-account-manager.c
@@ -272,16 +272,9 @@ created_cb (GObject *storage_plugin_obj,
     lad->account_lock = 1; /* will be released at the end of this function */
 
     /* actually fetch the data into our cache from the plugin: */
-    if (mcd_storage_add_account_from_plugin (storage, plugin, name))
-    {
-        account = mcd_account_new (am, name, priv->minotaur);
-        lad->account = account;
-    }
-    else
-    {
-        /* that function already warned about it */
-        goto finish;
-    }
+    mcd_storage_add_account_from_plugin (storage, plugin, name);
+    account = mcd_account_new (am, name, priv->minotaur);
+    lad->account = account;
 
     if (G_UNLIKELY (!account))
     {
diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index f82cb797..353a2b0a 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -75,8 +75,30 @@ typedef struct {
     /* set of owned strings
      * e.g. { 'password': 'password' } */
     GHashTable *secrets;
+
+    /* owned storage plugin owning this account */
+    McpAccountStorage *storage;
 } McdStorageAccount;
 
+static McdStorageAccount *
+mcd_storage_account_new (McpAccountStorage *storage)
+{
+  McdStorageAccount *sa;
+
+  sa = g_slice_new (McdStorageAccount);
+  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->escaped_parameters = g_hash_table_new_full (g_str_hash, g_str_equal,
+      g_free, g_free);
+  sa->secrets = g_hash_table_new_full (g_str_hash, g_str_equal,
+      g_free, NULL);
+  sa->storage = g_object_ref (storage);
+
+  return sa;
+}
+
 static void
 mcd_storage_account_free (gpointer p)
 {
@@ -86,6 +108,7 @@ mcd_storage_account_free (gpointer p)
   g_hash_table_unref (sa->parameters);
   g_hash_table_unref (sa->escaped_parameters);
   g_hash_table_unref (sa->secrets);
+  g_object_unref (sa->storage);
   g_slice_free (McdStorageAccount, sa);
 }
 
@@ -207,29 +230,6 @@ lookup_account (McdStorage *self,
   return g_hash_table_lookup (self->accounts, account);
 }
 
-static McdStorageAccount *
-ensure_account (McdStorage *self,
-    const gchar *account)
-{
-  McdStorageAccount *sa = lookup_account (self, account);
-
-  if (sa == NULL)
-    {
-      sa = g_slice_new (McdStorageAccount);
-      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->escaped_parameters = g_hash_table_new_full (g_str_hash, g_str_equal,
-          g_free, g_free);
-      sa->secrets = g_hash_table_new_full (g_str_hash, g_str_equal,
-          g_free, NULL);
-      g_hash_table_insert (self->accounts, g_strdup (account), sa);
-    }
-
-  return sa;
-}
-
 static gchar *
 get_value (const McpAccountManager *ma,
     const gchar *account,
@@ -401,7 +401,9 @@ mcpa_set_attribute (const McpAccountManager *ma,
     McpAttributeFlags flags)
 {
   McdStorage *self = MCD_STORAGE (ma);
-  McdStorageAccount *sa = ensure_account (self, account);
+  McdStorageAccount *sa = lookup_account (self, account);
+
+  g_return_if_fail (sa != NULL);
 
   if (value != NULL)
     {
@@ -422,7 +424,9 @@ mcpa_set_parameter (const McpAccountManager *ma,
     McpParameterFlags flags)
 {
   McdStorage *self = MCD_STORAGE (ma);
-  McdStorageAccount *sa = ensure_account (self, account);
+  McdStorageAccount *sa = lookup_account (self, account);
+
+  g_return_if_fail (sa != NULL);
 
   g_hash_table_remove (sa->parameters, parameter);
   g_hash_table_remove (sa->escaped_parameters, parameter);
@@ -445,7 +449,9 @@ set_value (const McpAccountManager *ma,
     const gchar *value)
 {
   McdStorage *self = MCD_STORAGE (ma);
-  McdStorageAccount *sa = ensure_account (self, account);
+  McdStorageAccount *sa = lookup_account (self, account);
+
+  g_return_if_fail (sa != NULL);
 
   if (g_str_has_prefix (key, "param-"))
     {
@@ -547,8 +553,10 @@ mcd_storage_make_secret (McdStorage *self,
   if (!g_str_has_prefix (key, "param-"))
     return;
 
+  sa = lookup_account (self, account);
+  g_return_if_fail (sa != NULL);
+
   DEBUG ("flagging %s parameter %s as secret", account, key + 6);
-  sa = ensure_account (self, account);
   g_hash_table_add (sa->secrets, g_strdup (key + 6));
 }
 
@@ -874,22 +882,15 @@ McpAccountStorage *
 mcd_storage_get_plugin (McdStorage *self,
     const gchar *account)
 {
-  GList *store = stores;
-  McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
-  McpAccountStorage *owner = NULL;
+  McdStorageAccount *sa;
 
   g_return_val_if_fail (MCD_IS_STORAGE (self), NULL);
   g_return_val_if_fail (account != NULL, NULL);
 
-  for (; store != NULL && owner == NULL; store = g_list_next (store))
-    {
-      McpAccountStorage *plugin = store->data;
-
-      if (mcp_account_storage_owns (plugin, ma, account))
-        owner = plugin;
-    }
+  sa = lookup_account (self, account);
+  g_return_val_if_fail (sa != NULL, NULL);
 
-  return owner;
+  return sa->storage;
 }
 
 /*
@@ -1550,50 +1551,42 @@ update_storage (McdStorage *self,
     const gchar *escaped,
     gboolean secret)
 {
-  GList *store;
-  gboolean done = FALSE;
-  gboolean parameter = g_str_has_prefix (key, "param-");
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
+  gboolean parameter = g_str_has_prefix (key, "param-");
+  McdStorageAccount *sa;
+  const gchar *pn;
 
   if (secret)
     mcd_storage_make_secret (self, account, key);
 
-  /* we're deleting, which is unconditional, no need to check if anyone *
-   * claims this setting for themselves                                 */
-  if (escaped == NULL)
-    done = TRUE;
+  sa = lookup_account (self, account);
+  g_return_if_fail (sa != NULL);
 
-  for (store = stores; store != NULL; store = g_list_next (store))
+  pn = mcp_account_storage_name (sa->storage);
+  if (escaped == NULL)
     {
-      McpAccountStorage *plugin = store->data;
-      const gchar *pn = mcp_account_storage_name (plugin);
+      DEBUG ("MCP:%s -> delete %s.%s", pn, account, key);
+      mcp_account_storage_delete (sa->storage, ma, account, key);
+    }
+  else if (variant != NULL && !parameter &&
+      mcp_account_storage_set_attribute (sa->storage, ma, account, key, variant,
+          MCP_ATTRIBUTE_FLAG_NONE))
+    {
+      DEBUG ("MCP:%s -> store attribute %s.%s", pn, account, key);
+    }
+  else if (variant != NULL && parameter &&
+      mcp_account_storage_set_parameter (sa->storage, ma, account, key + 6,
+          variant,
+          secret ? MCP_PARAMETER_FLAG_SECRET : MCP_PARAMETER_FLAG_NONE))
+    {
+      DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key);
+    }
+  else
+    {
+      gboolean done;
 
-      if (done)
-        {
-          DEBUG ("MCP:%s -> delete %s.%s", pn, account, key);
-          mcp_account_storage_delete (plugin, ma, account, key);
-        }
-      else if (variant != NULL && !parameter &&
-          mcp_account_storage_set_attribute (plugin, ma, account, key, variant,
-            MCP_ATTRIBUTE_FLAG_NONE))
-        {
-          done = TRUE;
-          DEBUG ("MCP:%s -> store attribute %s.%s", pn, account, key);
-        }
-      else if (variant != NULL && parameter &&
-          mcp_account_storage_set_parameter (plugin, ma, account, key + 6,
-            variant,
-            secret ? MCP_PARAMETER_FLAG_SECRET : MCP_PARAMETER_FLAG_NONE))
-        {
-          done = TRUE;
-          DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key);
-        }
-      else
-        {
-          done = mcp_account_storage_set (plugin, ma, account, key, escaped);
-          DEBUG ("MCP:%s -> %s %s.%s",
-              pn, done ? "store" : "ignore", account, key);
-        }
+      done = mcp_account_storage_set (sa->storage, ma, account, key, escaped);
+      DEBUG ("MCP:%s -> %s %s.%s", pn, done ? "store" : "ignore", account, key);
     }
 }
 
@@ -1675,7 +1668,8 @@ mcd_storage_set_attribute (McdStorage *self,
   g_return_val_if_fail (attribute != NULL, FALSE);
   g_return_val_if_fail (!g_str_has_prefix (attribute, "param-"), FALSE);
 
-  sa = ensure_account (self, account);
+  sa = lookup_account (self, account);
+  g_return_val_if_fail (sa != NULL, FALSE);
 
   if (value != NULL)
     new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value));
@@ -1745,7 +1739,8 @@ mcd_storage_set_parameter (McdStorage *self,
   g_return_val_if_fail (account != NULL, FALSE);
   g_return_val_if_fail (parameter != NULL, FALSE);
 
-  sa = ensure_account (self, account);
+  sa = lookup_account (self, account);
+  g_return_val_if_fail (sa != NULL, FALSE);
 
   if (value != NULL)
     {
@@ -2064,8 +2059,9 @@ mcd_storage_create_account (McdStorage *self,
     const gchar *identification,
     GError **error)
 {
-  GList *store;
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
+  GList *store;
+  gchar *account;
 
   g_return_val_if_fail (MCD_IS_STORAGE (self), NULL);
   g_return_val_if_fail (!tp_str_empty (manager), NULL);
@@ -2080,8 +2076,10 @@ mcd_storage_create_account (McdStorage *self,
 
           if (!tp_strdiff (mcp_account_storage_provider (plugin), provider))
             {
-              return mcp_account_storage_create (plugin, ma, manager,
+              account = mcp_account_storage_create (plugin, ma, manager,
                   protocol, identification, error);
+              mcd_storage_add_account_from_plugin (self, plugin, account);
+              return account;
             }
         }
 
@@ -2093,50 +2091,19 @@ mcd_storage_create_account (McdStorage *self,
 
   /* No provider specified, let's pick the first plugin able to create this
    * account in priority order.
-   *
-   * FIXME: This is rather subtle, and relies on the fact that accounts
-   * aren't always strongly tied to a single plugin.
-   *
-   * For plugins that only store their accounts set up specifically
-   * through them (like the libaccounts/SSO pseudo-plugin,
-   * McdAccountManagerSSO), create() will fail as unimplemented,
-   * and we'll fall through to the next plugin. Eventually we'll
-   * reach the default keyfile+gnome-keyring plugin, or another
-   * plugin that accepts arbitrary accounts. When set() is called,
-   * the libaccounts/SSO plugin will reject that too, and again,
-   * we'll fall through to a plugin that accepts arbitrary
-   * accounts.
-   *
-   * Plugins that will accept arbitrary accounts being created
-   * via D-Bus (like the default keyfile+gnome-keyring plugin,
-   * and the account-diversion plugin in tests/twisted)
-   * should, in principle, implement create() to be successful.
-   * If they do, their create() will succeed, and later, so will
-   * their set().
-   *
-   * We can't necessarily rely on all such plugins implementing
-   * create(), because it isn't a mandatory part of the plugin
-   * API (it was added later). However, as it happens, the
-   * default plugin returns successfully from create() without
-   * really doing anything. When we iterate through the accounts again
-   * to call set(), higher-priority plugins are given a second
-   * chance to intercept that; so we end up with create() in
-   * the default plugin being followed by set() from the
-   * higher-priority plugin. In theory that's bad because it
-   * splits the account across two plugins, but in practice
-   * it isn't a problem because the default plugin's create()
-   * doesn't really do anything anyway.
    */
   for (store = stores; store != NULL; store = g_list_next (store))
     {
       McpAccountStorage *plugin = store->data;
-      gchar *ret;
 
-      ret = mcp_account_storage_create (plugin, ma, manager, protocol,
+      account = mcp_account_storage_create (plugin, ma, manager, protocol,
           identification, error);
 
-      if (ret != NULL)
-        return ret;
+      if (account != NULL)
+        {
+          mcd_storage_add_account_from_plugin (self, plugin, account);
+          return account;
+        }
 
       g_clear_error (error);
     }
@@ -2165,20 +2132,17 @@ void
 mcd_storage_delete_account (McdStorage *self,
     const gchar *account)
 {
-  GList *store;
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
+  McdStorageAccount *sa;
 
   g_return_if_fail (MCD_IS_STORAGE (self));
   g_return_if_fail (account != NULL);
 
-  g_hash_table_remove (self->accounts, account);
-
-  for (store = stores; store != NULL; store = g_list_next (store))
-    {
-      McpAccountStorage *plugin = store->data;
+  sa = lookup_account (self, account);
+  g_return_if_fail (sa != NULL);
 
-      mcp_account_storage_delete (plugin, ma, account, NULL);
-    }
+  mcp_account_storage_delete (sa->storage, ma, account, NULL);
+  g_hash_table_remove (self->accounts, account);
 }
 
 /*
@@ -2192,26 +2156,30 @@ mcd_storage_delete_account (McdStorage *self,
 void
 mcd_storage_commit (McdStorage *self, const gchar *account)
 {
-  GList *store;
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
+  GList *store;
 
   g_return_if_fail (MCD_IS_STORAGE (self));
 
+  if (account != NULL)
+    {
+      McdStorageAccount *sa = lookup_account (self, account);
+
+      g_return_if_fail (sa != NULL);
+
+      DEBUG ("flushing plugin %s %s to long term storage",
+          mcp_account_storage_name (sa->storage), account);
+      mcp_account_storage_commit_one (sa->storage, ma, account);
+      return;
+    }
+
   for (store = stores; store != NULL; store = g_list_next (store))
     {
       McpAccountStorage *plugin = store->data;
-      const gchar *pname = mcp_account_storage_name (plugin);
 
-      if (account != NULL)
-        {
-          DEBUG ("flushing plugin %s %s to long term storage", pname, account);
-          mcp_account_storage_commit_one (plugin, ma, account);
-        }
-      else
-        {
-          DEBUG ("flushing plugin %s to long term storage", pname);
-          mcp_account_storage_commit (plugin, ma);
-        }
+      DEBUG ("flushing plugin %s to long term storage",
+          mcp_account_storage_name (plugin));
+      mcp_account_storage_commit_one (plugin, ma, NULL);
     }
 }
 
@@ -2291,18 +2259,19 @@ plugin_iface_init (McpAccountManagerIface *iface,
   iface->init_value_for_attribute = mcpa_init_value_for_attribute;
 }
 
-gboolean
+void
 mcd_storage_add_account_from_plugin (McdStorage *self,
     McpAccountStorage *plugin,
     const gchar *account)
 {
-  if (!mcp_account_storage_get (plugin, MCP_ACCOUNT_MANAGER (self),
-      account, NULL))
-    {
-      g_warning ("plugin %s disowned account %s",
-                 mcp_account_storage_name (plugin), account);
-      return FALSE;
-    }
+  g_return_if_fail (MCD_IS_STORAGE (self));
+  g_return_if_fail (MCP_IS_ACCOUNT_STORAGE (plugin));
+  g_return_if_fail (account != NULL);
+  g_return_if_fail (!g_hash_table_contains (self->accounts, account));
 
-  return TRUE;
+  g_hash_table_insert (self->accounts, g_strdup (account),
+      mcd_storage_account_new (plugin));
+
+  /* This will fill our parameters/attributes tables */
+  mcp_account_storage_get (plugin, MCP_ACCOUNT_MANAGER (self), account, NULL);
 }
diff --git a/src/mcd-storage.h b/src/mcd-storage.h
index e4408451..dc2435ff 100644
--- a/src/mcd-storage.h
+++ b/src/mcd-storage.h
@@ -131,7 +131,7 @@ McpAccountStorage * mcd_storage_get_plugin (McdStorage *storage,
 
 G_GNUC_INTERNAL void _mcd_storage_store_connections (McdStorage *storage);
 
-gboolean mcd_storage_add_account_from_plugin (McdStorage *storage,
+void mcd_storage_add_account_from_plugin (McdStorage *storage,
     McpAccountStorage *plugin,
     const gchar *account);
 
diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 3cf29ca1..063b886d 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -1370,9 +1370,8 @@ test_dbus_account_plugin_commit_one (const McpAccountStorage *storage,
 
   DEBUG ("%s", account_name);
 
-  /* MC does not call @commit_one with parameter %NULL (meaning "all accounts")
-   * if we also implement commit(), which, as it happens, we do */
-  g_return_val_if_fail (account_name != NULL, FALSE);
+  if (account_name == NULL)
+    return test_dbus_account_plugin_commit (storage, am);
 
   if (!self->active || account == NULL)
     return FALSE;
@@ -1580,22 +1579,6 @@ test_dbus_account_plugin_get_restrictions (const McpAccountStorage *storage,
   return account->restrictions;
 }
 
-static gboolean
-test_dbus_account_plugin_owns (McpAccountStorage *storage,
-    McpAccountManager *am,
-    const gchar *account_name)
-{
-  TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
-  Account *account = lookup_account (self, account_name);
-
-  DEBUG ("%s", account_name);
-
-  if (!self->active || account == NULL || (account->flags & UNCOMMITTED_DELETION))
-    return FALSE;
-
-  return TRUE;
-}
-
 static void
 account_storage_iface_init (McpAccountStorageIface *iface)
 {
@@ -1611,11 +1594,9 @@ account_storage_iface_init (McpAccountStorageIface *iface)
   iface->list = test_dbus_account_plugin_list;
   iface->ready = test_dbus_account_plugin_ready;
   iface->delete = test_dbus_account_plugin_delete;
-  iface->commit = test_dbus_account_plugin_commit;
   iface->commit_one = test_dbus_account_plugin_commit_one;
   iface->get_identifier = test_dbus_account_plugin_get_identifier;
   iface->get_additional_info = test_dbus_account_plugin_get_additional_info;
   iface->get_restrictions = test_dbus_account_plugin_get_restrictions;
   iface->create = test_dbus_account_plugin_create;
-  iface->owns = test_dbus_account_plugin_owns;
 }
diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c
index 923f51b4..9e8bb867 100644
--- a/tests/twisted/mcp-account-diversion.c
+++ b/tests/twisted/mcp-account-diversion.c
@@ -206,8 +206,9 @@ _delete (const McpAccountStorage *self,
 
 
 static gboolean
-_commit (const McpAccountStorage *self,
-    const McpAccountManager *am)
+_commit_one (const McpAccountStorage *self,
+    const McpAccountManager *am,
+    const gchar *account_name)
 {
   gsize n;
   gchar *data;
@@ -266,7 +267,7 @@ account_storage_iface_init (McpAccountStorageIface *iface,
   iface->get = _get;
   iface->set = _set;
   iface->delete = _delete;
-  iface->commit = _commit;
+  iface->commit_one = _commit_one;
   iface->list = _list;
 }