[Telepathy] On spec additions to allow a really simple IM API

[Olli Salli]

On Thu, Jul 1, 2010 at 10:26 PM, Will Thompson
<will.thompson at collabora.co.uk> wrote:
> Hello,
>
> I would like us to be able to implement this convenience API:
>
>  class SimpleTextThingy {
>    public:
>      SimpleTextThingy(Tp::Contact &contact);
>
>      /* Send a message to contact, optionally spawning/presenting the
>       * UI for the channel. */
>      void sendMessage(Tp::Message &message, bool presentUI);
>
>      /* Present a conversation window to contact, with a message ready
>       * to send. Think "share link with...": you want to give the user
>       * the chance to edit the message before sending it. */
>      void presentMessageForSending(Tp::Message &message);
>
>    public signals:
>      void messageSent(Tp::Message &message);
>      void messageReceived(Tp::Message &message);
>  }
>
> The signals are easy: just be an observer for Text channels to contact.  The
> methods are harder. Simon and I talked about them today.
>
> If we add a variation of the ChannelRequest.Succeeded signal which also
> gives you the channel path, this lets us implement sendMessage("foo", true),
> namely popping up the conversation window to that contact (by calling
> EnsureChannel() and letting the default handler get it) and then poking a
> message into the channel. Of course there's a race here: if the handler
> Close()s the channel before you call SendMessage(), you lose. Hack: the
> library could just try again, but that would probably piss the user off.
> This will happen only rarely in practice, but it would still be unpleasant.
>
> sendMessage("foo", false); - that is, without presenting the normal UI
> whether or not it's already open — is more problematic. If you use
> ChannelDispatcher.CreateChannel() with yourself as the preferred handler, it
> fails if the user has a conversation window open to that contact. If you use
> CD.EnsureChannel(), it refocuses the conversation window if there is one.
> Options, none of which I like:
>
> • We could add Connection.SendAMessage(). This is ugly, and would break
> loggers logging outgoing messages. Vetoed.
> • We could add send-only text channels, which you can always Create
> regardless of whether there's another channel going on. No incoming messages
> are reported by them; you can just call SendMessage(). MessageSent fires on
> the send-only channel, as probably do delivery reports for the messages you
> send on that channel. Incoming messages don't show up.
> • We could allow more than one fully-functional text channel to a contact,
> with MessageSent and MessageReceived firing on all of them. This involves
> loggers de-duplicating messages, but they kind of have to do that anyway.
> • Your really clever option here?

Andre pointed out on IRC that the Channel Dispatcher implementation is
in a rather good spot to perhaps implement this without any CM
modifications.

In essence, CD would implement a method, say
CD.I.Messages.Send(s:TargetID, s:message, <and perhaps some misc args
for flags, and maybe throw an a{sv} there just for the kicks>), in the
following way:

- EnsureChannel a suitable text channel for sending the message to the recipient
- Send the message on the resulting Channel
- If sending failed, which in particular can happen at least when
Yours wasn't true and somebody else Closed it when we tried to send,
just request the channel again, and try sending again

The resulting channels shouldn't be dispatched to any Handlers, but
they should be dispatched to Observers for logging purposes. However,
there's a problem: what if somebody makes a request to the CD which
should return a channel we're currently using internally for sending a
message? This can perhaps be solved:

- Have CD always issue a EnsureChannel to the CM, if the requested
ChannelType was Chan.T.Text (even if CreateChannel was used)
- If that returned an object path we're not using internally, then all
is fine: just dispatch to Observers, a Handler and and
CR.SucceededWithChannel() normally
- OTHERWISE: don't Succeed the ChannelRequest or dispatch the channel
to Handlers (Observers have it already!) until we're done sending
through that Channel, at which point dispatch it normally - though
again, note that Observers have it already at this point
- If nobody made a request matching a Text channel we were internally
using while we were using it, just Close it to prevent leaking it

This depends on a few things:
- If it is safe to implement CreateChannel in the CD as Ensure + not
Closing - the CD knows that it's the only other user of the Ensured
channel (unless somebody else is directly poking the Connection, but
that's broken anyway), so it could be the only one Closing it, but it
can avoid that because it knows of the new request
- If it is safe to Close the channel when we're done if nobody
requested it in the meantime: It should be, as if we use Close instead
of Destroy, if there were incoming messages the Channel should then be
closed and redispatched, right?
- If it is problematic that the Channel might be redispatched after
Closing with its pending message queue intact, namely for loggers not
wanting duplicate events? We haven't given the Channel to a Handler
which could ack the messages beforehand, that is.

Is there any other obvious race or misc problem I'm not seeing here?
Also, do you think this would complicate the CD implementation too
much (or be in fact impossible)? I'm fine with send-only Channels -
they'd solve the issue equally well, and without any CD changes, but
would in turn require changes in ALL of the CMs, and non-trivial
tp-glib support, which looks problematic at this point to say the
least. Not to mention the fact that "send-only" channels are an
awkward concept at best. Then again, so is bolting a message sending
method to the otherwise generic CD, just because it has some extra
knowledge to make that slightly less distantly possible.

>
> presentMessageForSending is actually the easiest of the bunch: if we allow
> associating hints with channel requests —
> https://bugs.freedesktop.org/show_bug.cgi?id=28866 — then we can just define
> a key which means "oi, present this message for sending".
>
> Thoughts very welcome,
> --
> Will
> _______________________________________________
> telepathy mailing list
> telepathy at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/telepathy
>



-- 

Br,
Olli Salli