fix(queue): dedupe episodeDict at read boundary + trust server response
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user