Merge branch 'main' into fix/dmrelay

Author: Xithrius

Pipfile

@@ -9,12 +9,14 @@ aiodns = "~=2.0"
 aiohttp = "~=3.7"
 aioping = "~=0.3.1"
 aioredis = "~=1.3.1"
+arrow = "~=1.0.3"
 "async-rediscache[fakeredis]" = "~=0.1.2"
 beautifulsoup4 = "~=4.9"
 colorama = {version = "~=0.4.3",sys_platform = "== 'win32'"}
 coloredlogs = "~=14.0"
 deepdiff = "~=4.0"
 "discord.py" = "~=1.6.0"
+emoji = "~=0.6"
 feedparser = "~=5.2"
 fuzzywuzzy = "~=0.17"
 lxml = "~=4.4"
@@ -26,11 +28,10 @@ requests = "~=2.22"
 sentry-sdk = "~=0.19"
 sphinx = "~=2.2"
 statsd = "~=3.3"
-arrow = "~=0.17"
-emoji = "~=0.6"
 
 [dev-packages]
 coverage = "~=5.0"
+coveralls = "~=2.1"
 flake8 = "~=3.8"
 flake8-annotations = "~=2.0"
 flake8-bugbear = "~=20.1"
@@ -41,7 +42,6 @@ flake8-tidy-imports = "~=4.0"
 flake8-todo = "~=0.7"
 pep8-naming = "~=0.9"
 pre-commit = "~=2.1"
-coveralls = "~=2.1"
 
 [requires]
 python_version = "3.8"

Pipfile.lock

@@ -590,7 +590,8 @@ class HelpChannels(metaclass=YAMLGetter):
     enable: bool
     claim_minutes: int
     cmd_whitelist: List[int]
-    idle_minutes: int
+    idle_minutes_claimant: int
+    idle_minutes_others: int
     deleted_idle_minutes: int
     max_available: int
     max_total_channels: int

bot/constants.py

@@ -8,12 +8,15 @@
 # RedisCache[discord.TextChannel.id, t.Union[discord.User.id, discord.Member.id]]
 claimants = RedisCache(namespace="HelpChannels.help_channel_claimants")
 
+# Stores the timestamp of the last message from the claimant of a help channel
+# RedisCache[discord.TextChannel.id, UtcPosixTimestamp]
+claimant_last_message_times = RedisCache(namespace="HelpChannels.claimant_last_message_times")
+
+# This cache maps a help channel to the timestamp of the last non-claimant message.
+# This cache being empty for a given help channel indicates the question is unanswered.
+# RedisCache[discord.TextChannel.id, UtcPosixTimestamp]
+non_claimant_last_message_times = RedisCache(namespace="HelpChannels.non_claimant_last_message_times")
+
 # This cache maps a help channel to original question message in same channel.
 # RedisCache[discord.TextChannel.id, discord.Message.id]
 question_messages = RedisCache(namespace="HelpChannels.question_messages")
-
-# 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(namespace="HelpChannels.unanswered")

bot/exts/help_channels/_caches.py

@@ -1,8 +1,11 @@
 import logging
 import typing as t
-from datetime import datetime, timedelta
+from datetime import timedelta
+from enum import Enum
 
+import arrow
 import discord
+from arrow import Arrow
 
 import bot
 from bot import constants
@@ -15,6 +18,17 @@
 EXCLUDED_CHANNELS = (constants.Channels.cooldown,)
 
 
+class ClosingReason(Enum):
+    """All possible closing reasons for help channels."""
+
+    COMMAND = "command"
+    LATEST_MESSSAGE = "auto.latest_message"
+    CLAIMANT_TIMEOUT = "auto.claimant_timeout"
+    OTHER_TIMEOUT = "auto.other_timeout"
+    DELETED = "auto.deleted"
+    CLEANUP = "auto.cleanup"
+
+
 def get_category_channels(category: discord.CategoryChannel) -> t.Iterable[discord.TextChannel]:
     """Yield the text channels of the `category` in an unsorted manner."""
     log.trace(f"Getting text channels in the category '{category}' ({category.id}).")
