[Telepathy] Semantics of the ChannelDispatcher API

[Simon McVittie]

On Fri, 03 Apr 2009 at 15:15:59 -0300, Andre Moreira Magalhaes wrote:
> Simon McVittie wrote:
> > On Fri, 03 Apr 2009 at 13:00:55 -0300, Andre Moreira Magalhaes wrote:

> All D-Bus methods can fail, some race condition for example, so enabling 
> them to fail is not something we should avoid IMHO.

This is true, but the question is whether it is useful to burden library users
with thinking "should I return an error or not? When?" for a particular method.

> Also the spec for Observer/Approver/Handler is still DRAFT, with some 
> FIXME/TODO's items, and the idea is that it can change at any time, so I 
> decided to make all methods able to fail, in case we add error support 
> in the future.

This is not how high-quality APIs are designed :-P

> Some methods probably don't need to be async, but we never know what the 
> app will do in order to implement the method, so allowing the app to 
> implement the method async is OK. Also it's better to make all methods 
> call finished/finishedWithError when finished then doing it case by 
> case, so we have a pattern.

That may be a reasonable point of view, though.

I think to design these APIs well, we need a better idea of application usage
and what the various operations *mean*. Below is some explanation, which we can
hopefully turn into text for telepathy-spec or telepathy-doc too - please read
and think about.

Regards,
    Simon

ChannelRequest
==============

A ChannelRequest encapsulates a request for a channel that has been started by
some client, but not yet completed.

A channel request can be cancelled by any client (not just the one that
requested it). This means that the ChannelDispatcher will Close() the
resulting channel (if necessary) rather than dispatching it to a handler.

The ChannelRequest also provides properties that indicate how the request
was made, when, etc.

I think that the C++ API for ChannelRequest should hide the Proceed method -
that method must be called by the client that made the request, exactly once
(it is wrong for any other client to call it), so the
library code that implements ensureChannel() and createChannel() ought to
do it. However, we do need to expose Cancel.

In fact, instead of being a DBusProxy, I think when we make channel requests,
the ChannelRequest should probably be represented by a PendingOperation
subclass (which should have a protected accessor for the
ChannelRequestInterface).

I'm not sure how that would affect telling handlers, approvers, observers about
channel requests (see below for why that is done).

Handler
=======

Handlers are the UI for a channel. They turn an abstract Telepathy channel into
something the user wants to see, like a text message stream or an audio and/or
video call.

Each channel should be in one of these states:

* being processed by the channel dispatcher
* being handled by precisely one Handler
* closed (not really a state, the channel no longer exists)

Basic, common usage of Handler is that HandleChannels is called, and the
handler accepts the channels and does something useful with them. After
HandleChannels has returned successfully, the Handler remains responsible for
the channels until they are closed.

Failing
-------

If HandleChannels fails, that indicates that the handler didn't and can't
handle the channels. The idea is that this means the channels will either
get passed on to another handler, or rejected (closed).

Notification of requests
------------------------

In addition to the basic functionality, handlers can be told about channel
requests for which the handler will "probably" later get asked to handle the
result. The handler can use this to open a new window/tab with a "please wait"
message, for instance. This is the AddRequest method.

If the request fails, or if it succeeds but ends up being handled by a
different process, the handler is notified by calling RemoveFailedRequest (if
the request ends up at a different process, the error code is NotYours). This
lets the handler close the window/tab or display an error notification.

If the request succeeds and is given to the expected handler, the handler
can use the Request path given to its HandleChannels method to match the
HandleChannels call up with a previously created window or tab.

(I'm starting to think this should be moved to a secondary D-Bus interface -
we could call it Client.Handler.RequestNotification - and then clients that
don't care won't be activated.)

These methods are implemented as methods, but they behave more like unicast
signals (we could have used unicast D-Bus signals, but there's really poor
binding support for them), and I don't think returning an error from them
should change the ChannelDispatcher's behaviour (the most it's likely to do
is to log the error message to stderr or syslog and carry on; it could even
send the method calls with the "no reply" flag).

Accordingly, I think AddRequest and RemoveFailedRequest shouldn't be allowed
to fail, in clients that use telepathy-qt4.

Perhaps instead of being virtual methods, these things should be Qt signals?

Bypassing approval
------------------

It's possible for a Handler to bypass Approvers (below) and directly steal
certain incoming channels. For instance, if my copy of
org.freedesktop.Telepathy.Client.Empathy is handling a video call from Rob,
it could add a second Client name (something like
...Client.Empathy._1_42.Window3) which matches text channels with Rob,
is a Handler, and has the "bypass approval" flag. That way, if Rob sends me
a Text message, I won't get flashing tray icons or other notifications - it'll
just quietly appear in the video-call window.

