From 1ef582528c9f87652ac15faffd4879277fa9385d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 12 Nov 2013 15:51:44 +0000 Subject: [PATCH 02/26] 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 --- mission-control-plugins/account-storage.c | 77 ++++--------------------------- mission-control-plugins/account-storage.h | 8 ---- src/mcd-account-manager-default.c | 2 +- src/mcd-storage.c | 4 +- tests/twisted/dbus-account-plugin.c | 16 +++---- tests/twisted/mcp-account-diversion.c | 6 ++- 6 files changed, 24 insertions(+), 89 deletions(-) diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c index 14f4f6d..8f5609c 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 : ""); 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 5c11102..ecc4e26 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 202e8c4..515cdbd 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 6cc603b..5932217 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 c81bce4..0e94b9b 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 923f51b..e36b1ac 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; -- 1.8.4.3