From 4ca8308108347e9009dd728ed44e31cf18904d2c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 14 Nov 2013 16:20:21 +0000 Subject: [PATCH 23/26] mcp_account_storage_get: replace with get_attribute, get_parameter The old API in which plugins poked new values into the McdStorage was non-obvious, and also fundamentally incompatible with the idea that each account is owned by at most one plugin: if an account in a high-priority plugin is masked by one in a low-priority plugin, the McdStorage can't prevent the low-priority plugin from changing its effective attribute and parameter values. --- mission-control-plugins/account-storage.c | 149 +++++++++++------ mission-control-plugins/account-storage.h | 36 ++-- mission-control-plugins/account.c | 94 ----------- mission-control-plugins/account.h | 17 -- mission-control-plugins/implementation.h | 17 -- src/mcd-account-manager-default.c | 130 +++++---------- src/mcd-storage.c | 263 +++++------------------------- src/mcd-storage.h | 2 +- tests/twisted/dbus-account-plugin.c | 163 +++++++----------- tests/twisted/mcp-account-diversion.c | 71 ++++---- 10 files changed, 300 insertions(+), 642 deletions(-) diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c index ca9c7fa..6ce8c72 100644 --- a/mission-control-plugins/account-storage.c +++ b/mission-control-plugins/account-storage.c @@ -56,7 +56,6 @@ * iface->desc = "The FOO storage backend"; * iface->provider = "org.freedesktop.Telepathy.MissionControl5.FooStorage"; * - * iface->get = foo_plugin_get; * iface->delete = foo_plugin_delete; * iface->commit = foo_plugin_commit; * iface->list = foo_plugin_list; @@ -65,6 +64,8 @@ * iface->get_additional_info = foo_plugin_get_additional_info; * iface->get_restrictions = foo_plugin_get_restrictions; * iface->create = foo_plugin_create; + * iface->get_attribute = foo_plugin_get_attribute; + * iface->get_parameter = foo_plugin_get_parameter; * iface->set_attribute = foo_plugin_set_attribute; * iface->set_parameter = foo_plugin_set_parameter; * } @@ -109,15 +110,6 @@ enum static guint signals[NO_SIGNAL] = { 0 }; -static gboolean -default_get (const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key) -{ - return FALSE; -} - static void default_delete_async (McpAccountStorage *storage, McpAccountManager *am, @@ -196,6 +188,28 @@ default_get_restrictions (const McpAccountStorage *storage, return 0; } +static GVariant * +default_get_attribute (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags) +{ + return NULL; +} + +static GVariant * +default_get_parameter (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags) +{ + return NULL; +} + static McpAccountStorageSetResult default_set_attribute (McpAccountStorage *storage, McpAccountManager *am, @@ -225,7 +239,6 @@ class_init (gpointer klass, GType type = G_TYPE_FROM_CLASS (klass); McpAccountStorageIface *iface = klass; - iface->get = default_get; iface->create = default_create; iface->delete_async = default_delete_async; iface->delete_finish = default_delete_finish; @@ -235,6 +248,8 @@ class_init (gpointer klass, iface->get_identifier = default_get_identifier; iface->get_additional_info = default_get_additional_info; iface->get_restrictions = default_get_restrictions; + iface->get_attribute = default_get_attribute; + iface->get_parameter = default_get_parameter; iface->set_attribute = default_set_attribute; iface->set_parameter = default_set_parameter; @@ -262,20 +277,20 @@ class_init (gpointer klass, /** * McpAccountStorage::altered-one * @account: the unique name of the altered account - * @name: the name of the altered property (its key) + * @name: either an attribute name such as DisplayName, + * or "param-" plus a parameter name, e.g. "param-require-encryption" * * Emitted if an external entity alters an account * in the backend that the emitting plugin handles. * - * Before emitting this signal, the plugin must call - * either mcp_account_manager_set_attribute(), - * either mcp_account_manager_set_parameter() or - * mcp_account_manager_set_value() to push the new value - * into the account manager. + * Before emitting this signal, the plugin must update its + * internal cache (if any) so that mcp_account_storage_get_attribute() + * or mcp_account_storage_get_parameter() will return the new value + * when queried. * - * Note that mcp_account_manager_set_parameter() does not use the - * "param-" prefix, but this signal and mcp_account_manager_set_value() - * both do. + * Note that mcp_account_manager_get_parameter() and + * mcp_account_manager_set_parameter() do not use the + * "param-" prefix, but this signal does. * * Should not be fired until mcp_account_storage_ready() has been called */ @@ -308,8 +323,11 @@ class_init (gpointer klass, * Emitted if an external entity enables/disables an account * in the backend the emitting plugin handles. This is similar to * emitting #McpAccountStorage::altered-one for the attribute - * "Enabled", except that the plugin is not required to call - * a function like mcp_account_manager_set_value() first. + * "Enabled". + * + * Before emitting this signal, the plugin must update its + * internal cache (if any) so that mcp_account_storage_get_attribute() + * will return the new value for Enabled when queried. * * Should not be fired until mcp_account_storage_ready() has been called * @@ -439,60 +457,83 @@ mcp_account_storage_priority (const McpAccountStorage *storage) } /** - * McpAccountStorageGetFunc: - * @storage: the account storage plugin - * @am: object used to call back into the account manager + * mcp_account_storage_get_attribute: + * @storage: an #McpAccountStorage instance + * @am: an #McpAccountManager instance * @account: the unique name of the account - * @key: the setting whose value we wish to fetch: either an attribute - * like "DisplayName", or "param-" plus a parameter like "account" + * @attribute: the name of an attribute, e.g. "DisplayName" + * @type: the expected type of @attribute, as a hint for + * legacy account storage plugins that do not store attributes' types + * @flags: (allow-none) (out): used to return attribute flags + * + * Retrieve an attribute. * - * An implementation of mcp_account_storage_get(). + * There is a default implementation, which just returns %NULL. + * All account storage plugins must override this method. * - * Returns: %TRUE if @storage is responsible for @account + * The returned variant does not necessarily have to match @type: + * Mission Control will coerce it to an appropriate type if required. In + * particular, plugins that store strongly-typed attributes may return + * the stored type, not the expected type, if they differ. + * + * Returns: (transfer full): the value of the attribute, or %NULL if it + * is not present */ +GVariant * +mcp_account_storage_get_attribute (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags) +{ + McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage); + + SDEBUG (storage, ""); + g_return_val_if_fail (iface != NULL, FALSE); + g_return_val_if_fail (iface->get_attribute != NULL, FALSE); + + return iface->get_attribute (storage, am, account, attribute, type, flags); +} /** - * mcp_account_storage_get: + * mcp_account_storage_get_parameter: * @storage: an #McpAccountStorage instance * @am: an #McpAccountManager instance * @account: the unique name of the account - * @key: the setting whose value we wish to fetch: either an attribute - * like "DisplayName", or "param-" plus a parameter like "account" - * - * Get a value from the plugin's in-memory cache. - * Before emitting this signal, the plugin must call - * either mcp_account_manager_set_attribute(), - * mcp_account_manager_set_parameter(), - * or mcp_account_manager_set_value() - * before returning from this method call. + * @parameter: the name of a parameter, e.g. "require-encryption" + * @type: the expected type of @parameter, as a hint for + * legacy account storage plugins that do not store parameters' types + * @flags: (allow-none) (out): used to return parameter flags * - * Note that mcp_account_manager_set_parameter() does not use the - * "param-" prefix, even if called from this function. + * Retrieve a parameter. * - * If @key is %NULL the plugin should iterate through all attributes and - * parameters, and push each of them into @am, as if this method had - * been called once for each attribute or parameter. It must then return - * %TRUE if any attributes or parameters were found, or %FALSE if it - * was not responsible for @account. + * There is a default implementation, which just returns %NULL. + * All account storage plugins must override this method. * - * The default implementation just returns %FALSE, and should always be - * overridden. + * The returned variant does not necessarily have to match @type: + * Mission Control will coerce it to an appropriate type if required. In + * particular, plugins that store strongly-typed parameters may return + * the stored type, not the expected type, if they differ. * - * Returns: %TRUE if @storage is responsible for @account + * Returns: (transfer full): the value of the parameter, or %NULL if it + * is not present */ -gboolean -mcp_account_storage_get (const McpAccountStorage *storage, +GVariant * +mcp_account_storage_get_parameter (McpAccountStorage *storage, McpAccountManager *am, const gchar *account, - const gchar *key) + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags) { McpAccountStorageIface *iface = MCP_ACCOUNT_STORAGE_GET_IFACE (storage); SDEBUG (storage, ""); g_return_val_if_fail (iface != NULL, FALSE); - g_return_val_if_fail (iface->get != NULL, FALSE); + g_return_val_if_fail (iface->get_parameter != NULL, FALSE); - return iface->get (storage, am, account, key); + return iface->get_parameter (storage, am, account, parameter, type, flags); } /** diff --git a/mission-control-plugins/account-storage.h b/mission-control-plugins/account-storage.h index 3a8766e..42898a5 100644 --- a/mission-control-plugins/account-storage.h +++ b/mission-control-plugins/account-storage.h @@ -65,11 +65,6 @@ struct _McpAccountStorage { }; GType mcp_account_storage_get_type (void); /* Virtual method implementation signatures */ -typedef gboolean (*McpAccountStorageGetFunc) ( - const McpAccountStorage *storage, - const McpAccountManager *am, - const gchar *account, - const gchar *key); typedef gchar * (*McpAccountStorageCreate) ( const McpAccountStorage *storage, const McpAccountManager *am, @@ -112,7 +107,6 @@ struct _McpAccountStorageIface const gchar *desc; const gchar *provider; - McpAccountStorageGetFunc get; void (*delete_async) (McpAccountStorage *storage, McpAccountManager *am, const gchar *account, @@ -130,6 +124,18 @@ struct _McpAccountStorageIface McpAccountStorageCreate create; /* Since 5.15.0 */ + GVariant *(*get_attribute) (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags); + GVariant *(*get_parameter) (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags); McpAccountStorageSetResult (*set_attribute) (McpAccountStorage *storage, McpAccountManager *am, const gchar *account, @@ -147,11 +153,6 @@ struct _McpAccountStorageIface /* virtual methods */ gint mcp_account_storage_priority (const McpAccountStorage *storage); -gboolean mcp_account_storage_get (const McpAccountStorage *storage, - McpAccountManager *am, - const gchar *account, - const gchar *key); - gchar * mcp_account_storage_create (const McpAccountStorage *storage, const McpAccountManager *am, const gchar *manager, @@ -196,6 +197,19 @@ 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); +GVariant *mcp_account_storage_get_attribute (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags); +GVariant *mcp_account_storage_get_parameter (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account, + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags); + McpAccountStorageSetResult mcp_account_storage_set_attribute ( McpAccountStorage *storage, McpAccountManager *am, diff --git a/mission-control-plugins/account.c b/mission-control-plugins/account.c index a6ca95a..2c19461 100644 --- a/mission-control-plugins/account.c +++ b/mission-control-plugins/account.c @@ -74,100 +74,6 @@ mcp_account_manager_get_type (void) } /** - * mcp_account_manager_set_value: - * @mcpa: an #McpAccountManager instance - * @account: the unique name of an account - * @key: the setting whose value we wish to change: either an attribute - * like "DisplayName", or "param-" plus a parameter like "account" - * @value: the new value, escaped as if for a #GKeyFile, or %NULL to delete - * the setting/parameter - * - * Inform Mission Control that @key has changed its value to @value. - * - * This function may either be called from mcp_account_storage_get(), - * or just before emitting #McpAccountStorage::altered-one. - * - * New plugins should call mcp_account_manager_set_attribute() or - * mcp_account_manager_set_parameter() instead. - */ -void -mcp_account_manager_set_value (const McpAccountManager *mcpa, - const gchar *account, - const gchar *key, - const gchar *value) -{ - McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa); - - g_return_if_fail (iface != NULL); - g_return_if_fail (iface->set_value != NULL); - - iface->set_value (mcpa, account, key, value); -} - -/** - * mcp_account_manager_set_attribute: - * @mcpa: an #McpAccountManager instance - * @account: the unique name of an account - * @attribute: the name of an attribute, such as "DisplayName" - * @value: (allow-none): the new value, or %NULL to delete the attribute - * @flags: flags for the new value (only used if @value is non-%NULL) - * - * Inform Mission Control that @attribute has changed its value to @value. - * - * If @value is a floating reference, Mission Control will take ownership - * of it, much like g_variant_builder_add_value(). - * - * This function may either be called from mcp_account_storage_get(), - * or just before emitting #McpAccountStorage::altered-one. - */ -void -mcp_account_manager_set_attribute (const McpAccountManager *mcpa, - const gchar *account, - const gchar *attribute, - GVariant *value, - McpAttributeFlags flags) -{ - McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa); - - g_return_if_fail (iface != NULL); - g_return_if_fail (iface->set_attribute != NULL); - - iface->set_attribute (mcpa, account, attribute, value, flags); -} - -/** - * mcp_account_manager_set_parameter: - * @mcpa: an #McpAccountManager instance - * @account: the unique name of an account - * @parameter: the name of a parameter, such as "account", without - * the "param-" prefix - * @value: (allow-none): the new value, or %NULL to delete the parameter - * @flags: flags for the new value (only used if @value is non-%NULL) - * - * Inform Mission Control that @parameter has changed its value to @value. - * - * If @value is a floating reference, Mission Control will take ownership - * of it, much like g_variant_builder_add_value(). - * - * This function may either be called from mcp_account_storage_get(), - * or just before emitting #McpAccountStorage::altered-one. - */ -void -mcp_account_manager_set_parameter (const McpAccountManager *mcpa, - const gchar *account, - const gchar *parameter, - GVariant *value, - McpParameterFlags flags) -{ - McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa); - - g_return_if_fail (iface != NULL); - g_return_if_fail (iface->set_parameter != NULL); - - iface->set_parameter (mcpa, account, parameter, value, flags); -} - -/** * mcp_account_manager_get_unique_name: * @mcpa: an #McpAccountManager instance * @manager: the name of the manager diff --git a/mission-control-plugins/account.h b/mission-control-plugins/account.h index 78e58d0..2a46c3a 100644 --- a/mission-control-plugins/account.h +++ b/mission-control-plugins/account.h @@ -45,23 +45,6 @@ typedef struct _McpAccountManagerIface McpAccountManagerIface; GType mcp_account_manager_get_type (void) G_GNUC_CONST; -void mcp_account_manager_set_value (const McpAccountManager *mcpa, - const gchar *account, - const gchar *key, - const gchar *value); - -void mcp_account_manager_set_attribute (const McpAccountManager *mcpa, - const gchar *account, - const gchar *attribute, - GVariant *value, - McpAttributeFlags flags); - -void mcp_account_manager_set_parameter (const McpAccountManager *mcpa, - const gchar *account, - const gchar *parameter, - GVariant *value, - McpParameterFlags flags); - gchar * mcp_account_manager_get_unique_name (McpAccountManager *mcpa, const gchar *manager, const gchar *protocol, diff --git a/mission-control-plugins/implementation.h b/mission-control-plugins/implementation.h index 7db25f5..59096f9 100644 --- a/mission-control-plugins/implementation.h +++ b/mission-control-plugins/implementation.h @@ -77,11 +77,6 @@ struct _McpDispatchOperationIface { struct _McpAccountManagerIface { GTypeInterface parent; - void (*set_value) (const McpAccountManager *ma, - const gchar *acct, - const gchar *key, - const gchar *value); - gchar * (* unique_name) (const McpAccountManager *ma, const gchar *manager, const gchar *protocol, @@ -94,18 +89,6 @@ struct _McpAccountManagerIface { const GVariantType *type, GError **error); - void (* set_attribute) (const McpAccountManager *mcpa, - const gchar *account, - const gchar *attribute, - GVariant *value, - McpAttributeFlags flags); - - void (* set_parameter) (const McpAccountManager *mcpa, - const gchar *account, - const gchar *parameter, - GVariant *value, - McpParameterFlags flags); - void (* identify_account_async) (McpAccountManager *mcpa, const gchar *manager, const gchar *protocol, diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c index 1ceb90b..47d261f 100644 --- a/src/mcd-account-manager-default.c +++ b/src/mcd-account-manager-default.c @@ -52,6 +52,12 @@ typedef struct { gboolean absent; } McdDefaultStoredAccount; +static GVariant * +variant_ref0 (GVariant *v) +{ + return (v == NULL ? NULL : g_variant_ref (v)); +} + static McdDefaultStoredAccount * lookup_stored_account (McdAccountManagerDefault *self, const gchar *account) @@ -267,116 +273,59 @@ set_attribute (McpAccountStorage *self, return MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED; } -static gboolean -get_parameter (const McpAccountStorage *self, - const McpAccountManager *am, +static GVariant * +get_attribute (McpAccountStorage *self, + McpAccountManager *am, const gchar *account, - const gchar *prefixed, - const gchar *parameter) + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags) { McdAccountManagerDefault *amd = MCD_ACCOUNT_MANAGER_DEFAULT (self); McdDefaultStoredAccount *sa = lookup_stored_account (amd, account); - if (parameter != NULL) - { - gchar *v = NULL; - GVariant *variant = NULL; - - if (sa == NULL || sa->absent) - return FALSE; - - variant = g_hash_table_lookup (sa->parameters, parameter); - - if (variant != NULL) - { - mcp_account_manager_set_parameter (am, account, parameter, - variant, MCP_PARAMETER_FLAG_NONE); - return TRUE; - } + if (flags != NULL) + *flags = 0; - v = g_hash_table_lookup (sa->untyped_parameters, parameter); - - if (v == NULL) - return FALSE; - - mcp_account_manager_set_value (am, account, prefixed, v); - } - else - { - g_assert_not_reached (); - } + if (sa == NULL || sa->absent) + return FALSE; - return TRUE; + /* ignore @type, we store every attribute with its type anyway; MC will + * coerce values to an appropriate type if needed */ + return variant_ref0 (g_hash_table_lookup (sa->attributes, attribute)); } -static gboolean -_get (const McpAccountStorage *self, - const McpAccountManager *am, +static GVariant * +get_parameter (McpAccountStorage *self, + McpAccountManager *am, const gchar *account, - const gchar *key) + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags) { McdAccountManagerDefault *amd = MCD_ACCOUNT_MANAGER_DEFAULT (self); McdDefaultStoredAccount *sa = lookup_stored_account (amd, account); + GVariant *variant; + gchar *str; + + if (flags != NULL) + *flags = 0; if (sa == NULL || sa->absent) return FALSE; - if (key != NULL) - { - GVariant *v = NULL; + variant = g_hash_table_lookup (sa->parameters, parameter); - if (g_str_has_prefix (key, "param-")) - { - return get_parameter (self, am, account, key, key + 6); - } + if (variant != NULL) + return g_variant_ref (variant); - v = g_hash_table_lookup (sa->attributes, key); - - if (v == NULL) - return FALSE; - - mcp_account_manager_set_attribute (am, account, key, v, - MCP_ATTRIBUTE_FLAG_NONE); - } - else - { - GHashTableIter iter; - gpointer k, v; - - g_hash_table_iter_init (&iter, sa->attributes); - - while (g_hash_table_iter_next (&iter, &k, &v)) - { - if (v != NULL) - mcp_account_manager_set_attribute (am, account, k, - v, MCP_ATTRIBUTE_FLAG_NONE); - } + str = g_hash_table_lookup (sa->untyped_parameters, parameter); - g_hash_table_iter_init (&iter, sa->parameters); - - while (g_hash_table_iter_next (&iter, &k, &v)) - { - if (v != NULL) - mcp_account_manager_set_parameter (am, account, k, v, - MCP_PARAMETER_FLAG_NONE); - } - - g_hash_table_iter_init (&iter, sa->untyped_parameters); - - while (g_hash_table_iter_next (&iter, &k, &v)) - { - if (v != NULL) - { - gchar *prefixed = g_strdup_printf ("param-%s", - (const gchar *) k); - - mcp_account_manager_set_value (am, account, prefixed, v); - g_free (prefixed); - } - } - } + if (str == NULL) + return NULL; - return TRUE; + return mcp_account_manager_unescape_variant_from_keyfile (am, + str, type, NULL); } static gchar * @@ -1026,7 +975,8 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->desc = PLUGIN_DESCRIPTION; iface->priority = PLUGIN_PRIORITY; - iface->get = _get; + iface->get_attribute = get_attribute; + iface->get_parameter = get_parameter; iface->set_attribute = set_attribute; iface->set_parameter = set_parameter; iface->create = _create; diff --git a/src/mcd-storage.c b/src/mcd-storage.c index af63dfb..dffc498 100644 --- a/src/mcd-storage.c +++ b/src/mcd-storage.c @@ -61,38 +61,11 @@ G_DEFINE_TYPE_WITH_CODE (McdStorage, mcd_storage, G_TYPE_OBJECT, G_IMPLEMENT_INTERFACE (MCP_TYPE_ACCOUNT_MANAGER, plugin_iface_init)) -typedef struct { - /* owned string => GVariant - * e.g. { 'DisplayName': <'Frederick Bloggs'> } */ - GHashTable *attributes; - /* owned string => owned GVariant - * e.g. { 'account': <'fred@example.com'>, 'password': <'foo'> } */ - GHashTable *parameters; - /* owned string => owned string escaped as if for a keyfile - * e.g. { 'account': 'fred@example.com', 'password': 'foo' } - * keys of @parameters and @escaped_parameters are disjoint */ - GHashTable *escaped_parameters; - /* reffed */ - McpAccountStorage *plugin; -} McdStorageAccount; - -static void -mcd_storage_account_free (gpointer p) -{ - McdStorageAccount *sa = p; - - g_hash_table_unref (sa->attributes); - g_hash_table_unref (sa->parameters); - g_hash_table_unref (sa->escaped_parameters); - g_object_unref (sa->plugin); - g_slice_free (McdStorageAccount, sa); -} - static void mcd_storage_init (McdStorage *self) { self->accounts = g_hash_table_new_full (g_str_hash, g_str_equal, - g_free, mcd_storage_account_free); + g_free, g_object_unref); } static void @@ -199,31 +172,6 @@ mcd_keyfile_escape_variant (GVariant *variant) return ret; } -static McdStorageAccount * -lookup_account (McdStorage *self, - const gchar *account) -{ - return g_hash_table_lookup (self->accounts, account); -} - -static McdStorageAccount * -mcd_storage_account_new (McpAccountStorage *plugin, - const gchar *account) -{ - 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->plugin = g_object_ref (plugin); - - return sa; -} - static struct { const gchar *type; const gchar *name; @@ -335,104 +283,6 @@ mcd_storage_init_value_for_attribute (GValue *value, return FALSE; } -static void -mcpa_set_attribute (const McpAccountManager *ma, - const gchar *account, - const gchar *attribute, - GVariant *value, - McpAttributeFlags flags) -{ - McdStorage *self = MCD_STORAGE (ma); - McdStorageAccount *sa = lookup_account (self, account); - - g_return_if_fail (sa != NULL); - - if (value != NULL) - { - g_hash_table_insert (sa->attributes, g_strdup (attribute), - g_variant_ref_sink (value)); - } - else - { - g_hash_table_remove (sa->attributes, attribute); - } -} - -static void -mcpa_set_parameter (const McpAccountManager *ma, - const gchar *account, - const gchar *parameter, - GVariant *value, - McpParameterFlags flags) -{ - McdStorage *self = MCD_STORAGE (ma); - 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); - - if (value != NULL) - g_hash_table_insert (sa->parameters, g_strdup (parameter), - g_variant_ref_sink (value)); -} - -static void -set_value (const McpAccountManager *ma, - const gchar *account, - const gchar *key, - const gchar *value) -{ - McdStorage *self = MCD_STORAGE (ma); - McdStorageAccount *sa = lookup_account (self, account); - - g_return_if_fail (sa != NULL); - - if (g_str_has_prefix (key, "param-")) - { - g_hash_table_remove (sa->parameters, key + 6); - g_hash_table_remove (sa->escaped_parameters, key + 6); - - if (value != NULL) - g_hash_table_insert (sa->escaped_parameters, g_strdup (key + 6), - g_strdup (value)); - } - else - { - if (value != NULL) - { - GValue tmp = G_VALUE_INIT; - GError *error = NULL; - - if (!mcd_storage_init_value_for_attribute (&tmp, key, NULL)) - { - g_warning ("Not sure what the type of '%s' is, assuming string", - key); - g_value_init (&tmp, G_TYPE_STRING); - } - - if (mcd_keyfile_unescape_value (value, &tmp, &error)) - { - g_hash_table_insert (sa->attributes, g_strdup (key), - g_variant_ref_sink (dbus_g_value_build_g_variant (&tmp))); - g_value_unset (&tmp); - } - else - { - g_warning ("Could not decode attribute '%s':'%s' from plugin: %s", - key, value, error->message); - g_error_free (error); - g_hash_table_remove (sa->attributes, key); - } - } - else - { - g_hash_table_remove (sa->attributes, key); - } - } -} - static gchar * unique_name (const McpAccountManager *ma, const gchar *manager, @@ -698,16 +548,13 @@ mcd_storage_dup_accounts (McdStorage *self, { GPtrArray *ret = g_ptr_array_new (); GHashTableIter iter; - gpointer k, v; + gpointer k; g_hash_table_iter_init (&iter, self->accounts); - while (g_hash_table_iter_next (&iter, &k, &v)) + while (g_hash_table_iter_next (&iter, &k, NULL)) { - McdStorageAccount *sa = v; - - if (g_hash_table_size (sa->attributes) > 0) - g_ptr_array_add (ret, g_strdup (k)); + g_ptr_array_add (ret, g_strdup (k)); } g_ptr_array_add (ret, NULL); @@ -731,15 +578,15 @@ McpAccountStorage * mcd_storage_get_plugin (McdStorage *self, const gchar *account) { - McdStorageAccount *sa; + McpAccountStorage *plugin; 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 (account != NULL, NULL); + plugin = g_hash_table_lookup (self->accounts, account); + g_return_val_if_fail (plugin != NULL, NULL); - return sa->plugin; + return plugin; } /* @@ -818,33 +665,38 @@ mcd_storage_get_attribute (McdStorage *self, GValue *value, GError **error) { - McdStorageAccount *sa; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); + McpAccountStorage *plugin; GVariant *variant; + gboolean ret; g_return_val_if_fail (MCD_IS_STORAGE (self), FALSE); g_return_val_if_fail (account != NULL, FALSE); 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); + plugin = g_hash_table_lookup (self->accounts, account); - if (sa == NULL) + if (plugin == NULL) { g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, "Account %s does not exist", account); return FALSE; } - variant = g_hash_table_lookup (sa->attributes, attribute); + variant = mcp_account_storage_get_attribute (plugin, ma, account, + attribute, type, NULL); if (variant == NULL) { g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, - "Setting '%s' not stored by account %s", attribute, account); + "Account %s has no attribute '%s'", account, attribute); return FALSE; } - return mcd_storage_coerce_variant_to_value (variant, value, error); + ret = mcd_storage_coerce_variant_to_value (variant, value, error); + g_variant_unref (variant); + return ret; } /* @@ -863,40 +715,38 @@ mcd_storage_get_parameter (McdStorage *self, GValue *value, GError **error) { - McdStorageAccount *sa; - const gchar *escaped; + McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self); + McpAccountStorage *plugin; GVariant *variant; + gboolean ret; g_return_val_if_fail (MCD_IS_STORAGE (self), FALSE); g_return_val_if_fail (account != NULL, FALSE); g_return_val_if_fail (parameter != NULL, FALSE); + g_return_val_if_fail (!g_str_has_prefix (parameter, "param-"), FALSE); - sa = lookup_account (self, account); + plugin = g_hash_table_lookup (self->accounts, account); - if (sa == NULL) + if (plugin == NULL) { g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, "Account %s does not exist", account); return FALSE; } - variant = g_hash_table_lookup (sa->parameters, parameter); + variant = mcp_account_storage_get_parameter (plugin, ma, account, + parameter, type, NULL); - if (variant != NULL) - return mcd_storage_coerce_variant_to_value (variant, value, error); - - /* OK, we don't have it as a variant. How about the keyfile-escaped - * version? */ - escaped = g_hash_table_lookup (sa->escaped_parameters, parameter); - - if (escaped == NULL) + if (variant == NULL) { g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, - "Parameter '%s' not stored by account %s", parameter, account); + "Account %s has no parameter '%s'", account, parameter); return FALSE; } - return mcd_keyfile_unescape_value (escaped, value, error); + ret = mcd_storage_coerce_variant_to_value (variant, value, error); + g_variant_unref (variant); + return ret; } /* @@ -1508,29 +1358,23 @@ mcd_storage_set_attribute (McdStorage *self, const gchar *attribute, const GValue *value) { - McdStorageAccount *sa; GVariant *new_v; gboolean updated = FALSE; + McpAccountStorage *plugin; g_return_val_if_fail (MCD_IS_STORAGE (self), FALSE); g_return_val_if_fail (account != NULL, FALSE); 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); + plugin = g_hash_table_lookup (self->accounts, account); + g_return_val_if_fail (plugin != NULL, FALSE); if (value != NULL) new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value)); else new_v = NULL; - if (new_v != NULL) - g_hash_table_insert (sa->attributes, g_strdup (attribute), - g_variant_ref (new_v)); - else - g_hash_table_remove (sa->attributes, attribute); - updated = update_storage (self, account, FALSE, attribute, new_v); tp_clear_pointer (&new_v, g_variant_unref); @@ -1560,29 +1404,21 @@ mcd_storage_set_parameter (McdStorage *self, const GValue *value) { GVariant *new_v = NULL; - McdStorageAccount *sa; gboolean updated = FALSE; + McpAccountStorage *plugin; g_return_val_if_fail (MCD_IS_STORAGE (self), FALSE); 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); + plugin = g_hash_table_lookup (self->accounts, account); + g_return_val_if_fail (plugin != NULL, FALSE); if (value != NULL) { new_v = g_variant_ref_sink (dbus_g_value_build_g_variant (value)); } - g_hash_table_remove (sa->escaped_parameters, parameter); - - if (new_v != NULL) - g_hash_table_insert (sa->parameters, g_strdup (parameter), - g_variant_ref (new_v)); - else - g_hash_table_remove (sa->parameters, parameter); - updated = update_storage (self, account, TRUE, parameter, new_v); tp_clear_pointer (&new_v, g_variant_unref); @@ -2118,9 +1954,6 @@ plugin_iface_init (McpAccountManagerIface *iface, { DEBUG (); - iface->set_value = set_value; - iface->set_attribute = mcpa_set_attribute; - iface->set_parameter = mcpa_set_parameter; iface->unique_name = unique_name; iface->identify_account_async = identify_account_async; iface->identify_account_finish = identify_account_finish; @@ -2134,32 +1967,20 @@ mcd_storage_add_account_from_plugin (McdStorage *self, const gchar *account, GError **error) { - McdStorageAccount *sa = lookup_account (self, account); + McpAccountStorage *other = g_hash_table_lookup (self->accounts, account); - if (sa != NULL) + if (other != NULL) { g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, "account %s already exists in plugin '%s', cannot create " "for plugin '%s'", account, - mcp_account_storage_name (sa->plugin), + mcp_account_storage_name (other), mcp_account_storage_name (plugin)); return FALSE; } g_hash_table_insert (self->accounts, g_strdup (account), - mcd_storage_account_new (plugin, account)); - - if (!mcp_account_storage_get (plugin, MCP_ACCOUNT_MANAGER (self), - account, NULL)) - { - g_set_error (error, TP_ERROR, TP_ERROR_NOT_AVAILABLE, - "plugin '%s' denied any knowledge of account %s", - mcp_account_storage_name (plugin), - account); - g_hash_table_remove (self->accounts, account); - return FALSE; - } - + g_object_ref (plugin)); return TRUE; } diff --git a/src/mcd-storage.h b/src/mcd-storage.h index be0d0aa..d78b595 100644 --- a/src/mcd-storage.h +++ b/src/mcd-storage.h @@ -30,7 +30,7 @@ G_BEGIN_DECLS typedef struct { GObject parent; TpDBusDaemon *dbusd; - /* owned string => owned McdStorageAccount */ + /* owned string => owned McpAccountStorage */ GHashTable *accounts; } McdStorage; diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c index 7eaccc3..9e0025a 100644 --- a/tests/twisted/dbus-account-plugin.c +++ b/tests/twisted/dbus-account-plugin.c @@ -443,8 +443,6 @@ test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self, g_hash_table_insert (account->attributes, g_strdup (attr), g_variant_ref (value)); - mcp_account_manager_set_attribute (self->feedback, - account_name, attr, value, flags); mcp_account_storage_emit_altered_one ( MCP_ACCOUNT_STORAGE (self), account_name, attr); @@ -468,8 +466,6 @@ test_dbus_account_plugin_process_attributes (TestDBusAccountPlugin *self, DEBUG ("%s deleted", attr); g_hash_table_remove (account->attributes, attr); - mcp_account_manager_set_attribute (self->feedback, - account_name, attr, NULL, 0); mcp_account_storage_emit_altered_one ( MCP_ACCOUNT_STORAGE (self), account_name, attr); @@ -552,8 +548,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self, g_hash_table_insert (account->parameters, g_strdup (param), g_variant_ref (value)); key = g_strdup_printf ("param-%s", param); - mcp_account_manager_set_parameter (self->feedback, - account_name, param, value, flags); mcp_account_storage_emit_altered_one ( MCP_ACCOUNT_STORAGE (self), account_name, key); g_free (key); @@ -585,8 +579,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self, g_hash_table_insert (account->untyped_parameters, g_strdup (param), g_strdup (escaped)); key = g_strdup_printf ("param-%s", param); - mcp_account_manager_set_value (self->feedback, - account_name, key, escaped); mcp_account_storage_emit_altered_one ( MCP_ACCOUNT_STORAGE (self), account_name, key); g_free (key); @@ -616,8 +608,6 @@ test_dbus_account_plugin_process_parameters (TestDBusAccountPlugin *self, g_hash_table_remove (account->untyped_parameters, param); key = g_strdup_printf ("param-%s", param); - mcp_account_manager_set_parameter (self->feedback, - account_name, param, NULL, 0); mcp_account_storage_emit_altered_one ( MCP_ACCOUNT_STORAGE (self), account_name, key); g_free (key); @@ -964,123 +954,81 @@ test_dbus_account_plugin_delete_finish (McpAccountStorage *storage, return g_task_propagate_boolean (G_TASK (res), error); } -static gboolean -test_dbus_account_plugin_get (const McpAccountStorage *storage, - const McpAccountManager *am, +static GVariant * +test_dbus_account_plugin_get_attribute (McpAccountStorage *storage, + McpAccountManager *am, const gchar *account_name, - const gchar *key) + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags) { TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage); Account *account = lookup_account (self, account_name); + GVariant *v; - if (!self->active || account == NULL) - return FALSE; - - if (key == NULL) - { - GHashTableIter iter; - gpointer k, v; - - /* get everything */ - g_dbus_connection_emit_signal (self->bus, NULL, - TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, - "GetAllKeys", - g_variant_new_parsed ("(%o,)", account->path), NULL); - - g_hash_table_iter_init (&iter, account->attributes); + if (flags != NULL) + *flags = 0; - while (g_hash_table_iter_next (&iter, &k, &v)) - { - gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am, - v); - - mcp_account_manager_set_value (am, account_name, k, escaped); - g_free (escaped); - } + if (!self->active || account == NULL) + return NULL; - g_hash_table_iter_init (&iter, account->untyped_parameters); + v = g_hash_table_lookup (account->attributes, attribute); - while (g_hash_table_iter_next (&iter, &k, &v)) - { - gchar *param_foo; + g_dbus_connection_emit_signal (self->bus, NULL, + TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, + "GetAttribute", + g_variant_new_parsed ("(%o, %s)", account->path, attribute), NULL); - param_foo = g_strdup_printf ("param-%s", (const gchar *) k); - mcp_account_manager_set_value (am, account_name, param_foo, v); + if (v != NULL) + { + return g_variant_ref (v); + } + else + { + return NULL; + } +} - g_free (param_foo); - } +static GVariant * +test_dbus_account_plugin_get_parameter (McpAccountStorage *storage, + McpAccountManager *am, + const gchar *account_name, + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags) +{ + TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage); + Account *account = lookup_account (self, account_name); + GVariant *v; + const gchar *s; - g_hash_table_iter_init (&iter, account->parameters); + if (flags != NULL) + *flags = 0; - while (g_hash_table_iter_next (&iter, &k, &v)) - { - gchar *param_foo; - gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am, - v); + if (!self->active || account == NULL) + return NULL; - param_foo = g_strdup_printf ("param-%s", (const gchar *) k); - mcp_account_manager_set_value (am, account_name, param_foo, escaped); - g_free (escaped); + g_dbus_connection_emit_signal (self->bus, NULL, + TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, + "GetParameter", + g_variant_new_parsed ("(%o, %s)", account->path, parameter), NULL); - g_free (param_foo); - } + v = g_hash_table_lookup (account->parameters, parameter); + s = g_hash_table_lookup (account->untyped_parameters, parameter); - return TRUE; + if (v != NULL) + { + return g_variant_ref (v); } - - /* get one parameter */ - - if (g_str_has_prefix (key, "param-")) + else if (s != NULL) { - GVariant *v = g_hash_table_lookup (account->parameters, key + 6); - const gchar *s = g_hash_table_lookup (account->untyped_parameters, key + 6); - - g_dbus_connection_emit_signal (self->bus, NULL, - TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, - "GetParameter", - g_variant_new_parsed ("(%o, %s)", account->path, key + 6), NULL); - - if (v != NULL) - { - gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am, - v); - - mcp_account_manager_set_value (am, account_name, key, escaped); - g_free (escaped); - } - else if (s != NULL) - { - mcp_account_manager_set_value (am, account_name, key, s); - } - else - { - return FALSE; - } + return mcp_account_manager_unescape_variant_from_keyfile (am, + s, type, NULL); } else { - GVariant *v = g_hash_table_lookup (account->attributes, key); - - g_dbus_connection_emit_signal (self->bus, NULL, - TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE, - "GetAttribute", - g_variant_new_parsed ("(%o, %s)", account->path, key), NULL); - - if (v != NULL) - { - gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am, - v); - - mcp_account_manager_set_value (am, account_name, key, escaped); - g_free (escaped); - } - else - { - return FALSE; - } + return NULL; } - - return TRUE; } static McpAccountStorageSetResult @@ -1586,7 +1534,8 @@ account_storage_iface_init (McpAccountStorageIface *iface) /* this should be higher priority than the diverted-keyfile one */ iface->priority = MCP_ACCOUNT_STORAGE_PLUGIN_PRIO_NORMAL + 100; - iface->get = test_dbus_account_plugin_get; + iface->get_attribute = test_dbus_account_plugin_get_attribute; + iface->get_parameter = test_dbus_account_plugin_get_parameter; iface->set_attribute = test_dbus_account_plugin_set_attribute; iface->set_parameter = test_dbus_account_plugin_set_parameter; iface->list = test_dbus_account_plugin_list; diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c index 7bba5d8..11c7caa 100644 --- a/tests/twisted/mcp-account-diversion.c +++ b/tests/twisted/mcp-account-diversion.c @@ -197,47 +197,57 @@ _set_parameter (McpAccountStorage *self, return ret; } -static gboolean -_get (const McpAccountStorage *self, - const McpAccountManager *am, +static GVariant * +_get_attribute (McpAccountStorage *self, + McpAccountManager *am, const gchar *account, - const gchar *key) + const gchar *attribute, + const GVariantType *type, + McpAttributeFlags *flags) { AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self); + gchar *v; + GVariant *ret; - if (key != NULL) - { - gchar *v = g_key_file_get_value (adp->keyfile, account, key, NULL); + if (flags != NULL) + *flags = 0; - if (v == NULL) - return FALSE; + v = g_key_file_get_value (adp->keyfile, account, attribute, NULL); - mcp_account_manager_set_value (am, account, key, v); - g_free (v); - } - else - { - gsize i; - gsize n; - GStrv keys = g_key_file_get_keys (adp->keyfile, account, &n, NULL); + if (v == NULL) + return NULL; - if (keys == NULL) - n = 0; + ret = mcp_account_manager_unescape_variant_from_keyfile (am, v, type, NULL); + g_free (v); + return ret; +} - for (i = 0; i < n; i++) - { - gchar *v = g_key_file_get_value (adp->keyfile, account, keys[i], NULL); +static GVariant * +_get_parameter (McpAccountStorage *self, + McpAccountManager *am, + const gchar *account, + const gchar *parameter, + const GVariantType *type, + McpParameterFlags *flags) +{ + AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self); + gchar *key; + gchar *v; + GVariant *ret; - if (v != NULL) - mcp_account_manager_set_value (am, account, keys[i], v); + if (flags != NULL) + *flags = 0; - g_free (v); - } + key = g_strdup_printf ("param-%s", parameter); + v = g_key_file_get_value (adp->keyfile, account, key, NULL); + g_free (key); - g_strfreev (keys); - } + if (v == NULL) + return NULL; - return TRUE; + ret = mcp_account_manager_unescape_variant_from_keyfile (am, v, type, NULL); + g_free (v); + return ret; } static gboolean _commit (const McpAccountStorage *self, @@ -371,7 +381,8 @@ account_storage_iface_init (McpAccountStorageIface *iface, iface->desc = PLUGIN_DESCRIPTION; iface->priority = PLUGIN_PRIORITY; - iface->get = _get; + iface->get_attribute = _get_attribute; + iface->get_parameter = _get_parameter; iface->set_attribute = _set_attribute; iface->set_parameter = _set_parameter; iface->delete_async = delete_async; -- 1.8.4.3