Crash recovery
--------------

Handlers are required to have some idea of what channels they're handling. This
is so a crashed channel dispatcher can recover the state of the system by
querying connection managers for all the channels that exist, querying
clients for all the channels they're handling, then dispatching all the
channels that are left.

This sort of information can also be used to let the channel dispatcher
auto-close channels that were being handled by a crashed client.

I think the current D-Bus API for this is rather half-baked. What we want is
something more like:

* each unique name (e.g. :1.42) has zero or more Client names (e.g. it might
  have Empathy, Empathy._1_42.Window1 and Empathy._1_42.Window2)

* Clients must keep at least one Client name as long as they are handling
  any channels

* the Handler.HandledChannels property for each Client name is the set of
  unclosed channels that its *unique* name is responsible for (so
  Empathy._1_42.Window1 and Empathy._1_42.Window2 have the same
  HandledChannels property)

* if a unique name that was handling channels either disappears from the bus
  or releases all its Client names, then the channel dispatcher considers it
  to have crashed, and closes or re-dispatches all the channels that it was
  handling

Approver and ChannelDispatchOperation
=====================================

Approvers are clients that ask the user whether some incoming channels are
OK to send to a handler, like the Empathy tray icon, or the answer/reject
window that pops up when you call a Maemo device.

Additionally, approvers can influence the choice of handler, if they want to.

Approvers and channel dispatch operations are two sides of the same coin.
The purpose of a ChannelDispatchOperation is to give an Approver something to
act on.

It's possible that channel dispatch operations should also look a bit like a
PendingOperation in C++.

Normal operation
----------------

An "approval" consists of calling AddDispatchOperation on every Approver.
If an Approver returns an error, that means it is somehow broken, and the
ChannelDispatcher should treat it as if it wasn't there; if every relevant
Approver is broken (or if there are none), then the ChannelDispatcher picks
an arbitrary handler and launches it.

If an Approver wants to accept a channel dispatch operation and pass the
channels on to the handler, it calls ChannelDispatchOperation.HandleWith.
If it doesn't care which handler is used, it can just take the first one
in the list (the channel dispatcher is meant to use some heuristic to put the
best handlers first).

All approvers are notified simultaneously; the first one to respond wins.
Subsequent attempts to approve the same channel will fail, probably with
NotYours.

When the channels are successfully handled, the channel dispatch operation
emits Finished and vanishes; the approver should stop notifying the user
(tray icon stops flashing, popup closes) and go away.

If a channel closes before it could be handled, the channel dispatch operation
emits ChannelLost, possibly followed by Finished if there are no channels left.

Claiming
--------

As a shortcut, approvers that are also handlers can call Claim. This is like
calling HandleWith(my_own_bus_name), except that HandleChannels will not be
called (there'd be no point).

Claim failing is just like HandleWith failing.

Rejecting
---------

At the D-Bus level, there is no "reject" operation. Approvers that want to
reject channels must Claim them and (if successful) close them. It would be
reasonable for us to provide a C++ shortcut for that.

Observers
=========

Observers watch channels going past. They're intended for things like a logger,
which needs to observe every text channel.

Normal operation
----------------

The observer is notified about every channel it cares about, by the CD calling
its ObserveChannels method.

Because it might take some time for an observer to get ready (for instance,
a Text logger needs to wait for TextChannel->becomeReady() to finish), the
channel dispatcher must wait (up to some timeout) for all observers to return
from ObserveChannels before letting anything destructive happen. Destructive
things are defined to be done by handlers, therefore HandleWith and Claim
aren't allowed to succeed until all observers are ready.

However, whether an observer succeeds or fails to get ready isn't very
interesting - a failed observer will just cause a message to be logged by
the channel dispatcher, probably. The only important thing is whether the
observer has replied or not.

We could perhaps do this quite nicely by having the "channel observation" be
a shared pointer that's passed to observeChannels(), and having ObserveChannels
return when object pointed to by that shared pointer is deleted? Or maybe
that's crack.

Notification of requests
------------------------

Like handlers, observers are told whether the channels they're being given
satisfy a particular request. An observer that is also a handler (perhaps
the logger is in the main Empathy process) could perhaps use that information.

[In the current telepathy-spec this is untrue, but I'm going to fix that, for
completeness]

Notification of CDOs
--------------------

For incoming channels, observers are also told what channel dispatch operation
the channels belong to. The observer can use this information to behave like
an approver, if it wants to (e.g. it might call Claim, although in this case
it needs to be careful to avoid deadlock with itself - it must return from
ObserveChannels before Claim can succeed!)

This is mainly useful because observers are invoked first, so a suitably
written observer can silence approvers by stealing channels for itself.