diff --git a/.gitignore b/.gitignore index da9a2f29..cf9bfc18 100644 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,4 @@ celerybeat-schedule #other .flake8 .pylintrc +Makefile diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 05cbfcb8..44952a08 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,8 +1,11 @@ stages: -- "test" +- test - deploy before_script: +- apt-get update && apt-get install redis-server -y +- redis-server --daemonize yes +- redis-cli ping - python -V - pip install wheel tox diff --git a/allianceauth/services/modules/discord/auth_hooks.py b/allianceauth/services/modules/discord/auth_hooks.py index f3fac01f..97119f46 100644 --- a/allianceauth/services/modules/discord/auth_hooks.py +++ b/allianceauth/services/modules/discord/auth_hooks.py @@ -102,7 +102,12 @@ class DiscordService(ServicesHook): @staticmethod def user_has_account(user: User) -> bool: - return DiscordUser.objects.user_has_account(user) + result = DiscordUser.objects.user_has_account(user) + if result: + logger.debug('User %s has a Discord account', user) + else: + logger.debug('User %s does not have a Discord account', user) + return result def validate_user(self, user): logger.debug('Validating user %s %s account', user, self.name) diff --git a/allianceauth/services/modules/discord/discord_client/__init__.py b/allianceauth/services/modules/discord/discord_client/__init__.py index 0b2d8f3e..40517860 100644 --- a/allianceauth/services/modules/discord/discord_client/__init__.py +++ b/allianceauth/services/modules/discord/discord_client/__init__.py @@ -1,2 +1,3 @@ from .client import DiscordClient # noqa -from .exceptions import DiscordApiBackoff # noqa \ No newline at end of file +from .exceptions import DiscordApiBackoff # noqa +from .helpers import DiscordRoles # noqa diff --git a/allianceauth/services/modules/discord/discord_client/client.py b/allianceauth/services/modules/discord/discord_client/client.py index 235716e1..701450aa 100644 --- a/allianceauth/services/modules/discord/discord_client/client.py +++ b/allianceauth/services/modules/discord/discord_client/client.py @@ -1,4 +1,5 @@ from hashlib import md5 +import json import logging from time import sleep from urllib.parse import urljoin @@ -22,6 +23,7 @@ from .app_settings import ( DISCORD_ROLES_CACHE_MAX_AGE, ) from .exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError +from .helpers import DiscordRoles from ..utils import LoggerAddTag @@ -72,8 +74,8 @@ class DiscordClient: _KEY_GLOBAL_BACKOFF_UNTIL = 'DISCORD_GLOBAL_BACKOFF_UNTIL' _KEY_GLOBAL_RATE_LIMIT_REMAINING = 'DISCORD_GLOBAL_RATE_LIMIT_REMAINING' _KEYPREFIX_GUILD_NAME = 'DISCORD_GUILD_NAME' + _KEYPREFIX_GUILD_ROLES = 'DISCORD_GUILD_ROLES' _KEYPREFIX_ROLE_NAME = 'DISCORD_ROLE_NAME' - _ROLE_NAME_MAX_CHARS = 100 _NICK_MAX_CHARS = 32 _HTTP_STATUS_CODE_NOT_FOUND = 404 @@ -166,23 +168,7 @@ class DiscordClient: ) return r.json() - # guild roles - - def create_guild_role(self, guild_id: int, role_name: str, **kwargs) -> dict: - """Create a new guild role with the given name. - See official documentation for additional optional parameters. - - Note that Discord allows creating multiple roles with the name name, - so it's important to check existing roles before creating new one - to avoid duplicates. - - return a new role object on success - """ - route = f"guilds/{guild_id}/roles" - data = {'name': self._sanitize_role_name(role_name)} - data.update(kwargs) - r = self._api_request(method='post', route=route, data=data) - return r.json() + # guild def guild_infos(self, guild_id: int) -> dict: """Returns all basic infos about this guild""" @@ -216,101 +202,131 @@ class DiscordClient: gen_key = DiscordClient._generate_hash(f'{guild_id}') return f'{cls._KEYPREFIX_GUILD_NAME}__{gen_key}' - def guild_roles(self, guild_id: int) -> list: - """Returns the list of all roles for this guild""" + # guild roles + + def guild_roles(self, guild_id: int, use_cache: bool = True) -> list: + """Returns the list of all roles for this guild + + If use_cache is set to False it will always hit the API to retrieve + fresh data and update the cache + """ + cache_key = self._guild_roles_cache_key(guild_id) + if use_cache: + roles_raw = self._redis.get(name=cache_key) + if roles_raw: + logger.debug('Returning roles for guild %s from cache', guild_id) + return json.loads(self._redis_decode(roles_raw)) + else: + logger.debug('No roles for guild %s in cache', guild_id) + route = f"guilds/{guild_id}/roles" - r = self._api_request(method='get', route=route) - return r.json() + r = self._api_request(method='get', route=route) + roles = r.json() + if roles and isinstance(roles, list): + self._redis.set( + name=cache_key, + value=json.dumps(roles), + px=DISCORD_ROLES_CACHE_MAX_AGE + ) + return roles + + def create_guild_role(self, guild_id: int, role_name: str, **kwargs) -> dict: + """Create a new guild role with the given name. + See official documentation for additional optional parameters. + + Note that Discord allows the creation of multiple roles with the same name, + so to avoid duplicates it's important to check existing roles + before creating new one + + returns a new role dict on success + """ + route = f"guilds/{guild_id}/roles" + data = {'name': DiscordRoles.sanitize_role_name(role_name)} + data.update(kwargs) + r = self._api_request(method='post', route=route, data=data) + role = r.json() + if role: + self._invalidate_guild_roles_cache(guild_id) + return role def delete_guild_role(self, guild_id: int, role_id: int) -> bool: """Deletes a guild role""" route = f"guilds/{guild_id}/roles/{role_id}" r = self._api_request(method='delete', route=route) if r.status_code == 204: + self._invalidate_guild_roles_cache(guild_id) return True else: return False + + def _invalidate_guild_roles_cache(self, guild_id: int) -> None: + cache_key = self._guild_roles_cache_key(guild_id) + self._redis.delete(cache_key) + logger.debug('Guild roles cache invalidated') - # guild role cache - - def match_guild_roles_to_names(self, guild_id: int, role_names: list) -> list: + @classmethod + def _guild_roles_cache_key(cls, guild_id: int) -> str: + """Returns key for accessing cached roles for a guild""" + gen_key = cls._generate_hash(f'{guild_id}') + return f'{cls._KEYPREFIX_GUILD_ROLES}__{gen_key}' + + def match_or_create_roles_from_names(self, guild_id: int, role_names: list) -> list: """returns Discord roles matching the given names Returns as list of tuple of role and created flag Will try to match with existing roles names Non-existing roles will be created, then created flag will be True - Roles names are cached to improve performance + Params: + - guild_id: ID of guild + - role_names: list of name strings each defining a role """ roles = list() + guild_roles = DiscordRoles(self.guild_roles(guild_id)) for role_name in role_names: - role, created = self.match_guild_role_to_name( - guild_id=guild_id, role_name=self._sanitize_role_name(role_name) + role, created = self.match_or_create_role_from_name( + guild_id=guild_id, + role_name=DiscordRoles.sanitize_role_name(role_name), + guild_roles=guild_roles ) if role: - roles.append((role, created)) + roles.append((role, created)) + if created: + guild_roles = guild_roles.union(DiscordRoles([role])) return roles - def match_guild_role_to_name(self, guild_id: int, role_name: str) -> tuple: + def match_or_create_role_from_name( + self, guild_id: int, role_name: str, guild_roles: DiscordRoles = None + ) -> tuple: """returns Discord role matching the given name Returns as tuple of role and created flag Will try to match with existing roles names Non-existing roles will be created, then created flag will be True - Roles names are cached to improve performance + Params: + - guild_id: ID of guild + - role_name: strings defining name of a role + - guild_roles: All known guild roles as DiscordRoles object. + Helps to void redundant lookups of guild roles + when this method is used multiple times. """ - created = False - role_name = self._sanitize_role_name(role_name) - role_id = self._redis_decode( - self._redis.get(name=self._role_cache_key(guild_id, role_name)) - ) - if not role_id: - role_id = None - for role in self.guild_roles(guild_id): - self._update_role_cache(guild_id, role) - if role['name'] == role_name: - role_id = role['id'] - - if role_id: - role = self._create_role(role_id, role_name) + if not isinstance(role_name, str): + raise TypeError('role_name must be of type string') + created = False + if guild_roles is None: + guild_roles = DiscordRoles(self.guild_roles(guild_id)) + role = guild_roles.role_by_name(role_name) + if not role: + if not DISCORD_DISABLE_ROLE_CREATION: + logger.debug('Need to create missing role: %s', role_name) + role = self.create_guild_role(guild_id, role_name) + created = True else: - if not DISCORD_DISABLE_ROLE_CREATION: - role_raw = self.create_guild_role(guild_id, role_name) - role = self._create_role(role_raw['id'], role_name) - self._update_role_cache(guild_id, role) - created = True - else: - role = None - else: - role = self._create_role(int(role_id), role_name) - - return role, created - - @staticmethod - def _create_role(role_id: int, role_name: str) -> dict: - return {'id': int(role_id), 'name': str(role_name)} + role = None - def _update_role_cache(self, guild_id: int, role: dict) -> bool: - """updates role cache with given role - - Returns True on success, else False or raises exception - """ - if not isinstance(role, dict): - raise TypeError('role must be a dict') - - return self._redis.set( - name=self._role_cache_key(guild_id=guild_id, role_name=role['name']), - value=role['id'], - px=DISCORD_ROLES_CACHE_MAX_AGE - ) - - @classmethod - def _role_cache_key(cls, guild_id: int, role_name: str) -> str: - """Returns key for accessing role given by name in the role cache""" - gen_key = DiscordClient._generate_hash(f'{guild_id}{role_name}') - return f'{cls._KEYPREFIX_ROLE_NAME}__{gen_key}' + return role, created # guild members @@ -524,10 +540,10 @@ class DiscordClient: args['json'] = data logger.info('%s: sending %s request to url \'%s\'', uid, method.upper(), url) - logger.debug('%s: request headers:\n%s', uid, headers) + logger.debug('%s: request headers: %s', uid, headers) r = getattr(requests, method)(**args) logger.debug( - '%s: returned status code %d with headers:\n%s', + '%s: returned status code %d with headers: %s', uid, r.status_code, r.headers @@ -589,7 +605,7 @@ class DiscordClient: resets_in = self._redis.pttl(self._KEY_GLOBAL_RATE_LIMIT_REMAINING) if requests_remaining >= 0: logger.debug( - '%s: Got %d remaining requests until reset in %s ms', + '%s: Got one of %d remaining requests until reset in %s ms', uid, requests_remaining + 1, resets_in @@ -679,11 +695,6 @@ class DiscordClient: """make sure its a list of integers""" return [int(role_id) for role_id in list(role_ids)] - @classmethod - def _sanitize_role_name(cls, role_name: str) -> str: - """shortens too long strings if necessary""" - return str(role_name)[:cls._ROLE_NAME_MAX_CHARS] - @classmethod def _sanitize_nick(cls, nick: str) -> str: """shortens too long strings if necessary""" diff --git a/allianceauth/services/modules/discord/discord_client/helpers.py b/allianceauth/services/modules/discord/discord_client/helpers.py new file mode 100644 index 00000000..ec890b7b --- /dev/null +++ b/allianceauth/services/modules/discord/discord_client/helpers.py @@ -0,0 +1,132 @@ +from copy import copy + + +class DiscordRoles: + """Container class that helps dealing with Discord roles. + + Objects of this class are immutable and work in many ways like sets. + + Ideally objects are initialized from raw API responses, + e.g. from DiscordClient.guild.roles() + """ + _ROLE_NAME_MAX_CHARS = 100 + + def __init__(self, roles_lst: list) -> None: + """roles_lst must be a list of dict, each defining a role""" + if not isinstance(roles_lst, (list, set, tuple)): + raise TypeError('roles_lst must be of type list, set or tuple') + self._roles = dict() + self._roles_by_name = dict() + for role in list(roles_lst): + self._assert_valid_role(role) + self._roles[int(role['id'])] = role + self._roles_by_name[self.sanitize_role_name(role['name'])] = role + + def __eq__(self, other): + if isinstance(other, type(self)): + return self.ids() == other.ids() + return NotImplemented + + def __hash__(self): + return hash(tuple(sorted(self._roles.keys()))) + + def __iter__(self): + for role in self._roles.values(): + yield role + + def __contains__(self, item) -> bool: + return int(item) in self._roles + + def __len__(self): + return len(self._roles.keys()) + + def has_roles(self, role_ids: set) -> bool: + """returns true if this objects contains all roles defined by given role_ids + incl. managed roles + """ + role_ids = {int(id) for id in role_ids} + all_role_ids = self._roles.keys() + return role_ids.issubset(all_role_ids) + + def ids(self) -> set: + """return a set of all role IDs""" + return set(self._roles.keys()) + + def subset(self, role_ids: set = None, managed_only: bool = False) -> object: + """returns a new object containing the subset of roles as defined + by given role IDs and/or including managed roles only + """ + if role_ids is not None: + role_ids = {int(id) for id in role_ids} + + if role_ids is not None and not managed_only: + return type(self)([ + role for role_id, role in self._roles.items() if role_id in role_ids + ]) + + elif role_ids is None and managed_only: + return type(self)([ + role for _, role in self._roles.items() if role['managed'] + ]) + + elif role_ids is not None and managed_only: + return type(self)([ + role for role_id, role in self._roles.items() + if role_id in role_ids and role['managed'] + ]) + + else: + return copy(self) + + def union(self, other: object) -> object: + """returns a new roles object that is the union of this roles object + with other""" + return type(self)(list(self) + list(other)) + + def difference(self, other: object) -> object: + """returns a new roles object that only contains the roles + that exist in the current objects, but not in other + """ + new_ids = self.ids().difference(other.ids()) + return self.subset(role_ids=new_ids) + + def role_by_name(self, role_name: str) -> dict: + """returns role if one with matching name is found else an empty dict""" + role_name = self.sanitize_role_name(role_name) + if role_name in self._roles_by_name: + return self._roles_by_name[role_name] + else: + return dict() + + @classmethod + def create_from_matched_roles(cls, matched_roles: list) -> None: + """returns a new object created from the given list of matches roles + + matches_roles must be a list of tuples in the form: (role, created) + """ + raw_roles = [x[0] for x in matched_roles] + return cls(raw_roles) + + @staticmethod + def _assert_valid_role(role: dict): + if not isinstance(role, dict): + raise TypeError('Roles must be of type dict: %s' % role) + + if 'id' not in role or 'name' not in role or 'managed' not in role: + raise ValueError('This role is not valid: %s' % role) + + @classmethod + def sanitize_role_name(cls, role_name: str) -> str: + """shortens too long strings if necessary""" + return str(role_name)[:cls._ROLE_NAME_MAX_CHARS] + + +def match_or_create_roles_from_names( + client: object, guild_id: int, role_names: list +) -> DiscordRoles: + """Shortcut for getting the result of matching role names as DiscordRoles object""" + return DiscordRoles.create_from_matched_roles( + client.match_or_create_roles_from_names( + guild_id=guild_id, role_names=role_names + ) + ) diff --git a/allianceauth/services/modules/discord/discord_client/tests/__init__.py b/allianceauth/services/modules/discord/discord_client/tests/__init__.py index e69de29b..41d75108 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/__init__.py +++ b/allianceauth/services/modules/discord/discord_client/tests/__init__.py @@ -0,0 +1,18 @@ +def create_role(id: int, name: str, managed=False): + return { + 'id': int(id), + 'name': str(name), + 'managed': bool(managed) + } + + +def create_matched_role(role, created=False) -> tuple: + return role, created + + +ROLE_ALPHA = create_role(1, 'alpha') +ROLE_BRAVO = create_role(2, 'bravo') +ROLE_CHARLIE = create_role(3, 'charlie') +ROLE_MIKE = create_role(13, 'mike', True) + +ALL_ROLES = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE] diff --git a/allianceauth/services/modules/discord/discord_client/tests/piloting_functionality.py b/allianceauth/services/modules/discord/discord_client/tests/piloting_functionality.py index 3cdc2a81..b20907c2 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/piloting_functionality.py +++ b/allianceauth/services/modules/discord/discord_client/tests/piloting_functionality.py @@ -50,10 +50,10 @@ class TestDiscordApiLive(TestCase): self.client.guild_name(DISCORD_GUILD_ID) sleep(RATE_LIMIT_DELAY_SECS) - self.client.match_guild_role_to_name(DISCORD_GUILD_ID, 'Testrole') + self.client.match_or_create_role_from_name(DISCORD_GUILD_ID, 'Testrole') sleep(RATE_LIMIT_DELAY_SECS) - self.client.match_guild_roles_to_names( + self.client.match_or_create_roles_from_names( DISCORD_GUILD_ID, ['Testrole A', 'Testrole B'] ) sleep(RATE_LIMIT_DELAY_SECS) diff --git a/allianceauth/services/modules/discord/discord_client/tests/piloting_tasks.py b/allianceauth/services/modules/discord/discord_client/tests/piloting_tasks.py deleted file mode 100755 index 551142a4..00000000 --- a/allianceauth/services/modules/discord/discord_client/tests/piloting_tasks.py +++ /dev/null @@ -1,47 +0,0 @@ -"""Load testing Discord services tasks - -This script will load test the Discord service tasks. -Note that his will run against your production Auth. -To run this test start a bunch of celery workers and then run this script directly. - -This script requires a user with a Discord account setup through Auth. -Please provide the respective Discord user ID by setting it as environment variable: - -export DISCORD_USER_ID="123456789" -""" - -import os -import sys - -myauth_dir = '/home/erik997/dev/python/aa/allianceauth-dev/myauth' -sys.path.insert(0, myauth_dir) - -import django # noqa: E402 - -# init and setup django project -os.environ.setdefault("DJANGO_SETTINGS_MODULE", "myauth.settings.local") -django.setup() - -from uuid import uuid1 # noqa: E402 - -from django.contrib.auth.models import User # noqa: E402 -# from allianceauth.services.modules.discord.tasks import update_groups # noqa: E402 - -if 'DISCORD_USER_ID' not in os.environ: - print('Please set DISCORD_USER_ID') - exit() - -DISCORD_USER_ID = os.environ['DISCORD_USER_ID'] - - -def run_many_updates(runs): - user = User.objects.get(discord__uid=DISCORD_USER_ID) - for _ in range(runs): - new_nick = f'Testnick {uuid1().hex}'[:32] - user.profile.main_character.character_name = new_nick - user.profile.main_character.save() - # update_groups.delay(user_pk=user.pk) - - -if __name__ == "__main__": - run_many_updates(20) diff --git a/allianceauth/services/modules/discord/discord_client/tests/test_client.py b/allianceauth/services/modules/discord/discord_client/tests/test_client.py index a070579a..84ef4121 100644 --- a/allianceauth/services/modules/discord/discord_client/tests/test_client.py +++ b/allianceauth/services/modules/discord/discord_client/tests/test_client.py @@ -9,6 +9,7 @@ from requests.exceptions import HTTPError from allianceauth import __title__ as AUTH_TITLE, __url__, __version__ +from . import ROLE_ALPHA, ROLE_BRAVO, ALL_ROLES, create_role, create_matched_role from ..client import DiscordClient, DURATION_CONTINGENCY, DEFAULT_BACKOFF_DELAY from ..exceptions import DiscordRateLimitExhausted, DiscordTooManyRequestsError from ...utils import set_logger_to_file @@ -71,13 +72,6 @@ class TestBasicsAndHelpers(TestCase): client = DiscordClient(TEST_BOT_TOKEN, mock_redis, is_rate_limited=True) self.assertTrue(client.is_rate_limited) - def test_sanitize_role_name(self): - client = DiscordClient(TEST_BOT_TOKEN, mock_redis) - role_name_input = 'x' * 110 - role_name_expected = 'x' * 100 - result = client._sanitize_role_name(role_name_input) - self.assertEqual(result, role_name_expected) - @patch(MODULE_PATH + '.caches') def test_use_default_redis_if_none_provided(self, mock_caches): my_redis = MagicMock(spec=Redis) @@ -125,25 +119,6 @@ class TestOtherMethods(TestCase): result = client.current_user() self.assertDictEqual(result, expected) - def test_guild_create_role(self, requests_mocker): - role_name_input = 'x' * 120 - role_name_used = 'x' * 100 - expected = {'name': role_name_used} - - def data_matcher(request): - return (json.loads(request.text) == expected) - - requests_mocker.post( - f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', - request_headers=self.headers, - additional_matcher=data_matcher, - text=json.dumps(expected), - ) - result = self.client.create_guild_role( - guild_id=TEST_GUILD_ID, role_name=role_name_input - ) - self.assertDictEqual(result, expected) - def test_get_infos(self, requests_mocker): expected = { 'id': TEST_GUILD_ID, @@ -157,19 +132,102 @@ class TestOtherMethods(TestCase): result = self.client.guild_infos(TEST_GUILD_ID) self.assertDictEqual(result, expected) - def test_get_roles(self, requests_mocker): + +@requests_mock.Mocker() +class TestGuildRoles(TestCase): + + def setUp(self): + self.url = f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' + + def test_without_cache(self, requests_mocker): expected = [ {'id': 1, 'name': 'alpha'}, {'id': 2, 'name': 'bravo'} ] - requests_mocker.get( - f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles', - request_headers=self.headers, + my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + requests_mocker.get( + url=self.url, + request_headers=DEFAULT_REQUEST_HEADERS, json=expected - ) - result = self.client.guild_roles(TEST_GUILD_ID) + ) + client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) + result = client.guild_roles(TEST_GUILD_ID, use_cache=False) self.assertListEqual(result, expected) + self.assertTrue(my_mock_redis.set.called) + def test_return_from_cache_if_in_cache(self, requests_mocker): + expected = [ + {'id': 1, 'name': 'alpha'}, + {'id': 2, 'name': 'bravo'} + ] + my_mock_redis = MagicMock(**{ + 'get.return_value': json.dumps(expected).encode('utf8') + }) + client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) + result = client.guild_roles(TEST_GUILD_ID) + self.assertEqual(result, expected) + self.assertFalse(my_mock_redis.set.called) + + def test_return_from_api_and_save_to_cache_if_not_in_cache( + self, requests_mocker + ): + expected = [ + {'id': 1, 'name': 'alpha'}, + {'id': 2, 'name': 'bravo'} + ] + my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + requests_mocker.get( + url=self.url, + request_headers=DEFAULT_REQUEST_HEADERS, + json=expected + ) + client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) + result = client.guild_roles(TEST_GUILD_ID) + self.assertEqual(result, expected) + self.assertTrue(my_mock_redis.set.called) + + def test_dont_save_in_cache_if_api_returns_invalid_response_1( + self, requests_mocker + ): + expected = {} + my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + requests_mocker.get( + url=self.url, + request_headers=DEFAULT_REQUEST_HEADERS, + json=expected + ) + client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) + result = client.guild_roles(TEST_GUILD_ID) + self.assertEqual(result, expected) + self.assertFalse(my_mock_redis.set.called) + + def test_dont_save_in_cache_if_api_returns_invalid_response_2( + self, requests_mocker + ): + expected = "api returns string" + my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + requests_mocker.get( + url=self.url, + request_headers=DEFAULT_REQUEST_HEADERS, + json=expected + ) + client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) + result = client.guild_roles(TEST_GUILD_ID) + self.assertEqual(result, expected) + self.assertFalse(my_mock_redis.set.called) + @requests_mock.Mocker() class TestGuildMember(TestCase): @@ -243,37 +301,86 @@ class TestGuildGetName(TestCase): self.assertFalse(my_mock_redis.set.called) +@requests_mock.Mocker() +class TestCreateGuildRole(TestCase): + + def setUp(self): + self.request_url = f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' + self.my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + self.client = DiscordClient2(TEST_BOT_TOKEN, self.my_mock_redis) + + def test_guild_create_role_normal(self, requests_mocker): + role_name_input = 'x' * 120 + role_name_used = 'x' * 100 + expected = {'name': role_name_used} + + def data_matcher(request): + return (json.loads(request.text) == expected) + + requests_mocker.post( + self.request_url, + request_headers=DEFAULT_REQUEST_HEADERS, + additional_matcher=data_matcher, + text=json.dumps(expected), + ) + result = self.client.create_guild_role( + guild_id=TEST_GUILD_ID, role_name=role_name_input + ) + self.assertDictEqual(result, expected) + self.assertTrue(self.my_mock_redis.delete.called) + + def test_guild_create_role_empty_response(self, requests_mocker): + expected = {} + requests_mocker.post( + self.request_url, + request_headers=DEFAULT_REQUEST_HEADERS, + text=json.dumps(expected), + ) + result = self.client.create_guild_role( + guild_id=TEST_GUILD_ID, role_name='dummy' + ) + self.assertDictEqual(result, expected) + self.assertFalse(self.my_mock_redis.delete.called) + + @requests_mock.Mocker() class TestGuildDeleteRole(TestCase): - def setUp(self): - self.access_token = 'accesstoken' - self.headers = DEFAULT_REQUEST_HEADERS + def setUp(self): self.request_url = \ - f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles/{TEST_ROLE_ID}' - self.client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + f'{API_BASE_URL}guilds/{TEST_GUILD_ID}/roles/{TEST_ROLE_ID}' + self.my_mock_redis = MagicMock(**{ + 'get.return_value': None, + 'pttl.return_value': -1, + }) + self.client = DiscordClient2(TEST_BOT_TOKEN, self.my_mock_redis) def test_guild_delete_role_success(self, requests_mocker): requests_mocker.delete( self.request_url, - request_headers=self.headers, + request_headers=DEFAULT_REQUEST_HEADERS, status_code=204 ) result = self.client.delete_guild_role( guild_id=TEST_GUILD_ID, role_id=TEST_ROLE_ID ) self.assertTrue(result) + self.assertTrue(self.my_mock_redis.delete.called) def test_guild_delete_role_failed(self, requests_mocker): requests_mocker.delete( self.request_url, - request_headers=self.headers, + request_headers=DEFAULT_REQUEST_HEADERS, status_code=200 ) result = self.client.delete_guild_role( guild_id=TEST_GUILD_ID, role_id=TEST_ROLE_ID ) self.assertFalse(result) + self.assertFalse(self.my_mock_redis.delete.called) @requests_mock.Mocker() @@ -735,141 +842,98 @@ class TestGuildMemberRemoveRole(TestCase): @patch(MODULE_PATH + '.DiscordClient.create_guild_role') @patch(MODULE_PATH + '.DiscordClient.guild_roles') -class TestGuildGetOrCreateRoles(TestCase): +class TestMatchGuildRolesToName(TestCase): - def test_return_id_if_role_in_cache( - self, mock_guild_get_roles, mock_guild_create_role, - ): - role_name = 'alpha' - my_mock_redis = MagicMock(**{'get.return_value': b'1'}) - client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) - mock_guild_get_roles.side_effect = RuntimeError - mock_guild_create_role.side_effect = RuntimeError - - expected = ({'id': 1, 'name': 'alpha'}, False) - result = client.match_guild_role_to_name(TEST_GUILD_ID, role_name) - self.assertEqual(result, expected) - - def test_return_id_for_role_known_by_api( - self, mock_guild_get_roles, mock_guild_create_role, - ): - my_mock_redis = MagicMock(**{'get.return_value': None}) - client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) - mock_guild_get_roles.return_value = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] - mock_guild_create_role.side_effect = RuntimeError - - expected = ({'id': 1, 'name': 'alpha'}, False) - result = client.match_guild_role_to_name(TEST_GUILD_ID, 'alpha') - self.assertEqual(result, expected) - - expected = ({'id': 2, 'name': 'bravo'}, False) - result = client.match_guild_role_to_name(TEST_GUILD_ID, 'bravo') - self.assertEqual(result, expected) - - @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', False) - def test_create_role_for_role_not_known_by_api( + def test_return_role_if_known( self, mock_guild_get_roles, mock_guild_create_role, ): - my_mock_redis = MagicMock(**{'get.return_value': None}) - client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) - mock_guild_get_roles.return_value = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] - mock_guild_create_role.return_value = {'id': 3, 'name': 'charlie'} - - expected = ({'id': 3, 'name': 'charlie'}, True) - result = client.match_guild_role_to_name(TEST_GUILD_ID, 'charlie') + role_name = 'alpha' + mock_guild_get_roles.return_value = ALL_ROLES + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name) + expected = (ROLE_ALPHA, False) self.assertEqual(result, expected) + self.assertFalse(mock_guild_create_role.called) + + def test_create_role_if_not_known_and_return_it( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_name = 'echo' + new_role = create_role(5, 'echo') + mock_guild_get_roles.return_value = ALL_ROLES + mock_guild_create_role.return_value = new_role + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name) + expected = (new_role, True) + self.assertEqual(result, expected) + self.assertTrue(mock_guild_create_role.called) @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', True) def test_return_none_if_role_creation_is_disabled( self, mock_guild_get_roles, mock_guild_create_role, ): - my_mock_redis = MagicMock(**{'get.return_value': None}) - client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) - mock_guild_get_roles.return_value = [ - {'id': 1, 'name': 'alpha'}, - {'id': 2, 'name': 'bravo'} - ] - mock_guild_create_role.return_value = {'id': 3, 'name': 'charlie'} - - result = client.match_guild_role_to_name(TEST_GUILD_ID, 'charlie') - self.assertIsNone(result[0]) - self.assertFalse(result[1]) - - def test_return_ids_if_role_in_cache( - self, mock_guild_get_roles, mock_guild_create_role, - ): - def my_cache_get(name): - map = { - DiscordClient._role_cache_key(TEST_GUILD_ID, 'alpha'): b'1', - DiscordClient._role_cache_key(TEST_GUILD_ID, 'bravo'): b'2', - DiscordClient._role_cache_key(TEST_GUILD_ID, 'charlie'): b'3' - } - if name in map: - return map[name] - else: - return None - - my_mock_redis = MagicMock(**{'get.side_effect': my_cache_get}) - client = DiscordClient2(TEST_BOT_TOKEN, my_mock_redis) - mock_guild_get_roles.side_effect = RuntimeError - mock_guild_create_role.side_effect = RuntimeError - - expected = [ - ({'id': 1, 'name': 'alpha'}, False), ({'id': 3, 'name': 'charlie'}, False) - ] - result = client.match_guild_roles_to_names(TEST_GUILD_ID, ['alpha', 'charlie']) - self.assertEqual(result, expected) - - @patch(MODULE_PATH + '.DiscordClient.match_guild_role_to_name') - def test_ignore_none_roles_in_guild_get_or_create_roles( - self, - mock_guild_get_or_create_role, - mock_guild_get_roles, - mock_guild_create_role, - ): - def my_guild_get_or_create_role(guild_id, role_name): - if role_name == 'alpha': - return {'id': 1, 'name': 'alpha'}, False - elif role_name == 'charlie': - return None, False - else: - raise ValueError('Unknown role') - - mock_guild_get_or_create_role.side_effect = my_guild_get_or_create_role - + role_name = 'echo' + mock_guild_get_roles.return_value = ALL_ROLES client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) - result = client.match_guild_roles_to_names(TEST_GUILD_ID, ['alpha', 'charlie']) - expected = [ - ({'id': 1, 'name': 'alpha'}, False), - ] + result = client.match_or_create_role_from_name(TEST_GUILD_ID, role_name) + expected = (None, False) self.assertEqual(result, expected) - - -class TestUpdateRoleCache(TestCase): + self.assertFalse(mock_guild_create_role.called) - def test_can_update_cache(self): - my_mock_redis = MagicMock() - client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis) - role = {'id': 1, 'name': 'alpha'} - client._update_role_cache(TEST_GUILD_ID, role) - self.assertTrue(my_mock_redis.set.called) - - def test_raises_exception_if_wrong_role_type(self): - my_mock_redis = MagicMock() - client = DiscordClient(TEST_BOT_TOKEN, my_mock_redis) - role = 'abc' + def test_raise_exception_if_name_has_invalid_type( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_name = ['echo'] + mock_guild_get_roles.return_value = ALL_ROLES + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) with self.assertRaises(TypeError): - client._update_role_cache(TEST_GUILD_ID, role) - - self.assertFalse(my_mock_redis.set.called) + client.match_or_create_role_from_name(TEST_GUILD_ID, role_name) +@patch(MODULE_PATH + '.DiscordClient.create_guild_role') +@patch(MODULE_PATH + '.DiscordClient.guild_roles') +class TestMatchGuildRolesToNames(TestCase): + + def test_return_roles_if_known( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_names = ['alpha', 'bravo'] + mock_guild_get_roles.return_value = ALL_ROLES + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) + expected = [create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO)] + self.assertEqual(result, expected) + self.assertFalse(mock_guild_create_role.called) + + def test_return_roles_if_known_and_create_if_not_known( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_names = ['alpha', 'echo'] + new_role = create_role(5, 'echo') + mock_guild_get_roles.return_value = ALL_ROLES + mock_guild_create_role.return_value = new_role + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) + expected = \ + [create_matched_role(ROLE_ALPHA), create_matched_role(new_role, True)] + self.assertEqual(result, expected) + self.assertTrue(mock_guild_create_role.called) + + @patch(MODULE_PATH + '.DISCORD_DISABLE_ROLE_CREATION', True) + def test_exclude_non_roles_from_result_list( + self, mock_guild_get_roles, mock_guild_create_role, + ): + role_names = ['alpha', 'echo'] + new_role = create_role(5, 'echo') + mock_guild_get_roles.return_value = ALL_ROLES + mock_guild_create_role.return_value = new_role + client = DiscordClient2(TEST_BOT_TOKEN, mock_redis) + result = client.match_or_create_roles_from_names(TEST_GUILD_ID, role_names) + expected = [create_matched_role(ROLE_ALPHA)] + self.assertEqual(result, expected) + self.assertFalse(mock_guild_create_role.called) + + class TestApiRequestBasics(TestCase): def setUp(self): diff --git a/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py b/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py new file mode 100644 index 00000000..9edf0e1a --- /dev/null +++ b/allianceauth/services/modules/discord/discord_client/tests/test_helpers.py @@ -0,0 +1,238 @@ +from unittest import TestCase + +from . import ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ALL_ROLES, create_role +from .. import DiscordRoles + + +MODULE_PATH = 'allianceauth.services.modules.discord.discord_client.client' + + +class TestDiscordRoles(TestCase): + + def setUp(self): + self.all_roles = DiscordRoles(ALL_ROLES) + + def test_can_create_simple(self): + roles_raw = [ROLE_ALPHA] + roles = DiscordRoles(roles_raw) + self.assertListEqual(list(roles), roles_raw) + + def test_can_create_empty(self): + roles_raw = [] + roles = DiscordRoles(roles_raw) + self.assertListEqual(list(roles), []) + + def test_raises_exception_if_roles_raw_of_wrong_type(self): + with self.assertRaises(TypeError): + DiscordRoles({'id': 1}) + + def test_raises_exception_if_list_contains_non_dict(self): + roles_raw = [ROLE_ALPHA, 'not_valid'] + with self.assertRaises(TypeError): + DiscordRoles(roles_raw) + + def test_raises_exception_if_invalid_role_1(self): + roles_raw = [{'name': 'alpha', 'managed': False}] + with self.assertRaises(ValueError): + DiscordRoles(roles_raw) + + def test_raises_exception_if_invalid_role_2(self): + roles_raw = [{'id': 1, 'managed': False}] + with self.assertRaises(ValueError): + DiscordRoles(roles_raw) + + def test_raises_exception_if_invalid_role_3(self): + roles_raw = [{'id': 1, 'name': 'alpha'}] + with self.assertRaises(ValueError): + DiscordRoles(roles_raw) + + def test_roles_are_equal(self): + roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_b = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + self.assertEqual(roles_a, roles_b) + + def test_roles_are_not_equal(self): + roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_b = DiscordRoles([ROLE_ALPHA]) + self.assertNotEqual(roles_a, roles_b) + + def test_different_objects_are_not_equal(self): + roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + self.assertFalse(roles_a == "invalid") + + def test_len(self): + self.assertEqual(len(self.all_roles), 4) + + def test_contains(self): + self.assertTrue(1 in self.all_roles) + self.assertFalse(99 in self.all_roles) + + def test_sanitize_role_name(self): + role_name_input = 'x' * 110 + role_name_expected = 'x' * 100 + result = DiscordRoles.sanitize_role_name(role_name_input) + self.assertEqual(result, role_name_expected) + + def test_objects_are_hashable(self): + roles_a = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_b = DiscordRoles([ROLE_BRAVO, ROLE_ALPHA]) + roles_c = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE]) + self.assertIsNotNone(hash(roles_a)) + self.assertEqual(hash(roles_a), hash(roles_b)) + self.assertNotEqual(hash(roles_a), hash(roles_c)) + + def test_create_from_matched_roles(self): + matched_roles = [ + (ROLE_ALPHA, True), + (ROLE_BRAVO, False) + ] + roles = DiscordRoles.create_from_matched_roles(matched_roles) + self.assertSetEqual(roles.ids(), {1, 2}) + + +class TestIds(TestCase): + + def setUp(self): + self.all_roles = DiscordRoles(ALL_ROLES) + + def test_return_role_ids_default(self): + result = self.all_roles.ids() + expected = {1, 2, 3, 13} + self.assertSetEqual(result, expected) + + def test_return_role_ids_empty(self): + roles = DiscordRoles([]) + self.assertSetEqual(roles.ids(), set()) + + +class TestSubset(TestCase): + + def setUp(self): + self.all_roles = DiscordRoles(ALL_ROLES) + + def test_ids_only(self): + role_ids = {1, 3} + roles_subset = self.all_roles.subset(role_ids) + expected = {1, 3} + self.assertSetEqual(roles_subset.ids(), expected) + + def test_ids_as_string_work_too(self): + role_ids = {'1', '3'} + roles_subset = self.all_roles.subset(role_ids) + expected = {1, 3} + self.assertSetEqual(roles_subset.ids(), expected) + + def test_managed_only(self): + roles = self.all_roles.subset(managed_only=True) + expected = {13} + self.assertSetEqual(roles.ids(), expected) + + def test_ids_and_managed_only(self): + role_ids = {1, 3, 13} + roles_subset = self.all_roles.subset(role_ids, managed_only=True) + expected = {13} + self.assertSetEqual(roles_subset.ids(), expected) + + def test_ids_are_empty(self): + roles = self.all_roles.subset([]) + expected = set() + self.assertSetEqual(roles.ids(), expected) + + def test_no_parameters(self): + roles = self.all_roles.subset() + expected = {1, 2, 3, 13} + self.assertSetEqual(roles.ids(), expected) + + +class TestHasRoles(TestCase): + + def setUp(self): + self.all_roles = DiscordRoles(ALL_ROLES) + + def test_true_if_all_roles_exit(self): + self.assertTrue(self.all_roles.has_roles([1, 2])) + + def test_true_if_all_roles_exit_str(self): + self.assertTrue(self.all_roles.has_roles(['1', '2'])) + + def test_false_if_role_does_not_exit(self): + self.assertFalse(self.all_roles.has_roles([99])) + + def test_false_if_one_role_does_not_exit(self): + self.assertFalse(self.all_roles.has_roles([1, 99])) + + def test_true_for_empty_roles(self): + self.assertTrue(self.all_roles.has_roles([])) + + +class TestGetMatchingRolesByName(TestCase): + + def setUp(self): + self.all_roles = DiscordRoles(ALL_ROLES) + + def test_return_role_if_matches(self): + role_name = 'alpha' + expected = ROLE_ALPHA + result = self.all_roles.role_by_name(role_name) + self.assertEqual(result, expected) + + def test_return_role_if_matches_and_limit_max_length(self): + role_name = 'x' * 120 + expected = create_role(77, 'x' * 100) + roles = DiscordRoles([expected]) + result = roles.role_by_name(role_name) + self.assertEqual(result, expected) + + def test_return_empty_if_not_matches(self): + role_name = 'lima' + expected = {} + result = self.all_roles.role_by_name(role_name) + self.assertEqual(result, expected) + + +class TestUnion(TestCase): + + def test_distinct_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_CHARLIE, ROLE_MIKE]) + roles_3 = roles_1.union(roles_2) + expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE]) + self.assertEqual(roles_3, expected) + + def test_overlapping_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_BRAVO, ROLE_MIKE]) + roles_3 = roles_1.union(roles_2) + expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE]) + self.assertEqual(roles_3, expected) + + def test_identical_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_3 = roles_1.union(roles_2) + expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + self.assertEqual(roles_3, expected) + + +class TestDifference(TestCase): + + def test_distinct_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_CHARLIE, ROLE_MIKE]) + roles_3 = roles_1.difference(roles_2) + expected = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + self.assertEqual(roles_3, expected) + + def test_overlapping_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_BRAVO, ROLE_MIKE]) + roles_3 = roles_1.difference(roles_2) + expected = DiscordRoles([ROLE_ALPHA]) + self.assertEqual(roles_3, expected) + + def test_identical_sets(self): + roles_1 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_2 = DiscordRoles([ROLE_ALPHA, ROLE_BRAVO]) + roles_3 = roles_1.difference(roles_2) + expected = DiscordRoles([]) + self.assertEqual(roles_3, expected) diff --git a/allianceauth/services/modules/discord/managers.py b/allianceauth/services/modules/discord/managers.py index dcc586f4..f11b79c1 100644 --- a/allianceauth/services/modules/discord/managers.py +++ b/allianceauth/services/modules/discord/managers.py @@ -20,6 +20,7 @@ from .app_settings import ( DISCORD_SYNC_NAMES ) from .discord_client import DiscordClient, DiscordApiBackoff +from .discord_client.helpers import match_or_create_roles_from_names from .utils import LoggerAddTag @@ -62,10 +63,12 @@ class DiscordUserManager(models.Manager): user_id = discord_user['id'] bot_client = self._bot_client(is_rate_limited=is_rate_limited) - if group_names: - role_ids = self.model._guild_get_or_create_role_ids( - bot_client, group_names - ) + if group_names: + role_ids = match_or_create_roles_from_names( + client=bot_client, + guild_id=DISCORD_GUILD_ID, + role_names=group_names + ).ids() else: role_ids = None diff --git a/allianceauth/services/modules/discord/models.py b/allianceauth/services/modules/discord/models.py index 06db360c..6c280b3e 100644 --- a/allianceauth/services/modules/discord/models.py +++ b/allianceauth/services/modules/discord/models.py @@ -10,7 +10,8 @@ from allianceauth.notifications import notify from . import __title__ from .app_settings import DISCORD_GUILD_ID -from .discord_client import DiscordClient, DiscordApiBackoff +from .discord_client import DiscordApiBackoff, DiscordRoles +from .discord_client.helpers import match_or_create_roles_from_names from .managers import DiscordUserManager from .utils import LoggerAddTag @@ -99,23 +100,58 @@ class DiscordUser(models.Model): - True on success - None if user is no longer a member of the Discord server - False on error or raises exception - """ - role_names = DiscordUser.objects.user_group_names(self.user) - client = DiscordUser.objects._bot_client() - requested_role_ids = self._guild_get_or_create_role_ids(client, role_names) - logger.debug( - 'Requested to update groups for user %s: %s', self.user, requested_role_ids - ) - success = client.modify_guild_member( - guild_id=DISCORD_GUILD_ID, - user_id=self.uid, - role_ids=requested_role_ids - ) - if success: - logger.info('Groups for %s have been updated', self.user) + """ + client = DiscordUser.objects._bot_client() + member_info = client.guild_member(guild_id=DISCORD_GUILD_ID, user_id=self.uid) + if member_info is None: + # User is no longer a member + return None + + guild_roles = DiscordRoles(client.guild_roles(guild_id=DISCORD_GUILD_ID)) + logger.debug('Current guild roles: %s', guild_roles.ids()) + if 'roles' in member_info: + if not guild_roles.has_roles(member_info['roles']): + guild_roles = DiscordRoles( + client.guild_roles(guild_id=DISCORD_GUILD_ID, use_cache=False) + ) + if not guild_roles.has_roles(member_info['roles']): + raise RuntimeError( + 'Member %s has unknown roles: %s' % ( + self.user, + set(member_info['roles']).difference(guild_roles.ids()) + ) + ) + member_roles = guild_roles.subset(member_info['roles']) else: - logger.warning('Failed to update groups for %s', self.user) - return success + raise RuntimeError('member_info from %s is not valid' % self.user) + + requested_roles = match_or_create_roles_from_names( + client=client, + guild_id=DISCORD_GUILD_ID, + role_names=DiscordUser.objects.user_group_names(self.user) + ) + logger.debug( + 'Requested roles for user %s: %s', self.user, requested_roles.ids() + ) + logger.debug('Current roles user %s: %s', self.user, member_roles.ids()) + member_roles_managed = member_roles.subset(managed_only=True) + if requested_roles != member_roles.difference(member_roles_managed): + logger.debug('Need to update roles for user %s', self.user) + new_roles = requested_roles.union(member_roles_managed) + success = client.modify_guild_member( + guild_id=DISCORD_GUILD_ID, + user_id=self.uid, + role_ids=list(new_roles.ids()) + ) + if success: + logger.info('Groups for %s have been updated', self.user) + else: + logger.warning('Failed to update groups for %s', self.user) + return success + + else: + logger.info('No need to update groups for user %s', self.user) + return True def update_username(self) -> bool: """Updates the username incl. the discriminator @@ -196,14 +232,3 @@ class DiscordUser(models.Model): 'Failed to remove user %s from Discord server: %s', self.user, ex ) return False - - @staticmethod - def _guild_get_or_create_role_ids(client: DiscordClient, role_names: list) -> list: - """wrapper for DiscordClient.match_guild_roles_to_names() - that only returns the list of IDs - """ - return [ - x[0]['id'] for x in client.match_guild_roles_to_names( - guild_id=DISCORD_GUILD_ID, role_names=role_names - ) - ] diff --git a/allianceauth/services/modules/discord/templates/services/discord/discord_service_ctrl.html b/allianceauth/services/modules/discord/templates/services/discord/discord_service_ctrl.html index 988832f3..5fe4f0fa 100644 --- a/allianceauth/services/modules/discord/templates/services/discord/discord_service_ctrl.html +++ b/allianceauth/services/modules/discord/templates/services/discord/discord_service_ctrl.html @@ -15,20 +15,20 @@ {% if not user_has_account %} - + {% else %} - + - + {% endif %} {% if request.user.is_superuser %}
- + {% trans "Link Discord Server" %}
diff --git a/allianceauth/services/modules/discord/tests/__init__.py b/allianceauth/services/modules/discord/tests/__init__.py index 9b2350f4..6e307de0 100644 --- a/allianceauth/services/modules/discord/tests/__init__.py +++ b/allianceauth/services/modules/discord/tests/__init__.py @@ -1,5 +1,6 @@ -from django.contrib.auth.models import Group, Permission +from django.contrib.auth.models import Group from allianceauth.tests.auth_utils import AuthUtils +from ..discord_client.tests import create_role DEFAULT_AUTH_GROUP = 'Member' MODULE_PATH = 'allianceauth.services.modules.discord' @@ -10,8 +11,13 @@ TEST_USER_NAME = 'Peter Parker' TEST_MAIN_NAME = 'Spiderman' TEST_MAIN_ID = 1005 +ROLE_ALPHA = create_role(1, 'alpha') +ROLE_BRAVO = create_role(2, 'bravo') +ROLE_CHARLIE = create_role(3, 'charlie') +ROLE_MIKE = create_role(13, 'mike', True) + def add_permissions_to_members(): - permission = Permission.objects.get(codename='access_discord') + permission = AuthUtils.get_permission_by_name('discord.access_discord') members = Group.objects.get_or_create(name=DEFAULT_AUTH_GROUP)[0] AuthUtils.add_permissions_to_groups([permission], [members]) diff --git a/allianceauth/services/modules/discord/tests/piloting_tasks.py b/allianceauth/services/modules/discord/tests/piloting_tasks.py new file mode 100755 index 00000000..60494c8b --- /dev/null +++ b/allianceauth/services/modules/discord/tests/piloting_tasks.py @@ -0,0 +1,83 @@ +# flake8: noqa + +"""Concurrency testing Discord service tasks + +This script will run many Discord service tasks in parallel to test concurrency +Note that it will run against your main Auth database and not test! + +Check allianceauth.log for the results. + +To run this test start a bunch of celery workers and then run this script directly. +Make sure to also set the environment variable AUTH_PROJECT_PATH to your Auth path +and DJANGO_SETTINGS_MODULE to the relative location of your settings: + +Example: +export AUTH_PROJECT_PATH="/home/erik997/dev/python/aa/allianceauth-dev/myauth" +export DJANGO_SETTINGS_MODULE="myauth.settings.local" + +Careful: This script will utilize all existing Discord users and make changes! +""" +# start django project +import os +import sys +if not 'AUTH_PROJECT_PATH' in os.environ: + print('AUTH_PROJECT_PATH is not set') + exit(1) +if not 'DJANGO_SETTINGS_MODULE' in os.environ: + print('DJANGO_SETTINGS_MODULE is not set') + exit(1) +sys.path.insert(0, os.environ['AUTH_PROJECT_PATH']) +import django +django.setup() + +# normal imports +import logging +from uuid import uuid1 +import random + +from django.core.cache import caches +from django.contrib.auth.models import User, Group + +from allianceauth.services.modules.discord.models import DiscordUser + +logger = logging.getLogger('allianceauth') +MAX_RUNS = 3 + +def clear_cache(): + default_cache = caches['default'] + redis = default_cache.get_master_client() + redis.flushall() + logger.info('Cache flushed') + +def run_many_updates(runs): + logger.info('Starting piloting_tasks for %d runs', runs) + users = list() + all_groups = Group.objects.all() + for i in range(runs): + if not users: + users = list(User.objects.filter(discord__isnull=False)) + user = users.pop() + logger.info('%d/%d: Starting run with user %s', i + 1, runs, user) + # force change of nick + new_nick = f'Testnick {uuid1().hex}'[:32] + logger.info( + '%d/%d: Changing nickname of %s to "%s"', i + 1, runs, user, new_nick + ) + user.profile.main_character.character_name = new_nick + user.profile.main_character.save() + + # force change of groups + user_groups = user.groups.all() + user.groups.remove(random.choice(user_groups)) + while True: + new_group = random.choice(all_groups) + if new_group not in user_groups: + break + logger.info('%d/%d: Adding group "%s" to user %s', i + 1, runs, new_group, user) + user.groups.add(new_group) + + logger.info('All %d runs have been started', runs) + +if __name__ == "__main__": + clear_cache() + run_many_updates(MAX_RUNS) diff --git a/allianceauth/services/modules/discord/tests/test_auth_hooks.py b/allianceauth/services/modules/discord/tests/test_auth_hooks.py index e06a1d97..0853d3df 100644 --- a/allianceauth/services/modules/discord/tests/test_auth_hooks.py +++ b/allianceauth/services/modules/discord/tests/test_auth_hooks.py @@ -7,7 +7,8 @@ from allianceauth.tests.auth_utils import AuthUtils from . import TEST_USER_NAME, TEST_USER_ID, add_permissions_to_members, MODULE_PATH from ..auth_hooks import DiscordService -from ..models import DiscordUser, DiscordClient +from ..discord_client import DiscordClient +from ..models import DiscordUser from ..utils import set_logger_to_file diff --git a/allianceauth/services/modules/discord/tests/test_integration.py b/allianceauth/services/modules/discord/tests/test_integration.py index 727c1e7e..33acdc4a 100644 --- a/allianceauth/services/modules/discord/tests/test_integration.py +++ b/allianceauth/services/modules/discord/tests/test_integration.py @@ -1,19 +1,211 @@ -from django_webtest import WebTest -from unittest.mock import patch +"""Integration tests +Testing all components of the service, with the exception of the Discord API. + +Please note that these tests require Redis and will flush it +""" +from collections import namedtuple +import logging +from unittest.mock import patch +from uuid import uuid1 + +from django_webtest import WebTest +import requests_mock + +from django.contrib.auth.models import Group, User +from django.core.cache import caches from django.shortcuts import reverse +from django.test import TransactionTestCase +from django.test.utils import override_settings from allianceauth.tests.auth_utils import AuthUtils from . import ( - add_permissions_to_members, - MODULE_PATH, - TEST_USER_NAME, - TEST_MAIN_NAME, - TEST_MAIN_ID + TEST_GUILD_ID, + TEST_USER_NAME, + TEST_USER_ID, + TEST_MAIN_NAME, + TEST_MAIN_ID, + MODULE_PATH, + add_permissions_to_members, + ROLE_ALPHA, + ROLE_BRAVO, + ROLE_CHARLIE, + ROLE_MIKE, + create_role +) +from ..discord_client.app_settings import DISCORD_API_BASE_URL +from ..models import DiscordUser + +logger = logging.getLogger('allianceauth') + +ROLE_MEMBER = create_role(99, 'Member') + +# Putting all requests to Discord into objects so we can compare them better +DiscordRequest = namedtuple('DiscordRequest', ['method', 'url']) +guild_roles_request = DiscordRequest( + method='GET', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' +) +create_guild_role_request = DiscordRequest( + method='POST', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/roles' +) +guild_member_request = DiscordRequest( + method='GET', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' +) +modify_guild_member_request = DiscordRequest( + method='PATCH', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' +) +remove_guild_member_request = DiscordRequest( + method='DELETE', + url=f'{DISCORD_API_BASE_URL}guilds/{TEST_GUILD_ID}/members/{TEST_USER_ID}' ) +def clear_cache(): + default_cache = caches['default'] + redis = default_cache.get_master_client() + redis.flushall() + logger.info('Cache flushed') + + +@patch(MODULE_PATH + '.models.DISCORD_GUILD_ID', TEST_GUILD_ID) +@override_settings(CELERY_ALWAYS_EAGER=True) +@requests_mock.Mocker() +class TestServiceFeatures(TransactionTestCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.maxDiff = None + + def setUp(self): + clear_cache() + AuthUtils.disconnect_signals() + Group.objects.all().delete() + User.objects.all().delete() + AuthUtils.connect_signals() + self.group_3 = Group.objects.create(name='charlie') + self.user = AuthUtils.create_member(TEST_USER_NAME) + AuthUtils.add_main_character_2( + self.user, + TEST_MAIN_NAME, + TEST_MAIN_ID, + corp_id='2', + corp_name='test_corp', + corp_ticker='TEST', + disconnect_signals=True + ) + self.discord_user = DiscordUser.objects.create(user=self.user, uid=TEST_USER_ID) + add_permissions_to_members() + + def test_name_of_main_changes(self, requests_mocker): + # modify_guild_member() + requests_mocker.patch(modify_guild_member_request.url, status_code=204) + + # changing nick to trigger signals + new_nick = f'Testnick {uuid1().hex}'[:32] + self.user.profile.main_character.character_name = new_nick + self.user.profile.main_character.save() + + # Need to have called modify_guild_member two times only + # Once for sync nickname + # Once for change of main character + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + expected = [modify_guild_member_request, modify_guild_member_request] + self.assertListEqual(requests_made, expected) + + def test_user_demoted_to_guest(self, requests_mocker): + # remove_guild_member() + requests_mocker.delete(remove_guild_member_request.url, status_code=204) + self.user.groups.clear() + + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + # compare the list of made requests with expected + expected = [remove_guild_member_request] + self.assertListEqual(requests_made, expected) + + def test_adding_group_to_user_role_exists(self, requests_mocker): + # guild_member() + requests_mocker.get( + guild_member_request.url, + json={ + 'user': {'id': str(TEST_USER_ID), 'username': TEST_MAIN_NAME}, + 'roles': ['1', '13', '99'] + } + ) + # guild_roles() + requests_mocker.get( + guild_roles_request.url, + json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE, ROLE_MEMBER] + ) + # create_guild_role() + requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE) + # modify_guild_member() + requests_mocker.patch(modify_guild_member_request.url, status_code=204) + + # adding new group to trigger signals + self.user.groups.add(self.group_3) + self.user.refresh_from_db() + + # compare the list of made requests with expected + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + expected = [ + guild_member_request, + guild_roles_request, + modify_guild_member_request + ] + self.assertListEqual(requests_made, expected) + + def test_adding_group_to_user_role_does_not_exist(self, requests_mocker): + # guild_member() + requests_mocker.get( + guild_member_request.url, + json={ + 'user': {'id': str(TEST_USER_ID), 'username': TEST_MAIN_NAME}, + 'roles': ['1', '13', '99'] + } + ) + # guild_roles() + requests_mocker.get( + guild_roles_request.url, + json=[ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE, ROLE_MEMBER] + ) + # create_guild_role() + requests_mocker.post(create_guild_role_request.url, json=ROLE_CHARLIE) + # modify_guild_member() + requests_mocker.patch(modify_guild_member_request.url, status_code=204) + + # adding new group to trigger signals + self.user.groups.add(self.group_3) + self.user.refresh_from_db() + + # compare the list of made requests with expected + requests_made = list() + for r in requests_mocker.request_history: + requests_made.append(DiscordRequest(r.method, r.url)) + + expected = [ + guild_member_request, + guild_roles_request, + create_guild_role_request, + modify_guild_member_request + ] + self.assertListEqual(requests_made, expected) + + class TestServiceUserActivation(WebTest): def setUp(self): diff --git a/allianceauth/services/modules/discord/tests/test_managers.py b/allianceauth/services/modules/discord/tests/test_managers.py index 7fcfda44..b4f7ef27 100644 --- a/allianceauth/services/modules/discord/tests/test_managers.py +++ b/allianceauth/services/modules/discord/tests/test_managers.py @@ -14,8 +14,12 @@ from . import ( TEST_USER_ID, TEST_MAIN_NAME, TEST_MAIN_ID, - MODULE_PATH + MODULE_PATH, + ROLE_ALPHA, + ROLE_BRAVO, + ROLE_CHARLIE ) +from ..discord_client.tests import create_matched_role from ..app_settings import ( DISCORD_APP_ID, DISCORD_APP_SECRET, @@ -32,7 +36,6 @@ logger = set_logger_to_file(MODULE_PATH + '.managers', __file__) @patch(MODULE_PATH + '.managers.DISCORD_GUILD_ID', TEST_GUILD_ID) @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient) @patch(MODULE_PATH + '.models.DiscordUser.objects._exchange_auth_code_for_token') -@patch(MODULE_PATH + '.models.DiscordUser.objects.model._guild_get_or_create_role_ids') @patch(MODULE_PATH + '.models.DiscordUser.objects.user_group_names') @patch(MODULE_PATH + '.models.DiscordUser.objects.user_formatted_nick') class TestAddUser(TestCase): @@ -50,16 +53,16 @@ class TestAddUser(TestCase): def test_can_create_user_no_roles_no_nick( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = None - mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = None + mock_user_group_names.return_value = [] mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.return_value = True result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -79,16 +82,20 @@ class TestAddUser(TestCase): self, mock_user_formatted_nick, mock_user_group_names, - mock_guild_get_or_create_role_ids, mock_exchange_auth_code_for_token, mock_DiscordClient ): - role_ids = [1, 2, 3] + roles = [ + create_matched_role(ROLE_ALPHA), + create_matched_role(ROLE_BRAVO), + create_matched_role(ROLE_CHARLIE) + ] mock_user_formatted_nick.return_value = None mock_user_group_names.return_value = ['a', 'b', 'c'] - mock_guild_get_or_create_role_ids.return_value = role_ids - mock_exchange_auth_code_for_token.return_value = self.access_token + mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = roles mock_DiscordClient.return_value.add_guild_member.return_value = True result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -101,7 +108,7 @@ class TestAddUser(TestCase): self.assertEqual(kwargs['guild_id'], TEST_GUILD_ID) self.assertEqual(kwargs['user_id'], TEST_USER_ID) self.assertEqual(kwargs['access_token'], self.access_token) - self.assertEqual(kwargs['role_ids'], role_ids) + self.assertSetEqual(set(kwargs['role_ids']), {1, 2, 3}) self.assertIsNone(kwargs['nick']) @patch(MODULE_PATH + '.managers.DISCORD_SYNC_NAMES', True) @@ -109,15 +116,15 @@ class TestAddUser(TestCase): self, mock_user_formatted_nick, mock_user_group_names, - mock_guild_get_or_create_role_ids, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = TEST_MAIN_NAME mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = [] mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.return_value = True result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -137,16 +144,16 @@ class TestAddUser(TestCase): def test_can_create_user_no_roles_and_without_nick_if_turned_off( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = TEST_MAIN_NAME mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = [] mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.return_value = True result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -165,16 +172,16 @@ class TestAddUser(TestCase): def test_can_activate_existing_guild_member( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = None - mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = None + mock_user_group_names.return_value = [] mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.return_value = None result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -187,16 +194,16 @@ class TestAddUser(TestCase): def test_return_false_when_user_creation_fails( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = None - mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = None + mock_user_group_names.return_value = [] mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.return_value = False result = DiscordUser.objects.add_user(self.user, authorization_code='abcdef') @@ -209,16 +216,16 @@ class TestAddUser(TestCase): def test_return_false_when_on_api_backoff( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = None mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = None mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_DiscordClient.return_value.add_guild_member.side_effect = \ DiscordApiBackoff(999) @@ -232,16 +239,16 @@ class TestAddUser(TestCase): def test_return_false_on_http_error( self, mock_user_formatted_nick, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_exchange_auth_code_for_token, mock_DiscordClient ): mock_user_formatted_nick.return_value = None mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = None mock_exchange_auth_code_for_token.return_value = self.access_token mock_DiscordClient.return_value.current_user.return_value = self.user_info + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = [] mock_exception = HTTPError('error') mock_exception.response = Mock() mock_exception.response.status_code = 500 diff --git a/allianceauth/services/modules/discord/tests/test_models.py b/allianceauth/services/modules/discord/tests/test_models.py index c809fbdc..268f6205 100644 --- a/allianceauth/services/modules/discord/tests/test_models.py +++ b/allianceauth/services/modules/discord/tests/test_models.py @@ -6,8 +6,19 @@ from django.test import TestCase from allianceauth.tests.auth_utils import AuthUtils -from . import TEST_USER_NAME, TEST_USER_ID, TEST_MAIN_NAME, TEST_MAIN_ID, MODULE_PATH +from . import ( + TEST_USER_NAME, + TEST_USER_ID, + TEST_MAIN_NAME, + TEST_MAIN_ID, + MODULE_PATH, + ROLE_ALPHA, + ROLE_BRAVO, + ROLE_CHARLIE, + ROLE_MIKE +) from ..discord_client import DiscordClient, DiscordApiBackoff +from ..discord_client.tests import create_matched_role from ..models import DiscordUser from ..utils import set_logger_to_file @@ -28,15 +39,6 @@ class TestBasicsAndHelpers(TestCase): discord_user = DiscordUser.objects.create(user=user, uid=TEST_USER_ID) expected = 'DiscordUser(user=\'Peter Parker\', uid=198765432012345678)' self.assertEqual(repr(discord_user), expected) - - def test_guild_get_or_create_role_ids(self): - mock_client = Mock(spec=DiscordClient) - mock_client.match_guild_roles_to_names.return_value = \ - [({'id': 1, 'name': 'alpha'}, True), ({'id': 2, 'name': 'bravo'}, True)] - - result = DiscordUser._guild_get_or_create_role_ids(mock_client, []) - excepted = [1, 2] - self.assertEqual(set(result), set(excepted)) @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient) @@ -235,61 +237,177 @@ class TestDeleteUser(TestCase): @patch(MODULE_PATH + '.managers.DiscordClient', spec=DiscordClient) -@patch(MODULE_PATH + '.models.DiscordUser._guild_get_or_create_role_ids') @patch(MODULE_PATH + '.models.DiscordUser.objects.user_group_names') class TestUpdateGroups(TestCase): - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.user = AuthUtils.create_user(TEST_USER_NAME) - def setUp(self): + self.user = AuthUtils.create_user(TEST_USER_NAME) self.discord_user = DiscordUser.objects.create( user=self.user, uid=TEST_USER_ID ) + self.guild_roles = [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE] + self.roles_requested = [ + create_matched_role(ROLE_ALPHA), create_matched_role(ROLE_BRAVO) + ] - def test_can_update( + def test_update_if_needed( self, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_DiscordClient - ): - roles_requested = [1, 2, 3] + ): + roles_current = [1] mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = roles_requested + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} mock_DiscordClient.return_value.modify_guild_member.return_value = True result = self.discord_user.update_groups() self.assertTrue(result) self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) + args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args + self.assertEqual(set(kwargs['role_ids']), {1, 2}) + + def test_update_if_needed_and_preserve_managed_roles( + self, + mock_user_group_names, + mock_DiscordClient + ): + roles_current = [1, 13] + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + mock_DiscordClient.return_value.modify_guild_member.return_value = True + + result = self.discord_user.update_groups() + self.assertTrue(result) + self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) + args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args + self.assertEqual(set(kwargs['role_ids']), {1, 2, 13}) + + def test_dont_update_if_not_needed( + self, + mock_user_group_names, + mock_DiscordClient + ): + roles_current = [1, 2, 13] + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + + result = self.discord_user.update_groups() + self.assertTrue(result) + self.assertFalse(mock_DiscordClient.return_value.modify_guild_member.called) + + def test_update_if_user_has_no_roles_on_discord( + self, + mock_user_group_names, + mock_DiscordClient + ): + roles_current = [] + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + mock_DiscordClient.return_value.modify_guild_member.return_value = True + + result = self.discord_user.update_groups() + self.assertTrue(result) + self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) + args, kwargs = mock_DiscordClient.return_value.modify_guild_member.call_args + self.assertEqual(set(kwargs['role_ids']), {1, 2}) def test_return_none_if_user_no_longer_a_member( - self, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + self, + mock_user_group_names, mock_DiscordClient - ): - roles_requested = [1, 2, 3] - mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = roles_requested - mock_DiscordClient.return_value.modify_guild_member.return_value = None + ): + mock_DiscordClient.return_value.guild_member.return_value = None result = self.discord_user.update_groups() self.assertIsNone(result) - self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) + self.assertFalse(mock_DiscordClient.return_value.modify_guild_member.called) def test_return_false_if_api_returns_false( self, - mock_user_group_names, - mock_guild_get_or_create_role_ids, + mock_user_group_names, mock_DiscordClient ): - roles_requested = [1, 2, 3] + roles_current = [1] mock_user_group_names.return_value = [] - mock_guild_get_or_create_role_ids.return_value = roles_requested - mock_DiscordClient.return_value.modify_guild_member.return_value = False + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + mock_DiscordClient.return_value.modify_guild_member.return_value = False result = self.discord_user.update_groups() self.assertFalse(result) self.assertTrue(mock_DiscordClient.return_value.modify_guild_member.called) + + def test_raise_exception_if_member_has_unknown_roles( + self, + mock_user_group_names, + mock_DiscordClient + ): + roles_current = [99] + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + mock_DiscordClient.return_value.modify_guild_member.return_value = True + + with self.assertRaises(RuntimeError): + self.discord_user.update_groups() + + def test_refresh_guild_roles_user_roles_dont_not_match( + self, + mock_user_group_names, + mock_DiscordClient + ): + def my_guild_roles(guild_id, use_cache=True): + if use_cache: + return [ROLE_ALPHA, ROLE_BRAVO, ROLE_MIKE] + else: + return [ROLE_ALPHA, ROLE_BRAVO, ROLE_CHARLIE, ROLE_MIKE] + + roles_current = [3] + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.side_effect = my_guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'roles': roles_current} + mock_DiscordClient.return_value.modify_guild_member.return_value = True + result = self.discord_user.update_groups() + self.assertTrue(result) + self.assertEqual(mock_DiscordClient.return_value.guild_roles.call_count, 2) + + def test_raise_exception_if_member_info_is_invalid( + self, + mock_user_group_names, + mock_DiscordClient + ): + mock_user_group_names.return_value = [] + mock_DiscordClient.return_value.match_or_create_roles_from_names\ + .return_value = self.roles_requested + mock_DiscordClient.return_value.guild_roles.return_value = self.guild_roles + mock_DiscordClient.return_value.guild_member.return_value = \ + {'user': 'dummy'} + mock_DiscordClient.return_value.modify_guild_member.return_value = True + + with self.assertRaises(RuntimeError): + self.discord_user.update_groups() diff --git a/allianceauth/services/modules/discord/tests/test_views.py b/allianceauth/services/modules/discord/tests/test_views.py index d3157c99..909d63df 100644 --- a/allianceauth/services/modules/discord/tests/test_views.py +++ b/allianceauth/services/modules/discord/tests/test_views.py @@ -7,7 +7,8 @@ from django.urls import reverse from allianceauth.tests.auth_utils import AuthUtils from . import MODULE_PATH, add_permissions_to_members, TEST_USER_NAME, TEST_USER_ID -from ..models import DiscordUser, DiscordClient +from ..discord_client import DiscordClient +from ..models import DiscordUser from ..utils import set_logger_to_file from ..views import ( discord_callback, diff --git a/docs/features/services/discord.md b/docs/features/services/discord.md index bc6205a5..76c61c86 100644 --- a/docs/features/services/discord.md +++ b/docs/features/services/discord.md @@ -10,7 +10,7 @@ Discord is very popular amongst ad-hoc small groups and larger organizations see ### Prepare Your Settings File -Make the following changing in your auth project's settings file (`local.py`): +Make the following changes in your auth project's settings file (`local.py`): - Add `'allianceauth.services.modules.discord',` to `INSTALLED_APPS` - Append the following to the bottom of the settings file: @@ -131,7 +131,7 @@ Name Description `DISCORD_BOT_TOKEN` Generated bot token for the Discord Auth app `''` `DISCORD_CALLBACK_URL` Oauth callback URL `''` `DISCORD_GUILD_ID` Discord ID of your Discord server `''` -`DISCORD_ROLES_CACHE_MAX_AGE` How long roles retrieved from the Discord server are caches locally in milliseconds `7200000` +`DISCORD_ROLES_CACHE_MAX_AGE` How long roles retrieved from the Discord server are cached locally in milliseconds `7200000` `DISCORD_SYNC_NAMES` When set to True the nicknames of Discord users will be set to the user's main character name `False` `DISCORD_TASKS_RETRY_PAUSE` Pause in seconds until next retry for tasks after an error occurred `60` `DISCORD_TASKS_MAX_RETRIES` max retries of tasks after an error occurred `3` diff --git a/tests/settings_all.py b/tests/settings_all.py index ade3edd5..f3d618c7 100644 --- a/tests/settings_all.py +++ b/tests/settings_all.py @@ -41,7 +41,15 @@ INSTALLED_APPS += [ ROOT_URLCONF = 'tests.urls' -CACHES['default'] = {'BACKEND': 'django.core.cache.backends.db.DatabaseCache'} +CACHES = { + "default": { + "BACKEND": "redis_cache.RedisCache", + "LOCATION": "localhost:6379", + "OPTIONS": { + "DB": 1, + } + } +} ######################## # XenForo Configuration diff --git a/tests/settings_core.py b/tests/settings_core.py index 84086fc4..554b53a8 100644 --- a/tests/settings_core.py +++ b/tests/settings_core.py @@ -24,7 +24,15 @@ INSTALLED_APPS += [ ROOT_URLCONF = 'tests.urls' -CACHES['default'] = {'BACKEND': 'django.core.cache.backends.db.DatabaseCache'} +CACHES = { + "default": { + "BACKEND": "redis_cache.RedisCache", + "LOCATION": "localhost:6379", + "OPTIONS": { + "DB": 1, + } + } +} PASSWORD_HASHERS = [ 'django.contrib.auth.hashers.MD5PasswordHasher',