[Bug 27269] review and clean up TplContact public API

[bugzilla-daemon at freedesktop.org]

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

--- Comment #13 from Cosimo Alfarano <cosimo.alfarano at collabora.co.uk> 2010-06-02 10:24:52 PDT ---
Simon, the example you did for the log entry is right.

Simon said:
> The receiver isn't really a property of the log entry: I'd say that
> conceptually, it's more like a property of the conversation. A given
> conversation is directed to the TargetID of its channel, which can either be a
> contact (self or remote), a named chatroom (as seen in XMPP/IRC), an ad-hoc
> chatroom (as seen in MSN/Skype), or some other thing that hasn't been defined
> yet.

The receiver for TPL needs to be a bit fat.
It needs to store Alias and AvatarTokens for each log entry.

This makes things a bit harder/huglier, but helps remembering historical
context (see a comment later)

> 2) Get rid of non-contact functionality from TplContact, and turn the type into
> a simple boolean, is_self. 1-1 messages have a non-NULL receiver contact. Other
> messages have a NULL receiver contact; the receiving chatroom can be inferred
> in some way from the message (via the chat ID if its type becomes
> better-defined, perhaps).

> 3) Get rid of non-contact functionality from TplContact, and turn the type into
> a simple boolean, is_self. Get rid of the receiver property too, and relate the
> message to a first-class Conversation object, which either has a pointer to a
> TplContact (called "target_contact" or something), or a way to get the chatroom
> name.


The issue with this approach 2 or 3 is that it might have problems with non
Telepathic LogStores, as far as I can see.
3 does not remember historical contexts as well.

Consider the logger importing and browsing logs which are not from Empathy/TP,
but from any other client from which TPL cannot infer the receiver from the
chat_id.

As I wrote elsewhere TPL stores data in a tree like:

* <cm>
** <proto>
*** <chat_id> (the remote room/buddy id)
**** <date>
***** <log entries>

Pidgin does something similar, but TP cannot infer the CM, since in libpurple
there is no such concept.

What does it happens if a client stores its logs using a hash for <chat_id>?
Most likely, I cannot infer the receiver or the room id from the hash.
I need to obtain it explicitly.

TPL plays on a wider field than TP, unless we decide to limit its
potentialities.

I'd rather to have a bit more properties, which IMHO do not create complexity
in the API, than denying the possibility for TPL to have a TplLogStore able to
understand a client storing logs as the example above.

Which means, I'd be for proposal 1, the one you like the least:
Renaming TplContact and using SELF, CONTACT, ROOM as contact types.

Another advantage of it is that allows to have all the info E. stored before
TPL, including historical contexts.


> (1) is the easiest, but is the one I like least. What would get_alias(),
> get_avatar_token() mean for a chatroom?

true.

For it I don't really have a solution. Either:

* add a GInterface for basic methods, considering Alias and Avatar "extra"
methods just for CONTACTS (see my comment below about historical contexts for a
use case, in any other case I don't see their utility).
* remove such methods from TplContact, dropping the historical context case
below.
* accept them to be NULL


> Another question this API gives me is: what is the scope of a TplContact? It's
> not clear to me which of these would typically be true:

Shortly:

Empathy log store stores the full name of the contact in the XML, along with
the id and the avatar id. In each log entry.

This means that each entry might have a different value for Alias and AvatarTok
and is a different TplContact.

> * Each conversation with Cosimo represents him as a different TplContact.
> get_alias(), get_avatar_token() return the most recent details from that
> conversation. I have to use get_identifier() to relate the contacts to each
> other.
> 
> * Each visually distinct version of Cosimo represents him as a different
> TplContact, i.e. whenever get_alias() or get_avatar_token() changes from one
> message to the next, the TplContact changes. I have to use get_identifier() to
> relate the contacts to each other.

If I understood correctly what you mean, it's the last.

Also, two objects having the same data are actually different instances. But
this can be changed.

The TplContact data is retrieved from each LogEntry, which holds such info in
its XML (assuming LogStoreXML, which will change eventually in a compatible
way).

This results in the E. log viewer (with the right theme) to have N entries from
Cosimo being actually N different TplContact and having a different "mutable"
property (avatar, alias).

The id is the only property which can be used to understand the relation.

> Why are the alias and the avatar token in the log? Is it basically a tiny
> version of persistent contact storage (analogous to libfolks or osso-abook), to
> be dropped in some future version?

Empathy log reminiscence, for backward compatibility, but it makes sense to me.

> The avatar token doesn't seem very useful when you can only get a cached avatar
> via a TpContact - we'll probably have to add API to telepathy-glib to see
> whether a cached avatar is still available, and if so, return them. This will
> require the connection manager name and the protocol name (both derivable from
> the TpAccount name), and can't be done for non-Telepathy avatars.


Originally I was thinking of caching avatar pics also, it's still an idea,
since it's not the scope of libfolks caching old information.
If I am not mistaken, currently empathy do some caching.

For non-telepathy LogStores, probably it won't be possible to have historical
contexts, depending on the way the log storing is done.

Why keeping avatar and alias in the log entries?
What does it happen if Cosimo changes his avatar with a funny pic and comments
with simon his new avatar, then changes it back?
The context of the log is not just the body, sender and receiver, but also the
avatar or alias. That's what I call historical context.

There is a (small, being really corner case) needing to record the context,
especially when it comes for free and does not really need much storage space.

Currently though, the avatar is not stored, only the avatar id, relying on
Empathy to remember it.

> Perhaps a better way to do the avatar would be to keep the avatar token, CM
> name and protocol name behind the scenes, and have:
> 
>     GFile *tpl_contact_get_avatar (TplContact *contact);
> 
> which can return NULL, and would be implemented in terms of new telepathy-glib
> API, something like:
> 
>     GFile *tp_account_get_cached_avatar (TpAccount *account,
>         const gchar *avatar_token);

That's OK.
I'd store it anyway, though, so that everything is self contained in the log
store dir (copying $XDG_DATA/logs/TpLogger somewhere else would remember the
avatars).

> > TplContact *tpl_contact_from_room_id (const gchar *chatroom_id);
> > TplContact *tpl_contact_from_tp_contact (TpContact *contact);
> 
> Shouldn't these be internal?

internalized in my tpl-channel-refactored branch, which now is rebased agaist
current master.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.