[Bug 52231] Storage plugins should be able to create new accounts

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=52231

--- Comment #9 from Xavier Claessens <xclaesse at gmail.com> 2012-07-20 08:03:27 PDT ---
(In reply to comment #8)
> > + if (iface->create == NULL)
> > + {
> > + g_set_error (error, TP_ERROR, TP_ERROR_NOT_IMPLEMENTED,
> > + "This storage does not implement create function");
> > + return NULL;
> > + }
> 
> This can break existing plugins. You can tell by the fact that you had to
> change the default pseudo-plugin to accommodate it...

Why would it break existing plugins? They don't implement iface->create, so it
returns NULL with an error and it will try the next one in priority order.
Since the default plugin always succeed and is build-in we are sure there is at
least one plugin that will return non-NULL value.

> In Comment #3 I suggested giving McpAccountStorage a default implementation of
> ->create (set up in the interface's base_init) which calls
> mcp_account_manager_get_unique_name, and tries to set the manager and protocol
> keys. Then you could move responsibility for setting those keys into ->create,
> removing it from _mcd_account_manager_create_account.

All plugins does not support creating accounts. For example GOA doesn't. So it
should not have a default implementation, only the default storage
pseudo-plugin can always create accounts.

> In existing plugins, this will work if and only if the "set" method works, i.e.
> if and only if the plugin was willing to make the account exist - that seems
> exactly what we want. In particular, you won't need any changes to the default
> pseudo-plugin any more.

I didn't want to set manager/protocol from within ->create() implementation
because then why would implementation have to set them and not the params asv?
It's a bit more complicated for plugins to handle the asv because MC has code
to convert the GValue to a string. So I though it would be better to just tell
that the ->create() implementation does not need to set anything but only
create a unique_name and ensure that _set() will recognize that unique_name.

> Conveniently, it also means ->create will never be NULL.

Which is what I want to avoid for plugins that does not support creating new
accounts like GOA.

> > + * Create an account in long term storage.
> 
> Nothing is created in long term storage until you commit(), which the default
> implementation doesn't. Please document whether implementations are expected to
> commit changes to long-term storage (by inspection, "no").

It is already documented on mcp_account_storage_create(), but
mcd_storage_create_account() can be improved indeed.

> The more I read the McpAccountStorage interface, the more I want to replace it
> with something nicer... but it'll have to do for now.

+1

> (In reply to comment #1)
> >   it should be documented/specified [whether] after a
> >   successful call, the "list" vfunc will include that account
> 
> It turns out that the "list" vfunc is documented to only be called once, during
> startup, so this never becomes significant. (Yay, non-state-recoverable
> interfaces.)
> 
> >   it should be documented/specified whether a call to "commit"
> >   is implied
> 
> Still needs doing. Also, please document whether the "created" signal is
> emitted (by inspection: "no").
> 
> > I'd like a regression test, ideally via
> > a test plugin that does something vaguely realistic.
> 
> Still true.

Since it goes through the default storage to create accounts now, it's all
tested already, AFAIK.

> (In reply to comment #7)
> In Telepathy 1.0, perhaps we should relax the definition of an account "unique
> name" (object-path tail) from "exactly 3 components" to "at least 3
> components". Then MC's own storage could use gabble/jabber/smcv_40example_2ecom
> and the GOA plugin could use gabble/jabber/goa/_123 or something.

+1

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
You are the assignee for the bug.