diff options
| -rw-r--r-- | bot/cogs/help_channels.py | 63 | ||||
| -rw-r--r-- | bot/utils/redis_cache.py | 19 | ||||
| -rw-r--r-- | tests/bot/utils/test_redis_cache.py | 4 | 
3 files changed, 53 insertions, 33 deletions
diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 70cef339a..01c38b408 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -15,6 +15,7 @@ from discord.ext import commands  from bot import constants  from bot.bot import Bot +from bot.utils import RedisCache  from bot.utils.checks import with_role_check  from bot.utils.scheduling import Scheduler @@ -99,13 +100,24 @@ class HelpChannels(Scheduler, commands.Cog):      Help channels are named after the chemical elements in `bot/resources/elements.json`.      """ +    # This cache tracks which channels are claimed by which members. +    # RedisCache[discord.TextChannel.id, t.Union[discord.User.id, discord.Member.id]] +    help_channel_claimants = RedisCache() + +    # This cache maps a help channel to whether it has had any +    # activity other than the original claimant. True being no other +    # activity and False being other activity. +    # RedisCache[discord.TextChannel.id, bool] +    unanswered = RedisCache() + +    # This dictionary maps a help channel to the time it was claimed +    # RedisCache[discord.TextChannel.id, datetime.datetime] +    claim_times = RedisCache() +      def __init__(self, bot: Bot):          super().__init__()          self.bot = bot -        self.help_channel_claimants: ( -            t.Dict[discord.TextChannel, t.Union[discord.Member, discord.User]] -        ) = {}          # Categories          self.available_category: discord.CategoryChannel = None @@ -125,16 +137,6 @@ class HelpChannels(Scheduler, commands.Cog):          self.on_message_lock = asyncio.Lock()          self.init_task = self.bot.loop.create_task(self.init_cog()) -        # Stats - -        # This dictionary maps a help channel to the time it was claimed -        self.claim_times: t.Dict[int, datetime] = {} - -        # This dictionary maps a help channel to whether it has had any -        # activity other than the original claimant. True being no other -        # activity and False being other activity. -        self.unanswered: t.Dict[int, bool] = {} -      def cog_unload(self) -> None:          """Cancel the init task and scheduled tasks when the cog unloads."""          log.trace("Cog unload: cancelling the init_cog task") @@ -197,7 +199,7 @@ class HelpChannels(Scheduler, commands.Cog):      async def dormant_check(self, ctx: commands.Context) -> bool:          """Return True if the user is the help channel claimant or passes the role check.""" -        if self.help_channel_claimants.get(ctx.channel) == ctx.author: +        if await self.help_channel_claimants.get(ctx.channel.id) == ctx.author.id:              log.trace(f"{ctx.author} is the help channel claimant, passing the check for dormant.")              self.bot.stats.incr("help.dormant_invoke.claimant")              return True @@ -223,7 +225,7 @@ class HelpChannels(Scheduler, commands.Cog):          if ctx.channel.category == self.in_use_category:              if await self.dormant_check(ctx):                  with suppress(KeyError): -                    del self.help_channel_claimants[ctx.channel] +                    await self.help_channel_claimants.delete(ctx.channel.id)                  await self.remove_cooldown_role(ctx.author)                  # Ignore missing task when cooldown has passed but the channel still isn't dormant. @@ -546,19 +548,20 @@ class HelpChannels(Scheduler, commands.Cog):          self.bot.stats.incr(f"help.dormant_calls.{caller}") -        if channel.id in self.claim_times: -            claimed = self.claim_times[channel.id] -            in_use_time = datetime.now() - claimed +        claimed_timestamp = await self.claim_times.get(channel.id) +        if claimed_timestamp: +            claimed = datetime.fromtimestamp(claimed_timestamp) +            in_use_time = datetime.utcnow() - claimed              self.bot.stats.timing("help.in_use_time", in_use_time) -        if channel.id in self.unanswered: -            if self.unanswered[channel.id]: +        unanswered = await self.unanswered.get(channel.id) +        if unanswered is not None: +            if unanswered:                  self.bot.stats.incr("help.sessions.unanswered")              else:                  self.bot.stats.incr("help.sessions.answered")          log.trace(f"Position of #{channel} ({channel.id}) is actually {channel.position}.") -          log.trace(f"Sending dormant message for #{channel} ({channel.id}).")          embed = discord.Embed(description=DORMANT_MSG)          await channel.send(embed=embed) @@ -638,16 +641,16 @@ class HelpChannels(Scheduler, commands.Cog):              log.trace(f"Checking if #{channel} ({channel.id}) has been answered.")              # Check if there is an entry in unanswered (does not persist across restarts) -            if channel.id in self.unanswered: -                claimant = self.help_channel_claimants.get(channel) -                if not claimant: -                    # The mapping for this channel was lost, we can't do anything. +            if await self.unanswered.contains(channel.id): +                claimant_id = await self.help_channel_claimants.get(channel.id) +                if not claimant_id: +                    # The mapping for this channel doesn't exist, we can't do anything.                      return                  # Check the message did not come from the claimant -                if claimant.id != message.author.id: +                if claimant_id != message.author.id:                      # Mark the channel as answered -                    self.unanswered[channel.id] = False +                    await self.unanswered.set(channel.id, False)      @commands.Cog.listener()      async def on_message(self, message: discord.Message) -> None: @@ -680,12 +683,12 @@ class HelpChannels(Scheduler, commands.Cog):              await self.move_to_in_use(channel)              await self.revoke_send_permissions(message.author)              # Add user with channel for dormant check. -            self.help_channel_claimants[channel] = message.author +            await self.help_channel_claimants.set(channel.id, message.author.id)              self.bot.stats.incr("help.claimed") -            self.claim_times[channel.id] = datetime.now() -            self.unanswered[channel.id] = True +            await self.claim_times.set(channel.id, datetime.utcnow().timestamp()) +            await self.unanswered.set(channel.id, True)              log.trace(f"Releasing on_message lock for {message.id}.") diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index de80cee84..347a0e54a 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -11,7 +11,7 @@ log = logging.getLogger(__name__)  # Type aliases  RedisKeyType = Union[str, int] -RedisValueType = Union[str, int, float] +RedisValueType = Union[str, int, float, bool]  RedisKeyOrValue = Union[RedisKeyType, RedisValueType]  # Prefix tuples @@ -20,6 +20,7 @@ _VALUE_PREFIXES = (      ("f|", float),      ("i|", int),      ("s|", str), +    ("b|", bool),  )  _KEY_PREFIXES = (      ("i|", int), @@ -117,8 +118,15 @@ class RedisCache:      def _to_typestring(key_or_value: RedisKeyOrValue, prefixes: _PrefixTuple) -> str:          """Turn a valid Redis type into a typestring."""          for prefix, _type in prefixes: -            if isinstance(key_or_value, _type): +            # Convert bools into integers before storing them. +            if type(key_or_value) is bool: +                bool_int = int(key_or_value) +                return f"{prefix}{bool_int}" + +            # isinstance is a bad idea here, because isintance(False, int) == True. +            if type(key_or_value) is _type:                  return f"{prefix}{key_or_value}" +          raise TypeError(f"RedisCache._to_typestring only supports the following: {prefixes}.")      @staticmethod @@ -131,6 +139,13 @@ class RedisCache:          # Now we convert our unicode string back into the type it originally was.          for prefix, _type in prefixes:              if key_or_value.startswith(prefix): + +                # For booleans, we need special handling because bool("False") is True. +                if prefix == "b|": +                    value = key_or_value[len(prefix):] +                    return bool(int(value)) + +                # Otherwise we can just convert normally.                  return _type(key_or_value[len(prefix):])          raise TypeError(f"RedisCache._from_typestring only supports the following: {prefixes}.") diff --git a/tests/bot/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py index 8c1a40640..62c411681 100644 --- a/tests/bot/utils/test_redis_cache.py +++ b/tests/bot/utils/test_redis_cache.py @@ -59,7 +59,9 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase):          test_cases = (              ('favorite_fruit', 'melon'),              ('favorite_number', 86), -            ('favorite_fraction', 86.54) +            ('favorite_fraction', 86.54), +            ('favorite_boolean', False), +            ('other_boolean', True),          )          # Test that we can get and set different types.  |