coretasks: track all channel modes

sopel/coretasks.py

@@ -79,7 +79,8 @@ def _join_event_processing(bot):
             channel = bot.memory['join_events_queue'].popleft()
         except IndexError:
             break
-        LOGGER.debug('Sending WHO after channel JOIN: %s', channel)
+        LOGGER.debug("Sending MODE and WHO after channel JOIN: %s", channel)
+        bot.write(["MODE", channel])
         _send_who(bot, channel)
 
 
@@ -355,35 +356,41 @@ def handle_names(bot, trigger):
 @module.thread(False)
 @module.unblockable
 def track_modes(bot, trigger):
-    """Track usermode changes and keep our lists of ops up to date."""
-    # Mode message format: <channel> *( ( "-" / "+" ) *<modes> *<modeparams> )
-    if len(trigger.args) < 3:
-        # We need at least [channel, mode, nickname] to do anything useful
-        # MODE messages with fewer args won't help us
-        LOGGER.debug("Received an apparently useless MODE message: {}"
-                     .format(trigger.raw))
+    """Parse channel mode changes from MODE command."""
+    _parse_modes(bot, trigger.args)
+
+
+@module.priority('high')
+@module.event(events.RPL_CHANNELMODEIS)
+@module.thread(False)
+@module.unblockable
+def initial_modes(bot, trigger):
+    """Parse channel modes from MODE query response."""
+    args = trigger.args[1:]
+    _parse_modes(bot, args)
+
+
+def _parse_modes(bot, args): #channel_name, modes, params):
+    """Track mode changes and keep our privilege and mode lists updated."""
+    channel_name = Identifier(args[0])
+    if channel_name.is_nick():
+        # We don't do anything with user modes
         return
+    channel = bot.channels[channel_name]
     # Our old MODE parsing code checked if any of the args was empty.
     # Somewhere around here would be a good place to re-implement that if it's
     # actually necessary to guard against some non-compliant IRCd. But for now
     # let's just log malformed lines to the debug log.
-    if not all(trigger.args):
-        LOGGER.debug("The server sent a possibly malformed MODE message: {}"
-                     .format(trigger.raw))
-
+    if len(args) < 2 or not all(args):
+        LOGGER.debug(
+            "The server sent a possibly malformed MODE message: %r",
+            args
+        )
     # From here on, we will make a (possibly dangerous) assumption that the
     # received MODE message is more-or-less compliant
-    channel = Identifier(trigger.args[0])
-    # If the first character of where the mode is being set isn't a #
-    # then it's a user mode, not a channel mode, so we'll ignore it.
-    # TODO: Handle CHANTYPES from ISUPPORT numeric (005)
-    # (Actually, most of this function should be rewritten again when we parse
-    # ISUPPORT...)
-    if channel.is_nick():
-        return
 
-    modestring = trigger.args[1]
-    nicks = [Identifier(nick) for nick in trigger.args[2:]]
+    modestring = args[1]
+    params = args[2:]
 
     mapping = {
         "v": module.VOICE,
@@ -395,49 +402,98 @@ def track_modes(bot, trigger):
         "Y": module.OPER,
     }
 
-    # Parse modes before doing anything else
-    modes = []
-    sign = ''
+    # Process modes
+    sign = ""
+    param_idx = 0
     for char in modestring:
-        # There was a comment claiming IRC allows e.g. MODE +aB-c foo, but it
-        # doesn't seem to appear in any RFCs. But modern.ircdocs.horse shows
-        # it, so we'll leave in the extra parsing for now.
-        if char in '+-':
+        # Are we setting or unsetting
+        if char in "+-":
             sign = char
-        elif char in mapping:
-            # Filter out unexpected modes and hope they don't have parameters
-            modes.append(sign + char)
-
-    # Try to map modes to arguments, after sanity-checking
-    if len(modes) != len(nicks) or not all([nick.is_nick() for nick in nicks]):
-        # Something fucky happening, like unusual batching of non-privilege
-        # modes together with the ones we expect. Way easier to just re-WHO
-        # than try to account for non-standard parameter-taking modes.
-        LOGGER.debug('Sending WHO for channel: %s', channel)
-        _send_who(bot, channel)
-        return
+            continue
 
