Fix N+1 and unbounded response body in /api/v1/media and /api/v1/history (#1004)

- Media-list API pages issued one query per deferred Item field per row
  (get_media_list defers heavy fields for the list scan; ItemSerializer
  reads them all), scaling query count with page size instead of the
  library. Rehydrate just the requested page's Items in one bulk query
  before serialization.
- History's type-only window paginates by day, not entries, so a single
  busy day (imports, binge sessions) could return megabytes for
  limit=10. Cap entries per day to HISTORY_ENTRIES_PER_DAY_PAGE, mirroring
  the existing web history page's per-day cap, with entry_count/
  entries_truncated markers.
- Extend scripts/bench.sh with podcast-history and game-media-list cases
  to measure both going forward.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: dannyvfilms

scripts/bench.sh

@@ -153,6 +153,12 @@ cmd_history_api() {
     "history_types|/api/v1/history/?limit=3&types=episodes,movies"
     "history_media_type|/api/v1/history/?limit=3&media_type=episode,movie"
     "history_all|/api/v1/history/?limit=3"
+    # issue #1004: podcast/game weren't covered by the #576/#948 optimization
+    # pass. bytes column shows the day-entry-cap fix; media_game exercises
+    # the media-list N+1 fix (add PERF_LOG_SLOW_REQUEST_MS=1 to the server
+    # env and grep gunicorn output for queries= to see the query count too).
+    "history_podcast|/api/v1/history/?limit=10&types=podcast"
+    "media_game|/api/v1/media/game/?status=1&limit=10"
   )
   echo -e "case\tendpoint\tstatus\tmedian_s\tp90_s\tmin_s\tmax_s\tbytes\tn" >"$out"
   local entry name path code t bytes stats mode

src/api/tests/test_fork_tracking.py

@@ -1,6 +1,7 @@
 # FORK: tests for the tracking parity endpoints — episode watch/drop,
 # tag management, and the history timeline.
 import datetime
+from decimal import Decimal
 from http import HTTPStatus as HTTP  # noqa: N814
 from unittest.mock import patch
 
@@ -103,6 +104,55 @@ def test_watch_invalid_date_rejected(self):
         response = self._watch(1, payload={"end_date": "not-a-date"})
         self.assertEqual(response.status_code, HTTP.BAD_REQUEST)
 
+    @patch(
+        "app.models.providers.services.get_media_metadata",
+        side_effect=_season_metadata_side_effect,
+    )
+    def test_watch_with_score_sets_episode_score(self, _mock):
+        """POST watch with a score sets it on all plays of the episode."""
+        response = self._watch(1, payload={"score": "8.3"})
+        self.assertEqual(response.status_code, HTTP.CREATED)
+        self.assertEqual(response.json()["score"], 8.3)
+        scores = set(
+            Episode.objects.filter(
+                related_season=self.season_medias[0],
+                item__episode_number=1,
+            ).values_list("score", flat=True),
+        )
+        self.assertEqual(scores, {Decimal("8.3")})
+
+    @patch(
+        "app.models.providers.services.get_media_metadata",
+        side_effect=_season_metadata_side_effect,
+    )
+    def test_watch_without_score_leaves_score_unset(self, _mock):
+        """POST watch without a score field doesn't touch the score."""
+        response = self._watch(1)
+        self.assertEqual(response.status_code, HTTP.CREATED)
+        self.assertIsNone(response.json()["score"])
+
+    @patch(
+        "app.models.providers.services.get_media_metadata",
+        side_effect=_season_metadata_side_effect,
+    )
+    def test_watch_invalid_score_rejected(self, _mock):
+        """POST watch with an out-of-range score 400s and creates no play."""
+        play_count_before = Episode.objects.filter(
+            related_season=self.season_medias[0],
+            item__episode_number=1,
+        ).count()
+
+        response = self._watch(1, payload={"score": "11"})
+
+        self.assertEqual(response.status_code, HTTP.BAD_REQUEST)
+        self.assertEqual(
+            Episode.objects.filter(
+                related_season=self.season_medias[0],
+                item__episode_number=1,
+            ).count(),
+            play_count_before,
+        )
+
     @patch(
         "app.models.providers.services.get_media_metadata",
         side_effect=_season_metadata_side_effect,
@@ -562,6 +612,45 @@ def test_history_returns_days(self):
             any(entry["media_type"] == "movie" for entry in all_entries),
         )
 
+    def test_history_day_entries_are_capped(self):
+        """A day with many plays doesn't blow up the response body (#1004).
+
+        `limit`/`offset` on this endpoint paginate over DAYS, not entries, so
+        a single busy day (imports, binge sessions, frequent podcast
+        scrobbles) must still be bounded — mirrors the web history page's
+        existing per-day cap (HISTORY_ENTRIES_PER_DAY_PAGE).
+        """
+        entry_count = history_cache.HISTORY_ENTRIES_PER_DAY_PAGE + 5
+        same_day = datetime.datetime(2024, 6, 1, tzinfo=datetime.UTC)
+        for index in range(entry_count):
+            item = Item.objects.create(
+                media_id=f"history-cap-movie-{index}",
+                source=Sources.TMDB.value,
+                media_type=MediaTypes.MOVIE.value,
+                title=f"History Cap Movie {index}",
+            )
+            Movie.objects.create(
+                item=item,
+                user=self.user1,
+                end_date=same_day,
+            )
+        cache.clear()
+
+        response = self.call_api(
+            "get",
+            "api_history",
+            params={"media_type": "movie", "limit": 1},
+            headers=self.auth_headers,
+        )
+
+        self.assertEqual(response.status_code, HTTP.OK)
+        days = response.json()["results"]
+        self.assertEqual(len(days), 1)
+        day = days[0]
+        self.assertLessEqual(len(day["entries"]), history_cache.HISTORY_ENTRIES_PER_DAY_PAGE)
+        self.assertEqual(day["entry_count"], entry_count)
+        self.assertTrue(day["entries_truncated"])
+
     def test_history_flat_returns_paginated_entry_list(self):
         """?flat=1 returns a flat, card-oriented entry list, not day buckets."""
         day_response = self.call_api(

src/api/tests/test_media_list_filters.py

@@ -1,8 +1,21 @@
 from http import HTTPStatus as HTTP  # noqa: N814
 
+from django.db import connection
+from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 
-from app.models import TV, CollectionEntry, Item, ItemTag, Season, Status, Tag
+from app.models import (
+    TV,
+    CollectionEntry,
+    Game,
+    Item,
+    ItemTag,
+    MediaTypes,
+    Season,
+    Sources,
+    Status,
+    Tag,
+)
 from events.models import Event
 
 from .base import FloppyApiTestCase
@@ -290,3 +303,59 @@ def test_invalid_filter_values_return_bad_request(self):
         ):
             response = self._get_tv(**params)
             self.assertEqual(response.status_code, HTTP.BAD_REQUEST, params)
+
+
+class MediaListQueryBudgetTests(FloppyApiTestCase):
+    """Pin the query count for a paginated media-list page (#1004).
+
+    Serializing a page must not scale with the size of the user's whole
+    library — regression test for a per-page-item N+1 caused by Item fields
+    that `get_media_list` defers for the list scan but `ItemSerializer`
+    (via `MediaSerializer`) reads in full.
+    """
+
+    def _seed_extra_games(self, count, *, start=0):
+        for index in range(start, start + count):
+            item = Item.objects.create(
+                media_id=f"query-budget-game-{index}",
+                source=Sources.IGDB.value,
+                media_type=MediaTypes.GAME.value,
+                title=f"Query Budget Game {index}",
+            )
+            Game.objects.create(item=item, user=self.user1)
+
+    def test_query_count_does_not_scale_with_library_size(self):
+        self._seed_extra_games(5)
+        with CaptureQueriesContext(connection) as small_ctx:
+            response = self.client.get(
+                "/api/v1/media/game/",
+                {"limit": 10},
+                **self.auth_headers,
+            )
+        self.assertEqual(response.status_code, HTTP.OK)
+        small_queries = len(small_ctx.captured_queries)
+
+        self._seed_extra_games(120, start=5)
+        with CaptureQueriesContext(connection) as big_ctx:
+            response = self.client.get(
+                "/api/v1/media/game/",
+                {"limit": 10},
+                **self.auth_headers,
+            )
+        self.assertEqual(response.status_code, HTTP.OK)
+        big_queries = len(big_ctx.captured_queries)
+
+        self.assertEqual(len(response.json()["results"]), 10)
+        self.assertLessEqual(
+            big_queries,
+            small_queries + 2,
+            f"query count grew from {small_queries} to {big_queries} as the "
+            "library grew — page serialization is scaling with library size "
+            "again, not just the requested page.",
+        )
+        self.assertLessEqual(
+            big_queries,
+            30,
+            f"{big_queries} queries for a 10-item page; budget is 30. If "
+            "this increase is intentional, update the pin deliberately.",
+        )

src/api/views.py

@@ -825,6 +825,30 @@ def get(self, request, list_id, item_id):
         )
 
 
