From 1d121b427dec22ee35c85d246bbbcdd7abe75503 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 15 Sep 2026 20:34:12 +0200 Subject: [PATCH] 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. --- src/server/SerieScanner.py | 21 ++-- src/server/services/anime_service.py | 49 ++++----- tests/unit/test_anime_service.py | 115 +++++++++++++++++++++ tests/unit/test_serie_scanner_db_writes.py | 28 +++-- 4 files changed, 166 insertions(+), 47 deletions(-) diff --git a/src/server/SerieScanner.py b/src/server/SerieScanner.py index 735c60d..2f38c5f 100644 --- a/src/server/SerieScanner.py +++ b/src/server/SerieScanner.py @@ -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,15 +304,9 @@ 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 - ) + await EpisodeService.delete_by_series( + db, series_id, season, ep_num + ) for season, eps in episode_dict.items(): for ep_num in eps: if (season, ep_num) not in existing_map: diff --git a/src/server/services/anime_service.py b/src/server/services/anime_service.py index 3a1927b..a1f0922 100644 --- a/src/server/services/anime_service.py +++ b/src/server/services/anime_service.py @@ -857,39 +857,39 @@ class AnimeService: async def _update_series_in_db(self, serie, existing, db) -> None: """Update an existing series in the database. - + 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 # Get existing episodes from database (all episodes, including downloaded) 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 {} - + # Build set of new missing episodes for quick lookup new_missing_set: set[tuple[int, int]] = set() for season, episode_numbers in new_dict.items(): for ep_num in episode_numbers: new_missing_set.add((season, ep_num)) - + # Add new missing episodes that are not in the database for season, episode_numbers in new_dict.items(): existing_season_eps = existing_dict.get(season, {}) @@ -907,25 +907,16 @@ class AnimeService: season, ep_num ) - + # 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): " @@ -934,7 +925,7 @@ class AnimeService: season, ep_num ) - + # Update folder if changed if existing.folder != serie.folder: await AnimeSeriesService.update( @@ -942,7 +933,7 @@ class AnimeService: existing.id, folder=serie.folder ) - + logger.debug( "Updated series in database: %s (key=%s)", serie.name, diff --git a/tests/unit/test_anime_service.py b/tests/unit/test_anime_service.py index 09cb88e..1d1a862 100644 --- a/tests/unit/test_anime_service.py +++ b/tests/unit/test_anime_service.py @@ -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 diff --git a/tests/unit/test_serie_scanner_db_writes.py b/tests/unit/test_serie_scanner_db_writes.py index 9dfeed4..5c7962c 100644 --- a/tests/unit/test_serie_scanner_db_writes.py +++ b/tests/unit/test_serie_scanner_db_writes.py @@ -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):