-    for (mode, nick) in zip(modes, nicks):
-        priv = bot.channels[channel].privileges.get(nick, 0)
-        # Log a warning if the two privilege-tracking data structures
-        # get out of sync. That should never happen.
-        # This is a good place to verify that bot.channels is doing
-        # what it's supposed to do before ultimately removing the old,
-        # deprecated bot.privileges structure completely.
-        ppriv = bot.privileges[channel].get(nick, 0)
-        if priv != ppriv:
-            LOGGER.warning("Privilege data error! Please share Sopel's"
-                           "raw log with the developers, if enabled. "
-                           "(Expected {} == {} for {} in {}.)"
-                           .format(priv, ppriv, nick, channel))
-        value = mapping.get(mode[1])
-        if value is not None:
-            if mode[0] == '+':
-                priv = priv | value
-            else:
-                priv = priv & ~value
-            bot.privileges[channel][nick] = priv
-            bot.channels[channel].privileges[nick] = priv
+        if "CHANMODES" in bot._isupport:
+            chanmodes = bot._isupport.CHANMODES
+        else:
+            chanmodes = {x: "" for x in "ABCD"}
+
+        if char in chanmodes["A"]:
+            # Type A (beI, etc) have a nick or address param to add/remove
+            param = params[param_idx]
+            if char not in channel.modes:
+                channel.modes[char] = set()
+            if sign == "+":
+                channel.modes[char].add(params[param_idx])
+            elif param in channel.modes[char]:
+                channel.modes[char].remove(params[param_idx])
+            param_idx += 1
+        elif char in chanmodes["B"]:
+            # Type B (k, etc) always have a param
+            param = params[param_idx]
+            if sign == "+":
+                channel.modes[char] = params[param_idx]
+            elif char in channel.modes:
+                channel.modes.pop(char)
+            param_idx += 1
+        elif char in chanmodes["C"]:
+            # Type C (l, etc) have a param only when setting
+            if sign == "+":
+                param = params[param_idx]
+                channel.modes[char] = params[param_idx]
+                param_idx += 1
+            elif char in channel.modes:
+                channel.modes.pop(char)
+        elif char in chanmodes["D"]:
+            # Type D (aciLmMnOpqrRst, etc) have no params
+            if sign == "+":
+                channel.modes[char] = True
+            elif char in channel.modes:
+                channel.modes.pop(char)
+        elif (
+            char in mapping and
+            (
+                "PREFIX" not in bot._isupport or
+                char in bot._isupport.PREFIX
+            )
+        ):
+            # User privs modes, always have a param
+            nick = Identifier(params[param_idx])
+            priv = channel.privileges.get(nick, 0)
+            # Log a warning if the two privilege-tracking data structures
+            # get out of sync. That should never happen.
+            # This is a good place to verify that bot.channels is doing
+            # what it's supposed to do before ultimately removing the old,
+            # deprecated bot.privileges structure completely.
+            ppriv = bot.privileges[channel_name].get(nick, 0)
+            if priv != ppriv:
+                LOGGER.warning(
+                    (
+                        "Privilege data error! Please share Sopel's "
+                        "raw log with the developers, if enabled. "
+                        "(Expected %s == %s for %r in %r)"
+                    ),
+                    priv,
+                    ppriv,
+                    nick,
+                    channel
+                )
+            value = mapping.get(char)
+            if value is not None:
+                if sign == "+":
+                    priv = priv | value
+                else:
+                    priv = priv & ~value
+                bot.privileges[channel_name][nick] = priv
+                channel.privileges[nick] = priv
+            param_idx += 1
+        else:
+            # Might be in a mode block past A/B/C/D, but we don't speak those.
+            # Send a WHO to ensure no user priv modes we're skipping are lost.
+            LOGGER.warning(
+                "Unknown MODE message, sending WHO. Message was: %r",
+                args
+            )
+            _send_who(bot, channel_name)
+            return
 
 
 @module.event('NICK')
@@ -594,7 +650,8 @@ def track_join(bot, trigger):
             LOGGER.debug('JOIN event added to queue for channel: %s', channel)
             bot.memory['join_events_queue'].append(channel)
         else:
-            LOGGER.debug('Send direct WHO for channel: %s', channel)
+            LOGGER.debug("Send MODE and direct WHO for channel: %s", channel)
+            bot.write(["MODE", channel])
             _send_who(bot, channel)
 
     # set initial values

sopel/tools/__init__.py

@@ -520,6 +520,7 @@ def is_nick(self):
         :return: ``True`` if this :py:class:`Identifier` is a nickname;
                  ``False`` if it appears to be a channel
         """
+        # TODO: Handle CHANTYPES from ISUPPORT numeric (005)
         return self and not self.startswith(_channel_prefixes)
 
 

sopel/tools/target.py

@@ -79,6 +79,17 @@ def __init__(self, name):
         self.topic = ''
         """The topic of the channel."""
 
+        self.modes = {}
+        """The channel's modes.
+
+        For type A modes (nick/address list), the value is a set. For type B
+        (parameter) or C (parameter when setting), the value is a string. For
+        type D, the value is True.
+
+        Note: Type A modes may only contain changes the bot has observed. Sopel
+        does not automatically populate all modes and lists.
+        """
+
         self.last_who = None
         """The last time a WHO was requested for the channel."""
 

test/test_coretasks.py

@@ -5,6 +5,7 @@
 import pytest
 
 from sopel import coretasks
+from sopel.irc import isupport
 from sopel.module import ADMIN, HALFOP, OP, OWNER, VOICE
 from sopel.tests import rawlist
 from sopel.tools import Identifier
@@ -61,6 +62,7 @@ def test_bot_mixed_mode_removal(mockbot, ircfactory):
         GitHub issue #1575 (https://github.com/sopel-irc/sopel/pull/1575).
     """
     irc = ircfactory(mockbot)
