Syncers: make functions static

The classes no longer hold any state since they can use the global bot
instance.

6 files changed, 49 insertions, 65 deletions

diff --git a/bot/exts/backend/sync/_cog.py b/bot/exts/backend/sync/_cog.py
index b71ed3e69..48d2b6f02 100644
--- a/bot/exts/backend/sync/_cog.py
+++ b/bot/exts/backend/sync/_cog.py
@@ -18,9 +18,6 @@ class Sync(Cog):
 
     def __init__(self, bot: Bot) -> None:
         self.bot = bot
-        self.role_syncer = _syncers.RoleSyncer()
-        self.user_syncer = _syncers.UserSyncer()
-
         self.bot.loop.create_task(self.sync_guild())
 
     async def sync_guild(self) -> None:
@@ -31,7 +28,7 @@ class Sync(Cog):
         if guild is None:
             return
 
-        for syncer in (self.role_syncer, self.user_syncer):
+        for syncer in (_syncers.RoleSyncer, _syncers.UserSyncer):
             await syncer.sync(guild)
 
     async def patch_user(self, user_id: int, json: Dict[str, Any], ignore_404: bool = False) -> None:
@@ -171,10 +168,10 @@ class Sync(Cog):
     @commands.has_permissions(administrator=True)
     async def sync_roles_command(self, ctx: Context) -> None:
         """Manually synchronise the guild's roles with the roles on the site."""
-        await self.role_syncer.sync(ctx.guild, ctx)
+        await _syncers.RoleSyncer.sync(ctx.guild, ctx)
 
     @sync_group.command(name='users')
     @commands.has_permissions(administrator=True)
     async def sync_users_command(self, ctx: Context) -> None:
         """Manually synchronise the guild's users with the users on the site."""
-        await self.user_syncer.sync(ctx.guild, ctx)
+        await _syncers.UserSyncer.sync(ctx.guild, ctx)
diff --git a/bot/exts/backend/sync/_syncers.py b/bot/exts/backend/sync/_syncers.py
index bdd76806b..2eb9f9971 100644
--- a/bot/exts/backend/sync/_syncers.py
+++ b/bot/exts/backend/sync/_syncers.py
@@ -17,12 +17,15 @@ _Role = namedtuple('Role', ('id', 'name', 'colour', 'permissions', 'position'))
 _Diff = namedtuple('Diff', ('created', 'updated', 'deleted'))
 
 
+# Implementation of static abstract methods are not enforced if the subclass is never instantiated.
+# However, methods are kept abstract to at least symbolise that they should be abstract.
 class Syncer(abc.ABC):
     """Base class for synchronising the database with objects in the Discord cache."""
 
+    @staticmethod
     @property
     @abc.abstractmethod
-    def name(self) -> str:
+    def name() -> str:
         """The name of the syncer; used in output messages and logging."""
         raise NotImplementedError  # pragma: no cover
 
@@ -38,35 +41,36 @@ class Syncer(abc.ABC):
         """Perform the API calls for synchronisation."""
         raise NotImplementedError  # pragma: no cover
 
-    async def sync(self, guild: Guild, ctx: t.Optional[Context] = None) -> None:
+    @classmethod
+    async def sync(cls, guild: Guild, ctx: t.Optional[Context] = None) -> None:
         """
         Synchronise the database with the cache of `guild`.
 
         If `ctx` is given, send a message with the results.
         """
-        log.info(f"Starting {self.name} syncer.")
+        log.info(f"Starting {cls.name} syncer.")
 
         if ctx:
-            message = await ctx.send(f"📊 Synchronising {self.name}s.")
+            message = await ctx.send(f"📊 Synchronising {cls.name}s.")
         else:
             message = None
-        diff = await self._get_diff(guild)
+        diff = await cls._get_diff(guild)
 
         try:
-            await self._sync(diff)
+            await cls._sync(diff)
         except ResponseCodeError as e:
-            log.exception(f"{self.name} syncer failed!")
+            log.exception(f"{cls.name} syncer failed!")
 
             # Don't show response text because it's probably some really long HTML.
             results = f"status {e.status}\n```{e.response_json or 'See log output for details'}```"
