[Telepathy-commits] [telepathy-gabble/master] Muc: emit failed delivery reports rather than SendError

Will Thompson will.thompson at collabora.co.uk
Tue Feb 3 06:34:49 PST 2009


---
 src/muc-channel.c               |   76 +++++++++++++++------
 src/muc-channel.h               |    3 +-
 src/muc-factory.c               |   33 ++++-----
 tests/twisted/Makefile.am       |    1 +
 tests/twisted/muc/send-error.py |  138 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 210 insertions(+), 41 deletions(-)
 create mode 100644 tests/twisted/muc/send-error.py

diff --git a/src/muc-channel.c b/src/muc-channel.c
index 196176d..a076a97 100644
--- a/src/muc-channel.c
+++ b/src/muc-channel.c
@@ -2219,64 +2219,88 @@ _gabble_muc_channel_handle_subject (GabbleMucChannel *chan,
 void
 _gabble_muc_channel_receive (GabbleMucChannel *chan,
                              TpChannelTextMessageType msg_type,
-                             TpHandleType handle_type,
+                             TpHandleType sender_handle_type,
                              TpHandle sender,
                              time_t timestamp,
                              const gchar *text,
-                             LmMessage *msg)
+                             LmMessage *msg,
+                             TpChannelTextSendError send_error,
+                             TpDeliveryStatus error_status)
 {
   GabbleMucChannelPrivate *priv;
   TpBaseConnection *base_conn;
   TpMessage *message;
-  gboolean is_echo = FALSE;
+  TpHandle muc_self_handle;
+  gboolean is_echo;
+  gboolean is_error;
 
   g_assert (GABBLE_IS_MUC_CHANNEL (chan));
 
   priv = GABBLE_MUC_CHANNEL_GET_PRIVATE (chan);
   base_conn = (TpBaseConnection *) priv->conn;
+  muc_self_handle = chan->group.self_handle;
+
+  /* Is this an error report? */
+  is_error = (send_error != GABBLE_TEXT_CHANNEL_SEND_NO_ERROR);
 
-  if (handle_type == TP_HANDLE_TYPE_ROOM)
+  if (is_error && sender == muc_self_handle)
     {
-      NODE_DEBUG (msg->node, "ignoring message from channel");
+      /* So this is a <message from="ourself" type="error">.  I can only think
+       * that this would happen if we send an error stanza and the MUC reflects
+       * it back at us, so let's just ignore it.
+       */
+      NODE_DEBUG (msg->node, "ignoring error stanza from ourself");
 
       return;
     }
 
-  /* If we sent the message and it's not delayed, this is an echo from the MUC;
-   * we'll emit a delivery report.
-   * For messages from other contacts, or our own delayed messages, we'll emit
-   * a received message.
+  /* Is this an echo from the MUC of a message we just sent? */
+  is_echo = ((sender == muc_self_handle) && (timestamp == 0));
+
+  /* Having excluded the "error from ourself" case, is_error and is_echo are
+   * mutually exclusive.
    */
-  is_echo = ((sender == chan->group.self_handle) && (timestamp == 0));
 
   message = tp_message_new (base_conn, 2, 2);
 
-  /* Header */
+  /* Header common to normal message and delivery-echo */
   if (msg_type != TP_CHANNEL_TEXT_MESSAGE_TYPE_NORMAL)
     tp_message_set_uint32 (message, 0, "message-type", msg_type);
 
   if (timestamp != 0)
-    {
-      tp_message_set_boolean (message, 0, "scrollback", TRUE);
-      tp_message_set_uint64 (message, 0, "message-sent", timestamp);
-    }
-
-  tp_message_set_uint64 (message, 0, "message-received", time (NULL));
-  tp_message_set_handle (message, 0, "message-sender", TP_HANDLE_TYPE_CONTACT,
-      sender);
+    tp_message_set_uint64 (message, 0, "message-sent", timestamp);
 
   /* Body */
   tp_message_set_string (message, 1, "content-type", "text/plain");
   tp_message_set_string (message, 1, "content", text);
 
-  if (is_echo)
+  if (is_error || is_echo)
     {
+      /* Error reports and echos of our own messages are represented as
+       * delivery reports.
+       */
+
       TpMessage *delivery_report = tp_message_new (base_conn, 1, 1);
+      TpDeliveryStatus status =
+          is_error ? error_status : TP_DELIVERY_STATUS_DELIVERED;
 
       tp_message_set_uint32 (delivery_report, 0, "message-type",
           TP_CHANNEL_TEXT_MESSAGE_TYPE_DELIVERY_REPORT);
-      tp_message_set_uint32 (delivery_report, 0, "delivery-status",
-          TP_DELIVERY_STATUS_DELIVERED);
+      tp_message_set_uint32 (delivery_report, 0, "delivery-status", status);
+
+      if (is_error)
+        tp_message_set_uint32 (delivery_report, 0, "delivery-error",
+            send_error);
+
+      /* We do not set a message-sender on the report: the intended recipient
+       * of the original message was the MUC, so the spec we should omit it.
+       *
+       * The sender of the echo, however, is ourself.  (Unless we get errors
+       * for messages that we didn't send, which would be odd.)
+       */
+      tp_message_set_handle (message, 0, "message-sender",
+          TP_HANDLE_TYPE_CONTACT, muc_self_handle);
+
       tp_message_take_message (delivery_report, 0, "delivery-echo",
           message);
 
@@ -2284,6 +2308,14 @@ _gabble_muc_channel_receive (GabbleMucChannel *chan,
     }
   else
     {
+      /* Messages from the MUC itself should have no sender. */
+      if (sender_handle_type == TP_HANDLE_TYPE_CONTACT)
+        tp_message_set_handle (message, 0, "message-sender",
+            TP_HANDLE_TYPE_CONTACT, sender);
+
+      if (timestamp != 0)
+        tp_message_set_boolean (message, 0, "scrollback", TRUE);
+
       tp_message_mixin_take_received (G_OBJECT (chan), message);
     }
 }
diff --git a/src/muc-channel.h b/src/muc-channel.h
index c206cd5..ef4c2fd 100644
--- a/src/muc-channel.h
+++ b/src/muc-channel.h
@@ -93,7 +93,8 @@ void _gabble_muc_channel_handle_subject (GabbleMucChannel *chan,
     TpHandle sender, time_t timestamp, const gchar *subject, LmMessage *msg);
 void _gabble_muc_channel_receive (GabbleMucChannel *chan,
     TpChannelTextMessageType msg_type, TpHandleType handle_type,
-    TpHandle sender, time_t timestamp, const gchar *text, LmMessage *msg);
+    TpHandle sender, time_t timestamp, const gchar *text, LmMessage *msg,
+    TpChannelTextSendError send_error, TpDeliveryStatus delivery_status);
 
 void _gabble_muc_channel_state_receive (GabbleMucChannel *chan,
     guint state, guint from_handle);
