fix: resolve security, reliability, and performance issues found in code review

.gitignore

@@ -22,3 +22,4 @@ _build/
 alliance_auth.sqlite3
 .temp/
 .env
+.claude/

structures/admin.py

@@ -326,7 +326,6 @@ def add_or_remove_timer(self, request, queryset):
             message = format_lazy("{first} {second}", first=first, second=second)
         else:
             message = first
-        message = format_lazy(first)
         self.message_user(request, message)
 
 

structures/core/notification_embeds/corporate_embeds.py

@@ -38,12 +38,11 @@ def __init__(self, notification: Notification) -> None:
         self._description = _(
             "New application from %(character_name)s to join %(corporation_name)s:\n"
             "> %(application_text)s"
-            % {
-                "character_name": self._character_link,
-                "corporation_name": self._corporation_link,
-                "application_text": self._application_text,
-            }
-        )
+        ) % {
+            "character_name": self._character_link,
+            "corporation_name": self._corporation_link,
+            "application_text": self._application_text,
+        }
         self._color = Webhook.Color.INFO
 
 
@@ -178,11 +177,10 @@ def __init__(self, notification: Notification) -> None:
         self._description = _(
             "Project **%(goal_name)s** has ben closed by %(closer)s "
             "and will not accept further contributions."
-            % {
-                "goal_name": self._goal_name,
-                "closer": gen_eve_entity_link(closer),
-            }
-        )
+        ) % {
+            "goal_name": self._goal_name,
+            "closer": gen_eve_entity_link(closer),
+        }
         self._color = Webhook.Color.INFO
 
 
@@ -193,11 +191,10 @@ def __init__(self, notification: Notification) -> None:
         self._description = _(
             "Project **%(goal_name)s** created by %(creator)s "
             "has been successfully completed after reaching it's target."
-            % {
-                "goal_name": self._goal_name,
-                "creator": self._creator_link,
-            }
-        )
+        ) % {
+            "goal_name": self._goal_name,
+            "creator": self._creator_link,
+        }
         self._color = Webhook.Color.SUCCESS
 
 
@@ -208,9 +205,8 @@ def __init__(self, notification: Notification) -> None:
         self._description = _(
             "Project **%(goal_name)s** has been created by %(creator)s "
             "and is open for contributions."
-            % {
-                "goal_name": self._goal_name,
-                "creator": self._creator_link,
-            }
-        )
+        ) % {
+            "goal_name": self._goal_name,
+            "creator": self._creator_link,
+        }
         self._color = Webhook.Color.INFO

structures/core/notification_embeds/skyhook_embeds.py

@@ -77,12 +77,8 @@ class NotificationSkyhookUnderAttack(NotificationSkyhookEmbed):
     def __init__(self, n: Notification) -> None:
         super().__init__(n)
         self._title = _("Skyhook under attack")
-        self._description += _(
-            "is under attack by %(attacker)s.\n"
-            "%(damage_text)s"
-            % {
-                "attacker": self.gen_attacker_link(),
-                "damage_text": self.compile_damage_text("Percentage"),
-            }
-        )
+        self._description += _("is under attack by %(attacker)s.\n%(damage_text)s") % {
+            "attacker": self.gen_attacker_link(),
+            "damage_text": self.compile_damage_text("Percentage"),
+        }
         self._color = Webhook.Color.DANGER

structures/core/notification_embeds/structures_embeds.py

@@ -95,7 +95,9 @@ def __init__(self, notification: Notification) -> None:
         super().__init__(notification)
         self._title = _("Jump gate low on Liquid Ozone")
         threshold_str = f"{self._data['threshold']:,}"
