[Bug 27642] review for some refactoring plus chatroom logging fix in TPL

[bugzilla-daemon at freedesktop.org]

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

--- Comment #5 from Cosimo Alfarano <cosimo.alfarano at collabora.co.uk> 2010-04-29 09:28:35 PDT ---
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.