[Bug 71384] improve account storage GInterface

[bugzilla-daemon at freedesktop.org]

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

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
Context:

<xclaesse> smcv, Hmm, I have an issue with MC storage plugins: the -default
plugin writes a .account for accounts that comes from -ring plugin
<xclaesse> then when I restart MC, the -default storage see that .account and
create the account before -ring plugin
<smcv> xclaesse: patches to make the account API more competent welcome
<smcv> xclaesse: the underlying bug is
https://bugs.freedesktop.org/show_bug.cgi?id=27727
<xclaesse> smcv, before making patches, do you agree that if an account comes
from a plugin, the -default should not store it, right?
<smcv> xclaesse: yes, and while we're breaking plugin ABI is a great time to
implement that
<xclaesse> smcv, if I understand correctly, mcd_storage_commit() does a commit
in all storages plugins, and each storage plugin is supposed to know if it is
repsonsible for that account, and do nothing if they aren't, right?
<smcv> xclaesse: yes
<xclaesse> ah, I understand
<smcv> xclaesse: however, feel free to change the system to
: MC remembers which single plugin is responsible for each account, and only
commits to that one
<xclaesse> I've set MCP_ACCOUNT_STORAGE_PLUGIN_PRIO_READONLY on the ring plugin
<xclaesse> but that's lower than MCP_ACCOUNT_STORAGE_PLUGIN_PRIO_DEFAULT
<smcv> ah, the priority system is probably a trap
<xclaesse> I should rather make -ring priority higher and just claim that it
commited
<smcv> it was designed when "accounts get split between separate account and
keyring backends" was thought to be a feature
<smcv> each account being in exactly one backend would be a more sensible
layout
<smcv> MC should create new accounts by going through backends in descending
order and saying "can you?"
<smcv> and for everything else it should save each account in the single
backend that it knows owns that one
<xclaesse> foreach (storage) if (mcp_account_storage_owns(storage, account))
{mcp_account_storage_commit(storage, account); break;}
<xclaesse> smcv, that's the correct way to do it now? ^
<smcv> looks sane
<smcv> might involve patching ring's and empathy's plugins so they actually
implement _owns
<xclaesse> so the priority's only purpose now should be to pick a storage to
create accounts, right?
<smcv> yeah, I think so
<xclaesse> ok, I'll check if I can cook a patch
<smcv> hmm, in fact, _owns might not even be necessary, if McdStorage stores
"this account belongs to" in each account
<smcv> in general, any simplification here is good
<xclaesse> hash table account-name -> storage
<xclaesse> makes sense
<xclaesse> if we have that, why do we have owns at all ?
<smcv> well, we already have a hash table account name -> McdStorageAccount
<smcv> but McdStorageAccount doesn't point to the plugin, because bees
<smcv> or something
<smcv> oh yeah I remember
<smcv> it's for backwards compat
<smcv> search for "FIXME: This is rather subtle, and relies on the fact that
accounts"
<smcv> it's because create() didn't originally exist
<smcv> xclaesse: ^ but now that we've broken API, we can just say "in the new
API, if you don't implement create, MC will never create accounts in your
plugin, deal with it"
<smcv> you might want to review
https://bugs.freedesktop.org/show_bug.cgi?id=71230
<xclaesse> smcv, ok, I'll check that, so if I add the storage in that struct, I
can remove iface->owns altogether, right?
<smcv> I think so

-- 
You are receiving this mail because:
You are the QA Contact for the bug.