[ModLog] Optimise get_next_case_number() (#2908)

* [ModLog] Optimise get_next_case_number()

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

* Address reviews

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

* Add changelog entry

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

* Delete get_next_case_number

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

* Add modlog.get_latest_case() and fix `[p]reason`

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>

changelog.d/2908.breaking.rst

@@ -0,0 +1 @@
+``redbot.core.modlog.get_next_case_number()`` has been removed.

changelog.d/2908.bugfix.rst

@@ -0,0 +1 @@
+Fixed error in `[p]reason` when setting the reason for a case without a moderator.

changelog.d/2908.enhance.rst

@@ -0,0 +1 @@
+ModLog is now much faster at creating cases, especially in large servers.

changelog.d/2908.feature.rst

@@ -0,0 +1 @@
+Added :func:`redbot.core.modlog.get_latest_case` to fetch the case object for the most recent ModLog case.

redbot/cogs/modlog/modlog.py

@@ -150,41 +150,28 @@ class ModLog(commands.Cog):
         guild = ctx.guild
         if case is None:
             # get the latest case
-            case = int(await modlog.get_next_case_number(guild)) - 1
+            case_obj = await modlog.get_latest_case(guild, self.bot)
-        try:
+            if case_obj is None:
-            case_before = await modlog.get_case(case, guild, self.bot)
+                await ctx.send(_("There are no modlog cases in this server."))
-        except RuntimeError:
-            await ctx.send(_("That case does not exist!"))
-            return
-        else:
-            if case_before.moderator is None:
-                # No mod set, so attempt to find out if the author
-                # triggered the case creation with an action
-                bot_perms = guild.me.guild_permissions
-                if bot_perms.view_audit_log:
-                    case_type = await modlog.get_casetype(case_before.action_type, guild)
-                    if case_type is not None and case_type.audit_type is not None:
-                        audit_type = getattr(discord.AuditLogAction, case_type.audit_type)
-                        if audit_type:
-                            audit_case = None
-                            async for entry in guild.audit_logs(action=audit_type):
-                                if (
-                                    entry.target.id == case_before.user.id
-                                    and entry.action == audit_type
-                                ):
-                                    audit_case = entry
-                                    break
-                            if audit_case:
-                                case_before.moderator = audit_case.user
-            is_guild_owner = author == guild.owner
-            is_case_author = author == case_before.moderator
-            author_is_mod = await ctx.bot.is_mod(author)
-            if not (is_guild_owner or is_case_author or author_is_mod):
-                await ctx.send(_("You are not authorized to modify that case!"))
                 return
-            to_modify = {"reason": reason}
+        else:
-            if case_before.moderator != author:
+            try:
-                to_modify["amended_by"] = author
+                case_obj = await modlog.get_case(case, guild, self.bot)
-            to_modify["modified_at"] = ctx.message.created_at.timestamp()
+            except RuntimeError:
-            await case_before.edit(to_modify)
+                await ctx.send(_("That case does not exist!"))
-            await ctx.send(_("Reason has been updated."))
+                return
+
+        is_guild_owner = author == guild.owner
+        is_case_author = author == case_obj.moderator
+        author_is_mod = await ctx.bot.is_mod(author)
+        if not (is_guild_owner or is_case_author or author_is_mod):
+            await ctx.send(_("You are not authorized to modify that case!"))
+            return
+        to_modify = {"reason": reason}
+        if case_obj.moderator != author:
+            to_modify["amended_by"] = author
+        to_modify["modified_at"] = ctx.message.created_at.timestamp()
+        await case_obj.edit(to_modify)
+        await ctx.send(
+            _("Reason for case #{num} has been updated.").format(num=case_obj.case_number)
+        )

redbot/core/modlog.py

@@ -20,7 +20,6 @@ from .generic_casetypes import all_generics
 __all__ = [
     "Case",
     "CaseType",
-    "get_next_case_number",
     "get_case",
     "get_all_cases",
     "get_cases_for_member",
@@ -39,7 +38,7 @@ _bot_ref: Optional[Red] = None
 
 _CASETYPES = "CASETYPES"
 _CASES = "CASES"
-_SCHEMA_VERSION = 3
+_SCHEMA_VERSION = 4
 
 
 _ = Translator("ModLog", __file__)
@@ -51,7 +50,7 @@ async def _init(bot: Red):
     _bot_ref = bot
     _conf = Config.get_conf(None, 1354799444, cog_name="ModLog")
     _conf.register_global(schema_version=1)
-    _conf.register_guild(mod_log=None, casetypes={})
+    _conf.register_guild(mod_log=None, casetypes={}, latest_case_number=0)
     _conf.init_custom(_CASETYPES, 1)
     _conf.init_custom(_CASES, 2)
     _conf.register_custom(_CASETYPES)
@@ -174,6 +173,16 @@ async def _migrate_config(from_version: int, to_version: int):
         await _conf.custom(_CASETYPES).set(all_casetypes)
         await _conf.schema_version.set(3)
 
+    if from_version < 4 <= to_version:
+        # set latest_case_number
+        for guild_id, cases in (await _conf.custom(_CASES).all()).items():
+            if cases:
+                await _conf.guild(
+                    cast(discord.Guild, discord.Object(id=guild_id))
+                ).latest_case_number.set(max(map(int, cases.keys())))
+
+        await _conf.schema_version.set(4)
+
 
 class Case:
     """A single mod log case"""
@@ -477,7 +486,7 @@ class CaseType:
         The emoji to use for the case type (for example, :boot:)
     case_str: str
         The string representation of the case (example: Ban)
-        
+
     """
 
     def __init__(
@@ -557,28 +566,6 @@ class CaseType:
         return cls(name=name, **data_copy, **kwargs)
 
 
-async def get_next_case_number(guild: discord.Guild) -> int:
-    """
-    Gets the next case number
-
-    Parameters
-    ----------
-    guild: `discord.Guild`
-        The guild to get the next case number for
-
-    Returns
-    -------
-    int
-        The next case number
-
-    """
-    case_numbers = (await _conf.custom(_CASES, guild.id).all()).keys()
-    if not case_numbers:
-        return 1
-    else:
-        return max(map(int, case_numbers)) + 1
-
-
 async def get_case(case_number: int, guild: discord.Guild, bot: Red) -> Case:
     """
     Gets the case with the associated case number
@@ -611,6 +598,27 @@ async def get_case(case_number: int, guild: discord.Guild, bot: Red) -> Case:
     return await Case.from_json(mod_channel, bot, case_number, case)
 
 
+async def get_latest_case(guild: discord.Guild, bot: Red) -> Optional[Case]:
+    """Get the latest case for the specified guild.
+
+    Parameters
+    ----------
+    guild : discord.Guild
+        The guild to get the latest case for.
+    bot : Red
+        The bot object.
+
+    Returns
+    -------
+    Optional[Case]
+        The latest case object. `None` if it the guild has no cases.
+
+    """
+    case_number = await _conf.guild(guild).latest_case_number()
+    if case_number:
+        return await get_case(case_number, guild, bot)
+
+
 async def get_all_cases(guild: discord.Guild, bot: Red) -> List[Case]:
     """
     Gets all cases for the specified guild
@@ -745,24 +753,29 @@ async def create_case(
     if user == bot.user:
         return
 
-    next_case_number = await get_next_case_number(guild)
+    async with _conf.guild(guild).latest_case_number.get_lock():
+        # We're getting the case number from config, incrementing it, awaiting something, then
+        # setting it again. This warrants acquiring the lock.
+        next_case_number = await _conf.guild(guild).latest_case_number() + 1
+
+        case = Case(
+            bot,
+            guild,
+            int(created_at.timestamp()),
+            action_type,
+            user,
+            moderator,
+            next_case_number,
+            reason,
+            int(until.timestamp()) if until else None,
+            channel,
+            amended_by=None,
+            modified_at=None,
+            message=None,
+        )
+        await _conf.custom(_CASES, str(guild.id), str(next_case_number)).set(case.to_json())
+        await _conf.guild(guild).latest_case_number.set(next_case_number)
 
-    case = Case(
-        bot,
-        guild,
-        int(created_at.timestamp()),
-        action_type,
-        user,
-        moderator,
-        next_case_number,
-        reason,
-        int(until.timestamp()) if until else None,
-        channel,
-        amended_by=None,
-        modified_at=None,
-        message=None,
-    )
-    await _conf.custom(_CASES, str(guild.id), str(next_case_number)).set(case.to_json())
     bot.dispatch("modlog_case_create", case)
     try:
         mod_channel = await get_modlog_channel(case.guild)
@@ -982,7 +995,7 @@ async def set_modlog_channel(
 
 async def reset_cases(guild: discord.Guild) -> None:
     """
-    Wipes all modlog cases for the specified guild
+    Wipes all modlog cases for the specified guild.
 
     Parameters
     ----------
@@ -991,6 +1004,7 @@ async def reset_cases(guild: discord.Guild) -> None:
 
     """
     await _conf.custom(_CASES, str(guild.id)).clear()
+    await _conf.guild(guild).latest_case_number.clear()
 
 
 def _strfdelta(delta):