callbacks: Make reply() send multiline batches when supybot.reply.mores.instant > 1

src/callbacks.py

@@ -937,6 +937,9 @@ class NestedCommandsIrcProxy(ReplyIrcProxy):
                 self._sendReply(
                     s=s, target=target, msg=msg,
                     sendImmediately=sendImmediately, stripCtcp=stripCtcp)
+            except:
+                log.exception('Error while sending reply')
+                raise
             finally:
                 self._resetReplyAttributes()
         else:
@@ -1045,15 +1048,29 @@ class NestedCommandsIrcProxy(ReplyIrcProxy):
                     chunk = '%s %s' % (chunk, n)
                 msgs.append(_makeReply(self, msg, chunk, **replyArgs))
 
+            instant_messages = []
+
             while instant > 0 and msgs:
                 instant -= 1
                 response = msgs.pop()
-                sendMsg(response)
+                instant_messages.append(response)
                 # XXX We should somehow allow these to be returned, but
                 #     until someone complains, we'll be fine :)  We
                 #     can't return from here, though, for obvious
                 #     reasons.
                 # return m
+
+            if conf.supybot.protocols.irc.experimentalExtensions() \
+                    and 'draft/multiline' in self.state.capabilities_ack \
+                    and len(instant_messages) > 1:
+                # More than one message to send now, and we are allowed to use
+                # multiline batches, so let's do it
+                self.queueMultilineBatches(
+                    instant_messages, target, allowedLength, sendImmediately)
+            else:
+                for instant_msg in instant_messages:
+                    sendMsg(instant_msg)
+
             if not msgs:
                 return
             prefix = msg.prefix
@@ -1070,6 +1087,55 @@ class NestedCommandsIrcProxy(ReplyIrcProxy):
             self._mores[msg.nick] = (private, msgs)
             return response
 
+    def queueMultilineBatches(self, msgs, target, allowedLength=0,
+                               sendImmediately=False):
+        """Queues the msgs passed as argument in batches using draft/multiline
+        batches.
+
+        This errors if experimentalExtensions is disabled or draft/multiline
+        was not negotiated."""
+        assert conf.supybot.protocols.irc.experimentalExtensions()
+        assert 'draft/multiline' in self.state.capabilities_ack
+
+        if not allowedLength: # 0 indicates this.
+            allowedLength = 512 - self._replyOverhead(target, msg)
+
+        multiline_cap_values = ircutils.parseCapabilityKeyValue(
+            self.state.capabilities_ls['draft/multiline'])
+
+        # All the messages in instant_messages are to be sent
+        # immediately, in multiline batches.
+        max_bytes_per_batch = int(multiline_cap_values['max-bytes'])
+
+        # We have to honor max_bytes_per_batch, but I don't want to
+        # encode messages again here just to have their length, so
+        # let's assume they all have the maximum length.
+        # It's not optimal, but close enough and simplifies the code.
+        messages_per_batch = max_bytes_per_batch // allowedLength
+
+        for batch_msgs in utils.iter.grouper(msgs, messages_per_batch):
+            # TODO: should use sendBatch instead of queueBatch if
+            # sendImmediately is True
+            batch_name = ircutils.makeLabel()
+            batch = []
+            batch.append(ircmsgs.IrcMsg(command='BATCH', args=(
+                '+' + batch_name, 'draft/multiline', target)))
+
+            for (i, batch_msg) in enumerate(batch_msgs):
+                if batch_msg is None:
+                    continue  # 'grouper' generates None at the end
+                assert 'batch' not in batch_msg.server_tags
+                batch_msg.server_tags['batch'] = batch_name
+                if i > 0:
+                    # Tell clients not to add a newline after this
+                    batch_msg.server_tags['draft/multiline-concat'] = None
+                batch.append(batch_msg)
+
+            batch.append(ircmsgs.IrcMsg(
+                command='BATCH', args=('-' + batch_name,)))
+
+            self.queueBatch(batch)
+
     def noReply(self, msg=None):
         if msg is None:
             msg = self.msg

