Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[mcp-account-manager] Delay creating accounts until enabled for the f…
…irst time

If a telepathy account is created immediately for a new account, the
telepathy property writes can race against other applications and result
in those properties being lost. Additionally, some properties of the
account may not be properly set at the instant when the account is
created. The enabling of the account is a good signal that it is fully
created and ready to use.
  • Loading branch information
John Brooks committed Nov 8, 2013
1 parent b6e8e1e commit 4739efe
Showing 1 changed file with 122 additions and 70 deletions.
192 changes: 122 additions & 70 deletions mcp-account-manager-uoa/mcp-account-manager-uoa.c
Expand Up @@ -52,6 +52,7 @@
#define KEY_READONLY_PARAMS "mc-readonly-params"

static void account_storage_iface_init (McpAccountStorageIface *iface);
static void create_account(AgAccountService *service, McpAccountManagerUoa *self);

G_DEFINE_TYPE_WITH_CODE (McpAccountManagerUoa, mcp_account_manager_uoa,
G_TYPE_OBJECT,
Expand All @@ -71,6 +72,11 @@ struct _McpAccountManagerUoaPrivate
* AgAccount, even if unlikely. */
GHashTable *accounts;

/* List of AgAccountService that are monitored but don't yet have an
* associated telepathy account and identifier. A reference must be held
* to watch signals. */
GList *pending_accounts;

/* Queue of owned DelayedSignalData */
GQueue *pending_signals;

Expand Down Expand Up @@ -200,16 +206,32 @@ _service_enabled_cb (AgAccountService *service,
McpAccountManagerUoa *self)
{
gchar *account_name = _service_dup_tp_account_name (service);
GList *node;

if (!self->priv->ready || account_name == NULL)
return;
if (account_name == NULL)
{
if (enabled)
{
create_account (service, self);

DEBUG ("UOA account %s toggled: %s", account_name,
enabled ? "enabled" : "disabled");
node = g_list_find (self->priv->pending_accounts, service);
if (node)
{
self->priv->pending_accounts = g_list_delete_link (self->priv->pending_accounts,
node);
g_object_unref (service);
}
}
}
else
{
DEBUG ("UOA account %s toggled: %s", account_name,
enabled ? "enabled" : "disabled");

/* FIXME: Should this update the username from signon credentials first,
* in case that was changed? */
g_signal_emit_by_name (self, "toggled", account_name, enabled);
/* FIXME: Should this update the username from signon credentials first,
* in case that was changed? */
g_signal_emit_by_name (self, "toggled", account_name, enabled);
}

g_free (account_name);
}
Expand Down Expand Up @@ -263,29 +285,22 @@ _add_service (McpAccountManagerUoa *self,
g_strdup (account_name),
g_object_ref (service));

g_signal_connect (service, "enabled",
G_CALLBACK (_service_enabled_cb), self);
g_signal_connect (service, "changed",
G_CALLBACK (_service_changed_cb), self);

return TRUE;
}

