[Bug 27269] API review: TplContact

[bugzilla-daemon at freedesktop.org]

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

--- Comment #9 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-05-26 06:16:34 PDT ---
(In reply to comment #7)
> For now we should focus on reviewing the API used by Empathy and make the rest
> private.
> Empathy uses the following contact API:
> 
> tpl_contact_get_alias ()
> tpl_contact_get_avatar_token ()
> tpl_contact_get_contact_type ()
> tpl_contact_get_identifier ()
> 
> Any comment on this API?

My major objection is that TplContact doesn't actually seem to be a contact!

  36 /* TplContactType:
  37  *
  38  * @TPL_CONTACT_UNKNOWN: the current contact's type is unknown
  39  * @TPL_CONTACT_USER: the contact's type represents a user (buddy), but
not
  40  * the the account's owner for which @TPL_CONTACT_SELF is used
  41  * @TPL_CONTACT_GROUP: a named chatroom (#TP_HANDLE_TYPE_ROOM)
  42  * @TPL_CONTACT_SELF: the contact's type represents the owner of the
account
  43  * whose channel has been logged, as opposed to @TPL_CONTACT_USER which
  44  * represents any other user
  45  */

Either it should be renamed to something that reflects its real purpose (which
KA explained in <https://bugs.freedesktop.org/show_bug.cgi?id=27642#c5>,
although I didn't fully understand the explanation), or it should only be used
for genuine contacts.

In Bug #27642, KA wrote:
> I agree with the renaming of TplContact into TplEntity or similar, I realized
> it when adding TPL_CONTACT_GROUP semantic, but for doing that I really need
> that most of the important pending branches are done.
> 
> This currently might means three branches including the misc-ka.
> 
> > Can tpl_contact_from_chatroom_id be internalized?
> > (_tpl_contact_from_chatroom_id)
> 
> Yep, this was the plan, but since it was introduced in misc-ka, I was waiting
> to be merged for the renaming. contact-internal.h does not exist "yet".
> 
> 
> > To be honest I'd be inclined to define these types in terms of handle types,
> > and say "a named chatroom (%TP_HANDLE_TYPE_ROOM)" if that's what you mean.
> 
> It's that yes. I'll also allow the empty string for a Room name.
> 
> 
> Most of the changes suggested by you, which involve renaming of symbols, I'd
> like to apply on the API review stage.
> 
> I have already a list of issues I'd like to rename/change
> 
> 
> > +  return (tpl_contact_get_contact_type (sender) == TPL_CONTACT_GROUP ||
> > > +      tpl_contact_get_contact_type (receiver) == TPL_CONTACT_GROUP);
> > 
> > What would it mean to have a message sent by a group? How would that be
> > represented in Telepathy? Unless the API is really screwed up, I don't think
> > this can happen?
> 
> 
> When A TplContact (or maybe-TplEntity) is set to TPL_CONTACT_GROUP
> (maybe-TPL_ENTITY_ROOM) it means that the TplLogEntry related to the
> contact/entity has been sent from/to a ROOM.
> 
> 
> A TplLogEntry has three important members:
> - sender (TpContact in tp-glib)
> - receiver (TpContact in tp-glib)
> - chat_id (which usually corresponds to the remote identifier or room id but it
> doesn't need to be, it depends on the TplLogStore implementation)
> 
> 
> Before:
> When on 1-1
> - sender and receiver are set to a proper TplContact
> - chat_id = remote id or similar
> 
> When on ROOM
> - the sender or the receiver is NULL (the remote one), the other one is a
> TplContact for the local user.
> - chat_id = room id or similar
> 
> In normal cases, chat_id is the remote/room id.
> 
> chat_id is, in practical terms, an identifier for the chats with the couple
> (local id,remote id) behind a CM (which for TPL translates in a generic name,
> since pidgin for example calls it "jabber" and not "gabble_jabber" as
> Empathy/TP do, some other client might not even have a proper name for it).
> 
> Empathy: .local/shares/TpLogger/logs/gabble_jabber_foo_40jabber_2eorg0/
> Pidgin: .purple/logs/jabber/foo at jabber.org/
> InventedClient: .invented_store/logs/jabber/1ab321db/
> 
> foo at jabber.org in the first two examples would the the chat_id.
> 1ab321db would be the chat_id in the third.
> 
> The three would represent the chats occurred between the user and a single
> remote identifier (foo at jabber.org) using jabber/gabble_jabber.
> The third one is not informative about the remote name.
> 
> What does it happen when a logstore has the chat_id != remote_id/room_id?
> 
> If TPL assumes that the chat_id and the remote identifier have the same value,
> an information might be lost.
> I need a way to store both a chat_id and a real remote identifier.
> 
> The solution I found is change the former semantic of TplContact to something
> you identified as TplEntity.
> 
> In this way, I might have a redundant information since it can be
> chat_id == tpl_contact.identifier but at least I do not lose it.
> 
> This translates in:
> 
> When on 1-1
> - sender and receiver are set to a TplContact and TPL_CONTACT_{SELF,CONTACT} is
> also set.
> - chat_id = as before
> 
> When on ROOM
> - sender and receiver are set to a TplContact, and TPL_CONTACT_{SELF,GROUP} is
> also set.
> - chat_id = as before
> 
> 
> 
> The alternative to this way is adding a further member calling room_id, which
> would be NULL on 1-1, while be set to the real room name/id on TYPE_ROOM with
> the remote TplContact (representing only a TYPE_CONTACT) set to NULL.
> 
> I found the TplEntity way easier, simplifying the what it can happened:
> always sender and receiver are set, instead of one being NULL depending on the
> situation and another member set instead.
> Also, it makes possible handling different cases "1-1 vs ROOM" as similar,
> which actually they are, from a logger point of view.
> 
> 
> Answering to your question, I'm not sure how it might translate in TP, I'd say
> it should be associated to an extended TPL/loggable version of TpHandle:
> 
> With a Text channel, I receive a remote handle being TYPE_ROOM when on ROOM or
> TYPE_CONTACT when on Contact.
> It represents a ROOM also, which TpContact does not.
> 
> One thing I'm sure of, TplContact has a wrong name, it would be associated to
> TpContact, which is not and should be renamed ASAP.
> 
> 
> 
> > Later
> > =====
> 
> > Contact IDs and room IDs are not necessarily distinguishable, or even
> > different. In XMPP, they have the same syntax (they're just JIDs).
> 
> True, I've already taken care of it.
> 
> > I would like the logger to not fail if/when we discover a protocol where
> > usernames and chatroom names can overlap (e.g. a user "smcv" can be in a
> > chatroom also called "smcv"). I'm fairly sure such a protocol exists, 
> > somewhere in the world.
> 
> 
> A TplLogEntryText can understand if it's handling a ROOM or not depending on
> its internal state.
> Checking whether the receiver or the sender is set as TPL_CONTACT_GROUP (using
> the current naming).
> 
> In facts, a ROOM is logged in a different place, so that a identifier clashing
> would not be a fail.

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