[Telepathy-commits] [telepathy-gabble/master] Implement a different and saner way to have SI fallback

Marco Barisione marco at barisione.org
Tue Jan 6 08:41:33 PST 2009


---
 src/bytestream-factory.c                |  226 +++++++++++++++++------------
 src/bytestream-factory.h                |   10 ++
 src/bytestream-multiple.c               |  241 ++++++++++++++++---------------
 src/bytestream-multiple.h               |    2 +-
 src/bytestream-socks5.c                 |    1 +
 src/namespaces.h                        |    1 +
 tests/twisted/tubes/test-si-fallback.py |   84 +++++++-----
 7 files changed, 321 insertions(+), 244 deletions(-)

diff --git a/src/bytestream-factory.c b/src/bytestream-factory.c
index 65aa1fc..5034c52 100644
--- a/src/bytestream-factory.c
+++ b/src/bytestream-factory.c
@@ -382,11 +382,10 @@ streaminit_parse_request (LmMessage *message,
                           const gchar **stream_init_id,
                           const gchar **mime_type,
                           GSList **stream_methods,
-                          gboolean *use_fallback)
+                          gboolean *multiple)
 {
   LmMessageNode *iq = message->node;
-  LmMessageNode *feature, *x, *field, *stream_method;
-  const gchar *list_type;
+  LmMessageNode *feature, *x, *field, *stream_method, *si_multiple;
 
   *stream_init_id = lm_message_node_get_attribute (iq, "id");
 
@@ -440,16 +439,8 @@ streaminit_parse_request (LmMessage *message,
         /* some future field, ignore it */
         continue;
 
-      list_type = lm_message_node_get_attribute (field, "type");
-      if (!tp_strdiff (list_type, "list-single"))
-        {
-          *use_fallback = FALSE;
-        }
-      else if (!tp_strdiff (list_type, "list-multiple"))
-        {
-          *use_fallback = TRUE;
-        }
-      else
+      if (tp_strdiff (lm_message_node_get_attribute (field, "type"),
+            "list-single"))
         {
           NODE_DEBUG (message->node, "SI request's stream-method field was "
               "not of type list-single");
@@ -491,6 +482,13 @@ streaminit_parse_request (LmMessage *message,
       return FALSE;
     }
 
+  si_multiple = lm_message_node_get_child_with_namespace (si, "si-multiple",
+      NS_SI_MULTIPLE);
+  if (si_multiple == NULL)
+    *multiple = FALSE;
+  else
+    *multiple = TRUE;
+
   return TRUE;
 }
 
@@ -525,7 +523,7 @@ gabble_bytestream_factory_make_stream_init_iq (const gchar *full_jid,
             '@', "type", "form",
             '(', "field", "",
               '@', "var", "stream-method",
-              '@', "type", "list-multiple",
+              '@', "type", "list-single",
               '(', "option", "",
                 '(', "value", NS_BYTESTREAMS,
                 ')',
@@ -535,6 +533,9 @@ gabble_bytestream_factory_make_stream_init_iq (const gchar *full_jid,
             ')',
           ')',
         ')',
+        '(', "si-multiple", "",
+          '@', "xmlns", NS_SI_MULTIPLE,
+        ')',
       ')', NULL);
 }
 
@@ -564,8 +565,7 @@ bytestream_factory_iq_si_cb (LmMessageHandler *handler,
   GSList *l;
   const gchar *profile, *from, *stream_id, *stream_init_id, *mime_type;
   GSList *stream_methods = NULL;
-  gboolean use_fallback;
-  GabbleBytestreamMultiple *multiple;
+  gboolean multiple;
   gchar *peer_resource = NULL;
 
   if (lm_message_get_sub_type (msg) != LM_MESSAGE_SUB_TYPE_SET)
@@ -579,7 +579,7 @@ bytestream_factory_iq_si_cb (LmMessageHandler *handler,
    * it or send an error reply */
 
   if (!streaminit_parse_request (msg, si, &profile, &from, &stream_id,
-        &stream_init_id, &mime_type, &stream_methods, &use_fallback))
+        &stream_init_id, &mime_type, &stream_methods, &multiple))
     {
       _gabble_connection_send_iq_error (priv->conn, msg,
           XMPP_ERROR_BAD_REQUEST, "failed to parse SI request");
@@ -603,46 +603,31 @@ bytestream_factory_iq_si_cb (LmMessageHandler *handler,
       gabble_decode_jid (from, NULL, NULL, &peer_resource);
     }
 
-  if (use_fallback)
-    multiple = gabble_bytestream_factory_create_multiple (self, peer_handle,
+  if (multiple)
+    bytestream = (GabbleBytestreamIface *) gabble_bytestream_factory_create_multiple (self, peer_handle,
         stream_id, stream_init_id, peer_resource,
         GABBLE_BYTESTREAM_STATE_LOCAL_PENDING);
-  else
-    multiple = NULL;
 
   /* check stream method */
   for (l = stream_methods; l != NULL; l = l->next)
     {
-      /* We create the stream according the stream method chosen.
-       * User has to accept it */
-      if (!tp_strdiff (l->data, NS_IBB))
+      if (multiple)
         {
-          bytestream = (GabbleBytestreamIface *)
-            gabble_bytestream_factory_create_ibb (self, peer_handle,
-              stream_id, stream_init_id, peer_resource,
-              GABBLE_BYTESTREAM_STATE_LOCAL_PENDING);
-        }
-      else if (!tp_strdiff (l->data, NS_BYTESTREAMS))
-        {
-          bytestream = (GabbleBytestreamIface *)
-            gabble_bytestream_factory_create_socks5 (self, peer_handle,
-              stream_id, stream_init_id, peer_resource,
-              GABBLE_BYTESTREAM_STATE_LOCAL_PENDING);
+          gabble_bytestream_multiple_add_bytestream (GABBLE_BYTESTREAM_MULTIPLE (bytestream), l->data);
         }
       else
         {
-          continue;
+          /* We create the stream according the stream method chosen.
+           * User has to accept it */
+          bytestream = gabble_bytestream_factory_create_from_method (self,
+              l->data, peer_handle, stream_id, stream_init_id, peer_resource,
+              GABBLE_BYTESTREAM_STATE_LOCAL_PENDING);
+          if (bytestream)
+            break;
         }
-
-      if (multiple != NULL)
-        gabble_bytestream_multiple_add_bytestream (multiple, bytestream);
-      else
-        break;
     }
 
   /* FIXME check if there are bytestream methods in the multiple one */
-  if (multiple != NULL)
-    bytestream = GABBLE_BYTESTREAM_IFACE (multiple);
 
   if (bytestream == NULL)
     {
@@ -1196,6 +1181,33 @@ gabble_bytestream_factory_generate_stream_id (void)
   return stream_id;
 }
 
+GabbleBytestreamIface *
+gabble_bytestream_factory_create_from_method (GabbleBytestreamFactory *self,
+                                              const gchar *stream_method,
+                                              TpHandle peer_handle,
+                                              const gchar *stream_id,
+                                              const gchar *stream_init_id,
+                                              const gchar *peer_resource,
+                                              GabbleBytestreamState state)
+{
+  GabbleBytestreamIface *bytestream = NULL;
+
+  if (!tp_strdiff (stream_method, NS_IBB))
+    {
+      bytestream = GABBLE_BYTESTREAM_IFACE (
+          gabble_bytestream_factory_create_ibb (self, peer_handle,
+            stream_id, stream_init_id, peer_resource, state));
+    }
+  else if (!tp_strdiff (stream_method, NS_BYTESTREAMS))
+    {
+      bytestream = GABBLE_BYTESTREAM_IFACE (
+          gabble_bytestream_factory_create_socks5 (self, peer_handle,
+            stream_id, stream_init_id, peer_resource, state));
+    }
+
+  return bytestream;
+}
+
 GabbleBytestreamIBB *
 gabble_bytestream_factory_create_ibb (GabbleBytestreamFactory *self,
                                       TpHandle peer_handle,
@@ -1315,6 +1327,7 @@ gabble_bytestream_factory_create_multiple (GabbleBytestreamFactory *self,
       "state", state,
       "stream-init-id", stream_init_id,
       "peer-resource", peer_resource,
+      "factory", self,
       NULL);
 
   g_signal_connect (multiple, "state-changed",
@@ -1342,7 +1355,6 @@ streaminit_reply_cb (GabbleConnection *conn,
   struct _streaminit_reply_cb_data *data =
     (struct _streaminit_reply_cb_data*) user_data;
   GabbleBytestreamIface *bytestream = NULL;
-  GSList *bytestream_list = NULL;
   gchar *peer_resource = NULL;
   LmMessageNode *si, *feature, *x, *field, *value;
   const gchar *from, *stream_method;
@@ -1384,6 +1396,31 @@ streaminit_reply_cb (GabbleConnection *conn,
       goto END;
     }
 
+  /* If the other client supports si-multiple we have directly a list of
+   * supported methods inside <value/> tags.
+   * If there is at least one <value/> we create a multiple bytestream and
+   * add the supported methods to it */
+  for (value = si->children; value; value = value->next)
+    {
+      if (tp_strdiff (value->name, "value"))
+        continue;
+
+      if (!bytestream)
+        bytestream = GABBLE_BYTESTREAM_IFACE (
+            gabble_bytestream_factory_create_multiple (self, peer_handle,
+               data->stream_id, NULL, peer_resource,
+               GABBLE_BYTESTREAM_STATE_INITIATING));
+
+      gabble_bytestream_multiple_add_bytestream (
+          GABBLE_BYTESTREAM_MULTIPLE (bytestream),
+          lm_message_node_get_value (value));
+    }
+
+  if (bytestream)
+    // XXX remove the goto when we see that this works
+    goto DONE;
+
+  /* The other client doesn't suppport si-multiple, use the normal method */
   feature = lm_message_node_get_child_with_namespace (si, "feature",
       NS_FEATURENEG);
   if (feature == NULL)
@@ -1407,65 +1444,28 @@ streaminit_reply_cb (GabbleConnection *conn,
         /* some future field, ignore it */
         continue;
 
-      for (value = field->children; value; value = value->next)
+      value = lm_message_node_get_child (field, "value");
+      if (value == NULL)
         {
-          stream_method = lm_message_node_get_value (value);
-
-          if (!tp_strdiff (stream_method, NS_IBB))
-            {
-              /* Remote user has accepted the stream */
-              bytestream = GABBLE_BYTESTREAM_IFACE (
-                  gabble_bytestream_factory_create_ibb (self, peer_handle,
-                  data->stream_id, NULL, peer_resource,
-                  GABBLE_BYTESTREAM_STATE_INITIATING));
-            }
-          else if (!tp_strdiff (stream_method, NS_BYTESTREAMS))
-            {
-              /* Remote user has accepted the stream */
-              bytestream = GABBLE_BYTESTREAM_IFACE (
-                  gabble_bytestream_factory_create_socks5 (self, peer_handle,
-                  data->stream_id, NULL, peer_resource,
-                  GABBLE_BYTESTREAM_STATE_INITIATING));
-            }
-          else
-            {
-              DEBUG ("Remote user chose an unsupported stream method");
-              goto END;
-            }
-
-          bytestream_list = g_slist_prepend (bytestream_list, bytestream);
+          NODE_DEBUG (reply_msg->node, "SI reply's stream-method field "
+              "doesn't contain stream-method value");
+          goto END;
         }
 
+      stream_method = lm_message_node_get_value (value);
+      bytestream = gabble_bytestream_factory_create_from_method (self,
+          stream_method, peer_handle, data->stream_id, NULL,
+          peer_resource, GABBLE_BYTESTREAM_STATE_INITIATING);
+
+      /* no need to parse the rest of the fields, we've found the one we
+       * wanted */
       break;
     }
 
-  if (bytestream_list == NULL)
+  if (bytestream == NULL)
     goto END;
 
-  /* Create a multiple bytestream if needed */
-  if (g_slist_length (bytestream_list) > 1)
-    {
-      GSList *l;
-
-      bytestream = GABBLE_BYTESTREAM_IFACE (
-          gabble_bytestream_factory_create_multiple (self, peer_handle,
-          data->stream_id, NULL, peer_resource,
-          GABBLE_BYTESTREAM_STATE_INITIATING));
-
-      l = bytestream_list = g_slist_reverse (bytestream_list);
-
-      while (l != NULL)
-        {
-          gabble_bytestream_multiple_add_bytestream (
-              GABBLE_BYTESTREAM_MULTIPLE (bytestream), l->data);
-
-          l = g_slist_next (l);
-        }
-    }
-
-  /* If there is only one bytestream than we already have
-     bytestream == bytestream_list->data */
-
+DONE:
   DEBUG ("stream %s accepted", data->stream_id);
 
   /* Let's start the initiation of the stream */
@@ -1496,7 +1496,6 @@ END:
 
   g_free (data->stream_id);
   g_slice_free (struct _streaminit_reply_cb_data, data);
-  g_slist_free (bytestream_list);
 
   return LM_HANDLER_RESULT_REMOVE_MESSAGE;
 }
@@ -1583,3 +1582,38 @@ gabble_bytestream_factory_make_accept_iq (const gchar *full_jid,
         ')',
       ')', NULL);
 }
+
+/*
+ * gabble_bytestream_factory_make_multi_accept_iq
+ *
+ * @full_jid: the full jid of the stream initiator
+ * @stream_init_id: the id of the SI request
+ * @stream_methods: a list of the accepted string methods
+ *
+ * Create an IQ stanza accepting a stream in response to
+ * a si-multiple SI request.
+ *
+ */
+LmMessage *
+gabble_bytestream_factory_make_multi_accept_iq (const gchar *full_jid,
+                                                const gchar *stream_init_id,
+                                                GList *stream_methods)
+{
+  LmMessage *msg;
+  GList *l;
+
+  msg = lm_message_build (full_jid, LM_MESSAGE_TYPE_IQ,
+      '@', "type", "result",
+      '@', "id", stream_init_id,
+      '(', "si", "",
+        '@', "xmlns", NS_SI,
+      ')', NULL);
+
+  for (l = stream_methods; l != NULL; l = l->next)
+    {
+      lm_message_node_add_child (msg->node->children,
+          "value", l->data);
+    }
+
+  return msg;
+}
diff --git a/src/bytestream-factory.h b/src/bytestream-factory.h
index 4523ef4..042b92a 100644
--- a/src/bytestream-factory.h
+++ b/src/bytestream-factory.h
@@ -73,6 +73,7 @@ typedef void (* GabbleBytestreamFactoryNegotiateReplyFunc) (
 GabbleBytestreamFactory *gabble_bytestream_factory_new (
     GabbleConnection *conn);
 
+/* FIXME: what's the point to have these public? */
 GabbleBytestreamIBB *gabble_bytestream_factory_create_ibb (
     GabbleBytestreamFactory *fac, TpHandle peer_handle, const gchar *stream_id,
     const gchar *stream_init_id, const gchar *peer_resource,
@@ -92,12 +93,21 @@ GabbleBytestreamMultiple *gabble_bytestream_factory_create_multiple (
     const gchar *stream_id, const gchar *stream_init_id,
     const gchar *peer_resource, GabbleBytestreamState state);
 
+GabbleBytestreamIface *gabble_bytestream_factory_create_from_method (
+    GabbleBytestreamFactory *self, const gchar *stream_method,
+    TpHandle peer_handle, const gchar *stream_id, const gchar *stream_init_id,
+    const gchar *peer_resource, GabbleBytestreamState state);
+
 LmMessage *gabble_bytestream_factory_make_stream_init_iq (
     const gchar *full_jid, const gchar *stream_id, const gchar *profile);
 
 LmMessage *gabble_bytestream_factory_make_accept_iq (const gchar *full_jid,
     const gchar *stream_init_id, const gchar *stream_method);
 
+LmMessage *gabble_bytestream_factory_make_multi_accept_iq (
+    const gchar *full_jid, const gchar *stream_init_id,
+    GList *stream_methods);
+
 gboolean gabble_bytestream_factory_negotiate_stream (
     GabbleBytestreamFactory *fac, LmMessage *msg, const gchar *stream_id,
     GabbleBytestreamFactoryNegotiateReplyFunc func,
diff --git a/src/bytestream-multiple.c b/src/bytestream-multiple.c
index 1767e56..f67bb07 100644
--- a/src/bytestream-multiple.c
+++ b/src/bytestream-multiple.c
@@ -69,6 +69,7 @@ enum
   PROP_STATE,
   PROP_PROTOCOL,
   PROP_CLOSE_ON_CONNECTION_ERROR,
+  PROP_FACTORY,
   LAST_PROPERTY
 };
 
@@ -82,17 +83,18 @@ struct _GabbleBytestreamMultiplePrivate
   GabbleBytestreamState state;
   gchar *peer_jid;
   gboolean close_on_connection_error;
+  GabbleBytestreamFactory *factory;
 
-  GList *bytestreams;
-  GabbleBytestreamIface *active;
+  /* List of gchar* */
+  GList *fallback_stream_methods;
+  GabbleBytestreamIface *active_bytestream;
 
   gboolean dispose_has_run;
 };
 
 #define GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE(obj) ((obj)->priv)
 
-static void gabble_bytestream_multiple_close (GabbleBytestreamIface *iface,
-    GError *error);
+static void bytestream_activate_next (GabbleBytestreamMultiple *self);
 
 static void
 gabble_bytestream_multiple_init (GabbleBytestreamMultiple *self)
@@ -125,6 +127,9 @@ gabble_bytestream_multiple_dispose (GObject *object)
       gabble_bytestream_iface_close (GABBLE_BYTESTREAM_IFACE (self), NULL);
     }
 
+  if (priv->factory)
+    g_object_unref (priv->factory);
+
   G_OBJECT_CLASS (gabble_bytestream_multiple_parent_class)->dispose (object);
 }
 
@@ -183,6 +188,9 @@ gabble_bytestream_multiple_get_property (GObject *object,
       case PROP_CLOSE_ON_CONNECTION_ERROR:
         g_value_set_boolean (value, priv->close_on_connection_error);
         break;
+      case PROP_FACTORY:
+        g_value_set_object (value, priv->factory);
+        break;
       default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
         break;
@@ -228,6 +236,9 @@ gabble_bytestream_multiple_set_property (GObject *object,
       case PROP_CLOSE_ON_CONNECTION_ERROR:
         priv->close_on_connection_error = g_value_get_boolean (value);
         break;
+      case PROP_FACTORY:
+        priv->factory = g_value_get_object (value);
+        break;
       default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
         break;
@@ -299,6 +310,7 @@ gabble_bytestream_multiple_class_init (
        "state");
    g_object_class_override_property (object_class, PROP_PROTOCOL,
        "protocol");
+   /* FIXME */
    g_object_class_override_property (object_class,
         PROP_CLOSE_ON_CONNECTION_ERROR, "close-on-connection-error");
 
@@ -320,6 +332,15 @@ gabble_bytestream_multiple_class_init (
   g_object_class_install_property (object_class, PROP_STREAM_INIT_ID,
       param_spec);
 
+  param_spec = g_param_spec_object (
+      "factory",
+      "Factory",
+      "The GabbleBytestreamFactory that created the stream",
+      GABBLE_TYPE_BYTESTREAM_FACTORY,
+      G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS);
+  g_object_class_install_property (object_class, PROP_FACTORY,
+      param_spec);
+
   signals[DATA_RECEIVED] =
     g_signal_new ("data-received",
                   G_OBJECT_CLASS_TYPE (gabble_bytestream_multiple_class),
@@ -368,9 +389,9 @@ gabble_bytestream_multiple_send (GabbleBytestreamIface *iface,
       return FALSE;
     }
 
-  g_assert(priv->active);
+  g_assert(priv->active_bytestream);
 
-  return gabble_bytestream_iface_send (priv->active, len, str);
+  return gabble_bytestream_iface_send (priv->active_bytestream, len, str);
 }
 
 /*
@@ -387,6 +408,11 @@ gabble_bytestream_multiple_accept (GabbleBytestreamIface *iface,
   GabbleBytestreamMultiplePrivate *priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
   LmMessage *msg;
   LmMessageNode *si;
+  GList *all_methods;
+  gchar *current_method;
+
+  /* We cannot just call the accept method of the active bytestream because
+   * the result stanza is different if we are using si-multiple */
 
   if (priv->state != GABBLE_BYTESTREAM_STATE_LOCAL_PENDING)
     {
@@ -394,8 +420,18 @@ gabble_bytestream_multiple_accept (GabbleBytestreamIface *iface,
       return;
     }
 
-  msg = gabble_bytestream_factory_make_accept_iq (priv->peer_jid,
-      priv->stream_init_id, NS_BYTESTREAMS);
+  g_return_if_fail (priv->active_bytestream);
+
+  all_methods = g_list_copy (priv->fallback_stream_methods);
+  g_object_get (priv->active_bytestream, "protocol", &current_method, NULL);
+  all_methods = g_list_prepend (all_methods, current_method);
+
+  msg = gabble_bytestream_factory_make_multi_accept_iq (priv->peer_jid,
+      priv->stream_init_id, all_methods);
+
+  g_free (current_method);
+  g_list_free (all_methods);
+
   si = lm_message_node_get_child_with_namespace (msg->node, "si", NS_SI);
   g_assert (si != NULL);
 
@@ -409,42 +445,13 @@ gabble_bytestream_multiple_accept (GabbleBytestreamIface *iface,
     {
       DEBUG ("stream %s with %s is now accepted", priv->stream_id,
           priv->peer_jid);
-      g_object_set (self, "state", GABBLE_BYTESTREAM_STATE_ACCEPTED, NULL);
+      g_object_set (priv->active_bytestream, "state",
+          GABBLE_BYTESTREAM_STATE_ACCEPTED, NULL);
     }
 
   lm_message_unref (msg);
 }
 
-static void
-gabble_bytestream_multiple_decline (GabbleBytestreamMultiple *self,
-                                    GError *error)
-{
-  GabbleBytestreamMultiplePrivate *priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
-  LmMessage *msg;
-
-  g_return_if_fail (priv->state == GABBLE_BYTESTREAM_STATE_LOCAL_PENDING);
-
-  msg = lm_message_build (priv->peer_jid, LM_MESSAGE_TYPE_IQ,
-      '@', "type", "error",
-      '@', "id", priv->stream_init_id,
-      NULL);
-
-  if (error != NULL && error->domain == GABBLE_XMPP_ERROR)
-    {
-      gabble_xmpp_error_to_node (error->code, msg->node, error->message);
-    }
-  else
-    {
-      gabble_xmpp_error_to_node (XMPP_ERROR_FORBIDDEN, msg->node,
-          "Offer Declined");
-    }
-
-  _gabble_connection_send (priv->conn, msg, NULL);
-
-  lm_message_unref (msg);
-
-  g_object_set (self, "state", GABBLE_BYTESTREAM_STATE_CLOSED, NULL);
-}
 
 /*
  * gabble_bytestream_multiple_close
@@ -453,7 +460,7 @@ gabble_bytestream_multiple_decline (GabbleBytestreamMultiple *self,
  */
 static void
 gabble_bytestream_multiple_close (GabbleBytestreamIface *iface,
-                                GError *error)
+                                  GError *error)
 {
   GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (iface);
   GabbleBytestreamMultiplePrivate *priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
@@ -462,18 +469,11 @@ gabble_bytestream_multiple_close (GabbleBytestreamIface *iface,
      /* bytestream already closed, do nothing */
      return;
 
-  if (priv->state == GABBLE_BYTESTREAM_STATE_LOCAL_PENDING)
-    {
-      /* Stream was created using SI so we decline the request */
-      gabble_bytestream_multiple_decline (self, error);
-    }
+  if (priv->active_bytestream)
+    gabble_bytestream_iface_close (priv->active_bytestream, error);
   else
-    {
-      /* FIXME: send a close stanza */
-      /* FIXME: close the sub-bytesreams */
-
-      g_object_set (self, "state", GABBLE_BYTESTREAM_STATE_CLOSED, NULL);
-    }
+    /* FIXME can it happen? maybe when there are no methods */
+    g_object_set (self, "state", GABBLE_BYTESTREAM_STATE_CLOSED, NULL);
 }
 
 /*
@@ -494,18 +494,41 @@ gabble_bytestream_multiple_initiate (GabbleBytestreamIface *iface)
       return FALSE;
     }
 
-  if (priv->bytestreams == NULL)
+  if (priv->active_bytestream == NULL)
     {
+      /* FIXME no stream methods? */
       DEBUG ("no bytestreams to initiate");
       return FALSE;
     }
 
-  g_assert (priv->active == NULL);
+  return gabble_bytestream_iface_initiate (priv->active_bytestream);
+}
+
+static void
+bytestream_data_received_cb (GabbleBytestreamIface *bytestream,
+                             TpHandle sender,
+                             GString *str,
+                             gpointer user_data)
+{
+  GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (user_data);
+
+  /* Just forward the data */
+  g_signal_emit (G_OBJECT (self), signals[DATA_RECEIVED], 0, sender, str);
+}
+
+static void
+bytestream_state_changed_cb (GabbleBytestreamIface *bytestream,
+                             GabbleBytestreamState state,
+                             gpointer user_data)
+{
+  GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (user_data);
 
-  /* Initiate the first available bytestream */
-  priv->active = priv->bytestreams->data;
+  /* When there is a connection error the state of the sub-bytestream becomes
+   * CLOSED. There is no risk to receive a notification for this kind of
+   * change because the signal handler is previously disconnected in
+   * bytestream_connection_error_cb */
 
-  return gabble_bytestream_iface_initiate (priv->active);
+  g_object_set (self, "state", state, NULL);
 }
 
 static void
@@ -515,68 +538,63 @@ bytestream_connection_error_cb (GabbleBytestreamIface *failed,
   GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (user_data);
   GabbleBytestreamMultiplePrivate *priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
 
-  g_assert (failed == priv->active);
+  g_assert (failed == priv->active_bytestream);
+ 
+  g_signal_handlers_disconnect_by_func (failed,
+      bytestream_connection_error_cb, self);
+  g_signal_handlers_disconnect_by_func (failed,
+      bytestream_data_received_cb, self);
+  g_signal_handlers_disconnect_by_func (failed,
+      bytestream_state_changed_cb, self);
 
-  priv->bytestreams = g_list_remove (priv->bytestreams, failed);
-  priv->active = NULL;
+  /* We don't have to unref it because the reference is kept by the
+   * factory */
+  priv->active_bytestream = NULL;
 
-  if (!priv->bytestreams)
+  if (priv->fallback_stream_methods == NULL)
     return;
 
-  /* If we have other methods to try, prevent the failed bytestrem to send a
+  /* If we have other methods to try, prevent the failed bytestream to send a
      close stanza */
-  g_object_set (failed, "close-on-connection-error", FALSE, NULL);
-
-  g_object_unref (failed);
+  // XXX
+  //g_object_set (failed, "close-on-connection-error", FALSE, NULL);
 
   DEBUG ("Trying alternative streaming method");
 
-  priv->active = priv->bytestreams->data;
-  g_object_set (priv->active, "state", GABBLE_BYTESTREAM_STATE_INITIATING, NULL);
-  gabble_bytestream_iface_initiate (priv->active);
+  bytestream_activate_next (self);
+
+  if (priv->state == GABBLE_BYTESTREAM_STATE_INITIATING)
+    /* The previous bytestream failed when initiating it, so now we have to
+     * initiate the new one */
+    gabble_bytestream_iface_initiate (priv->active_bytestream);
 }
 
 static void
-bytestream_data_received_cb (GabbleBytestreamIface *bytestream,
-                             TpHandle sender,
-                             GString *str,
-                             gpointer user_data)
+bytestream_activate_next (GabbleBytestreamMultiple *self)
 {
-  GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (user_data);
+  GabbleBytestreamMultiplePrivate *priv =
+      GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
+  const gchar *stream_method;
 
-  /* Just forward the data */
-  g_signal_emit (G_OBJECT (self), signals[DATA_RECEIVED], 0, sender, str);
-}
+  g_return_if_fail (priv->active_bytestream == NULL);
+  /* The caller has to be sure that there is a fallback method */
+  g_return_if_fail (priv->fallback_stream_methods != NULL);
 
-static void
-bytestream_state_changed_cb (GabbleBytestreamIface *bytestream,
-                             GabbleBytestreamState state,
-                             gpointer user_data)
-{
-  GabbleBytestreamMultiple *self = GABBLE_BYTESTREAM_MULTIPLE (user_data);
-  GabbleBytestreamMultiplePrivate *priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
+  /* Try the first stream method in the fallback list */
+  stream_method = priv->fallback_stream_methods->data;
+  priv->fallback_stream_methods = g_list_remove_link (
+      priv->fallback_stream_methods, priv->fallback_stream_methods);
 
-  if (state == GABBLE_BYTESTREAM_STATE_CLOSED &&
-      g_list_length (priv->bytestreams) <= 1)
-    {
-      return;
-    }
-  else if (state == GABBLE_BYTESTREAM_STATE_OPEN)
-    {
-      if (priv->active != NULL && priv->active != bytestream)
-        {
-          /* The old active bytestream is now useless as another one was
-           * open */
-          g_object_set (priv->active, "close-on-connection-error", FALSE, NULL);
-          gabble_bytestream_iface_close (priv->active, NULL);
-          priv->bytestreams = g_list_remove (priv->bytestreams, priv->active);
-          g_object_unref (priv->active);
-        }
-
-      priv->active = bytestream;
-    }
+  priv->active_bytestream = gabble_bytestream_factory_create_from_method (
+      priv->factory, stream_method, priv->peer_handle, priv->stream_id,
+      priv->stream_init_id, priv->peer_resource, priv->state);
 
-  g_object_set (self, "state", state, NULL);
+  g_signal_connect (priv->active_bytestream, "connection-error",
+      G_CALLBACK (bytestream_connection_error_cb), self);
+  g_signal_connect (priv->active_bytestream, "data-received",
+      G_CALLBACK (bytestream_data_received_cb), self);
+  g_signal_connect (priv->active_bytestream, "state-changed",
+      G_CALLBACK (bytestream_state_changed_cb), self);
 }
 
 /*
@@ -584,28 +602,25 @@ bytestream_state_changed_cb (GabbleBytestreamIface *bytestream,
  *
  * Add an alternative stream method.
  */
+/* FIXME -> add_stream_method */
 void
 gabble_bytestream_multiple_add_bytestream (GabbleBytestreamMultiple *self,
-                                           GabbleBytestreamIface *bytestream)
+                                           const gchar *method)
 {
   GabbleBytestreamMultiplePrivate *priv;
 
   g_return_if_fail (GABBLE_IS_BYTESTREAM_MULTIPLE (self));
-  g_return_if_fail (GABBLE_IS_BYTESTREAM_IFACE (bytestream));
+  g_return_if_fail (method != NULL);
 
   priv = GABBLE_BYTESTREAM_MULTIPLE_GET_PRIVATE (self);
 
-  DEBUG ("Add bytestream");
+  DEBUG ("Add bytestream method %s", method);
 
-  g_object_ref (bytestream);
-  priv->bytestreams = g_list_append (priv->bytestreams, bytestream);
+  priv->fallback_stream_methods = g_list_append (
+      priv->fallback_stream_methods, g_strdup (method));
 
-  g_signal_connect (bytestream, "connection-error",
-      G_CALLBACK (bytestream_connection_error_cb), self);
-  g_signal_connect (bytestream, "data-received",
-      G_CALLBACK (bytestream_data_received_cb), self);
-  g_signal_connect (bytestream, "state-changed",
-      G_CALLBACK (bytestream_state_changed_cb), self);
+  if (priv->active_bytestream == NULL)
+    bytestream_activate_next (self);
 }
 
 static void
diff --git a/src/bytestream-multiple.h b/src/bytestream-multiple.h
index 3334a19..19687e8 100644
--- a/src/bytestream-multiple.h
+++ b/src/bytestream-multiple.h
@@ -67,7 +67,7 @@ GType gabble_bytestream_multiple_get_type (void);
 
 void
 gabble_bytestream_multiple_add_bytestream (GabbleBytestreamMultiple *self,
-                                           GabbleBytestreamIface *bytestream);
+                                           const gchar *method);
 
 G_END_DECLS
 
diff --git a/src/bytestream-socks5.c b/src/bytestream-socks5.c
index 38fab10..29549c2 100644
--- a/src/bytestream-socks5.c
+++ b/src/bytestream-socks5.c
@@ -1180,6 +1180,7 @@ gabble_bytestream_socks5_decline (GabbleBytestreamSocks5 *self,
           "Offer Declined");
     }
 
+  /* FIXME: we should not send the SI error if we are falling back */
   _gabble_connection_send (priv->conn, msg, NULL);
 
   lm_message_unref (msg);
diff --git a/src/namespaces.h b/src/namespaces.h
index 96d300f..6dd2fbf 100644
--- a/src/namespaces.h
+++ b/src/namespaces.h
@@ -87,6 +87,7 @@
 #define NS_X_DELAY              "jabber:x:delay"
 #define NS_X_CONFERENCE         "jabber:x:conference"
 #define NS_XMPP_STANZAS         "urn:ietf:params:xml:ns:xmpp-stanzas"
+#define NS_SI_MULTIPLE          "http://telepathy.freedesktop.org/xmpp/si-multiple"
 
 
 #endif /* __GABBLE_NAMESPACES__H__ */
diff --git a/tests/twisted/tubes/test-si-fallback.py b/tests/twisted/tubes/test-si-fallback.py
index 984593b..4b76ba3 100644
--- a/tests/twisted/tubes/test-si-fallback.py
+++ b/tests/twisted/tubes/test-si-fallback.py
@@ -24,10 +24,11 @@ NS_FEATURE_NEG = 'http://jabber.org/protocol/feature-neg'
 NS_IBB = 'http://jabber.org/protocol/ibb'
 NS_X_DATA = 'jabber:x:data'
 NS_BYTESTREAMS = 'http://jabber.org/protocol/bytestreams'
+NS_SI_MULTIPLE = 'http://telepathy.freedesktop.org/xmpp/si-multiple'
 
 class Echo(Protocol):
     def dataReceived(self, data):
-        self.transport.write(data)
+        self.transport.write(data.upper())
 
 def set_up_echo():
     factory = Factory()
@@ -140,7 +141,7 @@ def test(q, bus, conn, stream):
     x['type'] = 'form'
     field = x.addElement((None, 'field'))
     field['var'] = 'stream-method'
-    field['type'] = 'list-multiple'
+    field['type'] = 'list-single'
     option = field.addElement((None, 'option'))
     value = option.addElement((None, 'value'))
     value.addContent(NS_BYTESTREAMS)
@@ -150,6 +151,9 @@ def test(q, bus, conn, stream):
 
     stream_node = si.addElement((NS_TUBES, 'stream'))
     stream_node['tube'] = str(stream_tube_id)
+
+    si_multiple = si.addElement((NS_SI_MULTIPLE, 'si-multiple'))
+
     stream.send(iq)
 
     si_reply_event, _ = q.expect_many(
@@ -157,12 +161,13 @@ def test(q, bus, conn, stream):
             EventPattern('dbus-signal', signal='TubeChannelStateChanged',
                 args=[2])) # 2 == OPEN
     iq = si_reply_event.stanza
-    si = xpath.queryForNodes('/iq/si[@xmlns="%s"]' % NS_SI,
-        iq)[0]
-    value = xpath.queryForNodes('/si/feature/x/field/value', si)
-    assert len(value) == 1
-    proto = value[0]
-    assert str(proto) == NS_BYTESTREAMS
+    si = xpath.queryForNodes('/iq/si[@xmlns="%s"]' % NS_SI, iq)[0]
+    methods = xpath.queryForNodes('/si/value', si)
+    assert len(methods) == 2
+    assert methods[0].name == 'value'
+    assert str(methods[0]) == NS_BYTESTREAMS
+    assert methods[1].name == 'value'
+    assert str(methods[1]) == NS_IBB
     tube = xpath.queryForNodes('/si/tube[@xmlns="%s"]' % NS_TUBES, si)
     assert len(tube) == 1
 
@@ -182,18 +187,19 @@ def test(q, bus, conn, stream):
     streamhost['port'] = '1234'
     stream.send(iq)
 
-    event = q.expect('stream-iq', iq_type='set', to='bob at localhost/Bob')
-    iq = event.stanza
-    open = xpath.queryForNodes('/iq/open', iq)[0]
-    assert open.uri == NS_IBB
-    assert open['sid'] == 'alpha'
+    event = q.expect('stream-iq', iq_type='error', to='bob at localhost/Bob')
 
-    result = IQ(stream, 'result')
-    result['id'] = iq['id']
-    result['from'] = iq['to']
-    result['to'] = 'test at localhost/Resource'
+    # Then try with IBB
 
-    stream.send(result)
+    iq = IQ(stream, 'set')
+    iq['to'] = 'test at localhost/Resource'
+    iq['from'] = 'bob at localhost/Bob'
+    open = iq.addElement((NS_IBB, 'open'))
+    open['sid'] = 'alpha'
+    open['block-size'] = '4096'
+    stream.send(iq)
+
+    q.expect('stream-iq', iq_type='result')
 
     # have the fake client send us some data
     message = domish.Element(('jabber:client', 'message'))
@@ -202,7 +208,7 @@ def test(q, bus, conn, stream):
     data_node = message.addElement((NS_IBB, 'data'))
     data_node['sid'] = 'alpha'
     data_node['seq'] = '0'
-    data_node.addContent(base64.b64encode('hello world'))
+    data_node.addContent(base64.b64encode('hello, world'))
     stream.send(message)
 
     event = q.expect('stream-message', to='bob at localhost/Bob')
@@ -215,7 +221,7 @@ def test(q, bus, conn, stream):
     ibb_data = data_nodes[0]
     assert ibb_data['sid'] == 'alpha'
     binary = base64.b64decode(str(ibb_data))
-    assert binary == 'hello world'
+    assert binary == 'HELLO, WORLD'
 
 
     # Accepting a tube
@@ -254,7 +260,7 @@ def test(q, bus, conn, stream):
     # accept the tube
     channels = filter(lambda x:
       x[1] == "org.freedesktop.Telepathy.Channel.Type.StreamTube.DRAFT" and
-      '42' in x[0],
+      '42' in x[0], # FIXME
       conn.ListChannels())
     assert len(channels) == 1
 
@@ -285,25 +291,22 @@ def test(q, bus, conn, stream):
     assert x['type'] == 'form'
     field = xpath.queryForNodes('/x/field', x)[0]
     assert field['var'] == 'stream-method'
-    assert field['type'] == 'list-multiple'
+    assert field['type'] == 'list-single'
     value = xpath.queryForNodes('/field/option/value', field)[0]
     assert str(value) == NS_BYTESTREAMS
     value = xpath.queryForNodes('/field/option/value', field)[1]
     assert str(value) == NS_IBB
+    si_multiple = xpath.queryForNodes('/si/si-multiple', si)[0]
+    assert si_multiple.uri == NS_SI_MULTIPLE
 
     result = IQ(stream, 'result')
     result['id'] = iq['id']
     result['from'] = iq['to']
     result['to'] = 'test at localhost/Resource'
     res_si = result.addElement((NS_SI, 'si'))
-    res_feature = res_si.addElement((NS_FEATURE_NEG, 'feature'))
-    res_x = res_feature.addElement((NS_X_DATA, 'x'))
-    res_x['type'] = 'submit'
-    res_field = res_x.addElement((None, 'field'))
-    res_field['var'] = 'stream-method'
-    res_value = res_field.addElement((None, 'value'))
+    res_value = res_si.addElement(('', 'value'))
     res_value.addContent(NS_BYTESTREAMS)
-    res_value = res_field.addElement((None, 'value'))
+    res_value = res_si.addElement(('', 'value'))
     res_value.addContent(NS_IBB)
 
     stream.send(result)
@@ -316,15 +319,28 @@ def test(q, bus, conn, stream):
     streamhost = xpath.queryForNodes('/iq/query/streamhost', iq)[0]
     assert streamhost
 
-    iq = IQ(stream, 'set')
+    response_id = iq['id']
+    iq = IQ(stream, 'error')
     iq['to'] = 'test at localhost/Resource'
     iq['from'] = 'bob at localhost/Bob'
-    open = iq.addElement((NS_IBB, 'open'))
-    open['sid'] = sid
-    open['block-size'] = '4096'
+    iq['id'] = response_id
+    error = iq.addElement(('', 'error'))
+    error['type'] = 'auth'
+    error['code'] = '403'
     stream.send(iq)
 
-    q.expect('stream-iq', iq_type='result')
+    event = q.expect('stream-iq', iq_type='set', to='bob at localhost/Bob')
+    iq = event.stanza
+    open = xpath.queryForNodes('/iq/open', iq)[0]
+    assert open.uri == NS_IBB
+    sid = open['sid']
+
+    result = IQ(stream, 'result')
+    result['id'] = iq['id']
+    result['from'] = iq['to']
+    result['to'] = 'test at localhost/Resource'
+
+    stream.send(result)
 
 if __name__ == '__main__':
     exec_test(test)
-- 
1.5.6.5




More information about the Telepathy-commits mailing list