From b465d89e88adf1235455bc2df40e5320e8f7cb9c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 20 Jul 2012 12:35:57 +0100 Subject: [PATCH 02/16] McdConnection: dispatch each channel from NewChannels separately This loses information - you can no longer tell the channels are related - but in practice nobody handles Text and Tube channels as a unit anyway, and signalling them together is now deprecated. --- src/mcd-connection.c | 68 +++++------ tests/twisted/dispatcher/exploding-bundles.py | 156 +++++++++++++++++-------- 2 files changed, 139 insertions(+), 85 deletions(-) diff --git a/src/mcd-connection.c b/src/mcd-connection.c index d71c5f5..1d95815 100644 --- a/src/mcd-connection.c +++ b/src/mcd-connection.c @@ -1221,7 +1221,8 @@ mcd_connection_find_channel_by_path (McdConnection *connection, } static gboolean mcd_connection_need_dispatch (McdConnection *connection, - const GPtrArray *channels); + const gchar *object_path, + GHashTable *props); static void on_new_channels (TpConnection *proxy, const GPtrArray *channels, @@ -1229,10 +1230,6 @@ on_new_channels (TpConnection *proxy, const GPtrArray *channels, { McdConnection *connection = MCD_CONNECTION (weak_object); McdConnectionPrivate *priv = user_data; - McdChannel *channel; - GList *channel_list = NULL; - gboolean requested = FALSE; - gboolean only_observe = FALSE; guint i; if (DEBUGGING) @@ -1263,8 +1260,6 @@ on_new_channels (TpConnection *proxy, const GPtrArray *channels, * FALSE: they'll also be in Channels in the GetAll(Requests) result */ if (!priv->dispatched_initial_channels) return; - only_observe = !mcd_connection_need_dispatch (connection, channels); - sp_timestamp ("NewChannels received"); for (i = 0; i < channels->len; i++) { @@ -1272,11 +1267,18 @@ on_new_channels (TpConnection *proxy, const GPtrArray *channels, const gchar *object_path; GHashTable *props; GValue *value; + gboolean requested = FALSE; + gboolean only_observe = FALSE; + GList *channel_list = NULL; + McdChannel *channel; va = g_ptr_array_index (channels, i); object_path = g_value_get_boxed (va->values); props = g_value_get_boxed (va->values + 1); + only_observe = !mcd_connection_need_dispatch (connection, object_path, + props); + /* Don't do anything for requested channels */ value = g_hash_table_lookup (props, TP_IFACE_CHANNEL ".Requested"); if (value && g_value_get_boolean (value)) @@ -1296,16 +1298,16 @@ on_new_channels (TpConnection *proxy, const GPtrArray *channels, } channel_list = g_list_prepend (channel_list, channel); - } - if (!requested) - { - /* we always dispatch unrequested (incoming) channels */ - only_observe = FALSE; - } + if (!requested) + { + /* we always dispatch unrequested (incoming) channels */ + only_observe = FALSE; + } - _mcd_dispatcher_take_channels (priv->dispatcher, channel_list, requested, - only_observe); + _mcd_dispatcher_take_channels (priv->dispatcher, channel_list, + requested, only_observe); + } } static void @@ -2131,18 +2133,19 @@ _mcd_connection_get_property (GObject * obj, guint prop_id, /* * mcd_connection_need_dispatch: * @connection: the #McdConnection. - * @channels: array of #McdChannel elements. + * @object_path: the object path of the new channel (only for debugging) + * @props: the properties of the new channel * * This functions must be called in response to a NewChannels signals, and is * responsible for deciding whether MC must handle the channels or not. */ static gboolean mcd_connection_need_dispatch (McdConnection *connection, - const GPtrArray *channels) + const gchar *object_path, + GHashTable *props) { McdAccount *account = mcd_connection_get_account (connection); - gboolean any_requested = FALSE, requested_by_us = FALSE; - guint i; + gboolean requested = FALSE, requested_by_us = FALSE; if (_mcd_account_needs_dispatch (account)) { @@ -2155,31 +2158,18 @@ mcd_connection_need_dispatch (McdConnection *connection, * have no McdChannel object associated: these are the channels directly * requested to the CM by some other application, and we must ignore them */ - for (i = 0; i < channels->len; i++) - { - GValueArray *va; - const gchar *object_path; - GHashTable *props; - gboolean requested; - - va = g_ptr_array_index (channels, i); - object_path = g_value_get_boxed (va->values); - props = g_value_get_boxed (va->values + 1); - - requested = tp_asv_get_boolean (props, TP_IFACE_CHANNEL ".Requested", - NULL); - if (requested) - { - any_requested = TRUE; - if (mcd_connection_find_channel_by_path (connection, object_path)) - requested_by_us = TRUE; - } + requested = tp_asv_get_boolean (props, TP_IFACE_CHANNEL ".Requested", + NULL); + if (requested) + { + if (mcd_connection_find_channel_by_path (connection, object_path)) + requested_by_us = TRUE; } /* handle only bundles which were not requested or that were requested * through MC */ - return !any_requested || requested_by_us; + return !requested || requested_by_us; } gboolean diff --git a/tests/twisted/dispatcher/exploding-bundles.py b/tests/twisted/dispatcher/exploding-bundles.py index a62a700..27d3be3 100644 --- a/tests/twisted/dispatcher/exploding-bundles.py +++ b/tests/twisted/dispatcher/exploding-bundles.py @@ -107,34 +107,30 @@ def test(q, bus, mc): conn.NewChannels([text_chan, media_chan]) - # A channel dispatch operation is created + # A channel dispatch operation is created for the Text channel first. e = q.expect('dbus-signal', path=cs.CD_PATH, interface=cs.CD_IFACE_OP_LIST, signal='NewDispatchOperation') - cdo_path = e.args[0] - cdo_properties = e.args[1] + text_cdo_path = e.args[0] + text_cdo_properties = e.args[1] - assert cdo_properties[cs.CDO + '.Account'] == account.object_path - assert cdo_properties[cs.CDO + '.Connection'] == conn.object_path + assert text_cdo_properties[cs.CDO + '.Account'] == account.object_path + assert text_cdo_properties[cs.CDO + '.Connection'] == conn.object_path - handlers = cdo_properties[cs.CDO + '.PossibleHandlers'][:] - # only Empathy can handle the whole batch - assert handlers == [cs.tp_name_prefix + '.Client.org.gnome.Empathy'], \ - handlers + handlers = text_cdo_properties[cs.CDO + '.PossibleHandlers'][:] + assert (sorted(handlers) == + [cs.tp_name_prefix + '.Client.org.gnome.Empathy', + cs.tp_name_prefix + '.Client.org.kde.Kopete']), handlers - assert cs.CD_IFACE_OP_LIST in cd_props.Get(cs.CD, 'Interfaces') - assert cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations') ==\ - [(cdo_path, cdo_properties)] - - cdo = bus.get_object(cs.CD, cdo_path) - cdo_iface = dbus.Interface(cdo, cs.CDO) + text_cdo = bus.get_object(cs.CD, text_cdo_path) + text_cdo_iface = dbus.Interface(text_cdo, cs.CDO) - # Both Observers are told about the new channels + # Both Observers are told about the new Text channel - e, k = q.expect_many( + e_observe_text, k_observe_text = q.expect_many( EventPattern('dbus-method-call', path=empathy.object_path, interface=cs.OBSERVER, method='ObserveChannels', @@ -144,62 +140,130 @@ def test(q, bus, mc): interface=cs.OBSERVER, method='ObserveChannels', handled=False), ) - assert e.args[0] == account.object_path, e.args - assert e.args[1] == conn.object_path, e.args - assert e.args[3] == cdo_path, e.args - assert e.args[4] == [], e.args # no requests satisfied - channels = e.args[2] - assert len(channels) == 2, channels + assert e_observe_text.args[0] == account.object_path, e_observe_text.args + assert e_observe_text.args[1] == conn.object_path, e_observe_text.args + assert e_observe_text.args[3] == text_cdo_path, e_observe_text.args + assert e_observe_text.args[4] == [], e_observe_text.args + channels = e_observe_text.args[2] + assert len(channels) == 1, channels assert (text_chan.object_path, text_channel_properties) in channels + + assert k_observe_text.args[0] == e_observe_text.args[0], k_observe_text.args + assert k_observe_text.args[1] == e_observe_text.args[1], k_observe_text.args + assert (k_observe_text.args[2] == + [(text_chan.object_path, text_channel_properties)]) + + # Now a separate CDO is created for the media channel. + + e = q.expect('dbus-signal', + path=cs.CD_PATH, + interface=cs.CD_IFACE_OP_LIST, + signal='NewDispatchOperation') + + media_cdo_path = e.args[0] + media_cdo_properties = e.args[1] + + assert media_cdo_properties[cs.CDO + '.Account'] == account.object_path + assert media_cdo_properties[cs.CDO + '.Connection'] == conn.object_path + + handlers = media_cdo_properties[cs.CDO + '.PossibleHandlers'][:] + # only Empathy can handle it + assert (sorted(handlers) == + [cs.tp_name_prefix + '.Client.org.gnome.Empathy']), handlers + + assert cs.CD_IFACE_OP_LIST in cd_props.Get(cs.CD, 'Interfaces') + assert (sorted(cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations')) == + [(text_cdo_path, text_cdo_properties), + (media_cdo_path, media_cdo_properties)]) + + media_cdo = bus.get_object(cs.CD, media_cdo_path) + media_cdo_iface = dbus.Interface(media_cdo, cs.CDO) + + # Only Empathy is told about the new media channel + + e_observe_media = q.expect('dbus-method-call', + path=empathy.object_path, + interface=cs.OBSERVER, method='ObserveChannels', + handled=False) + assert e_observe_media.args[0] == account.object_path, e_observe_media.args + assert e_observe_media.args[1] == conn.object_path, e_observe_media.args + assert e_observe_media.args[3] == media_cdo_path, e_observe_media.args + assert e_observe_media.args[4] == [], e_observe_media.args + channels = e_observe_media.args[2] + assert len(channels) == 1, channels assert (media_chan.object_path, media_channel_properties) in channels - # fd.o #21089: telepathy-spec doesn't say whether Kopete observes the whole - # batch or just the text channel. In current MC, it only observes the text. - assert k.args[0] == e.args[0], k.args - assert k.args[1] == e.args[1], e.args - assert k.args[2] == [(text_chan.object_path, text_channel_properties)] + # All Observers reply. - # Both Observers indicate that they are ready to proceed - q.dbus_return(k.message, signature='') - q.dbus_return(e.message, signature='') + q.dbus_return(e_observe_text.message, signature='') + q.dbus_return(k_observe_text.message, signature='') + q.dbus_return(e_observe_media.message, signature='') # The Approvers are next - # fd.o #21090: telepathy-spec doesn't say whether Kopete is asked to - # approve this CDO. In current MC, it is. - e, k = q.expect_many( + e_approve_text, k_approve_text, e_approve_media = q.expect_many( EventPattern('dbus-method-call', path=empathy.object_path, interface=cs.APPROVER, method='AddDispatchOperation', + predicate=lambda e: e.args[1] == text_cdo_path, handled=False), EventPattern('dbus-method-call', path=kopete.object_path, interface=cs.APPROVER, method='AddDispatchOperation', handled=False), + EventPattern('dbus-method-call', + path=empathy.object_path, + interface=cs.APPROVER, method='AddDispatchOperation', + predicate=lambda e: e.args[1] == media_cdo_path, + handled=False) ) - assert len(e.args[0]) == 2 - assert (text_chan.object_path, text_channel_properties) in e.args[0] - assert (media_chan.object_path, media_channel_properties) in e.args[0] - assert e.args[1:] == [cdo_path, cdo_properties] - assert k.args == e.args + assert len(e_approve_text.args[0]) == 1 + assert ((text_chan.object_path, text_channel_properties) in + e_approve_text.args[0]) + assert e_approve_text.args[1:] == [text_cdo_path, text_cdo_properties] + assert k_approve_text.args == e_approve_text.args - q.dbus_return(e.message, signature='') - q.dbus_return(k.message, signature='') + assert len(e_approve_media.args[0]) == 1 + assert ((media_chan.object_path, media_channel_properties) in + e_approve_media.args[0]) + assert e_approve_media.args[1:] == [media_cdo_path, media_cdo_properties] + + q.dbus_return(e_approve_text.message, signature='') + q.dbus_return(k_approve_text.message, signature='') + q.dbus_return(e_approve_media.message, signature='') # Both Approvers now have a flashing icon or something, trying to get the - # user's attention + # user's attention. The user clicks on Empathy + call_async(q, text_cdo_iface, 'HandleWith', + cs.tp_name_prefix + '.Client.org.gnome.Empathy') + + # Empathy is asked to handle the channel + e = q.expect('dbus-method-call', + path=empathy.object_path, + interface=cs.HANDLER, method='HandleChannels', + handled=False) + + # Empathy accepts the channel + q.dbus_return(e.message, signature='') + + q.expect_many( + EventPattern('dbus-return', method='HandleWith'), + EventPattern('dbus-signal', interface=cs.CDO, signal='Finished'), + EventPattern('dbus-signal', interface=cs.CD_IFACE_OP_LIST, + signal='DispatchOperationFinished'), + ) - # The user doesn't care which one will handle the channels - because + # The user doesn't care which client will handle the channel - because # Empathy is the only possibility, it will be chosen (this is also a # regression test for the ability to leave the handler unspecified). - call_async(q, cdo_iface, 'HandleWith', '') + call_async(q, media_cdo_iface, 'HandleWith', '') - # Empathy is asked to handle the channels + # Empathy is asked to handle the channel e = q.expect('dbus-method-call', path=empathy.object_path, interface=cs.HANDLER, method='HandleChannels', handled=False) - # Empathy accepts the channels + # Empathy accepts the channel q.dbus_return(e.message, signature='') q.expect_many( -- 1.7.10.4