From e968f5cdea2895bacf16cb298db51c39f3860d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Hells=C3=A9n?= Date: Mon, 9 Feb 2026 20:02:19 +0100 Subject: [PATCH] Add Silk middleware and related settings for performance monitoring - Introduced SILK_ENABLED setting to toggle Silk middleware. - Updated ALLOWED_HOSTS to include "testserver" when not in DEBUG mode. - Modified urlpatterns to conditionally include Silk URLs. - Added django-silk dependency to pyproject.toml. - Enhanced feed queries to optimize performance and reduce N+1 issues. - Updated tests to verify query limits for various feeds. --- .pre-commit-config.yaml | 2 +- config/settings.py | 5 +- config/tests/test_settings.py | 2 +- config/urls.py | 5 +- pyproject.toml | 1 + twitch/feeds.py | 36 +++-- twitch/tests/test_feeds.py | 239 +++++++++++++++++++++++++++++++++- twitch/urls.py | 33 +---- twitch/views.py | 23 ++-- 9 files changed, 289 insertions(+), 57 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 68508c5..14671bd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,7 +21,7 @@ repos: - id: trailing-whitespace - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.14.14 + rev: v0.15.0 hooks: - id: ruff-check args: ["--fix", "--exit-non-zero-on-fix"] diff --git a/config/settings.py b/config/settings.py index dbb5ee4..a415818 100644 --- a/config/settings.py +++ b/config/settings.py @@ -39,6 +39,7 @@ def env_int(key: str, default: int) -> int: DEBUG: bool = env_bool(key="DEBUG", default=True) +RUNNING_TESTS: bool = "PYTEST_VERSION" in os.environ or any("pytest" in arg for arg in sys.argv) def get_data_dir() -> Path: @@ -110,7 +111,7 @@ INTERNAL_IPS: list[str] = [] if DEBUG: INTERNAL_IPS = ["127.0.0.1", "localhost"] # pyright: ignore[reportConstantRedefinition] -ALLOWED_HOSTS: list[str] = [".localhost", "127.0.0.1", "[::1]"] +ALLOWED_HOSTS: list[str] = [".localhost", "127.0.0.1", "[::1]", "testserver"] if not DEBUG: ALLOWED_HOSTS = ["ttvdrops.lovinator.space"] # pyright: ignore[reportConstantRedefinition] @@ -141,10 +142,12 @@ INSTALLED_APPS: list[str] = [ "django.contrib.contenttypes", "django.contrib.staticfiles", "twitch.apps.TwitchConfig", + *(["silk"] if not RUNNING_TESTS else []), ] MIDDLEWARE: list[str] = [ "django.middleware.security.SecurityMiddleware", + *(["silk.middleware.SilkyMiddleware"] if not RUNNING_TESTS else []), "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", diff --git a/config/tests/test_settings.py b/config/tests/test_settings.py index 9e8c2a3..55357ae 100644 --- a/config/tests/test_settings.py +++ b/config/tests/test_settings.py @@ -114,7 +114,7 @@ def test_allowed_hosts_when_debug_true(reload_settings_module: Callable[..., Mod reloaded: ModuleType = reload_settings_module(DEBUG="1") assert reloaded.DEBUG is True - assert reloaded.ALLOWED_HOSTS == [".localhost", "127.0.0.1", "[::1]"] + assert reloaded.ALLOWED_HOSTS == [".localhost", "127.0.0.1", "[::1]", "testserver"] def test_debug_defaults_true_when_missing(reload_settings_module: Callable[..., ModuleType]) -> None: diff --git a/config/urls.py b/config/urls.py index 96da617..2202246 100644 --- a/config/urls.py +++ b/config/urls.py @@ -11,10 +11,13 @@ if TYPE_CHECKING: from django.urls.resolvers import URLPattern from django.urls.resolvers import URLResolver -urlpatterns: list[URLResolver] | list[URLPattern | URLResolver] = [ # type: ignore[assignment] +urlpatterns: [URLPattern | URLResolver] = [ # type: ignore[assignment] path(route="", view=include("twitch.urls", namespace="twitch")), ] +if getattr(settings, "ENABLE_SILK", False): + urlpatterns += [path("silk/", include("silk.urls", namespace="silk"))] + # Serve media in development if settings.DEBUG: urlpatterns += static( diff --git a/pyproject.toml b/pyproject.toml index be2b6f7..c59928f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ "tqdm", "colorama", "gunicorn", + "django-silk>=5.4.3", ] [dependency-groups] diff --git a/twitch/feeds.py b/twitch/feeds.py index 22375e8..a37a4ea 100644 --- a/twitch/feeds.py +++ b/twitch/feeds.py @@ -7,6 +7,7 @@ from typing import Literal from django.contrib.humanize.templatetags.humanize import naturaltime from django.contrib.syndication.views import Feed +from django.db.models import Prefetch from django.db.models.query import QuerySet from django.urls import reverse from django.utils import feedgenerator @@ -35,6 +36,20 @@ if TYPE_CHECKING: logger: logging.Logger = logging.getLogger("ttvdrops") +def _with_campaign_related(queryset: QuerySet[DropCampaign]) -> QuerySet[DropCampaign]: + """Apply related-selects/prefetches needed by feed rendering to avoid N+1 queries. + + Returns: + QuerySet[DropCampaign]: Queryset with related data preloaded for feed rendering. + """ + drops_prefetch: Prefetch = Prefetch( + "time_based_drops", + queryset=TimeBasedDrop.objects.prefetch_related("benefits"), + ) + + return queryset.select_related("game").prefetch_related("game__owners", "allow_channels", drops_prefetch) + + def insert_date_info(item: Model, parts: list[SafeText]) -> None: """Insert start and end date information into parts list. @@ -460,9 +475,8 @@ class DropCampaignFeed(Feed): def items(self) -> list[DropCampaign]: """Return the latest 200 drop campaigns ordered by most recent start date.""" - return list( - DropCampaign.objects.select_related("game").order_by("-start_at")[:200], - ) + queryset: QuerySet[DropCampaign] = DropCampaign.objects.order_by("-start_at") + return list(_with_campaign_related(queryset)[:200]) def item_title(self, item: Model) -> SafeText: """Return the campaign name as the item title (SafeText for RSS).""" @@ -477,7 +491,7 @@ class DropCampaignFeed(Feed): drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None) if drops: - drops_data = _build_drops_data(drops.select_related().prefetch_related("benefits").all()) + drops_data = _build_drops_data(drops.all()) parts: list[SafeText] = [] @@ -610,9 +624,8 @@ class GameCampaignFeed(Feed): def items(self, obj: Game) -> list[DropCampaign]: """Return the latest 200 drop campaigns for this game, ordered by most recent start date.""" - return list( - DropCampaign.objects.filter(game=obj).select_related("game").order_by("-start_at")[:200], - ) + queryset: QuerySet[DropCampaign] = DropCampaign.objects.filter(game=obj).order_by("-start_at") + return list(_with_campaign_related(queryset)[:200]) def item_title(self, item: Model) -> SafeText: """Return the campaign name as the item title (SafeText for RSS).""" @@ -625,7 +638,7 @@ class GameCampaignFeed(Feed): drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None) if drops: - drops_data = _build_drops_data(drops.select_related().prefetch_related("benefits").all()) + drops_data = _build_drops_data(drops.all()) parts: list[SafeText] = [] @@ -751,9 +764,8 @@ class OrganizationCampaignFeed(Feed): def items(self, obj: Organization) -> list[DropCampaign]: """Return the latest 200 drop campaigns for this organization, ordered by most recent start date.""" - return list( - DropCampaign.objects.filter(game__owners=obj).select_related("game").order_by("-start_at")[:200], - ) + queryset: QuerySet[DropCampaign] = DropCampaign.objects.filter(game__owners=obj).order_by("-start_at") + return list(_with_campaign_related(queryset)[:200]) def item_author_name(self, item: DropCampaign) -> str: """Return the author name for the campaign, typically the game name.""" @@ -818,7 +830,7 @@ class OrganizationCampaignFeed(Feed): drops: QuerySet[TimeBasedDrop] | None = getattr(item, "time_based_drops", None) if drops: - drops_data = _build_drops_data(drops.select_related().prefetch_related("benefits").all()) + drops_data = _build_drops_data(drops.all()) parts: list[SafeText] = [] diff --git a/twitch/tests/test_feeds.py b/twitch/tests/test_feeds.py index 27199fe..42a5f09 100644 --- a/twitch/tests/test_feeds.py +++ b/twitch/tests/test_feeds.py @@ -2,6 +2,8 @@ from __future__ import annotations +from collections.abc import Callable +from contextlib import AbstractContextManager from datetime import timedelta from typing import TYPE_CHECKING @@ -188,6 +190,237 @@ class RSSFeedTestCase(TestCase): assert "Other Campaign 2" not in content +QueryAsserter = Callable[..., AbstractContextManager[object]] + + +def _build_campaign(game: Game, idx: int) -> DropCampaign: + """Create a campaign with a channel, drop, and benefit for query counting. + + Returns: + DropCampaign: Newly created campaign instance. + """ + campaign: DropCampaign = DropCampaign.objects.create( + twitch_id=f"test-campaign-{idx}", + name=f"Test Campaign {idx}", + game=game, + start_at=timezone.now(), + end_at=timezone.now() + timedelta(days=7), + operation_names=["DropCampaignDetails"], + ) + + channel: Channel = Channel.objects.create( + twitch_id=f"test-channel-{idx}", + name=f"testchannel{idx}", + display_name=f"TestChannel{idx}", + ) + campaign.allow_channels.add(channel) + + drop: TimeBasedDrop = TimeBasedDrop.objects.create( + twitch_id=f"drop-{idx}", + name=f"Drop {idx}", + campaign=campaign, + required_minutes_watched=30, + start_at=timezone.now(), + end_at=timezone.now() + timedelta(hours=1), + ) + benefit: DropBenefit = DropBenefit.objects.create( + twitch_id=f"benefit-{idx}", + name=f"Benefit {idx}", + distribution_type="ITEM", + ) + drop.benefits.add(benefit) + + return campaign + + +def _build_reward_campaign(game: Game, idx: int) -> RewardCampaign: + """Create a reward campaign for query counting. + + Returns: + RewardCampaign: Newly created reward campaign instance. + """ + return RewardCampaign.objects.create( + twitch_id=f"test-reward-{idx}", + name=f"Test Reward {idx}", + brand="Test Brand", + starts_at=timezone.now(), + ends_at=timezone.now() + timedelta(days=14), + status="ACTIVE", + summary="Test reward summary", + instructions="Watch and complete objectives", + external_url="https://example.com/reward", + about_url="https://example.com/about", + is_sitewide=False, + game=game, + ) + + +@pytest.mark.django_db +def test_campaign_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Campaign feed should stay within a small, fixed query budget.""" + org: Organization = Organization.objects.create( + twitch_id="test-org-queries", + name="Query Org", + ) + game: Game = Game.objects.create( + twitch_id="test-game-queries", + slug="query-game", + name="Query Game", + display_name="Query Game", + ) + game.owners.add(org) + + for i in range(3): + _build_campaign(game, i) + + url: str = reverse("twitch:campaign_feed") + with django_assert_num_queries(20, 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.""" + org: Organization = Organization.objects.create( + twitch_id="test-org-game-queries", + name="Query Org Game", + ) + game: Game = Game.objects.create( + twitch_id="test-game-campaign-queries", + slug="query-game-campaign", + name="Query Game Campaign", + display_name="Query Game Campaign", + ) + game.owners.add(org) + + for i in range(3): + _build_campaign(game, i) + + url: str = reverse("twitch:game_campaign_feed", args=[game.twitch_id]) + with django_assert_num_queries(22, exact=False): + response: _MonkeyPatchedWSGIResponse = client.get(url) + + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_organization_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Organization RSS feed should stay within a modest query budget.""" + for i in range(5): + Organization.objects.create( + twitch_id=f"org-feed-{i}", + name=f"Org Feed {i}", + ) + + url: str = reverse("twitch:organization_feed") + 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_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Game RSS feed should stay within a modest query budget with multiple games.""" + org: Organization = Organization.objects.create( + twitch_id="game-feed-org", + name="Game Feed Org", + ) + + for i in range(3): + game: Game = Game.objects.create( + twitch_id=f"game-feed-{i}", + slug=f"game-feed-{i}", + name=f"Game Feed {i}", + display_name=f"Game Feed {i}", + ) + game.owners.add(org) + + url: str = reverse("twitch:game_feed") + with django_assert_num_queries(10, exact=False): + response: _MonkeyPatchedWSGIResponse = client.get(url) + + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_organization_campaign_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Organization campaign feed should not regress in query count.""" + org: Organization = Organization.objects.create( + twitch_id="org-campaign-feed", + name="Org Campaign Feed", + ) + game: Game = Game.objects.create( + twitch_id="org-campaign-game", + slug="org-campaign-game", + name="Org Campaign Game", + display_name="Org Campaign Game", + ) + game.owners.add(org) + + for i in range(3): + _build_campaign(game, i) + + url: str = reverse("twitch:organization_campaign_feed", args=[org.twitch_id]) + with django_assert_num_queries(22, exact=False): + response: _MonkeyPatchedWSGIResponse = client.get(url) + + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_reward_campaign_feed_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Reward campaign feed should stay within a modest query budget.""" + org: Organization = Organization.objects.create( + twitch_id="reward-feed-org", + name="Reward Feed Org", + ) + game: Game = Game.objects.create( + twitch_id="reward-feed-game", + slug="reward-feed-game", + name="Reward Feed Game", + display_name="Reward Feed Game", + ) + game.owners.add(org) + + for i in range(3): + _build_reward_campaign(game, i) + + url: str = reverse("twitch:reward_campaign_feed") + with django_assert_num_queries(8, exact=False): + response: _MonkeyPatchedWSGIResponse = client.get(url) + + assert response.status_code == 200 + + +@pytest.mark.django_db +def test_docs_rss_queries_bounded(client: Client, django_assert_num_queries: QueryAsserter) -> None: + """Docs RSS page should stay within a reasonable query budget.""" + org: Organization = Organization.objects.create( + twitch_id="docs-org", + name="Docs Org", + ) + game: Game = Game.objects.create( + twitch_id="docs-game", + slug="docs-game", + name="Docs Game", + display_name="Docs Game", + ) + game.owners.add(org) + + for i in range(2): + _build_campaign(game, i) + _build_reward_campaign(game, i) + + url: str = reverse("twitch:docs_rss") + with django_assert_num_queries(60, exact=False): + response: _MonkeyPatchedWSGIResponse = client.get(url) + + assert response.status_code == 200 + + URL_NAMES: list[tuple[str, dict[str, str]]] = [ ("twitch:dashboard", {}), ("twitch:badge_list", {}), @@ -213,12 +446,6 @@ URL_NAMES: list[tuple[str, dict[str, str]]] = [ ("twitch:organization_feed", {}), ("twitch:organization_campaign_feed", {"twitch_id": "test-org-123"}), ("twitch:reward_campaign_feed", {}), - ("twitch:campaign_feed_v1", {}), - ("twitch:game_feed_v1", {}), - ("twitch:game_campaign_feed_v1", {"twitch_id": "test-game-123"}), - ("twitch:organization_feed_v1", {}), - ("twitch:organization_campaign_feed_v1", {"twitch_id": "test-org-123"}), - ("twitch:reward_campaign_feed_v1", {}), ] diff --git a/twitch/urls.py b/twitch/urls.py index ef96467..efa18ef 100644 --- a/twitch/urls.py +++ b/twitch/urls.py @@ -17,31 +17,6 @@ if TYPE_CHECKING: app_name = "twitch" -# We have /rss/ that is always the latest, and versioned version to not break users regexes. - - -rss_feeds_latest: list[URLPattern] = [ - path("rss/campaigns/", DropCampaignFeed(), name="campaign_feed"), - path("rss/games/", GameFeed(), name="game_feed"), - path("rss/games//campaigns/", GameCampaignFeed(), name="game_campaign_feed"), - path("rss/organizations/", OrganizationRSSFeed(), name="organization_feed"), - path("rss/organizations//campaigns/", OrganizationCampaignFeed(), name="organization_campaign_feed"), - path("rss/reward-campaigns/", RewardCampaignFeed(), name="reward_campaign_feed"), -] - -v1_rss_feeds: list[URLPattern] = [ - path("rss/v1/campaigns/", DropCampaignFeed(), name="campaign_feed_v1"), - path("rss/v1/games/", GameFeed(), name="game_feed_v1"), - path("rss/v1/games//campaigns/", GameCampaignFeed(), name="game_campaign_feed_v1"), - path("rss/v1/organizations/", OrganizationRSSFeed(), name="organization_feed_v1"), - path( - "rss/v1/organizations//campaigns/", - OrganizationCampaignFeed(), - name="organization_campaign_feed_v1", - ), - path("rss/v1/reward-campaigns/", RewardCampaignFeed(), name="reward_campaign_feed_v1"), -] - urlpatterns: list[URLPattern] = [ path("", views.dashboard, name="dashboard"), @@ -62,6 +37,10 @@ urlpatterns: list[URLPattern] = [ path("reward-campaigns/", views.reward_campaign_list_view, name="reward_campaign_list"), path("reward-campaigns//", views.reward_campaign_detail_view, name="reward_campaign_detail"), path("search/", views.search_view, name="search"), - *rss_feeds_latest, - *v1_rss_feeds, + path("rss/campaigns/", DropCampaignFeed(), name="campaign_feed"), + path("rss/games/", GameFeed(), name="game_feed"), + path("rss/games//campaigns/", GameCampaignFeed(), name="game_campaign_feed"), + path("rss/organizations/", OrganizationRSSFeed(), name="organization_feed"), + path("rss/organizations//campaigns/", OrganizationCampaignFeed(), name="organization_campaign_feed"), + path("rss/reward-campaigns/", RewardCampaignFeed(), name="reward_campaign_feed"), ] diff --git a/twitch/views.py b/twitch/views.py index 2ec8113..9acbba2 100644 --- a/twitch/views.py +++ b/twitch/views.py @@ -1017,6 +1017,13 @@ def docs_rss_view(request: HttpRequest) -> HttpResponse: Rendered HTML response with list of RSS feeds. """ + def absolute(path: str) -> str: + try: + return request.build_absolute_uri(path) + except Exception: # pragma: no cover - defensive logging for docs only + logger.exception("Failed to build absolute URL for %s", path) + return path + def _pretty_example(xml_str: str, max_items: int = 1) -> str: try: trimmed = xml_str.strip() @@ -1045,25 +1052,25 @@ def docs_rss_view(request: HttpRequest) -> HttpResponse: { "title": "All Organizations", "description": "Latest organizations added to TTVDrops", - "url": reverse("twitch:organization_feed"), + "url": absolute(reverse("twitch:organization_feed")), "example_xml": render_feed(OrganizationRSSFeed()), }, { "title": "All Games", "description": "Latest games added to TTVDrops", - "url": reverse("twitch:game_feed"), + "url": absolute(reverse("twitch:game_feed")), "example_xml": render_feed(GameFeed()), }, { "title": "All Drop Campaigns", "description": "Latest drop campaigns across all games", - "url": reverse("twitch:campaign_feed"), + "url": absolute(reverse("twitch:campaign_feed")), "example_xml": render_feed(DropCampaignFeed()), }, { "title": "All Reward Campaigns", "description": "Latest reward campaigns (Quest rewards) on Twitch", - "url": reverse("twitch:reward_campaign_feed"), + "url": absolute(reverse("twitch:reward_campaign_feed")), "example_xml": render_feed(RewardCampaignFeed()), }, ] @@ -1076,9 +1083,9 @@ def docs_rss_view(request: HttpRequest) -> HttpResponse: "title": "Campaigns for a Single Game", "description": "Latest drop campaigns for one game.", "url": ( - reverse("twitch:game_campaign_feed", args=[sample_game.twitch_id]) + absolute(reverse("twitch:game_campaign_feed", args=[sample_game.twitch_id])) if sample_game - else "/rss/games//campaigns/" + else absolute("/rss/games//campaigns/") ), "has_sample": bool(sample_game), "example_xml": render_feed(GameCampaignFeed(), sample_game.twitch_id) if sample_game else "", @@ -1087,9 +1094,9 @@ def docs_rss_view(request: HttpRequest) -> HttpResponse: "title": "Campaigns for an Organization", "description": "Drop campaigns across games owned by one organization.", "url": ( - reverse("twitch:organization_campaign_feed", args=[sample_org.twitch_id]) + absolute(reverse("twitch:organization_campaign_feed", args=[sample_org.twitch_id])) if sample_org - else "/rss/organizations//campaigns/" + else absolute("/rss/organizations//campaigns/") ), "has_sample": bool(sample_org), "example_xml": render_feed(OrganizationCampaignFeed(), sample_org.twitch_id) if sample_org else "",