From b71f2407212bee48a3447f32f060352b2a0c7229 Mon Sep 17 00:00:00 2001 From: Xavier Claessens Date: Mon, 7 May 2012 17:19:18 +0200 Subject: [PATCH] TpChannel: move group_remove_error handling to channel-contacts.c It means that TP_CHANNEL_FEATURE_CONTACTS must be prepared to get the group error. https://bugs.freedesktop.org/show_bug.cgi?id=49371 --- telepathy-glib/channel-contacts.c | 180 +++++++++++++++++++++++++++++++++++-- telepathy-glib/channel-group.c | 145 ------------------------------ tests/dbus/cli-group.c | 29 ++++-- 3 files changed, 198 insertions(+), 156 deletions(-) diff --git a/telepathy-glib/channel-contacts.c b/telepathy-glib/channel-contacts.c index 058fe6e..f6e7603 100644 --- a/telepathy-glib/channel-contacts.c +++ b/telepathy-glib/channel-contacts.c @@ -35,6 +35,32 @@ #include "telepathy-glib/debug-internal.h" #include "telepathy-glib/util-internal.h" +/** + * TP_ERRORS_REMOVED_FROM_GROUP: + * + * #GError domain representing the local user being removed from a channel + * with the Group interface. The @code in a #GError with this domain must + * be a member of #TpChannelGroupChangeReason. + * + * This error may be raised on non-Group channels with certain reason codes + * if there's no better error code to use (mainly + * %TP_CHANNEL_GROUP_CHANGE_REASON_NONE). + * + * This macro expands to a function call returning a #GQuark. + * + * Since: 0.7.1 + */ +GQuark +tp_errors_removed_from_group_quark (void) +{ + static GQuark q = 0; + + if (q == 0) + q = g_quark_from_static_string ("tp_errors_removed_from_group_quark"); + + return q; +} + static GArray * dup_handle_array (const GArray *source) { @@ -472,6 +498,99 @@ set_local_pending_info (TpChannel *self, GUINT_TO_POINTER (tp_contact_get_handle (contact)), info); } +/* + * If the @group_remove_error is derived from a TpChannelGroupChangeReason, + * attempt to rewrite it into a TpError. + */ +static void +_tp_channel_group_improve_remove_error (TpChannel *self, + TpContact *actor) +{ + GError *error = self->priv->group_remove_error; + + if (error == NULL || error->domain != TP_ERRORS_REMOVED_FROM_GROUP) + return; + + switch (error->code) + { + case TP_CHANNEL_GROUP_CHANGE_REASON_NONE: + if (actor == self->priv->group_self_contact || + actor == tp_connection_get_self_contact (self->priv->connection)) + { + error->code = TP_ERROR_CANCELLED; + } + else + { + error->code = TP_ERROR_TERMINATED; + } + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_OFFLINE: + error->code = TP_ERROR_OFFLINE; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_KICKED: + error->code = TP_ERROR_CHANNEL_KICKED; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_BUSY: + error->code = TP_ERROR_BUSY; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_INVITED: + DEBUG ("%s: Channel_Group_Change_Reason_Invited makes no sense as a " + "removal reason!", tp_proxy_get_object_path (self)); + error->domain = TP_DBUS_ERRORS; + error->code = TP_DBUS_ERROR_INCONSISTENT; + return; + + case TP_CHANNEL_GROUP_CHANGE_REASON_BANNED: + error->code = TP_ERROR_CHANNEL_BANNED; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_ERROR: + /* hopefully all CMs that use this will also give us an error detail, + * but if they didn't, or gave us one we didn't understand... */ + error->code = TP_ERROR_NOT_AVAILABLE; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_INVALID_CONTACT: + error->code = TP_ERROR_DOES_NOT_EXIST; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_NO_ANSWER: + error->code = TP_ERROR_NO_ANSWER; + break; + + /* TP_CHANNEL_GROUP_CHANGE_REASON_RENAMED shouldn't be the last error + * seen in the channel - we'll get removed again with a real reason, + * later, so there's no point in doing anything special with this one */ + + case TP_CHANNEL_GROUP_CHANGE_REASON_PERMISSION_DENIED: + error->code = TP_ERROR_PERMISSION_DENIED; + break; + + case TP_CHANNEL_GROUP_CHANGE_REASON_SEPARATED: + DEBUG ("%s: Channel_Group_Change_Reason_Separated makes no sense as a " + "removal reason!", tp_proxy_get_object_path (self)); + error->domain = TP_DBUS_ERRORS; + error->code = TP_DBUS_ERROR_INCONSISTENT; + return; + + /* all values up to and including Separated have been checked */ + + default: + /* We don't understand this reason code, so keeping the domain and code + * the same (i.e. using TP_ERRORS_REMOVED_FROM_GROUP) is no worse than + * anything else we could do. */ + return; + } + + /* If we changed the code we also need to change the domain; if not, we did + * an early return, so we'll never reach this */ + error->domain = TP_ERROR; +} + typedef struct { GPtrArray *added; @@ -502,11 +621,16 @@ members_changed_prepared_cb (GObject *object, { TpChannel *self = (TpChannel *) object; MembersChangedData *data = user_data; + TpChannelGroupChangeReason reason; + const gchar *message; GPtrArray *removed; guint i; _tp_channel_contacts_queue_prepare_finish (self, result, NULL, NULL); + reason = tp_asv_get_uint32 (data->details, "change-reason", NULL); + message = tp_asv_get_string (data->details, "message"); + for (i = 0; i < data->added->len; i++) { TpContact *contact = g_ptr_array_index (data->added, i); @@ -523,17 +647,12 @@ members_changed_prepared_cb (GObject *object, { TpContact *contact = g_ptr_array_index (data->local_pending, i); gpointer key = GUINT_TO_POINTER (tp_contact_get_handle (contact)); - TpChannelGroupChangeReason reason; - const gchar *message; g_hash_table_remove (self->priv->group_members_contacts, key); g_hash_table_insert (self->priv->group_local_pending_contacts, key, g_object_ref (contact)); g_hash_table_remove (self->priv->group_remote_pending_contacts, key); - reason = tp_asv_get_uint32 (data->details, "change-reason", NULL); - message = tp_asv_get_string (data->details, "message"); - /* Special-case renaming a local-pending contact, if the * signal is spec-compliant. Keep the old actor/reason/message in * this case */ @@ -603,6 +722,57 @@ members_changed_prepared_cb (GObject *object, g_hash_table_remove (self->priv->group_local_pending_contacts, key); g_hash_table_remove (self->priv->group_local_pending_contact_info, key); g_hash_table_remove (self->priv->group_remote_pending_contacts, key); + + if (contact == self->priv->group_self_contact || + contact == tp_connection_get_self_contact (self->priv->connection)) + { + const gchar *error_detail = tp_asv_get_string (data->details, + "error"); + const gchar *debug_message = tp_asv_get_string (data->details, + "debug-message"); + + if (debug_message == NULL && message[0] != '\0') + debug_message = message; + + if (debug_message == NULL && error_detail != NULL) + debug_message = error_detail; + + if (debug_message == NULL) + debug_message = "(no message provided)"; + + if (self->priv->group_remove_error != NULL) + g_clear_error (&self->priv->group_remove_error); + + if (error_detail != NULL) + { + /* CM specified a D-Bus error name */ + tp_proxy_dbus_error_to_gerror (self, error_detail, + debug_message == NULL || debug_message[0] == '\0' + ? error_detail + : debug_message, + &self->priv->group_remove_error); + + /* ... but if we don't know anything about that D-Bus error + * name, we can still do better by using RemovedFromGroup */ + if (g_error_matches (self->priv->group_remove_error, + TP_DBUS_ERRORS, TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR)) + { + self->priv->group_remove_error->domain = + TP_ERRORS_REMOVED_FROM_GROUP; + self->priv->group_remove_error->code = reason; + + _tp_channel_group_improve_remove_error (self, data->actor); + } + } + else + { + /* Use our separate error domain */ + g_set_error_literal (&self->priv->group_remove_error, + TP_ERRORS_REMOVED_FROM_GROUP, reason, debug_message); + + _tp_channel_group_improve_remove_error (self, data->actor); + } + } } g_signal_emit_by_name (self, "group-contacts-changed", data->added, diff --git a/telepathy-glib/channel-group.c b/telepathy-glib/channel-group.c index 07eafc2..e3e2737 100644 --- a/telepathy-glib/channel-group.c +++ b/telepathy-glib/channel-group.c @@ -615,100 +615,6 @@ tp_channel_got_group_properties_cb (TpProxy *proxy, } } -/* - * If the @group_remove_error is derived from a TpChannelGroupChangeReason, - * attempt to rewrite it into a TpError. - */ -static void -_tp_channel_group_improve_remove_error (TpChannel *self, - TpHandle actor) -{ - GError *error = self->priv->group_remove_error; - - if (error == NULL || error->domain != TP_ERRORS_REMOVED_FROM_GROUP) - return; - - switch (error->code) - { - case TP_CHANNEL_GROUP_CHANGE_REASON_NONE: - if (actor == self->priv->group_self_handle || - actor == tp_contact_get_handle ( - tp_connection_get_self_contact (self->priv->connection))) - { - error->code = TP_ERROR_CANCELLED; - } - else - { - error->code = TP_ERROR_TERMINATED; - } - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_OFFLINE: - error->code = TP_ERROR_OFFLINE; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_KICKED: - error->code = TP_ERROR_CHANNEL_KICKED; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_BUSY: - error->code = TP_ERROR_BUSY; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_INVITED: - DEBUG ("%s: Channel_Group_Change_Reason_Invited makes no sense as a " - "removal reason!", tp_proxy_get_object_path (self)); - error->domain = TP_DBUS_ERRORS; - error->code = TP_DBUS_ERROR_INCONSISTENT; - return; - - case TP_CHANNEL_GROUP_CHANGE_REASON_BANNED: - error->code = TP_ERROR_CHANNEL_BANNED; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_ERROR: - /* hopefully all CMs that use this will also give us an error detail, - * but if they didn't, or gave us one we didn't understand... */ - error->code = TP_ERROR_NOT_AVAILABLE; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_INVALID_CONTACT: - error->code = TP_ERROR_DOES_NOT_EXIST; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_NO_ANSWER: - error->code = TP_ERROR_NO_ANSWER; - break; - - /* TP_CHANNEL_GROUP_CHANGE_REASON_RENAMED shouldn't be the last error - * seen in the channel - we'll get removed again with a real reason, - * later, so there's no point in doing anything special with this one */ - - case TP_CHANNEL_GROUP_CHANGE_REASON_PERMISSION_DENIED: - error->code = TP_ERROR_PERMISSION_DENIED; - break; - - case TP_CHANNEL_GROUP_CHANGE_REASON_SEPARATED: - DEBUG ("%s: Channel_Group_Change_Reason_Separated makes no sense as a " - "removal reason!", tp_proxy_get_object_path (self)); - error->domain = TP_DBUS_ERRORS; - error->code = TP_DBUS_ERROR_INCONSISTENT; - return; - - /* all values up to and including Separated have been checked */ - - default: - /* We don't understand this reason code, so keeping the domain and code - * the same (i.e. using TP_ERRORS_REMOVED_FROM_GROUP) is no worse than - * anything else we could do. */ - return; - } - - /* If we changed the code we also need to change the domain; if not, we did - * an early return, so we'll never reach this */ - error->domain = TP_ERROR; -} - static void handle_members_changed (TpChannel *self, const gchar *message, @@ -821,57 +727,6 @@ handle_members_changed (TpChannel *self, tp_intset_remove (self->priv->group_members, handle); tp_intset_remove (self->priv->group_local_pending, handle); tp_intset_remove (self->priv->group_remote_pending, handle); - - if (handle == self->priv->group_self_handle || - handle == tp_contact_get_handle (tp_connection_get_self_contact ( - self->priv->connection))) - { - const gchar *error_detail = tp_asv_get_string (details, "error"); - const gchar *debug_message = tp_asv_get_string (details, - "debug-message"); - - if (debug_message == NULL && message[0] != '\0') - debug_message = message; - - if (debug_message == NULL && error_detail != NULL) - debug_message = error_detail; - - if (debug_message == NULL) - debug_message = "(no message provided)"; - - if (self->priv->group_remove_error != NULL) - g_clear_error (&self->priv->group_remove_error); - - if (error_detail != NULL) - { - /* CM specified a D-Bus error name */ - tp_proxy_dbus_error_to_gerror (self, error_detail, - debug_message == NULL || debug_message[0] == '\0' - ? error_detail - : debug_message, - &self->priv->group_remove_error); - - /* ... but if we don't know anything about that D-Bus error - * name, we can still do better by using RemovedFromGroup */ - if (g_error_matches (self->priv->group_remove_error, - TP_DBUS_ERRORS, TP_DBUS_ERROR_UNKNOWN_REMOTE_ERROR)) - { - self->priv->group_remove_error->domain = - TP_ERRORS_REMOVED_FROM_GROUP; - self->priv->group_remove_error->code = reason; - - _tp_channel_group_improve_remove_error (self, actor); - } - } - else - { - /* Use our separate error domain */ - g_set_error_literal (&self->priv->group_remove_error, - TP_ERRORS_REMOVED_FROM_GROUP, reason, debug_message); - - _tp_channel_group_improve_remove_error (self, actor); - } - } } g_signal_emit_by_name (self, "group-members-changed", added, diff --git a/tests/dbus/cli-group.c b/tests/dbus/cli-group.c index 0798464..38877e2 100644 --- a/tests/dbus/cli-group.c +++ b/tests/dbus/cli-group.c @@ -213,10 +213,26 @@ check_invalidated_unknown_error_cb (TpProxy *proxy, REMOVED_MESSAGE); } +static void +run_until_members_changed (TpChannel *channel) +{ + GMainLoop *loop = g_main_loop_new (NULL, FALSE); + gulong id; + + id = g_signal_connect_swapped (channel, "group-contacts-changed", + G_CALLBACK (g_main_loop_quit), loop); + + g_main_loop_run (loop); + + g_signal_handler_disconnect (channel, id); + + g_main_loop_unref (loop); +} static void check_removed_unknown_error_in_invalidated (void) { + GQuark features[] = { TP_CHANNEL_FEATURE_CONTACTS, 0 }; gchar *chan_path; TpTestsTextChannelGroup *service_chan; TpChannel *chan; @@ -238,7 +254,7 @@ check_removed_unknown_error_in_invalidated (void) g_assert_no_error (error); - tp_tests_proxy_run_until_prepared (chan, NULL); + tp_tests_proxy_run_until_prepared (chan, features); DEBUG ("ready!"); g_signal_connect (chan, "invalidated", @@ -256,7 +272,7 @@ check_removed_unknown_error_in_invalidated (void) tp_clear_pointer (&details, g_hash_table_unref); - tp_tests_proxy_run_until_dbus_queue_processed (conn); + run_until_members_changed (chan); details = tp_asv_new ( "message", G_TYPE_STRING, REMOVED_MESSAGE, @@ -267,7 +283,7 @@ check_removed_unknown_error_in_invalidated (void) tp_group_mixin_change_members ((GObject *) service_chan, NULL, self_handle_singleton, NULL, NULL, details); - tp_tests_proxy_run_until_dbus_queue_processed (conn); + run_until_members_changed (chan); tp_cli_channel_call_close (chan, -1, NULL, NULL, NULL, NULL); @@ -308,6 +324,7 @@ check_invalidated_known_error_cb (TpProxy *proxy, static void check_removed_known_error_in_invalidated (void) { + GQuark features[] = { TP_CHANNEL_FEATURE_CONTACTS, 0 }; gchar *chan_path; TpTestsTextChannelGroup *service_chan; TpChannel *chan; @@ -329,7 +346,7 @@ check_removed_known_error_in_invalidated (void) g_assert_no_error (error); - tp_tests_proxy_run_until_prepared (chan, NULL); + tp_tests_proxy_run_until_prepared (chan, features); DEBUG ("ready!"); g_signal_connect (chan, "invalidated", @@ -347,7 +364,7 @@ check_removed_known_error_in_invalidated (void) tp_clear_pointer (&details, g_hash_table_unref); - tp_tests_proxy_run_until_dbus_queue_processed (conn); + run_until_members_changed (chan); details = tp_asv_new ( "message", G_TYPE_STRING, REMOVED_MESSAGE, @@ -358,7 +375,7 @@ check_removed_known_error_in_invalidated (void) tp_group_mixin_change_members ((GObject *) service_chan, NULL, self_handle_singleton, NULL, NULL, details); - tp_tests_proxy_run_until_dbus_queue_processed (conn); + run_until_members_changed (chan); tp_cli_channel_call_close (chan, -1, NULL, NULL, NULL, NULL); -- 1.7.9.5