[Bug 27175] Make TpMessage usable in clients

[bugzilla-daemon at freedesktop.org]

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

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-10-20 05:17:05 PDT ---
(In reply to comment #5)
> Most of the tp_message_* API makes sense as a base class except:
> tp_message_new() : as it takes a TpBaseConnection

Yeah, we should add tp_cm_message_new() or something, and eventually deprecate
tp_message_new().

> tp_message_take_message() ? not sure.

I don't think clients will ever use it, but it's relatively harmless.

One subtlety is that it needs to check that the taken message has the same
handle-reffing semantics as the parent message.

> > > > - handle refcounting works differently in the CM and in a client
> > > 
> > > Yeah, which make me think it could be cleaner to have 2 objects.
> > 
> > It's a sad time that the TpMessage name is already in use...
> 
> What should we do?
> 
> Make TpMessage the base class and have TpCMMessage (or TpSvcMessage?) and
> TpClientMessage (or TpCliMessage?) ? Then we have to document than the methods
> above can only be used with CM messages but are named that way for historical
> reason.

I think this is the way forward. It'd be extremely confusing if there's a
structure called TpMessage that isn't the right thing for clients to use.

I think I'd call them:

TpMessage
  |
  \--- TpCMMessage (used by TpMessageMixin)
  \--- TpClientMessage (if needed, or just use the base class?)
         \--- TpSignalledMessage
              (or TpReceivedMessage, but it's used for MessageSent too)
              (has a TpContact for the sender if possible)
  \--- TpSavedMessage (the handle-less version, for the logger)

Let's reserve the tp_cli, tp_svc naming for the auto-generated stuff.

I think there should be an absolute minimum of "type-specific" API; in
particular, the constructors should all return a TpMessage *.

I'm not sure whether methods like get_sender_contact should take a TpMessage *
or a TpClientMessage *, and whether they should be tp_message_get_sender() or
tp_client_message_get_sender().

(In reply to comment #4)
> There are three uses for a TpMessage:
[...]
> - In a CM. We can ref handles of any type, synchronously. Handle fields like
> parts[0]['message-sender'] are essential.

This breaks down into:

- Received message: the CM builds up a Message based on protocol information,
then passes it to the mixin to put it on the "pending messages" queue. The
mixin needs to stamp it with a pending message ID, so it must either be
mutable, or be moderately cheap to copy into a mutable form.

[This is TpCMMessage.]

- Sent message: telepathy-glib gets an aa{sv} from D-Bus, builds a Message, and
gives it to the CM. The CM must give the mixin back a message that corresponds
to what it actually sent, which may differ (e.g. if it got truncated, the
protocol always sends a particular Unicode normalization form, the protocol can
only send latin-1, etc.), so the CM needs to modify or copy+modify it.

[This is also TpCMMessage. I don't think we need a separate object.]

> - In a client dealing with a live Channel. Handles are reffed for as long as
> the message is unacknowledged, so we need to make sure we have a ref before
> acknowledging. telepathy-glib only makes this easy for contact handles.
> TpTextChannel can't do this generically, because it doesn't know which uint32
> keys might be a handle, and if so, which type they are.

This breaks down into:

- MessageReceived or MessageSent signal from D-Bus: the Channel gets an aa{sv}
from D-Bus, refs all applicable handles by building Contact objects, builds a
Message and passes it up to the UI. Ideally, I'd also like to define a
message-sender-id field, and copy the message-sender's ID to that field. The
constructor can be behind-the-scenes: I'd suggest making TpMessage and its
subclasses non-subclassable outside telepathy-glib, like TpContact.

[This is provisionally called TpSignalledMessage above.]

- Sending a message: the client has to construct a Message from scratch, and
pass it to a method. It doesn't need to do any handle refcounting at the
moment, because the sender is implicitly "me" and you're not allowed to set it
explicitly. If we forbid further handle-based Message fields, we'll be able to
avoid handle refcounting altogether, but perhaps the constructor ought to take
a TpConnection or even a TpChannel anyway.

[This is provisionally called TpClientMessage above, or perhaps the TpMessage
base class is enough.]

> Perhaps the solution is to say that message-sender is the only handle that will
> ever appear in a Message_Part, and deprecate tp_message_set_handle() in favour
> of a new tp_cm_message_set_sender()?

I'm very tempted to do this. Any thoughts?

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