fix(rescan): delete downloaded episode rows when no longer missing
A finished download marks the Episode row with is_downloaded=True and
populates file_path (commit 0ba2587). The intent was to preserve
download history, but the row stayed in the DB forever because two
sibling sync methods refused to delete it:
- AnimeService._update_series_in_db: `downloaded_set` guard skipped
deletion if the row was marked downloaded.
- SerieScanner._sync_episodes_to_db: `if ep.is_downloaded: continue`
kept downloaded rows.
The user-visible bug: the missing-list UI is correct (it filters by
is_downloaded and the broadcast rebuilds from the DB), but a finished
download left a stale entry in the DB that the user could see when
querying the database directly. Worse, this stale row accumulated
indefinitely across rescans.
Fix: drop the is_downloaded preservation guards. Once the scanner
confirms the file is on disk and the episode is no longer in the
missing set, the Episode row has no further purpose and is deleted to
keep the DB in sync with the filesystem. The UI derives "missing"
from row presence, so deleting the row is the correct way to make the
episode stop appearing as missing in *all* views (UI and DB queries).
Tests:
- test_update_series_deletes_downloaded_episodes_when_no_longer_missing
(RED): regression for _update_series_in_db.
- test_update_series_keeps_still_missing_episodes: sanity sibling
to ensure the fix does not over-reach and delete still-missing
episodes.
- test_deletes_downloaded_episodes_when_no_longer_missing
(RED): regression for SerieScanner._sync_episodes_to_db.
- Replaced test_preserves_downloaded_episodes (which asserted the
old buggy behavior) with the deletion-asserting variant.
This commit is contained in:
@@ -281,10 +281,13 @@ class SerieScanner:
|
||||
async def _sync_episodes_to_db(
|
||||
self, db, series_id: int, episode_dict: dict[int, list[int]]
|
||||
) -> None:
|
||||
"""Sync episodes to database, preserving downloaded flags.
|
||||
"""Sync episodes to database.
|
||||
|
||||
Adds missing episodes, removes episodes no longer missing,
|
||||
and preserves is_downloaded=True episodes.
|
||||
Adds missing episodes, removes episodes no longer missing
|
||||
(including those that were previously marked as downloaded:
|
||||
once the scanner confirms the file is on disk, the row has
|
||||
no further purpose and is deleted to keep the DB in sync
|
||||
with the filesystem).
|
||||
|
||||
Args:
|
||||
db: Async database session
|
||||
@@ -301,12 +304,6 @@ class SerieScanner:
|
||||
new_keys.add((season, ep_num))
|
||||
for (season, ep_num), ep in existing_map.items():
|
||||
if (season, ep_num) not in new_keys:
|
||||
if ep.is_downloaded:
|
||||
logger.debug(
|
||||
"Preserving downloaded episode S%02dE%02d for series_id=%d",
|
||||
season, ep_num, series_id
|
||||
)
|
||||
else:
|
||||
await EpisodeService.delete_by_series(
|
||||
db, series_id, season, ep_num
|
||||
)
|
||||
|
||||
@@ -861,9 +861,13 @@ class AnimeService:
|
||||
Syncs the database episodes with the current missing episodes from scan.
|
||||
- Adds new missing episodes that are not in the database
|
||||
- Removes episodes from database that are no longer missing
|
||||
(i.e., the file has been added to the filesystem)
|
||||
- Preserves episodes marked as downloaded (is_downloaded=True)
|
||||
so download history is not lost
|
||||
(i.e., the file has been added to the filesystem), including
|
||||
episodes that were previously marked as downloaded. A row
|
||||
that is no longer missing — by definition — does not need to
|
||||
stay in the DB; the UI derives "missing" from the row's
|
||||
presence, so keeping an ``is_downloaded=True`` row around
|
||||
leaves a stale entry that the user can see in the DB but
|
||||
not anywhere else.
|
||||
"""
|
||||
from src.server.database.service import AnimeSeriesService, EpisodeService
|
||||
|
||||
@@ -871,15 +875,11 @@ class AnimeService:
|
||||
existing_episodes = await EpisodeService.get_by_series(db, existing.id)
|
||||
|
||||
# Build dict of existing episodes: {season: {ep_num: episode_id}}
|
||||
# and track which ones are already downloaded
|
||||
existing_dict: dict[int, dict[int, int]] = {}
|
||||
downloaded_set: set[tuple[int, int]] = set()
|
||||
for ep in existing_episodes:
|
||||
if ep.season not in existing_dict:
|
||||
existing_dict[ep.season] = {}
|
||||
existing_dict[ep.season][ep.episode_number] = ep.id
|
||||
if ep.is_downloaded:
|
||||
downloaded_set.add((ep.season, ep.episode_number))
|
||||
|
||||
# Get new missing episodes from scan
|
||||
new_dict = serie.episodeDict or {}
|
||||
@@ -909,23 +909,14 @@ class AnimeService:
|
||||
)
|
||||
|
||||
# Remove episodes from database that are no longer missing
|
||||
# (i.e., the episode file now exists on the filesystem)
|
||||
# BUT: preserve episodes that are already downloaded (is_downloaded=True)
|
||||
# so we don't lose download history
|
||||
# (i.e., the episode file now exists on the filesystem).
|
||||
# This includes episodes previously marked as downloaded:
|
||||
# once the file is confirmed on disk by a rescan, the row
|
||||
# has no further purpose and is deleted to keep the DB
|
||||
# in sync with the filesystem.
|
||||
for season, eps_dict in existing_dict.items():
|
||||
for ep_num, episode_id in eps_dict.items():
|
||||
if (season, ep_num) not in new_missing_set:
|
||||
# Skip already-downloaded episodes — they should stay in DB
|
||||
# with is_downloaded=True to preserve download history
|
||||
if (season, ep_num) in downloaded_set:
|
||||
logger.debug(
|
||||
"Preserving downloaded episode in database: "
|
||||
"%s S%02dE%02d",
|
||||
serie.key,
|
||||
season,
|
||||
ep_num
|
||||
)
|
||||
continue
|
||||
await EpisodeService.delete(db, episode_id)
|
||||
logger.info(
|
||||
"Removed episode from database (no longer missing): "
|
||||
|
||||
@@ -966,6 +966,121 @@ class TestSaveAndLoadDB:
|
||||
mock_create.assert_called_once()
|
||||
assert mock_ep_create.call_count == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_series_deletes_downloaded_episodes_when_no_longer_missing(
|
||||
self, anime_service
|
||||
):
|
||||
"""Regression: a finished download marks the Episode row with
|
||||
is_downloaded=True. When a later rescan confirms the file is on
|
||||
disk (i.e. the episode is no longer in the missing set), the
|
||||
DB row should be deleted so it stops appearing in queries.
|
||||
|
||||
Bug shape: previously the ``downloaded_set`` guard in
|
||||
``_update_series_in_db`` preserved the row forever, so the DB
|
||||
kept stale ``is_downloaded=True`` entries that the user could
|
||||
see in the database but not in any UI.
|
||||
"""
|
||||
mock_serie = MagicMock()
|
||||
mock_serie.key = "the-100-girlfriends"
|
||||
mock_serie.name = "The 100 Girlfriends"
|
||||
mock_serie.site = "aniworld.to"
|
||||
mock_serie.folder = "The 100 Girlfriends (2023)"
|
||||
# Scanner reports no missing episodes for this series —
|
||||
# every file is on disk.
|
||||
mock_serie.episodeDict = {}
|
||||
|
||||
existing = MagicMock()
|
||||
existing.id = 1
|
||||
existing.folder = "The 100 Girlfriends (2023)"
|
||||
|
||||
# DB currently has one row for S03E08 marked as downloaded
|
||||
# (the result of an earlier successful download). The
|
||||
# scanner confirms the file is on disk, so the episode is
|
||||
# no longer missing.
|
||||
existing_eps = [
|
||||
MagicMock(
|
||||
id=10, season=3, episode_number=8, is_downloaded=True,
|
||||
),
|
||||
]
|
||||
|
||||
mock_session = AsyncMock()
|
||||
|
||||
with patch(
|
||||
"src.server.database.service.EpisodeService.get_by_series",
|
||||
new_callable=AsyncMock,
|
||||
return_value=existing_eps,
|
||||
), patch(
|
||||
"src.server.database.service.EpisodeService.delete",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_delete:
|
||||
await anime_service._update_series_in_db(
|
||||
mock_serie, existing, mock_session
|
||||
)
|
||||
|
||||
# The downloaded episode (S03E08) MUST be deleted — the file
|
||||
# is on disk and the scanner does not report it as missing.
|
||||
# EpisodeService.delete is (db, episode_id) — episode_id is
|
||||
# the second positional arg.
|
||||
deleted_ids = [
|
||||
call.args[1] for call in mock_delete.call_args_list
|
||||
]
|
||||
assert deleted_ids == [10], (
|
||||
f"Expected S03E08 (id=10) to be the only deleted row; "
|
||||
f"got delete calls for {deleted_ids}"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_series_keeps_still_missing_episodes(
|
||||
self, anime_service
|
||||
):
|
||||
"""A still-missing episode (is_downloaded=False, in scanner's
|
||||
missing set) must NOT be deleted by _update_series_in_db.
|
||||
Sanity-check sibling to the downloaded-episode regression
|
||||
test, ensuring the fix does not over-reach.
|
||||
"""
|
||||
mock_serie = MagicMock()
|
||||
mock_serie.key = "naruto"
|
||||
mock_serie.name = "Naruto"
|
||||
mock_serie.site = "aniworld.to"
|
||||
mock_serie.folder = "Naruto"
|
||||
# Scanner reports S01E07 still missing.
|
||||
mock_serie.episodeDict = {1: [7]}
|
||||
|
||||
existing = MagicMock()
|
||||
existing.id = 1
|
||||
existing.folder = "Naruto"
|
||||
|
||||
existing_eps = [
|
||||
MagicMock(
|
||||
id=20, season=1, episode_number=7, is_downloaded=False,
|
||||
),
|
||||
]
|
||||
|
||||
mock_session = AsyncMock()
|
||||
|
||||
with patch(
|
||||
"src.server.database.service.EpisodeService.get_by_series",
|
||||
new_callable=AsyncMock,
|
||||
return_value=existing_eps,
|
||||
), patch(
|
||||
"src.server.database.service.EpisodeService.delete",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_delete, patch(
|
||||
"src.server.database.service.EpisodeService.create",
|
||||
new_callable=AsyncMock,
|
||||
):
|
||||
await anime_service._update_series_in_db(
|
||||
mock_serie, existing, mock_session
|
||||
)
|
||||
|
||||
# S01E07 is still missing per the scanner — it must NOT be
|
||||
# deleted. (No new episode needs to be created either — it
|
||||
# is already in the DB.)
|
||||
assert mock_delete.call_count == 0, (
|
||||
f"Still-missing episode S01E07 must not be deleted; "
|
||||
f"got delete calls: {mock_delete.call_args_list}"
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_save_scan_results_updates_existing(
|
||||
self, anime_service
|
||||
|
||||
@@ -101,12 +101,20 @@ class TestSyncEpisodesToDb:
|
||||
"""Test _sync_episodes_to_db method."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_preserves_downloaded_episodes(self):
|
||||
"""Verify downloaded episodes are not removed even when no longer missing."""
|
||||
async def test_deletes_downloaded_episodes_when_no_longer_missing(self):
|
||||
"""Downloaded episodes are deleted once the rescan confirms the
|
||||
file is on disk and they are no longer missing. The DB stays
|
||||
in sync with the filesystem: a row that is not in the scanner's
|
||||
missing set has no further purpose and is removed.
|
||||
"""
|
||||
mock_session = AsyncMock()
|
||||
|
||||
# S01E1 was downloaded (file exists), S01E2 was missing but file now exists
|
||||
# Both are no longer in episode_dict
|
||||
# S01E1 was downloaded (file exists) and the scanner confirms
|
||||
# it is no longer missing; S01E2 was previously marked as
|
||||
# downloaded and is also no longer missing. Both should be
|
||||
# deleted — there is no notion of "preserving download history"
|
||||
# in the DB: the rescan's filesystem view is the source of
|
||||
# truth, and a row whose episode is no longer missing is dead.
|
||||
existing_eps = [
|
||||
MagicMock(id=1, season=1, episode_number=1, is_downloaded=True),
|
||||
MagicMock(id=2, season=1, episode_number=2, is_downloaded=True),
|
||||
@@ -126,8 +134,16 @@ class TestSyncEpisodesToDb:
|
||||
mock_session, 1, {} # No episodes missing
|
||||
)
|
||||
|
||||
# Neither should be deleted since both are downloaded
|
||||
mock_delete.assert_not_called()
|
||||
# Both downloaded rows should be deleted; the scanner
|
||||
# found the files on disk and they're not in the
|
||||
# missing set.
|
||||
assert mock_delete.call_count == 2
|
||||
deleted_calls = [
|
||||
(c.args[1], c.args[2], c.args[3])
|
||||
for c in mock_delete.call_args_list
|
||||
]
|
||||
assert (1, 1, 1) in deleted_calls
|
||||
assert (1, 1, 2) in deleted_calls
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_removes_missing_episodes_when_no_longer_missing(self):
|
||||
|
||||
Reference in New Issue
Block a user