-            content = f":x: Synchronisation of {self.name}s failed: {results}"
+            content = f":x: Synchronisation of {cls.name}s failed: {results}"
         else:
             diff_dict = diff._asdict()
             results = (f"{name} `{len(val)}`" for name, val in diff_dict.items() if val is not None)
             results = ", ".join(results)
 
-            log.info(f"{self.name} syncer finished: {results}.")
-            content = f":ok_hand: Synchronisation of {self.name}s complete: {results}"
+            log.info(f"{cls.name} syncer finished: {results}.")
+            content = f":ok_hand: Synchronisation of {cls.name}s complete: {results}"
 
         if message:
             await message.edit(content=content)
diff --git a/tests/bot/exts/backend/sync/test_base.py b/tests/bot/exts/backend/sync/test_base.py
index 157d42452..3ad9db9c3 100644
--- a/tests/bot/exts/backend/sync/test_base.py
+++ b/tests/bot/exts/backend/sync/test_base.py
@@ -15,15 +15,6 @@ class TestSyncer(Syncer):
     _sync = mock.AsyncMock()
 
 
-class SyncerBaseTests(unittest.TestCase):
-    """Tests for the syncer base class."""
-
-    def test_instantiation_fails_without_abstract_methods(self):
-        """The class must have abstract methods implemented."""
-        with self.assertRaisesRegex(TypeError, "Can't instantiate abstract class"):
-            Syncer()
-
-
 class SyncerSyncTests(unittest.IsolatedAsyncioTestCase):
     """Tests for main function orchestrating the sync."""
 
@@ -32,11 +23,13 @@ class SyncerSyncTests(unittest.IsolatedAsyncioTestCase):
         self.bot = patcher.start()
         self.addCleanup(patcher.stop)
 
-        self.syncer = TestSyncer()
         self.guild = helpers.MockGuild()
 
+        TestSyncer._get_diff.reset_mock(return_value=True, side_effect=True)
+        TestSyncer._sync.reset_mock(return_value=True, side_effect=True)
+
         # Make sure `_get_diff` returns a MagicMock, not an AsyncMock
-        self.syncer._get_diff.return_value = mock.MagicMock()
+        TestSyncer._get_diff.return_value = mock.MagicMock()
 
     async def test_sync_message_edited(self):
         """The message should be edited if one was sent, even if the sync has an API error."""
@@ -48,11 +41,11 @@ class SyncerSyncTests(unittest.IsolatedAsyncioTestCase):
 
         for message, side_effect, should_edit in subtests:
             with self.subTest(message=message, side_effect=side_effect, should_edit=should_edit):
-                self.syncer._sync.side_effect = side_effect
+                TestSyncer._sync.side_effect = side_effect
                 ctx = helpers.MockContext()
                 ctx.send.return_value = message
 
-                await self.syncer.sync(self.guild, ctx)
+                await TestSyncer.sync(self.guild, ctx)
 
                 if should_edit:
                     message.edit.assert_called_once()
@@ -67,7 +60,7 @@ class SyncerSyncTests(unittest.IsolatedAsyncioTestCase):
 
         for ctx, message in subtests:
             with self.subTest(ctx=ctx, message=message):
-                await self.syncer.sync(self.guild, ctx)
+                await TestSyncer.sync(self.guild, ctx)
 
                 if ctx is not None:
                     ctx.send.assert_called_once()
diff --git a/tests/bot/exts/backend/sync/test_cog.py b/tests/bot/exts/backend/sync/test_cog.py
index 1e1883558..22a07313e 100644
--- a/tests/bot/exts/backend/sync/test_cog.py
+++ b/tests/bot/exts/backend/sync/test_cog.py
@@ -73,8 +73,6 @@ class SyncCogTests(SyncCogTestCase):
 
         Sync(self.bot)
 
-        self.RoleSyncer.assert_called_once_with()
-        self.UserSyncer.assert_called_once_with()
         sync_guild.assert_called_once_with()
         self.bot.loop.create_task.assert_called_once_with(mock_sync_guild_coro)
 
@@ -83,8 +81,8 @@ class SyncCogTests(SyncCogTestCase):
         for guild in (helpers.MockGuild(), None):
             with self.subTest(guild=guild):
                 self.bot.reset_mock()
-                self.cog.role_syncer.reset_mock()
-                self.cog.user_syncer.reset_mock()
+                self.RoleSyncer.reset_mock()
+                self.UserSyncer.reset_mock()
 
                 self.bot.get_guild = mock.MagicMock(return_value=guild)
 
