Add allowed_campaign_count field to Channel model and implement counter cache logic
This commit is contained in:
parent
917bf8ac23
commit
293dd57263
7 changed files with 397 additions and 16 deletions
|
|
@ -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",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
]
|
||||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
86
twitch/tests/test_migrations.py
Normal file
86
twitch/tests/test_migrations.py
Normal file
|
|
@ -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,
|
||||
}
|
||||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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 = (
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue