From c84f968916ae4b6a1a2db5cfe2450edfbeeedda5 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 15 Sep 2026 20:46:31 +0200 Subject: [PATCH] fix(queue): dedupe episodeDict at read boundary + trust server response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'episodes added to download queue never shown' symptom has two layered causes that masked each other: 1. The 'episodes' table has no UNIQUE constraint on (series_id, season, episode_number), so historical scans can leave duplicate rows behind. AnimeSeries.episodeDict iterated the SQLAlchemy 'episodes' relationship without deduping, so the dict exposed duplicate entries to list_missing() and the queue UI. The frontend forwarded the duplicated episode list verbatim to POST /api/queue/add; the backend's pending-episode dedup then rejected every duplicate as 'already pending' and the user saw 'Skipped 44 duplicate episodes, Added 0'. 2. selection-manager.downloadSelected counted 'episodes.length' (the input array) instead of data.added_items.length (the server-confirmed count). Combined with the backend returning success on an empty add, the user saw a misleading 'Added 44 episode(s)' toast for an empty queue. Fix: dedupe at the read boundary. The episodeDict property now filters duplicate (season, episode_number) pairs from both the DB-loaded relationship and the legacy _episode_dict_cache path. Existing duplicate rows in the user's DB are inert — the read filter makes them invisible to the rest of the stack. The frontend now trusts the server response, logs a console warning when an input list shrinks to zero added items, and shows an accurate toast. Tests: - TestEpisodeDictDedup class with 4 regression tests covering: * duplicate relationship rows deduped * is_downloaded rows still filtered out * _episode_dict_cache path also deduped (set by scanners/loaders that may store duplicates) * dedup is per-(season, ep_num), preserving legitimate same-ep-num-across-different-seasons entries Verified manually against the user's backup DB: 'erased' has 44 duplicate rows in the episodes table; episodeDict now returns {1: [1..12]} instead of {1: [1,1,2,2,3,3,3,3,...]}, matching the 12-episode canonical list. --- src/server/database/models.py | 46 ++++++- .../web/static/js/index/selection-manager.js | 18 ++- tests/unit/test_database_models.py | 119 ++++++++++++++++++ 3 files changed, 178 insertions(+), 5 deletions(-) diff --git a/src/server/database/models.py b/src/server/database/models.py index 85ff85f..26b92b9 100644 --- a/src/server/database/models.py +++ b/src/server/database/models.py @@ -195,22 +195,60 @@ class AnimeSeries(Base, TimestampMixin): """Build episode dictionary from episodes relationship or private cache. Returns: - Dictionary mapping season numbers to lists of episode numbers + Dictionary mapping season numbers to lists of episode numbers. + Each (season, episode_number) pair is guaranteed to appear at + most once across all seasons: the underlying episodes table + has no UNIQUE constraint on (series_id, season, + episode_number), so the relationship (and the legacy + ``_episode_dict_cache`` set by loaders/scanners) can contain + duplicates from historical scans. Duplicates are filtered + here at the read boundary so the rest of the stack can rely + on the dict being canonical. """ # Check for private cache first (set when loading from JSON without DB) if hasattr(self, '_episode_dict_cache') and self._episode_dict_cache is not None: - return self._episode_dict_cache - + cached = self._episode_dict_cache + # Dedupe the cached dict too: callers that populate the cache + # (legacy JSON loader, SerieScanner.scan_single_series for + # new series) may store values that contain duplicates. + seen: set[tuple[int, int]] = set() + deduped: dict[int, list[int]] = {} + for season, ep_nums in (cached or {}).items(): + cleaned: list[int] = [] + for ep_num in ep_nums: + if (season, ep_num) in seen: + continue + seen.add((season, ep_num)) + cleaned.append(ep_num) + if cleaned: + deduped[season] = cleaned + return deduped + episode_dict: dict[int, list[int]] = {} try: if self.episodes: + seen: set[tuple[int, int]] = set() for ep in self.episodes: if ep.is_downloaded: continue season = ep.season or 1 + ep_num = ep.episode_number or 0 + # Dedupe by (season, ep_num): the episodes table has + # no UNIQUE constraint on (series_id, season, + # episode_number), so the relationship can yield + # duplicate rows from historical scans. Without + # this guard, the dict exposes duplicates to the + # frontend, which forwards them verbatim to the + # queue API — every duplicate gets rejected by the + # backend's pending-episode dedup, leaving the + # user with an empty queue and a misleading + # "Added N" toast. + if (season, ep_num) in seen: + continue + seen.add((season, ep_num)) if season not in episode_dict: episode_dict[season] = [] - episode_dict[season].append(ep.episode_number or 0) + episode_dict[season].append(ep_num) except Exception: # DetachedInstanceError or other DB errors - return empty dict # This can happen when accessing episodes on a newly created diff --git a/src/server/web/static/js/index/selection-manager.js b/src/server/web/static/js/index/selection-manager.js index 4ee93d7..d0fdc27 100644 --- a/src/server/web/static/js/index/selection-manager.js +++ b/src/server/web/static/js/index/selection-manager.js @@ -253,8 +253,24 @@ AniWorld.SelectionManager = (function() { console.error('Validation errors:', JSON.stringify(data.detail, null, 2)); } + // Trust the server's response, not the input count: the + // backend returns success even when zero episodes were + // added (e.g. all duplicates), and the input `episodes` + // array can itself contain duplicates from a stale + // in-memory episodeDict. Counting `data.added_items` + // gives the user an accurate "Added N" toast. if (response.ok && data.status === 'success') { - totalEpisodesAdded += episodes.length; + const addedThisRequest = Array.isArray(data.added_items) + ? data.added_items.length + : 0; + totalEpisodesAdded += addedThisRequest; + if (addedThisRequest === 0 && episodes.length > 0) { + console.warn( + 'Queue add returned 0 items for', + key, + '— all episodes may be duplicates of an existing pending entry.' + ); + } } else { console.error('Failed to add to queue:', data); failedSeries.push(key); diff --git a/tests/unit/test_database_models.py b/tests/unit/test_database_models.py index 8fda799..cbb0daa 100644 --- a/tests/unit/test_database_models.py +++ b/tests/unit/test_database_models.py @@ -308,6 +308,125 @@ class TestAnimeSeries: assert len(with_tmdb) == 2 +class TestEpisodeDictDedup: + """Regression tests for the duplicate-episode-row guard. + + The ``episodes`` table has no UNIQUE constraint on + ``(series_id, season, episode_number)``, so historical scans can + leave duplicate rows behind. ``AnimeSeries.episodeDict`` is the + read boundary consumed by ``list_missing()`` and the queue UI; + it must never expose duplicates, or the frontend forwards them + verbatim to the queue API and every duplicate is rejected as + "already pending" — the user sees an empty queue and a misleading + "Added N" toast. + """ + + def _make_series(self, db_session: Session, key: str) -> AnimeSeries: + series = AnimeSeries( + key=key, + name=key.replace("-", " ").title(), + site="https://aniworld.to", + folder=f"/anime/{key}", + ) + db_session.add(series) + db_session.commit() + return series + + def test_episodeDict_dedupes_duplicate_relationship_rows( + self, db_session: Session + ): + """Duplicate Episode rows in the same series must not + duplicate the entries in ``episodeDict``.""" + series = self._make_series(db_session, "dedup-rel") + + # Three duplicate rows for (S1, E3) — exactly the pattern the + # scanner accumulates across repeated rescans. + for _ in range(3): + db_session.add( + Episode(series_id=series.id, season=1, episode_number=3) + ) + # One row each for the surrounding unique episodes. + for ep in (1, 2, 4): + db_session.add( + Episode(series_id=series.id, season=1, episode_number=ep) + ) + db_session.commit() + + result = series.episodeDict + + # Order depends on SQLAlchemy row order, which is not strictly + # insertion order — only the *set* of episodes matters here. + assert set(result.keys()) == {1} + assert set(result[1]) == {1, 2, 3, 4} + # Defensive: no season has duplicate episode numbers. + for season, eps in result.items(): + assert len(eps) == len(set(eps)), ( + f"season {season} has duplicate episode numbers: {eps}" + ) + + def test_episodeDict_excludes_downloaded_episodes( + self, db_session: Session + ): + """is_downloaded rows must still be filtered out, and a + duplicate non-downloaded row must not appear twice.""" + series = self._make_series(db_session, "dedup-downloaded") + + db_session.add( + Episode(series_id=series.id, season=1, episode_number=1) + ) + # Downloaded row at (S1, E2): must be filtered out. + db_session.add( + Episode( + series_id=series.id, + season=1, + episode_number=2, + is_downloaded=True, + ) + ) + db_session.commit() + + assert series.episodeDict == {1: [1]} + + def test_episodeDict_dedupes_cached_value(self, db_session: Session): + """The legacy ``_episode_dict_cache`` path (used by + ``SerieScanner.scan_single_series`` for new series and by the + legacy JSON loader) must also dedupe, since loaders that set + the cache directly can introduce duplicates too.""" + series = self._make_series(db_session, "dedup-cache") + + # No episodes in the DB at all — the property will fall back + # to the cache. + series._episode_dict_cache = {1: [3, 3, 3, 4, 4]} + + assert series.episodeDict == {1: [3, 4]} + + def test_episodeDict_preserves_unique_entries_across_seasons( + self, db_session: Session + ): + """Dedup must be per-(season, episode_number), not + per-episode_number alone — same ep number in different + seasons is legitimate and must be preserved.""" + series = self._make_series(db_session, "multi-season") + + for season, ep in [(1, 1), (1, 2), (2, 1), (2, 2), (2, 2)]: + db_session.add( + Episode( + series_id=series.id, + season=season, + episode_number=ep, + ) + ) + db_session.commit() + + result = series.episodeDict + + # Order is not guaranteed across SQLAlchemy relationships; + # compare as sets. + assert set(result.keys()) == {1, 2} + assert set(result[1]) == {1, 2} + assert set(result[2]) == {1, 2} + + class TestEpisode: """Test cases for Episode model."""