@@ -94,11 +92,11 @@ class SyncCogTests(SyncCogTestCase):
                 self.bot.get_guild.assert_called_once_with(constants.Guild.id)
 
                 if guild is None:
-                    self.cog.role_syncer.sync.assert_not_called()
-                    self.cog.user_syncer.sync.assert_not_called()
+                    self.RoleSyncer.sync.assert_not_called()
+                    self.UserSyncer.sync.assert_not_called()
                 else:
-                    self.cog.role_syncer.sync.assert_called_once_with(guild)
-                    self.cog.user_syncer.sync.assert_called_once_with(guild)
+                    self.RoleSyncer.sync.assert_called_once_with(guild)
+                    self.UserSyncer.sync.assert_called_once_with(guild)
 
     async def patch_user_helper(self, side_effect: BaseException) -> None:
         """Helper to set a side effect for bot.api_client.patch and then assert it is called."""
@@ -394,14 +392,14 @@ class SyncCogCommandTests(SyncCogTestCase, CommandTestCase):
         ctx = helpers.MockContext()
         await self.cog.sync_roles_command(self.cog, ctx)
 
-        self.cog.role_syncer.sync.assert_called_once_with(ctx.guild, ctx)
+        self.RoleSyncer.sync.assert_called_once_with(ctx.guild, ctx)
 
     async def test_sync_users_command(self):
         """sync() should be called on the UserSyncer."""
         ctx = helpers.MockContext()
         await self.cog.sync_users_command(self.cog, ctx)
 
-        self.cog.user_syncer.sync.assert_called_once_with(ctx.guild, ctx)
+        self.UserSyncer.sync.assert_called_once_with(ctx.guild, ctx)
 
     async def test_commands_require_admin(self):
         """The sync commands should only run if the author has the administrator permission."""
diff --git a/tests/bot/exts/backend/sync/test_roles.py b/tests/bot/exts/backend/sync/test_roles.py
index fb63a4ae0..541074336 100644
--- a/tests/bot/exts/backend/sync/test_roles.py
+++ b/tests/bot/exts/backend/sync/test_roles.py
@@ -26,8 +26,6 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot = patcher.start()
         self.addCleanup(patcher.stop)
 
-        self.syncer = RoleSyncer()
-
     @staticmethod
     def get_guild(*roles):
         """Fixture to return a guild object with the given roles."""
@@ -47,7 +45,7 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot.api_client.get.return_value = [fake_role()]
         guild = self.get_guild(fake_role())
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await RoleSyncer._get_diff(guild)
         expected_diff = (set(), set(), set())
 
         self.assertEqual(actual_diff, expected_diff)
@@ -59,7 +57,7 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot.api_client.get.return_value = [fake_role(id=41, name="old"), fake_role()]
         guild = self.get_guild(updated_role, fake_role())
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await RoleSyncer._get_diff(guild)
         expected_diff = (set(), {_Role(**updated_role)}, set())
 
         self.assertEqual(actual_diff, expected_diff)
@@ -71,7 +69,7 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot.api_client.get.return_value = [fake_role()]
         guild = self.get_guild(fake_role(), new_role)
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await RoleSyncer._get_diff(guild)
         expected_diff = ({_Role(**new_role)}, set(), set())
 
         self.assertEqual(actual_diff, expected_diff)
@@ -83,7 +81,7 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot.api_client.get.return_value = [fake_role(), deleted_role]
         guild = self.get_guild(fake_role())
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await RoleSyncer._get_diff(guild)
         expected_diff = (set(), set(), {_Role(**deleted_role)})
 
         self.assertEqual(actual_diff, expected_diff)
@@ -101,7 +99,7 @@ class RoleSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         ]
         guild = self.get_guild(fake_role(), new, updated)
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await RoleSyncer._get_diff(guild)
         expected_diff = ({_Role(**new)}, {_Role(**updated)}, {_Role(**deleted)})
 
         self.assertEqual(actual_diff, expected_diff)
@@ -115,15 +113,13 @@ class RoleSyncerSyncTests(unittest.IsolatedAsyncioTestCase):
         self.bot = patcher.start()
         self.addCleanup(patcher.stop)
 