static void
_account_create(McpAccountManagerUoa *self, AgAccountService *service)
{
AgAccount *account = ag_account_service_get_account (service);
gchar *cm_name = _service_dup_tp_value (service, "manager");
gchar *protocol_name = _service_dup_tp_value (service, "protocol");
gchar *service_name = 0;
gchar *account_name = 0;
gchar *tmp;
guint account_id = 0;

g_object_get (ag_account_service_get_account (service), "id", &account_id, NULL);

if (tp_str_empty (cm_name) || tp_str_empty (protocol_name))
{
g_debug ("UOA _account_create missing manager/protocol for new account %u, ignoring", account_id);
g_debug ("UOA _account_create missing manager/protocol for new account %u, ignoring", account->id);
g_free (cm_name);
g_free (protocol_name);
return;
Expand All @@ -303,11 +318,10 @@ _account_create(McpAccountManagerUoa *self, AgAccountService *service)

service_name = tp_escape_as_identifier (ag_service_get_name (ag_account_service_get_service (service)));
account_name = g_strdup_printf ("%s/%s/%s_%u", cm_name, protocol_name,
service_name, account_id);
service_name, account->id);

_service_set_tp_account_name (service, account_name);
ag_account_store (ag_account_service_get_account (service),
_account_stored_cb, self);
ag_account_store (account, _account_stored_cb, self);

g_debug("UOA _account_create: %s", account_name);

Expand Down Expand Up @@ -352,7 +366,6 @@ _account_created_signon_cb(SignonIdentity *signon,
}

g_object_unref (data->service);
g_object_unref (data->account);
g_object_unref (signon);
g_free(data);
}
Expand All @@ -362,88 +375,102 @@ _account_created_cb (AgManager *manager,
AgAccountId id,
McpAccountManagerUoa *self)
{
AgAccount *account;
GList *l;
AgAccount *account = ag_manager_get_account (self->priv->manager, id);

if (!self->priv->ready)
{
DelayedSignalData *data = g_slice_new0 (DelayedSignalData);

data->signal = DELAYED_CREATE;
data->account_id = id;
data->account_id = account->id;

g_queue_push_tail (self->priv->pending_signals, data);
return;
}

account = ag_manager_get_account (self->priv->manager, id);

l = ag_account_list_services_by_type (account, SERVICE_TYPE);
while (l != NULL)
{
AgAccountService *service = ag_account_service_new (account, l->data);
gchar *account_name = _service_dup_tp_account_name (service);
g_signal_connect (service, "enabled",
G_CALLBACK (_service_enabled_cb), self);
g_signal_connect (service, "changed",
G_CALLBACK (_service_changed_cb), self);

ag_service_unref (l->data);
l = g_list_delete_link (l, l);

/* If this is the first time we see this service, we have to generate an
* account_name for it. */
if (account_name == NULL)
if (ag_account_get_enabled (account))
{
gchar *username = _service_dup_tp_value(service, "param-account");
if (!username)
{
/* Request auth data to get the username from signon; it's not available
* from the account. */
AgAuthData *auth_data = ag_account_service_get_auth_data (service);
if (!auth_data)
{
DEBUG("UOA account is missing auth data; ignored");
g_object_unref (service);
g_object_unref (account);
return;
}
create_account (service, self);
}
else
{
self->priv->pending_accounts = g_list_prepend (self->priv->pending_accounts,
g_object_ref (service));
}

guint cred_id = ag_auth_data_get_credentials_id (auth_data);
ag_auth_data_unref(auth_data);
g_object_unref (service);
ag_service_unref (l->data);
l = g_list_delete_link (l, l);
}

SignonIdentity *signon = signon_identity_new_from_db (cred_id);
if (!signon)
{
DEBUG("UOA cannot create signon identity from account (cred_id %u); ignored", cred_id);
g_object_unref (service);
g_object_unref (account);
return;
}
g_object_unref (account);
}

/* Callback frees/unrefs data */
AccountCreateData *data = g_new(AccountCreateData, 1);
data->account = account;
data->service = service;
data->self = self;
static void
create_account(AgAccountService *service,
McpAccountManagerUoa *self)
{
gchar *account_name = _service_dup_tp_account_name (service);

DEBUG("UOA querying account info from signon");
signon_identity_query_info(signon, _account_created_signon_cb, data);
/* If this is the first time we see this service, we have to generate an
* account_name for it. */
if (account_name == NULL)
{
gchar *username = _service_dup_tp_value(service, "param-account");
if (!username)
{
/* Request auth data to get the username from signon; it's not available
* from the account. */
AgAuthData *auth_data = ag_account_service_get_auth_data (service);
if (!auth_data)
{
DEBUG("UOA account is missing auth data; ignored");
return;
}
else

guint cred_id = ag_auth_data_get_credentials_id (auth_data);
ag_auth_data_unref(auth_data);

SignonIdentity *signon = signon_identity_new_from_db (cred_id);
if (!signon)
{
_account_create (self, service);
g_free (username);
DEBUG("UOA cannot create signon identity from account (cred_id %u); ignored", cred_id);
return;
}

/* Callback frees/unrefs data */
AccountCreateData *data = g_new(AccountCreateData, 1);
data->account = ag_account_service_get_account (service);
data->service = g_object_ref (service);
data->self = self;

DEBUG("UOA querying account info from signon");
signon_identity_query_info(signon, _account_created_signon_cb, data);
return;
}
else
{
if (_add_service (self, service, account_name))
g_signal_emit_by_name (self, "created", account_name);
_account_create (self, service);
g_free (username);
}

g_free (account_name);
g_object_unref (service);
}
else
{
if (_add_service (self, service, account_name))
g_signal_emit_by_name (self, "created", account_name);
}

g_object_unref (account);
g_free (account_name);
}

static void
Expand All @@ -453,6 +480,7 @@ _account_deleted_cb (AgManager *manager,
{
GHashTableIter iter;
gpointer value;
GList *node;

if (!self->priv->ready)
{
Expand Down Expand Up @@ -486,6 +514,22 @@ _account_deleted_cb (AgManager *manager,

g_free (account_name);
}

node = self->priv->pending_accounts;
while (node)
{
GList *next = g_list_next (node);
AgAccountService *service = node->data;
AgAccount *account = ag_account_service_get_account (service);

if (account->id == id)
{
g_object_unref (service);
self->priv->pending_accounts = g_list_delete_link (self->priv->pending_accounts, node);
}

node = next;
}
}

static void
Expand All @@ -498,6 +542,9 @@ mcp_account_manager_uoa_dispose (GObject *object)
tp_clear_pointer (&self->priv->accounts, g_hash_table_unref);
tp_clear_object (&self->priv->monitor);

g_list_free_full (self->priv->pending_accounts, g_object_unref);
self->priv->pending_accounts = NULL;

G_OBJECT_CLASS (mcp_account_manager_uoa_parent_class)->dispose (object);
}

Expand All @@ -511,6 +558,7 @@ mcp_account_manager_uoa_init (McpAccountManagerUoa *self)

self->priv->accounts = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, g_object_unref);
self->priv->pending_accounts = NULL;
self->priv->pending_signals = g_queue_new ();

self->priv->manager = ag_manager_new_for_service_type (SERVICE_TYPE);
Expand Down Expand Up @@ -559,6 +607,10 @@ _ensure_loaded (McpAccountManagerUoa *self)
{
/* This service was already known, we can add it now */
_add_service (self, service, account_name);
g_signal_connect (service, "enabled",
G_CALLBACK (_service_enabled_cb), self);
g_signal_connect (service, "changed",
G_CALLBACK (_service_changed_cb), self);
g_free (account_name);
}
else
Expand Down

0 comments on commit 4739efe

Please sign in to comment.