PR4c: lift _automatic_wishlist_cleanup_after_db_update to core/downloads/cleanup.py
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.
This commit is contained in:
parent
b02a06782e
commit
039f152f31
3 changed files with 372 additions and 82 deletions
100
core/downloads/cleanup.py
Normal file
100
core/downloads/cleanup.py
Normal file
|
|
@ -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()
|
||||
266
tests/downloads/test_downloads_cleanup.py
Normal file
266
tests/downloads/test_downloads_cleanup.py
Normal file
|
|
@ -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'
|
||||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Reference in a new issue