ibb: Improve message payload handling and documentation

* monkey patch a descriptor instead of using Message.ext for the
  payload

* rename StanzaType to IBBStanzaType

* extend and improve docs

aioxmpp/ibb/__init__.py

@@ -23,8 +23,10 @@
 :mod:`~aioxmpp.ibb` --- In-Band Bytestreams (:xep:`0047`)
 #########################################################
 
-This subpackage provides support for in-band bytestreams.
-
+This subpackage provides support for in-band bytestreams.  The
+bytestreams are exposed as instances of :class:`asyncio.Transport`,
+which allows to speak any protocol implemented as
+:class:`asyncio.Protocol` over them.
 
 .. autoclass:: IBBService
 
@@ -33,8 +35,12 @@
 .. currentmodule:: aioxmpp.ibb.service
 .. autoclass:: IBBTransport()
 
+For serializing and deserializing data payloads carried by
+:class:`~aioxmpp.Message` stanzas, a descriptor is added to them:
+
+.. attribute:: aioxmpp.Message.xep0047_data
 """
-from .xso import StanzaType as IBBStanzaType # NOQA
+from .xso import IBBStanzaType # NOQA
 from .service import IBBService # NOQA
 
 # import aioxmpp.ibb.service

aioxmpp/ibb/service.py

@@ -44,27 +44,23 @@ class IBBTransport(asyncio.Transport):
     """
     The transport for IBB sessions.
 
-    .. note:: Never instantiate this class, all instances of this
-              class are created by :class:`~aioxmpp.ibb.IBBService`.
+    .. note:: Never instantiate this class directly, all instances of
+              this class are created by :class:`~aioxmpp.ibb.IBBService`.
 
     The following keys are supported for
-    :meth:`~asyncio.Transport.get_extra_info`:
+    :meth:`~asyncio.BaseTransport.get_extra_info`:
 
        `block_size`
-
            The maximal block size of data in a IBB stanza.
 
-        `peer_jid`
-
-           The JID of the peer.
-
-        `sid`
-
-           The session id of the unerlying IBB session.
+       `peer_jid`
+          The JID of the peer.
 
-        `stanza_type`
+       `sid`
+          The session id of the unerlying IBB session.
 
-           The used stanza type.
+       `stanza_type`
+          The used stanza type.
     """
 
     def __init__(self, service, peer_jid, sid, stanza_type, block_size):
@@ -149,7 +145,7 @@ def _write_task_main(self):
                 data = self._write_buffer[:self._block_size]
                 self._write_buffer = self._write_buffer[len(data):]
 
-                if self._stanza_type == ibb_xso.StanzaType.IQ:
+                if self._stanza_type == ibb_xso.IBBStanzaType.IQ:
                     stanza = aioxmpp.IQ(
                         aioxmpp.IQType.SET,
                         to=self._peer_jid,
@@ -159,18 +155,15 @@ def _write_task_main(self):
                             data
                         )
                     )
-                elif self._stanza_type == ibb_xso.StanzaType.MESSAGE:
+                elif self._stanza_type == ibb_xso.IBBStanzaType.MESSAGE:
                     stanza = aioxmpp.Message(
                         aioxmpp.MessageType.NORMAL,
                         to=self._peer_jid,
                     )
-                    aioxmpp.Message.ext.fill_into_dict(
-                        [ibb_xso.Data(
-                            self._sid,
-                            self._outgoing_seq,
-                            data
-                        )],
-                        stanza.ext
+                    stanza.xep0047_data = ibb_xso.Data(
+                        self._sid,
+                        self._outgoing_seq,
+                        data
                     )
 
                 try:
@@ -296,7 +289,7 @@ class IBBService(service.Service):
 
        The protocol factory to be used when an unexpected connection
        is established. This *must* be set when changing
-       :attr:`SessionLimit` to a non-zero value.
+       :attr:`SESSION_LIMIT` to a non-zero value.
 
     .. signal:: on_session_accepted(transport, protocol)
 
@@ -338,7 +331,7 @@ def expect_session(self, protocol_factory, peer_jid, sid):
         """
         Whitelist the session with `peer_jid` and the session id `sid` and
         return it when it is established. This is meant to be used
-        with signalling protocols like jingle and is the counter part
+        with signalling protocols like Jingle and is the counterpart
         to :meth:`open_session`.
 
         :returns: an awaitable object, whose result is the tuple