@@ -25,23 +39,68 @@ def get_category_channels(category: discord.CategoryChannel) -> t.Iterable[disco
             yield channel
 
 
-async def get_idle_time(channel: discord.TextChannel) -> t.Optional[int]:
+async def get_closing_time(channel: discord.TextChannel, init_done: bool) -> t.Tuple[Arrow, ClosingReason]:
     """
-    Return the time elapsed, in seconds, since the last message sent in the `channel`.
+    Return the time at which the given help `channel` should be closed along with the reason.
 
-    Return None if the channel has no messages.
+    `init_done` is True if the cog has finished loading and False otherwise.
+
+    The time is calculated as follows:
+
+    * If `init_done` is True or the cached time for the claimant's last message is unavailable,
+      add the configured `idle_minutes_claimant` to the time the most recent message was sent.
+    * If the help session is empty (see `is_empty`), do the above but with `deleted_idle_minutes`.
+    * If either of the above is attempted but the channel is completely empty, close the channel
+      immediately.
+    * Otherwise, retrieve the times of the claimant's and non-claimant's last messages from the
+      cache. Add the configured `idle_minutes_claimant` and idle_minutes_others`, respectively, and
+      choose the time which is furthest in the future.
     """
-    log.trace(f"Getting the idle time for #{channel} ({channel.id}).")
+    log.trace(f"Getting the closing time for #{channel} ({channel.id}).")
+
+    is_empty = await _message.is_empty(channel)
+    if is_empty:
+        idle_minutes_claimant = constants.HelpChannels.deleted_idle_minutes
+    else:
+        idle_minutes_claimant = constants.HelpChannels.idle_minutes_claimant
+
+    claimant_time = await _caches.claimant_last_message_times.get(channel.id)
+
+    # The current session lacks messages, the cog is still starting, or the cache is empty.
+    if is_empty or not init_done or claimant_time is None:
+        msg = await _message.get_last_message(channel)
+        if not msg:
+            log.debug(f"No idle time available; #{channel} ({channel.id}) has no messages, closing now.")
+            return Arrow.min, ClosingReason.DELETED
+
+        # Use the greatest offset to avoid the possibility of prematurely closing the channel.
+        time = Arrow.fromdatetime(msg.created_at) + timedelta(minutes=idle_minutes_claimant)
+        return time, ClosingReason.LATEST_MESSSAGE
+
+    claimant_time = Arrow.utcfromtimestamp(claimant_time)
+    others_time = await _caches.non_claimant_last_message_times.get(channel.id)
+
+    if others_time:
+        others_time = Arrow.utcfromtimestamp(others_time)
+    else:
+        # The help session hasn't received any answers (messages from non-claimants) yet.
+        # Set to min value so it isn't considered when calculating the closing time.
+        others_time = Arrow.min
 
-    msg = await _message.get_last_message(channel)
-    if not msg:
-        log.debug(f"No idle time available; #{channel} ({channel.id}) has no messages.")
-        return None
+    # Offset the cached times by the configured values.
+    others_time += timedelta(minutes=constants.HelpChannels.idle_minutes_others)
+    claimant_time += timedelta(minutes=idle_minutes_claimant)
 
-    idle_time = (datetime.utcnow() - msg.created_at).seconds
+    # Use the time which is the furthest into the future.
+    if claimant_time >= others_time:
+        closing_time = claimant_time
+        reason = ClosingReason.CLAIMANT_TIMEOUT
+    else:
+        closing_time = others_time
+        reason = ClosingReason.OTHER_TIMEOUT
 
-    log.trace(f"#{channel} ({channel.id}) has been idle for {idle_time} seconds.")
-    return idle_time
+    log.trace(f"#{channel} ({channel.id}) should be closed at {closing_time} due to {reason}.")
+    return closing_time, reason
 
 
 async def get_in_use_time(channel_id: int) -> t.Optional[timedelta]:
@@ -50,8 +109,8 @@ async def get_in_use_time(channel_id: int) -> t.Optional[timedelta]:
 
     claimed_timestamp = await _caches.claim_times.get(channel_id)
     if claimed_timestamp:
-        claimed = datetime.utcfromtimestamp(claimed_timestamp)
-        return datetime.utcnow() - claimed
+        claimed = Arrow.utcfromtimestamp(claimed_timestamp)
+        return arrow.utcnow() - claimed
 
 
 def is_excluded_channel(channel: discord.abc.GuildChannel) -> bool:

bot/exts/help_channels/_channel.py

@@ -2,9 +2,10 @@
 import logging
 import random
 import typing as t
-from datetime import datetime, timezone
+from datetime import timedelta
 from operator import attrgetter
 
+import arrow
 import discord
 import discord.abc
 from discord.ext import commands
@@ -43,7 +44,9 @@ class HelpChannels(commands.Cog):
     In Use Category
 
     * Contains all channels which are occupied by someone needing help
-    * Channel moves to dormant category after `constants.HelpChannels.idle_minutes` of being idle
+    * Channel moves to dormant category after
+        - `constants.HelpChannels.idle_minutes_other` minutes since the last user message, or
+        - `constants.HelpChannels.idle_minutes_claimant` minutes since the last claimant message.
     * Command can prematurely mark a channel as dormant
         * Channel claimant is allowed to use the command
         * Allowed roles for the command are configurable with `constants.HelpChannels.cmd_whitelist`
@@ -70,7 +73,7 @@ def __init__(self, bot: Bot):
         self.channel_queue: asyncio.Queue[discord.TextChannel] = None
         self.name_queue: t.Deque[str] = None
 
-        self.last_notification: t.Optional[datetime] = None
+        self.last_notification: t.Optional[arrow.Arrow] = None
 
         # Asyncio stuff
         self.queue_tasks: t.List[asyncio.Task] = []
@@ -112,11 +115,13 @@ async def claim_channel(self, message: discord.Message) -> None:
 
         self.bot.stats.incr("help.claimed")
 
-        # Must use a timezone-aware datetime to ensure a correct POSIX timestamp.
-        timestamp = datetime.now(timezone.utc).timestamp()
-        await _caches.claim_times.set(message.channel.id, timestamp)
+        # datetime.timestamp() would assume it's local, despite d.py giving a (naïve) UTC time.
+        timestamp = arrow.Arrow.fromdatetime(message.created_at).timestamp()
 
-        await _caches.unanswered.set(message.channel.id, True)
+        await _caches.claim_times.set(message.channel.id, timestamp)
+        await _caches.claimant_last_message_times.set(message.channel.id, timestamp)
+        # Delete to indicate that the help session has yet to receive an answer.
+        await _caches.non_claimant_last_message_times.delete(message.channel.id)
 
         # Not awaited because it may indefinitely hold the lock while waiting for a channel.
         scheduling.create_task(self.move_to_available(), name=f"help_claim_{message.id}")
@@ -187,7 +192,7 @@ async def close_command(self, ctx: commands.Context) -> None:
         # Don't use a discord.py check because the check needs to fail silently.
         if await self.close_check(ctx):
             log.info(f"Close command invoked by {ctx.author} in #{ctx.channel}.")
-            await self.unclaim_channel(ctx.channel, is_auto=False)
+            await self.unclaim_channel(ctx.channel, closed_on=_channel.ClosingReason.COMMAND)
 
     async def get_available_candidate(self) -> discord.TextChannel:
         """
@@ -233,7 +238,7 @@ async def init_available(self) -> None:
         elif missing < 0:
             log.trace(f"Moving {abs(missing)} superfluous available channels over to the Dormant category.")
             for channel in channels[:abs(missing)]:
-                await self.unclaim_channel(channel)
+                await self.unclaim_channel(channel, closed_on=_channel.ClosingReason.CLEANUP)
 
     async def init_categories(self) -> None:
         """Get the help category objects. Remove the cog if retrieval fails."""
@@ -293,26 +298,23 @@ async def move_idle_channel(self, channel: discord.TextChannel, has_task: bool =
         """
         log.trace(f"Handling in-use channel #{channel} ({channel.id}).")
 
-        if not await _message.is_empty(channel):
-            idle_seconds = constants.HelpChannels.idle_minutes * 60
-        else:
-            idle_seconds = constants.HelpChannels.deleted_idle_minutes * 60
-
-        time_elapsed = await _channel.get_idle_time(channel)
+        closing_time, closed_on = await _channel.get_closing_time(channel, self.init_task.done())
 
-        if time_elapsed is None or time_elapsed >= idle_seconds:
+        # Closing time is in the past.
+        # Add 1 second due to POSIX timestamps being lower resolution than datetime objects.
+        if closing_time < (arrow.utcnow() + timedelta(seconds=1)):
             log.info(
-                f"#{channel} ({channel.id}) is idle longer than {idle_seconds} seconds "
-                f"and will be made dormant."
+                f"#{channel} ({channel.id}) is idle past {closing_time} "
+                f"and will be made dormant. Reason: {closed_on.value}"
             )
 
-            await self.unclaim_channel(channel)
+            await self.unclaim_channel(channel, closed_on=closed_on)
         else:
             # Cancel the existing task, if any.
             if has_task:
                 self.scheduler.cancel(channel.id)
 
-            delay = idle_seconds - time_elapsed
+            delay = (closing_time - arrow.utcnow()).seconds
             log.info(
                 f"#{channel} ({channel.id}) is still active; "
                 f"scheduling it to be moved after {delay} seconds."
@@ -356,15 +358,15 @@ async def move_to_dormant(self, channel: discord.TextChannel) -> None:
         _stats.report_counts()
 
     @lock.lock_arg(f"{NAMESPACE}.unclaim", "channel")
-    async def unclaim_channel(self, channel: discord.TextChannel, *, is_auto: bool = True) -> None:
+    async def unclaim_channel(self, channel: discord.TextChannel, *, closed_on: _channel.ClosingReason) -> None:
         """
         Unclaim an in-use help `channel` to make it dormant.
 
         Unpin the claimant's question message and move the channel to the Dormant category.
         Remove the cooldown role from the channel claimant if they have no other channels claimed.
         Cancel the scheduled cooldown role removal task.
 
-        Set `is_auto` to True if the channel was automatically closed or False if manually closed.
+        `closed_on` is the reason that the channel was closed. See _channel.ClosingReason for possible values.
         """
         claimant_id = await _caches.claimants.get(channel.id)
         _unclaim_channel = self._unclaim_channel
@@ -375,9 +377,14 @@ async def unclaim_channel(self, channel: discord.TextChannel, *, is_auto: bool =
             decorator = lock.lock_arg(f"{NAMESPACE}.unclaim", "claimant_id", wait=True)
             _unclaim_channel = decorator(_unclaim_channel)
 
-        return await _unclaim_channel(channel, claimant_id, is_auto)
+        return await _unclaim_channel(channel, claimant_id, closed_on)
 
-    async def _unclaim_channel(self, channel: discord.TextChannel, claimant_id: int, is_auto: bool) -> None:
+    async def _unclaim_channel(
+        self,
+        channel: discord.TextChannel,
+        claimant_id: int,
+        closed_on: _channel.ClosingReason
+    ) -> None:
         """Actual implementation of `unclaim_channel`. See that for full documentation."""
         await _caches.claimants.delete(channel.id)
 
@@ -393,12 +400,12 @@ async def _unclaim_channel(self, channel: discord.TextChannel, claimant_id: int,
             await _cooldown.remove_cooldown_role(claimant)
 
         await _message.unpin(channel)
-        await _stats.report_complete_session(channel.id, is_auto)
+        await _stats.report_complete_session(channel.id, closed_on)
         await self.move_to_dormant(channel)
 
         # Cancel the task that makes the channel dormant only if called by the close command.
         # In other cases, the task is either already done or not-existent.
-        if not is_auto:
+        if closed_on == _channel.ClosingReason.COMMAND:
             self.scheduler.cancel(channel.id)
 
     async def move_to_in_use(self, channel: discord.TextChannel) -> None:
@@ -410,7 +417,7 @@ async def move_to_in_use(self, channel: discord.TextChannel) -> None:
             category_id=constants.Categories.help_in_use,
         )
 
-        timeout = constants.HelpChannels.idle_minutes * 60
+        timeout = constants.HelpChannels.idle_minutes_claimant * 60
 
         log.trace(f"Scheduling #{channel} ({channel.id}) to become dormant in {timeout} sec.")
         self.scheduler.schedule_later(timeout, channel.id, self.move_idle_channel(channel))
@@ -428,7 +435,7 @@ async def on_message(self, message: discord.Message) -> None:
             if not _channel.is_excluded_channel(message.channel):
                 await self.claim_channel(message)
         else:
-            await _message.check_for_answer(message)
+            await _message.update_message_caches(message)
 
     @commands.Cog.listener()
     async def on_message_delete(self, msg: discord.Message) -> None: