diff --git a/Makefile b/Makefile index 74f4b63..8f75517 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ BACKEND_VIRTUALENV_ZIPAPP := $(BACKEND_DIR)/.virtualenv.pyz PYTHON ?= python3 NPM ?= pnpm -.PHONY: help backend-venv backend-lint backend-test backend-test-audiodb backend-test-audiodb-prewarm backend-test-audiodb-settings backend-test-coverart-audiodb backend-test-audiodb-phase8 backend-test-audiodb-phase9 backend-test-exception-handling backend-test-playlist backend-test-multidisc backend-test-performance backend-test-security backend-test-config-validation backend-test-home backend-test-home-genre backend-test-infra-hardening backend-test-library-pagination backend-test-search-top-result test-audiodb-all frontend-install frontend-build frontend-check frontend-lint frontend-test frontend-test-queuehelpers frontend-test-album-page frontend-test-playlist-detail frontend-test-audiodb-images frontend-browser-install project-map rebuild test check lint ci +.PHONY: help backend-venv backend-lint backend-test backend-test-audiodb backend-test-audiodb-prewarm backend-test-audiodb-settings backend-test-coverart-audiodb backend-test-audiodb-phase8 backend-test-audiodb-phase9 backend-test-exception-handling backend-test-playlist backend-test-multidisc backend-test-performance backend-test-security backend-test-config-validation backend-test-home backend-test-home-genre backend-test-infra-hardening backend-test-library-pagination backend-test-search-top-result test-audiodb-all backend-test-artist-page backend-test-monitoring-cache frontend-install frontend-build frontend-format-check frontend-check frontend-lint frontend-test frontend-test-queuehelpers frontend-test-album-page frontend-test-playlist-detail frontend-test-audiodb-images frontend-browser-install project-map rebuild test check lint ci help: ## Show available targets @grep -E '^[a-zA-Z0-9_.-]+:.*## ' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*## "}; {printf "%-26s %s\n", $$1, $$2}' @@ -82,6 +82,9 @@ backend-test-infra-hardening: $(BACKEND_VENV_STAMP) ## Run infrastructure harden backend-test-discovery-precache: $(BACKEND_VENV_STAMP) ## Run artist discovery precache tests cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/services/test_discovery_precache_progress.py tests/services/test_discovery_precache_lock.py tests/infrastructure/test_retry_non_breaking.py -v +backend-test-dedup-cancellation: $(BACKEND_VENV_STAMP) ## Run deduplicator cancellation tests + cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/infrastructure/test_dedup_cancellation.py tests/infrastructure/test_disconnect.py -v + backend-test-library-pagination: $(BACKEND_VENV_STAMP) ## Run library pagination tests cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/infrastructure/test_library_pagination.py -v @@ -130,12 +133,21 @@ backend-test-mus15-status-race: $(BACKEND_VENV_STAMP) ## Run MUS-15 status race backend-test-artist-monitoring: $(BACKEND_VENV_STAMP) ## Run MUS-15B artist monitoring tests cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_artist_monitoring.py -v +backend-test-monitoring-cache: $(BACKEND_VENV_STAMP) ## Run artist monitoring cache/flag refresh tests + cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/services/test_refresh_library_flags.py tests/test_queue_disk_invalidation.py tests/services/test_artist_utils_tags.py -v + backend-test-album-refresh: $(BACKEND_VENV_STAMP) ## Run album refresh endpoint tests cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/routes/test_album_refresh.py tests/services/test_navidrome_cache_invalidation.py -v +backend-test-artist-page: $(BACKEND_VENV_STAMP) ## Run artist page latency tests (basic route, releases, Last.fm fast path) + cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/routes/test_artist_basic_route.py tests/routes/test_artist_releases_route.py tests/services/test_artist_basic_info.py tests/services/test_top_albums_lastfm_fast.py -v + test-audiodb-all: backend-test-audiodb backend-test-audiodb-prewarm backend-test-audiodb-settings backend-test-coverart-audiodb backend-test-audiodb-phase8 backend-test-audiodb-phase9 frontend-test-audiodb-images ## Run every AudioDB test target -test-sync-all: backend-test-sync-watchdog backend-test-sync-resume backend-test-audiodb-parallel ## Run all sync robustness tests +backend-test-sync-generation: $(BACKEND_VENV_STAMP) ## Run MUS-19 sync generation counter tests + cd "$(BACKEND_DIR)" && .venv/bin/python -m pytest tests/test_sync_generation.py -v + +test-sync-all: backend-test-sync-watchdog backend-test-sync-resume backend-test-audiodb-parallel backend-test-sync-generation ## Run all sync robustness tests frontend-install: ## Install frontend npm dependencies cd "$(FRONTEND_DIR)" && $(NPM) install @@ -143,6 +155,9 @@ frontend-install: ## Install frontend npm dependencies frontend-build: ## Run frontend production build cd "$(FRONTEND_DIR)" && $(NPM) run build +frontend-format-check: ## Run frontend formatting checks + cd "$(FRONTEND_DIR)" && $(NPM) run format:check + frontend-check: ## Run frontend type checks cd "$(FRONTEND_DIR)" && $(NPM) run check @@ -155,6 +170,9 @@ frontend-test: ## Run the frontend vitest suite frontend-test-queuehelpers: ## Run queue helper regressions cd "$(FRONTEND_DIR)" && $(NPM) exec vitest run --project server src/lib/player/queueHelpers.spec.ts +frontend-test-monitored-artists: ## Run pending monitored artist store tests + cd "$(FRONTEND_DIR)" && $(NPM) exec vitest run --project server src/lib/stores/monitoredArtists.spec.ts + frontend-test-album-page: ## Run the album page browser test cd "$(FRONTEND_DIR)" && $(NPM) exec vitest run --project client src/routes/album/[id]/page.svelte.spec.ts diff --git a/backend/api/v1/routes/artists.py b/backend/api/v1/routes/artists.py index 57755f7..cc2c125 100644 --- a/backend/api/v1/routes/artists.py +++ b/backend/api/v1/routes/artists.py @@ -36,7 +36,7 @@ async def get_artist( ) try: - result = await artist_service.get_artist_info(artist_id) + result = await artist_service.get_artist_info_basic(artist_id) ctx = try_get_degradation_context() if ctx and ctx.has_degradation(): result = msgspec.structs.replace(result, service_status=ctx.degraded_summary()) diff --git a/backend/api/v1/routes/cache_status.py b/backend/api/v1/routes/cache_status.py index 37b4961..5855983 100644 --- a/backend/api/v1/routes/cache_status.py +++ b/backend/api/v1/routes/cache_status.py @@ -36,6 +36,17 @@ async def get_sync_status( ) +@router.post("/cancel") +async def cancel_sync( + status_service: CacheStatusService = Depends(get_cache_status_service), +): + from core.task_registry import TaskRegistry + await status_service.cancel_current_sync() + TaskRegistry.get_instance().cancel("precache-library") + await status_service.wait_for_completion() + return {"status": "cancelled"} + + @router.get("/stream") async def stream_sync_status( status_service: CacheStatusService = Depends(get_cache_status_service), diff --git a/backend/api/v1/schemas/advanced_settings.py b/backend/api/v1/schemas/advanced_settings.py index 29a94e9..123e9c8 100644 --- a/backend/api/v1/schemas/advanced_settings.py +++ b/backend/api/v1/schemas/advanced_settings.py @@ -62,8 +62,8 @@ class AdvancedSettings(AppStruct): delay_albums: float = 0.3 artist_discovery_warm_interval: int = 14400 artist_discovery_warm_delay: float = 0.5 - artist_discovery_precache_delay: float = 0.3 - artist_discovery_precache_concurrency: int = 3 + artist_discovery_precache_delay: float = 0.2 + artist_discovery_precache_concurrency: int = 5 memory_cache_max_entries: int = 10000 memory_cache_cleanup_interval: int = 300 cover_memory_cache_max_entries: int = 128 @@ -237,7 +237,7 @@ class AdvancedSettingsFrontend(AppStruct): delay_albums: float = 0.3 artist_discovery_warm_interval: int = 240 artist_discovery_warm_delay: float = 0.5 - artist_discovery_precache_delay: float = 0.3 + artist_discovery_precache_delay: float = 0.2 memory_cache_max_entries: int = 10000 memory_cache_cleanup_interval: int = 300 cover_memory_cache_max_entries: int = 128 @@ -285,7 +285,7 @@ class AdvancedSettingsFrontend(AppStruct): audiodb_prewarm_concurrency: int = 4 audiodb_prewarm_delay: float = 0.3 request_concurrency: int = 2 - artist_discovery_precache_concurrency: int = 3 + artist_discovery_precache_concurrency: int = 5 def __post_init__(self) -> None: int_coerce_fields = [ diff --git a/backend/core/dependencies/service_providers.py b/backend/core/dependencies/service_providers.py index f5ad2ac..5f6aa6f 100644 --- a/backend/core/dependencies/service_providers.py +++ b/backend/core/dependencies/service_providers.py @@ -1,4 +1,4 @@ -"""Tier 4 — Business-logic service providers.""" +"""Tier 4 - Business-logic service providers.""" from __future__ import annotations @@ -91,19 +91,8 @@ def get_album_service() -> "AlbumService": return AlbumService(lidarr_repo, mb_repo, library_db, memory_cache, disk_cache, preferences_service, audiodb_image_service, browse_queue) -@singleton -def get_request_queue() -> "RequestQueue": - from infrastructure.queue.request_queue import RequestQueue - from infrastructure.queue.queue_store import QueueStore - from infrastructure.persistence.request_history import RequestHistoryRecord - from core.config import get_settings - settings = get_settings() - - lidarr_repo = get_lidarr_repository() - disk_cache = get_disk_cache() - cover_repo = get_coverart_repository() - memory_cache = get_cache() - library_db = get_library_db() +def make_on_queue_import(memory_cache, disk_cache, library_db): + """Create the on_queue_import closure used by the request queue.""" async def on_queue_import(record: RequestHistoryRecord) -> None: """Invalidate caches when the queue worker detects an already-imported album.""" @@ -118,6 +107,9 @@ def get_request_queue() -> "RequestQueue": invalidations.append( memory_cache.delete(f"{ARTIST_INFO_PREFIX}{record.artist_mbid}") ) + invalidations.append( + disk_cache.delete_artist(record.artist_mbid) + ) await asyncio.gather(*invalidations, return_exceptions=True) try: await library_db.upsert_album({ @@ -133,6 +125,12 @@ def get_request_queue() -> "RequestQueue": logger.warning("Queue import: failed to upsert album %s: %s", record.musicbrainz_id[:8], ex) logger.info("Queue import: invalidated caches for album=%s", record.musicbrainz_id[:8]) + return on_queue_import + + +def make_processor(lidarr_repo, memory_cache, disk_cache, cover_repo, request_history): + """Create the processor closure used by the request queue.""" + async def processor(album_mbid: str) -> dict: result = await lidarr_repo.add_album(album_mbid) @@ -140,7 +138,7 @@ def get_request_queue() -> "RequestQueue": if payload and isinstance(payload, dict): is_monitored = payload.get("monitored", False) - # Belt-andsuspenders: prefer structured signal over payload inspection + # Prefer the explicit monitored flag before falling back to the top-level result. if not is_monitored: is_monitored = bool(result.get("monitored")) @@ -174,6 +172,7 @@ def get_request_queue() -> "RequestQueue": record.artist_mbid, monitored=True, monitor_new_items=monitor_new, ) await memory_cache.delete(f"{ARTIST_INFO_PREFIX}{record.artist_mbid}") + await disk_cache.delete_artist(record.artist_mbid) logger.info("Applied deferred artist monitoring for %s", record.artist_mbid[:8]) break except Exception: # noqa: BLE001 @@ -186,9 +185,29 @@ def get_request_queue() -> "RequestQueue": return result + return processor + + +@singleton +def get_request_queue() -> "RequestQueue": + from infrastructure.queue.request_queue import RequestQueue + from infrastructure.queue.queue_store import QueueStore + from core.config import get_settings + settings = get_settings() + + lidarr_repo = get_lidarr_repository() + disk_cache = get_disk_cache() + cover_repo = get_coverart_repository() + memory_cache = get_cache() + library_db = get_library_db() + + on_queue_import = make_on_queue_import(memory_cache, disk_cache, library_db) + store = QueueStore(db_path=settings.queue_db_path) request_history = get_request_history_store() + processor = make_processor(lidarr_repo, memory_cache, disk_cache, cover_repo, request_history) + concurrency = 2 try: from services.preferences_service import PreferencesService diff --git a/backend/core/task_registry.py b/backend/core/task_registry.py index 53cebbd..ae935b3 100644 --- a/backend/core/task_registry.py +++ b/backend/core/task_registry.py @@ -40,6 +40,21 @@ class TaskRegistry: with self._lock: self._tasks.pop(name, None) + async def cancel(self, name: str, grace_period: float = 10.0) -> None: + with self._lock: + task = self._tasks.pop(name, None) + + if task is None or task.done(): + return + + task.cancel() + try: + await asyncio.wait_for(task, timeout=grace_period) + except asyncio.CancelledError: + return + except asyncio.TimeoutError: + logger.warning("Task '%s' did not finish within grace period", name) + async def cancel_all(self, grace_period: float = 10.0) -> None: with self._lock: tasks = dict(self._tasks) diff --git a/backend/core/tasks.py b/backend/core/tasks.py index 37fba85..95a025b 100644 --- a/backend/core/tasks.py +++ b/backend/core/tasks.py @@ -116,10 +116,12 @@ async def sync_library_periodically( logger.info(f"Auto-syncing library (frequency: {sync_freq})") sync_success = False + should_update_status = True try: result = await library_service.sync_library() if result.status == "skipped": logger.info("Auto-sync skipped - sync already in progress") + should_update_status = False continue sync_success = True logger.info("Auto-sync completed successfully") @@ -129,12 +131,13 @@ async def sync_library_periodically( sync_success = False finally: - lidarr_settings = preferences_service.get_lidarr_settings() - updated_settings = clone_with_updates(lidarr_settings, { - 'last_sync': int(time()), - 'last_sync_success': sync_success - }) - preferences_service.save_lidarr_settings(updated_settings) + if should_update_status: + lidarr_settings = preferences_service.get_lidarr_settings() + updated_settings = clone_with_updates(lidarr_settings, { + 'last_sync': int(time()), + 'last_sync_success': sync_success + }) + preferences_service.save_lidarr_settings(updated_settings) except asyncio.CancelledError: logger.info("Library sync task cancelled") diff --git a/backend/infrastructure/http/deduplication.py b/backend/infrastructure/http/deduplication.py index 5fabeed..8460842 100644 --- a/backend/infrastructure/http/deduplication.py +++ b/backend/infrastructure/http/deduplication.py @@ -9,28 +9,36 @@ logger = logging.getLogger(__name__) T = TypeVar("T") +_MAX_DEDUP_RETRIES = 1 + class RequestDeduplicator: """ Prevents duplicate concurrent requests by coalescing identical requests. - + If request A is in-flight and request B arrives with the same key, request B will wait for A's result instead of making a duplicate call. + + Uses ``asyncio.shield`` so that a waiter's task cancellation propagates + cleanly without poisoning the shared future. If the leader disconnects + (``ClientDisconnectedError``), the shared future is cancelled and one + waiting follower is allowed to retry as the new leader (bounded to + ``_MAX_DEDUP_RETRIES`` attempts). """ - + def __init__(self): self._pending: dict[str, asyncio.Future[Any]] = {} self._lock = asyncio.Lock() - + async def dedupe( self, key: str, coro_factory: Callable[[], Awaitable[T]] ) -> T: + retries = 0 while True: async with self._lock: if key in self._pending: - logger.debug(f"Deduplicating request: {key}") future = self._pending[key] should_execute = False else: @@ -41,21 +49,35 @@ class RequestDeduplicator: if should_execute: try: result = await coro_factory() - future.set_result(result) + if not future.done(): + future.set_result(result) + return result except ClientDisconnectedError: - future.cancel() + if not future.done(): + future.cancel() + raise + except BaseException as exc: + if not future.done(): + future.set_exception(exc) raise - except Exception as e: # noqa: BLE001 - future.set_exception(e) finally: if not future.done(): future.cancel() async with self._lock: self._pending.pop(key, None) + # Follower path: shield prevents waiter cancellation from poisoning the shared future. try: - return await future + return await asyncio.shield(future) except asyncio.CancelledError: + task = asyncio.current_task() + if task is not None and task.cancelling() > 0: + raise + # Future was cancelled by the leader (disconnect). Retry once + # so this follower can take over as leader. + retries += 1 + if retries > _MAX_DEDUP_RETRIES: + raise continue @@ -70,7 +92,7 @@ def deduplicate(key_func: Callable[..., str]): """ Decorator that deduplicates concurrent calls to the same function with the same key. - + Usage: @deduplicate(lambda self, artist_id: f"artist:{artist_id}") async def get_artist(self, artist_id: str) -> Artist: diff --git a/backend/repositories/lidarr/album.py b/backend/repositories/lidarr/album.py index 75a4536..0dc897c 100644 --- a/backend/repositories/lidarr/album.py +++ b/backend/repositories/lidarr/album.py @@ -423,7 +423,7 @@ class LidarrAlbumRepository(LidarrHistoryRepository): async def album_is_indexed(): a = await self._get_album_by_foreign_id(musicbrainz_id) - return a and a.get("id") + return a if a and a.get("id") else None # Only wait for auto-indexing if we just created/refreshed the artist; # for existing artists nothing triggered new indexing, so skip the long wait. diff --git a/backend/services/artist_discovery_service.py b/backend/services/artist_discovery_service.py index abaaeba..c3b9c99 100644 --- a/backend/services/artist_discovery_service.py +++ b/backend/services/artist_discovery_service.py @@ -25,6 +25,7 @@ CIRCUIT_OPEN_CACHE_TTL = 30 DEFAULT_SIMILAR_COUNT = 15 DEFAULT_TOP_SONGS_COUNT = 10 DEFAULT_TOP_ALBUMS_COUNT = 10 +_DISCOVERY_WORKER_TIMEOUT = 120 # Module-level flag survives singleton cache invalidation / instance recreation _discovery_precache_running = False @@ -409,6 +410,7 @@ class ArtistDiscoveryService: delay: float = 0.5, status_service: Any = None, mbid_to_name: dict[str, str] | None = None, + generation: int = 0, ) -> int: global _discovery_precache_running if _discovery_precache_running: @@ -420,6 +422,7 @@ class ArtistDiscoveryService: return await self._do_precache_artist_discovery( artist_mbids, delay=delay, status_service=status_service, mbid_to_name=mbid_to_name, + generation=generation, ) finally: _discovery_precache_running = False @@ -430,6 +433,7 @@ class ArtistDiscoveryService: delay: float = 0.5, status_service: Any = None, mbid_to_name: dict[str, str] | None = None, + generation: int = 0, ) -> int: sources: list[Literal["listenbrainz", "lastfm"]] = [] if self._lb_repo.is_configured(): @@ -447,10 +451,11 @@ class ArtistDiscoveryService: cached_count = 0 source_fetches = 0 advanced = self._preferences_service.get_advanced_settings() if self._preferences_service else None - discovery_concurrency = getattr(advanced, 'artist_discovery_precache_concurrency', 3) if advanced else 3 + discovery_concurrency = getattr(advanced, 'artist_discovery_precache_concurrency', 5) if advanced else 5 sem = asyncio.Semaphore(discovery_concurrency) counter_lock = asyncio.Lock() progress_counter = 0 + counted_workers: set[int] = set() async def process_artist(idx: int, mbid: str) -> bool: nonlocal cached_count, source_fetches, progress_counter @@ -493,18 +498,18 @@ class ArtistDiscoveryService: async with counter_lock: source_fetches += 1 - # Sleep inside semaphore to hold slot and throttle API calls - if delay > 0: - await asyncio.sleep(delay) + if delay > 0: + await asyncio.sleep(delay) async with counter_lock: cached_count += 1 progress_counter += 1 local_progress = progress_counter + counted_workers.add(idx) if status_service: artist_name = (mbid_to_name or {}).get(mbid, mbid[:8]) - await status_service.update_progress(local_progress, current_item=artist_name) + await status_service.update_progress(local_progress, current_item=artist_name, generation=generation) if local_progress % 10 == 0: logger.info("Discovery precache progress: %d/%d artists", local_progress, len(artist_mbids)) @@ -515,9 +520,31 @@ class ArtistDiscoveryService: async with counter_lock: progress_counter += 1 local_progress = progress_counter + counted_workers.add(idx) if status_service: artist_name = (mbid_to_name or {}).get(mbid, mbid[:8]) - await status_service.update_progress(local_progress, current_item=artist_name) + await status_service.update_progress(local_progress, current_item=artist_name, generation=generation) + return False + + async def process_artist_with_timeout(idx: int, mbid: str) -> bool: + nonlocal progress_counter + try: + return await asyncio.wait_for( + process_artist(idx, mbid), timeout=_DISCOVERY_WORKER_TIMEOUT + ) + except asyncio.TimeoutError: + logger.warning("Discovery timed out for %s after %ds", mbid[:8], _DISCOVERY_WORKER_TIMEOUT) + async with counter_lock: + if idx not in counted_workers: + progress_counter += 1 + counted_workers.add(idx) + local_progress = progress_counter + if status_service: + artist_name = (mbid_to_name or {}).get(mbid, mbid[:8]) + await status_service.update_progress( + local_progress, current_item=f"{artist_name} (timed out)", + generation=generation, + ) return False chunk = max(discovery_concurrency * 4, 20) @@ -526,7 +553,7 @@ class ArtistDiscoveryService: logger.info("Discovery precache cancelled by user") break batch = artist_mbids[i:i + chunk] - batch_tasks = [asyncio.create_task(process_artist(i + j, mbid)) for j, mbid in enumerate(batch)] + batch_tasks = [asyncio.create_task(process_artist_with_timeout(i + j, mbid)) for j, mbid in enumerate(batch)] if batch_tasks: await asyncio.gather(*batch_tasks, return_exceptions=True) @@ -655,29 +682,26 @@ class ArtistDiscoveryService: ) trimmed = lfm_albums[:count] - mbids_from_lastfm = [ - a.mbid.strip().lower() for a in trimmed if a.mbid and a.mbid.strip() - ] - rg_map = await self._resolve_release_groups(mbids_from_lastfm) if mbids_from_lastfm else {} + # Last.fm usually returns release-group MBIDs here, so keep them as-is + # and let the discover queue resolve the rare mismatches. albums = [] for a in trimmed: raw_mbid = a.mbid.strip().lower() if a.mbid and a.mbid.strip() else None - resolved_mbid = rg_map.get(raw_mbid, raw_mbid) if raw_mbid else None albums.append( TopAlbum( - release_group_mbid=resolved_mbid, + release_group_mbid=raw_mbid, title=a.name, artist_name=a.artist_name, listen_count=a.playcount, in_library=( - resolved_mbid in library_album_mbids - if resolved_mbid + raw_mbid in library_album_mbids + if raw_mbid else False ), requested=( - resolved_mbid in requested_album_mbids - if resolved_mbid + raw_mbid in requested_album_mbids + if raw_mbid else False ), ) diff --git a/backend/services/artist_service.py b/backend/services/artist_service.py index 3afc0e7..8e72598 100644 --- a/backend/services/artist_service.py +++ b/backend/services/artist_service.py @@ -430,6 +430,7 @@ class ArtistService: artist_info = await self._apply_audiodb_artist_images( artist_info, artist_id, artist_info.name, allow_fetch=False, ) + await self._refresh_library_flags(artist_info) await self._save_artist_to_cache(artist_id, artist_info) if not future.done(): future.set_result(artist_info) @@ -457,7 +458,21 @@ class ArtistService: continue rg.in_library = rg_id in library_mbids rg.requested = rg_id in requested_mbids and not rg.in_library - artist_info.in_library = artist_info.musicbrainz_id.lower() in artist_mbids + mbid_lower = artist_info.musicbrainz_id.lower() + is_in_artist_mbids = mbid_lower in artist_mbids + artist_info.in_library = is_in_artist_mbids + if is_in_artist_mbids: + try: + lidarr_artist = await self._lidarr_repo.get_artist_details(artist_info.musicbrainz_id) + if lidarr_artist is not None: + artist_info.in_lidarr = True + artist_info.monitored = lidarr_artist.get("monitored", False) + artist_info.auto_download = lidarr_artist.get("monitor_new_items", "none") == "all" + elif not artist_info.in_lidarr: + artist_info.in_lidarr = True + except Exception: # noqa: BLE001 + if not artist_info.in_lidarr: + artist_info.in_lidarr = True except Exception as e: # noqa: BLE001 logger.warning(f"Failed to refresh library flags: {e}") diff --git a/backend/services/artist_utils.py b/backend/services/artist_utils.py index 4aa9552..b8d9713 100644 --- a/backend/services/artist_utils.py +++ b/backend/services/artist_utils.py @@ -54,7 +54,7 @@ def detect_platform(url: str, rel_type: str) -> tuple[str, str]: def extract_tags(mb_artist: dict[str, Any], limit: int = 10) -> list[str]: tags = [] if mb_tags := mb_artist.get("tags", []): - tags = [tag.get("name") for tag in mb_tags if tag.get("name")][:limit] + tags = list(dict.fromkeys(tag.get("name") for tag in mb_tags if tag.get("name")))[:limit] return tags diff --git a/backend/services/cache_status_service.py b/backend/services/cache_status_service.py index 64cde3a..ecff812 100644 --- a/backend/services/cache_status_service.py +++ b/backend/services/cache_status_service.py @@ -29,7 +29,7 @@ class CacheSyncProgress(msgspec.Struct): def progress_percent(self) -> int: if self.total_items == 0: return 0 - return int((self.processed_items / self.total_items) * 100) + return min(100, int((self.processed_items / self.total_items) * 100)) class CacheStatusService: @@ -48,6 +48,7 @@ class CacheStatusService: def _initialize(self, sync_state_store: Optional['SyncStateStore'] = None): self._sync_state_store = sync_state_store + self._sync_generation: int = 0 self._progress = CacheSyncProgress( is_syncing=False, phase=None, @@ -115,8 +116,10 @@ class CacheStatusService: for q in dead_queues: self._sse_subscribers.discard(q) - async def start_sync(self, phase: str, total_items: int, total_artists: int = 0, total_albums: int = 0): + async def start_sync(self, phase: str, total_items: int, total_artists: int = 0, total_albums: int = 0) -> int: async with self._state_lock: + self._sync_generation += 1 + generation = self._sync_generation self._cancel_event.clear() self._last_persist_time = 0.0 self._last_broadcast_time = 0.0 @@ -151,6 +154,7 @@ class CacheStatusService: logger.warning(f"Failed to persist sync state: {e}") await self.broadcast_progress() + return generation _BROADCAST_THROTTLE_SECONDS = 0.3 @@ -159,9 +163,12 @@ class CacheStatusService: processed: int, current_item: Optional[str] = None, processed_artists: Optional[int] = None, - processed_albums: Optional[int] = None + processed_albums: Optional[int] = None, + generation: int = 0, ): async with self._state_lock: + if generation and generation != self._sync_generation: + return if processed >= self._progress.processed_items: self._progress.processed_items = processed self._progress.current_item = current_item @@ -177,8 +184,10 @@ class CacheStatusService: self._last_broadcast_time = now await self.broadcast_progress() - async def update_phase(self, phase: str, total_items: int): + async def update_phase(self, phase: str, total_items: int, generation: int = 0): async with self._state_lock: + if generation and generation != self._sync_generation: + return self._progress.phase = phase self._progress.total_items = total_items self._progress.processed_items = 0 @@ -201,9 +210,11 @@ class CacheStatusService: await self.broadcast_progress() - async def skip_phase(self, phase: str): + async def skip_phase(self, phase: str, generation: int = 0): """Broadcast a phase with 0 items so the frontend sees it as skipped.""" async with self._state_lock: + if generation and generation != self._sync_generation: + return self._progress.phase = phase self._progress.total_items = 0 self._progress.processed_items = 0 @@ -218,11 +229,13 @@ class CacheStatusService: _PERSIST_INTERVAL_SECONDS = 5.0 _PERSIST_ITEM_INTERVAL = 10 - async def persist_progress(self, force: bool = False): + async def persist_progress(self, force: bool = False, generation: int = 0): if not self._progress.is_syncing: return if self.is_cancelled(): return + if generation and generation != self._sync_generation: + return self._persist_item_counter += 1 now = time.time() @@ -249,10 +262,12 @@ class CacheStatusService: except Exception as e: # noqa: BLE001 logger.warning(f"Failed to persist progress: {e}") - async def complete_sync(self, error_message: Optional[str] = None): + async def complete_sync(self, error_message: Optional[str] = None, generation: int = 0): async with self._state_lock: if not self._progress.is_syncing: return + if generation and generation != self._sync_generation: + return is_success = error_message is None status = 'completed' if is_success else 'failed' logger.info(f"Cache sync {status}: {self._progress.phase}") @@ -299,37 +314,36 @@ class CacheStatusService: async def cancel_current_sync(self): async with self._state_lock: - if self._progress.is_syncing: - logger.warning(f"Cancelling in-progress sync: phase={self._progress.phase}, progress={self._progress.processed_items}/{self._progress.total_items}") - self._cancel_event.set() + logger.warning(f"Cancelling sync: phase={self._progress.phase}, progress={self._progress.processed_items}/{self._progress.total_items}") + self._cancel_event.set() - if self._sync_state_store: - try: - await self._sync_state_store.save_sync_state( - status='cancelled', - phase=self._progress.phase, - total_artists=self._progress.total_artists, - processed_artists=self._progress.processed_artists, - total_albums=self._progress.total_albums, - processed_albums=self._progress.processed_albums, - started_at=self._progress.started_at - ) - except Exception as e: # noqa: BLE001 - logger.warning(f"Failed to persist cancellation: {e}") + if self._sync_state_store and self._progress.is_syncing: + try: + await self._sync_state_store.save_sync_state( + status='cancelled', + phase=self._progress.phase, + total_artists=self._progress.total_artists, + processed_artists=self._progress.processed_artists, + total_albums=self._progress.total_albums, + processed_albums=self._progress.processed_albums, + started_at=self._progress.started_at + ) + except Exception as e: # noqa: BLE001 + logger.warning(f"Failed to persist cancellation: {e}") - self._progress = CacheSyncProgress( - is_syncing=False, - phase=None, - total_items=0, - processed_items=0, - current_item=None, - started_at=None, - error_message=None, - total_artists=0, - processed_artists=0, - total_albums=0, - processed_albums=0 - ) + self._progress = CacheSyncProgress( + is_syncing=False, + phase=None, + total_items=0, + processed_items=0, + current_item=None, + started_at=None, + error_message=None, + total_artists=0, + processed_artists=0, + total_albums=0, + processed_albums=0 + ) await self.broadcast_progress() @@ -343,9 +357,9 @@ class CacheStatusService: task = self._current_task if task and not task.done(): try: - await asyncio.wait_for(task, timeout=5.0) + await asyncio.wait_for(task, timeout=30.0) except asyncio.TimeoutError: - logger.warning("Sync task did not complete within timeout, forcing cancellation") + logger.warning("Sync task did not complete within 30s timeout, forcing cancellation") if not task.done(): task.cancel() except Exception as e: # noqa: BLE001 @@ -365,11 +379,22 @@ class CacheStatusService: f"artists={state.get('processed_artists')}/{state.get('total_artists')}, " f"albums={state.get('processed_albums')}/{state.get('total_albums')}") + phase = state.get('phase') + if phase == 'albums': + total_items = state.get('total_albums') + processed_items = state.get('processed_albums') + elif phase == 'audiodb_prewarm': + total_items = 0 + processed_items = 0 + else: + total_items = state.get('total_artists') + processed_items = state.get('processed_artists') + self._progress = CacheSyncProgress( is_syncing=True, - phase=state.get('phase'), - total_items=state.get('total_albums') if state.get('phase') == 'albums' else state.get('total_artists'), - processed_items=state.get('processed_albums') if state.get('phase') == 'albums' else state.get('processed_artists'), + phase=phase, + total_items=total_items, + processed_items=processed_items, current_item=state.get('current_item'), started_at=state.get('started_at'), error_message=None, diff --git a/backend/services/library_service.py b/backend/services/library_service.py index 7d44b80..1266cf3 100644 --- a/backend/services/library_service.py +++ b/backend/services/library_service.py @@ -417,10 +417,23 @@ class LibraryService: exc = t.exception() if exc: logger.error(f"Precache task failed: {exc}") + task_success = False + else: + task_success = not t.cancelled() except asyncio.CancelledError: logger.info("Precache task was cancelled") + task_success = False finally: status_service.set_current_task(None) + try: + lidarr_settings = self._preferences_service.get_lidarr_settings() + if sync_started_at >= (lidarr_settings.last_sync or 0): + updated = clone_with_updates(lidarr_settings, { + 'last_sync_success': task_success, + }) + self._preferences_service.save_lidarr_settings(updated) + except Exception as e: # noqa: BLE001 + logger.warning(f"Failed to update last_sync_success: {e}") task.add_done_callback(on_task_done) status_service.set_current_task(task) @@ -428,6 +441,7 @@ class LibraryService: logger.info(f"Library sync complete: {len(artists)} artists, {len(albums)} albums") self._update_last_sync_timestamp() + sync_started_at = self._preferences_service.get_lidarr_settings().last_sync or 0 result = SyncLibraryResponse( status='success', diff --git a/backend/services/precache/album_phase.py b/backend/services/precache/album_phase.py index d0fad25..978c9a1 100644 --- a/backend/services/precache/album_phase.py +++ b/backend/services/precache/album_phase.py @@ -33,6 +33,7 @@ class AlbumPhase: status_service: CacheStatusService, library_album_mbids: dict[str, Any] = None, offset: int = 0, + generation: int = 0, ) -> None: from core.dependencies import get_album_service logger.info(f"Pre-caching {len(release_group_ids)} new/missing release-groups") @@ -47,11 +48,11 @@ class AlbumPhase: cache_key = f"{ALBUM_INFO_PREFIX}{rgid}" cached_info = await album_service._cache.get(cache_key) if not cached_info: - await status_service.update_progress(index + 1, f"Fetching metadata for {rgid[:8]}...", processed_albums=offset + index + 1) + await status_service.update_progress(index + 1, f"Fetching metadata for {rgid[:8]}...", processed_albums=offset + index + 1, generation=generation) await album_service.get_album_info(rgid, monitored_mbids=monitored_mbids) metadata_fetched = True else: - await status_service.update_progress(index + 1, f"Cached: {rgid[:8]}...", processed_albums=offset + index + 1) + await status_service.update_progress(index + 1, f"Cached: {rgid[:8]}...", processed_albums=offset + index + 1, generation=generation) if rgid.lower() in monitored_mbids: cache_filename = get_cache_filename(f"rg_{rgid}", "500") file_path = self._cover_repo.cache_dir / f"{cache_filename}.bin" @@ -73,7 +74,8 @@ class AlbumPhase: metadata_fetched = 0 covers_fetched = 0 consecutive_slow_batches = 0 - for i in range(0, len(release_group_ids), batch_size): + i = 0 + while i < len(release_group_ids): if status_service.is_cancelled(): logger.info("Album pre-caching cancelled by user") break @@ -98,7 +100,7 @@ class AlbumPhase: processed_mbids.append(rgid) if processed_mbids: await self._sync_state_store.mark_items_processed_batch('album', processed_mbids) - await status_service.persist_progress() + await status_service.persist_progress(generation=generation) batch_duration = time.time() - batch_start avg_time_per_item = batch_duration / len(batch) if batch else 1.0 if avg_time_per_item > 1.5: @@ -114,9 +116,11 @@ class AlbumPhase: if avg_time_per_item < 0.8 and batch_size < max_batch: batch_size = min(batch_size + 1, max_batch) logger.debug(f"Increasing batch size to {batch_size} (fast: {avg_time_per_item:.2f}s/item)") - if (i + batch_size) % 30 == 0 or (i + batch_size) >= len(release_group_ids): - percent = int((min(i + batch_size, len(release_group_ids)) / len(release_group_ids)) * 100) - logger.info(f"Album progress: {min(i + batch_size, len(release_group_ids))}/{len(release_group_ids)} ({percent}%) - metadata: {metadata_fetched}, covers: {covers_fetched} [batch: {batch_size}]") + next_i = i + len(batch) + if next_i % 30 == 0 or next_i >= len(release_group_ids): + percent = int((min(next_i, len(release_group_ids)) / len(release_group_ids)) * 100) + logger.info(f"Album progress: {min(next_i, len(release_group_ids))}/{len(release_group_ids)} ({percent}%) - metadata: {metadata_fetched}, covers: {covers_fetched} [batch: {batch_size}]") + i = next_i await asyncio.sleep(advanced_settings.delay_albums) - await status_service.persist_progress(force=True) + await status_service.persist_progress(force=True, generation=generation) logger.info(f"Album pre-caching complete: metadata fetched={metadata_fetched}, covers fetched={covers_fetched}, total processed={len(release_group_ids)}") diff --git a/backend/services/precache/artist_phase.py b/backend/services/precache/artist_phase.py index af2d002..f26eed1 100644 --- a/backend/services/precache/artist_phase.py +++ b/backend/services/precache/artist_phase.py @@ -36,18 +36,32 @@ class ArtistPhase: library_artist_mbids: set[str] = None, library_album_mbids: dict[str, Any] = None, offset: int = 0, + generation: int = 0, ) -> None: logger.info(f"Pre-caching metadata+images for {len(artists)} artists") from core.dependencies import get_artist_service from infrastructure.validators import is_unknown_mbid artist_service = get_artist_service() + seen_mbids: set[str] = set() + unique_artists: list[dict] = [] + for a in artists: + mbid = a.get('mbid') + if not mbid or is_unknown_mbid(mbid): + unique_artists.append(a) + elif mbid.lower() not in seen_mbids: + seen_mbids.add(mbid.lower()) + unique_artists.append(a) + if len(unique_artists) < len(artists): + logger.info("Deduplicated %d artists to %d unique", len(artists), len(unique_artists)) + artists = unique_artists + async def cache_artist(artist: dict, index: int) -> str: mbid = artist.get('mbid') try: artist_name = artist.get('name', 'Unknown') if is_unknown_mbid(mbid): - await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1) + await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1, generation=generation) return mbid artist_cache_key = f"{ARTIST_INFO_PREFIX}{mbid}" cached_artist = await artist_service._cache.get(artist_cache_key) @@ -64,18 +78,18 @@ class ArtistPhase: file_path_500 = self._cover_repo.cache_dir / f"{cache_filename_500}.bin" if file_path_250.exists() and file_path_500.exists(): logger.debug(f"Artist images for {artist_name} already cached, skipping") - await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1) + await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1, generation=generation) return mbid - await status_service.update_progress(index + 1, f"Fetching images for {artist_name}", processed_artists=offset + index + 1) + await status_service.update_progress(index + 1, f"Fetching images for {artist_name}", processed_artists=offset + index + 1, generation=generation) if not file_path_250.exists(): await self._cover_repo.get_artist_image(mbid, size=250) if not file_path_500.exists(): await self._cover_repo.get_artist_image(mbid, size=500) - await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1) + await status_service.update_progress(index + 1, artist_name, processed_artists=offset + index + 1, generation=generation) return mbid except Exception as e: # noqa: BLE001 logger.warning(f"Failed to cache artist {artist.get('name')} (mbid: {mbid}): {e}", exc_info=True) - await status_service.update_progress(index + 1, f"Failed: {artist.get('name', 'Unknown')}", processed_artists=offset + index + 1) + await status_service.update_progress(index + 1, f"Failed: {artist.get('name', 'Unknown')}", processed_artists=offset + index + 1, generation=generation) return mbid advanced_settings = self._preferences_service.get_advanced_settings() @@ -97,9 +111,9 @@ class ArtistPhase: processed_mbids.append(result) if processed_mbids: await self._sync_state_store.mark_items_processed_batch('artist', processed_mbids) - await status_service.persist_progress() + await status_service.persist_progress(generation=generation) await asyncio.sleep(advanced_settings.delay_artist) - await status_service.persist_progress(force=True) + await status_service.persist_progress(force=True, generation=generation) logger.info("Artist metadata+image pre-caching complete") await self._cache_artist_genres(artists) diff --git a/backend/services/precache/audiodb_phase.py b/backend/services/precache/audiodb_phase.py index d26cd61..7d03d8a 100644 --- a/backend/services/precache/audiodb_phase.py +++ b/backend/services/precache/audiodb_phase.py @@ -179,15 +179,16 @@ class AudioDBPhase: artists: list[dict], albums: list[Any], status_service: CacheStatusService, + generation: int = 0, ) -> None: if self._audiodb_image_service is None: - await status_service.skip_phase('audiodb_prewarm') + await status_service.skip_phase('audiodb_prewarm', generation=generation) return settings = self._preferences_service.get_advanced_settings() if not settings.audiodb_enabled: logger.info("AudioDB pre-warming skipped (audiodb_enabled=false)") - await status_service.skip_phase('audiodb_prewarm') + await status_service.skip_phase('audiodb_prewarm', generation=generation) return concurrency = settings.audiodb_prewarm_concurrency @@ -197,7 +198,7 @@ class AudioDBPhase: total = len(needed_artists) + len(needed_albums) if total == 0: logger.info("AudioDB prewarm: all items already cached") - await status_service.skip_phase('audiodb_prewarm') + await status_service.skip_phase('audiodb_prewarm', generation=generation) return original_total = len(artists) + len(albums) @@ -206,7 +207,7 @@ class AudioDBPhase: "Phase 5 (AudioDB): Pre-warming %d items (%d artists, %d albums), %.0f%% already cached, concurrency=%d delay=%.1fs", total, len(needed_artists), len(needed_albums), initial_hit_rate, concurrency, inter_item_delay, ) - await status_service.update_phase('audiodb_prewarm', total) + await status_service.update_phase('audiodb_prewarm', total, generation=generation) needed_artists = self.sort_by_cover_priority(needed_artists, "artist") needed_albums = self.sort_by_cover_priority(needed_albums, "album") @@ -249,7 +250,7 @@ class AudioDBPhase: processed += 1 local_processed = processed snap_ok, snap_fail = bytes_ok, bytes_fail - await status_service.update_progress(local_processed, f"AudioDB: {name}") + await status_service.update_progress(local_processed, f"AudioDB: {name}", generation=generation) if local_processed % _AUDIODB_PREWARM_LOG_INTERVAL == 0: logger.info( @@ -291,7 +292,7 @@ class AudioDBPhase: processed += 1 local_processed = processed snap_ok, snap_fail = bytes_ok, bytes_fail - await status_service.update_progress(local_processed, f"AudioDB: {album_name or 'Unknown'}") + await status_service.update_progress(local_processed, f"AudioDB: {album_name or 'Unknown'}", generation=generation) if local_processed % _AUDIODB_PREWARM_LOG_INTERVAL == 0: logger.info( diff --git a/backend/services/precache/orchestrator.py b/backend/services/precache/orchestrator.py index da0a731..b44dca0 100644 --- a/backend/services/precache/orchestrator.py +++ b/backend/services/precache/orchestrator.py @@ -98,9 +98,13 @@ class LibraryPrecacheService: if not task.done(): task.cancel() try: - await task - except (asyncio.CancelledError, Exception): - pass + await asyncio.wait_for(asyncio.shield(task), timeout=15) + except asyncio.CancelledError: + if asyncio.current_task().cancelling() > 0: + raise # outer task cancelled; propagate + # inner task exited cleanly after cancel + except (asyncio.TimeoutError, Exception): + logger.warning("Precache task did not exit within 15s of cancel") await status_service.complete_sync(str(exc)) raise ExternalServiceError(str(exc)) @@ -141,6 +145,7 @@ class LibraryPrecacheService: async def _do_precache(self, artists: list[dict], albums: list[Any], status_service: CacheStatusService, resume: bool = False) -> None: from core.dependencies import get_album_service + generation = 0 try: processed_artists: set[str] = set() processed_albums: set[str] = set() @@ -152,9 +157,9 @@ class LibraryPrecacheService: processed_albums = await self._sync_state_store.get_processed_items('album') state = await self._sync_state_store.get_sync_state() - if state and state.get('phase') == 'albums': + if state and state.get('phase') in ('albums', 'audiodb_prewarm'): skip_artists = True - logger.info(f"Resuming from albums phase, {len(processed_albums)} albums already processed") + logger.info(f"Resuming from {state.get('phase')} phase, {len(processed_albums)} albums already processed") else: logger.info(f"Resuming from artists phase, {len(processed_artists)} artists already processed") @@ -172,23 +177,26 @@ class LibraryPrecacheService: remaining_artists = [a for a in artists if a.get('mbid') not in processed_artists] logger.info(f"Phase 1: Caching {len(remaining_artists)} artist metadata + images ({len(processed_artists)} already done)") if remaining_artists: - await status_service.start_sync('artists', len(remaining_artists), total_artists=total_artists, total_albums=total_albums) - await self._artist_phase.precache_artist_images(remaining_artists, status_service, library_artist_mbids, library_album_mbids, len(processed_artists)) + generation = await status_service.start_sync('artists', len(remaining_artists), total_artists=total_artists, total_albums=total_albums) + await self._artist_phase.precache_artist_images(remaining_artists, status_service, library_artist_mbids, library_album_mbids, len(processed_artists), generation=generation) else: - await status_service.start_sync('artists', 0, total_artists=total_artists, total_albums=total_albums) - await status_service.skip_phase('artists') + generation = await status_service.start_sync('artists', 0, total_artists=total_artists, total_albums=total_albums) + await status_service.skip_phase('artists', generation=generation) + else: + generation = await status_service.start_sync('albums', 0, total_artists=total_artists, total_albums=total_albums) + logger.info("Resuming sync, skipping artists/discovery phases") if status_service.is_cancelled(): logger.info("Pre-cache cancelled after Phase 1") return if self._artist_discovery_service and not skip_artists: - artist_mbids = [ + artist_mbids = list(dict.fromkeys( a.get('mbid') for a in artists if a.get('mbid') and not a.get('mbid', '').startswith('unknown_') - ] + )) if artist_mbids: logger.info(f"Phase 1.5: Pre-caching discovery data (popular albums/songs/similar) for {len(artist_mbids)} library artists") - await status_service.update_phase('discovery', len(artist_mbids)) + await status_service.update_phase('discovery', len(artist_mbids), generation=generation) mbid_to_name = { a.get('mbid'): a.get('name', a.get('mbid', '')[:8]) for a in artists if a.get('mbid') @@ -199,13 +207,14 @@ class LibraryPrecacheService: await self._artist_discovery_service.precache_artist_discovery( artist_mbids, delay=precache_delay, status_service=status_service, mbid_to_name=mbid_to_name, + generation=generation, ) except Exception as e: # noqa: BLE001 logger.warning(f"Discovery precache failed (non-fatal): {e}") else: - await status_service.skip_phase('discovery') + await status_service.skip_phase('discovery', generation=generation) elif not skip_artists: - await status_service.skip_phase('discovery') + await status_service.skip_phase('discovery', generation=generation) if status_service.is_cancelled(): logger.info("Pre-cache cancelled after Phase 1.5") @@ -253,27 +262,25 @@ class LibraryPrecacheService: f"{already_cached} already cached, {len(processed_albums)} from previous run" ) if items_to_process: - await status_service.update_phase('albums', len(items_to_process)) - await self._album_phase.precache_album_data(items_to_process, monitored_mbids, status_service, library_album_mbids, len(processed_albums)) + await status_service.update_phase('albums', len(items_to_process), generation=generation) + await self._album_phase.precache_album_data(items_to_process, monitored_mbids, status_service, library_album_mbids, len(processed_albums), generation=generation) else: - await status_service.skip_phase('albums') + await status_service.skip_phase('albums', generation=generation) + + if status_service.is_cancelled(): + logger.info("Pre-cache cancelled after albums phase") + return + + try: + logger.info("Starting AudioDB image prewarm...") + await self._audiodb_phase.precache_audiodb_data(artists, albums, status_service, generation=generation) + logger.info("AudioDB image prewarm complete") + except Exception as e: # noqa: BLE001 + logger.warning(f"AudioDB pre-warming failed (non-fatal): {e}") if not status_service.is_cancelled(): - await status_service.complete_sync() - logger.info("Library resource pre-caching complete (core phases done)") - - try: - audiodb_timeout = self._preferences_service.get_advanced_settings().sync_max_timeout_hours * 3600 - logger.info("Starting AudioDB image prewarm as background enhancement...") - await asyncio.wait_for( - self._audiodb_phase.precache_audiodb_data(artists, albums, status_service), - timeout=audiodb_timeout, - ) - logger.info("AudioDB image prewarm complete") - except asyncio.TimeoutError: - logger.warning("AudioDB pre-warming timed out (non-fatal)") - except Exception as e: # noqa: BLE001 - logger.warning(f"AudioDB pre-warming failed (non-fatal): {e}") + await status_service.complete_sync(generation=generation) + logger.info("Library resource pre-caching complete") else: logger.info("Library resource pre-caching complete (cancelled)") except Exception as e: @@ -281,4 +288,4 @@ class LibraryPrecacheService: raise finally: if status_service.is_syncing(): - await status_service.complete_sync() + await status_service.complete_sync(generation=generation) diff --git a/backend/tests/infrastructure/test_dedup_cancellation.py b/backend/tests/infrastructure/test_dedup_cancellation.py new file mode 100644 index 0000000..51f6d50 --- /dev/null +++ b/backend/tests/infrastructure/test_dedup_cancellation.py @@ -0,0 +1,181 @@ +import asyncio +import pytest + +from infrastructure.http.deduplication import RequestDeduplicator + + +@pytest.mark.anyio +async def test_follower_task_cancellation_propagates(): + """When a follower's own task is cancelled, CancelledError must propagate.""" + dedup = RequestDeduplicator() + leader_started = asyncio.Event() + leader_release = asyncio.Event() + + async def slow_coro(): + leader_started.set() + await leader_release.wait() + return "result" + + async def run_follower(): + await leader_started.wait() + return await dedup.dedupe("key", slow_coro) + + leader_task = asyncio.create_task(dedup.dedupe("key", slow_coro)) + await asyncio.sleep(0) + follower_task = asyncio.create_task(run_follower()) + await asyncio.sleep(0) + + follower_task.cancel() + with pytest.raises(asyncio.CancelledError): + await follower_task + + leader_release.set() + result = await leader_task + assert result == "result" + + +@pytest.mark.anyio +async def test_leader_exception_propagates_to_follower(): + """When the leader raises, the follower receives the same exception.""" + dedup = RequestDeduplicator() + leader_started = asyncio.Event() + + async def failing_coro(): + leader_started.set() + await asyncio.sleep(0) + raise ValueError("boom") + + async def run_follower(): + await leader_started.wait() + return await dedup.dedupe("key", failing_coro) + + leader_task = asyncio.create_task(dedup.dedupe("key", failing_coro)) + await asyncio.sleep(0) + follower_task = asyncio.create_task(run_follower()) + + with pytest.raises(ValueError, match="boom"): + await leader_task + + with pytest.raises(ValueError, match="boom"): + await follower_task + + +@pytest.mark.anyio +async def test_leader_cancellation_follower_retries_as_leader(): + """When leader is cancelled, follower retries as new leader (bounded retry).""" + dedup = RequestDeduplicator() + leader_started = asyncio.Event() + call_count = 0 + + async def coro(): + nonlocal call_count + call_count += 1 + if call_count == 1: + leader_started.set() + await asyncio.sleep(60) + return "never" + return "retried" + + async def run_follower(): + await leader_started.wait() + return await dedup.dedupe("key", coro) + + leader_task = asyncio.create_task(dedup.dedupe("key", coro)) + await asyncio.sleep(0) + follower_task = asyncio.create_task(run_follower()) + await asyncio.sleep(0) + + leader_task.cancel() + + with pytest.raises(asyncio.CancelledError): + await leader_task + + result = await follower_task + assert result == "retried" + assert call_count == 2 + + +@pytest.mark.anyio +async def test_concurrent_followers_coalesce(): + """Multiple followers all receive the same result from one leader execution.""" + dedup = RequestDeduplicator() + call_count = 0 + leader_started = asyncio.Event() + release_leader = asyncio.Event() + + async def counted_coro(): + nonlocal call_count + call_count += 1 + leader_started.set() + await release_leader.wait() + return "shared-result" + + async def run_follower(): + await leader_started.wait() + return await dedup.dedupe("key", counted_coro) + + leader_task = asyncio.create_task(dedup.dedupe("key", counted_coro)) + await asyncio.sleep(0) + + followers = [asyncio.create_task(run_follower()) for _ in range(5)] + # Let all followers register as waiters on the shared future + for _ in range(10): + await asyncio.sleep(0) + + release_leader.set() + + results = await asyncio.gather(leader_task, *followers) + assert all(r == "shared-result" for r in results) + assert call_count == 1 + + +@pytest.mark.anyio +async def test_disconnect_leader_follower_retries_as_leader(): + """When leader disconnects, one follower retries as the new leader.""" + from core.exceptions import ClientDisconnectedError + + dedup = RequestDeduplicator() + follower_registered = asyncio.Event() + expected_result = ("image-bytes", "image/png", "source") + leader_error = None + + async def leader_coro(): + await follower_registered.wait() + raise ClientDisconnectedError("leader disconnected") + + async def run_leader(): + nonlocal leader_error + try: + await dedup.dedupe("key1", leader_coro) + except ClientDisconnectedError as e: + leader_error = e + + async def follower_coro(): + return expected_result + + async def run_follower(): + await asyncio.sleep(0) + follower_registered.set() + return await dedup.dedupe("key1", follower_coro) + + leader_task = asyncio.create_task(run_leader()) + await asyncio.sleep(0) + follower_task = asyncio.create_task(run_follower()) + + await asyncio.gather(leader_task, follower_task) + + assert isinstance(leader_error, ClientDisconnectedError) + assert follower_task.result() == expected_result + + +@pytest.mark.anyio +async def test_key_cleanup_after_completion(): + """After dedupe completes, the key is removed from _pending.""" + dedup = RequestDeduplicator() + + async def simple_coro(): + return 42 + + result = await dedup.dedupe("key", simple_coro) + assert result == 42 + assert "key" not in dedup._pending diff --git a/backend/tests/repositories/test_album_is_indexed.py b/backend/tests/repositories/test_album_is_indexed.py new file mode 100644 index 0000000..a5716b8 --- /dev/null +++ b/backend/tests/repositories/test_album_is_indexed.py @@ -0,0 +1,126 @@ +"""Test that album_is_indexed returns a dict, not an int. + +Regression test for: 'argument of type int is not iterable' when +album_is_indexed returned a.get("id") (int) instead of the album dict. +""" +import pytest +from unittest.mock import AsyncMock, MagicMock, patch + +from repositories.lidarr.album import LidarrAlbumRepository + + +@pytest.fixture +def album_repo(): + settings = MagicMock() + settings.lidarr_url = "http://lidarr:8686" + settings.lidarr_api_key = "test-key" + settings.quality_profile_id = 1 + cache = AsyncMock() + cache.get.return_value = None + cache.set.return_value = None + cache.delete.return_value = None + cache.clear_prefix.return_value = 0 + http_client = AsyncMock() + return LidarrAlbumRepository(settings=settings, http_client=http_client, cache=cache) + + +class TestAlbumIsIndexedReturnType: + """The album_is_indexed closure must return a dict (or None), never an int.""" + + @pytest.mark.asyncio + async def test_add_album_new_album_existing_artist_no_type_error(self, album_repo): + """When album needs POST-adding for an existing artist, album_obj must be a dict. + + Before the fix, album_is_indexed returned a.get("id") (int), + causing 'argument of type int is not iterable' at the 'id not in album_obj' check. + """ + artist_repo = AsyncMock() + artist_repo._ensure_artist_exists.return_value = ( + {"id": 42, "artistName": "MCR", "foreignArtistId": "artist-1", + "qualityProfileId": 1, "metadataProfileId": 1, "rootFolderPath": "/music"}, + False, # artist NOT created (already existed) + ) + + album_dict = { + "id": 99, + "title": "Greatest Hits", + "foreignAlbumId": "ae700a64-0890-457e-9440-51cdb06d58e1", + "monitored": True, + "statistics": {"trackFileCount": 0}, + "artist": {"foreignArtistId": "artist-1", "artistName": "MCR"}, + } + + lookup_response = [{ + "foreignAlbumId": "ae700a64-0890-457e-9440-51cdb06d58e1", + "title": "Greatest Hits", + "albumType": "Album", + "secondaryTypes": [], + "artist": {"mbId": "artist-1", "foreignArtistId": "artist-1", "artistName": "MCR"}, + }] + + album_repo._get = AsyncMock(side_effect=[ + lookup_response, # album/lookup + [{"id": 42}], # /api/v1/artist (existing artist check) + [album_dict], # /api/v1/album?artistId=42 (pre_add_monitored_ids) + [{"id": 1}], # /api/v1/qualityprofile + album_dict, # POST /api/v1/album response (via _post) + [album_dict], # /api/v1/album?artistId=42 (unmonitor check) + ]) + + # First call: not found. Second call onward: found (after POST). + album_repo._get_album_by_foreign_id = AsyncMock(side_effect=[ + None, # initial check — album not in Lidarr yet + album_dict, # after POST — album now indexed + album_dict, # final fetch + ]) + album_repo._post = AsyncMock(return_value=album_dict) + album_repo._put = AsyncMock(return_value=album_dict) + + result = await album_repo.add_album( + "ae700a64-0890-457e-9440-51cdb06d58e1", artist_repo + ) + + assert isinstance(result, dict) + assert "payload" in result + payload = result["payload"] + assert isinstance(payload, dict), ( + f"payload should be dict, got {type(payload).__name__}. " + "This was the original bug — album_is_indexed returned int." + ) + assert payload.get("id") == 99 + + @pytest.mark.asyncio + async def test_add_album_existing_album_no_regression(self, album_repo): + """Existing monitored+downloaded album returns immediately (no type error).""" + artist_repo = AsyncMock() + artist_repo._ensure_artist_exists.return_value = ( + {"id": 42, "artistName": "MCR", "foreignArtistId": "artist-1", + "qualityProfileId": 1, "metadataProfileId": 1, "rootFolderPath": "/music"}, + False, + ) + + album_dict = { + "id": 50, + "title": "Three Cheers", + "foreignAlbumId": "bbbb-cccc", + "monitored": True, + "statistics": {"trackFileCount": 12}, + "artist": {"foreignArtistId": "artist-1"}, + } + + album_repo._get = AsyncMock(side_effect=[ + [{"foreignAlbumId": "bbbb-cccc", "title": "Three Cheers", "albumType": "Album", + "secondaryTypes": [], + "artist": {"mbId": "artist-1", "foreignArtistId": "artist-1", "artistName": "MCR"}}], + [{"id": 42}], # existing artist + [album_dict], # albums_before for pre_add_monitored_ids + ]) + album_repo._get_album_by_foreign_id = AsyncMock(return_value=album_dict) + album_repo._put = AsyncMock() + album_repo._post = AsyncMock() + + result = await album_repo.add_album("bbbb-cccc", artist_repo) + + assert isinstance(result, dict) + assert "already downloaded" in result["message"] + assert isinstance(result["payload"], dict) diff --git a/backend/tests/routes/test_artist_basic_route.py b/backend/tests/routes/test_artist_basic_route.py new file mode 100644 index 0000000..1066afc --- /dev/null +++ b/backend/tests/routes/test_artist_basic_route.py @@ -0,0 +1,95 @@ +import os +import tempfile + +os.environ.setdefault("ROOT_APP_DIR", tempfile.mkdtemp()) + +import pytest +from unittest.mock import AsyncMock + +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from api.v1.routes.artists import router +from core.dependencies import get_artist_service, get_artist_discovery_service, get_artist_enrichment_service +from models.artist import ArtistInfo, ReleaseItem + + +VALID_MBID = "f4a31f0a-51dd-4fa7-986d-3095c40c5ed9" + + +def _minimal_artist_info(mbid: str = VALID_MBID) -> ArtistInfo: + return ArtistInfo( + name="Test Artist", + musicbrainz_id=mbid, + albums=[ReleaseItem(id="rg-1", title="Album One", type="Album", year=2024)], + singles=[], + eps=[], + release_group_count=1, + in_library=False, + ) + + +@pytest.fixture +def mock_artist_service(): + mock = AsyncMock() + mock.get_artist_info_basic = AsyncMock(return_value=_minimal_artist_info()) + mock.get_artist_info = AsyncMock(side_effect=AssertionError( + "get_artist_info should NOT be called — route must use get_artist_info_basic" + )) + mock.get_artist_releases = AsyncMock() + mock.get_artist_extended_info = AsyncMock() + return mock + + +@pytest.fixture +def mock_discovery_service(): + return AsyncMock() + + +@pytest.fixture +def mock_enrichment_service(): + return AsyncMock() + + +@pytest.fixture +def client(mock_artist_service, mock_discovery_service, mock_enrichment_service): + app = FastAPI() + app.include_router(router, prefix="/api/v1") + app.dependency_overrides[get_artist_service] = lambda: mock_artist_service + app.dependency_overrides[get_artist_discovery_service] = lambda: mock_discovery_service + app.dependency_overrides[get_artist_enrichment_service] = lambda: mock_enrichment_service + return TestClient(app) + + +class TestGetArtistBasicRoute: + def test_get_artist_calls_basic_method(self, client, mock_artist_service): + response = client.get(f"/api/v1/artists/{VALID_MBID}") + + assert response.status_code == 200 + mock_artist_service.get_artist_info_basic.assert_awaited_once_with(VALID_MBID) + + def test_get_artist_does_not_call_full_method(self, client, mock_artist_service): + response = client.get(f"/api/v1/artists/{VALID_MBID}") + + assert response.status_code == 200 + mock_artist_service.get_artist_info.assert_not_awaited() + + def test_get_artist_returns_valid_response(self, client): + response = client.get(f"/api/v1/artists/{VALID_MBID}") + body = response.json() + + assert response.status_code == 200 + assert body["name"] == "Test Artist" + assert body["musicbrainz_id"] == VALID_MBID + assert body["description"] is None + assert body["image"] is None + assert len(body["albums"]) == 1 + assert body["release_group_count"] == 1 + + def test_get_artist_value_error_returns_400(self, client, mock_artist_service): + mock_artist_service.get_artist_info_basic = AsyncMock( + side_effect=ValueError("Invalid artist request") + ) + response = client.get(f"/api/v1/artists/{VALID_MBID}") + + assert response.status_code == 400 diff --git a/backend/tests/routes/test_artist_releases_route.py b/backend/tests/routes/test_artist_releases_route.py new file mode 100644 index 0000000..134bb38 --- /dev/null +++ b/backend/tests/routes/test_artist_releases_route.py @@ -0,0 +1,83 @@ +import os +import tempfile + +os.environ.setdefault("ROOT_APP_DIR", tempfile.mkdtemp()) + +import pytest +from unittest.mock import AsyncMock + +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from api.v1.routes.artists import router +from api.v1.schemas.artist import ArtistReleases +from core.dependencies import get_artist_service, get_artist_discovery_service, get_artist_enrichment_service +from models.artist import ReleaseItem + + +VALID_MBID = "f4a31f0a-51dd-4fa7-986d-3095c40c5ed9" + + +@pytest.fixture +def mock_artist_service(): + mock = AsyncMock() + mock.get_artist_releases = AsyncMock( + return_value=ArtistReleases( + albums=[ReleaseItem(id="rg-2", title="Album Two", type="Album", year=2023)], + singles=[], + eps=[], + total_count=120, + has_more=True, + ) + ) + mock.get_artist_info_basic = AsyncMock() + return mock + + +@pytest.fixture +def mock_discovery_service(): + return AsyncMock() + + +@pytest.fixture +def mock_enrichment_service(): + return AsyncMock() + + +@pytest.fixture +def client(mock_artist_service, mock_discovery_service, mock_enrichment_service): + app = FastAPI() + app.include_router(router, prefix="/api/v1") + app.dependency_overrides[get_artist_service] = lambda: mock_artist_service + app.dependency_overrides[get_artist_discovery_service] = lambda: mock_discovery_service + app.dependency_overrides[get_artist_enrichment_service] = lambda: mock_enrichment_service + return TestClient(app) + + +class TestGetArtistReleasesRoute: + def test_pagination_params_forwarded(self, client, mock_artist_service): + response = client.get(f"/api/v1/artists/{VALID_MBID}/releases?offset=50&limit=50") + + assert response.status_code == 200 + mock_artist_service.get_artist_releases.assert_awaited_once_with(VALID_MBID, 50, 50) + + def test_has_more_flag_propagated(self, client): + response = client.get(f"/api/v1/artists/{VALID_MBID}/releases?offset=0&limit=50") + body = response.json() + + assert response.status_code == 200 + assert body["has_more"] is True + assert body["total_count"] == 120 + + def test_default_params(self, client, mock_artist_service): + response = client.get(f"/api/v1/artists/{VALID_MBID}/releases") + + assert response.status_code == 200 + mock_artist_service.get_artist_releases.assert_awaited_once_with(VALID_MBID, 0, 50) + + def test_value_error_returns_400(self, client, mock_artist_service): + mock_artist_service.get_artist_releases = AsyncMock( + side_effect=ValueError("Invalid artist request") + ) + response = client.get(f"/api/v1/artists/{VALID_MBID}/releases") + assert response.status_code == 400 diff --git a/backend/tests/services/test_artist_basic_info.py b/backend/tests/services/test_artist_basic_info.py new file mode 100644 index 0000000..83f3455 --- /dev/null +++ b/backend/tests/services/test_artist_basic_info.py @@ -0,0 +1,125 @@ +"""Tests that the basic artist info path returns correctly and skips Wikidata enrichment.""" +import pytest +from unittest.mock import AsyncMock, MagicMock + +from models.artist import ArtistInfo, ReleaseItem +from services.artist_service import ArtistService + + +ARTIST_MBID = "f4a31f0a-51dd-4fa7-986d-3095c40c5ed9" + + +def _make_mb_artist() -> dict: + return { + "id": ARTIST_MBID, + "name": "Test Artist", + "type": "Group", + "country": "GB", + "disambiguation": "", + "life-span": {"begin": "2000", "end": None, "ended": "false"}, + "tag-list": [{"name": "rock", "count": 5}], + "alias-list": [], + "url-relation-list": [], + "release-group-list": [ + { + "id": "rg-001", + "title": "First Album", + "type": "Album", + "primary-type": "Album", + "secondary-type-list": [], + "first-release-date": "2020-01-01", + } + ], + "release-group-count": 1, + } + + +def _make_service(*, cached_artist: ArtistInfo | None = None) -> tuple[ArtistService, AsyncMock]: + mb_repo = AsyncMock() + mb_repo.get_artist_by_id = AsyncMock(return_value=_make_mb_artist()) + + lidarr_repo = MagicMock() + lidarr_repo.is_configured.return_value = False + lidarr_repo.get_library_mbids = AsyncMock(return_value=set()) + lidarr_repo.get_requested_mbids = AsyncMock(return_value=set()) + lidarr_repo.get_artist_mbids = AsyncMock(return_value=set()) + + wikidata_repo = AsyncMock() + wikidata_repo.get_wikidata_info = AsyncMock( + side_effect=AssertionError("Wikidata should NOT be called in basic path") + ) + + prefs = MagicMock() + prefs.get_preferences.return_value = MagicMock( + primary_types=["Album", "Single", "EP"], + secondary_types=[], + ) + prefs.get_advanced_settings.return_value = MagicMock( + cache_ttl_artist_library=21600, + cache_ttl_artist_non_library=3600, + ) + + memory_cache = AsyncMock() + memory_cache.get = AsyncMock(return_value=cached_artist) + memory_cache.set = AsyncMock() + + disk_cache = AsyncMock() + disk_cache.get_artist = AsyncMock(return_value=None) + disk_cache.set_artist = AsyncMock() + + svc = ArtistService( + mb_repo=mb_repo, + lidarr_repo=lidarr_repo, + wikidata_repo=wikidata_repo, + preferences_service=prefs, + memory_cache=memory_cache, + disk_cache=disk_cache, + ) + return svc, wikidata_repo + + +class TestGetArtistInfoBasic: + @pytest.mark.asyncio + async def test_cold_cache_skips_wikidata(self): + svc, wikidata_repo = _make_service() + + result = await svc.get_artist_info_basic(ARTIST_MBID) + + assert result.name == "Test Artist" + assert result.musicbrainz_id == ARTIST_MBID + assert result.description is None + assert result.image is None + wikidata_repo.get_wikidata_info.assert_not_awaited() + + @pytest.mark.asyncio + async def test_cold_cache_sets_release_group_count(self): + svc, _ = _make_service() + + result = await svc.get_artist_info_basic(ARTIST_MBID) + + assert result.release_group_count == 1 + + @pytest.mark.asyncio + async def test_cached_artist_returned_directly(self): + cached = ArtistInfo( + name="Cached Artist", + musicbrainz_id=ARTIST_MBID, + description="Cached description", + image="https://example.com/img.jpg", + albums=[ReleaseItem(id="rg-cached", title="Cached Album", type="Album")], + ) + svc, wikidata_repo = _make_service(cached_artist=cached) + + result = await svc.get_artist_info_basic(ARTIST_MBID) + + assert result.name == "Cached Artist" + assert result.description == "Cached description" + assert result.image == "https://example.com/img.jpg" + wikidata_repo.get_wikidata_info.assert_not_awaited() + + @pytest.mark.asyncio + async def test_invalid_mbid_raises_value_error(self): + svc, _ = _make_service() + + with pytest.raises(ValueError): + await svc.get_artist_info_basic("not-a-uuid") diff --git a/backend/tests/services/test_artist_discovery_service.py b/backend/tests/services/test_artist_discovery_service.py index e43f975..966f5c1 100644 --- a/backend/tests/services/test_artist_discovery_service.py +++ b/backend/tests/services/test_artist_discovery_service.py @@ -416,30 +416,29 @@ class TestGetTopAlbumsSource: assert result.albums[2].requested is False @pytest.mark.asyncio - async def test_source_lastfm_resolves_release_mbids_to_release_groups(self): + async def test_source_lastfm_uses_raw_mbids_without_resolution(self): lastfm_albums = [ LastFmAlbum(name="Album A", artist_name="Artist", mbid="release-mbid-a", playcount=100), LastFmAlbum(name="Album B", artist_name="Artist", mbid="release-mbid-b", playcount=50), ] svc, _, lastfm_repo, _ = _make_service() lastfm_repo.get_artist_top_albums.return_value = lastfm_albums - svc._lidarr_repo.get_library_mbids = AsyncMock(return_value={"rg-resolved-a"}) + svc._lidarr_repo.get_library_mbids = AsyncMock(return_value={"release-mbid-a"}) svc._lidarr_repo.get_requested_mbids = AsyncMock(return_value=set()) - async def mock_resolve(rid): - return {"release-mbid-a": "rg-resolved-a", "release-mbid-b": "rg-resolved-b"}.get(rid) - - svc._mb_repo.get_release_group_id_from_release = mock_resolve + svc._mb_repo.get_release_group_id_from_release = AsyncMock( + side_effect=AssertionError("Resolution should not be called") + ) result = await svc.get_top_albums("mbid-123", count=10, source="lastfm") - assert result.albums[0].release_group_mbid == "rg-resolved-a" + assert result.albums[0].release_group_mbid == "release-mbid-a" assert result.albums[0].in_library is True - assert result.albums[1].release_group_mbid == "rg-resolved-b" + assert result.albums[1].release_group_mbid == "release-mbid-b" assert result.albums[1].in_library is False @pytest.mark.asyncio - async def test_source_lastfm_keeps_original_mbid_when_resolution_fails(self): + async def test_source_lastfm_keeps_raw_mbid_directly(self): lastfm_albums = [ LastFmAlbum(name="Album A", artist_name="Artist", mbid="already-rg-mbid", playcount=100), ] @@ -448,8 +447,6 @@ class TestGetTopAlbumsSource: svc._lidarr_repo.get_library_mbids = AsyncMock(return_value=set()) svc._lidarr_repo.get_requested_mbids = AsyncMock(return_value=set()) - svc._mb_repo.get_release_group_id_from_release = AsyncMock(return_value=None) - result = await svc.get_top_albums("mbid-123", count=10, source="lastfm") assert result.albums[0].release_group_mbid == "already-rg-mbid" diff --git a/backend/tests/services/test_artist_utils_tags.py b/backend/tests/services/test_artist_utils_tags.py new file mode 100644 index 0000000..95d05ce --- /dev/null +++ b/backend/tests/services/test_artist_utils_tags.py @@ -0,0 +1,63 @@ +"""Tests for extract_tags deduplication in artist_utils.""" + +from services.artist_utils import extract_tags + + +def test_extract_tags_deduplicates(): + mb_artist = { + "tags": [ + {"name": "rock"}, + {"name": "indie"}, + {"name": "rock"}, + {"name": "alternative"}, + {"name": "indie"}, + ] + } + result = extract_tags(mb_artist) + assert result == ["rock", "indie", "alternative"] + + +def test_extract_tags_preserves_order(): + mb_artist = { + "tags": [ + {"name": "electronic"}, + {"name": "ambient"}, + {"name": "electronic"}, + {"name": "downtempo"}, + ] + } + result = extract_tags(mb_artist) + assert result == ["electronic", "ambient", "downtempo"] + + +def test_extract_tags_respects_limit_after_dedup(): + mb_artist = { + "tags": [ + {"name": "a"}, + {"name": "b"}, + {"name": "a"}, + {"name": "c"}, + {"name": "d"}, + ] + } + result = extract_tags(mb_artist, limit=2) + assert result == ["a", "b"] + + +def test_extract_tags_empty(): + assert extract_tags({}) == [] + assert extract_tags({"tags": []}) == [] + + +def test_extract_tags_skips_empty_names(): + mb_artist = { + "tags": [ + {"name": "rock"}, + {"name": ""}, + {"name": None}, + {}, + {"name": "rock"}, + ] + } + result = extract_tags(mb_artist) + assert result == ["rock"] diff --git a/backend/tests/services/test_discovery_precache_lock.py b/backend/tests/services/test_discovery_precache_lock.py index 6186276..b0b164d 100644 --- a/backend/tests/services/test_discovery_precache_lock.py +++ b/backend/tests/services/test_discovery_precache_lock.py @@ -103,8 +103,8 @@ async def test_lock_released_after_exception(): @pytest.mark.asyncio -async def test_delay_holds_semaphore_slot(): - """Delay is applied inside the semaphore, blocking other artists from starting.""" +async def test_delay_does_not_hold_semaphore_slot(): + """Delay is applied outside the semaphore, allowing other artists to start during sleep.""" svc = _make_service() timestamps: list[float] = [] @@ -125,12 +125,13 @@ async def test_delay_holds_semaphore_slot(): ) assert len(timestamps) == 4 - # With concurrency=2 and delay=0.15s inside semaphore, the 3rd artist - # cannot start until one of the first two finishes its delay. - # The gap between the 2nd and 3rd timestamps should be >= delay. + # With concurrency=2 and delay=0.15s OUTSIDE semaphore, the 3rd artist + # can start as soon as a semaphore slot frees (before the delay finishes). + # All four API calls should start quickly — the gap between 1st and 3rd + # should be small since the semaphore isn't held during sleep. sorted_ts = sorted(timestamps) gap = sorted_ts[2] - sorted_ts[0] - assert gap >= 0.1, f"Expected >=0.1s gap due to semaphore-held delay, got {gap:.3f}s" + assert gap < 0.15, f"Expected <0.15s gap since sleep is outside semaphore, got {gap:.3f}s" @pytest.mark.asyncio @@ -190,3 +191,32 @@ async def test_guard_survives_instance_recreation(): await task1 assert _ads_module._discovery_precache_running is False + + +@pytest.mark.asyncio +async def test_worker_timeout_fires_and_updates_progress(): + """A worker that exceeds the per-artist timeout is killed and progress is updated.""" + svc = _make_service() + + async def hang_forever(*args, **kwargs): + await asyncio.sleep(9999) + return MagicMock() # pragma: no cover + + status = MagicMock() + status.is_cancelled = MagicMock(return_value=False) + status.update_progress = AsyncMock() + + with ( + patch.object(svc, "get_similar_artists", new_callable=AsyncMock, side_effect=hang_forever), + patch.object(svc, "get_top_songs", new_callable=AsyncMock, side_effect=hang_forever), + patch.object(svc, "get_top_albums", new_callable=AsyncMock, side_effect=hang_forever), + patch("services.artist_discovery_service._DISCOVERY_WORKER_TIMEOUT", 0.1), + ): + result = await svc.precache_artist_discovery( + ["mbid-a"], delay=0, status_service=status, + ) + + assert result == 0 + assert status.update_progress.await_count >= 1 + last_call_args = status.update_progress.call_args + assert "timed out" in str(last_call_args) diff --git a/backend/tests/services/test_discovery_precache_progress.py b/backend/tests/services/test_discovery_precache_progress.py index 566f496..5085b7f 100644 --- a/backend/tests/services/test_discovery_precache_progress.py +++ b/backend/tests/services/test_discovery_precache_progress.py @@ -58,8 +58,8 @@ async def test_progress_updates_on_success(): ) assert status.update_progress.call_count == 2 - status.update_progress.assert_any_call(1, current_item="Artist A") - status.update_progress.assert_any_call(2, current_item="Artist B") + status.update_progress.assert_any_call(1, current_item="Artist A", generation=0) + status.update_progress.assert_any_call(2, current_item="Artist B", generation=0) @pytest.mark.asyncio @@ -82,9 +82,9 @@ async def test_progress_updates_even_on_failure(): ) assert status.update_progress.call_count == 3 - status.update_progress.assert_any_call(1, current_item="A") - status.update_progress.assert_any_call(2, current_item="B") - status.update_progress.assert_any_call(3, current_item="C") + status.update_progress.assert_any_call(1, current_item="A", generation=0) + status.update_progress.assert_any_call(2, current_item="B", generation=0) + status.update_progress.assert_any_call(3, current_item="C", generation=0) @pytest.mark.asyncio diff --git a/backend/tests/services/test_refresh_library_flags.py b/backend/tests/services/test_refresh_library_flags.py new file mode 100644 index 0000000..0511231 --- /dev/null +++ b/backend/tests/services/test_refresh_library_flags.py @@ -0,0 +1,158 @@ +"""Tests for _refresh_library_flags in_lidarr/monitored/auto_download refresh.""" +import pytest +from unittest.mock import AsyncMock, MagicMock + +from models.artist import ArtistInfo +from services.artist_service import ArtistService + + +@pytest.fixture +def mock_lidarr_repo(): + repo = AsyncMock() + repo.is_configured = MagicMock(return_value=True) + repo.get_library_mbids.return_value = set() + repo.get_requested_mbids.return_value = set() + repo.get_artist_mbids.return_value = set() + repo.get_artist_details.return_value = None + return repo + + +@pytest.fixture +def artist_service(mock_lidarr_repo): + return ArtistService( + mb_repo=AsyncMock(), + lidarr_repo=mock_lidarr_repo, + wikidata_repo=AsyncMock(), + preferences_service=MagicMock(), + memory_cache=AsyncMock(), + disk_cache=AsyncMock(), + ) + + +def _make_artist(mbid: str = "aaa-bbb", in_lidarr: bool = False, + monitored: bool = False, auto_download: bool = False) -> ArtistInfo: + return ArtistInfo( + name="Test Artist", + musicbrainz_id=mbid, + in_lidarr=in_lidarr, + monitored=monitored, + auto_download=auto_download, + ) + + +class TestRefreshLibraryFlagsLidarrTransition: + """When an artist transitions into artist_mbids, in_lidarr/monitored/auto_download should update.""" + + @pytest.mark.asyncio + async def test_transition_sets_in_lidarr_and_monitoring(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = {"aaa-bbb"} + mock_lidarr_repo.get_artist_details.return_value = { + "monitored": True, "monitor_new_items": "all", + } + artist = _make_artist(in_lidarr=False) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is True + assert artist.monitored is True + assert artist.auto_download is True + mock_lidarr_repo.get_artist_details.assert_awaited_once_with("aaa-bbb") + + @pytest.mark.asyncio + async def test_transition_monitored_false_auto_download_none(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = {"aaa-bbb"} + mock_lidarr_repo.get_artist_details.return_value = { + "monitored": False, "monitor_new_items": "none", + } + artist = _make_artist(in_lidarr=False) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is True + assert artist.monitored is False + assert artist.auto_download is False + + @pytest.mark.asyncio + async def test_transition_details_none_still_sets_in_lidarr(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = {"aaa-bbb"} + mock_lidarr_repo.get_artist_details.return_value = None + artist = _make_artist(in_lidarr=False) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is True + + @pytest.mark.asyncio + async def test_transition_details_exception_graceful_degradation(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = {"aaa-bbb"} + mock_lidarr_repo.get_artist_details.side_effect = Exception("Lidarr down") + artist = _make_artist(in_lidarr=False) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is True + assert artist.monitored is False + assert artist.auto_download is False + + @pytest.mark.asyncio + async def test_already_in_lidarr_refreshes_monitoring_flags(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = {"aaa-bbb"} + mock_lidarr_repo.get_artist_details.return_value = { + "monitored": False, "monitor_new_items": "none", + } + artist = _make_artist(in_lidarr=True, monitored=True, auto_download=True) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is True + assert artist.monitored is False + assert artist.auto_download is False + mock_lidarr_repo.get_artist_details.assert_awaited_once_with("aaa-bbb") + + @pytest.mark.asyncio + async def test_removed_from_artist_mbids_preserves_lidarr_flags(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_artist_mbids.return_value = set() + artist = _make_artist(in_lidarr=True, monitored=True, auto_download=True) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_library is False + assert artist.in_lidarr is True + assert artist.monitored is True + assert artist.auto_download is True + + @pytest.mark.asyncio + async def test_not_configured_skips(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.is_configured.return_value = False + artist = _make_artist(in_lidarr=False) + + await artist_service._refresh_library_flags(artist) + + assert artist.in_lidarr is False + mock_lidarr_repo.get_artist_mbids.assert_not_awaited() + + @pytest.mark.asyncio + async def test_release_in_library_flags_still_refreshed(self, artist_service, mock_lidarr_repo): + mock_lidarr_repo.get_library_mbids.return_value = {"album-1"} + mock_lidarr_repo.get_requested_mbids.return_value = {"album-2"} + mock_lidarr_repo.get_artist_mbids.return_value = set() + + from models.artist import ReleaseItem + artist = _make_artist() + artist = ArtistInfo( + name="Test", musicbrainz_id="aaa-bbb", + albums=[ + ReleaseItem(id="album-1", title="A"), + ReleaseItem(id="album-2", title="B"), + ReleaseItem(id="album-3", title="C"), + ], + ) + + await artist_service._refresh_library_flags(artist) + + assert artist.albums[0].in_library is True + assert artist.albums[0].requested is False + assert artist.albums[1].in_library is False + assert artist.albums[1].requested is True + assert artist.albums[2].in_library is False + assert artist.albums[2].requested is False diff --git a/backend/tests/services/test_top_albums_lastfm_fast.py b/backend/tests/services/test_top_albums_lastfm_fast.py new file mode 100644 index 0000000..2a3d6aa --- /dev/null +++ b/backend/tests/services/test_top_albums_lastfm_fast.py @@ -0,0 +1,98 @@ +import pytest +from unittest.mock import AsyncMock, MagicMock + +from repositories.lastfm_models import LastFmAlbum +from services.artist_discovery_service import ArtistDiscoveryService + + +ARTIST_MBID = "f4a31f0a-51dd-4fa7-986d-3095c40c5ed9" +RELEASE_MBID_1 = "aaaaaaaa-0000-0000-0000-000000000001" +RELEASE_MBID_2 = "aaaaaaaa-0000-0000-0000-000000000002" + + +def _make_lastfm_albums() -> list[LastFmAlbum]: + return [ + LastFmAlbum(name="Album A", artist_name="Test Artist", mbid=RELEASE_MBID_1, playcount=5000), + LastFmAlbum(name="Album B", artist_name="Test Artist", mbid=RELEASE_MBID_2, playcount=3000), + LastFmAlbum(name="Album C (no mbid)", artist_name="Test Artist", mbid="", playcount=1000), + ] + + +def _make_service() -> tuple[ArtistDiscoveryService, AsyncMock]: + lb_repo = MagicMock() + lb_repo.is_configured.return_value = False + + lastfm_repo = AsyncMock() + lastfm_repo.get_artist_top_albums = AsyncMock(return_value=_make_lastfm_albums()) + + prefs = MagicMock() + prefs.is_lastfm_enabled.return_value = True + + library_db = AsyncMock() + library_db.get_all_artist_mbids = AsyncMock(return_value=set()) + + memory_cache = AsyncMock() + memory_cache.get = AsyncMock(return_value=None) + memory_cache.set = AsyncMock() + + mb_repo = AsyncMock() + mb_repo.get_release_group_id_from_release = AsyncMock( + side_effect=AssertionError("MusicBrainz resolution should NOT be called for Last.fm top-albums") + ) + + lidarr_repo = AsyncMock() + lidarr_repo.get_library_mbids = AsyncMock(return_value={RELEASE_MBID_1}) + lidarr_repo.get_requested_mbids = AsyncMock(return_value={RELEASE_MBID_2}) + + svc = ArtistDiscoveryService( + listenbrainz_repo=lb_repo, + musicbrainz_repo=mb_repo, + library_db=library_db, + lidarr_repo=lidarr_repo, + memory_cache=memory_cache, + lastfm_repo=lastfm_repo, + preferences_service=prefs, + ) + return svc, mb_repo + + +class TestLastFmTopAlbumsNoResolution: + @pytest.mark.asyncio + async def test_no_musicbrainz_resolution_called(self): + svc, mb_repo = _make_service() + + result = await svc.get_top_albums(ARTIST_MBID, count=10, source="lastfm") + + assert len(result.albums) == 3 + mb_repo.get_release_group_id_from_release.assert_not_awaited() + + @pytest.mark.asyncio + async def test_uses_raw_lastfm_mbid(self): + svc, _ = _make_service() + + result = await svc.get_top_albums(ARTIST_MBID, count=10, source="lastfm") + + assert result.albums[0].release_group_mbid == RELEASE_MBID_1 + assert result.albums[1].release_group_mbid == RELEASE_MBID_2 + assert result.albums[2].release_group_mbid is None + + @pytest.mark.asyncio + async def test_library_flags_use_raw_mbid(self): + svc, _ = _make_service() + + result = await svc.get_top_albums(ARTIST_MBID, count=10, source="lastfm") + + assert result.albums[0].in_library is True + assert result.albums[0].requested is False + assert result.albums[1].in_library is False + assert result.albums[1].requested is True + assert result.albums[2].in_library is False + assert result.albums[2].requested is False + + @pytest.mark.asyncio + async def test_source_is_lastfm(self): + svc, _ = _make_service() + + result = await svc.get_top_albums(ARTIST_MBID, count=10, source="lastfm") + + assert result.source == "lastfm" diff --git a/backend/tests/test_advanced_settings_roundtrip.py b/backend/tests/test_advanced_settings_roundtrip.py index 3a30b32..2c97432 100644 --- a/backend/tests/test_advanced_settings_roundtrip.py +++ b/backend/tests/test_advanced_settings_roundtrip.py @@ -209,7 +209,8 @@ class TestSyncSettingsRoundTrip: ("sync_max_timeout_hours", 8), ("audiodb_prewarm_concurrency", 4), ("audiodb_prewarm_delay", 0.3), - ("artist_discovery_precache_concurrency", 3), + ("artist_discovery_precache_concurrency", 5), + ("artist_discovery_precache_delay", 0.2), ]) def test_defaults_match(self, field: str, default_val) -> None: backend = AdvancedSettings() diff --git a/backend/tests/test_audiodb_parallel.py b/backend/tests/test_audiodb_parallel.py index cc575c1..1b1aa8d 100644 --- a/backend/tests/test_audiodb_parallel.py +++ b/backend/tests/test_audiodb_parallel.py @@ -126,7 +126,7 @@ class TestAudioDBParallel: status = _make_status_service() await phase.precache_audiodb_data([], [], status) - status.skip_phase.assert_called_once_with('audiodb_prewarm') + status.skip_phase.assert_called_once_with('audiodb_prewarm', generation=0) assert True @pytest.mark.asyncio @@ -149,5 +149,5 @@ class TestAudioDBParallel: await phase.precache_audiodb_data(artists, [], status) - status.skip_phase.assert_called_once_with('audiodb_prewarm') + status.skip_phase.assert_called_once_with('audiodb_prewarm', generation=0) assert True diff --git a/backend/tests/test_queue_disk_invalidation.py b/backend/tests/test_queue_disk_invalidation.py new file mode 100644 index 0000000..b0a2bb0 --- /dev/null +++ b/backend/tests/test_queue_disk_invalidation.py @@ -0,0 +1,84 @@ +"""Tests that queue processor and on_queue_import invalidate disk cache for artist.""" +import os +import tempfile +os.environ.setdefault("ROOT_APP_DIR", tempfile.mkdtemp()) + +import pytest +from unittest.mock import AsyncMock, MagicMock + +from core.dependencies.service_providers import make_on_queue_import, make_processor +from infrastructure.persistence.request_history import RequestHistoryRecord + + +def _make_record(artist_mbid: str | None = "artist-aaa") -> RequestHistoryRecord: + return RequestHistoryRecord( + musicbrainz_id="album-111", + artist_name="Test", + album_title="Album", + requested_at="2025-01-01", + status="pending", + artist_mbid=artist_mbid, + monitor_artist=True, + auto_download_artist=False, + ) + + +class TestOnQueueImportDiskInvalidation: + """on_queue_import should call disk_cache.delete_artist when artist_mbid is present.""" + + @pytest.mark.asyncio + async def test_disk_cache_deleted_for_artist(self): + disk_cache = AsyncMock() + memory_cache = AsyncMock() + memory_cache.delete.return_value = None + memory_cache.clear_prefix.return_value = 0 + library_db = AsyncMock() + + record = _make_record(artist_mbid="artist-aaa") + + on_queue_import = make_on_queue_import(memory_cache, disk_cache, library_db) + await on_queue_import(record) + + disk_cache.delete_artist.assert_awaited_once_with("artist-aaa") + + @pytest.mark.asyncio + async def test_disk_cache_not_called_without_artist_mbid(self): + disk_cache = AsyncMock() + memory_cache = AsyncMock() + memory_cache.delete.return_value = None + memory_cache.clear_prefix.return_value = 0 + library_db = AsyncMock() + + record = _make_record(artist_mbid=None) + + on_queue_import = make_on_queue_import(memory_cache, disk_cache, library_db) + await on_queue_import(record) + + disk_cache.delete_artist.assert_not_awaited() + + +class TestProcessorDiskInvalidation: + """processor should call disk_cache.delete_artist after deferred monitoring.""" + + @pytest.mark.asyncio + async def test_disk_cache_deleted_after_artist_monitoring(self): + disk_cache = AsyncMock() + memory_cache = AsyncMock() + memory_cache.delete.return_value = None + lidarr_repo = AsyncMock() + lidarr_repo.add_album.return_value = { + "payload": {"monitored": True, "artist": {"foreignArtistId": "artist-aaa"}}, + "monitored": True, + } + lidarr_repo.update_artist_monitoring.return_value = {} + + request_history = MagicMock() + record = _make_record() + request_history.async_get_record = AsyncMock(return_value=record) + + cover_repo = AsyncMock() + + processor = make_processor(lidarr_repo, memory_cache, disk_cache, cover_repo, request_history) + await processor("album-111") + + disk_cache.delete_artist.assert_awaited_once_with("artist-aaa") diff --git a/backend/tests/test_sync_generation.py b/backend/tests/test_sync_generation.py new file mode 100644 index 0000000..487ca4d --- /dev/null +++ b/backend/tests/test_sync_generation.py @@ -0,0 +1,219 @@ +"""Tests for MUS-19: sync generation counter, false-failed status, cancel, progress clamp.""" + +import asyncio +import os + +import pytest +from unittest.mock import AsyncMock, MagicMock, patch + +from services.cache_status_service import CacheStatusService, CacheSyncProgress + + +def _make_status_service() -> CacheStatusService: + store = AsyncMock() + store.save_sync_state = AsyncMock() + svc = CacheStatusService(store) + svc._sse_subscribers = [] + return svc + + +class TestGenerationCounter: + """Generation counter rejects stale writes from old syncs.""" + + @pytest.mark.asyncio + async def test_start_sync_returns_generation(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + assert gen1 >= 1 + gen2 = await svc.start_sync('artists', 5) + assert gen2 == gen1 + 1 + + @pytest.mark.asyncio + async def test_stale_update_progress_rejected(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + gen2 = await svc.start_sync('artists', 5) + + await svc.update_progress(3, 'old item', generation=gen1) + progress = svc.get_progress() + assert progress.processed_items == 0, "Stale generation write should be rejected" + + await svc.update_progress(2, 'new item', generation=gen2) + progress = svc.get_progress() + assert progress.processed_items == 2 + + @pytest.mark.asyncio + async def test_stale_update_phase_rejected(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + gen2 = await svc.start_sync('albums', 5) + + await svc.update_phase('audiodb_prewarm', 100, generation=gen1) + progress = svc.get_progress() + assert progress.phase == 'albums', "Stale generation should not change phase" + + @pytest.mark.asyncio + async def test_stale_complete_sync_rejected(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + _gen2 = await svc.start_sync('artists', 5) + + await svc.complete_sync(generation=gen1) + progress = svc.get_progress() + assert progress.is_syncing is True, "Stale complete_sync should not stop current sync" + + @pytest.mark.asyncio + async def test_stale_skip_phase_rejected(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + gen2 = await svc.start_sync('albums', 5) + + await svc.skip_phase('albums', generation=gen1) + progress = svc.get_progress() + assert progress.phase == 'albums', "Stale skip_phase should be rejected" + + @pytest.mark.asyncio + async def test_stale_persist_progress_rejected(self): + svc = _make_status_service() + gen1 = await svc.start_sync('artists', 10) + _gen2 = await svc.start_sync('artists', 5) + + svc._sync_state_store.save_sync_state.reset_mock() + await svc.persist_progress(generation=gen1) + svc._sync_state_store.save_sync_state.assert_not_called() + + @pytest.mark.asyncio + async def test_generation_zero_bypasses_guard(self): + """generation=0 (default) always passes through, for backward compatibility.""" + svc = _make_status_service() + _gen = await svc.start_sync('artists', 10) + + await svc.update_progress(5, 'item', generation=0) + progress = svc.get_progress() + assert progress.processed_items == 5 + + +class TestProgressClamp: + """progress_percent is clamped to 100.""" + + @pytest.mark.asyncio + async def test_percent_clamped_to_100(self): + svc = _make_status_service() + await svc.start_sync('artists', 5) + await svc.update_progress(20, 'overflow') + progress = svc.get_progress() + assert progress.progress_percent <= 100 + + +class TestSkippedAutoSync: + """Skipped auto-sync must not flip last_sync_success to False.""" + + @pytest.mark.asyncio + async def test_skipped_sync_does_not_update_status(self): + from core.tasks import sync_library_periodically + from api.v1.schemas.library import SyncLibraryResponse + + mock_lib = AsyncMock() + mock_lib._lidarr_repo = MagicMock() + mock_lib._lidarr_repo.is_configured.return_value = True + mock_lib.sync_library.return_value = SyncLibraryResponse( + status="skipped", artists=0, albums=0 + ) + + mock_prefs = MagicMock() + lidarr_settings = MagicMock() + lidarr_settings.sync_frequency = "5min" + mock_prefs.get_lidarr_settings.return_value = lidarr_settings + + call_count = 0 + + original_sleep = asyncio.sleep + async def fake_sleep(duration): + nonlocal call_count + call_count += 1 + if call_count >= 2: + raise asyncio.CancelledError() + await original_sleep(0) + + with patch('asyncio.sleep', side_effect=fake_sleep): + try: + await sync_library_periodically(mock_lib, mock_prefs) + except asyncio.CancelledError: + pass + + mock_prefs.save_lidarr_settings.assert_not_called() + + +class TestCancelSync: + """Cancel endpoint and cancellation behavior.""" + + @pytest.mark.asyncio + async def test_cancel_always_sets_event(self): + svc = _make_status_service() + assert not svc.is_cancelled() + await svc.cancel_current_sync() + assert svc.is_cancelled() + + @pytest.mark.asyncio + async def test_cancel_works_when_not_syncing(self): + """Cancel should work even when is_syncing is False (post-completion AudioDB).""" + svc = _make_status_service() + await svc.cancel_current_sync() + assert svc.is_cancelled() + + +class TestCancelRoute: + """Cancel sync API endpoint.""" + + @pytest.mark.skipif( + not os.access('/app', os.W_OK), + reason="Route tests require /app to be writable (Docker environment)", + ) + def test_cancel_endpoint_calls_service_and_registry(self): + from fastapi import FastAPI + from fastapi.testclient import TestClient + from api.v1.routes.cache_status import router + from core.dependencies import get_cache_status_service + + mock_svc = MagicMock() + mock_svc.cancel_current_sync = AsyncMock() + mock_svc.wait_for_completion = AsyncMock() + + app = FastAPI() + app.include_router(router) + app.dependency_overrides[get_cache_status_service] = lambda: mock_svc + + with patch("core.task_registry.TaskRegistry") as MockRegistry: + mock_registry_instance = MagicMock() + MockRegistry.get_instance.return_value = mock_registry_instance + + client = TestClient(app) + resp = client.post("/cache/sync/cancel") + + assert resp.status_code == 200 + assert resp.json() == {"status": "cancelled"} + mock_svc.cancel_current_sync.assert_awaited_once() + mock_registry_instance.cancel.assert_called_once_with("precache-library") + mock_svc.wait_for_completion.assert_awaited_once() + + +class TestRestoreAudioDBPhase: + """restore_from_persistence handles audiodb_prewarm phase.""" + + @pytest.mark.asyncio + async def test_audiodb_prewarm_phase_restores(self): + svc = _make_status_service() + svc._sync_state_store.get_sync_state = AsyncMock(return_value={ + 'status': 'running', + 'phase': 'audiodb_prewarm', + 'total_artists': 100, + 'processed_artists': 100, + 'total_albums': 50, + 'processed_albums': 50, + 'started_at': 1000, + }) + await svc.restore_from_persistence() + progress = svc.get_progress() + assert progress.is_syncing is True + assert progress.phase == 'audiodb_prewarm' + assert progress.total_items == 0 diff --git a/frontend/src/lib/components/ArtistHero.svelte b/frontend/src/lib/components/ArtistHero.svelte index e80da91..45bc9b3 100644 --- a/frontend/src/lib/components/ArtistHero.svelte +++ b/frontend/src/lib/components/ArtistHero.svelte @@ -1,11 +1,12 @@