From b2feeae3c51a7ee493446aba3035acc670bbbfce Mon Sep 17 00:00:00 2001 From: Ariel Rin Date: Thu, 28 May 2026 10:11:20 +0000 Subject: [PATCH] Reconnect signals in AA v5 --- allianceauth/admin_status/apps.py | 3 + allianceauth/analytics/apps.py | 3 + allianceauth/apps.py | 2 +- allianceauth/authentication/apps.py | 7 +- .../task_statistics/tests/test_signals.py | 26 +- .../authentication/tests/test_admin.py | 42 ++ .../authentication/tests/test_signals.py | 234 ++++++++--- allianceauth/corputils/apps.py | 3 + allianceauth/crontab/apps.py | 3 + allianceauth/custom_css/apps.py | 3 + allianceauth/eveonline/autogroups/apps.py | 3 +- allianceauth/fleetactivitytracking/apps.py | 3 + allianceauth/framework/apps.py | 3 + allianceauth/groupmanagement/apps.py | 5 +- .../groupmanagement/tests/test_admin.py | 42 +- allianceauth/hrapplications/apps.py | 3 + allianceauth/menu/apps.py | 2 +- allianceauth/notifications/apps.py | 3 + allianceauth/optimer/apps.py | 3 + allianceauth/permissions_tool/apps.py | 3 + allianceauth/services/apps.py | 5 +- .../services/modules/teamspeak3/apps.py | 3 +- allianceauth/services/tests/test_signals.py | 18 + allianceauth/srp/apps.py | 3 + allianceauth/tests/test_signal_inventory.py | 396 ++++++++++++++++++ allianceauth/theme/apps.py | 2 +- allianceauth/timerboard/apps.py | 3 + 27 files changed, 738 insertions(+), 88 deletions(-) create mode 100644 allianceauth/tests/test_signal_inventory.py diff --git a/allianceauth/admin_status/apps.py b/allianceauth/admin_status/apps.py index c375cfa3..5ddf104a 100644 --- a/allianceauth/admin_status/apps.py +++ b/allianceauth/admin_status/apps.py @@ -7,3 +7,6 @@ class AdminStatusApplication(AppConfig): name = 'allianceauth.admin_status' label = 'admin_status' verbose_name = _("Admin Status") + + def ready(self) -> None: + pass diff --git a/allianceauth/analytics/apps.py b/allianceauth/analytics/apps.py index 1a07e15d..55c7a73f 100644 --- a/allianceauth/analytics/apps.py +++ b/allianceauth/analytics/apps.py @@ -6,3 +6,6 @@ class AnalyticsConfig(AppConfig): name = 'allianceauth.analytics' label = 'analytics' verbose_name = _('Analytics') + + def ready(self) -> None: + pass diff --git a/allianceauth/apps.py b/allianceauth/apps.py index 1191fed9..dc270705 100644 --- a/allianceauth/apps.py +++ b/allianceauth/apps.py @@ -5,4 +5,4 @@ class AllianceAuthConfig(AppConfig): name = 'allianceauth' def ready(self) -> None: - import allianceauth.checks # noqa + import allianceauth.checks # noqa: F401 diff --git a/allianceauth/authentication/apps.py b/allianceauth/authentication/apps.py index e53cb659..e47140bc 100644 --- a/allianceauth/authentication/apps.py +++ b/allianceauth/authentication/apps.py @@ -1,5 +1,5 @@ from django.apps import AppConfig -from django.core.checks import register, Tags +from django.core.checks import Tags, register from django.utils.translation import gettext_lazy as _ @@ -8,8 +8,9 @@ class AuthenticationConfig(AppConfig): label = "authentication" verbose_name = _("Authentication") - def ready(self): - from allianceauth.authentication import checks, signals # noqa: F401 + def ready(self) -> None: + from allianceauth.authentication import signals # noqa: F401 + from allianceauth.authentication import checks from allianceauth.authentication.task_statistics import ( signals as celery_signals, ) diff --git a/allianceauth/authentication/task_statistics/tests/test_signals.py b/allianceauth/authentication/task_statistics/tests/test_signals.py index d04f11e2..65635c8a 100644 --- a/allianceauth/authentication/task_statistics/tests/test_signals.py +++ b/allianceauth/authentication/task_statistics/tests/test_signals.py @@ -5,13 +5,11 @@ from celery.exceptions import Retry from django.test import TestCase, override_settings from allianceauth.authentication.task_statistics.counters import ( - failed_tasks, - retried_tasks, - succeeded_tasks, + failed_tasks, retried_tasks, succeeded_tasks, ) from allianceauth.authentication.task_statistics.signals import ( - is_enabled, - reset_counters, + is_enabled, record_task_internal_error, reset_counters, + reset_counters_when_celery_restarted, ) from allianceauth.eveonline.tasks import update_character @@ -75,6 +73,24 @@ class TestTaskSignals(TestCase): self.assertEqual(retried_tasks.count(), 0) self.assertEqual(failed_tasks.count(), 0) + def test_should_reset_counters_when_worker_becomes_ready(self): + succeeded_tasks.add() + retried_tasks.add() + failed_tasks.add() + + reset_counters_when_celery_restarted() + + self.assertEqual(succeeded_tasks.count(), 0) + self.assertEqual(retried_tasks.count(), 0) + self.assertEqual(failed_tasks.count(), 0) + + def test_should_record_internal_error_as_failed_task(self): + record_task_internal_error() + + self.assertEqual(succeeded_tasks.count(), 0) + self.assertEqual(retried_tasks.count(), 0) + self.assertEqual(failed_tasks.count(), 1) + class TestIsEnabled(TestCase): @override_settings(ALLIANCEAUTH_DASHBOARD_TASK_STATISTICS_DISABLED=False) diff --git a/allianceauth/authentication/tests/test_admin.py b/allianceauth/authentication/tests/test_admin.py index bbd0f68f..feab5177 100644 --- a/allianceauth/authentication/tests/test_admin.py +++ b/allianceauth/authentication/tests/test_admin.py @@ -17,6 +17,7 @@ from allianceauth.eveonline.models import ( from allianceauth.services.hooks import ServicesHook from allianceauth.tests.auth_utils import AuthUtils +from .. import admin as authentication_admin from ..admin import ( BaseUserAdmin, CharacterOwnershipAdmin, MainAllianceFilter, MainCorporationsFilter, MainFactionFilter, OwnershipRecordAdmin, @@ -819,3 +820,44 @@ class TestMakeServicesHooksActions(TestCaseWithTestData): action(MagicMock(), MagicMock(), [self.user_1]) self.assertFalse(mock_service.sync_nickname.called) self.assertTrue(mock_service.sync_nicknames_bulk.called) + + +class TestAdminSignalRedirects(TestCaseWithTestData): + def test_redirect_receivers_forward_user_signals_to_base_user(self): + cases = [ + ( + authentication_admin.redirect_pre_save, + authentication_admin.pre_save, + {"instance": self.user_1}, + ), + ( + authentication_admin.redirect_post_save, + authentication_admin.post_save, + {"instance": self.user_1, "created": False}, + ), + ( + authentication_admin.redirect_pre_delete, + authentication_admin.pre_delete, + {"instance": self.user_1}, + ), + ( + authentication_admin.redirect_post_delete, + authentication_admin.post_delete, + {"instance": self.user_1}, + ), + ( + authentication_admin.redirect_m2m_changed_groups, + authentication_admin.m2m_changed, + {"instance": self.user_1, "action": "post_add", "pk_set": {self.group_1.pk}}, + ), + ( + authentication_admin.redirect_m2m_changed_permissions, + authentication_admin.m2m_changed, + {"instance": self.user_1, "action": "post_add", "pk_set": set()}, + ), + ] + + for receiver, signal, kwargs in cases: + with self.subTest(receiver=receiver.__name__), patch.object(signal, "send") as send: + receiver(sender=User, **kwargs) + send.assert_called_once_with(authentication_admin.BaseUser, **kwargs) diff --git a/allianceauth/authentication/tests/test_signals.py b/allianceauth/authentication/tests/test_signals.py index 95e7fb9a..c1bbc20c 100644 --- a/allianceauth/authentication/tests/test_signals.py +++ b/allianceauth/authentication/tests/test_signals.py @@ -1,85 +1,183 @@ -from django.db.models.signals import post_save -from django.test.testcases import TestCase +from unittest.mock import patch -from allianceauth.authentication.models import User, UserProfile -from allianceauth.eveonline.models import EveAllianceInfo, EveCharacter, EveCorporationInfo +from django.contrib.auth.models import User +from django.test import TestCase + +from esi.models import Token + +from allianceauth.authentication.models import ( + CharacterOwnership, OwnershipRecord, State, UserProfile, +) +from allianceauth.authentication.signals import ( + state_member_alliances_changed, state_member_characters_changed, + state_member_corporations_changed, state_member_factions_changed, + state_saved, +) +from allianceauth.eveonline.models import EveCharacter from allianceauth.tests.auth_utils import AuthUtils -from . import patch +MODULE_PATH = "allianceauth.authentication.signals" -class TestUserProfileSignals(TestCase): - +class AuthenticationSignalsTestCase(TestCase): def setUp(self): - state = AuthUtils.get_member_state() + self.member_state = AuthUtils.get_member_state() + self.guest_state = AuthUtils.get_guest_state() + self.user = AuthUtils.create_user("test_user") + self.main_character = EveCharacter.objects.create( + character_id=1001, + character_name="Main Character", + corporation_id=2001, + corporation_name="Main Corp", + corporation_ticker="MAIN", + ) + self.alt_character = EveCharacter.objects.create( + character_id=1002, + character_name="Alt Character", + corporation_id=2002, + corporation_name="Alt Corp", + corporation_ticker="ALT", + ) + self.user.profile.main_character = self.main_character + self.user.profile.state = self.member_state + self.user.profile.save(update_fields=["main_character", "state"]) - self.char = EveCharacter.objects.create( - character_id='1234', - character_name='test character', - corporation_id='2345', - corporation_name='test corp', - corporation_ticker='tickr', - alliance_id='3456', - alliance_name='alliance name', + @patch(MODULE_PATH + ".trigger_state_check") + def test_state_membership_receivers_trigger_state_check(self, trigger_state_check): + state = State.objects.create(name="Signal State", priority=900) + trigger_state_check.reset_mock() + + for receiver in [ + state_member_characters_changed, + state_member_corporations_changed, + state_member_alliances_changed, + state_member_factions_changed, + ]: + with self.subTest(receiver=receiver.__name__): + receiver(sender=None, instance=state, action="post_add") + + state_saved(sender=State, instance=state) + + self.assertEqual(trigger_state_check.call_count, 5) + + @patch.object(UserProfile, "assign_state") + def test_reassess_on_profile_save_reassigns_state(self, assign_state): + self.user.profile.language = UserProfile.Language.ENGLISH + + self.user.profile.save(update_fields=["language"]) + + assign_state.assert_called_once_with() + + def test_create_required_models_creates_profile_for_new_user(self): + user = User.objects.create(username="created_by_signal") + + self.assertTrue(UserProfile.objects.filter(user=user).exists()) + + def test_create_required_models_does_not_duplicate_existing_profile(self): + profile_pk = self.user.profile.pk + self.user.username = "updated_user" + + self.user.save(update_fields=["username"]) + + self.assertEqual(UserProfile.objects.filter(user=self.user).count(), 1) + self.assertEqual(self.user.profile.pk, profile_pk) + + def test_record_character_ownership_creates_ownership_and_history(self): + Token.objects.create( + user=self.user, + character_id=self.alt_character.character_id, + character_name=self.alt_character.character_name, + character_owner_hash="owner-hash-1", + access_token="access-token", + refresh_token="refresh-token", ) - self.alliance = EveAllianceInfo.objects.create( - alliance_id='3456', - alliance_name='alliance name', - alliance_ticker='TIKR', - executor_corp_id='2345', + ownership = CharacterOwnership.objects.get(character=self.alt_character) + + self.assertEqual(ownership.user, self.user) + self.assertEqual(ownership.owner_hash, "owner-hash-1") + self.assertTrue( + OwnershipRecord.objects.filter( + user=self.user, + character=self.alt_character, + owner_hash="owner-hash-1", + ).exists() ) - self.corp = EveCorporationInfo.objects.create( - corporation_id='2345', - corporation_name='corp name', - corporation_ticker='TIKK', - member_count=10, - alliance=self.alliance, + def test_validate_main_character_clears_profile_on_ownership_delete(self): + ownership = CharacterOwnership.objects.create( + character=self.main_character, + user=self.user, + owner_hash="owner-hash-main", ) - state.member_alliances.add(self.alliance) - state.member_corporations.add(self.corp) + ownership.delete() + self.user.profile.refresh_from_db() - self.member = AuthUtils.create_user('test user') - self.member.profile.main_character = self.char - self.member.profile.save() + self.assertIsNone(self.user.profile.main_character) - @patch('.signals.create_required_models') - def test_create_required_models_triggered_true( - self, create_required_models): - """ - Create a User object here, - to generate UserProfile models - """ - post_save.connect(create_required_models, sender=User) - AuthUtils.create_user('test_create_required_models_triggered') - self.assertTrue = create_required_models.called - self.assertEqual(create_required_models.call_count, 1) - - user = User.objects.get(username='test_create_required_models_triggered') - self.assertIsNot(UserProfile.objects.get(user=user), False) - - @patch('.signals.create_required_models') - def test_create_required_models_triggered_false( - self, create_required_models): - """ - Only call a User object Update here, - which does not need to generate UserProfile models - """ - post_save.connect(create_required_models, sender=User) - char = EveCharacter.objects.create( - character_id='1266', - character_name='test character2', - corporation_id='2345', - corporation_name='test corp', - corporation_ticker='tickr', - alliance_id='3456', - alliance_name='alliance name', + def test_validate_ownership_deletes_ownership_when_last_refreshable_token_deleted(self): + ownership = CharacterOwnership.objects.create( + character=self.alt_character, + user=self.user, + owner_hash="owner-hash-2", + ) + token = Token.objects.create( + user=self.user, + character_id=self.alt_character.character_id, + character_name=self.alt_character.character_name, + character_owner_hash="owner-hash-2", + access_token="access-token", + refresh_token="refresh-token", ) - self.member.profile.main_character = char - self.member.profile.save() - self.assertTrue = create_required_models.called - self.assertEqual(create_required_models.call_count, 0) - self.assertIsNot(UserProfile.objects.get(user=self.member), False) + token.delete() + + self.assertFalse(CharacterOwnership.objects.filter(pk=ownership.pk).exists()) + + def test_assign_state_on_active_change_sets_guest_state_for_inactive_user(self): + self.user.is_active = False + + self.user.save(update_fields=["is_active"]) + self.user.profile.refresh_from_db() + + self.assertEqual(self.user.profile.state, self.guest_state) + + @patch.object(UserProfile, "assign_state") + def test_assign_state_on_active_change_reassigns_state_when_user_reactivated( + self, assign_state + ): + self.user.is_active = False + self.user.save(update_fields=["is_active"]) + + self.user.is_active = True + self.user.save(update_fields=["is_active"]) + + assign_state.assert_called_once_with() + + @patch.object(UserProfile, "assign_state") + def test_check_state_on_character_update_reassesses_main_character_owner( + self, assign_state + ): + self.main_character.character_name = "Renamed Main Character" + + self.main_character.save(update_fields=["character_name"]) + + assign_state.assert_called_once_with() + + def test_ownership_record_creation_skips_duplicate_latest_record(self): + CharacterOwnership.objects.create( + character=self.alt_character, + user=self.user, + owner_hash="owner-hash-3", + ) + self.assertEqual(OwnershipRecord.objects.count(), 1) + + CharacterOwnership.objects.filter(character=self.alt_character).delete() + CharacterOwnership.objects.create( + character=self.alt_character, + user=self.user, + owner_hash="owner-hash-3", + ) + + self.assertEqual(OwnershipRecord.objects.count(), 1) diff --git a/allianceauth/corputils/apps.py b/allianceauth/corputils/apps.py index d3ab7611..9c013b35 100644 --- a/allianceauth/corputils/apps.py +++ b/allianceauth/corputils/apps.py @@ -6,3 +6,6 @@ class CorpUtilsConfig(AppConfig): name = 'allianceauth.corputils' label = 'corputils' verbose_name = _('Corporation Stats') + + def ready(self) -> None: + pass diff --git a/allianceauth/crontab/apps.py b/allianceauth/crontab/apps.py index ff1232d4..95871bd1 100644 --- a/allianceauth/crontab/apps.py +++ b/allianceauth/crontab/apps.py @@ -14,3 +14,6 @@ class CrontabConfig(AppConfig): name = "allianceauth.crontab" label = "crontab" verbose_name = _("Crontab") + + def ready(self) -> None: + pass diff --git a/allianceauth/custom_css/apps.py b/allianceauth/custom_css/apps.py index 614f3463..26e56b06 100644 --- a/allianceauth/custom_css/apps.py +++ b/allianceauth/custom_css/apps.py @@ -11,3 +11,6 @@ class CustomCSSConfig(AppConfig): name = "allianceauth.custom_css" label = "custom_css" verbose_name = _("Custom CSS") + + def ready(self) -> None: + pass diff --git a/allianceauth/eveonline/autogroups/apps.py b/allianceauth/eveonline/autogroups/apps.py index 8aad8fca..03ae9ddf 100644 --- a/allianceauth/eveonline/autogroups/apps.py +++ b/allianceauth/eveonline/autogroups/apps.py @@ -8,4 +8,5 @@ class EveAutogroupsConfig(AppConfig): verbose_name = _('EVE Online Autogroups') def ready(self) -> None: - pass + # Connect Signals + from allianceauth.eveonline.autogroups import signals # noqa:F401 diff --git a/allianceauth/fleetactivitytracking/apps.py b/allianceauth/fleetactivitytracking/apps.py index 7307946d..63684cd9 100644 --- a/allianceauth/fleetactivitytracking/apps.py +++ b/allianceauth/fleetactivitytracking/apps.py @@ -6,3 +6,6 @@ class FatConfig(AppConfig): name = 'allianceauth.fleetactivitytracking' label = 'fleetactivitytracking' verbose_name = _('Fleet Activity Tracking') + + def ready(self) -> None: + pass diff --git a/allianceauth/framework/apps.py b/allianceauth/framework/apps.py index 7942d437..696ff449 100644 --- a/allianceauth/framework/apps.py +++ b/allianceauth/framework/apps.py @@ -14,3 +14,6 @@ class FrameworkConfig(AppConfig): name = "allianceauth.framework" label = "framework" verbose_name = _("Framework") + + def ready(self) -> None: + pass diff --git a/allianceauth/groupmanagement/apps.py b/allianceauth/groupmanagement/apps.py index 747acdb3..3b81268a 100644 --- a/allianceauth/groupmanagement/apps.py +++ b/allianceauth/groupmanagement/apps.py @@ -7,5 +7,6 @@ class GroupManagementConfig(AppConfig): label = 'groupmanagement' verbose_name = _('Group Management') - def ready(self): - from . import signals # noqa: F401 + def ready(self) -> None: + # Connect Signals + from allianceauth.groupmanagement import signals # noqa: F401 diff --git a/allianceauth/groupmanagement/tests/test_admin.py b/allianceauth/groupmanagement/tests/test_admin.py index 0f7a1ed1..1e139322 100644 --- a/allianceauth/groupmanagement/tests/test_admin.py +++ b/allianceauth/groupmanagement/tests/test_admin.py @@ -10,12 +10,11 @@ from django.test import Client, RequestFactory, TestCase, override_settings from allianceauth.authentication.models import CharacterOwnership, State from allianceauth.eveonline.models import ( - EveAllianceInfo, - EveCharacter, - EveCorporationInfo, + EveAllianceInfo, EveCharacter, EveCorporationInfo, ) from allianceauth.tests.auth_utils import AuthUtils +from .. import admin as groupmanagement_admin from ..admin import Group, GroupAdmin, HasLeaderFilter from ..models import ReservedGroupName from . import get_admin_change_view_url @@ -585,6 +584,43 @@ class TestGroupAdminChangeFormSuperuserExclusiveEdits(WebTest): self.assertNotIn(field, form.fields) +class TestAdminSignalRedirects(TestCase): + def test_redirect_receivers_forward_group_signals_to_base_group(self): + group = Group.objects.create(name='Signal Group') + cases = [ + ( + groupmanagement_admin.redirect_pre_save, + groupmanagement_admin.pre_save, + {"instance": group}, + ), + ( + groupmanagement_admin.redirect_post_save, + groupmanagement_admin.post_save, + {"instance": group, "created": False}, + ), + ( + groupmanagement_admin.redirect_pre_delete, + groupmanagement_admin.pre_delete, + {"instance": group}, + ), + ( + groupmanagement_admin.redirect_post_delete, + groupmanagement_admin.post_delete, + {"instance": group}, + ), + ( + groupmanagement_admin.redirect_m2m_changed_permissions, + groupmanagement_admin.m2m_changed, + {"instance": group, "action": "post_add", "pk_set": set()}, + ), + ] + + for receiver, signal, kwargs in cases: + with self.subTest(receiver=receiver.__name__), patch.object(signal, "send") as send: + receiver(sender=Group, **kwargs) + send.assert_called_once_with(groupmanagement_admin.BaseGroup, **kwargs) + + @override_settings(CELERY_ALWAYS_EAGER=True, CELERY_EAGER_PROPAGATES_EXCEPTIONS=True) class TestGroupAdmin2(TestCase): @classmethod diff --git a/allianceauth/hrapplications/apps.py b/allianceauth/hrapplications/apps.py index 80af757e..0fef4ff0 100644 --- a/allianceauth/hrapplications/apps.py +++ b/allianceauth/hrapplications/apps.py @@ -6,3 +6,6 @@ class HRApplicationsConfig(AppConfig): name = 'allianceauth.hrapplications' label = 'hrapplications' verbose_name = _('HR Applications') + + def ready(self) -> None: + pass diff --git a/allianceauth/menu/apps.py b/allianceauth/menu/apps.py index e500004d..13e93aa4 100644 --- a/allianceauth/menu/apps.py +++ b/allianceauth/menu/apps.py @@ -15,7 +15,7 @@ class MenuConfig(AppConfig): label = "menu" verbose_name = _("Menu") - def ready(self): + def ready(self) -> None: from allianceauth.menu.core import smart_sync smart_sync.reset_menu_items_sync() diff --git a/allianceauth/notifications/apps.py b/allianceauth/notifications/apps.py index 097326f6..ac155ed9 100644 --- a/allianceauth/notifications/apps.py +++ b/allianceauth/notifications/apps.py @@ -6,3 +6,6 @@ class NotificationsConfig(AppConfig): name = 'allianceauth.notifications' label = 'notifications' verbose_name = _('Notifications') + + def ready(self) -> None: + pass diff --git a/allianceauth/optimer/apps.py b/allianceauth/optimer/apps.py index 6eb56f05..f7151f6d 100644 --- a/allianceauth/optimer/apps.py +++ b/allianceauth/optimer/apps.py @@ -6,3 +6,6 @@ class OptimerConfig(AppConfig): name = 'allianceauth.optimer' label = 'optimer' verbose_name = _('Fleet Operations') + + def ready(self) -> None: + pass diff --git a/allianceauth/permissions_tool/apps.py b/allianceauth/permissions_tool/apps.py index 4d795aba..0cb6e76c 100644 --- a/allianceauth/permissions_tool/apps.py +++ b/allianceauth/permissions_tool/apps.py @@ -6,3 +6,6 @@ class PermissionsToolConfig(AppConfig): name = 'allianceauth.permissions_tool' label = 'permissions_tool' verbose_name = _('Permissions Audit') + + def ready(self) -> None: + pass diff --git a/allianceauth/services/apps.py b/allianceauth/services/apps.py index 9c537476..e88a8bc9 100644 --- a/allianceauth/services/apps.py +++ b/allianceauth/services/apps.py @@ -7,5 +7,6 @@ class ServicesConfig(AppConfig): label = 'services' verbose_name = _('Services') - def ready(self): - pass + def ready(self) -> None: + # Connect Services + from allianceauth.services import signals # noqa: F401 diff --git a/allianceauth/services/modules/teamspeak3/apps.py b/allianceauth/services/modules/teamspeak3/apps.py index 25f168b6..5fc236f5 100644 --- a/allianceauth/services/modules/teamspeak3/apps.py +++ b/allianceauth/services/modules/teamspeak3/apps.py @@ -8,4 +8,5 @@ class Teamspeak3ServiceConfig(AppConfig): verbose_name = _('TeamSpeak 3 Service') def ready(self): - pass + from allianceauth.services.modules.teamspeak3 import \ + signals # noqa: F401 diff --git a/allianceauth/services/tests/test_signals.py b/allianceauth/services/tests/test_signals.py index 50c36d9c..632cc8cd 100644 --- a/allianceauth/services/tests/test_signals.py +++ b/allianceauth/services/tests/test_signals.py @@ -251,6 +251,24 @@ class ServicesSignalsTestCase(TestCase): self.assertFalse(svc.validate_user.called) self.assertFalse(svc.sync_nickname.called) + @mock.patch(MODULE_PATH + '.ServicesHook') + def test_process_main_character_update_validates_and_syncs_nickname( + self, services_hook + ): + svc = mock.Mock() + svc.validate_user.return_value = None + svc.sync_nickname.return_value = None + svc.access_perm = 'auth.access_testsvc' + + services_hook.get_services.return_value = [svc] + + self.member.profile.main_character.character_name = 'Updated Name' + self.member.profile.main_character.save() + + self.assertTrue(services_hook.get_services.called) + svc.validate_user.assert_called_once_with(self.member) + svc.sync_nickname.assert_called_once_with(self.member) + @mock.patch( "allianceauth.services.modules.mumble.auth_hooks.MumbleService.update_groups" diff --git a/allianceauth/srp/apps.py b/allianceauth/srp/apps.py index dd4820a6..515045f8 100644 --- a/allianceauth/srp/apps.py +++ b/allianceauth/srp/apps.py @@ -6,3 +6,6 @@ class SRPConfig(AppConfig): name = 'allianceauth.srp' label = 'srp' verbose_name = _('Ship Replacement') + + def ready(self) -> None: + pass diff --git a/allianceauth/tests/test_signal_inventory.py b/allianceauth/tests/test_signal_inventory.py new file mode 100644 index 00000000..eda72169 --- /dev/null +++ b/allianceauth/tests/test_signal_inventory.py @@ -0,0 +1,396 @@ +import ast +import importlib +import sys +from pathlib import Path + +from django.test import SimpleTestCase + +BASE_DIR = Path(__file__).resolve().parents[1] + + +# This inventory is the contract for signal modules that must be loaded by +# AppConfig.ready() during Django startup. If a new receiver is added to one of +# these files, this inventory must be updated and the receiver must be covered +# by a focused test in that app. +SIGNAL_INVENTORY = { + "allianceauth.authentication.signals": { + "file": "authentication/signals.py", + "ready_file": "authentication/apps.py", + "receivers": [ + { + "name": "state_member_characters_changed", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.authentication.models.State.member_characters.through", + }, + { + "name": "state_member_corporations_changed", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.authentication.models.State.member_corporations.through", + }, + { + "name": "state_member_alliances_changed", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.authentication.models.State.member_alliances.through", + }, + { + "name": "state_member_factions_changed", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.authentication.models.State.member_factions.through", + }, + { + "name": "state_saved", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.authentication.models.State", + }, + { + "name": "reassess_on_profile_save", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.authentication.models.UserProfile", + }, + { + "name": "create_required_models", + "signal": "django.db.models.signals.post_save", + "sender": "django.contrib.auth.models.User", + }, + { + "name": "record_character_ownership", + "signal": "django.db.models.signals.post_save", + "sender": "esi.models.Token", + }, + { + "name": "validate_main_character", + "signal": "django.db.models.signals.pre_delete", + "sender": "allianceauth.authentication.models.CharacterOwnership", + }, + { + "name": "validate_ownership", + "signal": "django.db.models.signals.post_delete", + "sender": "esi.models.Token", + }, + { + "name": "assign_state_on_active_change", + "signal": "django.db.models.signals.pre_save", + "sender": "django.contrib.auth.models.User", + }, + { + "name": "check_state_on_character_update", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.eveonline.models.EveCharacter", + }, + { + "name": "ownership_record_creation", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.authentication.models.CharacterOwnership", + }, + ], + }, + "allianceauth.authentication.task_statistics.signals": { + "file": "authentication/task_statistics/signals.py", + "ready_file": "authentication/apps.py", + "receivers": [ + { + "name": "reset_counters_when_celery_restarted", + "signal": "celery.signals.worker_ready", + "sender": None, + }, + { + "name": "record_task_succeeded", + "signal": "celery.signals.task_success", + "sender": None, + }, + { + "name": "record_task_retried", + "signal": "celery.signals.task_retry", + "sender": None, + }, + { + "name": "record_task_failed", + "signal": "celery.signals.task_failure", + "sender": None, + }, + { + "name": "record_task_internal_error", + "signal": "celery.signals.task_internal_error", + "sender": None, + }, + ], + }, + "allianceauth.groupmanagement.signals": { + "file": "groupmanagement/signals.py", + "ready_file": "groupmanagement/apps.py", + "receivers": [ + { + "name": "find_new_name_for_conflicting_groups", + "signal": "django.db.models.signals.pre_save", + "sender": "django.contrib.auth.models.Group", + }, + { + "name": "create_auth_group", + "signal": "django.db.models.signals.post_save", + "sender": "django.contrib.auth.models.Group", + }, + { + "name": "check_groups_on_state_change", + "signal": "allianceauth.authentication.signals.state_changed", + "sender": None, + }, + ], + }, + "allianceauth.services.signals": { + "file": "services/signals.py", + "ready_file": "services/apps.py", + "receivers": [ + { + "name": "m2m_changed_user_groups", + "signal": "django.db.models.signals.m2m_changed", + "sender": "django.contrib.auth.models.User.groups.through", + }, + { + "name": "m2m_changed_user_permissions", + "signal": "django.db.models.signals.m2m_changed", + "sender": "django.contrib.auth.models.User.user_permissions.through", + }, + { + "name": "m2m_changed_group_permissions", + "signal": "django.db.models.signals.m2m_changed", + "sender": "django.contrib.auth.models.Group.permissions.through", + }, + { + "name": "m2m_changed_state_permissions", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.authentication.models.State.permissions.through", + }, + { + "name": "check_service_accounts_state_changed", + "signal": "allianceauth.authentication.signals.state_changed", + "sender": None, + }, + { + "name": "pre_delete_user", + "signal": "django.db.models.signals.pre_delete", + "sender": "django.contrib.auth.models.User", + }, + { + "name": "disable_services_on_inactive", + "signal": "django.db.models.signals.pre_save", + "sender": "django.contrib.auth.models.User", + }, + { + "name": "process_main_character_change", + "signal": "django.db.models.signals.pre_save", + "sender": "allianceauth.authentication.models.UserProfile", + }, + { + "name": "process_main_character_update", + "signal": "django.db.models.signals.pre_save", + "sender": "allianceauth.eveonline.models.EveCharacter", + }, + ], + }, + "allianceauth.services.modules.teamspeak3.signals": { + "file": "services/modules/teamspeak3/signals.py", + "ready_file": "services/modules/teamspeak3/apps.py", + "receivers": [ + { + "name": "m2m_changed_authts_group", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.services.modules.teamspeak3.models.AuthTS.ts_group.through", + }, + { + "name": "post_save_authts", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.services.modules.teamspeak3.models.AuthTS", + }, + { + "name": "post_save_authts", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.services.modules.teamspeak3.models.StateGroup", + }, + { + "name": "post_delete_authts", + "signal": "django.db.models.signals.post_delete", + "sender": "allianceauth.services.modules.teamspeak3.models.AuthTS", + }, + { + "name": "post_delete_authts", + "signal": "django.db.models.signals.post_delete", + "sender": "allianceauth.services.modules.teamspeak3.models.StateGroup", + }, + { + "name": "check_groups_on_state_change", + "signal": "allianceauth.authentication.signals.state_changed", + "sender": None, + }, + ], + }, + "allianceauth.eveonline.autogroups.signals": { + "file": "eveonline/autogroups/signals.py", + "ready_file": "eveonline/autogroups/apps.py", + "receivers": [ + { + "name": "pre_save_config", + "signal": "django.db.models.signals.pre_save", + "sender": "allianceauth.eveonline.autogroups.models.AutogroupsConfig", + }, + { + "name": "pre_delete_config", + "signal": "django.db.models.signals.pre_delete", + "sender": "allianceauth.eveonline.autogroups.models.AutogroupsConfig", + }, + { + "name": "check_groups_on_profile_update", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.authentication.models.UserProfile", + }, + { + "name": "autogroups_states_changed", + "signal": "django.db.models.signals.m2m_changed", + "sender": "allianceauth.eveonline.autogroups.models.AutogroupsConfig.states.through", + }, + { + "name": "check_groups_on_character_update", + "signal": "django.db.models.signals.post_save", + "sender": "allianceauth.eveonline.models.EveCharacter", + }, + ], + }, +} + +# Signal modules that intentionally do not participate in the AppConfig.ready() +# startup-loading contract. New entries should be rare and justified. +NOT_TESTED_SIGNAL_MODULES = set() + + +class TestSignalInventory(SimpleTestCase): + maxDiff = None + + def test_all_signal_modules_are_classified(self): + # Welcome to the Twilight Zone + # You created some signals but haven't defined them above. + # Please do so <3 + discovered_modules = self._find_signal_modules() + classified_modules = set(SIGNAL_INVENTORY) | NOT_TESTED_SIGNAL_MODULES + self.assertSetEqual(discovered_modules, classified_modules) + + def test_inventory_matches_decorated_receivers_in_source(self): + for module_name, config in SIGNAL_INVENTORY.items(): + with self.subTest(module=module_name): + actual_receivers = self._find_receiver_names(BASE_DIR / config["file"]) + documented_receivers = {entry["name"] for entry in config["receivers"]} + self.assertSetEqual(actual_receivers, documented_receivers) + + def test_signal_modules_are_loaded_during_startup(self): + for module_name in SIGNAL_INVENTORY: + with self.subTest(module=module_name): + self.assertIn( + module_name, + sys.modules, + msg=( + f"{module_name} was not loaded during Django startup. " + "If this module is expected to auto-register signals, " + "import it from the owning AppConfig.ready()." + ), + ) + + def test_signal_modules_are_imported_in_owning_ready_function(self): + for module_name, config in SIGNAL_INVENTORY.items(): + with self.subTest(module=module_name): + ready_imports = self._find_ready_imports(BASE_DIR / config["ready_file"]) + self.assertIn( + module_name, + ready_imports, + msg=( + f"{module_name} is not imported in " + f"{config['ready_file']}::ready(). " + "Signal modules must be explicitly imported by their owning " + "AppConfig.ready() so registration failures are caught early." + ), + ) + + def test_documented_receivers_are_registered(self): + for module_name, config in SIGNAL_INVENTORY.items(): + module = sys.modules[module_name] + for entry in config["receivers"]: + with self.subTest(module=module_name, receiver=entry["name"]): + receiver = getattr(module, entry["name"]) + signal = self._resolve_dotted_path( + entry["signal"], require_project_module_loaded=True + ) + sender = None + if entry["sender"]: + sender = self._resolve_dotted_path(entry["sender"]) + self.assertIn(receiver, self._live_receivers(signal, sender)) + + @staticmethod + def _find_receiver_names(file_path: Path) -> set[str]: + tree = ast.parse(file_path.read_text()) + names = set() + for node in tree.body: + if not isinstance(node, ast.FunctionDef): + continue + for decorator in node.decorator_list: + if isinstance(decorator, ast.Call) and getattr(decorator.func, "id", None) == "receiver": + names.add(node.name) + break + if isinstance(decorator, ast.Attribute) and decorator.attr == "connect": + names.add(node.name) + break + return names + + @staticmethod + def _find_ready_imports(file_path: Path) -> set[str]: + tree = ast.parse(file_path.read_text()) + imports = set() + for node in tree.body: + if not isinstance(node, ast.ClassDef): + continue + for class_node in node.body: + if not isinstance(class_node, ast.FunctionDef) or class_node.name != "ready": + continue + for statement in class_node.body: + if isinstance(statement, ast.Import): + for alias in statement.names: + imports.add(alias.name) + elif isinstance(statement, ast.ImportFrom): + if statement.module is None: + continue + for alias in statement.names: + imports.add(f"{statement.module}.{alias.name}") + return imports + + @staticmethod + def _find_signal_modules() -> set[str]: + modules = set() + for file_path in BASE_DIR.rglob("signals.py"): + relative_path = file_path.relative_to(BASE_DIR) + module_name = ".".join(("allianceauth",) + relative_path.with_suffix("").parts) + modules.add(module_name) + return modules + + def _resolve_dotted_path( + self, dotted_path: str, require_project_module_loaded: bool = False + ): + parts = dotted_path.split(".") + for index in range(len(parts), 0, -1): + module_name = ".".join(parts[:index]) + module = sys.modules.get(module_name) + if module is None: + if require_project_module_loaded and module_name.startswith("allianceauth."): + continue + try: + module = importlib.import_module(module_name) + except ModuleNotFoundError: + continue + value = module + for attr in parts[index:]: + value = getattr(value, attr) + return value + self.fail(f"Could not resolve dotted path: {dotted_path}") + + @staticmethod + def _live_receivers(signal, sender): + receivers = signal._live_receivers(sender) + if isinstance(receivers, tuple): + sync_receivers, async_receivers = receivers + return sync_receivers + async_receivers + return list(receivers) diff --git a/allianceauth/theme/apps.py b/allianceauth/theme/apps.py index e613c1e6..c45303ac 100644 --- a/allianceauth/theme/apps.py +++ b/allianceauth/theme/apps.py @@ -5,5 +5,5 @@ class ThemeConfig(AppConfig): name = "allianceauth.theme" label = "theme" - def ready(self): + def ready(self) -> None: pass diff --git a/allianceauth/timerboard/apps.py b/allianceauth/timerboard/apps.py index 26759601..0289f0bb 100644 --- a/allianceauth/timerboard/apps.py +++ b/allianceauth/timerboard/apps.py @@ -6,3 +6,6 @@ class TimerBoardConfig(AppConfig): name = 'allianceauth.timerboard' label = 'timerboard' verbose_name = _('Structure Timers') + + def ready(self) -> None: + pass