diff --git a/twitch/feeds.py b/twitch/feeds.py index 4588b06..783bdeb 100644 --- a/twitch/feeds.py +++ b/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: - 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: +def _construct_drops_summary(drops_data: list[dict], channel_name: str | None = None) -> 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("
{}
", _construct_drops_summary(drops_data))) + parts.append(format_html("{}
", _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): - channels: list[Channel] | None = getattr(item, "channels_ordered", None) - if channels is not None: - game: Game | None = getattr(item, "game", None) - parts.append(_build_channels_html(channels, game=game)) + if not getattr(item, "is_subscription_only", False) and channels is not None: + 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("{}
", _construct_drops_summary(drops_data))) + parts.append(format_html("{}
", _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): - channels: list[Channel] | None = getattr(item, "channels_ordered", None) - if channels is not None: - game: Game | None = getattr(item, "game", None) - parts.append(_build_channels_html(channels, game=game)) + if not getattr(item, "is_subscription_only", False) and channels is not None: + 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("{}
", _construct_drops_summary(drops_data))) + parts.append(format_html("{}
", _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): - channels: list[Channel] | None = getattr(item, "channels_ordered", None) - if channels is not None: - game: Game | None = getattr(item, "game", None) - parts.append(_build_channels_html(channels, game=game)) + if not getattr(item, "is_subscription_only", False) and channels is not None: + 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: diff --git a/twitch/tests/test_feeds.py b/twitch/tests/test_feeds.py index f2d0448..31e7abf 100644 --- a/twitch/tests/test_feeds.py +++ b/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