Use Django's cache framework for service group names (#857)

Use django-redis-cache backend for locking get_or_set
No longer require group-related tasks to be locked to one simultaneous execution.
Remove legacy service group cache models.

Truncate Discord nicknames to 32 characters
Correct Discourse group name extension using only valid leading characters.
Prevent name slicing from ending with illegal character

Closes #801
Closes #847
Closes #835
Closes #852

services/modules/discord/manager.py

@@ -3,14 +3,13 @@ import requests
 import json
 import re
 from django.conf import settings
-from services.models import GroupCache
 from requests_oauthlib import OAuth2Session
 from functools import wraps
 import logging
 import datetime
 import time
-from django.utils import timezone
 from django.core.cache import cache
+from hashlib import md5
 
 logger = logging.getLogger(__name__)
 
@@ -20,9 +19,13 @@ EVE_IMAGE_SERVER = "https://image.eveonline.com"
 AUTH_URL = "https://discordapp.com/api/oauth2/authorize"
 TOKEN_URL = "https://discordapp.com/api/oauth2/token"
 
-# needs administrator, since Discord can't get their permissions system to work
-# was kick members, manage roles, manage nicknames
-#BOT_PERMISSIONS = 0x00000002 + 0x10000000 + 0x08000000
+"""
+Previously all we asked for was permission to kick members, manage roles, and manage nicknames.
+Users have reported weird unauthorized errors we don't understand. So now we ask for full server admin.
+It's almost fixed the problem.
+"""
+# kick members, manage roles, manage nicknames
+# BOT_PERMISSIONS = 0x00000002 + 0x10000000 + 0x08000000
 BOT_PERMISSIONS = 0x00000008
 
 # get user ID, accept invite
@@ -31,7 +34,7 @@ SCOPES = [
     'guilds.join',
 ]
 
-GROUP_CACHE_MAX_AGE = datetime.timedelta(minutes=30)
+GROUP_CACHE_MAX_AGE = int(getattr(settings, 'DISCORD_GROUP_CACHE_MAX_AGE', 2 * 60 * 60))  # 2 hours default
 
 
 class DiscordApiException(Exception):
@@ -150,10 +153,14 @@ class DiscordOAuthManager:
     def __init__(self):
         pass
 
+    @staticmethod
+    def _sanitize_name(name):
+        return re.sub('[^\w.-]', '', name)[:32]
+
     @staticmethod
     def _sanitize_groupname(name):
         name = name.strip(' _')
-        return re.sub('[^\w.-]', '', name)
+        return DiscordOAuthManager._sanitize_name(name)
 
     @staticmethod
     def generate_bot_add_url():
@@ -198,8 +205,9 @@ class DiscordOAuthManager:
     @staticmethod
     def update_nickname(user_id, nickname):
         try:
+            nickname = DiscordOAuthManager._sanitize_name(nickname)
             custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-            data = {'nick': nickname, }
