[Telepathy] Review of MissionControl spec

[Simon McVittie]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

OK, I've actually started on this now...

One thing I notice here is that we're conflating two (or more) things which
are, in principle, completely separate.

* Account management. Telepathy components need to be able to keep track
  of what accounts are available to us. This is the part that needs to
  care about connections that aren't actually connected, connection
  managers that aren't actually running, and so on.

* Dispatching channels. So we've got a channel up and running. What now?
  Do we launch an Empathy chat window, or what?

(I note in passing here that the term "Chandler" must die. A chandler
is a maker of candles, or a dealer in specialist (usually ship-related)
equipment - nothing whatsoever to do with IM. If it survives in our
API, we should capitalize it as CHandler, but I'd much prefer
ChannelHandler. Let's keep the puns for project naming rather than
public API please.)

Related to dispatching channels is the concept of filtering. I'm
somewhat fuzzy on what filtering, in fact, does: perhaps a Nokian could
explain it to me, since I gather it's used fairly extensively in the
current NMC.

Anyway, in principle there'd be nothing to stop these being entirely
independent services, and I'm not convinced that we want to force there
to be one process that does everything NMC does today.

I think it would also be instructive to look at the proposed API from the
point of view of "can we do this without MC?" rather than "might someone
find this useful?". A lot of the API seems to provide short-cuts for
things you can do right now, using existing Telepathy API. Perhaps some
of it would be better implemented in libraries rather than as a D-Bus
service; having just spent around 3 months on client API in
telepathy-glib, I'm open to the possibility of making a client library
that doesn't suck, rather than bouncing everything through a daemon
because libtelepathy is too hard.

Now for the actual API, referring to Xavier's current proposal on merge
monkey:

In general
==========

The specification really needs to explain the distinctions (if any) between
default, actual, global and current presence.

Expected error behaviour is completely missing.

There's an awful lot of spec of the form GetBadger, SetBadger, BadgerChanged.
I imagine George and Xavier got very bored while writing this. If we
could code up a better Properties interface, we'd instantly obsolete
about 60% of the text.

Not coincidentally, I've been pondering the value of standard
D-Bus Properties (possibly with extensions) for Telepathy in general - I think
it could be extremely useful for reducing the amount of boilerplate in the
spec, and using GetAll() would save us round-trips too.

Account
=======

GetConnection() -> s, o
Documented as returning (s, o) "if it's online". What happens if it's
offline needs documenting too.
The bus name and object path are theoretically redundant - you can derive
either from the other, if we constrain the connection to be one that obeys the
Telepathy spec - but it would save clients a round-trip to the bus
daemon if we mentioned the *unique* bus name (e.g. :1.35) here.

GetConnectionStatus() -> u / signal ConnectionStatusChanged(u, u)
Do we really need this? Connections already signal it. If we had a
robust way to go from an Account to a Connection, and from a Connection
to an Account, clients could just do a wildcard signal match on
StatusChanged (ignoring for the moment dbus-glib's inability to do such
things - this is the wonderful world of open source, we could just add
that code to dbus-glib and stop working around its absence.)

RequestChannel(s: ctype, s: htype, u: handle)
This method seems to me to be entirely useless - either we make it take
a string contact ID (the same thing you pass to RequestHandle()) as Mika N
wants, or we kill it. If you have a handle, then presumably you know which
Connection you got it from, and that that connection is still alive (and if
you don't, then you should re-read the Telepathy spec, paying particular
attention to the part where we explain that handles are only valid for a given
connection).

(Asides: Perhaps we need to spell out in the Telepathy spec "When a
connection closes, you must forget about all its handles. Immediately. No,
really, we mean it"; and after some thought about this, I've filed
https://bugs.freedesktop.org/show_bug.cgi?id=14188 with the intention
that it blocks release of telepathy-glib.)

signal Deleted() ?
It would perhaps be helpful if Account had a Deleted signal too, but
failing that, making correct use of an Account dependent on its
AccountManager would be no big deal.

UpdateParameters() -> a{sv}
I'm going to contradict everyone and say I think we should have this
*and* SetParameters(). A typical accounts widget a la Empathy or
osso-accounts-ui sets all parameters simultaneously, so the appropriate
thing to do is probably in fact an atomic overwrite (Set). If you're
silly enough to press OK in two accounts dialogs simultaneously, the
second one wins. However, if we have some sort of migration tool or
something (perhaps a tool that goes through all your accounts removing
deprecated parameters, or setting require-encryption) it might want to
set only the parameters it cares about, to reduce races.

UnsetParameters(as)
Do we really need this, or is setting a parameter to its CM (or preset)
default equivalent to unsetting it? We just don't know. Something to think
about.

On the subject of parameters: how do we cope if we have a value
specified for a parameter the CM no longer supports? How do we cope if
we have no value for something the CM now needs? Probably, disable the
account and notify the user. The former case is tricky: most of the
time, the user will be irritated, because the correct resolution is
"obvious" and they'll resent having to say "OK". However, if the parameter
being removed (from Gabble) is "old-ssl", silently removing it would
make their connection no longer secure. Loss.

The solution here might be "well, don't do that" - deprecate obsolete
parameters but never remove them. I suspect this would cause Will (of
telepathy-haze fame) many headaches, though.

AccountManager
==============

Profiles are dead, long live presets. Much rejoicing.

CreateAccount / CreateAccountFromPresets
Before we resort to libdbus naming conventions right away
(dbus_connection_read_write_dispatch_and_also_make_me_a_sandwich)
could we perhaps have CreateAccount(s: CM, s: protocol, s: preset, a{sv})
where preset can be empty? We could even consider making preset names only
unique per (CM, protocol) pair - maybe install them like
"/usr/share/telepathy/presets/gabble/jabber/gtalk.preset"?

GetActualStatus
I understand vaguely what this is - it's the status that Empathy
displays. I'm not sure exactly what it's *for*, though (Maemo seems to
get by fine without it).

RegisterChandler
No. Wrong interface. Try again.

ChannelHandler
==============

The patch "New Chandler system" completely rewrites the API. It's not
clear to me which one is better or why, and the patch naming doesn't
give me much help.

I think we need to get a clear picture of what's going on with Tubes and
File Transfer before we go much further with this.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHlRHmWSc8zVUw7HYRAsUrAKDiSrQ1JbwhpgmexbZlRtMcoWaFMQCeN9s/
AaC3TGVPSMqXu0xmUwssD7k=
=6C/P
-----END PGP SIGNATURE-----