diff --git a/src/muc-factory.c b/src/muc-factory.c
index 80997a9..540c127 100644
--- a/src/muc-factory.c
+++ b/src/muc-factory.c
@@ -731,6 +731,7 @@ muc_factory_message_cb (LmMessageHandler *handler,
   TpHandle room_handle, handle;
   GabbleMucChannel *chan;
   gint state;
+  gboolean is_error;
   TpChannelTextSendError send_error;
   TpDeliveryStatus delivery_status;
   gchar *room;
@@ -809,30 +810,26 @@ muc_factory_message_cb (LmMessageHandler *handler,
         }
     }
 
-  if (send_error != GABBLE_TEXT_CHANNEL_SEND_NO_ERROR)
-    {
-      /* FIXME: emit a delivery report */
-      tp_svc_channel_type_text_emit_send_error ((TpSvcChannelTypeText *) chan,
-          send_error, stamp, msgtype, body);
-      goto done;
-    }
-
-  if (state != -1 && handle_type == TP_HANDLE_TYPE_CONTACT)
-    _gabble_muc_channel_state_receive (chan, state, handle);
-
   if (body != NULL)
     _gabble_muc_channel_receive (chan, msgtype, handle_type, handle, stamp,
-        body, message);
+        body, message, send_error, delivery_status);
 
