From 753e101df0ba4f51c85a426a7a9d9678f96e0fd7 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 14 Mar 2022 19:01:56 +0000 Subject: Revert "Update all references of discord.py to disnake" This reverts commit 960619c23300c56c8aaa454edc7241e2badf80ad. --- tests/helpers.py | 124 +++++++++++++++++++++++++++---------------------------- 1 file changed, 62 insertions(+), 62 deletions(-) (limited to 'tests/helpers.py') diff --git a/tests/helpers.py b/tests/helpers.py index bd1418ab9..9d4988d23 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -7,9 +7,9 @@ import unittest.mock from asyncio import AbstractEventLoop from typing import Iterable, Optional -import disnake +import discord from aiohttp import ClientSession -from disnake.ext.commands import Context +from discord.ext.commands import Context from bot.api import APIClient from bot.async_stats import AsyncStatsClient @@ -26,11 +26,11 @@ for logger in logging.Logger.manager.loggerDict.values(): logger.setLevel(logging.CRITICAL) -class HashableMixin(disnake.mixins.EqualityComparable): +class HashableMixin(discord.mixins.EqualityComparable): """ - Mixin that provides similar hashing and equality functionality as disnake's `Hashable` mixin. + Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. - Note: disnake`s `Hashable` mixin bit-shifts `self.id` (`>> 22`); to prevent hash-collisions + Note: discord.py`s `Hashable` mixin bit-shifts `self.id` (`>> 22`); to prevent hash-collisions for the relative small `id` integers we generally use in tests, this bit-shift is omitted. """ @@ -39,22 +39,22 @@ class HashableMixin(disnake.mixins.EqualityComparable): class ColourMixin: - """A mixin for Mocks that provides the aliasing of (accent_)color->(accent_)colour like disnake does.""" + """A mixin for Mocks that provides the aliasing of (accent_)color->(accent_)colour like discord.py does.""" @property - def color(self) -> disnake.Colour: + def color(self) -> discord.Colour: return self.colour @color.setter - def color(self, color: disnake.Colour) -> None: + def color(self, color: discord.Colour) -> None: self.colour = color @property - def accent_color(self) -> disnake.Colour: + def accent_color(self) -> discord.Colour: return self.accent_colour @accent_color.setter - def accent_color(self, color: disnake.Colour) -> None: + def accent_color(self, color: discord.Colour) -> None: self.accent_colour = color @@ -63,7 +63,7 @@ class CustomMockMixin: Provides common functionality for our custom Mock types. The `_get_child_mock` method automatically returns an AsyncMock for coroutine methods of the mock - object. As disnake also uses synchronous methods that nonetheless return coroutine objects, the + object. As discord.py also uses synchronous methods that nonetheless return coroutine objects, the class attribute `additional_spec_asyncs` can be overwritten with an iterable containing additional attribute names that should also mocked with an AsyncMock instead of a regular MagicMock/Mock. The class method `spec_set` can be overwritten with the object that should be uses as the specification @@ -119,7 +119,7 @@ class CustomMockMixin: return klass(**kw) -# Create a guild instance to get a realistic Mock of `disnake.Guild` +# Create a guild instance to get a realistic Mock of `discord.Guild` guild_data = { 'id': 1, 'name': 'guild', @@ -139,20 +139,20 @@ guild_data = { 'owner_id': 1, 'afk_channel_id': 464033278631084042, } -guild_instance = disnake.Guild(data=guild_data, state=unittest.mock.MagicMock()) +guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ - A `Mock` subclass to mock `disnake.Guild` objects. + A `Mock` subclass to mock `discord.Guild` objects. - A MockGuild instance will follow the specifications of a `disnake.Guild` instance. This means + A MockGuild instance will follow the specifications of a `discord.Guild` instance. This means that if the code you're testing tries to access an attribute or method that normally does not - exist for a `disnake.Guild` object this will raise an `AttributeError`. This is to make sure our - tests fail if the code we're testing uses a `disnake.Guild` object in the wrong way. + exist for a `discord.Guild` object this will raise an `AttributeError`. This is to make sure our + tests fail if the code we're testing uses a `discord.Guild` object in the wrong way. One restriction of that is that if the code tries to access an attribute that normally does not - exist for `disnake.Guild` instance but was added dynamically, this will raise an exception with + exist for `discord.Guild` instance but was added dynamically, this will raise an exception with the mocked object. To get around that, you can set the non-standard attribute explicitly for the instance of `MockGuild`: @@ -160,10 +160,10 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): >>> guild.attribute_that_normally_does_not_exist = unittest.mock.MagicMock() In addition to attribute simulation, mocked guild object will pass an `isinstance` check against - `disnake.Guild`: + `discord.Guild`: >>> guild = MockGuild() - >>> isinstance(guild, disnake.Guild) + >>> isinstance(guild, discord.Guild) True For more info, see the `Mocking` section in `tests/README.md`. @@ -179,16 +179,16 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): self.roles.extend(roles) -# Create a Role instance to get a realistic Mock of `disnake.Role` +# Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} -role_instance = disnake.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) +role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ - A Mock subclass to mock `disnake.Role` objects. + A Mock subclass to mock `discord.Role` objects. - Instances of this class will follow the specifications of `disnake.Role` instances. For more + Instances of this class will follow the specifications of `discord.Role` instances. For more information, see the `MockGuild` docstring. """ spec_set = role_instance @@ -198,40 +198,40 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): 'id': next(self.discord_id), 'name': 'role', 'position': 1, - 'colour': disnake.Colour(0xdeadbf), - 'permissions': disnake.Permissions(), + 'colour': discord.Colour(0xdeadbf), + 'permissions': discord.Permissions(), } super().__init__(**collections.ChainMap(kwargs, default_kwargs)) if isinstance(self.colour, int): - self.colour = disnake.Colour(self.colour) + self.colour = discord.Colour(self.colour) if isinstance(self.permissions, int): - self.permissions = disnake.Permissions(self.permissions) + self.permissions = discord.Permissions(self.permissions) if 'mention' not in kwargs: self.mention = f'&{self.name}' def __lt__(self, other): - """Simplified position-based comparisons similar to those of `disnake.Role`.""" + """Simplified position-based comparisons similar to those of `discord.Role`.""" return self.position < other.position def __ge__(self, other): - """Simplified position-based comparisons similar to those of `disnake.Role`.""" + """Simplified position-based comparisons similar to those of `discord.Role`.""" return self.position >= other.position -# Create a Member instance to get a realistic Mock of `disnake.Member` +# Create a Member instance to get a realistic Mock of `discord.Member` member_data = {'user': 'lemon', 'roles': [1]} state_mock = unittest.mock.MagicMock() -member_instance = disnake.Member(data=member_data, guild=guild_instance, state=state_mock) +member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. - Instances of this class will follow the specifications of `disnake.Member` instances. For more + Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ spec_set = member_instance @@ -249,11 +249,11 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin self.mention = f"@{self.name}" -# Create a User instance to get a realistic Mock of `disnake.User` +# Create a User instance to get a realistic Mock of `discord.User` _user_data_mock = collections.defaultdict(unittest.mock.MagicMock, { "accent_color": 0 }) -user_instance = disnake.User( +user_instance = discord.User( data=unittest.mock.MagicMock(get=unittest.mock.Mock(side_effect=_user_data_mock.get)), state=unittest.mock.MagicMock() ) @@ -263,7 +263,7 @@ class MockUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock User objects. - Instances of this class will follow the specifications of `disnake.User` instances. For more + Instances of this class will follow the specifications of `discord.User` instances. For more information, see the `MockGuild` docstring. """ spec_set = user_instance @@ -305,7 +305,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Bot objects. - Instances of this class will follow the specifications of `disnake.ext.commands.Bot` instances. + Instances of this class will follow the specifications of `discord.ext.commands.Bot` instances. For more information, see the `MockGuild` docstring. """ spec_set = Bot( @@ -324,7 +324,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): self.stats = unittest.mock.create_autospec(spec=AsyncStatsClient, spec_set=True) -# Create a TextChannel instance to get a realistic MagicMock of `disnake.TextChannel` +# Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { 'id': 1, 'type': 'TextChannel', @@ -337,17 +337,17 @@ channel_data = { } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() -text_channel_instance = disnake.TextChannel(state=state, guild=guild, data=channel_data) +text_channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) channel_data["type"] = "VoiceChannel" -voice_channel_instance = disnake.VoiceChannel(state=state, guild=guild, data=channel_data) +voice_channel_instance = discord.VoiceChannel(state=state, guild=guild, data=channel_data) class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. - Instances of this class will follow the specifications of `disnake.TextChannel` instances. For + Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ spec_set = text_channel_instance @@ -364,7 +364,7 @@ class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock VoiceChannel objects. - Instances of this class will follow the specifications of `disnake.VoiceChannel` instances. For + Instances of this class will follow the specifications of `discord.VoiceChannel` instances. For more information, see the `MockGuild` docstring. """ spec_set = voice_channel_instance @@ -381,14 +381,14 @@ class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): state = unittest.mock.MagicMock() me = unittest.mock.MagicMock() dm_channel_data = {"id": 1, "recipients": [unittest.mock.MagicMock()]} -dm_channel_instance = disnake.DMChannel(me=me, state=state, data=dm_channel_data) +dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data) class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. - Instances of this class will follow the specifications of `disnake.TextChannel` instances. For + Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ spec_set = dm_channel_instance @@ -398,17 +398,17 @@ class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): super().__init__(**collections.ChainMap(kwargs, default_kwargs)) -# Create CategoryChannel instance to get a realistic MagicMock of `disnake.CategoryChannel` +# Create CategoryChannel instance to get a realistic MagicMock of `discord.CategoryChannel` category_channel_data = { 'id': 1, - 'type': disnake.ChannelType.category, + 'type': discord.ChannelType.category, 'name': 'category', 'position': 1, } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() -category_channel_instance = disnake.CategoryChannel( +category_channel_instance = discord.CategoryChannel( state=state, guild=guild, data=category_channel_data ) @@ -419,7 +419,7 @@ class MockCategoryChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): super().__init__(**collections.ChainMap(default_kwargs, kwargs)) -# Create a Message instance to get a realistic MagicMock of `disnake.Message` +# Create a Message instance to get a realistic MagicMock of `discord.Message` message_data = { 'id': 1, 'webhook_id': 431341013479718912, @@ -438,10 +438,10 @@ message_data = { } state = unittest.mock.MagicMock() channel = unittest.mock.MagicMock() -message_instance = disnake.Message(state=state, channel=channel, data=message_data) +message_instance = discord.Message(state=state, channel=channel, data=message_data) -# Create a Context instance to get a realistic MagicMock of `disnake.ext.commands.Context` +# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` context_instance = Context( message=unittest.mock.MagicMock(), prefix="$", @@ -455,7 +455,7 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Context objects. - Instances of this class will follow the specifications of `disnake.ext.commands.Context` + Instances of this class will follow the specifications of `discord.ext.commands.Context` instances. For more information, see the `MockGuild` docstring. """ spec_set = context_instance @@ -471,14 +471,14 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): self.invoked_from_error_handler = kwargs.get('invoked_from_error_handler', False) -attachment_instance = disnake.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) +attachment_instance = discord.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Attachment objects. - Instances of this class will follow the specifications of `disnake.Attachment` instances. For + Instances of this class will follow the specifications of `discord.Attachment` instances. For more information, see the `MockGuild` docstring. """ spec_set = attachment_instance @@ -488,7 +488,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. - Instances of this class will follow the specifications of `disnake.Message` instances. For more + Instances of this class will follow the specifications of `discord.Message` instances. For more information, see the `MockGuild` docstring. """ spec_set = message_instance @@ -501,14 +501,14 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): emoji_data = {'require_colons': True, 'managed': True, 'id': 1, 'name': 'hyperlemon'} -emoji_instance = disnake.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) +emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Emoji objects. - Instances of this class will follow the specifications of `disnake.Emoji` instances. For more + Instances of this class will follow the specifications of `discord.Emoji` instances. For more information, see the `MockGuild` docstring. """ spec_set = emoji_instance @@ -518,27 +518,27 @@ class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): self.guild = kwargs.get('guild', MockGuild()) -partial_emoji_instance = disnake.PartialEmoji(animated=False, name='guido') +partial_emoji_instance = discord.PartialEmoji(animated=False, name='guido') class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock PartialEmoji objects. - Instances of this class will follow the specifications of `disnake.PartialEmoji` instances. For + Instances of this class will follow the specifications of `discord.PartialEmoji` instances. For more information, see the `MockGuild` docstring. """ spec_set = partial_emoji_instance -reaction_instance = disnake.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) +reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) class MockReaction(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Reaction objects. - Instances of this class will follow the specifications of `disnake.Reaction` instances. For + Instances of this class will follow the specifications of `discord.Reaction` instances. For more information, see the `MockGuild` docstring. """ spec_set = reaction_instance @@ -556,14 +556,14 @@ class MockReaction(CustomMockMixin, unittest.mock.MagicMock): self.__str__.return_value = str(self.emoji) -webhook_instance = disnake.Webhook(data=unittest.mock.MagicMock(), session=unittest.mock.MagicMock()) +webhook_instance = discord.Webhook(data=unittest.mock.MagicMock(), session=unittest.mock.MagicMock()) class MockAsyncWebhook(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Webhook objects using an AsyncWebhookAdapter. - Instances of this class will follow the specifications of `disnake.Webhook` instances. For + Instances of this class will follow the specifications of `discord.Webhook` instances. For more information, see the `MockGuild` docstring. """ spec_set = webhook_instance -- cgit v1.2.3 From 047705ac91c2997ccb509ea4e1fb3fad38840412 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Thu, 31 Mar 2022 20:46:45 +0100 Subject: Remove async stats and site api wrapper We now source them from bot-core, so no need to have them here too. --- bot/api.py | 102 --------------------- bot/async_stats.py | 40 -------- bot/converters.py | 2 +- bot/exts/backend/error_handler.py | 2 +- bot/exts/backend/sync/_cog.py | 2 +- bot/exts/backend/sync/_syncers.py | 2 +- bot/exts/filters/filter_lists.py | 2 +- bot/exts/filters/filtering.py | 2 +- bot/exts/fun/off_topic_names.py | 2 +- bot/exts/info/doc/_cog.py | 2 +- bot/exts/info/information.py | 2 +- bot/exts/moderation/infraction/_scheduler.py | 2 +- bot/exts/moderation/infraction/_utils.py | 2 +- bot/exts/moderation/voice_gate.py | 2 +- bot/exts/moderation/watchchannels/_watchchannel.py | 2 +- bot/exts/recruitment/talentpool/_cog.py | 2 +- bot/exts/recruitment/talentpool/_review.py | 2 +- tests/bot/exts/backend/sync/test_base.py | 3 +- tests/bot/exts/backend/sync/test_cog.py | 2 +- tests/bot/exts/backend/test_error_handler.py | 2 +- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- tests/bot/test_api.py | 66 ------------- tests/helpers.py | 4 +- 23 files changed, 22 insertions(+), 229 deletions(-) delete mode 100644 bot/api.py delete mode 100644 bot/async_stats.py delete mode 100644 tests/bot/test_api.py (limited to 'tests/helpers.py') diff --git a/bot/api.py b/bot/api.py deleted file mode 100644 index 856f7c865..000000000 --- a/bot/api.py +++ /dev/null @@ -1,102 +0,0 @@ -import asyncio -from typing import Optional -from urllib.parse import quote as quote_url - -import aiohttp - -from bot.log import get_logger - -from .constants import Keys, URLs - -log = get_logger(__name__) - - -class ResponseCodeError(ValueError): - """Raised when a non-OK HTTP response is received.""" - - def __init__( - self, - response: aiohttp.ClientResponse, - response_json: Optional[dict] = None, - response_text: str = "" - ): - self.status = response.status - self.response_json = response_json or {} - self.response_text = response_text - self.response = response - - def __str__(self): - response = self.response_json if self.response_json else self.response_text - return f"Status: {self.status} Response: {response}" - - -class APIClient: - """Django Site API wrapper.""" - - # These are class attributes so they can be seen when being mocked for tests. - # See commit 22a55534ef13990815a6f69d361e2a12693075d5 for details. - session: Optional[aiohttp.ClientSession] = None - loop: asyncio.AbstractEventLoop = None - - def __init__(self, **session_kwargs): - auth_headers = { - 'Authorization': f"Token {Keys.site_api}" - } - - if 'headers' in session_kwargs: - session_kwargs['headers'].update(auth_headers) - else: - session_kwargs['headers'] = auth_headers - - # aiohttp will complain if APIClient gets instantiated outside a coroutine. Thankfully, we - # don't and shouldn't need to do that, so we can avoid scheduling a task to create it. - self.session = aiohttp.ClientSession(**session_kwargs) - - @staticmethod - def _url_for(endpoint: str) -> str: - return f"{URLs.site_api_schema}{URLs.site_api}/{quote_url(endpoint)}" - - async def close(self) -> None: - """Close the aiohttp session.""" - await self.session.close() - - async def maybe_raise_for_status(self, response: aiohttp.ClientResponse, should_raise: bool) -> None: - """Raise ResponseCodeError for non-OK response if an exception should be raised.""" - if should_raise and response.status >= 400: - try: - response_json = await response.json() - raise ResponseCodeError(response=response, response_json=response_json) - except aiohttp.ContentTypeError: - response_text = await response.text() - raise ResponseCodeError(response=response, response_text=response_text) - - async def request(self, method: str, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> dict: - """Send an HTTP request to the site API and return the JSON response.""" - async with self.session.request(method.upper(), self._url_for(endpoint), **kwargs) as resp: - await self.maybe_raise_for_status(resp, raise_for_status) - return await resp.json() - - async def get(self, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> dict: - """Site API GET.""" - return await self.request("GET", endpoint, raise_for_status=raise_for_status, **kwargs) - - async def patch(self, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> dict: - """Site API PATCH.""" - return await self.request("PATCH", endpoint, raise_for_status=raise_for_status, **kwargs) - - async def post(self, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> dict: - """Site API POST.""" - return await self.request("POST", endpoint, raise_for_status=raise_for_status, **kwargs) - - async def put(self, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> dict: - """Site API PUT.""" - return await self.request("PUT", endpoint, raise_for_status=raise_for_status, **kwargs) - - async def delete(self, endpoint: str, *, raise_for_status: bool = True, **kwargs) -> Optional[dict]: - """Site API DELETE.""" - async with self.session.delete(self._url_for(endpoint), **kwargs) as resp: - if resp.status == 204: - return None - - await self.maybe_raise_for_status(resp, raise_for_status) - return await resp.json() diff --git a/bot/async_stats.py b/bot/async_stats.py deleted file mode 100644 index 0303de7a1..000000000 --- a/bot/async_stats.py +++ /dev/null @@ -1,40 +0,0 @@ -import asyncio -import socket - -from botcore.utils import scheduling -from statsd.client.base import StatsClientBase - - -class AsyncStatsClient(StatsClientBase): - """An async transport method for statsd communication.""" - - def __init__( - self, - loop: asyncio.AbstractEventLoop, - host: str = 'localhost', - port: int = 8125, - prefix: str = None - ): - """Create a new client.""" - family, _, _, _, addr = socket.getaddrinfo( - host, port, socket.AF_INET, socket.SOCK_DGRAM)[0] - self._addr = addr - self._prefix = prefix - self._loop = loop - self._transport = None - - async def create_socket(self) -> None: - """Use the loop.create_datagram_endpoint method to create a socket.""" - self._transport, _ = await self._loop.create_datagram_endpoint( - asyncio.DatagramProtocol, - family=socket.AF_INET, - remote_addr=self._addr - ) - - def _send(self, data: str) -> None: - """Start an async task to send data to statsd.""" - scheduling.create_task(self._async_send(data), event_loop=self._loop) - - async def _async_send(self, data: str) -> None: - """Send data to the statsd server using the async transport.""" - self._transport.sendto(data.encode('ascii'), self._addr) diff --git a/bot/converters.py b/bot/converters.py index e819e4713..a3f4630a0 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -8,13 +8,13 @@ from ssl import CertificateError import dateutil.parser import discord from aiohttp import ClientConnectorError +from botcore.site_api import ResponseCodeError from botcore.utils.regex import DISCORD_INVITE from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument, Bot, Context, Converter, IDConverter, MemberConverter, UserConverter from discord.utils import escape_markdown, snowflake_time from bot import exts -from bot.api import ResponseCodeError from bot.constants import URLs from bot.errors import InvalidInfraction from bot.exts.info.doc import _inventory_parser diff --git a/bot/exts/backend/error_handler.py b/bot/exts/backend/error_handler.py index fabb2dbb5..5391a7f15 100644 --- a/bot/exts/backend/error_handler.py +++ b/bot/exts/backend/error_handler.py @@ -1,10 +1,10 @@ import difflib +from botcore.site_api import ResponseCodeError from discord import Embed from discord.ext.commands import ChannelNotFound, Cog, Context, TextChannelConverter, VoiceChannelConverter, errors from sentry_sdk import push_scope -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Colours, Icons, MODERATION_ROLES from bot.errors import InvalidInfractedUserError, LockedResourceError diff --git a/bot/exts/backend/sync/_cog.py b/bot/exts/backend/sync/_cog.py index 58aabc141..a5bf82397 100644 --- a/bot/exts/backend/sync/_cog.py +++ b/bot/exts/backend/sync/_cog.py @@ -1,11 +1,11 @@ from typing import Any, Dict +from botcore.site_api import ResponseCodeError from discord import Member, Role, User from discord.ext import commands from discord.ext.commands import Cog, Context from bot import constants -from bot.api import ResponseCodeError from bot.bot import Bot from bot.exts.backend.sync import _syncers from bot.log import get_logger diff --git a/bot/exts/backend/sync/_syncers.py b/bot/exts/backend/sync/_syncers.py index 45301b098..e1c4541ef 100644 --- a/bot/exts/backend/sync/_syncers.py +++ b/bot/exts/backend/sync/_syncers.py @@ -2,12 +2,12 @@ import abc import typing as t from collections import namedtuple +from botcore.site_api import ResponseCodeError from discord import Guild from discord.ext.commands import Context from more_itertools import chunked import bot -from bot.api import ResponseCodeError from bot.log import get_logger from bot.utils.members import get_or_fetch_member diff --git a/bot/exts/filters/filter_lists.py b/bot/exts/filters/filter_lists.py index 3e3f5c562..fc9cfbeca 100644 --- a/bot/exts/filters/filter_lists.py +++ b/bot/exts/filters/filter_lists.py @@ -1,11 +1,11 @@ import re from typing import Optional +from botcore.site_api import ResponseCodeError from discord import Colour, Embed from discord.ext.commands import BadArgument, Cog, Context, IDConverter, group, has_any_role from bot import constants -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels from bot.converters import ValidDiscordServerInvite, ValidFilterListType diff --git a/bot/exts/filters/filtering.py b/bot/exts/filters/filtering.py index cabb7f0b6..6982f5948 100644 --- a/bot/exts/filters/filtering.py +++ b/bot/exts/filters/filtering.py @@ -9,6 +9,7 @@ import dateutil.parser import regex import tldextract from async_rediscache import RedisCache +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from botcore.utils.regex import DISCORD_INVITE from dateutil.relativedelta import relativedelta @@ -16,7 +17,6 @@ from discord import ChannelType, Colour, Embed, Forbidden, HTTPException, Member from discord.ext.commands import Cog from discord.utils import escape_markdown -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, Colours, Filter, Guild, Icons, URLs from bot.exts.events.code_jams._channels import CATEGORY_NAME as JAM_CATEGORY_NAME diff --git a/bot/exts/fun/off_topic_names.py b/bot/exts/fun/off_topic_names.py index ac172f2a8..d8111bdf5 100644 --- a/bot/exts/fun/off_topic_names.py +++ b/bot/exts/fun/off_topic_names.py @@ -2,12 +2,12 @@ import difflib from datetime import timedelta import arrow +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from discord import Colour, Embed from discord.ext.commands import Cog, Context, group, has_any_role from discord.utils import sleep_until -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, MODERATION_ROLES from bot.converters import OffTopicName diff --git a/bot/exts/info/doc/_cog.py b/bot/exts/info/doc/_cog.py index 8c3038c5b..bbdc4e82a 100644 --- a/bot/exts/info/doc/_cog.py +++ b/bot/exts/info/doc/_cog.py @@ -10,11 +10,11 @@ from typing import Dict, NamedTuple, Optional, Tuple, Union import aiohttp import discord +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from botcore.utils.scheduling import Scheduler from discord.ext import commands -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import MODERATION_ROLES, RedirectOutput from bot.converters import Inventory, PackageName, ValidURL, allowed_strings diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index b56fd171a..e7d17c971 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -6,12 +6,12 @@ from textwrap import shorten from typing import Any, DefaultDict, Mapping, Optional, Tuple, Union import rapidfuzz +from botcore.site_api import ResponseCodeError from discord import AllowedMentions, Colour, Embed, Guild, Message, Role from discord.ext.commands import BucketType, Cog, Context, Greedy, Paginator, command, group, has_any_role from discord.utils import escape_markdown from bot import constants -from bot.api import ResponseCodeError from bot.bot import Bot from bot.converters import MemberOrUser from bot.decorators import in_whitelist diff --git a/bot/exts/moderation/infraction/_scheduler.py b/bot/exts/moderation/infraction/_scheduler.py index 137358ec3..9c73bde5f 100644 --- a/bot/exts/moderation/infraction/_scheduler.py +++ b/bot/exts/moderation/infraction/_scheduler.py @@ -6,11 +6,11 @@ from gettext import ngettext import arrow import dateutil.parser import discord +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from discord.ext.commands import Context from bot import constants -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Colours from bot.converters import MemberOrUser diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index c1be18362..3a2485ec2 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -3,10 +3,10 @@ from datetime import datetime import arrow import discord +from botcore.site_api import ResponseCodeError from discord.ext.commands import Context import bot -from bot.api import ResponseCodeError from bot.constants import Colours, Icons from bot.converters import MemberOrUser from bot.errors import InvalidInfractedUserError diff --git a/bot/exts/moderation/voice_gate.py b/bot/exts/moderation/voice_gate.py index 33096e7e0..9b1621c01 100644 --- a/bot/exts/moderation/voice_gate.py +++ b/bot/exts/moderation/voice_gate.py @@ -5,10 +5,10 @@ from datetime import timedelta import arrow import discord from async_rediscache import RedisCache +from botcore.site_api import ResponseCodeError from discord import Colour, Member, VoiceState from discord.ext.commands import Cog, Context, command -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, MODERATION_ROLES, Roles, VoiceGate as GateConf from bot.decorators import has_no_roles, in_whitelist diff --git a/bot/exts/moderation/watchchannels/_watchchannel.py b/bot/exts/moderation/watchchannels/_watchchannel.py index ab5ce62f9..bc78b3934 100644 --- a/bot/exts/moderation/watchchannels/_watchchannel.py +++ b/bot/exts/moderation/watchchannels/_watchchannel.py @@ -7,11 +7,11 @@ from dataclasses import dataclass from typing import Any, Dict, Optional import discord +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from discord import Color, DMChannel, Embed, HTTPException, Message, errors from discord.ext.commands import Cog, Context -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import BigBrother as BigBrotherConfig, Guild as GuildConfig, Icons from bot.exts.filters.token_remover import TokenRemover diff --git a/bot/exts/recruitment/talentpool/_cog.py b/bot/exts/recruitment/talentpool/_cog.py index 8aa124536..24496af54 100644 --- a/bot/exts/recruitment/talentpool/_cog.py +++ b/bot/exts/recruitment/talentpool/_cog.py @@ -5,11 +5,11 @@ from typing import Optional, Union import discord from async_rediscache import RedisCache +from botcore.site_api import ResponseCodeError from botcore.utils import scheduling from discord import Color, Embed, Member, PartialMessage, RawReactionActionEvent, User from discord.ext.commands import BadArgument, Cog, Context, group, has_any_role -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles, STAFF_ROLES from bot.converters import MemberOrUser, UnambiguousMemberOrUser diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index d0edf5388..be181d005 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -9,12 +9,12 @@ from datetime import datetime, timedelta from typing import List, Optional, Union import arrow +from botcore.site_api import ResponseCodeError from botcore.utils.scheduling import Scheduler from dateutil.parser import isoparse from discord import Embed, Emoji, Member, Message, NotFound, PartialMessage, TextChannel from discord.ext.commands import Context -from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels, Colours, Emojis, Guild, Roles from bot.log import get_logger diff --git a/tests/bot/exts/backend/sync/test_base.py b/tests/bot/exts/backend/sync/test_base.py index 9dc46005b..a17c1fa10 100644 --- a/tests/bot/exts/backend/sync/test_base.py +++ b/tests/bot/exts/backend/sync/test_base.py @@ -1,7 +1,8 @@ import unittest from unittest import mock -from bot.api import ResponseCodeError +from botcore.site_api import ResponseCodeError + from bot.exts.backend.sync._syncers import Syncer from tests import helpers diff --git a/tests/bot/exts/backend/sync/test_cog.py b/tests/bot/exts/backend/sync/test_cog.py index 7dff38f96..4ec36e39f 100644 --- a/tests/bot/exts/backend/sync/test_cog.py +++ b/tests/bot/exts/backend/sync/test_cog.py @@ -2,9 +2,9 @@ import unittest from unittest import mock import discord +from botcore.site_api import ResponseCodeError from bot import constants -from bot.api import ResponseCodeError from bot.exts.backend import sync from bot.exts.backend.sync._cog import Sync from bot.exts.backend.sync._syncers import Syncer diff --git a/tests/bot/exts/backend/test_error_handler.py b/tests/bot/exts/backend/test_error_handler.py index 35fa0ee59..04a018289 100644 --- a/tests/bot/exts/backend/test_error_handler.py +++ b/tests/bot/exts/backend/test_error_handler.py @@ -1,9 +1,9 @@ import unittest from unittest.mock import AsyncMock, MagicMock, call, patch +from botcore.site_api import ResponseCodeError from discord.ext.commands import errors -from bot.api import ResponseCodeError from bot.errors import InvalidInfractedUserError, LockedResourceError from bot.exts.backend.error_handler import ErrorHandler, setup from bot.exts.info.tags import Tags diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index ff81ddd65..5cf02033d 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -3,9 +3,9 @@ from collections import namedtuple from datetime import datetime from unittest.mock import AsyncMock, MagicMock, call, patch +from botcore.site_api import ResponseCodeError from discord import Embed, Forbidden, HTTPException, NotFound -from bot.api import ResponseCodeError from bot.constants import Colours, Icons from bot.exts.moderation.infraction import _utils as utils from tests.helpers import MockBot, MockContext, MockMember, MockUser diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py deleted file mode 100644 index 76bcb481d..000000000 --- a/tests/bot/test_api.py +++ /dev/null @@ -1,66 +0,0 @@ -import unittest -from unittest.mock import MagicMock - -from bot import api - - -class APIClientTests(unittest.IsolatedAsyncioTestCase): - """Tests for the bot's API client.""" - - @classmethod - def setUpClass(cls): - """Sets up the shared fixtures for the tests.""" - cls.error_api_response = MagicMock() - cls.error_api_response.status = 999 - - def test_response_code_error_default_initialization(self): - """Test the default initialization of `ResponseCodeError` without `text` or `json`""" - error = api.ResponseCodeError(response=self.error_api_response) - - self.assertIs(error.status, self.error_api_response.status) - self.assertEqual(error.response_json, {}) - self.assertEqual(error.response_text, "") - self.assertIs(error.response, self.error_api_response) - - def test_response_code_error_string_representation_default_initialization(self): - """Test the string representation of `ResponseCodeError` initialized without text or json.""" - error = api.ResponseCodeError(response=self.error_api_response) - self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: ") - - def test_response_code_error_initialization_with_json(self): - """Test the initialization of `ResponseCodeError` with json.""" - json_data = {'hello': 'world'} - error = api.ResponseCodeError( - response=self.error_api_response, - response_json=json_data, - ) - self.assertEqual(error.response_json, json_data) - self.assertEqual(error.response_text, "") - - def test_response_code_error_string_representation_with_nonempty_response_json(self): - """Test the string representation of `ResponseCodeError` initialized with json.""" - json_data = {'hello': 'world'} - error = api.ResponseCodeError( - response=self.error_api_response, - response_json=json_data - ) - self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: {json_data}") - - def test_response_code_error_initialization_with_text(self): - """Test the initialization of `ResponseCodeError` with text.""" - text_data = 'Lemon will eat your soul' - error = api.ResponseCodeError( - response=self.error_api_response, - response_text=text_data, - ) - self.assertEqual(error.response_text, text_data) - self.assertEqual(error.response_json, {}) - - def test_response_code_error_string_representation_with_nonempty_response_text(self): - """Test the string representation of `ResponseCodeError` initialized with text.""" - text_data = 'Lemon will eat your soul' - error = api.ResponseCodeError( - response=self.error_api_response, - response_text=text_data - ) - self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: {text_data}") diff --git a/tests/helpers.py b/tests/helpers.py index 9d4988d23..3e6290e58 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -9,10 +9,10 @@ from typing import Iterable, Optional import discord from aiohttp import ClientSession +from botcore.async_stats import AsyncStatsClient +from botcore.site_api import APIClient from discord.ext.commands import Context -from bot.api import APIClient -from bot.async_stats import AsyncStatsClient from bot.bot import Bot from tests._autospec import autospec # noqa: F401 other modules import it via this module -- cgit v1.2.3 From 277bb011d8ae6b1c58c9de80d10b61791ee1fc49 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sat, 2 Apr 2022 22:42:58 +0100 Subject: Adding missing kwargs required by BotBase in test helper --- tests/helpers.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests/helpers.py') diff --git a/tests/helpers.py b/tests/helpers.py index 3e6290e58..e6e95c20c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -312,6 +312,9 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): command_prefix=unittest.mock.MagicMock(), loop=_get_mock_loop(), redis_session=unittest.mock.MagicMock(), + http_session=unittest.mock.MagicMock(), + allowed_roles=[1], + guild_id=1, ) additional_spec_asyncs = ("wait_for", "redis_ready") -- cgit v1.2.3 From 56e38eeb38de10611611f0f81f5cbc429d7f2bc8 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Sat, 2 Apr 2022 22:44:53 +0100 Subject: Update test helpers with breaking d.py changes region was removed from the guild object, so this has been replaced with features add_cog is now async, so it is now an async_mock during tests Two new required voice_channel attrs were added channel.type is required to be set to ChannelType due to a new isinstance check in d.py --- tests/helpers.py | 4 ++++ tests/test_helpers.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'tests/helpers.py') diff --git a/tests/helpers.py b/tests/helpers.py index e6e95c20c..2f0c9b4ad 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -325,6 +325,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): self.api_client = MockAPIClient(loop=self.loop) self.http_session = unittest.mock.create_autospec(spec=ClientSession, spec_set=True) self.stats = unittest.mock.create_autospec(spec=AsyncStatsClient, spec_set=True) + self.add_cog = unittest.mock.AsyncMock() # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` @@ -337,6 +338,8 @@ channel_data = { 'position': 1, 'nsfw': False, 'last_message_id': 1, + 'bitrate': 1337, + 'user_limit': 25, } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() @@ -441,6 +444,7 @@ message_data = { } state = unittest.mock.MagicMock() channel = unittest.mock.MagicMock() +channel.type = discord.ChannelType.text message_instance = discord.Message(state=state, channel=channel, data=message_data) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 81285e009..f3040b305 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -327,7 +327,7 @@ class MockObjectTests(unittest.TestCase): def test_spec_propagation_of_mock_subclasses(self): """Test if the `spec` does not propagate to attributes of the mock object.""" test_values = ( - (helpers.MockGuild, "region"), + (helpers.MockGuild, "features"), (helpers.MockRole, "mentionable"), (helpers.MockMember, "display_name"), (helpers.MockBot, "owner_id"), -- cgit v1.2.3 From 4c1a076dd0b7c021ac1b352589bb353139f86a6f Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 19 Apr 2022 17:23:28 +0100 Subject: Pass the now required intents kwarg when creating MockBot --- tests/helpers.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tests/helpers.py') diff --git a/tests/helpers.py b/tests/helpers.py index 2f0c9b4ad..a6e4bdd66 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -315,6 +315,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): http_session=unittest.mock.MagicMock(), allowed_roles=[1], guild_id=1, + intents=discord.Intents.all(), ) additional_spec_asyncs = ("wait_for", "redis_ready") -- cgit v1.2.3 From 8f9f25a796f6cc07f01f8f7f56e825cb5ebf56c8 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 23 Apr 2022 14:24:20 +0400 Subject: Speed Up Sync Cog Loading The user syncer was blocking the startup of the sync cog due to having to perform thousands of pointless member fetch requests. This speeds up that process by increasing the probability that the cache is up-to-date using `Guild.chunked`, and limiting the fetches to members who were in the guild during the previous sync only. Co-authored-by: ChrisJL Co-authored-by: wookie184 Signed-off-by: Hassan Abouelela --- bot/exts/backend/sync/_cog.py | 6 ++++++ bot/exts/backend/sync/_syncers.py | 13 +++++++++++-- tests/helpers.py | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) (limited to 'tests/helpers.py') diff --git a/bot/exts/backend/sync/_cog.py b/bot/exts/backend/sync/_cog.py index a5bf82397..4ec822d3f 100644 --- a/bot/exts/backend/sync/_cog.py +++ b/bot/exts/backend/sync/_cog.py @@ -1,3 +1,4 @@ +import asyncio from typing import Any, Dict from botcore.site_api import ResponseCodeError @@ -27,6 +28,11 @@ class Sync(Cog): if guild is None: return + log.info("Waiting for guild to be chunked to start syncers.") + while not guild.chunked: + await asyncio.sleep(10) + log.info("Starting syncers.") + for syncer in (_syncers.RoleSyncer, _syncers.UserSyncer): await syncer.sync(guild) diff --git a/bot/exts/backend/sync/_syncers.py b/bot/exts/backend/sync/_syncers.py index e1c4541ef..799137cb9 100644 --- a/bot/exts/backend/sync/_syncers.py +++ b/bot/exts/backend/sync/_syncers.py @@ -2,6 +2,7 @@ import abc import typing as t from collections import namedtuple +import discord.errors from botcore.site_api import ResponseCodeError from discord import Guild from discord.ext.commands import Context @@ -9,7 +10,6 @@ from more_itertools import chunked import bot from bot.log import get_logger -from bot.utils.members import get_or_fetch_member log = get_logger(__name__) @@ -157,7 +157,16 @@ class UserSyncer(Syncer): if db_user[db_field] != guild_value: updated_fields[db_field] = guild_value - if guild_user := await get_or_fetch_member(guild, db_user["id"]): + guild_user = guild.get_member(db_user["id"]) + if not guild_user and db_user["in_guild"]: + # The member was in the guild during the last sync. + # We try to fetch them to verify cache integrity. + try: + guild_user = await guild.fetch_member(db_user["id"]) + except discord.errors.NotFound: + guild_user = None + + if guild_user: seen_guild_users.add(guild_user.id) maybe_update("name", guild_user.name) diff --git a/tests/helpers.py b/tests/helpers.py index a6e4bdd66..5f3111616 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -171,7 +171,7 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): spec_set = guild_instance def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: - default_kwargs = {'id': next(self.discord_id), 'members': []} + default_kwargs = {'id': next(self.discord_id), 'members': [], "chunked": True} super().__init__(**collections.ChainMap(kwargs, default_kwargs)) self.roles = [MockRole(name="@everyone", position=1, id=0)] -- cgit v1.2.3 From ac5bb147f5dc6b80c20c4c9abd5cc1fb7ac4abfa Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Tue, 28 Jun 2022 13:17:39 +0100 Subject: Use new application format for message data in test helper --- tests/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/helpers.py') diff --git a/tests/helpers.py b/tests/helpers.py index 5f3111616..17214553c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -432,7 +432,7 @@ message_data = { 'webhook_id': 431341013479718912, 'attachments': [], 'embeds': [], - 'application': 'Python Discord', + 'application': {"id": 4, "description": "A Python Bot", "name": "Python Discord", "icon": None}, 'activity': 'mocking', 'channel': unittest.mock.MagicMock(), 'edited_timestamp': '2019-10-14T15:33:48+00:00', -- cgit v1.2.3 From dada405211eac996196cdfb0496f4ff22f9a656a Mon Sep 17 00:00:00 2001 From: arl Date: Thu, 18 Aug 2022 19:01:22 -0400 Subject: fix: don't include replied mentions in mention filter (#2017) Co-authored-by: Izan Co-authored-by: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com> --- bot/rules/mentions.py | 56 +++++++++++++++++++++++++++++++++----- tests/bot/rules/test_mentions.py | 58 ++++++++++++++++++++++++++++++++++++---- tests/helpers.py | 22 +++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) (limited to 'tests/helpers.py') diff --git a/bot/rules/mentions.py b/bot/rules/mentions.py index 6f5addad1..ca1d0c01c 100644 --- a/bot/rules/mentions.py +++ b/bot/rules/mentions.py @@ -1,23 +1,65 @@ from typing import Dict, Iterable, List, Optional, Tuple -from discord import Member, Message +from discord import DeletedReferencedMessage, Member, Message, MessageType, NotFound + +import bot +from bot.log import get_logger + +log = get_logger(__name__) async def apply( last_message: Message, recent_messages: List[Message], config: Dict[str, int] ) -> Optional[Tuple[str, Iterable[Member], Iterable[Message]]]: - """Detects total mentions exceeding the limit sent by a single user.""" + """ + Detects total mentions exceeding the limit sent by a single user. + + Excludes mentions that are bots, themselves, or replied users. + + In very rare cases, may not be able to determine a + mention was to a reply, in which case it is not ignored. + """ relevant_messages = tuple( msg for msg in recent_messages if msg.author == last_message.author ) + # We use `msg.mentions` here as that is supplied by the api itself, to determine who was mentioned. + # Additionally, `msg.mentions` includes the user replied to, even if the mention doesn't occur in the body. + # In order to exclude users who are mentioned as a reply, we check if the msg has a reference + # + # While we could use regex to parse the message content, and get a list of + # the mentions, that solution is very prone to breaking. + # We would need to deal with codeblocks, escaping markdown, and any discrepancies between + # our implementation and discord's markdown parser which would cause false positives or false negatives. + total_recent_mentions = 0 + for msg in relevant_messages: + # We check if the message is a reply, and if it is try to get the author + # since we ignore mentions of a user that we're replying to + reply_author = None - total_recent_mentions = sum( - not user.bot - for msg in relevant_messages - for user in msg.mentions - ) + if msg.type == MessageType.reply: + ref = msg.reference + + if not (resolved := ref.resolved): + # It is possible, in a very unusual situation, for a message to have a reference + # that is both not in the cache, and deleted while running this function. + # In such a situation, this will throw an error which we catch. + try: + resolved = await bot.instance.get_partial_messageable(resolved.channel_id).fetch_message( + resolved.message_id + ) + except NotFound: + log.info('Could not fetch the reference message as it has been deleted.') + + if resolved and not isinstance(resolved, DeletedReferencedMessage): + reply_author = resolved.author + + for user in msg.mentions: + # Don't count bot or self mentions, or the user being replied to (if applicable) + if user.bot or user in {msg.author, reply_author}: + continue + total_recent_mentions += 1 if total_recent_mentions > config['max']: return ( diff --git a/tests/bot/rules/test_mentions.py b/tests/bot/rules/test_mentions.py index f8805ac48..e1f904917 100644 --- a/tests/bot/rules/test_mentions.py +++ b/tests/bot/rules/test_mentions.py @@ -1,15 +1,32 @@ -from typing import Iterable +from typing import Iterable, Optional + +import discord from bot.rules import mentions from tests.bot.rules import DisallowedCase, RuleTest -from tests.helpers import MockMember, MockMessage +from tests.helpers import MockMember, MockMessage, MockMessageReference -def make_msg(author: str, total_user_mentions: int, total_bot_mentions: int = 0) -> MockMessage: - """Makes a message with `total_mentions` mentions.""" +def make_msg( + author: str, + total_user_mentions: int, + total_bot_mentions: int = 0, + *, + reference: Optional[MockMessageReference] = None +) -> MockMessage: + """Makes a message from `author` with `total_user_mentions` user mentions and `total_bot_mentions` bot mentions.""" user_mentions = [MockMember() for _ in range(total_user_mentions)] bot_mentions = [MockMember(bot=True) for _ in range(total_bot_mentions)] - return MockMessage(author=author, mentions=user_mentions+bot_mentions) + + mentions = user_mentions + bot_mentions + if reference is not None: + # For the sake of these tests we assume that all references are mentions. + mentions.append(reference.resolved.author) + msg_type = discord.MessageType.reply + else: + msg_type = discord.MessageType.default + + return MockMessage(author=author, mentions=mentions, reference=reference, type=msg_type) class TestMentions(RuleTest): @@ -56,6 +73,16 @@ class TestMentions(RuleTest): ("bob",), 3, ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference())], + ("bob",), + 3, + ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference(reference_author_is_bot=True))], + ("bob",), + 3 + ) ) await self.run_disallowed(cases) @@ -71,6 +98,27 @@ class TestMentions(RuleTest): await self.run_allowed(cases) + async def test_ignore_reply_mentions(self): + """Messages with an allowed amount of mentions in the content, also containing reply mentions.""" + cases = ( + [ + make_msg("bob", 2, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)) + ], + [ + make_msg("bob", 2, reference=MockMessageReference()), + make_msg("bob", 0, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)), + make_msg("bob", 0, reference=MockMessageReference(reference_author_is_bot=True)) + ] + ) + + await self.run_allowed(cases) + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: last_message = case.recent_messages[0] return tuple( diff --git a/tests/helpers.py b/tests/helpers.py index 17214553c..687e15b96 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -492,6 +492,28 @@ class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): spec_set = attachment_instance +message_reference_instance = discord.MessageReference( + message_id=unittest.mock.MagicMock(id=1), + channel_id=unittest.mock.MagicMock(id=2), + guild_id=unittest.mock.MagicMock(id=3) +) + + +class MockMessageReference(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock MessageReference objects. + + Instances of this class will follow the specification of `discord.MessageReference` instances. + For more information, see the `MockGuild` docstring. + """ + spec_set = message_reference_instance + + def __init__(self, *, reference_author_is_bot: bool = False, **kwargs): + super().__init__(**kwargs) + referenced_msg_author = MockMember(name="bob", bot=reference_author_is_bot) + self.resolved = MockMessage(author=referenced_msg_author) + + class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. -- cgit v1.2.3