@@ -354,7 +347,7 @@ def on_done(fut):
 
     @asyncio.coroutine
     def open_session(self, protocol_factory, peer_jid, *,
-                     stanza_type=ibb_xso.StanzaType.IQ,
+                     stanza_type=ibb_xso.IBBStanzaType.IQ,
                      block_size=4096, sid=None):
         """
         Establish an with `peer_jid` in-band bytestream session and return
@@ -366,7 +359,7 @@ def open_session(self, protocol_factory, peer_jid, *,
         :param peer_jid: the JID with which to establish the byte-stream.
         :type peer_jid: :class:`aioxmpp.JID`
         :param stanza_type: the stanza type to use
-        :type stanza_type: class:`aioxmpp.ibb.xso.StanzaType`
+        :type stanza_type: class:`~aioxmpp.ibb.IBBStanzaType`
         :param block_size: the maximal size of blocks to transfer
         :type block_size: :class:`int`
         :param sid: the session id to use
@@ -494,11 +487,10 @@ def _handle_data(self, iq):
 
     @service.inbound_message_filter
     def _handle_message(self, msg):
-        if not msg.ext[ibb_xso.Data.TAG]:
+        if msg.xep0047_data is None:
             return msg
 
-        payload = msg.ext[ibb_xso.Data.TAG][0]
-
+        payload = msg.xep0047_data
         peer_jid = msg.from_
         sid = payload.sid
 

aioxmpp/ibb/xso.py

@@ -30,12 +30,24 @@
 namespaces.xep0047 = "http://jabber.org/protocol/ibb"
 
 
-class StanzaType(enum.Enum):
+class IBBStanzaType(enum.Enum):
     """
-    The two stanza types supported by IBB for transporting data.
+    Enumeration of the the two stanza types supported by IBB for
+    transporting data.
 
