From d30e83b5fc8b311ec7ac87625ef4389955e3f5d9 Mon Sep 17 00:00:00 2001 From: Draper <27962761+Drapersniper@users.noreply.github.com> Date: Wed, 8 Jul 2020 18:25:14 +0100 Subject: [PATCH] Reduce config calls when changing white/blacklist and use sets for their cache (#3910) * optimise use of config ctx manager to reduce calls to config * fine you potato * since jack said yes i'll abuse it * Apply suggestions from code review Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> * difference_update and update * Apply suggestions from code review Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> * Update redbot/core/settings_caches.py Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> * one last tweak Co-authored-by: jack1142 <6032823+jack1142@users.noreply.github.com> --- redbot/core/core_commands.py | 20 ++--- redbot/core/settings_caches.py | 148 ++++++++++++++------------------- 2 files changed, 73 insertions(+), 95 deletions(-) diff --git a/redbot/core/core_commands.py b/redbot/core/core_commands.py index 4d16ca4a7..a53b22314 100644 --- a/redbot/core/core_commands.py +++ b/redbot/core/core_commands.py @@ -1877,7 +1877,7 @@ class Core(commands.Cog, CoreLogic): await ctx.send_help() return - uids = [getattr(user, "id", user) for user in users] + uids = {getattr(user, "id", user) for user in users} await self.bot._whiteblacklist_cache.add_to_whitelist(None, uids) await ctx.send(_("Users added to whitelist.")) @@ -1909,7 +1909,7 @@ class Core(commands.Cog, CoreLogic): await ctx.send_help() return - uids = [getattr(user, "id", user) for user in users] + uids = {getattr(user, "id", user) for user in users} await self.bot._whiteblacklist_cache.remove_from_whitelist(None, uids) await ctx.send(_("Users have been removed from whitelist.")) @@ -1948,7 +1948,7 @@ class Core(commands.Cog, CoreLogic): await ctx.send(_("You cannot blacklist an owner!")) return - uids = [getattr(user, "id", user) for user in users] + uids = {getattr(user, "id", user) for user in users} await self.bot._whiteblacklist_cache.add_to_blacklist(None, uids) await ctx.send(_("User added to blacklist.")) @@ -1980,7 +1980,7 @@ class Core(commands.Cog, CoreLogic): await ctx.send_help() return - uids = [getattr(user, "id", user) for user in users] + uids = {getattr(user, "id", user) for user in users} await self.bot._whiteblacklist_cache.remove_from_blacklist(None, uids) await ctx.send(_("Users have been removed from blacklist.")) @@ -2016,7 +2016,7 @@ class Core(commands.Cog, CoreLogic): names = [getattr(u_or_r, "name", u_or_r) for u_or_r in users_or_roles] uids = {getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles} if not (ctx.guild.owner == ctx.author or await self.bot.is_owner(ctx.author)): - current_whitelist = set(await self.bot._whiteblacklist_cache.get_whitelist(ctx.guild)) + current_whitelist = await self.bot._whiteblacklist_cache.get_whitelist(ctx.guild) theoretical_whitelist = current_whitelist.union(uids) ids = {i for i in (ctx.author.id, *(getattr(ctx.author, "_roles", [])))} if ids.isdisjoint(theoretical_whitelist): @@ -2027,7 +2027,7 @@ class Core(commands.Cog, CoreLogic): "please ensure to add yourself to the whitelist first." ) ) - await self.bot._whiteblacklist_cache.add_to_whitelist(ctx.guild, list(uids)) + await self.bot._whiteblacklist_cache.add_to_whitelist(ctx.guild, uids) await ctx.send(_("{names} added to whitelist.").format(names=humanize_list(names))) @@ -2063,7 +2063,7 @@ class Core(commands.Cog, CoreLogic): names = [getattr(u_or_r, "name", u_or_r) for u_or_r in users_or_roles] uids = {getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles} if not (ctx.guild.owner == ctx.author or await self.bot.is_owner(ctx.author)): - current_whitelist = set(await self.bot._whiteblacklist_cache.get_whitelist(ctx.guild)) + current_whitelist = await self.bot._whiteblacklist_cache.get_whitelist(ctx.guild) theoretical_whitelist = current_whitelist - uids ids = {i for i in (ctx.author.id, *(getattr(ctx.author, "_roles", [])))} if theoretical_whitelist and ids.isdisjoint(theoretical_whitelist): @@ -2073,7 +2073,7 @@ class Core(commands.Cog, CoreLogic): "remove your ability to run commands." ) ) - await self.bot._whiteblacklist_cache.remove_from_whitelist(ctx.guild, list(uids)) + await self.bot._whiteblacklist_cache.remove_from_whitelist(ctx.guild, uids) await ctx.send( _("{names} removed from the local whitelist.").format(names=humanize_list(names)) @@ -2119,7 +2119,7 @@ class Core(commands.Cog, CoreLogic): await ctx.send(_("You cannot blacklist a bot owner!")) return names = [getattr(u_or_r, "name", u_or_r) for u_or_r in users_or_roles] - uids = [getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles] + uids = {getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles} await self.bot._whiteblacklist_cache.add_to_blacklist(ctx.guild, uids) await ctx.send( @@ -2156,7 +2156,7 @@ class Core(commands.Cog, CoreLogic): return names = [getattr(u_or_r, "name", u_or_r) for u_or_r in users_or_roles] - uids = [getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles] + uids = {getattr(u_or_r, "id", u_or_r) for u_or_r in users_or_roles} await self.bot._whiteblacklist_cache.remove_from_blacklist(ctx.guild, uids) await ctx.send( diff --git a/redbot/core/settings_caches.py b/redbot/core/settings_caches.py index 51a60839d..67e577e91 100644 --- a/redbot/core/settings_caches.py +++ b/redbot/core/settings_caches.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Dict, List, Optional, Union +from typing import Dict, List, Optional, Union, Set, Iterable from argparse import Namespace import discord @@ -122,11 +122,11 @@ class IgnoreManager: class WhitelistBlacklistManager: def __init__(self, config: Config): self._config: Config = config - self._cached_whitelist: Dict[Optional[int], List[int]] = {} - self._cached_blacklist: Dict[Optional[int], List[int]] = {} + self._cached_whitelist: Dict[Optional[int], Set[int]] = {} + self._cached_blacklist: Dict[Optional[int], Set[int]] = {} - async def get_whitelist(self, guild: Optional[discord.Guild] = None) -> List[int]: - ret: List[int] + async def get_whitelist(self, guild: Optional[discord.Guild] = None) -> Set[int]: + ret: Set[int] gid: Optional[int] = guild.id if guild else None @@ -134,76 +134,66 @@ class WhitelistBlacklistManager: ret = self._cached_whitelist[gid].copy() else: if gid is not None: - ret = await self._config.guild_from_id(gid).whitelist() - if not ret: - ret = [] + ret = set(await self._config.guild_from_id(gid).whitelist()) else: - ret = await self._config.whitelist() + ret = set(await self._config.whitelist()) self._cached_whitelist[gid] = ret.copy() return ret - async def add_to_whitelist(self, guild: Optional[discord.Guild], role_or_user: List[int]): + async def add_to_whitelist(self, guild: Optional[discord.Guild], role_or_user: Iterable[int]): gid: Optional[int] = guild.id if guild else None role_or_user = role_or_user or [] - if not isinstance(role_or_user, list) or not all( - isinstance(r_or_u, int) for r_or_u in role_or_user - ): - raise TypeError("Whitelisted objects must be a list of ints") + if not all(isinstance(r_or_u, int) for r_or_u in role_or_user): + raise TypeError("`role_or_user` must be an iterable of `int`s.") if gid is None: if gid not in self._cached_whitelist: - self._cached_whitelist[gid] = await self._config.whitelist() - for obj_id in role_or_user: - if obj_id not in self._cached_whitelist[gid]: - self._cached_whitelist[gid].append(obj_id) - async with self._config.whitelist() as curr_list: - curr_list.append(obj_id) + self._cached_whitelist[gid] = set(await self._config.whitelist()) + self._cached_whitelist[gid].update(role_or_user) + await self._config.whitelist.set(list(self._cached_whitelist[gid])) + else: if gid not in self._cached_whitelist: - self._cached_whitelist[gid] = await self._config.guild_from_id(gid).whitelist() - for obj_id in role_or_user: - if obj_id not in self._cached_whitelist[gid]: - self._cached_whitelist[gid].append(obj_id) - async with self._config.guild_from_id(gid).whitelist() as curr_list: - curr_list.append(obj_id) + self._cached_whitelist[gid] = set( + await self._config.guild_from_id(gid).whitelist() + ) + self._cached_whitelist[gid].update(role_or_user) + await self._config.guild_from_id(gid).whitelist.set(list(self._cached_whitelist[gid])) async def clear_whitelist(self, guild: Optional[discord.Guild] = None): gid: Optional[int] = guild.id if guild else None - self._cached_whitelist[gid] = [] + self._cached_whitelist[gid] = set() if gid is None: await self._config.whitelist.clear() else: await self._config.guild_from_id(gid).whitelist.clear() - async def remove_from_whitelist(self, guild: Optional[discord.Guild], role_or_user: List[int]): + async def remove_from_whitelist( + self, guild: Optional[discord.Guild], role_or_user: Iterable[int] + ): gid: Optional[int] = guild.id if guild else None role_or_user = role_or_user or [] - if not isinstance(role_or_user, list) or not all( - isinstance(r_or_u, int) for r_or_u in role_or_user - ): - raise TypeError("Whitelisted objects must be a list of ints") + if not all(isinstance(r_or_u, int) for r_or_u in role_or_user): + raise TypeError("`role_or_user` must be an iterable of `int`s.") if gid is None: if gid not in self._cached_whitelist: - self._cached_whitelist[gid] = await self._config.whitelist() - for obj_id in role_or_user: - if obj_id in self._cached_whitelist[gid]: - self._cached_whitelist[gid].remove(obj_id) - async with self._config.whitelist() as curr_list: - curr_list.remove(obj_id) + self._cached_whitelist[gid] = set(await self._config.whitelist()) + self._cached_whitelist[gid].difference_update(role_or_user) + await self._config.whitelist.set(list(self._cached_whitelist[gid])) + else: if gid not in self._cached_whitelist: - self._cached_whitelist[gid] = await self._config.guild_from_id(gid).whitelist() - for obj_id in role_or_user: - if obj_id in self._cached_whitelist[gid]: - self._cached_whitelist[gid].remove(obj_id) - async with self._config.guild_from_id(gid).whitelist() as curr_list: - curr_list.remove(obj_id) + self._cached_whitelist[gid] = set( + await self._config.guild_from_id(gid).whitelist() + ) + self._cached_whitelist[gid].difference_update(role_or_user) + await self._config.guild_from_id(gid).whitelist.set(list(self._cached_whitelist[gid])) - async def get_blacklist(self, guild: Optional[discord.Guild] = None) -> List[int]: - ret: List[int] + async def get_blacklist(self, guild: Optional[discord.Guild] = None) -> Set[int]: + ret: Set[int] gid: Optional[int] = guild.id if guild else None @@ -211,68 +201,56 @@ class WhitelistBlacklistManager: ret = self._cached_blacklist[gid].copy() else: if gid is not None: - ret = await self._config.guild_from_id(gid).blacklist() - if not ret: - ret = [] + ret = set(await self._config.guild_from_id(gid).blacklist()) else: - ret = await self._config.blacklist() + ret = set(await self._config.blacklist()) self._cached_blacklist[gid] = ret.copy() return ret - async def add_to_blacklist(self, guild: Optional[discord.Guild], role_or_user: List[int]): + async def add_to_blacklist(self, guild: Optional[discord.Guild], role_or_user: Iterable[int]): gid: Optional[int] = guild.id if guild else None role_or_user = role_or_user or [] - if not isinstance(role_or_user, list) or not all( - isinstance(r_or_u, int) for r_or_u in role_or_user - ): - raise TypeError("Blacklisted objects must be a list of ints") + if not all(isinstance(r_or_u, int) for r_or_u in role_or_user): + raise TypeError("`role_or_user` must be an iterable of `int`s.") if gid is None: if gid not in self._cached_blacklist: - self._cached_blacklist[gid] = await self._config.blacklist() - for obj_id in role_or_user: - if obj_id not in self._cached_blacklist[gid]: - self._cached_blacklist[gid].append(obj_id) - async with self._config.blacklist() as curr_list: - curr_list.append(obj_id) + self._cached_blacklist[gid] = set(await self._config.blacklist()) + self._cached_blacklist[gid].update(role_or_user) + await self._config.blacklist.set(list(self._cached_blacklist[gid])) else: if gid not in self._cached_blacklist: - self._cached_blacklist[gid] = await self._config.guild_from_id(gid).blacklist() - for obj_id in role_or_user: - if obj_id not in self._cached_blacklist[gid]: - self._cached_blacklist[gid].append(obj_id) - async with self._config.guild_from_id(gid).blacklist() as curr_list: - curr_list.append(obj_id) + self._cached_blacklist[gid] = set( + await self._config.guild_from_id(gid).blacklist() + ) + self._cached_blacklist[gid].update(role_or_user) + await self._config.guild_from_id(gid).blacklist.set(list(self._cached_blacklist[gid])) async def clear_blacklist(self, guild: Optional[discord.Guild] = None): gid: Optional[int] = guild.id if guild else None - self._cached_blacklist[gid] = [] + self._cached_blacklist[gid] = set() if gid is None: await self._config.blacklist.clear() else: await self._config.guild_from_id(gid).blacklist.clear() - async def remove_from_blacklist(self, guild: Optional[discord.Guild], role_or_user: List[int]): + async def remove_from_blacklist( + self, guild: Optional[discord.Guild], role_or_user: Iterable[int] + ): gid: Optional[int] = guild.id if guild else None role_or_user = role_or_user or [] - if not isinstance(role_or_user, list) or not all( - isinstance(r_or_u, int) for r_or_u in role_or_user - ): - raise TypeError("Blacklisted objects must be a list of ints") + if not all(isinstance(r_or_u, int) for r_or_u in role_or_user): + raise TypeError("`role_or_user` must be an iterable of `int`s.") if gid is None: if gid not in self._cached_blacklist: - self._cached_blacklist[gid] = await self._config.blacklist() - for obj_id in role_or_user: - if obj_id in self._cached_blacklist[gid]: - self._cached_blacklist[gid].remove(obj_id) - async with self._config.blacklist() as curr_list: - curr_list.remove(obj_id) + self._cached_blacklist[gid] = set(await self._config.blacklist()) + self._cached_blacklist[gid].difference_update(role_or_user) + await self._config.blacklist.set(list(self._cached_blacklist[gid])) else: if gid not in self._cached_blacklist: - self._cached_blacklist[gid] = await self._config.guild_from_id(gid).blacklist() - for obj_id in role_or_user: - if obj_id in self._cached_blacklist[gid]: - self._cached_blacklist[gid].remove(obj_id) - async with self._config.guild_from_id(gid).blacklist() as curr_list: - curr_list.remove(obj_id) + self._cached_blacklist[gid] = set( + await self._config.guild_from_id(gid).blacklist() + ) + self._cached_blacklist[gid].difference_update(role_or_user) + await self._config.guild_from_id(gid).blacklist.set(list(self._cached_blacklist[gid]))