[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 #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-07-18 12:31:45 PDT ---
> Storage plugins should be able to create new accounts into their storage.

That's not what the branch does...

> This
> would be done by calling CreateAccount() with
> Account.Interface.Storage.StorageProvider key in the 'properties' asv.

... what the branch provides is "Telepathy clients should be able to request
that an account they create gets stored in a particular storage backend".

+gchar *
+mcp_account_storage_create_account (const McpAccountStorage *storage,
+ const gchar *manager,
+ const gchar *protocol,
+ GHashTable *params)

What do you expect this vfunc to have to do? I'm a bit concerned that none of
the in-tree plugins or pseudo-plugins, not even the regression tests, implement
it - which makes it rather non-obvious how an implementation would look.

Here are some possible answers:

* Suggest a unique name for the account. In this case, it should be
  called suggest_unique_name and there should be a default implementation
  which calls mcp_account_manager_get_unique_name.

* Confirm whether it considers the manager/protocol/params tuple to be
  something it can deal with, but do not do anything irrevocable yet.
  In this case, it should raise a TpError if it returns NULL, and
  be called check_account_creation or something, and it should be
  specified that after a successful call, the "list" vfunc will *not*
  include that account (unless it already existed, but perhaps that
  should be specified to be an error). Perhaps it should
  take the properties as an additional parameter. The default
  implementation should be to do nothing, successfully.

* Actually create a blank account, including parameter validation and
  perhaps some other stuff. In this case, it should raise a TpError if it
  returns NULL, it should be documented/specified that after a
  successful call, the "list" vfunc will include that account, and
  it should be documented/specified whether a call to "commit"
  is implied. The default implementation should probably do set() for
  a couple of essential keys (CM name, protocol, Enabled=FALSE?),
  and commit() if appropriate, so that it does the right thing for
  existing storage backends.

>From its location in the code, I infer that it's actually more or less my
second option (check_account_creation) at the moment.

Another possibility is two-phase account creation: suggest a unique name/do
basic checks at the location of the current call to this vfunc, then do the
real account creation later on, when construct-time properties are set.

The implementation looks reasonable but I'd like a regression test, ideally via
a test plugin that does something vaguely realistic. If it's
check_account_creation, it should do something like "only store HypotheticalIM
accounts whose account key ends with @example.com". If it's full account
creation, it should make blocking D-Bus calls to an imaginary account storage
daemon for which the test can be a mock implementation.

-- 
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.