[Bug 27270] TplLogManager: clean up API

[bugzilla-daemon at freedesktop.org]

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

--- Comment #16 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-01-07 11:57:48 PST ---
>diff --git a/telepathy-logger/log-manager.h b/telepathy-logger/log-manager.h
>@@ -84,39 +92,39 @@ gboolean tpl_log_manager_get_dates_finish (TplLogManager *self,

> -gboolean tpl_log_manager_get_filtered_messages_finish (TplLogManager *self,
> +gboolean tpl_log_manager_get_filtered_events_finish (TplLogManager *self,
>     GAsyncResult *result,
>     GList **messages,
>     GError **error);

>-gboolean tpl_log_manager_get_filtered_messages_finish (TplLogManager *self,
>+gboolean tpl_log_manager_get_filtered_events_finish (TplLogManager *self,
>     GAsyncResult *result,
>     GList **messages,
>     GError **error);

Need to change GList **messages into GList **events

>diff --git a/telepathy-logger/log-manager.c b/telepathy-logger/log-manager.c
>@@ -383,36 +382,34 @@ _tpl_log_manager_register_log_store (TplLogManager *self,
>+ * Checks if @id exists for @account and is of type @type.

In this case can we have multiple types? Does it mean it exists for all type,
or one of ?

>@@ -520,18 +515,18 @@ log_manager_message_date_cmp (gconstpointer a,
>   one_time = tpl_entry_get_timestamp (one);
>   two_time = tpl_entry_get_timestamp (two);
> 
>-  /* return -1, o or 1 depending on message1 is newer, the same or older than
>+  /* return -1, 0 or 1 depending on message1 is newer, the same or older than
>    * message2 */
is newer -> being newer? and message -> event?

> gint
> _tpl_log_manager_search_hit_compare (TplLogSearchHit *a,
>     TplLogSearchHit *b)

There is something weird about this function now, and removing the
documentation does not help much.

>+      if (a->type == b->type)
>+        return 0;
>     }

This function seem to be used to sort the result, so it must have a finit order
(whatever the order is). Maybe you should do that instead:

  if (a->type < b->type)
    return -1;
  else if (a->type > b->type)
   return 1;
  else
    return strcmp(a->id, b->id);

Take note that I don't waste time doing strcmp() if type mismatch.

>diff --git a/extensions/Logger.xml b/extensions/Logger.xml
>@@ -82,6 +82,10 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.</
>+      <!-- FIXME: we're returning a list of events that can be
>+           messages (chats), but also calls and any other event
>+           type in the future, so make the return type a list of
>+           TplLogSearchHit or something generic than a(ssx) ? -->
>       <arg direction="out" name="Messages" type="a(ssx)"
>            tp:type="Chat_Message[]" />

I think we need aa(sv). Some field will be shared between events and other
specific (which will have to be documented in the protocol). But that can be
kept for when we move from TplSearchHit to TplEvent (TplEvent will simply have
to know how to serialise/deserialise itself into a(sv).

>diff --git a/telepathy-logger/log-store-internal.h b/telepathy-logger/log-store-internal.h
>@@ -63,21 +63,22 @@ typedef struct
>   gboolean (*add_message) (TplLogStore *self, TplEntry *message,
>       GError **error);

Is this suppose to be add_event now ?

>diff --git a/telepathy-logger/log-store-xml.c b/telepathy-logger/log-store-xml.c
>@@ -1042,6 +1043,7 @@ log_store_xml_search_in_identifier_chats_new (TplLogStore *store,
>   g_return_val_if_fail (!TPL_STR_EMPTY (text), NULL);
> 
>   account_dir = log_store_account_to_dirname (account);
>+  /* FIXME: take @type into account for the dir. */

Is it something you forgot, or it's not needed for now ?

>@@ -1120,35 +1122,35 @@ log_store_xml_get_chats_for_dir (TplLogStoreXml *self,
>-/* returns a Glist of TplEntryText instances */
>+/* returns a Glist of FIXME TplEntryText instances */

As this one used TplEntryText (sub-class of TplEntry) it should be very easy to
fix, s/TplEntryText/TplEntry/ ;). Unless you wanted this note for
implementation of Call support ?

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