From 0ba2587bc8bf7d4c4ce41f4f17c7d04b1a712937 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 25 May 2026 14:14:33 +0200 Subject: [PATCH] refactor(download): mark episode downloaded instead of deleting Change _remove_episode_from_missing_list to set is_downloaded=True and populate file_path via EpisodeService.mark_downloaded, instead of deleting the Episode row. Preserves download history so queries can distinguish series with downloaded episodes from completely unwatched series. - Pass serie_folder to construct file_path - Look up series_id via AnimeSeriesService.get_by_key - Update tests to mock mark_downloaded path - Document episode lifecycle in docs/DEVELOPMENT.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/DEVELOPMENT.md | 16 +++++ src/server/services/download_service.py | 87 ++++++++++++++++++------- tests/unit/test_download_service.py | 57 +++++++++++++--- 3 files changed, 127 insertions(+), 33 deletions(-) diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 10a8447..92463f0 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -82,6 +82,22 @@ The download queue prevents duplicate entries at two levels: - 5-minute cooldown prevents rapid re-triggers - Checked at start of `_auto_download_missing()` +### Episode Lifecycle + +Episodes transition through states stored in the `episodes` table: + +| State | `is_downloaded` | `file_path` | Description | +|-------|----------------|-------------|-------------| +| Missing | `False` | `NULL` | Episode not yet downloaded | +| Downloaded | `True` | Set | Episode exists on disk | + +**State Transitions:** +1. **Missing → Downloaded**: When download completes, `_remove_episode_from_missing_list()` calls `EpisodeService.mark_downloaded()` to set `is_downloaded=True` and populate `file_path`. The episode record is NOT deleted. + +**Query Implications:** +- `get_series_with_missing_episodes()`: Filters for `is_downloaded=False` to find series with undownloaded episodes +- `get_series_with_no_episodes()`: Finds series with `is_downloaded=False` episodes but NO `is_downloaded=True` episodes (completely unwatched series) + ### Mocking the Download Queue When testing components that use the download queue: diff --git a/src/server/services/download_service.py b/src/server/services/download_service.py index da23a55..f31e825 100644 --- a/src/server/services/download_service.py +++ b/src/server/services/download_service.py @@ -14,6 +14,7 @@ import uuid from collections import deque from concurrent.futures import ThreadPoolExecutor from datetime import datetime, timezone +from pathlib import Path from typing import TYPE_CHECKING, Dict, List, Optional import structlog @@ -68,6 +69,7 @@ class DownloadService: progress_service: Optional progress service for tracking """ self._anime_service = anime_service + self._directory = anime_service._directory self._max_retries = max_retries self._progress_service = progress_service or get_progress_service() @@ -210,30 +212,33 @@ class DownloadService: series_key: str, season: int, episode: int, + serie_folder: Optional[str] = None, ) -> bool: - """Remove a downloaded episode from the missing episodes list. + """Mark a downloaded episode as downloaded instead of deleting it. Called when a download completes successfully to update both: - 1. The database (Episode record deleted) + 1. The database (Episode record marked is_downloaded=True) 2. The in-memory Serie.episodeDict and series_list cache This ensures the episode no longer appears as missing in both - the API responses and the UI immediately after download. + the API responses and the UI immediately after download, + while preserving the download history. Args: series_key: Unique provider key for the series season: Season number episode: Episode number within season + serie_folder: Series folder name (required for file_path) Returns: - True if episode was removed, False otherwise + True if episode was updated, False otherwise """ try: from src.server.database.connection import get_db_session - from src.server.database.service import EpisodeService + from src.server.database.service import EpisodeService, AnimeSeriesService logger.info( - "Attempting to remove missing episode from DB: " + "Attempting to mark episode as downloaded in DB: " "%s S%02dE%02d", series_key, season, @@ -241,28 +246,63 @@ class DownloadService: ) async with get_db_session() as db: - deleted = await EpisodeService.delete_by_series_and_episode( + # Get series by key to find series_id + series = await AnimeSeriesService.get_by_key(db, series_key) + if not series: + logger.warning( + "Series not found for key: %s", series_key + ) + return False + + # Get episode by series_id, season, episode_number + ep = await EpisodeService.get_by_episode( db=db, - series_key=series_key, + series_id=series.id, season=season, episode_number=episode, ) - if deleted: + if not ep: + logger.warning( + "Episode not found in DB: %s S%02dE%02d", + series_key, + season, + episode, + ) + return False + + # Construct file_path if serie_folder provided + file_path = None + if serie_folder: + season_folder = f"Season {season}" + file_path = str( + Path(self._directory) / serie_folder / season_folder + ) + + # Mark episode as downloaded instead of deleting + updated = await EpisodeService.mark_downloaded( + db=db, + episode_id=ep.id, + file_path=file_path or "", + ) + + if updated: logger.info( - "Successfully removed episode from DB missing list: " + "Marked episode as downloaded in DB: " + "%s S%02dE%02d, file_path=%s", + series_key, + season, + episode, + file_path, + ) + else: + logger.warning( + "Failed to mark episode as downloaded: " "%s S%02dE%02d", series_key, season, episode, ) - else: - logger.warning( - "Episode not found in DB missing list " - "(may already be removed): %s S%02dE%02d", - series_key, - season, - episode, - ) + return False # Update in-memory Serie.episodeDict so list_missing is # immediately consistent without a full DB reload @@ -273,8 +313,8 @@ class DownloadService: try: self._anime_service._cached_list_missing.cache_clear() logger.debug( - "Cleared list_missing cache after removing " - "%s S%02dE%02d", + "Cleared list_missing cache after marking " + "%s S%02dE%02d as downloaded", series_key, season, episode, @@ -282,10 +322,10 @@ class DownloadService: except Exception: pass - return deleted + return True except Exception as e: logger.error( - "Failed to remove episode from missing list: " + "Failed to mark episode as downloaded: " "%s S%02dE%02d - %s", series_key, season, @@ -1119,12 +1159,13 @@ class DownloadService: # Delete completed item from download queue database await self._delete_from_database(item.id) - # Remove episode from missing episodes list + # Mark episode as downloaded in missing episodes list # (both database and in-memory) removed = await self._remove_episode_from_missing_list( series_key=item.serie_id, season=item.episode.season, episode=item.episode.episode, + serie_folder=item.serie_folder, ) logger.info( diff --git a/tests/unit/test_download_service.py b/tests/unit/test_download_service.py index 63044db..3458a3e 100644 --- a/tests/unit/test_download_service.py +++ b/tests/unit/test_download_service.py @@ -79,6 +79,7 @@ def mock_anime_service(): """Create a mock AnimeService.""" service = MagicMock(spec=AnimeService) service.download = AsyncMock(return_value=True) + service._directory = "/mock/anime/directory" return service @@ -731,13 +732,22 @@ class TestRemoveEpisodeFromMissingList: download_service._anime_service._app = mock_app download_service._anime_service._cached_list_missing = MagicMock() - # Mock DB call + # Mock DB session mock_db_session = AsyncMock() - mock_delete = AsyncMock(return_value=True) + + # Mock series returned by get_by_key + mock_series = MagicMock() + mock_series.id = 1 + + # Mock episode returned by get_by_episode + mock_episode = MagicMock() + mock_episode.id = 100 with patch( "src.server.database.connection.get_db_session" ) as mock_get_db, patch( + "src.server.database.service.AnimeSeriesService" + ) as mock_series_svc, patch( "src.server.database.service.EpisodeService" ) as mock_ep_svc: mock_get_db.return_value.__aenter__ = AsyncMock( @@ -746,20 +756,30 @@ class TestRemoveEpisodeFromMissingList: mock_get_db.return_value.__aexit__ = AsyncMock( return_value=False ) - mock_ep_svc.delete_by_series_and_episode = mock_delete + + # Mock get_by_key to return series + mock_series_svc.get_by_key = AsyncMock(return_value=mock_series) + + # Mock get_by_episode to return episode + mock_ep_svc.get_by_episode = AsyncMock(return_value=mock_episode) + + # Mock mark_downloaded to succeed + mock_ep_svc.mark_downloaded = AsyncMock(return_value=mock_episode) result = await download_service._remove_episode_from_missing_list( series_key="test-series", season=1, episode=2, + serie_folder="Test Series (2024)", ) - # DB deletion was called - mock_delete.assert_awaited_once_with( + # mark_downloaded was called instead of delete + mock_ep_svc.mark_downloaded.assert_awaited_once_with( db=mock_db_session, - series_key="test-series", - season=1, - episode_number=2, + episode_id=100, + file_path=( + f"{download_service._directory}/Test Series (2024)/Season 1" + ), ) # In-memory update happened assert 2 not in serie.episodeDict[1] @@ -807,11 +827,20 @@ class TestRemoveEpisodeFromMissingList: # Mock DB calls mock_db_session = AsyncMock() - mock_delete = AsyncMock(return_value=True) + + # Mock series returned by get_by_key + mock_series = MagicMock() + mock_series.id = 1 + + # Mock episode returned by get_by_episode + mock_episode = MagicMock() + mock_episode.id = 100 with patch( "src.server.database.connection.get_db_session" ) as mock_get_db, patch( + "src.server.database.service.AnimeSeriesService" + ) as mock_series_svc, patch( "src.server.database.service.EpisodeService" ) as mock_ep_svc: mock_get_db.return_value.__aenter__ = AsyncMock( @@ -820,7 +849,15 @@ class TestRemoveEpisodeFromMissingList: mock_get_db.return_value.__aexit__ = AsyncMock( return_value=False ) - mock_ep_svc.delete_by_series_and_episode = mock_delete + + # Mock get_by_key to return series + mock_series_svc.get_by_key = AsyncMock(return_value=mock_series) + + # Mock get_by_episode to return episode + mock_ep_svc.get_by_episode = AsyncMock(return_value=mock_episode) + + # Mock mark_downloaded to succeed + mock_ep_svc.mark_downloaded = AsyncMock(return_value=mock_episode) # Process the download item = download_service._pending_queue.popleft()