src/ircutils.py

@@ -936,14 +936,22 @@ class AuthenticateDecoder(object):
         return base64.b64decode(b''.join(self.chunks))
 
 
-def parseStsPolicy(logger, policy, parseDuration):
-    parsed_policy = {}
-    for kv in policy.split(','):
+def parseCapabilityKeyValue(s):
+    """Parses a key-value string, in the format used by 'sts' and
+    'draft/multiline."""
+    d = {}
+    for kv in s.split(','):
         if '=' in kv:
             (k, v) = kv.split('=', 1)
-            parsed_policy[k] = v
+            d[k] = v
         else:
-            parsed_policy[kv] = None
+            d[kv] = None
+
+    return d
+
+
+def parseStsPolicy(logger, policy, parseDuration):
+    parsed_policy = parseCapabilityKeyValue(policy)
 
     for key in ('port', 'duration'):
         if key == 'duration' and not parseDuration:

src/utils/iter.py

@@ -161,4 +161,14 @@ def limited(iterable, limit):
         raise ValueError('Expected %s elements in iterable (%r), got %s.' % \
               (limit, iterable, limit-i))
 
+
+def grouper(iterable, n, fillvalue=None):
+    """Collect data into fixed-length chunks or blocks
+
+    grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx
+
+    From https://docs.python.org/3/library/itertools.html#itertools-recipes"""
+    args = [iter(iterable)] * n
+    return zip_longest(*args, fillvalue=fillvalue)
+
 # vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79:

test/test_callbacks.py

@@ -318,7 +318,6 @@ class FunctionsTestCase(SupyTestCase):
         irc.state.supported['statusmsg'] = '+'
         msg = ircmsgs.privmsg('+#foo', 'bar baz', prefix=prefix)
         irc._tagMsg(msg)
-        print(msg.channel)
         self.assertEqual(ircmsgs.privmsg('+#foo', '%s: foo' % msg.nick),
                          callbacks._makeReply(irc, msg, 'foo'))
 
@@ -536,7 +535,6 @@ class PrivmsgTestCase(ChannelPluginTestCase):
 
     def testReplyInstant(self):
         self.assertNoResponse(' ')
-        print(conf.supybot.reply.mores.instant())
         self.assertResponse(
             "eval 'foo '*300",
             "'" + "foo " * 110 + " \x02(2 more messages)\x02")
@@ -695,6 +693,185 @@ class PrivmsgTestCase(ChannelPluginTestCase):
             conf.supybot.reply.whenNotCommand.set(original)
 
 
