[Bug 27271] API review: TplLogEntry, TplLogEntryText

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Cosimo Alfarano <cosimo.alfarano at collabora.co.uk> 2010-04-20 07:22:27 PDT ---
> Will code outside the library need to subclass TplLogEntry? If not, 
> we could move TplLogEntryClass to a log-entry-internal.h, reducing our
> API surface area.

No, subclassing is just for who wants to extend the logger, so only within the
logger until TPL will support plug-ins

I'll move the class to -private.

> #define TPL_LOG_ENTRY_MSG_ID_IS_VALID(msg) (msg >= 0)
> #define TPL_LOG_ENTRY_MSG_ID_UNKNOWN -2
> #define TPL_LOG_ENTRY_MSG_ID_ACKNOWLEDGED -1

> I don't like this overloading of message IDs. I think the log entry should 
> have a separate flag for whether the message has been acknowledged, 
> and perhaps also a flag for whether the message ID is meaningful.

the pending_msg_id bits are used only to be able to understand what, in the
pending message list, has been already logged, at TPL start up.

Yeah, it might confuse people when using such values in TPL and with different
semantic in tp-glib.

I'll add another method with a flag.


> gint tpl_log_entry_get_pending_msg_id (TplLogEntry *self);
>> Will API users ever care about this? It'd be good if this could be internal
>> to the logger and the library.

OK.

> gboolean tpl_log_entry_is_pending (TplLogEntry *self);
>> This only seems to make sense for Text messages? Perhaps it should 
>> only exist in the subclass?

ok.

> const gchar *tpl_log_entry_get_channel_path (TplLogEntry *self);
>> What's this for? Does anything use it? Channel paths aren't unique 
>>or meaningful over time, so it seems unwise.

I understand your point but I needed it. I think you suggested a similar
approach in the past, but if you have a way to remove it, I'd be glad to get
rid of lots of methods carrying transient of misleading data.

This method would only be internal.

I might use a separate object/struct and add
tpl_log_store_text_set_transient_data(), or similar, which might include both
pending msg id related data and the channel path.


The method is used by TplLogStoreSqlite to retrieve all the pending messages
for a channel and set by TplChannelText.

When I instantiate TplChannelText I prepare it with the following steps (among
other things):
- prepare the object's parent
- hook the signal
- check the pending messages need to be logged

Especially I need to know if any message in the pending msg list has already
been logged.

The only way to do it, not having a unique, trustable and always present
message id (at the time), was to create a tpl-log-id as result of a hash
function
tpl-log-id(channel_path, timestamp, pending_msg_id).

This function returns a hash which accomplishes uniqueness, trustfulness and
it's always present since it's TPL creating it (I might change it in the
future, after the tp-specs change about it). 

What I do, once obtained the tpl-log-id is storing it in a SQLite table,
together with its channel path, so that on channel Created notification, I have
a list of tpl-log-id for it to check.

The worse case happens when the created channel path matches one store in SQL
without being the same channel (ie re-utilised), in this case no tpl-log-id
would match so its just a useless comparison.

It's just a way to filter out data that are not interesting for me.

If I do not check against the couple (channel-path, tpl-log-id) but just look
for tpl-log-id, I'd retrieve much more log-ids (related to any channel),
slowing down the TplChannelText preparation.


For instance:

Pending message list: 1 2 3
SQLITE: log-id3 log-id4 log-id5
related to the same channel path.

I compute a tpl-log-id from the pending messages ids obtaining 
log-id1 log-id2 log-id3. -> I have to log only log-id1 and log-id2.


Pending message list: 1 2 3 -> log-id1 log-id2 log-id3
SQLITE: log-id4 log-id5

In this scenario I have nothing to log, I don't know if it's because I already
logged them all or because it's because they refer actually to two completely
different channels and I'm not interested in knowing it at this point.

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