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>
This commit is contained in:
@@ -82,6 +82,22 @@ The download queue prevents duplicate entries at two levels:
|
|||||||
- 5-minute cooldown prevents rapid re-triggers
|
- 5-minute cooldown prevents rapid re-triggers
|
||||||
- Checked at start of `_auto_download_missing()`
|
- 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
|
### Mocking the Download Queue
|
||||||
|
|
||||||
When testing components that use the download queue:
|
When testing components that use the download queue:
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import uuid
|
|||||||
from collections import deque
|
from collections import deque
|
||||||
from concurrent.futures import ThreadPoolExecutor
|
from concurrent.futures import ThreadPoolExecutor
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
|
from pathlib import Path
|
||||||
from typing import TYPE_CHECKING, Dict, List, Optional
|
from typing import TYPE_CHECKING, Dict, List, Optional
|
||||||
|
|
||||||
import structlog
|
import structlog
|
||||||
@@ -68,6 +69,7 @@ class DownloadService:
|
|||||||
progress_service: Optional progress service for tracking
|
progress_service: Optional progress service for tracking
|
||||||
"""
|
"""
|
||||||
self._anime_service = anime_service
|
self._anime_service = anime_service
|
||||||
|
self._directory = anime_service._directory
|
||||||
self._max_retries = max_retries
|
self._max_retries = max_retries
|
||||||
self._progress_service = progress_service or get_progress_service()
|
self._progress_service = progress_service or get_progress_service()
|
||||||
|
|
||||||
@@ -210,30 +212,33 @@ class DownloadService:
|
|||||||
series_key: str,
|
series_key: str,
|
||||||
season: int,
|
season: int,
|
||||||
episode: int,
|
episode: int,
|
||||||
|
serie_folder: Optional[str] = None,
|
||||||
) -> bool:
|
) -> 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:
|
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
|
2. The in-memory Serie.episodeDict and series_list cache
|
||||||
|
|
||||||
This ensures the episode no longer appears as missing in both
|
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:
|
Args:
|
||||||
series_key: Unique provider key for the series
|
series_key: Unique provider key for the series
|
||||||
season: Season number
|
season: Season number
|
||||||
episode: Episode number within season
|
episode: Episode number within season
|
||||||
|
serie_folder: Series folder name (required for file_path)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
True if episode was removed, False otherwise
|
True if episode was updated, False otherwise
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
from src.server.database.connection import get_db_session
|
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(
|
logger.info(
|
||||||
"Attempting to remove missing episode from DB: "
|
"Attempting to mark episode as downloaded in DB: "
|
||||||
"%s S%02dE%02d",
|
"%s S%02dE%02d",
|
||||||
series_key,
|
series_key,
|
||||||
season,
|
season,
|
||||||
@@ -241,28 +246,63 @@ class DownloadService:
|
|||||||
)
|
)
|
||||||
|
|
||||||
async with get_db_session() as db:
|
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,
|
db=db,
|
||||||
series_key=series_key,
|
series_id=series.id,
|
||||||
season=season,
|
season=season,
|
||||||
episode_number=episode,
|
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(
|
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",
|
"%s S%02dE%02d",
|
||||||
series_key,
|
series_key,
|
||||||
season,
|
season,
|
||||||
episode,
|
episode,
|
||||||
)
|
)
|
||||||
else:
|
return False
|
||||||
logger.warning(
|
|
||||||
"Episode not found in DB missing list "
|
|
||||||
"(may already be removed): %s S%02dE%02d",
|
|
||||||
series_key,
|
|
||||||
season,
|
|
||||||
episode,
|
|
||||||
)
|
|
||||||
|
|
||||||
# Update in-memory Serie.episodeDict so list_missing is
|
# Update in-memory Serie.episodeDict so list_missing is
|
||||||
# immediately consistent without a full DB reload
|
# immediately consistent without a full DB reload
|
||||||
@@ -273,8 +313,8 @@ class DownloadService:
|
|||||||
try:
|
try:
|
||||||
self._anime_service._cached_list_missing.cache_clear()
|
self._anime_service._cached_list_missing.cache_clear()
|
||||||
logger.debug(
|
logger.debug(
|
||||||
"Cleared list_missing cache after removing "
|
"Cleared list_missing cache after marking "
|
||||||
"%s S%02dE%02d",
|
"%s S%02dE%02d as downloaded",
|
||||||
series_key,
|
series_key,
|
||||||
season,
|
season,
|
||||||
episode,
|
episode,
|
||||||
@@ -282,10 +322,10 @@ class DownloadService:
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
return deleted
|
return True
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(
|
logger.error(
|
||||||
"Failed to remove episode from missing list: "
|
"Failed to mark episode as downloaded: "
|
||||||
"%s S%02dE%02d - %s",
|
"%s S%02dE%02d - %s",
|
||||||
series_key,
|
series_key,
|
||||||
season,
|
season,
|
||||||
@@ -1119,12 +1159,13 @@ class DownloadService:
|
|||||||
# Delete completed item from download queue database
|
# Delete completed item from download queue database
|
||||||
await self._delete_from_database(item.id)
|
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)
|
# (both database and in-memory)
|
||||||
removed = await self._remove_episode_from_missing_list(
|
removed = await self._remove_episode_from_missing_list(
|
||||||
series_key=item.serie_id,
|
series_key=item.serie_id,
|
||||||
season=item.episode.season,
|
season=item.episode.season,
|
||||||
episode=item.episode.episode,
|
episode=item.episode.episode,
|
||||||
|
serie_folder=item.serie_folder,
|
||||||
)
|
)
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
|
|||||||
@@ -79,6 +79,7 @@ def mock_anime_service():
|
|||||||
"""Create a mock AnimeService."""
|
"""Create a mock AnimeService."""
|
||||||
service = MagicMock(spec=AnimeService)
|
service = MagicMock(spec=AnimeService)
|
||||||
service.download = AsyncMock(return_value=True)
|
service.download = AsyncMock(return_value=True)
|
||||||
|
service._directory = "/mock/anime/directory"
|
||||||
return service
|
return service
|
||||||
|
|
||||||
|
|
||||||
@@ -731,13 +732,22 @@ class TestRemoveEpisodeFromMissingList:
|
|||||||
download_service._anime_service._app = mock_app
|
download_service._anime_service._app = mock_app
|
||||||
download_service._anime_service._cached_list_missing = MagicMock()
|
download_service._anime_service._cached_list_missing = MagicMock()
|
||||||
|
|
||||||
# Mock DB call
|
# Mock DB session
|
||||||
mock_db_session = AsyncMock()
|
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(
|
with patch(
|
||||||
"src.server.database.connection.get_db_session"
|
"src.server.database.connection.get_db_session"
|
||||||
) as mock_get_db, patch(
|
) as mock_get_db, patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService"
|
||||||
|
) as mock_series_svc, patch(
|
||||||
"src.server.database.service.EpisodeService"
|
"src.server.database.service.EpisodeService"
|
||||||
) as mock_ep_svc:
|
) as mock_ep_svc:
|
||||||
mock_get_db.return_value.__aenter__ = AsyncMock(
|
mock_get_db.return_value.__aenter__ = AsyncMock(
|
||||||
@@ -746,20 +756,30 @@ class TestRemoveEpisodeFromMissingList:
|
|||||||
mock_get_db.return_value.__aexit__ = AsyncMock(
|
mock_get_db.return_value.__aexit__ = AsyncMock(
|
||||||
return_value=False
|
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(
|
result = await download_service._remove_episode_from_missing_list(
|
||||||
series_key="test-series",
|
series_key="test-series",
|
||||||
season=1,
|
season=1,
|
||||||
episode=2,
|
episode=2,
|
||||||
|
serie_folder="Test Series (2024)",
|
||||||
)
|
)
|
||||||
|
|
||||||
# DB deletion was called
|
# mark_downloaded was called instead of delete
|
||||||
mock_delete.assert_awaited_once_with(
|
mock_ep_svc.mark_downloaded.assert_awaited_once_with(
|
||||||
db=mock_db_session,
|
db=mock_db_session,
|
||||||
series_key="test-series",
|
episode_id=100,
|
||||||
season=1,
|
file_path=(
|
||||||
episode_number=2,
|
f"{download_service._directory}/Test Series (2024)/Season 1"
|
||||||
|
),
|
||||||
)
|
)
|
||||||
# In-memory update happened
|
# In-memory update happened
|
||||||
assert 2 not in serie.episodeDict[1]
|
assert 2 not in serie.episodeDict[1]
|
||||||
@@ -807,11 +827,20 @@ class TestRemoveEpisodeFromMissingList:
|
|||||||
|
|
||||||
# Mock DB calls
|
# Mock DB calls
|
||||||
mock_db_session = AsyncMock()
|
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(
|
with patch(
|
||||||
"src.server.database.connection.get_db_session"
|
"src.server.database.connection.get_db_session"
|
||||||
) as mock_get_db, patch(
|
) as mock_get_db, patch(
|
||||||
|
"src.server.database.service.AnimeSeriesService"
|
||||||
|
) as mock_series_svc, patch(
|
||||||
"src.server.database.service.EpisodeService"
|
"src.server.database.service.EpisodeService"
|
||||||
) as mock_ep_svc:
|
) as mock_ep_svc:
|
||||||
mock_get_db.return_value.__aenter__ = AsyncMock(
|
mock_get_db.return_value.__aenter__ = AsyncMock(
|
||||||
@@ -820,7 +849,15 @@ class TestRemoveEpisodeFromMissingList:
|
|||||||
mock_get_db.return_value.__aexit__ = AsyncMock(
|
mock_get_db.return_value.__aexit__ = AsyncMock(
|
||||||
return_value=False
|
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
|
# Process the download
|
||||||
item = download_service._pending_queue.popleft()
|
item = download_service._pending_queue.popleft()
|
||||||
|
|||||||
Reference in New Issue
Block a user