+def _rehydrate_deferred_items(page_entries):
+    """Refetch full Item rows for one page of entries.
+
+    `get_media_list`/`get_media_list_entries` defer several rarely-used Item
+    fields (trakt/provider metadata) to keep the whole-library scan cheap.
+    `ItemSerializer` (via `MediaSerializer`/`UntrackedMediaSerializer`) reads
+    every Item field, so each deferred field on a paginated instance would
+    otherwise trigger its own single-row query — a full-page-sized N+1. One
+    bulk, undeferred fetch of just the page's items avoids that without
+    touching the list-scan's defer optimization.
+    """
+    item_ids = {entry.item.id for entry in page_entries if entry.item is not None}
+    if not item_ids:
+        return
+    fresh_items_by_id = {item.pk: item for item in Item.objects.filter(pk__in=item_ids)}
+    for entry in page_entries:
+        fresh_item = fresh_items_by_id.get(entry.item.id if entry.item else None)
+        if fresh_item is None:
+            continue
+        entry.item = fresh_item
+        if entry.media is not None:
+            entry.media.item = fresh_item
+
+
 # /api/v1/media/
 def _media_list_response(request, media_type=None):
     """Build a filtered and serialized media-list response."""
@@ -849,6 +873,7 @@ def _media_list_response(request, media_type=None):
 
     paginated_data = paginate_data(request, entries, limit, offset)
     page_entries = paginated_data["results"]
