From 039f152f31c03fed9071ee35d189f3c55cb349f5 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 27 Apr 2026 22:23:41 -0700 Subject: [PATCH] PR4c: lift _automatic_wishlist_cleanup_after_db_update to core/downloads/cleanup.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third sub-PR in the download orchestrator series. Strict 1:1 lift — zero behavior change. What moved: - _automatic_wishlist_cleanup_after_db_update → cleanup_wishlist_after_db_update The lifted fn takes config_manager as an arg (so core/downloads/cleanup.py doesn't need to import web_server). Other deps (wishlist_service, MusicDatabase, get_database) stay as in-function imports — matches the original deferred-import pattern. The single caller in web_server.py (missing_download_executor.submit at L18028) keeps using the same wrapper name with no signature change. Behavior parity: - Same per-profile iteration via get_all_profiles() - Same essential-field skip (no name / no artists / no spotify_track_id) - Same artist normalization (string / dict / fallback to str()) - Same 0.7 confidence threshold for db match - Same break-on-first-artist-match semantics - Same album extraction (dict.name vs string passthrough) - Same active_server pulled via config_manager.get_active_media_server() - Same per-track exception swallowing inside the loops - Same top-level exception swallow with traceback.print_exc() - Same logger messages (exact text match for "[Auto Cleanup]" prefix) Tests: 13 new under tests/downloads/test_downloads_cleanup.py covering empty wishlist short-circuit, found-in-db removal, missed track stays, low-confidence skip, missing-fields skip, dict + string artist formats, break-on-first-match, multi-profile walk, album dict/string handling, db check failure continuing to next artist, top-level exception swallow, active server propagation. Full suite: 934 passing (was 921). Ruff clean. --- core/downloads/cleanup.py | 100 ++++++++ tests/downloads/test_downloads_cleanup.py | 266 ++++++++++++++++++++++ web_server.py | 88 +------ 3 files changed, 372 insertions(+), 82 deletions(-) create mode 100644 core/downloads/cleanup.py create mode 100644 tests/downloads/test_downloads_cleanup.py diff --git a/core/downloads/cleanup.py b/core/downloads/cleanup.py new file mode 100644 index 00000000..88d46531 --- /dev/null +++ b/core/downloads/cleanup.py @@ -0,0 +1,100 @@ +"""Automatic wishlist cleanup after database updates. + +Runs as a background task after the library DB refresh completes — walks +every profile's wishlist, fuzzy-matches each track against the freshly +scanned library, and removes hits. Best-effort: logs and continues on +per-track failure, swallows top-level exceptions so the executor doesn't +get a propagated failure. + +Lifted verbatim from web_server.py's `_automatic_wishlist_cleanup_after_db_update`. +The single global dep (`config_manager`) is passed in to keep this module +free of web_server imports. +""" + +from __future__ import annotations + +import logging +import traceback + +logger = logging.getLogger(__name__) + + +def cleanup_wishlist_after_db_update(config_manager) -> None: + """Walk all profiles' wishlists and remove tracks now present in the library.""" + try: + from core.wishlist_service import get_wishlist_service + from database.music_database import MusicDatabase, get_database + + wishlist_service = get_wishlist_service() + db = MusicDatabase() + active_server = config_manager.get_active_media_server() + + logger.info("[Auto Cleanup] Starting automatic wishlist cleanup after database update...") + + # Get all wishlist tracks (across all profiles - cleanup is global) + database = get_database() + all_profiles = database.get_all_profiles() + wishlist_tracks = [] + for p in all_profiles: + wishlist_tracks.extend(wishlist_service.get_wishlist_tracks_for_download(profile_id=p['id'])) + if not wishlist_tracks: + logger.warning("[Auto Cleanup] No tracks in wishlist to clean up") + return + + logger.info(f"[Auto Cleanup] Found {len(wishlist_tracks)} tracks in wishlist") + + removed_count = 0 + + for track in wishlist_tracks: + track_name = track.get('name', '') + artists = track.get('artists', []) + spotify_track_id = track.get('spotify_track_id') or track.get('id') + track_album = track.get('album', {}).get('name') if isinstance(track.get('album'), dict) else track.get('album') + + # Skip if no essential data + if not track_name or not artists or not spotify_track_id: + continue + + # Check each artist + found_in_db = False + for artist in artists: + # Handle both string format and dict format + if isinstance(artist, str): + artist_name = artist + elif isinstance(artist, dict) and 'name' in artist: + artist_name = artist['name'] + else: + artist_name = str(artist) + + try: + db_track, confidence = db.check_track_exists( + track_name, artist_name, + confidence_threshold=0.7, + server_source=active_server, + album=track_album, + ) + + if db_track and confidence >= 0.7: + found_in_db = True + logger.info(f"[Auto Cleanup] Track found in database: '{track_name}' by {artist_name} (confidence: {confidence:.2f})") + break + + except Exception as db_error: + logger.error(f"[Auto Cleanup] Error checking database for track '{track_name}': {db_error}") + continue + + # If found in database, remove from wishlist + if found_in_db: + try: + removed = wishlist_service.mark_track_download_result(spotify_track_id, success=True) + if removed: + removed_count += 1 + logger.info(f"[Auto Cleanup] Removed track from wishlist: '{track_name}' ({spotify_track_id})") + except Exception as remove_error: + logger.error(f"[Auto Cleanup] Error removing track from wishlist: {remove_error}") + + logger.info(f"[Auto Cleanup] Completed automatic cleanup: {removed_count} tracks removed from wishlist") + + except Exception as e: + logger.error(f"[Auto Cleanup] Error in automatic wishlist cleanup: {e}") + traceback.print_exc() diff --git a/tests/downloads/test_downloads_cleanup.py b/tests/downloads/test_downloads_cleanup.py new file mode 100644 index 00000000..69cb9dff --- /dev/null +++ b/tests/downloads/test_downloads_cleanup.py @@ -0,0 +1,266 @@ +"""Tests for core/downloads/cleanup.py — automatic wishlist cleanup after DB updates.""" + +from __future__ import annotations + +import pytest + + +# --------------------------------------------------------------------------- +# Fakes +# --------------------------------------------------------------------------- + +class _FakeWishlistService: + def __init__(self, tracks_per_profile=None, mark_results=None): + # tracks_per_profile: {profile_id: [track_dict, ...]} + self._tracks = tracks_per_profile or {} + # mark_results: {spotify_track_id: True/False} — whether mark_track_download_result returns True + self._mark = mark_results or {} + self.mark_calls = [] + + def get_wishlist_tracks_for_download(self, profile_id): + return list(self._tracks.get(profile_id, [])) + + def mark_track_download_result(self, spotify_track_id, success): + self.mark_calls.append((spotify_track_id, success)) + return self._mark.get(spotify_track_id, True) + + +class _FakeMusicDB: + def __init__(self, hits=None): + # hits: {(track_name, artist_name): (db_track_obj, confidence)} + self._hits = hits or {} + self.check_calls = [] + + def check_track_exists(self, track_name, artist_name, confidence_threshold=0.7, + server_source=None, album=None): + self.check_calls.append((track_name, artist_name, server_source, album)) + return self._hits.get((track_name, artist_name), (None, 0.0)) + + +class _FakeProfileDB: + def __init__(self, profiles): + self._profiles = profiles + + def get_all_profiles(self): + return list(self._profiles) + + +class _FakeConfig: + def __init__(self, server='plex'): + self._server = server + + def get_active_media_server(self): + return self._server + + +# --------------------------------------------------------------------------- +# monkeypatch helper — wires fakes into core.downloads.cleanup imports +# --------------------------------------------------------------------------- + +@pytest.fixture +def install(monkeypatch): + def _install(profiles, tracks_per_profile, hits, mark_results=None): + ws = _FakeWishlistService(tracks_per_profile, mark_results) + mdb = _FakeMusicDB(hits) + pdb = _FakeProfileDB(profiles) + + # Patch the in-function imports + import core.wishlist_service as wls_mod + import database.music_database as mdb_mod + monkeypatch.setattr(wls_mod, 'get_wishlist_service', lambda: ws) + monkeypatch.setattr(mdb_mod, 'MusicDatabase', lambda: mdb) + monkeypatch.setattr(mdb_mod, 'get_database', lambda: pdb) + + return ws, mdb, pdb + return _install + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + +def test_no_wishlist_tracks_returns_early_with_no_marks(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + ws, _, _ = install(profiles=[{'id': 1}], tracks_per_profile={1: []}, hits={}) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [] + + +def test_track_found_in_db_gets_removed(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', + 'name': 'Money', + 'artists': ['Pink Floyd'], + 'album': {'name': 'DSOTM'}, + } + ws, mdb, _ = install( + profiles=[{'id': 1}], + tracks_per_profile={1: [track]}, + hits={('Money', 'Pink Floyd'): (object(), 0.95)}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [('sp-1', True)] + + +def test_track_not_in_db_stays_in_wishlist(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', + 'name': 'Phantom', + 'artists': ['Nobody'], + 'album': 'NoAlbum', + } + ws, _, _ = install( + profiles=[{'id': 1}], + tracks_per_profile={1: [track]}, + hits={}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [] + + +def test_low_confidence_match_does_not_remove(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = {'spotify_track_id': 'sp-1', 'name': 'Money', 'artists': ['Pink Floyd']} + # Confidence below 0.7 threshold + ws, _, _ = install( + profiles=[{'id': 1}], + tracks_per_profile={1: [track]}, + hits={('Money', 'Pink Floyd'): (object(), 0.5)}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [] + + +def test_skip_when_missing_essential_fields(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + tracks = [ + {'spotify_track_id': 'sp-1', 'name': '', 'artists': ['A']}, # no name + {'spotify_track_id': 'sp-2', 'name': 'X', 'artists': []}, # no artists + {'name': 'Y', 'artists': ['B']}, # no track_id + ] + ws, mdb, _ = install(profiles=[{'id': 1}], tracks_per_profile={1: tracks}, hits={}) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert mdb.check_calls == [] + assert ws.mark_calls == [] + + +def test_artist_dict_format_normalized(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', + 'name': 'X', + 'artists': [{'name': 'Aretha'}], # dict format + } + ws, mdb, _ = install( + profiles=[{'id': 1}], + tracks_per_profile={1: [track]}, + hits={('X', 'Aretha'): (object(), 0.9)}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [('sp-1', True)] + + +def test_breaks_on_first_artist_match(install): + """Multi-artist track stops checking after first hit.""" + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', + 'name': 'X', + 'artists': ['First', 'Second'], + } + ws, mdb, _ = install( + profiles=[{'id': 1}], + tracks_per_profile={1: [track]}, + hits={('X', 'First'): (object(), 0.9)}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert ws.mark_calls == [('sp-1', True)] + # Only first artist checked + assert mdb.check_calls == [('X', 'First', 'plex', None)] + + +def test_walks_all_profiles(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track_p1 = {'spotify_track_id': 'sp-p1', 'name': 'A', 'artists': ['X']} + track_p2 = {'spotify_track_id': 'sp-p2', 'name': 'B', 'artists': ['Y']} + ws, mdb, _ = install( + profiles=[{'id': 1}, {'id': 2}], + tracks_per_profile={1: [track_p1], 2: [track_p2]}, + hits={('A', 'X'): (object(), 0.9), ('B', 'Y'): (object(), 0.9)}, + ) + cleanup_wishlist_after_db_update(_FakeConfig()) + marked = {c[0] for c in ws.mark_calls} + assert marked == {'sp-p1', 'sp-p2'} + + +def test_album_string_form_passed_through(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', 'name': 'X', 'artists': ['A'], + 'album': 'StringAlbumName', # not a dict + } + ws, mdb, _ = install(profiles=[{'id': 1}], tracks_per_profile={1: [track]}, hits={}) + cleanup_wishlist_after_db_update(_FakeConfig()) + # check_track_exists should receive the string album + assert mdb.check_calls[0] == ('X', 'A', 'plex', 'StringAlbumName') + + +def test_album_dict_form_uses_name(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = { + 'spotify_track_id': 'sp-1', 'name': 'X', 'artists': ['A'], + 'album': {'name': 'DSOTM'}, + } + _, mdb, _ = install(profiles=[{'id': 1}], tracks_per_profile={1: [track]}, hits={}) + cleanup_wishlist_after_db_update(_FakeConfig()) + assert mdb.check_calls[0] == ('X', 'A', 'plex', 'DSOTM') + + +def test_db_check_failure_continues_to_next_artist(install, monkeypatch): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = {'spotify_track_id': 'sp-1', 'name': 'X', 'artists': ['Bad', 'Good']} + + class _ExplodingDB: + def __init__(self): + self.calls = 0 + + def check_track_exists(self, track_name, artist_name, **kw): + self.calls += 1 + if artist_name == 'Bad': + raise RuntimeError("db boom") + return (object(), 0.9) + + edb = _ExplodingDB() + import database.music_database as mdb_mod + import core.wishlist_service as wls_mod + ws = _FakeWishlistService({1: [track]}) + monkeypatch.setattr(wls_mod, 'get_wishlist_service', lambda: ws) + monkeypatch.setattr(mdb_mod, 'MusicDatabase', lambda: edb) + monkeypatch.setattr(mdb_mod, 'get_database', lambda: _FakeProfileDB([{'id': 1}])) + + cleanup_wishlist_after_db_update(_FakeConfig()) + # Both artists tried; second match removed track + assert edb.calls == 2 + assert ws.mark_calls == [('sp-1', True)] + + +def test_top_level_exception_swallowed(monkeypatch): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + import core.wishlist_service as wls_mod + + def boom(): + raise RuntimeError("service init dead") + + monkeypatch.setattr(wls_mod, 'get_wishlist_service', boom) + # Must not raise + cleanup_wishlist_after_db_update(_FakeConfig()) + + +def test_uses_active_server_from_config(install): + from core.downloads.cleanup import cleanup_wishlist_after_db_update + track = {'spotify_track_id': 'sp-1', 'name': 'X', 'artists': ['A']} + _, mdb, _ = install(profiles=[{'id': 1}], tracks_per_profile={1: [track]}, hits={}) + cleanup_wishlist_after_db_update(_FakeConfig(server='jellyfin')) + assert mdb.check_calls[0][2] == 'jellyfin' diff --git a/web_server.py b/web_server.py index 3c5f89e0..b37a2d9a 100644 --- a/web_server.py +++ b/web_server.py @@ -17272,89 +17272,13 @@ def _check_and_remove_track_from_wishlist_by_metadata(track_data): traceback.print_exc() return False +# Auto-wishlist cleanup logic lives in core/downloads/cleanup.py. +from core.downloads import cleanup as _downloads_cleanup + + def _automatic_wishlist_cleanup_after_db_update(): - """ - Automatic wishlist cleanup that runs after database updates. - This is a simplified version of the cleanup API endpoint designed for background execution. - """ - try: - from core.wishlist_service import get_wishlist_service - from database.music_database import MusicDatabase - - wishlist_service = get_wishlist_service() - db = MusicDatabase() - active_server = config_manager.get_active_media_server() - - logger.info("[Auto Cleanup] Starting automatic wishlist cleanup after database update...") - - # Get all wishlist tracks (across all profiles - cleanup is global) - database = get_database() - all_profiles = database.get_all_profiles() - wishlist_tracks = [] - for p in all_profiles: - wishlist_tracks.extend(wishlist_service.get_wishlist_tracks_for_download(profile_id=p['id'])) - if not wishlist_tracks: - logger.warning("[Auto Cleanup] No tracks in wishlist to clean up") - return - - logger.info(f"[Auto Cleanup] Found {len(wishlist_tracks)} tracks in wishlist") - - removed_count = 0 - - for track in wishlist_tracks: - track_name = track.get('name', '') - artists = track.get('artists', []) - spotify_track_id = track.get('spotify_track_id') or track.get('id') - track_album = track.get('album', {}).get('name') if isinstance(track.get('album'), dict) else track.get('album') - - # Skip if no essential data - if not track_name or not artists or not spotify_track_id: - continue - - # Check each artist - found_in_db = False - for artist in artists: - # Handle both string format and dict format - if isinstance(artist, str): - artist_name = artist - elif isinstance(artist, dict) and 'name' in artist: - artist_name = artist['name'] - else: - artist_name = str(artist) - - try: - db_track, confidence = db.check_track_exists( - track_name, artist_name, - confidence_threshold=0.7, - server_source=active_server, - album=track_album - ) - - if db_track and confidence >= 0.7: - found_in_db = True - logger.info(f"[Auto Cleanup] Track found in database: '{track_name}' by {artist_name} (confidence: {confidence:.2f})") - break - - except Exception as db_error: - logger.error(f"[Auto Cleanup] Error checking database for track '{track_name}': {db_error}") - continue - - # If found in database, remove from wishlist - if found_in_db: - try: - removed = wishlist_service.mark_track_download_result(spotify_track_id, success=True) - if removed: - removed_count += 1 - logger.info(f"[Auto Cleanup] Removed track from wishlist: '{track_name}' ({spotify_track_id})") - except Exception as remove_error: - logger.error(f"[Auto Cleanup] Error removing track from wishlist: {remove_error}") - - logger.info(f"[Auto Cleanup] Completed automatic cleanup: {removed_count} tracks removed from wishlist") - - except Exception as e: - logger.error(f"[Auto Cleanup] Error in automatic wishlist cleanup: {e}") - import traceback - traceback.print_exc() + """Automatic wishlist cleanup that runs after database updates.""" + _downloads_cleanup.cleanup_wishlist_after_db_update(config_manager) # ── Update detection ───────────────────────────────────────────── _GITHUB_REPO = "Nezreka/SoulSync"