-  subj_node = lm_message_node_get_child (message->node, "subject");
-  if (subj_node != NULL)
+  is_error = (send_error != GABBLE_TEXT_CHANNEL_SEND_NO_ERROR);
+
+  if (!is_error)
     {
-      subject = lm_message_node_get_value (subj_node);
-      _gabble_muc_channel_handle_subject (chan, msgtype, handle_type, handle,
-          stamp, subject, message);
+      if (state != -1 && handle_type == TP_HANDLE_TYPE_CONTACT)
+        _gabble_muc_channel_state_receive (chan, state, handle);
+
+      subj_node = lm_message_node_get_child (message->node, "subject");
+      if (subj_node != NULL)
+        {
+          subject = lm_message_node_get_value (subj_node);
+          _gabble_muc_channel_handle_subject (chan, msgtype, handle_type, handle,
+              stamp, subject, message);
+        }
     }
 
-done:
   tp_handle_unref (handle_source, handle);
 
   return LM_HANDLER_RESULT_REMOVE_MESSAGE;
diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
index dcf24d3..60e5d93 100644
--- a/tests/twisted/Makefile.am
+++ b/tests/twisted/Makefile.am
@@ -1,5 +1,6 @@
 TWISTED_TESTS = \
 	muc/roomlist.py \
+	muc/send-error.py \
 	muc/test-ensure.py \
 	muc/test-muc-alias.py \
 	muc/test-muc-invitation.py \
