Bug 54010

Forgot to attach my branch.

From IRC discussion, I understand you were unsure about create_async vs. commit_async.

I feel as though if anything is async, committing ought to be async (intuitively, it's the thing that ought to take time and be failable).

On the other hand, for your usage in UOA, you can't return the object-path tail synchronously... so account creation can't usefully be sync either?

There is also the issue that UOA is currently the only thing overriding create() (true?), which makes create() rather useless. I wish we'd thought about this before merging create().

I'm tempted to merge a branch I'm working on (which combines McdStorage into McdPluginAccountManager) before this one, so that you don't have to write so much boilerplate...

(I haven't reviewed in detail til we agree on an approach.)

The fallback chain through attempts to create accounts, asynchronously, seems rather undesirable: if you have, say, 5 plugins, then it takes 5 async round-trips (of which the first 4 are just "do you want to store this account?" "not really") before we actually do anything. Could we perhaps change it to be more like this?

    create() -> unique ID, a special constant not syntactically allowed
        in account names meaning "in progress" (maybe #define
        MCP_ACCOUNT_CREATION_IN_PROGRESS to "<in progress>"?),
        or NULL to decline

    complete_account_creation_async(), _finish() -> unique ID
        (should only be called if MCP_ACCOUNT_CREATION_IN_PROGRESS
        was returned)

Then we could do (pseudocode):

    foreach (storage_plugin)
      {
        id = storage_plugin.create(...)

        if (!tp_strdiff (id, MCP_ACCOUNT_CREATION_IN_PROGRESS))
          {
            storage_plugin.complete_account_creation_async(...,
                lambda id: raise error or actually create the account (id));
            break;
          }
        else if (id != NULL)
          {
            actually create the account (id);
            break;
          }
        /* else continue to next backend */
      }

    if (nobody took responsibility)
       raise error;

and even though you can't necessarily know what the account's ID is until you have actually done the "complete account creation" step, you can at least know whether this plugin is going to be the one to store the account or not. I'm assuming that plugins are at least intelligent enough to know what they can and can't store in a synchronous way?

I'm currently thinking about whether to take Bug #35896 as an excuse to write an actually helpful plugin API with a sensible data model. Perhaps we can discuss that on IRC later?

(In reply to comment #2)
> I feel as though if anything is async, committing ought to be async
> (intuitively, it's the thing that ought to take time and be failable).

Yep, even independently to this bug, I think commit_async() is a good thing.

> On the other hand, for your usage in UOA, you can't return the object-path tail
> synchronously... so account creation can't usefully be sync either?

Actually in my case I can. The problem I have is the identifier returned by tp_account_get_storage_identifier() is known only after the account has been stored. So if CreateAccount returns too early and client gets the storage identifier it will be 0 instead of a real identifier.

I didn't though about this when writing the patch above, but actually just making commit_async() and make sure CreateAccount does not return before commit_async is done is enough to fix my case.

> There is also the issue that UOA is currently the only thing overriding
> create() (true?), which makes create() rather useless. I wish we'd thought
> about this before merging create().

With RH ignoring what Canonical does, I think they'll soon come with the same kid of storage in GOA.

> I'm tempted to merge a branch I'm working on (which combines McdStorage into
> McdPluginAccountManager) before this one, so that you don't have to write so
> much boilerplate...

Oh yes please do!

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-mission-control/issues/65.