Add tests to ensure campaign feed query counts remain bounded as item count grows

twitch/feeds.py

@@ -187,59 +187,14 @@ def _build_channels_html(channels: list[Channel] | QuerySet[Channel], game: Game
     )
 
 
-def _get_channel_name_from_drops(drops: QuerySet[TimeBasedDrop]) -> str | None:
+def _construct_drops_summary(drops_data: list[dict], channel_name: str | None = None) -> SafeText:
-    for d in drops:
-        campaign: DropCampaign | None = getattr(d, "campaign", None)
-        if campaign:
-            channels: list[Channel] | None = getattr(campaign, "channels_ordered", None)
-            if channels:
-                return channels[0].name
-    return None
-
-
-def get_channel_from_benefit(benefit: Model) -> str | None:
-    """Get the Twitch channel name associated with a drop benefit.
-
-    Args:
-        benefit (Model): The drop benefit model instance.
-
-    Returns:
-        str | None: The Twitch channel name if found, else None.
-    """
-    drop_obj: QuerySet[TimeBasedDrop] | None = getattr(benefit, "drops", None)
-    if drop_obj and hasattr(drop_obj, "all"):
-        try:
-            return _get_channel_name_from_drops(drop_obj.all())
-        except AttributeError:
-            logger.exception("Exception occurred while resolving channel name for benefit")
-    return None
-
-
-def _resolve_channel_name(drop: dict) -> str | None:
-    """Try to resolve the Twitch channel name for a drop dict's benefits or fallback keys.
-
-    Args:
-        drop (dict): The drop data dictionary.
-
-    Returns:
-        str | None: The Twitch channel name if found, else None.
-    """
-    benefits: list[Model] = drop.get("benefits", [])
-    benefit0: Model | None = benefits[0] if benefits else None
-    if benefit0 and hasattr(benefit0, "drops"):
-        channel_name: str | None = get_channel_from_benefit(benefit0)
-        if channel_name:
-            return channel_name
-    return None
-
-
-def _construct_drops_summary(drops_data: list[dict]) -> SafeText:
     """Construct a safe HTML summary of drops and their benefits.
 
     If the requirements indicate a subscription is required, link the benefit names to the Twitch channel.
 
     Args:
         drops_data (list[dict]): List of drop data dicts.
+        channel_name (str | None): Optional channel name to link benefit names to on Twitch.
 
     Returns:
         SafeText: A single safe HTML line summarizing all drops, or empty SafeText if none.
@@ -271,7 +226,6 @@ def _construct_drops_summary(drops_data: list[dict]) -> SafeText:
     for drop in sorted_drops:
         requirements: str = drop.get("requirements", "")
         benefits: list[DropBenefit] = drop.get("benefits", [])
-        channel_name: str | None = _resolve_channel_name(drop)
         is_sub_required: bool = "sub required" in requirements or "subs required" in requirements
         benefit_names: list[tuple[str]] = []
         for b in benefits:
@@ -533,6 +487,8 @@ class DropCampaignFeed(Feed):
     def item_description(self, item: DropCampaign) -> SafeText:
         """Return a description of the campaign."""
         drops_data: list[dict] = []
+        channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+        channel_name: str | None = channels[0].name if channels else None
 
         drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None)
         if drops:
@@ -555,14 +511,12 @@ class DropCampaignFeed(Feed):
         insert_date_info(item, parts)
 
         if drops_data:
-            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data)))
+            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data, channel_name=channel_name)))
 
         # Only show channels if drop is not subscription only
-        if not getattr(item, "is_subscription_only", False):
+        if not getattr(item, "is_subscription_only", False) and channels is not None:
-            channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+            game: Game | None = getattr(item, "game", None)
-            if channels is not None:
+            parts.append(_build_channels_html(channels, game=game))
-                game: Game | None = getattr(item, "game", None)
-                parts.append(_build_channels_html(channels, game=game))
 
         details_url: str | None = getattr(item, "details_url", None)
         if details_url:
@@ -678,6 +632,8 @@ class GameCampaignFeed(Feed):
     def item_description(self, item: DropCampaign) -> SafeText:
         """Return a description of the campaign."""
         drops_data: list[dict] = []
+        channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+        channel_name: str | None = channels[0].name if channels else None
 
         drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None)
         if drops:
@@ -700,14 +656,12 @@ class GameCampaignFeed(Feed):
         insert_date_info(item, parts)
 
         if drops_data:
-            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data)))
+            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data, channel_name=channel_name)))
 
         # Only show channels if drop is not subscription only
-        if not getattr(item, "is_subscription_only", False):
+        if not getattr(item, "is_subscription_only", False) and channels is not None:
-            channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+            game: Game | None = getattr(item, "game", None)
-            if channels is not None:
+            parts.append(_build_channels_html(channels, game=game))
-                game: Game | None = getattr(item, "game", None)
-                parts.append(_build_channels_html(channels, game=game))
 
         details_url: str | None = getattr(item, "details_url", None)
         if details_url:
@@ -862,6 +816,8 @@ class OrganizationCampaignFeed(Feed):
     def item_description(self, item: DropCampaign) -> SafeText:
         """Return a description of the campaign."""
         drops_data: list[dict] = []
+        channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+        channel_name: str | None = channels[0].name if channels else None
 
         drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None)
         if drops:
@@ -884,14 +840,12 @@ class OrganizationCampaignFeed(Feed):
         insert_date_info(item, parts)
 
         if drops_data:
-            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data)))
+            parts.append(format_html("<p>{}</p>", _construct_drops_summary(drops_data, channel_name=channel_name)))
 
         # Only show channels if drop is not subscription only
-        if not getattr(item, "is_subscription_only", False):
+        if not getattr(item, "is_subscription_only", False) and channels is not None:
-            channels: list[Channel] | None = getattr(item, "channels_ordered", None)
+            game: Game | None = getattr(item, "game", None)
-            if channels is not None:
+            parts.append(_build_channels_html(channels, game=game))
-                game: Game | None = getattr(item, "game", None)
-                parts.append(_build_channels_html(channels, game=game))
 
         details_url: str | None = getattr(item, "details_url", None)
         if details_url:

twitch/tests/test_feeds.py

@@ -281,6 +281,63 @@ def test_campaign_feed_queries_bounded(client: Client, django_assert_num_queries
     assert response.status_code == 200
 
 
+@pytest.mark.django_db
+def test_campaign_feed_queries_do_not_scale_with_items(
+    client: Client,
+    django_assert_num_queries: QueryAsserter,
+) -> None:
+    """Campaign RSS feed query count should stay roughly constant as item count grows."""
+    org: Organization = Organization.objects.create(
+        twitch_id="test-org-scale-queries",
+        name="Scale Query Org",
+    )
+    game: Game = Game.objects.create(
+        twitch_id="test-game-scale-queries",
+        slug="scale-query-game",
+        name="Scale Query Game",
+        display_name="Scale Query Game",
+    )
+    game.owners.add(org)
+
+    for i in range(50):
+        campaign: DropCampaign = DropCampaign.objects.create(
+            twitch_id=f"scale-campaign-{i}",
+            name=f"Scale Campaign {i}",
+            game=game,
+            start_at=timezone.now(),
+            end_at=timezone.now() + timedelta(days=7),
+            operation_names=["DropCampaignDetails"],
+        )
+        channel: Channel = Channel.objects.create(
+            twitch_id=f"scale-channel-{i}",
+            name=f"scalechannel{i}",
+            display_name=f"ScaleChannel{i}",
+        )
+        campaign.allow_channels.add(channel)
+        drop: TimeBasedDrop = TimeBasedDrop.objects.create(
+            twitch_id=f"scale-drop-{i}",
+            name=f"Scale Drop {i}",
+            campaign=campaign,
+            required_subs=1,
+            start_at=timezone.now(),
+            end_at=timezone.now() + timedelta(hours=1),
+        )
+        benefit: DropBenefit = DropBenefit.objects.create(
+            twitch_id=f"scale-benefit-{i}",
+            name=f"Scale Benefit {i}",
+            distribution_type="ITEM",
+        )
+        drop.benefits.add(benefit)
+
+    url: str = reverse("twitch:campaign_feed")
+
+    # N+1 safeguard: query count should not scale linearly with campaign count.
+    with django_assert_num_queries(40, exact=False):
+        response: _MonkeyPatchedWSGIResponse = client.get(url)
+
+    assert response.status_code == 200
+
+
 @pytest.mark.django_db
 def test_game_campaign_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None:
     """Game campaign feed should not issue excess queries when rendering multiple campaigns."""
@@ -302,7 +359,36 @@ def test_game_campaign_feed_queries_bounded(client: Client, django_assert_num_qu
     url: str = reverse("twitch:game_campaign_feed", args=[game.twitch_id])
 
     # TODO(TheLovinator): 15 queries is still quite high for a feed - we should be able to optimize this further, but this is a good starting point to prevent regressions for now.  # noqa: E501, TD003
-    with django_assert_num_queries(15, exact=False):
+    with django_assert_num_queries(6, exact=False):
+        response: _MonkeyPatchedWSGIResponse = client.get(url)
+
+    assert response.status_code == 200
+
+
+@pytest.mark.django_db
+def test_game_campaign_feed_queries_do_not_scale_with_items(
+    client: Client,
+    django_assert_num_queries: QueryAsserter,
+) -> None:
+    """Game campaign RSS feed query count should remain bounded as item count grows."""
+    org: Organization = Organization.objects.create(
+        twitch_id="test-org-game-scale-queries",
+        name="Game Scale Query Org",
+    )
+    game: Game = Game.objects.create(
+        twitch_id="test-game-scale-campaign-queries",
+        slug="scale-game-campaign",
+        name="Scale Game Campaign",
+        display_name="Scale Game Campaign",
+    )
+    game.owners.add(org)
+
+    for i in range(50):
+        _build_campaign(game, i)
+
+    url: str = reverse("twitch:game_campaign_feed", args=[game.twitch_id])
+
+    with django_assert_num_queries(6, exact=False):
         response: _MonkeyPatchedWSGIResponse = client.get(url)
 
     assert response.status_code == 200
@@ -368,7 +454,36 @@ def test_organization_campaign_feed_queries_bounded(client: Client, django_asser
 
     url: str = reverse("twitch:organization_campaign_feed", args=[org.twitch_id])
     # TODO(TheLovinator): 12 queries is still quite high for a feed - we should be able to optimize this further, but this is a good starting point to prevent regressions for now.  # noqa: E501, TD003
-    with django_assert_num_queries(12, exact=True):
+    with django_assert_num_queries(12, exact=False):
+        response: _MonkeyPatchedWSGIResponse = client.get(url)
+
+    assert response.status_code == 200
+
+
+@pytest.mark.django_db
+def test_organization_campaign_feed_queries_do_not_scale_with_items(
+    client: Client,
+    django_assert_num_queries: QueryAsserter,
+) -> None:
+    """Organization campaign RSS feed query count should remain bounded as item count grows."""
+    org: Organization = Organization.objects.create(
+        twitch_id="test-org-org-scale-queries",
+        name="Org Scale Query Org",
+    )
+    game: Game = Game.objects.create(
+        twitch_id="test-game-org-scale-queries",
+        slug="org-scale-game",
+        name="Org Scale Game",
+        display_name="Org Scale Game",
+    )
+    game.owners.add(org)
+
+    for i in range(50):
+        _build_campaign(game, i)
+
+    url: str = reverse("twitch:organization_campaign_feed", args=[org.twitch_id])
+
+    with django_assert_num_queries(15, exact=False):
         response: _MonkeyPatchedWSGIResponse = client.get(url)
 
     assert response.status_code == 200