diff --git a/src/callbacks.py b/src/callbacks.py index 60a232025..7cc6ecc99 100644 --- a/src/callbacks.py +++ b/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 diff --git a/src/ircutils.py b/src/ircutils.py index 8440a876e..ccc8e1ae6 100644 --- a/src/ircutils.py +++ b/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: diff --git a/src/utils/iter.py b/src/utils/iter.py index dc60f093c..9619307c4 100644 --- a/src/utils/iter.py +++ b/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: diff --git a/test/test_callbacks.py b/test/test_callbacks.py index 3cf7a20a7..bfb1d0e15 100644 --- a/test/test_callbacks.py +++ b/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):