From 7818d410e868a1bd8a42eb0de1a754387561cedc Mon Sep 17 00:00:00 2001 From: Debarshi Ray Date: Fri, 31 Aug 2012 14:18:43 +0200 Subject: log-walker: Run the filter synchronously in the walker This ensures that the TplLogEventFilter is always run from the same thread which invoked the walker. This is implemented by keeping track of skipped events in the history instead of silently ignoring them within the LogIters. This has the nice side effect that we do not need to run the filter while rewinding. Fixes: https://bugs.freedesktop.org/54270 --- telepathy-logger/log-iter-pidgin-internal.h | 5 +- telepathy-logger/log-iter-pidgin.c | 58 ++------------- telepathy-logger/log-iter-xml-internal.h | 4 +- telepathy-logger/log-iter-xml.c | 59 ++------------- telepathy-logger/log-manager.c | 6 +- telepathy-logger/log-store-internal.h | 6 +- telepathy-logger/log-store-pidgin.c | 7 +- telepathy-logger/log-store-xml.c | 7 +- telepathy-logger/log-store.c | 6 +- telepathy-logger/log-walker-internal.h | 4 +- telepathy-logger/log-walker.c | 107 ++++++++++++++++++++++++++-- tests/dbus/test-tpl-log-iter-pidgin.c | 4 +- tests/dbus/test-tpl-log-iter-xml.c | 8 +-- 13 files changed, 133 insertions(+), 148 deletions(-) diff --git a/telepathy-logger/log-iter-pidgin-internal.h b/telepathy-logger/log-iter-pidgin-internal.h index 89777bc..230a57e 100644 --- a/telepathy-logger/log-iter-pidgin-internal.h +++ b/telepathy-logger/log-iter-pidgin-internal.h @@ -25,7 +25,6 @@ #include #include -#include #include G_BEGIN_DECLS @@ -72,9 +71,7 @@ GType tpl_log_iter_pidgin_get_type (void) G_GNUC_CONST; TplLogIter *tpl_log_iter_pidgin_new (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data); + gint type_mask); G_END_DECLS diff --git a/telepathy-logger/log-iter-pidgin.c b/telepathy-logger/log-iter-pidgin.c index 86ea0df..a9263d4 100644 --- a/telepathy-logger/log-iter-pidgin.c +++ b/telepathy-logger/log-iter-pidgin.c @@ -30,17 +30,13 @@ struct _TplLogIterPidginPriv GList *next_event; TpAccount *account; TplEntity *target; - TplLogEventFilter filter; TplLogStore *store; gint type_mask; - gpointer filter_data; }; enum { PROP_ACCOUNT = 1, - PROP_FILTER, - PROP_FILTER_DATA, PROP_STORE, PROP_TARGET, PROP_TYPE_MASK @@ -89,12 +85,8 @@ tpl_log_iter_pidgin_get_events (TplLogIter *iter, } event = TPL_EVENT (priv->next_event->data); - - if (priv->filter == NULL || (*priv->filter) (event, priv->filter_data)) - { - events = g_list_prepend (events, g_object_ref (event)); - i++; - } + events = g_list_prepend (events, g_object_ref (event)); + i++; priv->next_event = g_list_previous (priv->next_event); } @@ -124,8 +116,6 @@ tpl_log_iter_pidgin_rewind (TplLogIter *iter, i = 0; while (i < num_events) { - TplEvent *event; - if (e == NULL) { GList *d; @@ -159,15 +149,9 @@ tpl_log_iter_pidgin_rewind (TplLogIter *iter, e = priv->events; } - event = TPL_EVENT (e->data); - - if (priv->filter == NULL || (*priv->filter) (event, priv->filter_data)) - { - priv->next_event = e; - i++; - } - + priv->next_event = e; e = g_list_next (e); + i++; } } @@ -216,14 +200,6 @@ tpl_log_iter_pidgin_get_property (GObject *object, g_value_set_object (value, priv->account); break; - case PROP_FILTER: - g_value_set_pointer (value, priv->filter); - break; - - case PROP_FILTER_DATA: - g_value_set_pointer (value, priv->filter_data); - break; - case PROP_STORE: g_value_set_object (value, priv->store); break; @@ -259,14 +235,6 @@ tpl_log_iter_pidgin_set_property (GObject *object, priv->account = g_value_dup_object (value); break; - case PROP_FILTER: - priv->filter = g_value_get_pointer (value); - break; - - case PROP_FILTER_DATA: - priv->filter_data = g_value_get_pointer (value); - break; - case PROP_STORE: priv->store = g_value_dup_object (value); break; @@ -315,18 +283,6 @@ tpl_log_iter_pidgin_class_init (TplLogIterPidginClass *klass) G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); g_object_class_install_property (object_class, PROP_ACCOUNT, param_spec); - param_spec = g_param_spec_pointer ("filter", - "Filter", - "An optional filter function", - G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); - g_object_class_install_property (object_class, PROP_FILTER, param_spec); - - param_spec = g_param_spec_pointer ("filter-data", - "Filter Data", - "User data to pass to the filter function", - G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); - g_object_class_install_property (object_class, PROP_FILTER_DATA, param_spec); - param_spec = g_param_spec_object ("store", "Store", "The storage backend from which the logs are to be retrieved", @@ -358,16 +314,12 @@ TplLogIter * tpl_log_iter_pidgin_new (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data) + gint type_mask) { return g_object_new (TPL_TYPE_LOG_ITER_PIDGIN, "store", store, "account", account, "target", target, "type-mask", type_mask, - "filter", filter, - "filter-data", filter_data, NULL); } diff --git a/telepathy-logger/log-iter-xml-internal.h b/telepathy-logger/log-iter-xml-internal.h index 5b9d0bf..789c91f 100644 --- a/telepathy-logger/log-iter-xml-internal.h +++ b/telepathy-logger/log-iter-xml-internal.h @@ -72,9 +72,7 @@ GType tpl_log_iter_xml_get_type (void) G_GNUC_CONST; TplLogIter *tpl_log_iter_xml_new (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data); + gint type_mask); G_END_DECLS diff --git a/telepathy-logger/log-iter-xml.c b/telepathy-logger/log-iter-xml.c index 17f14c3..1684a74 100644 --- a/telepathy-logger/log-iter-xml.c +++ b/telepathy-logger/log-iter-xml.c @@ -30,17 +30,14 @@ struct _TplLogIterXmlPriv GList *next_event; TpAccount *account; TplEntity *target; - TplLogEventFilter filter; TplLogStore *store; gint type_mask; - gpointer filter_data; + }; enum { PROP_ACCOUNT = 1, - PROP_FILTER, - PROP_FILTER_DATA, PROP_STORE, PROP_TARGET, PROP_TYPE_MASK @@ -89,12 +86,8 @@ tpl_log_iter_xml_get_events (TplLogIter *iter, } event = TPL_EVENT (priv->next_event->data); - - if (priv->filter == NULL || (*priv->filter) (event, priv->filter_data)) - { - events = g_list_prepend (events, g_object_ref (event)); - i++; - } + events = g_list_prepend (events, g_object_ref (event)); + i++; priv->next_event = g_list_previous (priv->next_event); } @@ -124,8 +117,6 @@ tpl_log_iter_xml_rewind (TplLogIter *iter, i = 0; while (i < num_events) { - TplEvent *event; - if (e == NULL) { GList *d; @@ -159,15 +150,9 @@ tpl_log_iter_xml_rewind (TplLogIter *iter, e = priv->events; } - event = TPL_EVENT (e->data); - - if (priv->filter == NULL || (*priv->filter) (event, priv->filter_data)) - { - priv->next_event = e; - i++; - } - + priv->next_event = e; e = g_list_next (e); + i++; } } @@ -216,14 +201,6 @@ tpl_log_iter_xml_get_property (GObject *object, g_value_set_object (value, priv->account); break; - case PROP_FILTER: - g_value_set_pointer (value, priv->filter); - break; - - case PROP_FILTER_DATA: - g_value_set_pointer (value, priv->filter_data); - break; - case PROP_STORE: g_value_set_object (value, priv->store); break; @@ -259,14 +236,6 @@ tpl_log_iter_xml_set_property (GObject *object, priv->account = g_value_dup_object (value); break; - case PROP_FILTER: - priv->filter = g_value_get_pointer (value); - break; - - case PROP_FILTER_DATA: - priv->filter_data = g_value_get_pointer (value); - break; - case PROP_STORE: priv->store = g_value_dup_object (value); break; @@ -315,18 +284,6 @@ tpl_log_iter_xml_class_init (TplLogIterXmlClass *klass) G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); g_object_class_install_property (object_class, PROP_ACCOUNT, param_spec); - param_spec = g_param_spec_pointer ("filter", - "Filter", - "An optional filter function", - G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); - g_object_class_install_property (object_class, PROP_FILTER, param_spec); - - param_spec = g_param_spec_pointer ("filter-data", - "Filter Data", - "User data to pass to the filter function", - G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); - g_object_class_install_property (object_class, PROP_FILTER_DATA, param_spec); - param_spec = g_param_spec_object ("store", "Store", "The storage backend from which the logs are to be retrieved", @@ -358,16 +315,12 @@ TplLogIter * tpl_log_iter_xml_new (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data) + gint type_mask) { return g_object_new (TPL_TYPE_LOG_ITER_XML, "store", store, "account", account, "target", target, "type-mask", type_mask, - "filter", filter, - "filter-data", filter_data, NULL); } diff --git a/telepathy-logger/log-manager.c b/telepathy-logger/log-manager.c index f3f1795..ede6d26 100644 --- a/telepathy-logger/log-manager.c +++ b/telepathy-logger/log-manager.c @@ -1202,16 +1202,14 @@ tpl_log_manager_walk_filtered_events (TplLogManager *manager, g_return_val_if_fail (TPL_IS_ENTITY (target), NULL); priv = manager->priv; - walker = tpl_log_walker_new (); + walker = tpl_log_walker_new (filter, filter_data); for (l = priv->readable_stores; l != NULL; l = g_list_next (l)) { TplLogStore *store = TPL_LOG_STORE (l->data); TplLogIter *iter; - iter = _tpl_log_store_create_iter (store, account, target, type_mask, - filter, filter_data); - + iter = _tpl_log_store_create_iter (store, account, target, type_mask); if (iter != NULL) tpl_log_walker_add_iter (walker, iter); } diff --git a/telepathy-logger/log-store-internal.h b/telepathy-logger/log-store-internal.h index aa08474..9b0a666 100644 --- a/telepathy-logger/log-store-internal.h +++ b/telepathy-logger/log-store-internal.h @@ -82,8 +82,7 @@ typedef struct void (*clear_entity) (TplLogStore *self, TpAccount *account, TplEntity *entity); TplLogIter * (*create_iter) (TplLogStore *self, TpAccount *account, - TplEntity *target, gint type_mask, TplLogEventFilter filter, - gpointer filter_data); + TplEntity *target, gint type_mask); } TplLogStoreInterface; GType _tpl_log_store_get_type (void); @@ -110,8 +109,7 @@ void _tpl_log_store_clear_account (TplLogStore *self, TpAccount *account); void _tpl_log_store_clear_entity (TplLogStore *self, TpAccount *account, TplEntity *entity); TplLogIter * _tpl_log_store_create_iter (TplLogStore *self, - TpAccount *account, TplEntity *target, gint type_mask, - TplLogEventFilter filter, gpointer filter_data); + TpAccount *account, TplEntity *target, gint type_mask); gboolean _tpl_log_store_is_writable (TplLogStore *self); gboolean _tpl_log_store_is_readable (TplLogStore *self); diff --git a/telepathy-logger/log-store-pidgin.c b/telepathy-logger/log-store-pidgin.c index 9c60a81..284783b 100644 --- a/telepathy-logger/log-store-pidgin.c +++ b/telepathy-logger/log-store-pidgin.c @@ -1129,16 +1129,13 @@ static TplLogIter * log_store_pidgin_create_iter (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data) + gint type_mask) { g_return_val_if_fail (TPL_IS_LOG_STORE_PIDGIN (store), NULL); g_return_val_if_fail (TP_IS_ACCOUNT (account), NULL); g_return_val_if_fail (TPL_IS_ENTITY (target), NULL); - return tpl_log_iter_pidgin_new (store, account, target, type_mask, filter, - filter_data); + return tpl_log_iter_pidgin_new (store, account, target, type_mask); } diff --git a/telepathy-logger/log-store-xml.c b/telepathy-logger/log-store-xml.c index a723964..f02785a 100644 --- a/telepathy-logger/log-store-xml.c +++ b/telepathy-logger/log-store-xml.c @@ -1921,16 +1921,13 @@ static TplLogIter * log_store_xml_create_iter (TplLogStore *store, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data) + gint type_mask) { g_return_val_if_fail (TPL_IS_LOG_STORE_XML (store), NULL); g_return_val_if_fail (TP_IS_ACCOUNT (account), NULL); g_return_val_if_fail (TPL_IS_ENTITY (target), NULL); - return tpl_log_iter_xml_new (store, account, target, type_mask, filter, - filter_data); + return tpl_log_iter_xml_new (store, account, target, type_mask); } diff --git a/telepathy-logger/log-store.c b/telepathy-logger/log-store.c index 77a7fd6..6e1d10f 100644 --- a/telepathy-logger/log-store.c +++ b/telepathy-logger/log-store.c @@ -342,16 +342,14 @@ TplLogIter * _tpl_log_store_create_iter (TplLogStore *self, TpAccount *account, TplEntity *target, - gint type_mask, - TplLogEventFilter filter, - gpointer filter_data) + gint type_mask) { g_return_val_if_fail (TPL_IS_LOG_STORE (self), NULL); if (TPL_LOG_STORE_GET_INTERFACE (self)->create_iter == NULL) return NULL; return TPL_LOG_STORE_GET_INTERFACE (self)->create_iter (self, - account, target, type_mask, filter, filter_data); + account, target, type_mask); } diff --git a/telepathy-logger/log-walker-internal.h b/telepathy-logger/log-walker-internal.h index 29cc816..7425290 100644 --- a/telepathy-logger/log-walker-internal.h +++ b/telepathy-logger/log-walker-internal.h @@ -22,11 +22,13 @@ #define __TPL_LOG_WALKER_INTERNAL_H__ #include "log-iter-internal.h" +#include "log-manager.h" #include "log-walker.h" G_BEGIN_DECLS -TplLogWalker *tpl_log_walker_new (void); +TplLogWalker *tpl_log_walker_new (TplLogEventFilter filter, + gpointer filter_data); void tpl_log_walker_add_iter (TplLogWalker *walker, TplLogIter *iter); diff --git a/telepathy-logger/log-walker.c b/telepathy-logger/log-walker.c index a873384..94cb6d7 100644 --- a/telepathy-logger/log-walker.c +++ b/telepathy-logger/log-walker.c @@ -182,8 +182,16 @@ struct _TplLogWalkerPriv GList *history; GList *iters; GMutex mutex; + TplLogEventFilter filter; gboolean is_start; gboolean is_end; + gpointer filter_data; +}; + +enum +{ + PROP_FILTER = 1, + PROP_FILTER_DATA }; @@ -208,6 +216,7 @@ typedef struct typedef struct { TplLogIter *iter; + gboolean skip; guint count; } TplLogWalkerHistoryData; @@ -426,20 +435,33 @@ tpl_log_walker_get_events (GObject *source_object, if (async_data->latest_event != NULL) { GList *h; + TplEvent *event; TplLogWalkerHistoryData *data; + gboolean skip; + + event = async_data->latest_event->data; + skip = TRUE; + + if (priv->filter == NULL || + (*priv->filter) (event, priv->filter_data)) + { + events = g_list_prepend (events, event); + i++; + skip = FALSE; + } - events = g_list_prepend (events, async_data->latest_event->data); async_data->latest_cache->data = g_list_delete_link ( async_data->latest_cache->data, async_data->latest_event); - i++; h = priv->history; if (h == NULL || ((TplLogWalkerHistoryData *) h->data)->iter != - async_data->latest_iter->data) + async_data->latest_iter->data || + ((TplLogWalkerHistoryData *) h->data)->skip != skip) { data = tpl_log_walker_history_data_new (); data->iter = g_object_ref (async_data->latest_iter->data); + data->skip = skip; priv->history = g_list_prepend (priv->history, data); } else @@ -532,7 +554,8 @@ tpl_log_walker_rewind (TplLogWalker *walker, tpl_log_iter_rewind (data->iter, 1, error); data->count--; - i++; + if (!data->skip) + i++; if (data->count == 0) { @@ -604,6 +627,60 @@ tpl_log_walker_finalize (GObject *object) } +static void +tpl_log_walker_get_property (GObject *object, + guint param_id, + GValue *value, + GParamSpec *pspec) +{ + TplLogWalkerPriv *priv; + + priv = TPL_LOG_WALKER (object)->priv; + + switch (param_id) + { + case PROP_FILTER: + g_value_set_pointer (value, priv->filter); + break; + + case PROP_FILTER_DATA: + g_value_set_pointer (value, priv->filter_data); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, param_id, pspec); + break; + } +} + + +static void +tpl_log_walker_set_property (GObject *object, + guint param_id, + const GValue *value, + GParamSpec *pspec) +{ + TplLogWalkerPriv *priv; + + priv = TPL_LOG_WALKER (object)->priv; + + switch (param_id) + { + case PROP_FILTER: + priv->filter = g_value_get_pointer (value); + break; + + case PROP_FILTER_DATA: + priv->filter_data = g_value_get_pointer (value); + break; + + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, param_id, pspec); + break; + } +} + + static void tpl_log_walker_init (TplLogWalker *walker) { @@ -624,18 +701,36 @@ static void tpl_log_walker_class_init (TplLogWalkerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); + GParamSpec *param_spec; object_class->dispose = tpl_log_walker_dispose; object_class->finalize = tpl_log_walker_finalize; + object_class->get_property = tpl_log_walker_get_property; + object_class->set_property = tpl_log_walker_set_property; + + param_spec = g_param_spec_pointer ("filter", + "Filter", + "An optional filter function", + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); + g_object_class_install_property (object_class, PROP_FILTER, param_spec); + + param_spec = g_param_spec_pointer ("filter-data", + "Filter Data", + "User data to pass to the filter function", + G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); + g_object_class_install_property (object_class, PROP_FILTER_DATA, param_spec); g_type_class_add_private (klass, sizeof (TplLogWalkerPriv)); } TplLogWalker * -tpl_log_walker_new (void) +tpl_log_walker_new (TplLogEventFilter filter, gpointer filter_data) { - return g_object_new (TPL_TYPE_LOG_WALKER, NULL); + return g_object_new (TPL_TYPE_LOG_WALKER, + "filter", filter, + "filter-data", filter_data, + NULL); } diff --git a/tests/dbus/test-tpl-log-iter-pidgin.c b/tests/dbus/test-tpl-log-iter-pidgin.c index 059b32b..1859057 100644 --- a/tests/dbus/test-tpl-log-iter-pidgin.c +++ b/tests/dbus/test-tpl-log-iter-pidgin.c @@ -154,7 +154,7 @@ test_get_events (PidginTestCaseFixture *fixture, room = tpl_entity_new_from_room_id ("#telepathy"); iter = tpl_log_iter_pidgin_new (fixture->store, fixture->account, room, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); events = tpl_log_iter_get_events (iter, 5, &error); events = events; @@ -557,7 +557,7 @@ test_rewind (PidginTestCaseFixture *fixture, room = tpl_entity_new_from_room_id ("#telepathy"); iter = tpl_log_iter_pidgin_new (fixture->store, fixture->account, room, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); tpl_log_iter_rewind (iter, 8, &error); g_assert_no_error (error); diff --git a/tests/dbus/test-tpl-log-iter-xml.c b/tests/dbus/test-tpl-log-iter-xml.c index b01a322..044bf47 100644 --- a/tests/dbus/test-tpl-log-iter-xml.c +++ b/tests/dbus/test-tpl-log-iter-xml.c @@ -87,7 +87,7 @@ test_get_events (XmlTestCaseFixture *fixture, /* Text events spanning multiple days */ iter = tpl_log_iter_xml_new (fixture->store, fixture->account, user2, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); events = tpl_log_iter_get_events (iter, 5, &error); g_assert_no_error (error); @@ -177,7 +177,7 @@ test_get_events (XmlTestCaseFixture *fixture, /* A mix of call and text events */ iter = tpl_log_iter_xml_new (fixture->store, fixture->account, user4, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); events = tpl_log_iter_get_events (iter, 4, &error); g_assert_no_error (error); @@ -240,7 +240,7 @@ test_rewind (XmlTestCaseFixture *fixture, /* Text events spanning multiple days */ iter = tpl_log_iter_xml_new (fixture->store, fixture->account, user2, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); tpl_log_iter_rewind (iter, 8, &error); g_assert_no_error (error); @@ -355,7 +355,7 @@ test_rewind (XmlTestCaseFixture *fixture, /* A mix of call and text events */ iter = tpl_log_iter_xml_new (fixture->store, fixture->account, user4, - TPL_EVENT_MASK_ANY, NULL, NULL); + TPL_EVENT_MASK_ANY); tpl_log_iter_rewind (iter, 8, &error); g_assert_no_error (error); -- cgit v1.2.1