[Bug 54198] Logger should read logs from superseded accounts as well

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from Xavier Claessens <xclaesse at gmail.com> 2012-08-30 14:31:43 UTC ---
(In reply to comment #2)
> >-  const char *account_name = get_account_name (account);
> >+  const gchar *account_name = account_path +
> >+      strlen (TP_ACCOUNT_OBJECT_PATH_BASE);
> 
> Please add an assert to check that account_path is long enough. (multiple
> places)

Done.

> >log_store_xml_exists (TplLogStore *store,
> 
> Ishh, that synchronous function becomes worst then ever, hopefully it won't
> heart to much the UI performance.
> 
> >+          if (g_list_find_custom (ret, new->data,
> >+                (GCompareFunc) g_date_compare))
> >+            g_date_free (new->data);
> >+          else
> >+            ret = g_list_insert_sorted (ret, new->data,
> >+                  (GCompareFunc) g_date_compare);
> 
> We have had serious performance issues in the past. In this case there is
> faster solution, like inserting everything once, and removing the duplicates in
> 1 pass. Second option is to implement an _insert_sorted_and_unique() method in
> util.c, so it can be reused all over. This one is probably the optimal
> solution.

It should be using GSequence, really.

> >+          ret = g_list_insert_sorted (ret, new->data,
> >+                (GCompareFunc) _tpl_event_timestamp_compare);
> 
> Use of insert sorted with events is wrong as it reverts the order of event with
> equal timestamps. See util.h for an insert_after, you'll have to use queues.

It would be hard to have equals, since msg are coming from different accounts
that never existed at the same time.

Note that in _tpl_log_manager_get_events_for_date() it does not even sort the
result :/

> Finally, this won't go through without rigorous unit tests. Thinking of it, I'm
> not sure that superseded account API really works. It seems you can break it by
> creating a second time the same account and removing the old one. The resulting
> supersedes will not match.

supersedes is used for account migration, for example when deleting a butterfly
account and create a new haze account. It could be that the new account won't
reuse the old account's path. That's why in that case we set superseded to tell
logger we still want to use old account's logs.

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