-    The default is to use `IQ` stanzas and ususally this should not be
-    changed.
+   .. attribute:: IQ
+
+      Send the in-band bytestream data using IQ stanzas. This is
+      recommended and default. The reply mechanism of IQ allows
+      tracking the connectivitiy and implements basic rate limiting,
+      since we wait for the reply to the previous message before
+      sending a new one.
+
+   .. attribute:: MESSAGE
+
+      Send the in-band bytestream data using Message stanzas. This is
+      not recommended since lost packages due to intermittent
+      connectivity failures will not be obvious.
     """
     IQ = "iq"
     MESSAGE = "message"
@@ -52,8 +64,8 @@ class Open(xso.XSO):
 
     stanza = xso.Attr(
         "stanza",
-        type_=xso.EnumCDataType(StanzaType),
-        default=StanzaType.IQ,
+        type_=xso.EnumCDataType(IBBStanzaType),
+        default=IBBStanzaType.IQ,
     )
 
 
@@ -76,4 +88,5 @@ def __init__(self, sid, seq, content):
         self.sid = sid
         self.content = content
 
-stanza.Message.ext._register(Data)
+
+stanza.Message.xep0047_data = xso.Child([Data])

tests/ibb/test_e2e.py

@@ -83,6 +83,38 @@ def test_ibb(self):
         transport2, proto2 = yield from handle2_fut
 
 
+        # transfer data
+        transport2.write(b"this")
+        transport2.write(b"is")
+        transport2.write(b"data")
+        transport2.close()
+
+        # assert that both protocols get notified (otherwise time out)
+        e1 = yield from proto1.connection_lost_fut
+        self.assertIsNone(e1)
+        e2 = yield from proto2.connection_lost_fut
+        self.assertIsNone(e2)
+
+        self.assertEqual(proto1.data, b"thisisdata")
+
+    @blocking_timed
+    @asyncio.coroutine
+    def test_ibb_message(self):
+        client1 = yield from self.make_client()
+        client2 = yield from self.make_client()
+
+        s1 = client1.summon(aioxmpp.ibb.IBBService)
+        s2 = client2.summon(aioxmpp.ibb.IBBService)
+
+        # set-up the session
+        handle2_fut = s2.expect_session(
+            TestProtocol, client1.local_jid, "fnord")
+        transport1, proto1 = yield from s1.open_session(
+            TestProtocol, client2.local_jid, sid="fnord",
+            stanza_type=aioxmpp.ibb.IBBStanzaType.MESSAGE
+        )
+        transport2, proto2 = yield from handle2_fut
+
         # transfer data
         transport2.write(b"this")
         transport2.write(b"is")

tests/ibb/test_service.py

@@ -90,7 +90,7 @@ def test_open_session_default(self):
 
         self.assertEqual(
             handle.get_extra_info("stanza_type"),
-            ibb_xso.StanzaType.IQ
+            ibb_xso.IBBStanzaType.IQ
         )
 
         self.assertEqual(
@@ -102,7 +102,7 @@ def test_open_session_default(self):
         self.assertFalse(kwargs)
         self.assertEqual(iq.to, TEST_JID1)
         self.assertEqual(iq.payload.sid, "sentinel")
-        self.assertEqual(iq.payload.stanza, ibb_xso.StanzaType.IQ)
+        self.assertEqual(iq.payload.stanza, ibb_xso.IBBStanzaType.IQ)
         self.assertEqual(iq.payload.block_size, 4096)
 
 
@@ -115,7 +115,7 @@ def test_open_session_non_default(self):
                 self.s.open_session(
                     protocol,
                     TEST_JID1,
-                    stanza_type=ibb_xso.StanzaType.MESSAGE,
+                    stanza_type=ibb_xso.IBBStanzaType.MESSAGE,
                     block_size=8192,
                 )
             )
@@ -137,21 +137,21 @@ def test_open_session_non_default(self):
 
         self.assertEqual(
             handle.get_extra_info("stanza_type"),
-            ibb_xso.StanzaType.MESSAGE
+            ibb_xso.IBBStanzaType.MESSAGE
         )
 
         (s, (iq,), kwargs), = self.cc.send.mock_calls
         self.assertFalse(kwargs)
         self.assertEqual(iq.to, TEST_JID1)
         self.assertEqual(iq.payload.sid, "sentinel")
-        self.assertEqual(iq.payload.stanza, ibb_xso.StanzaType.MESSAGE)
+        self.assertEqual(iq.payload.stanza, ibb_xso.IBBStanzaType.MESSAGE)
         self.assertEqual(iq.payload.block_size, 8192)
 
     def test_open_request_enforce_limit(self):
         open_ = ibb_xso.Open()
         open_.sid = "sentinel"
         open_.block_size = 8192
-        open_.stanza = ibb_xso.StanzaType.IQ
+        open_.stanza = ibb_xso.IBBStanzaType.IQ
 
         iq = aioxmpp.IQ(
             aioxmpp.IQType.SET,
@@ -172,7 +172,7 @@ def test_open_request_resource_constraint(self):
         open_ = ibb_xso.Open()
         open_.sid = "sentinel"
         open_.block_size = 1<<32
-        open_.stanza = ibb_xso.StanzaType.IQ
+        open_.stanza = ibb_xso.IBBStanzaType.IQ
 
         iq = aioxmpp.IQ(
             aioxmpp.IQType.SET,
@@ -203,7 +203,7 @@ def on_connection_accepted(transport, protocol):
         open_ = ibb_xso.Open()
         open_.sid = "sentinel"
         open_.block_size = 8192
-        open_.stanza = ibb_xso.StanzaType.IQ
+        open_.stanza = ibb_xso.IBBStanzaType.IQ
 
         iq = aioxmpp.IQ(
             aioxmpp.IQType.SET,
@@ -220,7 +220,7 @@ def test_open_request_expected_session(self):
         open_ = ibb_xso.Open()
         open_.sid = "sentinel"
         open_.block_size = 8192
-        open_.stanza = ibb_xso.StanzaType.IQ
+        open_.stanza = ibb_xso.IBBStanzaType.IQ
 
         iq = aioxmpp.IQ(
             aioxmpp.IQType.SET,
@@ -478,7 +478,7 @@ def setUp(self):
         protocol_factory = unittest.mock.Mock()
         self.handle, self.protocol = run_coroutine(
             self.s.open_session(protocol_factory, TEST_JID1,
-                                stanza_type=ibb_xso.StanzaType.MESSAGE)
+                                stanza_type=ibb_xso.IBBStanzaType.MESSAGE)
         )
         self.cc.send.mock_calls.clear()
 
@@ -511,11 +511,11 @@ def patched_send(stanza):
             run_coroutine(fut)
             msg = patched_send.argument
             self.assertEqual(msg.to, TEST_JID1)
-            self.assertIsInstance(msg.ext[ibb_xso.Data.TAG][0], ibb_xso.Data)
-            self.assertEqual(msg.ext[ibb_xso.Data.TAG][0].sid,
+            self.assertIsInstance(msg.xep0047_data, ibb_xso.Data)
+            self.assertEqual(msg.xep0047_data.sid,
                              self.handle.get_extra_info("sid"))
-            self.assertEqual(msg.ext[ibb_xso.Data.TAG][0].seq, i)
-            self.assertEqual(msg.ext[ibb_xso.Data.TAG][0].content, b"some data")
+            self.assertEqual(msg.xep0047_data.seq, i)
+            self.assertEqual(msg.xep0047_data.content, b"some data")
 
     # def test_send_long(self):
     #     run_coroutine(self.handle.send(b" " * 4097))
@@ -536,13 +536,10 @@ def test_receive(self):
                 from_=TEST_JID1,
                 to=TEST_FROM,
             )
-            aioxmpp.Message.ext.fill_into_dict(
-                [ibb_xso.Data(
-                    self.handle.get_extra_info("sid"),
-                    i,
-                    text
-                )],
-                stanza.ext
+            stanza.xep0047_data = ibb_xso.Data(
+                self.handle.get_extra_info("sid"),
+                i,
+                text
             )
 
             total_text += text
@@ -561,13 +558,10 @@ def test_receive_invalid_session(self):
             from_=TEST_JID1,
             to=TEST_FROM,
         )
-        aioxmpp.Message.ext.fill_into_dict(
-            [ibb_xso.Data(
-                "fnord",
-                0,
-                b"data",
-            )],
-            stanza.ext
+        stanza.xep0047_data = ibb_xso.Data(
+            "fnord",
+            0,
+            b"data",
         )
 
         self.s._handle_message(stanza)
@@ -583,13 +577,10 @@ def test_receive_invalid_seq(self):
             to=TEST_FROM,
         )
 
-        aioxmpp.Message.ext.fill_into_dict(
-            [ibb_xso.Data(
-                self.handle.get_extra_info("sid"),
-                1,
-                b"data",
-            )],
-            stanza.ext
+        stanza.xep0047_data = ibb_xso.Data(
+            self.handle.get_extra_info("sid"),
+            1,
+            b"data",
         )
 
         self.s._handle_message(stanza)