+            data = {'nick': nickname}
             path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
             r = requests.patch(path, headers=custom_headers, json=data)
             logger.debug("Got status code %s after setting nickname for Discord user ID %s (%s)" % (
@@ -230,7 +238,7 @@ class DiscordOAuthManager:
             return False
 
     @staticmethod
-    def __get_groups():
+    def _get_groups():
         custom_headers = {'accept': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
         path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/roles"
         r = requests.get(path, headers=custom_headers)
@@ -239,41 +247,20 @@ class DiscordOAuthManager:
         return r.json()
 
     @staticmethod
-    def __update_group_cache():
-        GroupCache.objects.filter(service="discord").delete()
-        cache = GroupCache.objects.create(service="discord")
-        cache.groups = json.dumps(DiscordOAuthManager.__get_groups())
-        cache.save()
-        return cache
+    def _generate_cache_role_key(name):
+        return 'DISCORD_ROLE_NAME__%s' % md5(str(name).encode('utf-8')).hexdigest()
 
     @staticmethod
-    def __get_group_cache():
-        if not GroupCache.objects.filter(service="discord").exists():
-            DiscordOAuthManager.__update_group_cache()
-        cache = GroupCache.objects.get(service="discord")
-        age = timezone.now() - cache.created
-        if age > GROUP_CACHE_MAX_AGE:
-            logger.debug("Group cache has expired. Triggering update.")
-            cache = DiscordOAuthManager.__update_group_cache()
-        return json.loads(cache.groups)
+    def _group_name_to_id(name):
+        name = DiscordOAuthManager._sanitize_groupname(name)
 
-    @staticmethod
-    def __group_name_to_id(name):
-        cache = DiscordOAuthManager.__get_group_cache()
-        for g in cache:
-            if g['name'] == name:
-                return g['id']
-        logger.debug("Group %s not found on Discord. Creating" % name)
-        DiscordOAuthManager.__create_group(name)
-        return DiscordOAuthManager.__group_name_to_id(name)
-
-    @staticmethod
-    def __group_id_to_name(id):
-        cache = DiscordOAuthManager.__get_group_cache()
-        for g in cache:
-            if g['id'] == id:
-                return g['name']
-        raise KeyError("Group ID %s not found on Discord" % id)
+        def get_or_make_role():
+            groups = DiscordOAuthManager._get_groups()
+            for g in groups:
+                if g['name'] == name:
+                    return g['id']
+            return DiscordOAuthManager._create_group(name)['id']
+        return cache.get_or_set(DiscordOAuthManager._generate_cache_role_key(name), get_or_make_role, GROUP_CACHE_MAX_AGE)
 
     @staticmethod
     def __generate_role():
@@ -300,16 +287,15 @@ class DiscordOAuthManager:
         return r.json()
 
     @staticmethod
-    def __create_group(name):
+    def _create_group(name):
         role = DiscordOAuthManager.__generate_role()
-        DiscordOAuthManager.__edit_role(role['id'], name)
-        DiscordOAuthManager.__update_group_cache()
+        return DiscordOAuthManager.__edit_role(role['id'], name)
 
     @staticmethod
     @api_backoff
     def update_groups(user_id, groups):
         custom_headers = {'content-type': 'application/json', 'authorization': 'Bot ' + settings.DISCORD_BOT_TOKEN}
-        group_ids = [DiscordOAuthManager.__group_name_to_id(DiscordOAuthManager._sanitize_groupname(g)) for g in groups]
+        group_ids = [DiscordOAuthManager._group_name_to_id(DiscordOAuthManager._sanitize_groupname(g)) for g in groups]
         path = DISCORD_URL + "/guilds/" + str(settings.DISCORD_GUILD_ID) + "/members/" + str(user_id)
         data = {'roles': group_ids}
         r = requests.patch(path, headers=custom_headers, json=data)

services/modules/discord/tasks.py

@@ -10,7 +10,6 @@ from django.core.exceptions import ObjectDoesNotExist
 from eveonline.managers import EveManager
 from notifications import notify
 from services.modules.discord.manager import DiscordOAuthManager, DiscordApiBackoff
-from services.tasks import only_one
 from .models import DiscordUser
 
 logger = logging.getLogger(__name__)

services/modules/discord/tests.py

@@ -351,7 +351,7 @@ class DiscordManagerTestCase(TestCase):
         # Assert
         self.assertTrue(result)
 
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._DiscordOAuthManager__get_group_cache')
+    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_groups')
     @requests_mock.Mocker()
     def test_update_groups(self, group_cache, m):
         from . import manager
@@ -385,7 +385,7 @@ class DiscordManagerTestCase(TestCase):
         self.assertNotIn(444, history['roles'], 'The group id 444 must NOT be added to the request')
 
     @mock.patch(MODULE_PATH + '.manager.cache')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._DiscordOAuthManager__get_group_cache')
+    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_groups')
     @requests_mock.Mocker()
     def test_update_groups_backoff(self, group_cache, djcache, m):
         from . import manager
@@ -420,7 +420,7 @@ class DiscordManagerTestCase(TestCase):
         self.assertTrue(datetime.datetime.strptime(args[1], manager.cache_time_format) > datetime.datetime.now())
 
     @mock.patch(MODULE_PATH + '.manager.cache')
-    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._DiscordOAuthManager__get_group_cache')
+    @mock.patch(MODULE_PATH + '.manager.DiscordOAuthManager._get_groups')
     @requests_mock.Mocker()
     def test_update_groups_global_backoff(self, group_cache, djcache, m):
         from . import manager