+    _rehydrate_deferred_items(page_entries)
     lists_by_item_id = build_lists_by_item_id(request.user, page_entries)
     next_episode_by_item_id = get_next_episode_map(page_entries)
     serializer_context = {

src/app/history_cache_reader.py

@@ -33,6 +33,7 @@
     HISTORY_COVERAGE_REPAIR_BATCH_SIZE,
     HISTORY_DAY_CACHE_TIMEOUT,
     HISTORY_DAYS_PER_PAGE,
+    HISTORY_ENTRIES_PER_DAY_PAGE,
     HISTORY_STALE_AFTER,
     HISTORY_WARM_DAYS,
     _cache_key,
@@ -376,8 +377,25 @@ def get_cached_history_window(
             )
 
     history_days.sort(key=lambda day: day.get("date") or date.min, reverse=True)
+
+    # A day's entry count is unbounded (imports, binge sessions, frequent
+    # podcast scrobbles can put hundreds of entries on one day), while
+    # `limit`/`offset` here only bound the number of DAYS returned. Without
+    # this cap a single busy day can blow up the response to megabytes even
+    # for `limit=1` — mirrors the web history page's existing per-day
+    # entry cap (HISTORY_ENTRIES_PER_DAY_PAGE, see history_views.py).
+    total_entries = 0
+    for day_payload in history_days:
+        entries = day_payload.get("entries", [])
+        entry_count = len(entries)
+        total_entries += entry_count
+        if entry_count > HISTORY_ENTRIES_PER_DAY_PAGE:
+            day_payload["entries"] = entries[:HISTORY_ENTRIES_PER_DAY_PAGE]
+        day_payload["entry_count"] = entry_count
+        day_payload["entries_truncated"] = entry_count > HISTORY_ENTRIES_PER_DAY_PAGE
+
     logger.info(
-        "history_cached_window user_id=%s logging_style=%s filters=%s indexed=%s offset=%s limit=%s cached=%s missing=%s returned=%s",
+        "history_cached_window user_id=%s logging_style=%s filters=%s indexed=%s offset=%s limit=%s cached=%s missing=%s returned=%s entries=%s",
         user.id,
         logging_style,
         filters,
@@ -387,6 +405,7 @@ def get_cached_history_window(
         len(payloads),
         len(missing_day_keys),
         len(history_days),
+        total_entries,
     )
     return history_days, total_days