diff options
| -rw-r--r-- | mission-control-plugins/account-storage.c | 254 | ||||
| -rw-r--r-- | mission-control-plugins/account-storage.h | 15 | ||||
| -rw-r--r-- | src/mcd-account-manager.c | 13 | ||||
| -rw-r--r-- | src/mcd-storage.c | 257 | ||||
| -rw-r--r-- | src/mcd-storage.h | 2 | ||||
| -rw-r--r-- | tests/twisted/dbus-account-plugin.c | 23 | ||||
| -rw-r--r-- | tests/twisted/mcp-account-diversion.c | 7 |
7 files changed, 342 insertions, 229 deletions
diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c index 28ca4cfa..b51c1264 100644 --- a/mission-control-plugins/account-storage.c +++ b/mission-control-plugins/account-storage.c @@ -59,6 +59,7 @@ * 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; @@ -66,6 +67,7 @@ * 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; * } @@ -142,63 +144,17 @@ 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_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) -{ - 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, +default_owns (McpAccountStorage *storage, + McpAccountManager *am, const gchar *account) { - return 0; + /* 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"); } static void @@ -208,16 +164,10 @@ 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) { @@ -363,6 +313,7 @@ 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() @@ -502,8 +453,9 @@ 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 - * mcp_account_storage_commit_one() after a short delay. + * 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. * * Plugins that implement mcp_storage_set_attribute() and * mcp_account_storage_set_parameter() can just return %FALSE here. @@ -522,7 +474,6 @@ 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); } @@ -642,7 +593,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_one() will be called later. + * and mcp_account_storage_commit() 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 @@ -650,10 +601,7 @@ 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_one() will be called. - * - * There is a default implementation, which just returns %NULL and raise an - * error. + * not even when mcp_account_storage_commit() will be called. * * 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 @@ -669,9 +617,14 @@ 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); - g_return_val_if_fail (iface->create != 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; + } return iface->create (storage, am, manager, protocol, identification, error); } @@ -711,9 +664,6 @@ 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 @@ -729,29 +679,24 @@ 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); } /** - * McpAccountStorageCommitOneFunc: + * McpAccountStorageCommitFunc: * @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(). + * An implementation of mcp_account_storage_commit(). * * Returns: %TRUE if the commit process was started successfully */ /** - * mcp_account_storage_commit_one: + * 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. @@ -760,11 +705,67 @@ 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 @account = %NULL it means that it should commit all accounts owned by the - * storage plugin. + * 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. * - * 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 + * 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 @@ -779,9 +780,12 @@ 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); - return iface->commit_one (storage, am, account); + 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); } /** @@ -818,7 +822,6 @@ 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); } @@ -839,9 +842,6 @@ 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,9 +850,12 @@ 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); - iface->ready (storage, am); + /* 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); } /** @@ -877,8 +880,6 @@ 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, @@ -889,11 +890,18 @@ 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)); - iface->get_identifier (storage, account, 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); + } } /** @@ -918,8 +926,6 @@ 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 */ @@ -928,12 +934,18 @@ 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); - return iface->get_additional_info (storage, account); + 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; } /** @@ -954,8 +966,6 @@ 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. */ @@ -966,9 +976,11 @@ 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); - return iface->get_restrictions (storage, account); + if (iface->get_restrictions == NULL) + return 0; + else + return iface->get_restrictions (storage, account); } /** @@ -1100,3 +1112,33 @@ 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 fc0cc8f3..5c111025 100644 --- a/mission-control-plugins/account-storage.h +++ b/mission-control-plugins/account-storage.h @@ -85,6 +85,9 @@ 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, @@ -115,6 +118,7 @@ struct _McpAccountStorageIface McpAccountStorageSetFunc set; McpAccountStorageGetFunc get; McpAccountStorageDeleteFunc delete; + McpAccountStorageCommitFunc commit; McpAccountStorageListFunc list; McpAccountStorageReadyFunc ready; McpAccountStorageCommitOneFunc commit_one; @@ -124,6 +128,9 @@ 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, @@ -168,6 +175,10 @@ 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); @@ -192,6 +203,10 @@ 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 822058a3..806f38f9 100644 --- a/src/mcd-account-manager.c +++ b/src/mcd-account-manager.c @@ -272,9 +272,16 @@ 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: */ - mcd_storage_add_account_from_plugin (storage, plugin, name); - account = mcd_account_new (am, name, priv->minotaur); - lad->account = account; + 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; + } if (G_UNLIKELY (!account)) { diff --git a/src/mcd-storage.c b/src/mcd-storage.c index 353a2b0a..f82cb797 100644 --- a/src/mcd-storage.c +++ b/src/mcd-storage.c @@ -75,30 +75,8 @@ 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) { @@ -108,7 +86,6 @@ 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); } @@ -230,6 +207,29 @@ 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,9 +401,7 @@ mcpa_set_attribute (const McpAccountManager *ma, McpAttributeFlags flags) { McdStorage *self = MCD_STORAGE (ma); - McdStorageAccount *sa = lookup_account (self, account); - - g_return_if_fail (sa != NULL); + McdStorageAccount *sa = ensure_account (self, account); if (value != NULL) { @@ -424,9 +422,7 @@ mcpa_set_parameter (const McpAccountManager *ma, McpParameterFlags flags) { McdStorage *self = MCD_STORAGE (ma); - McdStorageAccount *sa = lookup_account (self, account); - - g_return_if_fail (sa != NULL); + McdStorageAccount *sa = ensure_account (self, account); g_hash_table_remove (sa->parameters, parameter); g_hash_table_remove (sa->escaped_parameters, parameter); @@ -449,9 +445,7 @@ set_value (const McpAccountManager *ma, const gchar *value) { McdStorage *self = MCD_STORAGE (ma); - McdStorageAccount *sa = lookup_account (self, account); - - g_return_if_fail (sa != NULL); + McdStorageAccount *sa = ensure_account (self, account); if (g_str_has_prefix (key, "param-")) { @@ -553,10 +547,8 @@ 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)); } @@ -882,15 +874,22 @@ McpAccountStorage * mcd_storage_get_plugin (McdStorage *self, const gchar *account) { - McdStorageAccount *sa; + GList *store = stores; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); + McpAccountStorage *owner = NULL; g_return_val_if_fail (MCD_IS_STORAGE (self), NULL); g_return_val_if_fail (account != NULL, NULL); - sa = lookup_account (self, account); - g_return_val_if_fail (sa != 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; + } - return sa->storage; + return owner; } /* @@ -1551,42 +1550,50 @@ update_storage (McdStorage *self, const gchar *escaped, gboolean secret) { - McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); + GList *store; + gboolean done = FALSE; gboolean parameter = g_str_has_prefix (key, "param-"); - McdStorageAccount *sa; - const gchar *pn; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); if (secret) mcd_storage_make_secret (self, account, key); - sa = lookup_account (self, account); - g_return_if_fail (sa != NULL); - - pn = mcp_account_storage_name (sa->storage); + /* we're deleting, which is unconditional, no need to check if anyone * + * claims this setting for themselves */ if (escaped == NULL) + done = TRUE; + + for (store = stores; store != NULL; store = g_list_next (store)) { - 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; + McpAccountStorage *plugin = store->data; + const gchar *pn = mcp_account_storage_name (plugin); - done = mcp_account_storage_set (sa->storage, ma, account, key, escaped); - DEBUG ("MCP:%s -> %s %s.%s", pn, done ? "store" : "ignore", account, key); + 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); + } } } @@ -1668,8 +1675,7 @@ 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 = lookup_account (self, account); - g_return_val_if_fail (sa != NULL, FALSE); + sa = ensure_account (self, account); if (value != NULL) new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value)); @@ -1739,8 +1745,7 @@ mcd_storage_set_parameter (McdStorage *self, g_return_val_if_fail (account != NULL, FALSE); g_return_val_if_fail (parameter != NULL, FALSE); - sa = lookup_account (self, account); - g_return_val_if_fail (sa != NULL, FALSE); + sa = ensure_account (self, account); if (value != NULL) { @@ -2059,9 +2064,8 @@ mcd_storage_create_account (McdStorage *self, const gchar *identification, GError **error) { - McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); GList *store; - gchar *account; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); g_return_val_if_fail (MCD_IS_STORAGE (self), NULL); g_return_val_if_fail (!tp_str_empty (manager), NULL); @@ -2076,10 +2080,8 @@ mcd_storage_create_account (McdStorage *self, if (!tp_strdiff (mcp_account_storage_provider (plugin), provider)) { - account = mcp_account_storage_create (plugin, ma, manager, + return mcp_account_storage_create (plugin, ma, manager, protocol, identification, error); - mcd_storage_add_account_from_plugin (self, plugin, account); - return account; } } @@ -2091,19 +2093,50 @@ 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; - account = mcp_account_storage_create (plugin, ma, manager, protocol, + ret = mcp_account_storage_create (plugin, ma, manager, protocol, identification, error); - if (account != NULL) - { - mcd_storage_add_account_from_plugin (self, plugin, account); - return account; - } + if (ret != NULL) + return ret; g_clear_error (error); } @@ -2132,17 +2165,20 @@ 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); - sa = lookup_account (self, account); - g_return_if_fail (sa != NULL); - - mcp_account_storage_delete (sa->storage, ma, account, NULL); g_hash_table_remove (self->accounts, account); + + for (store = stores; store != NULL; store = g_list_next (store)) + { + McpAccountStorage *plugin = store->data; + + mcp_account_storage_delete (plugin, ma, account, NULL); + } } /* @@ -2156,30 +2192,26 @@ mcd_storage_delete_account (McdStorage *self, void mcd_storage_commit (McdStorage *self, const gchar *account) { - McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); GList *store; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); 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); - DEBUG ("flushing plugin %s to long term storage", - mcp_account_storage_name (plugin)); - mcp_account_storage_commit_one (plugin, ma, NULL); + 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); + } } } @@ -2259,19 +2291,18 @@ plugin_iface_init (McpAccountManagerIface *iface, iface->init_value_for_attribute = mcpa_init_value_for_attribute; } -void +gboolean mcd_storage_add_account_from_plugin (McdStorage *self, McpAccountStorage *plugin, const gchar *account) { - 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)); - - g_hash_table_insert (self->accounts, g_strdup (account), - mcd_storage_account_new (plugin)); + 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; + } - /* This will fill our parameters/attributes tables */ - mcp_account_storage_get (plugin, MCP_ACCOUNT_MANAGER (self), account, NULL); + return TRUE; } diff --git a/src/mcd-storage.h b/src/mcd-storage.h index dc2435ff..e4408451 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); -void mcd_storage_add_account_from_plugin (McdStorage *storage, +gboolean 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 063b886d..3cf29ca1 100644 --- a/tests/twisted/dbus-account-plugin.c +++ b/tests/twisted/dbus-account-plugin.c @@ -1370,8 +1370,9 @@ test_dbus_account_plugin_commit_one (const McpAccountStorage *storage, DEBUG ("%s", account_name); - if (account_name == NULL) - return test_dbus_account_plugin_commit (storage, am); + /* 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 (!self->active || account == NULL) return FALSE; @@ -1579,6 +1580,22 @@ 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) { @@ -1594,9 +1611,11 @@ 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 9e8bb867..923f51b4 100644 --- a/tests/twisted/mcp-account-diversion.c +++ b/tests/twisted/mcp-account-diversion.c @@ -206,9 +206,8 @@ _delete (const McpAccountStorage *self, static gboolean -_commit_one (const McpAccountStorage *self, - const McpAccountManager *am, - const gchar *account_name) +_commit (const McpAccountStorage *self, + const McpAccountManager *am) { gsize n; gchar *data; @@ -267,7 +266,7 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->get = _get; iface->set = _set; iface->delete = _delete; - iface->commit_one = _commit_one; + iface->commit = _commit; iface->list = _list; } |