diff --git a/tests/twisted/muc/send-error.py b/tests/twisted/muc/send-error.py
new file mode 100644
index 0000000..62d9c89
--- /dev/null
+++ b/tests/twisted/muc/send-error.py
@@ -0,0 +1,138 @@
+"""
+Test incoming error messages in MUC channels.
+"""
+
+import dbus
+
+from twisted.words.xish import domish
+
+from gabbletest import exec_test, make_muc_presence, request_muc_handle
+from servicetest import call_async, EventPattern
+import ns
+
+def test(q, bus, conn, stream):
+    conn.Connect()
+    q.expect('dbus-signal', signal='StatusChanged', args=[0, 1])
+
+    room_handle = request_muc_handle(q, conn, stream, 'chat at conf.localhost')
+    call_async(q, conn, 'RequestChannel',
+        'org.freedesktop.Telepathy.Channel.Type.Text', 2, room_handle, True)
+
+    gfc, _, _ = q.expect_many(
+        EventPattern('dbus-signal', signal='GroupFlagsChanged'),
+        EventPattern('dbus-signal', signal='MembersChanged',
+            args=[u'', [], [], [], [2], 0, 0]),
+        EventPattern('stream-presence', to='chat at conf.localhost/test'))
+    assert gfc.args[1] == 0
+
+    # Send presence for other member of room.
+    stream.send(make_muc_presence(
+        'owner', 'moderator', 'chat at conf.localhost', 'bob'))
+
+    # Send presence for own membership of room.
+    stream.send(make_muc_presence(
+        'none', 'participant', 'chat at conf.localhost', 'test'))
+
+    event = q.expect('dbus-signal', signal='MembersChanged',
+        args=[u'', [2, 3], [], [], [], 0, 0])
+    assert conn.InspectHandles(1, [2]) == ['chat at conf.localhost/test']
+    assert conn.InspectHandles(1, [3]) == ['chat at conf.localhost/bob']
+
+    event = q.expect('dbus-return', method='RequestChannel')
+    text_chan = bus.get_object(conn.bus_name, event.value[0])
+
+
+    # Suppose we don't have permission to speak in this MUC.  Send a message to
+    # the channel, and have the MUC reject it as unauthorized.
+    content = u"hi r ther ne warez n this chanel?"
+    greeting = [
+        dbus.Dictionary({ }, signature='sv'),
+        { 'content-type': 'text/plain',
+          'content': content,
+        }
+    ]
+
+    dbus.Interface(text_chan,
+        u'org.freedesktop.Telepathy.Channel.Interface.Messages.DRAFT'
+        ).SendMessage(greeting, dbus.UInt32(0))
+
+    stream_message, _, _ = q.expect_many(
+        EventPattern('stream-message'),
+        EventPattern('dbus-signal', signal='Sent'),
+        EventPattern('dbus-signal', signal='MessageSent'),
+        )
+
+    # computer says no
+    elem = stream_message.stanza
+    elem['from'] = 'chat at conf.localhost'
+    elem['to'] = 'chat at conf.localhost/test'
+    elem['type'] = 'error'
+    error = elem.addElement('error')
+    error['code'] = '401'
+    error['type'] = 'auth'
+    error.addElement((ns.STANZA, 'not-authorized'))
+
+    stream.send(elem)
+
+    # check that we got a failed delivery report and a SendError
+    send_error, received, message_received = q.expect_many(
+        EventPattern('dbus-signal', signal='SendError'),
+        EventPattern('dbus-signal', signal='Received'),
+        EventPattern('dbus-signal', signal='MessageReceived'),
+        )
+
+    PERMISSION_DENIED = 3
+
+    err, timestamp, type, text = send_error.args
+    assert err == PERMISSION_DENIED, send_error.args
+    # there's no way to tell when the original message was sent from the error stanza
+    assert timestamp == 0, send_error.args
+    # Gabble can't determine the type of the original message; see muc/test-muc.py
+    # assert type == 0, send_error.args
+    assert text == content, send_error.args
+
+    # The Text.Received signal should be a "you're not tall enough" stub
+    id, timestamp, sender, type, flags, text = received.args
+    assert sender == 0, old_received.args
+    assert type == 4, old_received.args # Message_Type_Delivery_Report
+    assert flags == 2, old_received.args # Non_Text_Content
+    assert text == '', old_received.args
+
+    # Check that the Messages.MessageReceived signal was a failed delivery report
+    assert len(message_received.args) == 1, message_received.args
+    parts = message_received.args[0]
+    # The delivery report should just be a header, no body.
+    assert len(parts) == 1, parts
+    part = parts[0]
+    # The intended recipient was the MUC, so there's no contact handle
+    # suitable for being 'message-sender'.
+    assert 'message-sender' not in part or part['message-sender'] == 0, part
+    assert part['message-type'] == 4, part # Message_Type_Delivery_Report
+    assert part['delivery-status'] == 3, part # Delivery_Status_Permanently_Failed
+    assert part['delivery-error'] == PERMISSION_DENIED, part
+    # Gabble doesn't issue tokens for messages you send, so no token should be
+    # in the report
+    assert 'delivery-token' not in part, part
+
+    # Check that the included echo is from us, and matches all the keys in the
+    # message we sent.
+    assert 'delivery-echo' in part, part
+    echo = part['delivery-echo']
+    assert len(echo) == len(greeting), (echo, greeting)
+    # Earlier in this test we checked that handle 2 is us.
+    assert echo[0]['message-sender'] == 2, echo[0]
+    for i in range(0, len(echo)):
+        for key in greeting[i]:
+            assert key in echo[i], (i, key, echo)
+            assert echo[i][key] == greeting[i][key], (i, key, echo, greeting)
+
+
+    conn.Disconnect()
+
+    q.expect('dbus-signal', signal='StatusChanged', args=[2, 1])
+
+    return True
+
+if __name__ == '__main__':
+    exec_test(test)
+
-- 
1.5.6.5




More information about the telepathy-commits mailing list