Attachment 88642 Details for Bug 30043 – Remove BypassObservers support

[patch] Remove BypassObservers support

0005-Remove-BypassObservers-support.patch (text/plain), 19.51 KB, created by Simon McVittie on 2013-11-04 18:48:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-11-04 18:48:05 UTC

Size: 19.51 KB

patch

obsolete

>From a622bc81993d2b960bc9ba64a56bc335f000a0ca Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 30043: 88642

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.