-        self.syncer = RoleSyncer()
-
     async def test_sync_created_roles(self):
         """Only POST requests should be made with the correct payload."""
         roles = [fake_role(id=111), fake_role(id=222)]
 
         role_tuples = {_Role(**role) for role in roles}
         diff = _Diff(role_tuples, set(), set())
-        await self.syncer._sync(diff)
+        await RoleSyncer._sync(diff)
 
         calls = [mock.call("bot/roles", json=role) for role in roles]
         self.bot.api_client.post.assert_has_calls(calls, any_order=True)
@@ -138,7 +134,7 @@ class RoleSyncerSyncTests(unittest.IsolatedAsyncioTestCase):
 
         role_tuples = {_Role(**role) for role in roles}
         diff = _Diff(set(), role_tuples, set())
-        await self.syncer._sync(diff)
+        await RoleSyncer._sync(diff)
 
         calls = [mock.call(f"bot/roles/{role['id']}", json=role) for role in roles]
         self.bot.api_client.put.assert_has_calls(calls, any_order=True)
@@ -153,7 +149,7 @@ class RoleSyncerSyncTests(unittest.IsolatedAsyncioTestCase):
 
         role_tuples = {_Role(**role) for role in roles}
         diff = _Diff(set(), set(), role_tuples)
-        await self.syncer._sync(diff)
+        await RoleSyncer._sync(diff)
 
         calls = [mock.call(f"bot/roles/{role['id']}") for role in roles]
         self.bot.api_client.delete.assert_has_calls(calls, any_order=True)
diff --git a/tests/bot/exts/backend/sync/test_users.py b/tests/bot/exts/backend/sync/test_users.py
index 9f28d0162..61673e1bb 100644
--- a/tests/bot/exts/backend/sync/test_users.py
+++ b/tests/bot/exts/backend/sync/test_users.py
@@ -24,8 +24,6 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         self.bot = patcher.start()
         self.addCleanup(patcher.stop)
 
-        self.syncer = UserSyncer()
-
     @staticmethod
     def get_guild(*members):
         """Fixture to return a guild object with the given members."""
@@ -61,7 +59,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         }
         guild = self.get_guild()
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([], [], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -77,7 +75,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
         guild = self.get_guild(fake_user())
 
         guild.get_member.return_value = self.get_mock_member(fake_user())
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([], [], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -98,7 +96,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
             self.get_mock_member(fake_user())
         ]
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([], [{"id": 99, "name": "new"}], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -118,7 +116,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
             self.get_mock_member(fake_user()),
             self.get_mock_member(new_user)
         ]
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([new_user], [], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -137,7 +135,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
             None
         ]
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([], [{"id": 63, "in_guild": False}], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -161,7 +159,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
             None
         ]
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([new_user], [{"id": 55, "name": "updated"}, {"id": 63, "in_guild": False}], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -180,7 +178,7 @@ class UserSyncerDiffTests(unittest.IsolatedAsyncioTestCase):
             None
         ]
 
-        actual_diff = await self.syncer._get_diff(guild)
+        actual_diff = await UserSyncer._get_diff(guild)
         expected_diff = ([], [], None)
 
         self.assertEqual(actual_diff, expected_diff)
@@ -194,14 +192,12 @@ class UserSyncerSyncTests(unittest.IsolatedAsyncioTestCase):
         self.bot = patcher.start()
         self.addCleanup(patcher.stop)
 
-        self.syncer = UserSyncer()
-
     async def test_sync_created_users(self):
         """Only POST requests should be made with the correct payload."""
         users = [fake_user(id=111), fake_user(id=222)]
 
         diff = _Diff(users, [], None)
-        await self.syncer._sync(diff)
+        await UserSyncer._sync(diff)
 
         self.bot.api_client.post.assert_called_once_with("bot/users", json=diff.created)
 
@@ -213,7 +209,7 @@ class UserSyncerSyncTests(unittest.IsolatedAsyncioTestCase):
         users = [fake_user(id=111), fake_user(id=222)]
 
         diff = _Diff([], users, None)
-        await self.syncer._sync(diff)
+        await UserSyncer._sync(diff)
 
         self.bot.api_client.patch.assert_called_once_with("bot/users/bulk_patch", json=diff.updated)