diff --git a/twitch/apps.py b/twitch/apps.py index 07e8fe2..9b8d77f 100644 --- a/twitch/apps.py +++ b/twitch/apps.py @@ -39,6 +39,7 @@ class TwitchConfig(AppConfig): # Register post_save signal handlers that dispatch image download tasks # when new Twitch records are created. + from django.db.models.signals import m2m_changed # noqa: I001, PLC0415 from django.db.models.signals import post_save # noqa: PLC0415 from twitch.models import DropBenefit # noqa: PLC0415 @@ -46,6 +47,7 @@ class TwitchConfig(AppConfig): from twitch.models import Game # noqa: PLC0415 from twitch.models import RewardCampaign # noqa: PLC0415 from twitch.signals import on_drop_benefit_saved # noqa: PLC0415 + from twitch.signals import on_drop_campaign_allow_channels_changed # noqa: PLC0415 from twitch.signals import on_drop_campaign_saved # noqa: PLC0415 from twitch.signals import on_game_saved # noqa: PLC0415 from twitch.signals import on_reward_campaign_saved # noqa: PLC0415 @@ -54,3 +56,8 @@ class TwitchConfig(AppConfig): post_save.connect(on_drop_campaign_saved, sender=DropCampaign) post_save.connect(on_drop_benefit_saved, sender=DropBenefit) post_save.connect(on_reward_campaign_saved, sender=RewardCampaign) + m2m_changed.connect( + on_drop_campaign_allow_channels_changed, + sender=DropCampaign.allow_channels.through, + dispatch_uid="twitch_drop_campaign_allow_channels_counter_cache", + ) diff --git a/twitch/migrations/0021_channel_allowed_campaign_count_cache.py b/twitch/migrations/0021_channel_allowed_campaign_count_cache.py new file mode 100644 index 0000000..53c93c0 --- /dev/null +++ b/twitch/migrations/0021_channel_allowed_campaign_count_cache.py @@ -0,0 +1,83 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from django.db import migrations +from django.db import models + +if TYPE_CHECKING: + from django.db.backends.base.schema import BaseDatabaseSchemaEditor + from django.db.migrations.state import StateApps + + from twitch.models import Channel + from twitch.models import DropCampaign + + +def backfill_allowed_campaign_count( + apps: StateApps, + schema_editor: BaseDatabaseSchemaEditor, +) -> None: + """Populate Channel.allowed_campaign_count from the M2M through table.""" + del schema_editor + + Channel: type[Channel] = apps.get_model("twitch", "Channel") + DropCampaign: type[DropCampaign] = apps.get_model("twitch", "DropCampaign") + through_model: type[Channel] = DropCampaign.allow_channels.through + + counts_by_channel = { + row["channel_id"]: row["campaign_count"] + for row in ( + through_model.objects.values("channel_id").annotate( + campaign_count=models.Count("dropcampaign_id"), + ) + ) + } + + channels: list[Channel] = list( + Channel.objects.all().only("id", "allowed_campaign_count"), + ) + for channel in channels: + channel.allowed_campaign_count = counts_by_channel.get(channel.pk, 0) + + if channels: + Channel.objects.bulk_update( + channels, + ["allowed_campaign_count"], + batch_size=1000, + ) + + +def noop_reverse( + apps: StateApps, + schema_editor: BaseDatabaseSchemaEditor, +) -> None: + """No-op reverse migration for cached counters.""" + del apps + del schema_editor + + +class Migration(migrations.Migration): + """Add cached channel campaign counts and backfill existing rows.""" + + dependencies = [ + ("twitch", "0020_rewardcampaign_tw_reward_ends_starts_idx"), + ] + + operations = [ + migrations.AddField( + model_name="channel", + name="allowed_campaign_count", + field=models.PositiveIntegerField( + default=0, + help_text="Cached number of drop campaigns that allow this channel.", + ), + ), + migrations.AddIndex( + model_name="channel", + index=models.Index( + fields=["-allowed_campaign_count", "name"], + name="tw_chan_cc_name_idx", + ), + ), + migrations.RunPython(backfill_allowed_campaign_count, noop_reverse), + ] diff --git a/twitch/models.py b/twitch/models.py index f1e7a79..070c6f0 100644 --- a/twitch/models.py +++ b/twitch/models.py @@ -2,12 +2,15 @@ import logging from collections import OrderedDict from typing import TYPE_CHECKING from typing import Any +from typing import Self import auto_prefetch from django.conf import settings from django.contrib.postgres.indexes import GinIndex from django.db import models +from django.db.models import F from django.db.models import Prefetch +from django.db.models import Q from django.urls import reverse from django.utils import timezone from django.utils.html import format_html @@ -347,6 +350,11 @@ class Channel(auto_prefetch.Model): auto_now=True, ) + allowed_campaign_count = models.PositiveIntegerField( + help_text="Cached number of drop campaigns that allow this channel.", + default=0, + ) + class Meta(auto_prefetch.Model.Meta): ordering = ["display_name"] indexes = [ @@ -355,12 +363,47 @@ class Channel(auto_prefetch.Model): models.Index(fields=["twitch_id"]), models.Index(fields=["added_at"]), models.Index(fields=["updated_at"]), + models.Index( + fields=["-allowed_campaign_count", "name"], + name="tw_chan_cc_name_idx", + ), ] def __str__(self) -> str: """Return a string representation of the channel.""" return self.display_name or self.name or self.twitch_id + @classmethod + def for_list_view( + cls, + search_query: str | None = None, + ) -> models.QuerySet[Channel]: + """Return channels for list view with campaign counts. + + Args: + search_query: Optional free-text search for username/display name. + + Returns: + QuerySet optimized for channel list rendering. + """ + queryset: models.QuerySet[Self, Self] = cls.objects.only( + "twitch_id", + "name", + "display_name", + "allowed_campaign_count", + ) + + normalized_query: str = (search_query or "").strip() + if normalized_query: + queryset = queryset.filter( + Q(name__icontains=normalized_query) + | Q(display_name__icontains=normalized_query), + ) + + return queryset.annotate( + campaign_count=F("allowed_campaign_count"), + ).order_by("-campaign_count", "name") + # MARK: DropCampaign class DropCampaign(auto_prefetch.Model): diff --git a/twitch/signals.py b/twitch/signals.py index f8f66af..b1e0b98 100644 --- a/twitch/signals.py +++ b/twitch/signals.py @@ -3,6 +3,8 @@ from __future__ import annotations import logging from typing import Any +from django.db.models import Count + logger = logging.getLogger("ttvdrops.signals") @@ -63,3 +65,65 @@ def on_reward_campaign_saved( from twitch.tasks import download_reward_campaign_image # noqa: PLC0415 _dispatch(download_reward_campaign_image, instance.pk) + + +def _refresh_allowed_campaign_counts(channel_ids: set[int]) -> None: + """Recompute and persist cached campaign counters for the given channels.""" + if not channel_ids: + return + + from twitch.models import Channel # noqa: PLC0415 + from twitch.models import DropCampaign # noqa: PLC0415 + + through_model: type[Channel] = DropCampaign.allow_channels.through + counts_by_channel: dict[int, int] = { + row["channel_id"]: row["campaign_count"] + for row in ( + through_model.objects + .filter(channel_id__in=channel_ids) + .values("channel_id") + .annotate(campaign_count=Count("dropcampaign_id")) + ) + } + + channels = list( + Channel.objects.filter(pk__in=channel_ids).only("pk", "allowed_campaign_count"), + ) + for channel in channels: + channel.allowed_campaign_count = counts_by_channel.get(channel.pk, 0) + + if channels: + Channel.objects.bulk_update(channels, ["allowed_campaign_count"]) + + +def on_drop_campaign_allow_channels_changed( # noqa: PLR0913, PLR0917 + sender: Any, # noqa: ANN401 + instance: Any, # noqa: ANN401 + action: str, + reverse: bool, # noqa: FBT001 + model: Any, # noqa: ANN401 + pk_set: set[int] | None, + **kwargs: Any, # noqa: ANN401 +) -> None: + """Keep Channel.allowed_campaign_count in sync for allow_channels M2M changes.""" + if action == "pre_clear" and not reverse: + # post_clear does not expose removed channel IDs; snapshot before clearing. + instance._pre_clear_channel_ids = set( # pyright: ignore[reportAttributeAccessIssue] # noqa: SLF001 + instance.allow_channels.values_list("pk", flat=True), # pyright: ignore[reportAttributeAccessIssue] + ) + return + + if action not in {"post_add", "post_remove", "post_clear"}: + return + + channel_ids: set[int] = set() + if reverse: + channel_pk: int | None = getattr(instance, "pk", None) + if isinstance(channel_pk, int): + channel_ids.add(channel_pk) + elif action == "post_clear": + channel_ids = set(getattr(instance, "_pre_clear_channel_ids", set())) + else: + channel_ids = set(pk_set or set()) + + _refresh_allowed_campaign_counts(channel_ids) diff --git a/twitch/tests/test_migrations.py b/twitch/tests/test_migrations.py new file mode 100644 index 0000000..9972ff8 --- /dev/null +++ b/twitch/tests/test_migrations.py @@ -0,0 +1,86 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest +from django.db import connection +from django.db.migrations.executor import MigrationExecutor +from django.db.migrations.state import StateApps + +if TYPE_CHECKING: + from django.db.migrations.state import StateApps + + from twitch.models import Channel + from twitch.models import DropCampaign + from twitch.models import Game + + +@pytest.mark.django_db(transaction=True) +def test_0021_backfills_allowed_campaign_count() -> None: # noqa: PLR0914 + """Migration 0021 should backfill cached allowed campaign counts.""" + migrate_from: list[tuple[str, str]] = [ + ("twitch", "0020_rewardcampaign_tw_reward_ends_starts_idx"), + ] + migrate_to: list[tuple[str, str]] = [ + ("twitch", "0021_channel_allowed_campaign_count_cache"), + ] + + executor = MigrationExecutor(connection) + executor.migrate(migrate_from) + old_apps: StateApps = executor.loader.project_state(migrate_from).apps + + Game: type[Game] = old_apps.get_model("twitch", "Game") + Channel: type[Channel] = old_apps.get_model("twitch", "Channel") + DropCampaign: type[DropCampaign] = old_apps.get_model("twitch", "DropCampaign") + + game = Game.objects.create( + twitch_id="migration_backfill_game", + name="Migration Backfill Game", + display_name="Migration Backfill Game", + ) + channel1 = Channel.objects.create( + twitch_id="migration_backfill_channel_1", + name="migrationbackfillchannel1", + display_name="Migration Backfill Channel 1", + ) + channel2 = Channel.objects.create( + twitch_id="migration_backfill_channel_2", + name="migrationbackfillchannel2", + display_name="Migration Backfill Channel 2", + ) + _channel3 = Channel.objects.create( + twitch_id="migration_backfill_channel_3", + name="migrationbackfillchannel3", + display_name="Migration Backfill Channel 3", + ) + campaign1 = DropCampaign.objects.create( + twitch_id="migration_backfill_campaign_1", + name="Migration Backfill Campaign 1", + game=game, + operation_names=["DropCampaignDetails"], + ) + campaign2 = DropCampaign.objects.create( + twitch_id="migration_backfill_campaign_2", + name="Migration Backfill Campaign 2", + game=game, + operation_names=["DropCampaignDetails"], + ) + + campaign1.allow_channels.add(channel1, channel2) + campaign2.allow_channels.add(channel1) + + executor = MigrationExecutor(connection) + executor.migrate(migrate_to) + new_apps: StateApps = executor.loader.project_state(migrate_to).apps + new_channel: type[Channel] = new_apps.get_model("twitch", "Channel") + + counts_by_twitch_id: dict[str, int] = { + channel.twitch_id: channel.allowed_campaign_count + for channel in new_channel.objects.order_by("twitch_id") + } + + assert counts_by_twitch_id == { + "migration_backfill_channel_1": 2, + "migration_backfill_channel_2": 1, + "migration_backfill_channel_3": 0, + } diff --git a/twitch/tests/test_views.py b/twitch/tests/test_views.py index 4421e71..1307be9 100644 --- a/twitch/tests/test_views.py +++ b/twitch/tests/test_views.py @@ -494,6 +494,115 @@ class TestChannelListView: assert len(channels) == 1 assert channels[0].twitch_id == channel.twitch_id + def test_channel_list_queryset_only_selects_rendered_fields(self) -> None: + """Channel list queryset should defer non-rendered fields.""" + channel: Channel = Channel.objects.create( + twitch_id="channel_minimal_fields", + name="channelminimalfields", + display_name="Channel Minimal Fields", + ) + + queryset: QuerySet[Channel] = Channel.for_list_view() + fetched_channel: Channel | None = queryset.filter( + twitch_id=channel.twitch_id, + ).first() + + assert fetched_channel is not None + assert hasattr(fetched_channel, "campaign_count") + + deferred_fields: set[str] = fetched_channel.get_deferred_fields() + assert "added_at" in deferred_fields + assert "updated_at" in deferred_fields + assert "name" not in deferred_fields + assert "display_name" not in deferred_fields + assert "twitch_id" not in deferred_fields + + def test_channel_list_queryset_uses_counter_cache_without_join(self) -> None: + """Channel list SQL should use cached count and avoid campaign join/grouping.""" + sql: str = str(Channel.for_list_view().query).upper() + + assert "TWITCH_DROPCAMPAIGN_ALLOW_CHANNELS" not in sql + assert "GROUP BY" not in sql + assert "ALLOWED_CAMPAIGN_COUNT" in sql + + def test_channel_allowed_campaign_count_updates_on_add_remove_clear(self) -> None: + """Counter cache should stay in sync when campaign-channel links change.""" + game: Game = Game.objects.create( + twitch_id="counter_cache_game", + name="Counter Cache Game", + display_name="Counter Cache Game", + ) + + channel: Channel = Channel.objects.create( + twitch_id="counter_cache_channel", + name="countercachechannel", + display_name="Counter Cache Channel", + ) + campaign1: DropCampaign = DropCampaign.objects.create( + twitch_id="counter_cache_campaign_1", + name="Counter Cache Campaign 1", + game=game, + operation_names=["DropCampaignDetails"], + ) + campaign2: DropCampaign = DropCampaign.objects.create( + twitch_id="counter_cache_campaign_2", + name="Counter Cache Campaign 2", + game=game, + operation_names=["DropCampaignDetails"], + ) + + campaign1.allow_channels.add(channel) + channel.refresh_from_db() + assert channel.allowed_campaign_count == 1 + + campaign2.allow_channels.add(channel) + channel.refresh_from_db() + assert channel.allowed_campaign_count == 2 + + campaign1.allow_channels.remove(channel) + channel.refresh_from_db() + assert channel.allowed_campaign_count == 1 + + campaign2.allow_channels.clear() + channel.refresh_from_db() + assert channel.allowed_campaign_count == 0 + + def test_channel_allowed_campaign_count_updates_on_set(self) -> None: + """Counter cache should stay in sync when allow_channels.set(...) is used.""" + game: Game = Game.objects.create( + twitch_id="counter_cache_set_game", + name="Counter Cache Set Game", + display_name="Counter Cache Set Game", + ) + channel1: Channel = Channel.objects.create( + twitch_id="counter_cache_set_channel_1", + name="countercachesetchannel1", + display_name="Counter Cache Set Channel 1", + ) + channel2: Channel = Channel.objects.create( + twitch_id="counter_cache_set_channel_2", + name="countercachesetchannel2", + display_name="Counter Cache Set Channel 2", + ) + campaign: DropCampaign = DropCampaign.objects.create( + twitch_id="counter_cache_set_campaign", + name="Counter Cache Set Campaign", + game=game, + operation_names=["DropCampaignDetails"], + ) + + campaign.allow_channels.set([channel1, channel2]) + channel1.refresh_from_db() + channel2.refresh_from_db() + assert channel1.allowed_campaign_count == 1 + assert channel2.allowed_campaign_count == 1 + + campaign.allow_channels.set([channel2]) + channel1.refresh_from_db() + channel2.refresh_from_db() + assert channel1.allowed_campaign_count == 0 + assert channel2.allowed_campaign_count == 1 + @pytest.mark.django_db def test_dashboard_view(self, client: Client) -> None: """Test dashboard view returns 200 and has grouped campaign data in context.""" diff --git a/twitch/views.py b/twitch/views.py index e2b7922..371a2e1 100644 --- a/twitch/views.py +++ b/twitch/views.py @@ -9,6 +9,7 @@ from collections import defaultdict from typing import TYPE_CHECKING from typing import Any from typing import Literal +from urllib.parse import urlencode from django.core.paginator import EmptyPage from django.core.paginator import Page @@ -1253,19 +1254,8 @@ class ChannelListView(ListView): Returns: QuerySet: Filtered channels. """ - queryset: QuerySet[Channel] = super().get_queryset() search_query: str | None = self.request.GET.get("search") - - if search_query: - queryset = queryset.filter( - Q(name__icontains=search_query) - | Q(display_name__icontains=search_query), - ) - - return queryset.annotate(campaign_count=Count("allowed_campaigns")).order_by( - "-campaign_count", - "name", - ) + return Channel.for_list_view(search_query) def get_context_data(self, **kwargs) -> dict[str, Any]: """Add additional context data. @@ -1277,12 +1267,11 @@ class ChannelListView(ListView): dict: Context data. """ context: dict[str, Any] = super().get_context_data(**kwargs) - search_query: str = self.request.GET.get("search", "") + search_query: str = self.request.GET.get("search", "").strip() # Build pagination info - base_url = "/channels/" - if search_query: - base_url += f"?search={search_query}" + query_string: str = urlencode({"search": search_query}) if search_query else "" + base_url: str = f"/channels/?{query_string}" if query_string else "/channels/" page_obj: Page | None = context.get("page_obj") pagination_info: list[dict[str, str]] | None = (