From a622bc81993d2b960bc9ba64a56bc335f000a0ca Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 4 Nov 2013 18:46:22 +0000 Subject: [PATCH 5/5] Remove BypassObservers support It wasn't implemented correctly (see bug #40305) and I wasn't convinced by the design (see bug #30043). Let's think about it more before we bring it back. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=30043 Bug: https://bugs.freedesktop.org/show_bug.cgi?id=40305 --- src/mcd-client-priv.h | 2 - src/mcd-client.c | 17 -- src/mcd-dispatch-operation.c | 41 +--- src/mcd-dispatcher.c | 10 - tests/twisted/Makefile.am | 1 - tests/twisted/dispatcher/bypass-observers.py | 291 --------------------------- tests/twisted/mctest.py | 9 +- 7 files changed, 3 insertions(+), 368 deletions(-) delete mode 100644 tests/twisted/dispatcher/bypass-observers.py diff --git a/src/mcd-client-priv.h b/src/mcd-client-priv.h index c00ce13..069d2a6 100644 --- a/src/mcd-client-priv.h +++ b/src/mcd-client-priv.h @@ -97,8 +97,6 @@ G_GNUC_INTERNAL const GList *_mcd_client_proxy_get_handler_filters (McdClientProxy *self); G_GNUC_INTERNAL gboolean _mcd_client_proxy_get_bypass_approval (McdClientProxy *self); -G_GNUC_INTERNAL gboolean _mcd_client_proxy_get_bypass_observers - (McdClientProxy *self); G_GNUC_INTERNAL gboolean _mcd_client_proxy_get_delay_approvers (McdClientProxy *self); diff --git a/src/mcd-client.c b/src/mcd-client.c index 455cfd7..ec5c504 100644 --- a/src/mcd-client.c +++ b/src/mcd-client.c @@ -68,7 +68,6 @@ struct _McdClientProxyPrivate gboolean introspect_started; gboolean ready; gboolean bypass_approval; - gboolean bypass_observers; gboolean delay_approvers; gboolean recover; @@ -400,10 +399,6 @@ parse_client_file (McdClientProxy *client, g_key_file_get_boolean (file, TP_IFACE_CLIENT_HANDLER, "BypassApproval", NULL); - client->priv->bypass_observers = - g_key_file_get_boolean (file, TP_IFACE_CLIENT_HANDLER, - "BypassObservers", NULL); - client->priv->delay_approvers = g_key_file_get_boolean (file, TP_IFACE_CLIENT_OBSERVER, "DelayApprovers", NULL); @@ -664,10 +659,6 @@ _mcd_client_proxy_handler_get_all_cb (TpProxy *proxy, self->priv->bypass_approval = bypass; DEBUG ("%s has BypassApproval=%c", bus_name, bypass ? 'T' : 'F'); - bypass = tp_asv_get_boolean (properties, "BypassObservers", NULL); - self->priv->bypass_observers = bypass; - DEBUG ("%s has BypassObservers=%c", bus_name, bypass ? 'T' : 'F'); - /* don't emit handler-capabilities-changed if we're not actually available * any more - if that's the case, then we already signalled our loss of * any capabilities */ @@ -1369,14 +1360,6 @@ _mcd_client_proxy_get_bypass_approval (McdClientProxy *self) } gboolean -_mcd_client_proxy_get_bypass_observers (McdClientProxy *self) -{ - g_return_val_if_fail (MCD_IS_CLIENT_PROXY (self), FALSE); - - return self->priv->bypass_observers; -} - -gboolean _mcd_client_proxy_get_delay_approvers (McdClientProxy *self) { g_return_val_if_fail (MCD_IS_CLIENT_PROXY (self), FALSE); diff --git a/src/mcd-dispatch-operation.c b/src/mcd-dispatch-operation.c index 020d6b8..5299a5b 100644 --- a/src/mcd-dispatch-operation.c +++ b/src/mcd-dispatch-operation.c @@ -388,8 +388,6 @@ static void mcd_dispatch_operation_set_channel_handled_by ( const gchar *well_known_name); static gboolean _mcd_dispatch_operation_handlers_can_bypass_approval ( McdDispatchOperation *self); -static gboolean _mcd_dispatch_operation_handlers_can_bypass_observers ( - McdDispatchOperation *self); static void _mcd_dispatch_operation_check_client_locks (McdDispatchOperation *self) @@ -1828,34 +1826,6 @@ _mcd_dispatch_operation_handlers_can_bypass_approval ( return FALSE; } -/* this is analogous to *_can_bypass_handlers() method above */ -static gboolean -_mcd_dispatch_operation_handlers_can_bypass_observers ( - McdDispatchOperation *self) -{ - gchar **iter; - - for (iter = self->priv->possible_handlers; - iter != NULL && *iter != NULL; - iter++) - { - McdClientProxy *handler = _mcd_client_registry_lookup ( - self->priv->client_registry, *iter); - - if (handler != NULL) - { - gboolean bypass = _mcd_client_proxy_get_bypass_observers ( - handler); - - DEBUG ("%s has BypassObservers=%c", *iter, bypass ? 'T' : 'F'); - return bypass; - } - } - - return FALSE; -} - - gboolean _mcd_dispatch_operation_has_channel (McdDispatchOperation *self, McdChannel *channel) @@ -2240,15 +2210,8 @@ _mcd_dispatch_operation_run_clients (McdDispatchOperation *self) { const GList *mini_plugins; - if (_mcd_dispatch_operation_handlers_can_bypass_observers (self)) - { - DEBUG ("Bypassing observers"); - } - else - { - DEBUG ("Running observers"); - _mcd_dispatch_operation_run_observers (self); - } + DEBUG ("Running observers"); + _mcd_dispatch_operation_run_observers (self); for (mini_plugins = mcp_list_objects (); mini_plugins != NULL; diff --git a/src/mcd-dispatcher.c b/src/mcd-dispatcher.c index 3184446..8275e5e 100644 --- a/src/mcd-dispatcher.c +++ b/src/mcd-dispatcher.c @@ -589,17 +589,7 @@ mcd_dispatcher_client_needs_recovery_cb (McdClientProxy *client, for (list = channels; list; list = list->next) { TpChannel *channel = list->data; - const gchar *object_path = tp_proxy_get_object_path (channel); GVariant *properties; - McdClientProxy *handler; - - /* FIXME: This is not exactly the right behaviour, see fd.o#40305 */ - handler = _mcd_dispatcher_lookup_handler (self, channel, NULL); - if (handler && _mcd_client_proxy_get_bypass_observers (handler)) - { - DEBUG ("skipping unobservable channel %s", object_path); - continue; - } properties = tp_channel_dup_immutable_properties (channel); diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am index d62c521..480f62a 100644 --- a/tests/twisted/Makefile.am +++ b/tests/twisted/Makefile.am @@ -29,7 +29,6 @@ TWISTED_BASIC_TESTS = \ dispatcher/already-has-channel.py \ dispatcher/approver-fails.py \ dispatcher/bypass-approval.py \ - dispatcher/bypass-observers.py \ dispatcher/cancel.py \ dispatcher/capture-bundle.py \ dispatcher/cdo-claim.py \ diff --git a/tests/twisted/dispatcher/bypass-observers.py b/tests/twisted/dispatcher/bypass-observers.py deleted file mode 100644 index 1aeb318..0000000 --- a/tests/twisted/dispatcher/bypass-observers.py +++ /dev/null @@ -1,291 +0,0 @@ -# Copyright (C) 2009-2010 Nokia Corporation -# Copyright (C) 2009-2010 Collabora Ltd. -# -# This library is free software; you can redistribute it and/or -# modify it under the terms of the GNU Lesser General Public -# License as published by the Free Software Foundation; either -# version 2.1 of the License, or (at your option) any later version. -# -# This library is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -# Lesser General Public License for more details. -# -# You should have received a copy of the GNU Lesser General Public -# License along with this library; if not, write to the Free Software -# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA -# 02110-1301 USA - -import dbus -"""Regression test for dispatching an incoming Text channel with bypassed -observers. -""" - -import dbus -import dbus.service - -from servicetest import EventPattern, tp_name_prefix, tp_path_prefix, \ - call_async, sync_dbus, assertEquals, assertLength, assertContains -from mctest import exec_test, SimulatedConnection, SimulatedClient, \ - create_fakecm_account, enable_fakecm_account, SimulatedChannel, \ - expect_client_setup -import constants as cs - -text_fixed_properties = dbus.Dictionary({ - cs.CHANNEL + '.ChannelType': cs.CHANNEL_TYPE_TEXT, - }, signature='sv') -contact_text_fixed_properties = dbus.Dictionary({ - cs.CHANNEL + '.TargetHandleType': cs.HT_CONTACT, - cs.CHANNEL + '.ChannelType': cs.CHANNEL_TYPE_TEXT, - }, signature='sv') -secret_fixed_properties = dbus.Dictionary({ - cs.CHANNEL + '.TargetHandleType': cs.HT_CONTACT, - cs.CHANNEL + '.ChannelType': cs.CHANNEL_TYPE_TEXT, - 'com.example.Secrecy.Secret': True, - }, signature='sv') - -def announce_common(q, bus, empathy, kopete, account, conn, cd_props, - secret=False): - if secret: - jid = 'friar.lawrence' - else: - jid = 'juliet' - - channel_properties = dbus.Dictionary(contact_text_fixed_properties, - signature='sv') - channel_properties[cs.CHANNEL + '.TargetID'] = jid - channel_properties[cs.CHANNEL + '.TargetHandle'] = \ - conn.ensure_handle(cs.HT_CONTACT, jid) - channel_properties[cs.CHANNEL + '.InitiatorID'] = jid - channel_properties[cs.CHANNEL + '.InitiatorHandle'] = \ - conn.ensure_handle(cs.HT_CONTACT, jid) - channel_properties[cs.CHANNEL + '.Requested'] = False - channel_properties[cs.CHANNEL + '.Interfaces'] = dbus.Array(signature='s') - - if secret: - channel_properties['com.example.Secrecy.Secret'] = True - - chan = SimulatedChannel(conn, channel_properties) - chan.announce() - - # A channel dispatch operation is created - - 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] - - assertEquals(cdo_properties[cs.CDO + '.Account'], account.object_path) - assertEquals(cdo_properties[cs.CDO + '.Connection'], conn.object_path) - assertContains(cs.CDO + '.Interfaces', cdo_properties) - - handlers = cdo_properties[cs.CDO + '.PossibleHandlers'][:] - - if secret: - # The handler with BypassApproval is first - assertEquals(cs.tp_name_prefix + '.Client.Kopete.Bypasser', - handlers[0]) - else: - handlers.sort() - assertEquals([cs.tp_name_prefix + '.Client.Empathy', - cs.tp_name_prefix + '.Client.Kopete'], handlers) - - assertContains(cs.CD_IFACE_OP_LIST, cd_props.Get(cs.CD, 'Interfaces')) - - assertEquals([(cdo_path, cdo_properties)], - cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations')) - - cdo = bus.get_object(cs.CD, cdo_path) - cdo_iface = dbus.Interface(cdo, cs.CDO) - - # Both Observers are told about the new channel - - if secret: - observe_events = [] - else: - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False), - ) - assertEquals(account.object_path, e.args[0]) - assertEquals(conn.object_path, e.args[1]) - assertEquals(cdo_path, e.args[3]) - assertEquals([], e.args[4]) # no requests satisfied - channels = e.args[2] - assertLength(1, channels) - assertEquals(chan.object_path, channels[0][0]) - assertEquals(channel_properties, channels[0][1]) - - assertEquals(k.args, e.args) - observe_events = [e, k] - - return cdo_iface, chan, channel_properties, observe_events - -def expect_and_exercise_approval(q, bus, chan, channel_properties, - empathy, kopete, cdo_iface, cd_props): - # The Approvers are next - - e, k = q.expect_many( - EventPattern('dbus-method-call', - path=empathy.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - EventPattern('dbus-method-call', - path=kopete.object_path, - interface=cs.APPROVER, method='AddDispatchOperation', - handled=False), - ) - - assertEquals([(chan.object_path, channel_properties)], e.args[0]) - assertEquals(k.args, e.args) - - # Both Approvers indicate that they are ready to proceed - q.dbus_return(e.message, signature='') - q.dbus_return(k.message, signature='') - - # Both Approvers now have a flashing icon or something, trying to get the - # user's attention - - # The user responds to Kopete first - call_async(q, cdo_iface, 'HandleWith', - cs.tp_name_prefix + '.Client.Kopete') - - # Kopete is asked to handle the channels - e = q.expect('dbus-method-call', - path=kopete.object_path, - interface=cs.HANDLER, method='HandleChannels', - handled=False) - - # Kopete accepts the channels - 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'), - ) - - # Now there are no more active channel dispatch operations - assertEquals([], cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations')) - - -def test(q, bus, mc): - params = dbus.Dictionary({"account": "someguy@example.com", - "password": "secrecy"}, signature='sv') - simulated_cm, account = create_fakecm_account(q, bus, mc, params) - conn = enable_fakecm_account(q, bus, mc, account, params) - - # Two clients want to observe, approve and handle channels. Additionally, - # Kopete recognises a "Secret" flag on certain incoming channels, and - # wants to bypass approval and observers for them. Also, Empathy is a - # respawnable observer, which wants to get notified of existing channels - # if it gets restarted. - empathy = SimulatedClient(q, bus, 'Empathy', - observe=[text_fixed_properties], approve=[text_fixed_properties], - handle=[text_fixed_properties], bypass_approval=False, - wants_recovery=True) - kopete = SimulatedClient(q, bus, 'Kopete', - observe=[contact_text_fixed_properties], - approve=[contact_text_fixed_properties], - handle=[contact_text_fixed_properties], bypass_approval=False) - bypass = SimulatedClient(q, bus, 'Kopete.Bypasser', - observe=[], approve=[], - handle=[secret_fixed_properties], - bypass_approval=True, bypass_observers=True) - - # wait for MC to download the properties - expect_client_setup(q, [empathy, kopete, bypass]) - - # subscribe to the OperationList interface (MC assumes that until this - # property has been retrieved once, nobody cares) - - cd = bus.get_object(cs.CD, cs.CD_PATH) - cd_props = dbus.Interface(cd, cs.PROPERTIES_IFACE) - assertEquals([], cd_props.Get(cs.CD_IFACE_OP_LIST, 'DispatchOperations')) - - # First, a non-secret channel is created - - cdo_iface, chan, channel_properties, observe_events = announce_common(q, - bus, empathy, kopete, account, conn, cd_props, False) - - # Both Observers indicate that they are ready to proceed - for e in observe_events: - q.dbus_return(e.message, signature='') - - expect_and_exercise_approval(q, bus, chan, channel_properties, - empathy, kopete, cdo_iface, cd_props) - - nonsecret_chan = chan - - # Now a channel that bypasses approval and observers comes in. - # During this process, we should never be asked to approve or - # observe anything. - - approval = [ - EventPattern('dbus-method-call', method='AddDispatchOperation'), - ] - - q.forbid_events(approval) - - cdo_iface, chan, channel_properties, observe_events = announce_common(q, - bus, empathy, kopete, account, conn, cd_props, True) - - # Both Observers indicate that they are ready to proceed - for e in observe_events: - q.dbus_return(e.message, signature='') - - # Kopete's BypassApproval part is asked to handle the channels - e = q.expect('dbus-method-call', - path=bypass.object_path, - interface=cs.HANDLER, method='HandleChannels', - handled=False) - # Kopete accepts the channels - q.dbus_return(e.message, signature='') - - q.unforbid_events(approval) - - # Empathy, the observer, crashes - empathy.release_name() - - e = q.expect('dbus-signal', - signal='NameOwnerChanged', - predicate=(lambda e: - e.args[0] == empathy.bus_name and e.args[2] == ''), - ) - empathy_unique_name = e.args[1] - - bus.flush() - - # Empathy gets restarted - empathy.reacquire_name() - - e = q.expect('dbus-signal', - signal='NameOwnerChanged', - predicate=(lambda e: - e.args[0] == empathy.bus_name and e.args[1] == ''), - ) - empathy_unique_name = e.args[2] - - # Empathy is told to observe only the non-secret channel - e = q.expect('dbus-method-call', - path=empathy.object_path, - interface=cs.OBSERVER, method='ObserveChannels', - handled=False) - - channels = e.args[2] - assertLength(1, channels) - assertEquals(nonsecret_chan.object_path, channels[0][0]) - -if __name__ == '__main__': - exec_test(test, {}) - diff --git a/tests/twisted/mctest.py b/tests/twisted/mctest.py index b093ee8..35ccec9 100644 --- a/tests/twisted/mctest.py +++ b/tests/twisted/mctest.py @@ -845,7 +845,7 @@ class SimulatedClient(object): observe=[], approve=[], handle=[], cap_tokens=[], bypass_approval=False, wants_recovery=False, request_notification=True, implement_get_interfaces=True, - is_handler=None, bypass_observers=False, delay_approvers=False): + is_handler=None, delay_approvers=False): self.q = q self.bus = bus self.bus_name = '.'.join([cs.tp_name_prefix, 'Client', clientname]) @@ -855,7 +855,6 @@ class SimulatedClient(object): self.approve = aasv(approve) self.handle = aasv(handle) self.bypass_approval = bool(bypass_approval) - self.bypass_observers = bool(bypass_observers) self.delay_approvers = bool(delay_approvers) self.wants_recovery = bool(wants_recovery) self.request_notification = bool(request_notification) @@ -971,7 +970,6 @@ class SimulatedClient(object): self.q.dbus_return(e.message, { 'HandlerChannelFilter': self.handle, 'BypassApproval': self.bypass_approval, - 'BypassObservers': self.bypass_observers, 'HandledChannels': self.handled_channels, 'Capabilities': self.cap_tokens, }, @@ -995,11 +993,6 @@ class SimulatedClient(object): self.q.dbus_return(e.message, self.bypass_approval, signature='v', bus=self.bus) - def Get_BypassApproval(self, e): - assert self.handle - self.q.dbus_return(e.message, self.bypass_observers, signature='v', - bus=self.bus) - def Get_Recover(self, e): assert self.handle self.q.dbus_return(e.message, self.recover, signature='v', -- 1.8.4.2