-        quantity_str = f"{self._structure.jump_fuel_quantity():,}"
+        quantity_str = (
+            f"{self._structure.jump_fuel_quantity():,}" if self._structure else "?"
+        )
         self._description += _(
             "is below %(threshold)s units on Liquid Ozone.\n"
             "Remaining units: %(remaining)s."
@@ -262,13 +264,19 @@ class NotificationStructureReinforceChange(NotificationBaseEmbed):
 
     def __init__(self, notification: Notification) -> None:
         super().__init__(notification)
+        structure_ids = [
+            structure_info[0] for structure_info in self._data["allStructureInfo"]
+        ]
+        structures_by_id = {
+            structure.id: structure
+            for structure in Structure.objects.select_related_defaults().filter(
+                id__in=structure_ids
+            )
+        }
         all_structure_info = []
         for structure_info in self._data["allStructureInfo"]:
-            try:
-                structure = Structure.objects.select_related_defaults().get(
-                    id=structure_info[0]
-                )
-            except Structure.DoesNotExist:
+            structure = structures_by_id.get(structure_info[0])
+            if structure is None:
                 all_structure_info.append(
                     self.StructureInfo(
                         name=structure_info[1],

structures/core/notification_embeds/war_embeds.py

@@ -296,7 +296,7 @@ def __init__(self, notification: Notification) -> None:
 class NotificationWarHQRemovedFromSpace(NotificationWarBaseEmbed):
     def __init__(self, notification: Notification) -> None:
         super().__init__(notification)
-        war_hq = self._data["warHQ"]
+        war_hq = strip_tags(self._data["warHQ"])
         time_declared = ldap_time_2_datetime(self._data["timeDeclared"])
         self._title = _("WarHQ %s lost") % war_hq
         self._description = _(

structures/core/notification_timers.py

@@ -323,8 +323,6 @@ def _gen_timer_moon_extraction(notif: Notification) -> bool:
                 timer_processed = True
 
         case NotificationType.MOONMINING_EXTRACTION_CANCELLED:
-            timer_processed = True
-
             for notification in Notification.objects.filter(
                 notif_type=NotificationType.MOONMINING_EXTRACTION_STARTED,
                 owner=notif.owner,
@@ -335,6 +333,7 @@ def _gen_timer_moon_extraction(notif: Notification) -> bool:
                 parsed_text_2 = notification.parsed_text()
                 my_structure_type_id = parsed_text_2["structureTypeID"]
                 if my_structure_type_id == parsed_text["structureTypeID"]:
+                    timer_processed = True
                     eve_time_2 = _extract_eve_time(parsed_text_2)
 
                     if AuthTimer:

structures/core/serializers.py

@@ -140,10 +140,10 @@ def _add_name_and_tags(
         self, structure: Structure, row: dict, check_tags: bool = True
     ):
         structure_name_html = escape(structure.name)
-        tags = []
-        if check_tags and structure.tags.exists():
-            tags += [x.html for x in structure.tags.all()]
-            structure_name_html += format_html("<br>{}", mark_safe(" ".join(tags)))
+        if check_tags:
+            tags = [x.html for x in structure.tags.all()]
+            if tags:
+                structure_name_html += format_html("<br>{}", mark_safe(" ".join(tags)))
 
         row["structure_name_and_tags"] = structure_name_html
 

structures/models/notifications.py

@@ -309,10 +309,7 @@ def relevant_webhooks(self) -> models.QuerySet:
         else:
             structures_qs = self.calc_related_structures()
 
-        if (
-            structures_qs.exists()
-            and structures_qs.filter(webhooks__isnull=False).count() == 1
-        ):
+        if structures_qs.count() == 1 and structures_qs.first().webhooks.exists():
             webhooks_qs = structures_qs.first().webhooks
         else:
             webhooks_qs = self.owner.webhooks
@@ -896,6 +893,7 @@ def relevant_webhooks(self) -> models.QuerySet:
             )
             .filter(is_active=True)
             .filter(Q(owners__isnull=False) | Q(structures__isnull=False))
+            .distinct()
         )
 
 

structures/models/owners.py

@@ -1215,6 +1215,7 @@ def send_new_notifications(self, user: Optional[User] = None):
         )
         for notif in new_eve_notifications:
             notif.send_to_configured_webhooks()
+            notifications_count += 1
 
         new_generated_notifications: models.QuerySet[GeneratedNotification] = (
             self.generatednotification_set.filter(**my_filter)
@@ -1223,6 +1224,7 @@ def send_new_notifications(self, user: Optional[User] = None):
         )
         for notif in new_generated_notifications:
             notif.send_to_configured_webhooks()
+            notifications_count += 1
 
         if (
             not new_eve_notifications.exists()

structures/models/structures_1.py

@@ -259,6 +259,8 @@ class PowerMode(models.TextChoices):
         verbose_name=_("fuel expires at"),
         help_text=_("Date on which the structure will run out of fuel"),
     )
+    # despite the name, also true when the structure only has fuel/cargo/fighters, not just a fit
+    # TODO: rename to has_items with the next migration
     has_fitting = models.BooleanField(
         null=True,
         default=None,

structures/providers.py

@@ -20,10 +20,8 @@
         "GetCorporationsCorporationIdStructures",
         "GetSovereigntyMap",
         "GetUniverseStructuresStructureId",
-        "GetUniverseStructuresStructureId",
         "PostCorporationsCorporationIdAssetsNames",
         "PostCorporationsCorporationIdAssetsLocations",
-        "PostCorporationsCorporationIdAssetsNames",
     ],
     spec_file=spec_file,
 )

structures/tasks.py

@@ -49,14 +49,15 @@ def update_sov_map(_self):
 @shared_task(time_limit=STRUCTURES_TASKS_TIME_LIMIT)
 def update_structures():
     """Update all structures for all active owners from ESI."""
-    for owner in Owner.objects.all():
-        if owner.is_active:
-            update_structures_for_owner.delay(owner.pk)
+    has_active_owner = False
+    has_alliance_main = False
+    for owner in Owner.objects.filter(is_active=True):
+        has_active_owner = True
+        update_structures_for_owner.delay(owner.pk)
+        if owner.is_alliance_main:
+            has_alliance_main = True
 
-    if (
-        Owner.objects.filter(is_active=True).count() > 0
-        and Owner.objects.filter(is_active=True, is_alliance_main=True).count() == 0
-    ):
+    if has_active_owner and not has_alliance_main:
         logger.warning(
             "No owner configured to process alliance wide notifications. "
             "Please set 'is alliance main' to True for the designated owner."
@@ -204,7 +205,11 @@ def generate_new_timers_for_owner(owner_pk: int):
     owner.add_or_remove_timers_from_notifications()
 
 
-@shared_task(time_limit=STRUCTURES_TASKS_TIME_LIMIT)
+@shared_task(
+    base=QueueOnce,
+    once={"keys": ["owner_pk"]},
+    time_limit=STRUCTURES_TASKS_TIME_LIMIT,
+)
 def send_new_notifications_for_owner(owner_pk: int):
     """Send new notifications to Discord."""
     owner = Owner.objects.get(pk=owner_pk)

structures/tests/core/notification_embeds/test_main.py

@@ -1,9 +1,12 @@
 import datetime as dt
+import re
 
 import dhooks_lite
 
 from django.core.cache import cache
+from django.db import connection
 from django.test import TestCase, override_settings
+from django.test.utils import CaptureQueriesContext
 from django.utils.timezone import now
 from eveuniverse.tests.testdata.factories_2 import EveMoonFactory, EveSolarSystemFactory
 
@@ -240,6 +243,23 @@ def test_should_set_special_footer_for_generated_notifications(self):
         self.assertEqual(discord_embed.footer.text, "Structures")
         self.assertIn("structures_logo.png", discord_embed.footer.icon_url)
 
+    def test_should_generate_jump_fuel_alert_embed_when_structure_no_longer_exists(
+        self,
+    ):
+        # given
+        structure = StructureFactory(owner=self.owner)
+        notification = Notification.create_from_structure(
+            structure,
+            notif_type=NotificationType.STRUCTURE_JUMP_FUEL_ALERT,
+            threshold=1000,
+        )
+        structure.delete()
+        notification_embed = NotificationBaseEmbed.create(notification)
+        # when
+        discord_embed = notification_embed.generate_embed()
+        # then
+        self.assertIsInstance(discord_embed, dhooks_lite.Embed)
+
     def test_should_not_break_with_too_large_description(self):
         # given
         notification = Notification.objects.get(notification_id=1000000403)
@@ -404,4 +424,52 @@ def test_should_create_sov_embed_without_sender(self):
         self.assertTrue(obj.description)
         self.assertTrue(obj.description)
         self.assertTrue(obj.description)
+
+
+class TestNotificationStructureReinforceChange(NoSocketsTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.owner = OwnerFactory()
+
+    def test_should_batch_fetch_structures_and_handle_missing_ones(self):
+        # given
+        structure_1 = StructureFactory(owner=self.owner)
+        structure_2 = StructureFactory(owner=self.owner)
+        missing_structure_id = 1_599_999_999_999
+        notif = NotificationFactory(
+            owner=self.owner,
+            notif_type=NotificationType.STRUCTURE_REINFORCEMENT_CHANGED,
+            text_from_dict={
+                "allStructureInfo": [
+                    [structure_1.id, structure_1.name, structure_1.eve_type_id],
+                    [
+                        missing_structure_id,
+                        "Ghost Structure",
+                        structure_2.eve_type_id,
+                    ],
+                    [structure_2.id, structure_2.name, structure_2.eve_type_id],
+                ],
+                "hour": 19,
+                "numStructures": 3,
+                "timestamp": 132141703753688216,
+                "weekday": 255,
+            },
+        )
+        # when
+        with CaptureQueriesContext(connection) as ctx:
+            embed = NotificationBaseEmbed.create(notif)
+        # then: structures are fetched with a single batched query, not one per entry
+        structure_queries = [
+            query
+            for query in ctx.captured_queries
+            if re.search(r"\bstructures_structure\b", query["sql"])
+        ]
+        self.assertEqual(len(structure_queries), 1)
+
+        obj = embed.generate_embed()
+        self.assertIn(structure_1.name, obj.description)
+        self.assertIn(structure_2.name, obj.description)
+        self.assertIn("Ghost Structure", obj.description)
+        self.assertIn("(unknown)", obj.description)
         self.assertTrue(obj.description)