McpAccountStorage: merge commit and commit_one into one function

Based on part of a patch by Xavier Claessens.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71384
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

6 files changed, 24 insertions, 89 deletions

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index 14f4f6d5..8f5609ce 100644
--- a/mission-control-plugins/account-storage.c
+++ b/mission-control-plugins/account-storage.c
@@ -60,7 +60,6 @@
  *   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;
  *   iface->get_identifier = foo_plugin_get_identifier;
@@ -316,7 +315,6 @@ mcp_account_storage_get_type (void)
  * @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()
  * @get_identifier: implementation of mcp_account_storage_get_identifier()
  * @get_additional_info: implementation of
  *  mcp_account_storage_get_additional_info()
@@ -453,7 +451,7 @@ mcp_account_storage_get (const McpAccountStorage *storage,
  *
  * 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()
+ * mcp_account_storage_commit() with either @account or %NULL
  * after a short delay.
  *
  * Plugins that implement mcp_storage_set_attribute() and
@@ -686,6 +684,8 @@ mcp_account_storage_delete (const McpAccountStorage *storage,
  * McpAccountStorageCommitFunc:
  * @storage: an #McpAccountStorage instance
  * @am: an #McpAccountManager instance
+ * @account: (allow-none): the unique suffix of an account's object path,
+ *  or %NULL to commit all accounts
  *
  * An implementation of mcp_account_storage_commit().
  *
@@ -696,6 +696,8 @@ mcp_account_storage_delete (const McpAccountStorage *storage,
  * mcp_account_storage_commit:
  * @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.
@@ -704,74 +706,12 @@ 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.
- *
  * 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.
- *
- * 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_one (const McpAccountStorage *storage,
     const McpAccountManager *am,
     const gchar *account)
 {
@@ -780,11 +720,10 @@ 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);
 
-  if (iface->commit_one != NULL)
-    return iface->commit_one (storage, am, account);
+  if (iface->commit != NULL)
+    return iface->commit (storage, am, account);
   else
-    /* Fall back to plain ->commit() */
-    return mcp_account_storage_commit (storage, am);
+    return FALSE;
 }
 
 /**
diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h
index 5c111025..ecc4e267 100644
--- a/mission-control-plugins/account-storage.h
+++ b/mission-control-plugins/account-storage.h
@@ -87,9 +87,6 @@ typedef GList * (*McpAccountStorageListFunc) (
     const McpAccountManager *am);
 typedef gboolean (*McpAccountStorageCommitFunc) (
     const McpAccountStorage *storage,
-    const McpAccountManager *am);
-typedef gboolean (*McpAccountStorageCommitOneFunc) (
-    const McpAccountStorage *storage,
     const McpAccountManager *am,
     const gchar *account);
 typedef void (*McpAccountStorageReadyFunc) (
@@ -121,7 +118,6 @@ struct _McpAccountStorageIface
   McpAccountStorageCommitFunc commit;
   McpAccountStorageListFunc list;
   McpAccountStorageReadyFunc ready;
-  McpAccountStorageCommitOneFunc commit_one;
   McpAccountStorageGetIdentifierFunc get_identifier;
   McpAccountStorageGetAdditionalInfoFunc get_additional_info;
   McpAccountStorageGetRestrictionsFunc get_restrictions;
@@ -176,10 +172,6 @@ void mcp_account_storage_ready (const McpAccountStorage *storage,
 
 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);
 
diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c
index 85b5e4e5..0a19e48f 100644
--- a/src/mcd-account-manager-default.c
+++ b/src/mcd-account-manager-default.c
@@ -1015,7 +1015,7 @@ account_storage_iface_init (McpAccountStorageIface *iface,
   iface->set_parameter = set_parameter;
   iface->create = _create;
   iface->delete = _delete;
-  iface->commit_one = _commit;
+  iface->commit = _commit;
   iface->list = _list;
 
 }
diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index 6cc603b4..59322175 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -2144,12 +2144,12 @@ mcd_storage_commit (McdStorage *self, const gchar *account)
       if (account != NULL)
         {
           DEBUG ("flushing plugin %s %s to long term storage", pname, account);
-          mcp_account_storage_commit_one (plugin, ma, account);
+          mcp_account_storage_commit (plugin, ma, account);
         }
       else
         {
           DEBUG ("flushing plugin %s to long term storage", pname);
-          mcp_account_storage_commit (plugin, ma);
+          mcp_account_storage_commit (plugin, ma, NULL);
         }
     }
 }
diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index c81bce48..0e94b9b3 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -1174,7 +1174,7 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
 }
 
 static gboolean
-test_dbus_account_plugin_commit (const McpAccountStorage *storage,
+test_dbus_account_plugin_commit_all (const McpAccountStorage *storage,
     const McpAccountManager *am)
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
@@ -1194,7 +1194,7 @@ test_dbus_account_plugin_commit (const McpAccountStorage *storage,
 
   while (g_hash_table_iter_next (&iter, &k, NULL))
     {
-      if (!mcp_account_storage_commit_one (storage, am, k))
+      if (!mcp_account_storage_commit (storage, am, k))
         {
           g_warning ("declined to commit account %s", (const gchar *) k);
         }
@@ -1334,12 +1334,12 @@ update_parameters_cb (GObject *source_object,
 }
 
 static gboolean
-test_dbus_account_plugin_commit_one (const McpAccountStorage *storage,
+test_dbus_account_plugin_commit (const McpAccountStorage *storage,
     const McpAccountManager *am,
     const gchar *account_name)
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
-  Account *account = lookup_account (self, account_name);
+  Account *account;
   GHashTableIter iter;
   gpointer k;
   GVariantBuilder a_sv_builder;
@@ -1349,9 +1349,10 @@ 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_all (storage, am);
+
+  account = lookup_account (self, account_name);
 
   if (!self->active || account == NULL)
     return FALSE;
@@ -1591,7 +1592,6 @@ account_storage_iface_init (McpAccountStorageIface *iface)
   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;
diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c
index 923f51b4..e36b1ac5 100644
--- a/tests/twisted/mcp-account-diversion.c
+++ b/tests/twisted/mcp-account-diversion.c
@@ -207,13 +207,17 @@ _delete (const McpAccountStorage *self,
 
 static gboolean
 _commit (const McpAccountStorage *self,
-    const McpAccountManager *am)
+    const McpAccountManager *am,
+    const gchar *account_name G_GNUC_UNUSED)
 {
   gsize n;
   gchar *data;
   AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self);
   gboolean rval = FALSE;
 
+  /* This simple implementation ignores account_name and commits everything:
+   * we're writing out the whole keyfile anyway */
+
   if (!adp->save)
     return TRUE;