+    irc.bot._isupport = isupport.ISupport(chanmodes=("b", "", "", "m", tuple()))
     irc.channel_joined('#test', ['Uvoice', 'Uop'])
 
     irc.mode_set('#test', '+qao', ['Uvoice', 'Uvoice', 'Uvoice'])
@@ -86,22 +88,70 @@ def test_bot_mixed_mode_types(mockbot, ircfactory):
         GitHub issue #1575 (https://github.com/sopel-irc/sopel/pull/1575).
     """
     irc = ircfactory(mockbot)
+    irc.bot._isupport = isupport.ISupport(chanmodes=("be", "", "", "mn", tuple()))
     irc.channel_joined('#test', [
         'Uvoice', 'Uop', 'Uadmin', 'Uvoice2', 'Uop2', 'Uadmin2'])
-    irc.mode_set('#test', '+amov', ['Uadmin', 'Uop', 'Uvoice'])
+    irc.mode_set('#test', '+amovn', ['Uadmin', 'Uop', 'Uvoice'])
 
     assert mockbot.channels["#test"].privileges[Identifier("Uadmin")] == ADMIN
+    assert mockbot.channels["#test"].modes["m"]
     assert mockbot.channels["#test"].privileges[Identifier("Uop")] == OP
     assert mockbot.channels["#test"].privileges[Identifier("Uvoice")] == VOICE
+    assert mockbot.channels["#test"].modes["n"]
 
-    irc.mode_set('#test', '+abov', ['Uadmin2', 'x!y@z', 'Uop2', 'Uvoice2'])
+    irc.mode_set('#test', '+above', ['Uadmin2', 'x!y@z', 'Uop2', 'Uvoice2', 'a!b@c'])
 
-    assert mockbot.channels["#test"].privileges[Identifier("Uadmin2")] == 0
-    assert mockbot.channels["#test"].privileges[Identifier("Uop2")] == 0
-    assert mockbot.channels["#test"].privileges[Identifier("Uvoice2")] == 0
+    assert mockbot.channels["#test"].privileges[Identifier("Uadmin2")] == ADMIN
+    assert "x!y@z" in mockbot.channels["#test"].modes["b"]
+    assert mockbot.channels["#test"].privileges[Identifier("Uop2")] == OP
+    assert mockbot.channels["#test"].privileges[Identifier("Uvoice2")] == VOICE
+    assert "a!b@c" in mockbot.channels["#test"].modes["e"]
 
-    assert mockbot.backend.message_sent == rawlist('WHO #test'), (
-        'Upon finding an unexpected nick, the bot must send a WHO request.')
+
+def test_bot_unknown_mode(mockbot, ircfactory):
+    """Ensure modes not in PREFIX or CHANMODES trigger a WHO"""
+    irc = ircfactory(mockbot)
+    irc.bot._isupport = isupport.ISupport(chanmodes=("b", "", "", "mnt", tuple()))
+    irc.channel_joined("#test", ["Alex", "Bob", "Cheryl"])
+    irc.mode_set("#test", "+te", ["Alex"])
+
+    assert mockbot.channels["#test"].privileges[Identifier("Alex")] == 0
+    assert mockbot.backend.message_sent == rawlist("WHO #test"), (
+        "Upon finding an unknown mode, the bot must send a WHO request.")
+
+
+def test_bot_unknown_priv_mode(mockbot, ircfactory):
+    """Ensure modes in `mapping` but not PREFIX are treated as unknown"""
+    irc = ircfactory(mockbot)
+    irc.bot._isupport = isupport.ISupport(prefix={"o": "@", "v": "+"})
+    irc.channel_joined("#test", ["Alex", "Bob", "Cheryl"])
+    irc.mode_set("#test", "+oh", ["Alex", "Bob"])
+
+    assert mockbot.channels["#test"].privileges[Identifier("Bob")] == 0
+    assert mockbot.backend.message_sent == rawlist("WHO #test"), (
+        "The bot must treat mapped but non-PREFIX modes as unknown")
+
+
+def test_handle_rpl_channelmodeis(mockbot, ircfactory):
+    """Test handling RPL_CHANNELMODEIS events, response to MODE query."""
+    rpl_channelmodeis= " ".join([
+        ":niven.freenode.net",
+        "324",
+        "TestName",
+        "#test",
+        "+knlt",
+        "hunter2",
+        ":1",
+    ])
+    irc = ircfactory(mockbot)
+    irc.bot._isupport = isupport.ISupport(chanmodes=("b", "k", "l", "mnt", tuple()))
+    irc.channel_joined("#test", ["Alex", "Bob", "Cheryl"])
+    mockbot.on_message(rpl_channelmodeis)
+
+    assert mockbot.channels["#test"].modes["k"] == "hunter2"
+    assert mockbot.channels["#test"].modes["n"]
+    assert mockbot.channels["#test"].modes["l"] == "1"
+    assert mockbot.channels["#test"].modes["t"]
 
 
 def test_mode_colon(mockbot, ircfactory):