+class MultilinePrivmsgTestCase(ChannelPluginTestCase):
+    plugins = ('Utilities', 'Misc', 'Web', 'String')
+    conf.allowEval = True
+
+    def setUp(self):
+        super().setUp()
+        self.irc.state.capabilities_ack.add('batch')
+        self.irc.state.capabilities_ack.add('draft/multiline')
+        self.irc.state.capabilities_ls['draft/multiline'] = 'max-bytes=4096'
+        conf.supybot.protocols.irc.experimentalExtensions.setValue(True)
+
+    def tearDown(self):
+        conf.supybot.protocols.irc.experimentalExtensions.setValue(False)
+        conf.supybot.reply.mores.instant.setValue(1)
+        super().tearDown()
+
+    def testReplyInstantSingle(self):
+        self.assertIsNone(self.irc.takeMsg())
+
+        # Single message as reply, no batch
+        self.irc.feedMsg(ircmsgs.privmsg(
+            self.channel, "@eval 'foo '*300", prefix=self.prefix))
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG', args=(self.channel,
+                "test: '" + "foo " * 110 + " \x02(2 more messages)\x02")))
+        self.assertIsNone(self.irc.takeMsg())
+
+    def testReplyInstantBatchPartial(self):
+        """response is shared as a batch + (1 more message)"""
+        self.assertIsNone(self.irc.takeMsg())
+
+        conf.supybot.reply.mores.instant.setValue(2)
+        self.irc.feedMsg(ircmsgs.privmsg(
+            self.channel, "@eval 'foo '*300", prefix=self.prefix))
+
+        # First message opens the batch
+        m = self.irc.takeMsg()
+        self.assertEqual(m.command, 'BATCH', m)
+        batch_name = m.args[0][1:]
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH',
+                args=('+' + batch_name,
+                    'draft/multiline', self.channel)))
+
+        # Second message, first PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel, "test: '" + "foo " * 110),
+                server_tags={'batch': batch_name}))
+
+        # Third message, last PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel,
+                    "test: " + "foo " * 111 + "\x02(1 more message)\x02"),
+                server_tags={'batch': batch_name,
+                    'draft/multiline-concat': None}))
+
+        # Last message, closes the batch
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH', args=(
+                '-' + batch_name,)))
+
+    def testReplyInstantBatchFull(self):
+        """response is entirely instant"""
+        self.assertIsNone(self.irc.takeMsg())
+
+        conf.supybot.reply.mores.instant.setValue(3)
+        self.irc.feedMsg(ircmsgs.privmsg(
+            self.channel, "@eval 'foo '*300", prefix=self.prefix))
+
+        # First message opens the batch
+        m = self.irc.takeMsg()
+        self.assertEqual(m.command, 'BATCH', m)
+        batch_name = m.args[0][1:]
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH',
+                args=('+' + batch_name,
+                    'draft/multiline', self.channel)))
+
+        # Second message, first PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel, "test: '" + "foo " * 110),
+                server_tags={'batch': batch_name}))
+
+        # Third message, a PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel,
+                    "test: " + "foo " * 110 + "foo"),
+                server_tags={'batch': batch_name,
+                    'draft/multiline-concat': None}))
+
+        # Fourth message, last PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel,
+                    "test:  " + "foo " * 79 + "'"),
+                server_tags={'batch': batch_name,
+                    'draft/multiline-concat': None}))
+
+        # Last message, closes the batch
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH', args=(
+                '-' + batch_name,)))
+
+    def testReplyInstantBatchFullMaxBytes(self):
+        """response is entirely instant, but does not fit in a single batch"""
+        self.irc.state.capabilities_ls['draft/multiline'] = 'max-bytes=900'
+        self.assertIsNone(self.irc.takeMsg())
+
+        conf.supybot.reply.mores.instant.setValue(3)
+        self.irc.feedMsg(ircmsgs.privmsg(
+            self.channel, "@eval 'foo '*300", prefix=self.prefix))
+
+        # First message opens the first batch
+        m = self.irc.takeMsg()
+        self.assertEqual(m.command, 'BATCH', m)
+        batch_name = m.args[0][1:]
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH',
+                args=('+' + batch_name,
+                    'draft/multiline', self.channel)))
+
+        # Second message, first PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel, "test: '" + "foo " * 110),
+                server_tags={'batch': batch_name}))
+
+        # Third message, a PRIVMSG
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel,
+                    "test: " + "foo " * 110 + "foo"),
+                server_tags={'batch': batch_name,
+                    'draft/multiline-concat': None}))
+
+        # closes the first batch
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH', args=(
+                '-' + batch_name,)))
+
+        # opens the second batch
+        m = self.irc.takeMsg()
+        self.assertEqual(m.command, 'BATCH', m)
+        batch_name = m.args[0][1:]
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH',
+                args=('+' + batch_name,
+                    'draft/multiline', self.channel)))
+
+        # last PRIVMSG (and also the first of its batch)
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='PRIVMSG',
+                args=(self.channel,
+                    "test:  " + "foo " * 79 + "'"),
+                server_tags={'batch': batch_name}))
+
+        # Last message, closes the second batch
+        m = self.irc.takeMsg()
+        self.assertEqual(
+            m, ircmsgs.IrcMsg(command='BATCH', args=(
+                '-' + batch_name,)))
+
+
 class PluginRegexpTestCase(PluginTestCase):
     plugins = ()
     class PCAR(callbacks.PluginRegexp):