[telepathy-mission-control/master] fd.o #21174: break AddDispatchOperation API
Simon McVittie
simon.mcvittie at collabora.co.uk
Mon Apr 20 08:04:50 PDT 2009
This is necessary because the previous API assumed that Channels was not
mutable, which is untrue.
---
src/mcd-dispatch-operation.c | 4 +
src/mcd-dispatcher.c | 7 ++-
test/twisted/dispatcher/already-has-channel.py | 8 +-
test/twisted/dispatcher/dispatch-text.py | 8 +-
test/twisted/dispatcher/exploding-bundles.py | 34 +++-----
test/twisted/dispatcher/lose-text.py | 8 +-
xml/Client_Approver.xml | 112 +++++++++++++++++-------
7 files changed, 113 insertions(+), 68 deletions(-)
diff --git a/src/mcd-dispatch-operation.c b/src/mcd-dispatch-operation.c
index 9702d0f..7a3ab06 100644
--- a/src/mcd-dispatch-operation.c
+++ b/src/mcd-dispatch-operation.c
@@ -542,6 +542,10 @@ mcd_dispatch_operation_get_properties (McdDispatchOperation *operation)
if (!property->getprop) continue;
+ /* The Channels property is mutable, so cannot be returned
+ * here */
+ if (!tp_strdiff (property->name, "Channels")) continue;
+
value = g_slice_new0 (GValue);
property->getprop ((TpSvcDBusProperties *)operation,
property->name, value);
diff --git a/src/mcd-dispatcher.c b/src/mcd-dispatcher.c
index e3a4ea5..e54b0e6 100644
--- a/src/mcd-dispatcher.c
+++ b/src/mcd-dispatcher.c
@@ -1430,6 +1430,7 @@ mcd_dispatcher_run_approvers (McdDispatcherContext *context)
g_hash_table_iter_init (&iter, priv->clients);
while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &client))
{
+ GPtrArray *channel_details;
const gchar *dispatch_operation;
GHashTable *properties;
gboolean matched = FALSE;
@@ -1454,16 +1455,20 @@ mcd_dispatcher_run_approvers (McdDispatcherContext *context)
mcd_dispatch_operation_get_path (context->operation);
properties =
mcd_dispatch_operation_get_properties (context->operation);
+ channel_details =
+ _mcd_dispatch_operation_dup_channel_details (context->operation);
context->approvers_invoked++;
_mcd_dispatch_operation_block_finished (context->operation);
mcd_dispatcher_context_ref (context);
mc_cli_client_approver_call_add_dispatch_operation (client->proxy, -1,
- dispatch_operation, properties,
+ channel_details, dispatch_operation, properties,
add_dispatch_operation_cb,
context, (GDestroyNotify)mcd_dispatcher_context_unref,
(GObject *)context->dispatcher);
+
+ g_boxed_free (TP_ARRAY_TYPE_CHANNEL_DETAILS_LIST, channel_details);
}
/* This matches the approvers count set to 1 at the beginning of the
diff --git a/test/twisted/dispatcher/already-has-channel.py b/test/twisted/dispatcher/already-has-channel.py
index c63c8b6..81c66d3 100644
--- a/test/twisted/dispatcher/already-has-channel.py
+++ b/test/twisted/dispatcher/already-has-channel.py
@@ -148,9 +148,6 @@ def test(q, bus, mc):
assert handlers == [cs.tp_name_prefix + '.Client.Empathy',
cs.tp_name_prefix + '.Client.Kopete'], handlers
- assert cdo_properties[cs.CDO + '.Channels'] == [(chan.object_path,
- channel_properties)]
-
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)]
@@ -196,8 +193,9 @@ def test(q, bus, mc):
handled=False),
)
- assert e.args == [cdo_path, cdo_properties]
- assert k.args == [cdo_path, cdo_properties]
+ assert e.args == [[(chan.object_path, channel_properties)],
+ cdo_path, cdo_properties]
+ assert k.args == e.args
q.dbus_return(e.message, signature='')
q.dbus_return(k.message, signature='')
diff --git a/test/twisted/dispatcher/dispatch-text.py b/test/twisted/dispatcher/dispatch-text.py
index c43f577..3635b72 100644
--- a/test/twisted/dispatcher/dispatch-text.py
+++ b/test/twisted/dispatcher/dispatch-text.py
@@ -106,9 +106,6 @@ def test(q, bus, mc):
assert handlers == [cs.tp_name_prefix + '.Client.Empathy',
cs.tp_name_prefix + '.Client.Kopete'], handlers
- assert cdo_properties[cs.CDO + '.Channels'] == [(chan.object_path,
- channel_properties)]
-
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)]
@@ -154,8 +151,9 @@ def test(q, bus, mc):
handled=False),
)
- assert e.args == [cdo_path, cdo_properties]
- assert k.args == [cdo_path, cdo_properties]
+ assert e.args == [[(chan.object_path, channel_properties)],
+ cdo_path, cdo_properties]
+ assert k.args == e.args
q.dbus_return(e.message, signature='')
q.dbus_return(k.message, signature='')
diff --git a/test/twisted/dispatcher/exploding-bundles.py b/test/twisted/dispatcher/exploding-bundles.py
index 8222b07..f0d8fa0 100644
--- a/test/twisted/dispatcher/exploding-bundles.py
+++ b/test/twisted/dispatcher/exploding-bundles.py
@@ -139,12 +139,6 @@ def test(q, bus, mc):
assert handlers == [cs.tp_name_prefix + '.Client.org.gnome.Empathy'], \
handlers
- assert len(cdo_properties[cs.CDO + '.Channels']) == 2
- assert (text_chan.object_path, text_channel_properties) in \
- cdo_properties[cs.CDO + '.Channels']
- assert (media_chan.object_path, media_channel_properties) in \
- cdo_properties[cs.CDO + '.Channels']
-
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)]
@@ -194,9 +188,11 @@ def test(q, bus, mc):
interface=cs.APPROVER, method='AddDispatchOperation',
handled=False),
)
-
- assert e.args == [cdo_path, cdo_properties]
- assert k.args == [cdo_path, cdo_properties]
+ 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
q.dbus_return(e.message, signature='')
q.dbus_return(k.message, signature='')
@@ -275,7 +271,7 @@ def test(q, bus, mc):
# CDO here - we look at the others later.
e_observe_media, e_observe_text, k_observe_text, \
e_approve_media, e_approve_text, k_approve_text, \
- _, _, _, _ = q.expect_many(
+ _, _, _ = q.expect_many(
EventPattern('dbus-method-call',
path=empathy.object_path,
interface=cs.OBSERVER, method='ObserveChannels',
@@ -295,21 +291,21 @@ def test(q, bus, mc):
path=empathy.object_path,
interface=cs.APPROVER, method='AddDispatchOperation',
predicate=(lambda e:
- e.args[1][cs.CDO + '.Channels'][0][0] ==
+ e.args[0][0][0] ==
media_chan.object_path),
handled=False),
EventPattern('dbus-method-call',
path=empathy.object_path,
interface=cs.APPROVER, method='AddDispatchOperation',
predicate=(lambda e:
- e.args[1][cs.CDO + '.Channels'][0][0] ==
+ e.args[0][0][0] ==
text_chan.object_path),
handled=False),
EventPattern('dbus-method-call',
path=kopete.object_path,
interface=cs.APPROVER, method='AddDispatchOperation',
predicate=(lambda e:
- e.args[1][cs.CDO + '.Channels'][0][0] ==
+ e.args[0][0][0] ==
text_chan.object_path),
handled=False),
EventPattern('dbus-method-call',
@@ -322,18 +318,12 @@ def test(q, bus, mc):
method='Close',
path=ext_chan.object_path,
handled=True),
+ # we can't distinguish between the two NewDispatchOperation signals
+ # since we no longer see the Channels property (it's mutable)
EventPattern('dbus-signal',
path=cs.CD_PATH,
interface=cs.CD_IFACE_OP_LIST,
- signal='NewDispatchOperation',
- predicate=(lambda e: e.args[1][cs.CDO + '.Channels'][0][0] ==
- media_chan.object_path)),
- EventPattern('dbus-signal',
- path=cs.CD_PATH,
- interface=cs.CD_IFACE_OP_LIST,
- signal='NewDispatchOperation',
- predicate=(lambda e: e.args[1][cs.CDO + '.Channels'][0][0] ==
- text_chan.object_path)),
+ signal='NewDispatchOperation'),
)
q.dbus_return(e_observe_media.message, signature='')
diff --git a/test/twisted/dispatcher/lose-text.py b/test/twisted/dispatcher/lose-text.py
index d9bc1b3..642465f 100644
--- a/test/twisted/dispatcher/lose-text.py
+++ b/test/twisted/dispatcher/lose-text.py
@@ -107,9 +107,6 @@ def test(q, bus, mc):
assert handlers == [cs.tp_name_prefix + '.Client.Empathy',
cs.tp_name_prefix + '.Client.Kopete'], handlers
- assert cdo_properties[cs.CDO + '.Channels'] == [(chan.object_path,
- channel_properties)]
-
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)]
@@ -155,8 +152,9 @@ def test(q, bus, mc):
handled=False),
)
- assert e.args == [cdo_path, cdo_properties]
- assert k.args == [cdo_path, cdo_properties]
+ assert e.args == [[(chan.object_path, channel_properties)],
+ cdo_path, cdo_properties]
+ assert k.args == e.args
q.dbus_return(e.message, signature='')
diff --git a/xml/Client_Approver.xml b/xml/Client_Approver.xml
index 8725c81..5c8cb21 100644
--- a/xml/Client_Approver.xml
+++ b/xml/Client_Approver.xml
@@ -27,14 +27,23 @@
<tp:requires interface="org.freedesktop.Telepathy.Client.DRAFT"/>
<tp:docstring xmlns="http://www.w3.org/1999/xhtml">
- <p>Approvers notify the user that new channels have been created,
- and allow the user to accept or reject those channels.</p>
-
- <p>They can also select which channel handler will be used for the
+ <p>Approvers are clients that notify the user that new channels have
+ been created by a contact, and allow the user to accept or reject
+ those channels. The new channels are represented by a <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy">ChannelDispatcher.DRAFT</tp:dbus-ref>
+ object, which is passed to the
+ <tp:member-ref>AddDispatchOperation</tp:member-ref> method.</p>
+
+ <tp:rationale>
+ <p>For instance, Empathy's tray icon, or the answer/reject window
+ seen when a Maemo device receives a VoIP call, should be
+ Approvers.</p>
+ </tp:rationale>
+
+ <p>Approvers can also select which channel handler will be used for the
channel, for instance by offering the user a list of possible
- handlers rather than just an accept/reject choice.</p>
-
- <p>However, the Channel Dispatcher must be able to prioritize
+ handlers rather than just an accept/reject choice.
+ However, the Channel Dispatcher must be able to prioritize
possible handlers on its own using some reasonable heuristic,
probably based on user configuration.</p>
@@ -49,12 +58,22 @@
window and highlights contacts there, and one that is part
of a full-screen media player.</p>
- <p>Any approver can approve the handling of a channel with a
- particular channel handler. Approvers can also request that the
- channel is rejected. The first approver to reply gets its decision
- acted on; any other approvers that reply at the same time will
- get a D-Bus error, indicating that the channel has already been
- dealt with.</p>
+ <p>Any approver can approve the handling of a channel dispatch operation
+ with a particular channel handler by calling the <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">HandleWith</tp:dbus-ref>
+ method. Approvers can also attempt to <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Claim</tp:dbus-ref>
+ channels; if this succeeds, the approver may handle the channels
+ itself (if it is also a Handler), or close the channels in order to
+ reject them.</p>
+
+ <p>At the D-Bus level, there is no "reject" operation: approvers wishing
+ to reject channels SHOULD call the Claim method, then (if it succeeds)
+ close the channels in any way they see fit.</p>
+
+ <p>The first approver to reply gets its decision acted on; any other
+ approvers that reply at approximately the same time will get a D-Bus
+ error, indicating that the channel has already been dealt with.</p>
<p>Approvers should usually prompt the user and ask for
confirmation, rather than dispatching the channel to a handler
@@ -76,7 +95,10 @@
<p>This property works in exactly the same way as the
<tp:dbus-ref namespace="org.freedesktop.Telepathy">Client.Observer.DRAFT.ObserverChannelFilter</tp:dbus-ref>
- property. In the .client file, it is represented in the
+ property. In particular, it cannot change while the approver process
+ continues to own the corresponding Client bus name.</p>
+
+ <p>In the .client file, it is represented in the
same way as ObserverChannelFilter, but the group has the same
name as this interface and the keys start with
ApproverChannelFilter instead of ObserverChannelFilter.</p>
@@ -92,18 +114,51 @@
operations already exist.</p>
<p>The channel dispatcher SHOULD call this method on all approvers
- at the same time. If no approvers return from this method
+ at the same time. If an approver returns an error from this method,
+ the approver is assumed to be faulty.</p>
+
+ <p>If no approvers return from this method
successfully (including situations where there are no matching
approvers at all), the channel dispatcher SHOULD consider this
to be an error, and recover by dispatching the channel to the
most preferred handler.</p>
<tp:rationale>
- Processes that aren't approvers shouldn't be making connections
- anyway - there should always be at least one approver running.
+ Processes that aren't approvers (or don't at least ensure that there
+ is some approver) probably shouldn't be making connections
+ anyway, so there should always be at least one approver running.
</tp:rationale>
</tp:docstring>
+ <arg name="Channels" direction="in"
+ type="a(oa{sv})" tp:type="Channel_Details[]">
+ <tp:docstring xmlns="http://www.w3.org/1999/xhtml">
+ <p>The initial value of the <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.Channels</tp:dbus-ref>
+ property, containing the <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy">Channel</tp:dbus-ref>s
+ to be dispatched and their properties.</p>
+
+ <tp:rationale>
+ <p>This can't be signalled to the approver through the Properties
+ parameter of this method, because Channels is not an immutable
+ property.</p>
+ </tp:rationale>
+
+ <p>The actual channels to be dispatched may reduce as channels are
+ closed: this is signalled by <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.ChannelLost</tp:dbus-ref>.
+ </p>
+
+ <p>Approvers SHOULD connect to ChannelLost and <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy">ChannelDispatchOperation.DRAFT.Finished</tp:dbus-ref>.
+ (if desired) before returning from AddDispatchOperation, since
+ those signals are guaranteed not to be emitted until after all
+ AddDispatchOperation calls have returned (with success or failure)
+ or timed out.</p>
+ </tp:docstring>
+ </arg>
+
<arg name="DispatchOperation" type="o" direction="in">
<tp:docstring xmlns="http://www.w3.org/1999/xhtml">
<p>The
@@ -115,19 +170,16 @@
<arg name="Properties" type="a{sv}"
tp:type="Qualified_Property_Value_Map" direction="in">
<tp:docstring>
- Properties of the channel dispatch operation. This MUST NOT include
- properties that could change, SHOULD include as many properties as
- possible given that constraint, and MUST include at least the
- <tp:dbus-ref
- namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Account</tp:dbus-ref>,
- <tp:dbus-ref
- namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Connection</tp:dbus-ref>,
- <tp:dbus-ref
- namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Channels</tp:dbus-ref>
- and
- <tp:dbus-ref
- namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">PossibleHandlers</tp:dbus-ref>
- properties.
+ <p>Properties of the channel dispatch operation. This MUST NOT
+ include properties that could change, SHOULD include as many
+ properties as possible given that constraint, and MUST include at
+ least the <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Account</tp:dbus-ref>,
+ <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">Connection</tp:dbus-ref>
+ and <tp:dbus-ref
+ namespace="org.freedesktop.Telepathy.ChannelDispatchOperation.DRAFT">PossibleHandlers</tp:dbus-ref>
+ properties.</p>
</tp:docstring>
</arg>
</method>
--
1.5.6.5
More information about the telepathy-commits
mailing list