From e56a67a8890f028a7e06256de4740c0f27aff29b Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 9 Apr 2014 19:13:41 +0100 Subject: [PATCH 8/8] tp_group_mixin_change_members: take a GVariant for the details Bug: https://bugs.freedesktop.org/show_bug.cgi?id=77190 --- examples/cm/channelspecific/room.c | 34 ++++------------- telepathy-glib/group-mixin.c | 47 ++++++++++++++--------- telepathy-glib/group-mixin.h | 2 +- tests/dbus/cli-group.c | 76 ++++++++++---------------------------- tests/dbus/group-mixin.c | 66 ++++++++++----------------------- tests/lib/textchan-group.c | 20 +++------- 6 files changed, 82 insertions(+), 163 deletions(-) diff --git a/examples/cm/channelspecific/room.c b/examples/cm/channelspecific/room.c index f8a83ad..d6cdfc8 100644 --- a/examples/cm/channelspecific/room.c +++ b/examples/cm/channelspecific/room.c @@ -98,7 +98,6 @@ complete_join (ExampleCSHRoomChannel *self) TpHandle alice_global, bob_global, chris_global; TpGroupMixin *mixin = TP_GROUP_MIXIN (self); TpIntset *added; - GHashTable *details; /* For this example, we assume that all chatrooms initially contain * Alice, Bob and Chris (and that their global IDs are also known), @@ -145,16 +144,11 @@ complete_join (ExampleCSHRoomChannel *self) tp_base_connection_get_self_handle (conn)); tp_group_mixin_change_self_handle ((GObject *) self, new_self); - details = tp_asv_new ( - "message", G_TYPE_STRING, "", - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_RENAMED, - "actor", G_TYPE_UINT, 0, - NULL); - tp_group_mixin_change_members ((GObject *) self, NULL, removed, NULL, - rp, details); + rp, + g_variant_new_parsed ("{'change-reason': <%u>}", + (guint32) TP_CHANNEL_GROUP_CHANGE_REASON_RENAMED)); - tp_clear_pointer (&details, g_hash_table_unref); tp_intset_destroy (removed); tp_intset_destroy (rp); } @@ -177,16 +171,8 @@ complete_join (ExampleCSHRoomChannel *self) tp_intset_add (added, anon_local); tp_intset_add (added, mixin->self_handle); - details = tp_asv_new ( - "message", G_TYPE_STRING, "", - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - "actor", G_TYPE_UINT, 0, - NULL); - tp_group_mixin_change_members ((GObject *) self, added, NULL, NULL, - NULL, details); - - tp_clear_pointer (&details, g_hash_table_unref); + NULL, NULL); /* now that the dust has settled, we can also invite people */ tp_group_mixin_change_flags ((GObject *) self, @@ -202,7 +188,6 @@ join_room (ExampleCSHRoomChannel *self) TpGroupMixin *mixin = TP_GROUP_MIXIN (self); GObject *object = (GObject *) self; TpIntset *add_remote_pending; - GHashTable *details; g_assert (!tp_handle_set_is_member (mixin->members, mixin->self_handle)); g_assert (!tp_handle_set_is_member (mixin->remote_pending, @@ -213,18 +198,13 @@ join_room (ExampleCSHRoomChannel *self) add_remote_pending = tp_intset_new (); tp_intset_add (add_remote_pending, mixin->self_handle); - details = tp_asv_new ( - "message", G_TYPE_STRING, "", - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - "actor", G_TYPE_UINT, tp_base_connection_get_self_handle (conn), - NULL); - tp_group_mixin_add_handle_owner (object, mixin->self_handle, tp_base_connection_get_self_handle (conn)); tp_group_mixin_change_members (object, NULL, NULL, NULL, - add_remote_pending, details); + add_remote_pending, + g_variant_new_parsed ("{'actor': <%u>}", + (guint32) tp_base_connection_get_self_handle (conn))); - g_hash_table_unref (details); tp_intset_destroy (add_remote_pending); /* Actually join the room. In a real implementation this would be a network diff --git a/telepathy-glib/group-mixin.c b/telepathy-glib/group-mixin.c index 8c5308a..ffaa35c 100644 --- a/telepathy-glib/group-mixin.c +++ b/telepathy-glib/group-mixin.c @@ -1107,10 +1107,10 @@ emit_members_changed_signals (GObject *channel, const GArray *remote_pending, TpHandle actor, TpChannelGroupChangeReason reason, - const GHashTable *details) + GVariant *details) { TpGroupMixin *mixin = TP_GROUP_MIXIN (channel); - GHashTable *details_ = (GHashTable *) details; /* Cast the pain away! */ + GHashTable *details_ = tp_asv_from_vardict (details); gboolean added_contact_ids; if (DEBUGGING) @@ -1160,6 +1160,8 @@ emit_members_changed_signals (GObject *channel, if (added_contact_ids) remove_contact_ids (details_); + + g_hash_table_unref (details_); } @@ -1172,13 +1174,15 @@ change_members (GObject *obj, const TpIntset *add_remote_pending, TpHandle actor, TpChannelGroupChangeReason reason, - const GHashTable *details) + GVariant *details) { TpGroupMixin *mixin = TP_GROUP_MIXIN (obj); TpIntset *new_add, *new_remove, *new_local_pending, *new_remote_pending, *tmp, *tmp2, *empty; gboolean ret; + g_assert (!g_variant_is_floating (details)); + empty = tp_intset_new (); if (message == NULL) @@ -1345,7 +1349,7 @@ change_members (GObject *obj, * and removed from members and remote pending * @add_remote_pending: A set of contact handles to be added to remote pending, * and removed from members and local pending - * @details: a map from strings to GValues detailing the change + * @details: (allow-none): a %G_VARIANT_TYPE_VARDICT describing the change * * Change the sets of members as given by the arguments, and emit the * MembersChanged signal if the changes were not a no-op. @@ -1361,11 +1365,14 @@ change_members (GObject *obj, * equivalent to an empty set. * * details may contain, among other entries, the well-known - * keys (and corresponding type, wrapped in a GValue) defined by the + * keys (and corresponding type, wrapped in a variant) defined by the * Group.MembersChanged signal's specification; these include "actor" - * (a handle as G_TYPE_UINT), "change-reason" (an element of - * #TpChannelGroupChangeReason as G_TYPE_UINT), "message" (G_TYPE_STRING), - * "error" (G_TYPE_STRING), "debug-message" (G_TYPE_STRING). + * (a handle as %G_VARIANT_TYPE_UINT32), "change-reason" (an element of + * #TpChannelGroupChangeReason as %G_VARIANT_TYPE_UINT32), + * "message" (%G_VARIANT_TYPE_STRING), + * "error" (%G_VARIANT_TYPE_STRING), "debug-message" (%G_VARIANT_TYPE_STRING). + * If it is a floating reference, ownership is taken; if it is %NULL, + * it is treated as an empty map. * * Returns: %TRUE if the group was changed and the MembersChanged * signals were emitted; %FALSE if nothing actually changed and the signals @@ -1379,33 +1386,39 @@ tp_group_mixin_change_members (GObject *obj, const TpIntset *del, const TpIntset *add_local_pending, const TpIntset *add_remote_pending, - const GHashTable *details) + GVariant *details) { const gchar *message; TpHandle actor; TpChannelGroupChangeReason reason; gboolean valid; + gboolean ret; - g_return_val_if_fail (details != NULL, FALSE); + if (details == NULL) + details = g_variant_new ("a{sv}", NULL); + + g_variant_ref_sink (details); /* For each detail we're extracting for the benefit of old-school * MembersChanged, warn if it's present but badly typed. */ - message = tp_asv_get_string (details, "message"); - g_warn_if_fail (message != NULL || tp_asv_lookup (details, "message") == NULL); + message = tp_vardict_get_string (details, "message"); + g_warn_if_fail (message != NULL || !tp_vardict_has_key (details, "message")); /* change_members will cry (via tp_handle_set_add) if actor is non-zero and * invalid. */ - actor = tp_asv_get_uint32 (details, "actor", &valid); - g_warn_if_fail (valid || tp_asv_lookup (details, "actor") == NULL); + actor = tp_vardict_get_uint32 (details, "actor", &valid); + g_warn_if_fail (valid || !tp_vardict_has_key (details, "actor")); - reason = tp_asv_get_uint32 (details, "change-reason", &valid); - g_warn_if_fail (valid || tp_asv_lookup (details, "change-reason") == NULL); + reason = tp_vardict_get_uint32 (details, "change-reason", &valid); + g_warn_if_fail (valid || !tp_vardict_has_key (details, "change-reason")); - return change_members (obj, message, add, del, add_local_pending, + ret = change_members (obj, message, add, del, add_local_pending, add_remote_pending, actor, reason, details); + g_variant_unref (details); + return ret; } /** diff --git a/telepathy-glib/group-mixin.h b/telepathy-glib/group-mixin.h index 677335e..318c817 100644 --- a/telepathy-glib/group-mixin.h +++ b/telepathy-glib/group-mixin.h @@ -192,7 +192,7 @@ void tp_group_mixin_change_flags (GObject *obj, gboolean tp_group_mixin_change_members (GObject *obj, const TpIntset *add, const TpIntset *del, const TpIntset *add_local_pending, const TpIntset *add_remote_pending, - const GHashTable *details); + GVariant *details); void tp_group_mixin_change_self_handle (GObject *obj, TpHandle new_self_handle); diff --git a/tests/dbus/cli-group.c b/tests/dbus/cli-group.c index 2c4b89e..609e756 100644 --- a/tests/dbus/cli-group.c +++ b/tests/dbus/cli-group.c @@ -72,7 +72,6 @@ test_channel_proxy (TpTestsTextChannelGroup *service_chan, TpChannel *chan) { TpIntset *add, *rem, *expected_members; - GHashTable *details; GQuark features[] = { TP_CHANNEL_FEATURE_GROUP, 0 }; GMainLoop *loop = g_main_loop_new (NULL, FALSE); @@ -90,16 +89,10 @@ test_channel_proxy (TpTestsTextChannelGroup *service_chan, expected_reason++; - details = tp_asv_new ( - "message", G_TYPE_STRING, "quantum tunnelling", - "change-reason", G_TYPE_UINT, expected_reason, - "actor", G_TYPE_UINT, 0, - NULL); - tp_group_mixin_change_members ((GObject *) service_chan, - add, NULL, NULL, NULL, details); - - tp_clear_pointer (&details, g_hash_table_unref); + add, NULL, NULL, NULL, + g_variant_new_parsed ("{'message': <'quantum tunnelling'>, " + "'change-reason': <%u>}", (guint32) expected_reason)); /* Run until we get "group-members-changed" signal */ g_main_loop_run (loop); @@ -116,16 +109,12 @@ test_channel_proxy (TpTestsTextChannelGroup *service_chan, expecting_group_members_changed = TRUE; expected_reason++; - details = tp_asv_new ( - "message", G_TYPE_STRING, "goat", - "change-reason", G_TYPE_UINT, expected_reason, - "actor", G_TYPE_UINT, 0, - NULL); - tp_group_mixin_change_members ((GObject *) service_chan, - add, rem, NULL, NULL, details); + add, rem, NULL, NULL, + g_variant_new_parsed ("{'message': <'goat'>, " + "'change-reason': <%u>, " + "'actor': }", (guint32) expected_reason)); - tp_clear_pointer (&details, g_hash_table_unref); tp_intset_destroy (add); tp_intset_destroy (rem); @@ -246,7 +235,6 @@ check_removed_unknown_error_in_invalidated (void) TpIntset *self_handle_singleton = tp_intset_new (); gboolean invalidated = FALSE; GError *error = NULL; - GHashTable *details; chan_path = g_strdup_printf ("%s/Channel_1_6180339887", tp_proxy_get_object_path (conn)); @@ -267,35 +255,23 @@ check_removed_unknown_error_in_invalidated (void) g_signal_connect (chan, "invalidated", (GCallback) check_invalidated_unknown_error_cb, &invalidated); - details = tp_asv_new ( - "message", G_TYPE_STRING, "hello", - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - "actor", G_TYPE_UINT, 0, - NULL); - tp_intset_add (self_handle_singleton, self_handle); tp_group_mixin_change_members ((GObject *) service_chan, - self_handle_singleton, NULL, NULL, NULL, details); - - tp_clear_pointer (&details, g_hash_table_unref); + self_handle_singleton, NULL, NULL, NULL, + g_variant_new_parsed ("{'message': <'hello'>}")); run_until_members_changed (chan); - details = tp_asv_new ( - "message", G_TYPE_STRING, REMOVED_MESSAGE, - "change-reason", G_TYPE_UINT, REMOVED_REASON, - "error", G_TYPE_STRING, REMOVED_UNKNOWN_ERROR, - NULL); - tp_group_mixin_change_members ((GObject *) service_chan, NULL, - self_handle_singleton, NULL, NULL, details); + self_handle_singleton, NULL, NULL, + g_variant_new_parsed ("{'message': <%s>, 'change-reason': <%u>, " + "'error': <%s> }", REMOVED_MESSAGE, (guint32) REMOVED_REASON, + REMOVED_UNKNOWN_ERROR)); run_until_members_changed (chan); tp_cli_channel_call_close (chan, -1, NULL, NULL, NULL, NULL); - tp_clear_pointer (&details, g_hash_table_unref); - tp_tests_proxy_run_until_dbus_queue_processed (conn); MYASSERT (invalidated, ""); @@ -336,8 +312,6 @@ check_removed_known_error_in_invalidated (void) TpTestsTextChannelGroup *service_chan; TpChannel *chan; TpIntset *self_handle_singleton = tp_intset_new (); - GHashTable *details = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, - (GDestroyNotify) tp_g_value_slice_free); gboolean invalidated = FALSE; GError *error = NULL; @@ -359,35 +333,23 @@ check_removed_known_error_in_invalidated (void) g_signal_connect (chan, "invalidated", (GCallback) check_invalidated_known_error_cb, &invalidated); - details = tp_asv_new ( - "message", G_TYPE_STRING, "hello", - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - "actor", G_TYPE_UINT, 0, - NULL); - tp_intset_add (self_handle_singleton, self_handle); tp_group_mixin_change_members ((GObject *) service_chan, - self_handle_singleton, NULL, NULL, NULL, details); - - tp_clear_pointer (&details, g_hash_table_unref); + self_handle_singleton, NULL, NULL, NULL, + g_variant_new_parsed ("{'message': <'hello'>}")); run_until_members_changed (chan); - details = tp_asv_new ( - "message", G_TYPE_STRING, REMOVED_MESSAGE, - "change-reason", G_TYPE_UINT, REMOVED_REASON, - "error", G_TYPE_STRING, REMOVED_KNOWN_ERROR_STR, - NULL); - tp_group_mixin_change_members ((GObject *) service_chan, NULL, - self_handle_singleton, NULL, NULL, details); + self_handle_singleton, NULL, NULL, + g_variant_new_parsed ("{'message': <%s>, 'change-reason': <%u>, " + "'error': <%s> }", REMOVED_MESSAGE, (guint32) REMOVED_REASON, + REMOVED_KNOWN_ERROR_STR)); run_until_members_changed (chan); tp_cli_channel_call_close (chan, -1, NULL, NULL, NULL, NULL); - tp_clear_pointer (&details, g_hash_table_unref); - tp_tests_proxy_run_until_dbus_queue_processed (conn); MYASSERT (invalidated, ""); diff --git a/tests/dbus/group-mixin.c b/tests/dbus/group-mixin.c index 77c0643..ccde94f 100644 --- a/tests/dbus/group-mixin.c +++ b/tests/dbus/group-mixin.c @@ -274,22 +274,20 @@ check_incoming_invitation (void) /* We get an invitation to the channel */ { TpIntset *add_local_pending = tp_intset_new (); - GHashTable *details = tp_asv_new ( - "message", G_TYPE_STRING, "HELLO THAR", - "actor", G_TYPE_UINT, 0, - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_INVITED, - NULL); + tp_intset_add (add_local_pending, self_handle); expect_signals ("HELLO THAR", 0, TP_CHANNEL_GROUP_CHANGE_REASON_INVITED, self_added_to_lp); tp_group_mixin_change_members ((GObject *) service_chan, NULL, - NULL, add_local_pending, NULL, details); + NULL, add_local_pending, NULL, + g_variant_new_parsed ("{'message': <'HELLO THAR'>, " + "'change-reason': <%u>}", + (guint32) TP_CHANNEL_GROUP_CHANGE_REASON_INVITED)); wait_for_outstanding_signals (); MYASSERT (!outstanding_signals (), ": MembersChanged should have fired once"); - g_hash_table_unref (details); tp_intset_destroy (add_local_pending); } @@ -399,18 +397,14 @@ in_the_desert (void) /* A camel is approaching */ { TpIntset *add = tp_intset_new (); - GHashTable *details = tp_asv_new ( - "message", G_TYPE_STRING, "", - "actor", G_TYPE_UINT, camel, - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - NULL); tp_intset_add (add, camel); tp_intset_add (expected_members, camel); expect_signals ("", camel, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, camel_added); tp_group_mixin_change_members ((GObject *) service_chan, add, NULL, - NULL, NULL, details); + NULL, NULL, + g_variant_new_parsed ("{'actor': <%u>}", (guint32) camel)); wait_for_outstanding_signals (); MYASSERT (!outstanding_signals (), ": MembersChanged should have fired once"); @@ -421,66 +415,50 @@ in_the_desert (void) /* A second camel is approaching (invited by the first camel) */ { TpIntset *add = tp_intset_new (); - GHashTable *details = g_hash_table_new_full (g_str_hash, g_str_equal, - NULL, (GDestroyNotify) tp_g_value_slice_free); tp_intset_add (add, camel2); tp_intset_add (expected_members, camel2); - g_hash_table_insert (details, "actor", tp_g_value_slice_new_uint (camel)); - expect_signals ("", camel, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, camel2_added); tp_group_mixin_change_members ((GObject *) service_chan, add, - NULL, NULL, NULL, details); + NULL, NULL, NULL, + g_variant_new_parsed ("{'actor': <%u>}", (guint32) camel)); wait_for_outstanding_signals (); MYASSERT (!outstanding_signals (), ": MembersChanged should have fired once"); tp_intset_destroy (add); - g_hash_table_unref (details); } { TpIntset *del = tp_intset_new (); - GHashTable *details = g_hash_table_new_full (g_str_hash, g_str_equal, - NULL, (GDestroyNotify) tp_g_value_slice_free); tp_intset_add (del, camel); tp_intset_remove (expected_members, camel); - g_hash_table_insert (details, "actor", tp_g_value_slice_new_uint (camel2)); - /* It turns out that spitting was not included in the GroupChangeReason - * enum. - */ - g_hash_table_insert (details, "error", - tp_g_value_slice_new_static_string ("le.mac.Spat")); - g_hash_table_insert (details, "saliva-consistency", - tp_g_value_slice_new_static_string ("fluid")); - - /* Kicking is the closest we have to this .. unsavory act. */ - g_hash_table_insert (details, "change-reason", - tp_g_value_slice_new_uint (TP_CHANNEL_GROUP_CHANGE_REASON_KICKED)); - g_hash_table_insert (details, "message", - tp_g_value_slice_new_static_string ("*ptooey*")); - - /* Check that all the right information was extracted from the dict. */ + * enum. Kicking is the closest we have to this .. unsavory act. */ expect_signals ("*ptooey*", camel2, TP_CHANNEL_GROUP_CHANGE_REASON_KICKED, camel_removed); tp_group_mixin_change_members ((GObject *) service_chan, NULL, - del, NULL, NULL, details); + del, NULL, NULL, + g_variant_new_parsed ("{ 'actor': <%u>, " + "'error': <'le.mac.Spat'>, " + "'saliva-consistency': <'fluid'>, " + "'change-reason': <%u>, " + "'message': <'*ptooey*'> }", + (guint32) camel2, + (guint32) TP_CHANNEL_GROUP_CHANGE_REASON_KICKED)); wait_for_outstanding_signals (); MYASSERT (!outstanding_signals (), ": MembersChanged should have fired once"); tp_intset_destroy (del); - g_hash_table_unref (details); } /* We and the second camel should be left in the channel */ { - GArray *service_members; TpIntset *service_members_intset; tp_tests_channel_assert_expect_members (chan, expected_members); @@ -488,13 +466,9 @@ in_the_desert (void) /* And let's check that the group mixin agrees, in case that's just the * client binding being wrong. */ - tp_group_mixin_get_members ((GObject *) service_chan, &service_members, - NULL); - service_members_intset = tp_intset_from_array (service_members); + service_members_intset = tp_handle_set_peek ( + TP_GROUP_MIXIN (service_chan)->members); g_assert (tp_intset_is_equal (service_members_intset, expected_members)); - - g_array_unref (service_members); - tp_intset_destroy (service_members_intset); } tp_intset_destroy (expected_members); diff --git a/tests/lib/textchan-group.c b/tests/lib/textchan-group.c index 02ed289..dcdeefa 100644 --- a/tests/lib/textchan-group.c +++ b/tests/lib/textchan-group.c @@ -61,18 +61,14 @@ add_member (GObject *obj, { TpTestsTextChannelGroup *self = TP_TESTS_TEXT_CHANNEL_GROUP (obj); TpIntset *add = tp_intset_new (); - GHashTable *details = tp_asv_new ( - "actor", G_TYPE_UINT, tp_base_connection_get_self_handle (self->conn), - "change-reason", G_TYPE_UINT, TP_CHANNEL_GROUP_CHANGE_REASON_NONE, - "message", G_TYPE_STRING, message, - NULL); tp_intset_add (add, handle); - tp_group_mixin_change_members (obj, add, NULL, NULL, NULL, details); + tp_group_mixin_change_members (obj, add, NULL, NULL, NULL, + g_variant_new_parsed ("{'actor': <%u>, 'message': <%s>}", + (guint32) tp_base_connection_get_self_handle (self->conn), + message)); tp_intset_destroy (add); - g_hash_table_unref (details); - return TRUE; } @@ -218,11 +214,6 @@ void tp_tests_text_channel_group_join (TpTestsTextChannelGroup *self) { TpIntset *add, *empty; - GHashTable *details = tp_asv_new ( - "actor", G_TYPE_UINT, 0, - "change-reason", G_TYPE_UINT, 0, - "message", G_TYPE_STRING, "", - NULL); /* Add ourself as a member */ add = tp_intset_new_containing ( @@ -230,11 +221,10 @@ tp_tests_text_channel_group_join (TpTestsTextChannelGroup *self) empty = tp_intset_new (); tp_group_mixin_change_members ((GObject *) self, add, empty, - empty, empty, details); + empty, empty, NULL); tp_intset_destroy (add); tp_intset_destroy (empty); - g_hash_table_unref (details); } void -- 1.9.1