Tighten metadata and import safety

- Normalize album import track display handling so queue labels and match rows stay consistent
- Bound MusicBrainz caches and avoid caching transient lookup failures
- Stop swallowing programmer errors in source enrichment helpers
- Restore import config test seams without reintroducing lazy imports
- Guard task completion calls and fix the Windows path test expectation
- Keep file lock tracking from growing without bound
This commit is contained in:
Antti Kettunen 2026-04-27 20:25:01 +03:00
parent 9315e74bea
commit 02305096a3
No known key found for this signature in database
GPG key ID: C6B2A3D250359BD7
10 changed files with 477 additions and 298 deletions

View file

@ -9,12 +9,9 @@ import subprocess
import time import time
from pathlib import Path from pathlib import Path
logger = logging.getLogger("imports.file_ops")
def _get_config_manager():
from config.settings import config_manager from config.settings import config_manager
return config_manager
logger = logging.getLogger("imports.file_ops")
def safe_move_file(src, dst): def safe_move_file(src, dst):
@ -176,7 +173,6 @@ def downsample_hires_flac(final_path, context):
"""Downsample a hi-res FLAC to 16-bit/44.1kHz if enabled.""" """Downsample a hi-res FLAC to 16-bit/44.1kHz if enabled."""
from mutagen.flac import FLAC from mutagen.flac import FLAC
config_manager = _get_config_manager()
if not config_manager.get("lossy_copy.downsample_hires", False): if not config_manager.get("lossy_copy.downsample_hires", False):
return None return None
@ -287,7 +283,6 @@ def create_lossy_copy(final_path):
"""Convert a FLAC file to a lossy copy using the configured codec.""" """Convert a FLAC file to a lossy copy using the configured codec."""
from mutagen.flac import FLAC from mutagen.flac import FLAC
config_manager = _get_config_manager()
if not config_manager.get("lossy_copy.enabled", False): if not config_manager.get("lossy_copy.enabled", False):
return None return None

View file

@ -8,6 +8,7 @@ from datetime import datetime
from pathlib import Path from pathlib import Path
from typing import Any, Dict, Optional from typing import Any, Dict, Optional
from config.settings import config_manager
from core.imports.context import ( from core.imports.context import (
get_import_clean_artist, get_import_clean_artist,
get_import_clean_title, get_import_clean_title,
@ -25,15 +26,12 @@ logger = get_logger("imports.guards")
def _get_config_manager(): def _get_config_manager():
from config.settings import config_manager
return config_manager return config_manager
def move_to_quarantine(file_path: str, context: dict, reason: str, automation_engine=None) -> str: def move_to_quarantine(file_path: str, context: dict, reason: str, automation_engine=None) -> str:
"""Move a file to the quarantine folder and write a metadata sidecar.""" """Move a file to the quarantine folder and write a metadata sidecar."""
config_manager = _get_config_manager() download_dir = _get_config_manager().get("soulseek.download_path", "./downloads")
download_dir = config_manager.get("soulseek.download_path", "./downloads")
quarantine_dir = Path(download_dir) / "ss_quarantine" quarantine_dir = Path(download_dir) / "ss_quarantine"
quarantine_dir.mkdir(parents=True, exist_ok=True) quarantine_dir.mkdir(parents=True, exist_ok=True)
@ -95,7 +93,6 @@ def check_flac_bit_depth(file_path: str, context: dict) -> Optional[str]:
if not context.get("_audio_quality", "").startswith("FLAC"): if not context.get("_audio_quality", "").startswith("FLAC"):
return None return None
config_manager = _get_config_manager()
quality_profile = MusicDatabase().get_quality_profile() quality_profile = MusicDatabase().get_quality_profile()
flac_config = quality_profile.get("qualities", {}).get("flac", {}) flac_config = quality_profile.get("qualities", {}).get("flac", {})
flac_pref = flac_config.get("bit_depth", "any") flac_pref = flac_config.get("bit_depth", "any")
@ -107,7 +104,7 @@ def check_flac_bit_depth(file_path: str, context: dict) -> Optional[str]:
return None return None
flac_fallback = flac_config.get("bit_depth_fallback", True) flac_fallback = flac_config.get("bit_depth_fallback", True)
downsample_enabled = config_manager.get("lossy_copy.downsample_hires", False) downsample_enabled = _get_config_manager().get("lossy_copy.downsample_hires", False)
track_info = context.get("track_info", {}) track_info = context.get("track_info", {})
track_name = track_info.get("name", os.path.basename(file_path)) track_name = track_info.get("name", os.path.basename(file_path))

View file

@ -7,6 +7,7 @@ import json
import os import os
from typing import Any, Dict, List, Optional from typing import Any, Dict, List, Optional
from config.settings import config_manager
from core.imports.context import ( from core.imports.context import (
extract_artist_name, extract_artist_name,
get_import_clean_album, get_import_clean_album,
@ -31,8 +32,6 @@ logger = get_logger("imports.side_effects")
def _get_config_manager(): def _get_config_manager():
from config.settings import config_manager
return config_manager return config_manager
@ -222,8 +221,7 @@ def record_download_provenance(context: Dict[str, Any]) -> None:
def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[str, Any], album_info: Dict[str, Any]) -> None: def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[str, Any], album_info: Dict[str, Any]) -> None:
"""Write imported media to the SoulSync library tables when the active server is SoulSync.""" """Write imported media to the SoulSync library tables when the active server is SoulSync."""
try: try:
config_manager = _get_config_manager() if _get_config_manager().get_active_media_server() != "soulsync":
if config_manager.get_active_media_server() != "soulsync":
return return
context = normalize_import_context(context) context = normalize_import_context(context)
@ -288,7 +286,7 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
if genres: if genres:
from core.genre_filter import filter_genres as _filter_genres from core.genre_filter import filter_genres as _filter_genres
genres = _filter_genres(genres, config_manager) genres = _filter_genres(genres, _get_config_manager())
genres_json = json.dumps(genres) if genres else "" genres_json = json.dumps(genres) if genres else ""
bitrate = 0 bitrate = 0

View file

@ -4,8 +4,9 @@ from __future__ import annotations
import os import os
import threading import threading
import weakref
from types import SimpleNamespace from types import SimpleNamespace
from typing import Any, Dict from typing import Any
from utils.logging_config import get_logger as _create_logger from utils.logging_config import get_logger as _create_logger
@ -26,7 +27,7 @@ __all__ = [
"wipe_source_tags", "wipe_source_tags",
] ]
_FILE_LOCKS: Dict[str, threading.Lock] = {} _FILE_LOCKS: "weakref.WeakValueDictionary[str, threading.Lock]" = weakref.WeakValueDictionary()
_FILE_LOCKS_LOCK = threading.Lock() _FILE_LOCKS_LOCK = threading.Lock()
@ -117,6 +118,8 @@ def get_mutagen_symbols():
def get_file_lock(file_path: str) -> threading.Lock: def get_file_lock(file_path: str) -> threading.Lock:
# Keep a per-path lock while it is actively referenced, but let it
# fall out of the cache once nobody is using it anymore.
with _FILE_LOCKS_LOCK: with _FILE_LOCKS_LOCK:
lock = _FILE_LOCKS.get(file_path) lock = _FILE_LOCKS.get(file_path)
if lock is None: if lock is None:

View file

@ -3,10 +3,14 @@
from __future__ import annotations from __future__ import annotations
import re import re
import socket
import threading import threading
import time import time
from collections import OrderedDict
from typing import Any, Dict from typing import Any, Dict
import requests
from core.imports.context import ( from core.imports.context import (
extract_artist_name, extract_artist_name,
get_import_clean_artist, get_import_clean_artist,
@ -38,12 +42,17 @@ __all__ = [
"mb_release_detail_cache_lock", "mb_release_detail_cache_lock",
] ]
mb_release_cache: Dict[tuple, str] = {} _MB_RELEASE_CACHE_MAX_ENTRIES = 4096
_MB_RELEASE_DETAIL_CACHE_MAX_ENTRIES = 4096
mb_release_cache: "OrderedDict[tuple, str]" = OrderedDict()
mb_release_cache_lock = threading.RLock() mb_release_cache_lock = threading.RLock()
mb_release_detail_cache: Dict[str, Dict[str, Any]] = {} mb_release_detail_cache: "OrderedDict[str, Dict[str, Any]]" = OrderedDict()
mb_release_detail_cache_lock = threading.RLock() mb_release_detail_cache_lock = threading.RLock()
logger = _create_logger("metadata.source") logger = _create_logger("metadata.source")
_SOURCE_NETWORK_EXCEPTIONS = (requests.RequestException, socket.timeout, TimeoutError)
_EDITION_PAREN_RE = re.compile( _EDITION_PAREN_RE = re.compile(
r'\s*[\(\[]\s*(?:deluxe|expanded|remaster(?:ed)?|anniversary|special|collector|' r'\s*[\(\[]\s*(?:deluxe|expanded|remaster(?:ed)?|anniversary|special|collector|'
r'limited|bonus|platinum|gold|super\s*deluxe|standard)' r'limited|bonus|platinum|gold|super\s*deluxe|standard)'
@ -64,6 +73,29 @@ def normalize_album_cache_key(album_name: str) -> str:
return result.lower().strip() return result.lower().strip()
def _bounded_cache_get(cache, key):
value = cache.get(key)
if value is not None and hasattr(cache, "move_to_end"):
cache.move_to_end(key)
return value
def _bounded_cache_set(cache, key, value, max_entries: int) -> None:
cache[key] = value
if hasattr(cache, "move_to_end"):
cache.move_to_end(key)
while len(cache) > max_entries:
cache.popitem(last=False)
def _call_source_lookup(label: str, func, *args, **kwargs):
try:
return func(*args, **kwargs)
except _SOURCE_NETWORK_EXCEPTIONS as exc:
logger.warning("%s lookup failed (network): %s", label, exc)
return None
SOURCE_TAG_CONFIG = { SOURCE_TAG_CONFIG = {
"SPOTIFY_TRACK_ID": "spotify.tags.track_id", "SPOTIFY_TRACK_ID": "spotify.tags.track_id",
"SPOTIFY_ARTIST_ID": "spotify.tags.artist_id", "SPOTIFY_ARTIST_ID": "spotify.tags.artist_id",
@ -194,12 +226,16 @@ def _process_musicbrainz_source(pp: dict, metadata: dict, cfg, runtime, track_ti
if not mb_service: if not mb_service:
return return
try: result = _call_source_lookup("MusicBrainz recording", mb_service.match_recording, track_title, artist_name)
result = mb_service.match_recording(track_title, artist_name)
if result and result.get("mbid"): if result and result.get("mbid"):
pp["recording_mbid"] = result["mbid"] pp["recording_mbid"] = result["mbid"]
pp["id_tags"]["MUSICBRAINZ_RECORDING_ID"] = pp["recording_mbid"] pp["id_tags"]["MUSICBRAINZ_RECORDING_ID"] = pp["recording_mbid"]
details = mb_service.mb_client.get_recording(pp["recording_mbid"], includes=["isrcs", "genres"]) details = _call_source_lookup(
"MusicBrainz recording details",
mb_service.mb_client.get_recording,
pp["recording_mbid"],
includes=["isrcs", "genres"],
)
if details: if details:
isrcs = details.get("isrcs", []) isrcs = details.get("isrcs", [])
if isrcs: if isrcs:
@ -209,7 +245,7 @@ def _process_musicbrainz_source(pp: dict, metadata: dict, cfg, runtime, track_ti
track_artist_name = metadata.get("artist", "") or artist_name track_artist_name = metadata.get("artist", "") or artist_name
if ", " in track_artist_name: if ", " in track_artist_name:
track_artist_name = track_artist_name.split(", ")[0] track_artist_name = track_artist_name.split(", ")[0]
artist_result = mb_service.match_artist(track_artist_name) artist_result = _call_source_lookup("MusicBrainz artist", mb_service.match_artist, track_artist_name)
if artist_result and artist_result.get("mbid"): if artist_result and artist_result.get("mbid"):
pp["artist_mbid"] = artist_result["mbid"] pp["artist_mbid"] = artist_result["mbid"]
pp["id_tags"]["MUSICBRAINZ_ARTIST_ID"] = pp["artist_mbid"] pp["id_tags"]["MUSICBRAINZ_ARTIST_ID"] = pp["artist_mbid"]
@ -219,33 +255,36 @@ def _process_musicbrainz_source(pp: dict, metadata: dict, cfg, runtime, track_ti
artist_key = (pp.get("batch_artist_name") or artist_name).lower().strip() artist_key = (pp.get("batch_artist_name") or artist_name).lower().strip()
rc_key_norm = (normalize_album_cache_key(album_name_for_mb), artist_key) rc_key_norm = (normalize_album_cache_key(album_name_for_mb), artist_key)
rc_key_exact = (album_name_for_mb.lower().strip(), artist_key) rc_key_exact = (album_name_for_mb.lower().strip(), artist_key)
release_mbid = None
with mb_release_cache_lock: with mb_release_cache_lock:
cached = mb_release_cache.get(rc_key_norm) cached = _bounded_cache_get(mb_release_cache, rc_key_norm)
if cached is None: if cached is None:
cached = mb_release_cache.get(rc_key_exact) cached = _bounded_cache_get(mb_release_cache, rc_key_exact)
if cached is not None: if cached:
pp["release_mbid"] = cached release_mbid = cached
else: else:
try: rc_result = _call_source_lookup("MusicBrainz release", mb_service.match_release, album_name_for_mb, artist_name)
rc_result = mb_service.match_release(album_name_for_mb, artist_name) if rc_result and rc_result.get("mbid"):
pp["release_mbid"] = rc_result.get("mbid", "") if rc_result else "" release_mbid = rc_result["mbid"]
except Exception: if release_mbid:
pp["release_mbid"] = "" _bounded_cache_set(mb_release_cache, rc_key_norm, release_mbid, _MB_RELEASE_CACHE_MAX_ENTRIES)
mb_release_cache[rc_key_norm] = pp["release_mbid"] _bounded_cache_set(mb_release_cache, rc_key_exact, release_mbid, _MB_RELEASE_CACHE_MAX_ENTRIES)
mb_release_cache[rc_key_exact] = pp["release_mbid"] pp["release_mbid"] = release_mbid or ""
if pp["release_mbid"]: if pp["release_mbid"]:
pp["id_tags"]["MUSICBRAINZ_RELEASE_ID"] = pp["release_mbid"] pp["id_tags"]["MUSICBRAINZ_RELEASE_ID"] = pp["release_mbid"]
if pp["release_mbid"]: if pp["release_mbid"]:
with mb_release_detail_cache_lock: with mb_release_detail_cache_lock:
release_detail = mb_release_detail_cache.get(pp["release_mbid"]) release_detail = _bounded_cache_get(mb_release_detail_cache, pp["release_mbid"])
if release_detail is None: if release_detail is None:
release_detail = mb_service.mb_client.get_release( release_detail = _call_source_lookup(
"MusicBrainz release details",
mb_service.mb_client.get_release,
pp["release_mbid"], pp["release_mbid"],
includes=["release-groups", "labels", "media", "artist-credits", "recordings"], includes=["release-groups", "labels", "media", "artist-credits", "recordings"],
) or {} ) or {}
with mb_release_detail_cache_lock: with mb_release_detail_cache_lock:
mb_release_detail_cache[pp["release_mbid"]] = release_detail _bounded_cache_set(mb_release_detail_cache, pp["release_mbid"], release_detail, _MB_RELEASE_DETAIL_CACHE_MAX_ENTRIES)
if release_detail: if release_detail:
rg = release_detail.get("release-group", {}) rg = release_detail.get("release-group", {})
if rg.get("id"): if rg.get("id"):
@ -305,8 +344,6 @@ def _process_musicbrainz_source(pp: dict, metadata: dict, cfg, runtime, track_ti
break break
except (ValueError, TypeError): except (ValueError, TypeError):
pass pass
except Exception as exc:
logger.error("MusicBrainz lookup failed (non-fatal): %s", exc)
def _process_deezer_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_deezer_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -315,19 +352,18 @@ def _process_deezer_source(pp: dict, metadata: dict, cfg, runtime, track_title:
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
deezer_worker = getattr(runtime, "deezer_worker", None) deezer_worker = getattr(runtime, "deezer_worker", None)
dz_client = deezer_worker.client if deezer_worker else None dz_client = deezer_worker.client if deezer_worker else None
if not dz_client: if not dz_client:
return return
dz_result = dz_client.search_track(artist_name, track_title) dz_result = _call_source_lookup("Deezer track", dz_client.search_track, artist_name, track_title)
if dz_result and _names_match(dz_result.get("title", ""), track_title) and _names_match(dz_result.get("artist", {}).get("name", ""), artist_name): if dz_result and _names_match(dz_result.get("title", ""), track_title) and _names_match(dz_result.get("artist", {}).get("name", ""), artist_name):
dz_track_id = dz_result["id"] dz_track_id = dz_result["id"]
pp["id_tags"]["DEEZER_TRACK_ID"] = str(dz_track_id) pp["id_tags"]["DEEZER_TRACK_ID"] = str(dz_track_id)
dz_artist_id = dz_result.get("artist", {}).get("id") dz_artist_id = dz_result.get("artist", {}).get("id")
if dz_artist_id: if dz_artist_id:
pp["id_tags"]["DEEZER_ARTIST_ID"] = str(dz_artist_id) pp["id_tags"]["DEEZER_ARTIST_ID"] = str(dz_artist_id)
dz_details = dz_client.get_track_details(dz_track_id) dz_details = _call_source_lookup("Deezer track details", dz_client.get_track_details, dz_track_id)
if dz_details: if dz_details:
bpm_val = dz_details.get("bpm") bpm_val = dz_details.get("bpm")
if bpm_val and bpm_val > 0: if bpm_val and bpm_val > 0:
@ -340,8 +376,6 @@ def _process_deezer_source(pp: dict, metadata: dict, cfg, runtime, track_title:
dz_release = (dz_album.get("release_date", "") if isinstance(dz_album, dict) else "") or "" dz_release = (dz_album.get("release_date", "") if isinstance(dz_album, dict) else "") or ""
if len(dz_release) >= 4 and dz_release[:4].isdigit(): if len(dz_release) >= 4 and dz_release[:4].isdigit():
pp["release_year"] = dz_release[:4] pp["release_year"] = dz_release[:4]
except Exception as exc:
logger.error("Deezer lookup failed (non-fatal): %s", exc)
def _process_audiodb_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_audiodb_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -350,12 +384,11 @@ def _process_audiodb_source(pp: dict, metadata: dict, cfg, runtime, track_title:
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
audiodb_worker = getattr(runtime, "audiodb_worker", None) audiodb_worker = getattr(runtime, "audiodb_worker", None)
adb_client = audiodb_worker.client if audiodb_worker else None adb_client = audiodb_worker.client if audiodb_worker else None
if not adb_client: if not adb_client:
return return
adb_result = adb_client.search_track(artist_name, track_title) adb_result = _call_source_lookup("AudioDB track", adb_client.search_track, artist_name, track_title)
if adb_result and _names_match(adb_result.get("strTrack", ""), track_title) and _names_match(adb_result.get("strArtist", ""), artist_name): if adb_result and _names_match(adb_result.get("strTrack", ""), track_title) and _names_match(adb_result.get("strArtist", ""), artist_name):
adb_track_id = adb_result.get("idTrack") adb_track_id = adb_result.get("idTrack")
if adb_track_id: if adb_track_id:
@ -371,8 +404,6 @@ def _process_audiodb_source(pp: dict, metadata: dict, cfg, runtime, track_title:
pp["audiodb_mood"] = adb_result.get("strMood") or None pp["audiodb_mood"] = adb_result.get("strMood") or None
pp["audiodb_style"] = adb_result.get("strStyle") or None pp["audiodb_style"] = adb_result.get("strStyle") or None
pp["audiodb_genre"] = adb_result.get("strGenre") or None pp["audiodb_genre"] = adb_result.get("strGenre") or None
except Exception as exc:
logger.error("AudioDB lookup failed (non-fatal): %s", exc)
def _process_tidal_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_tidal_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -381,11 +412,10 @@ def _process_tidal_source(pp: dict, metadata: dict, cfg, runtime, track_title: s
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
tidal_client = getattr(runtime, "tidal_client", None) tidal_client = getattr(runtime, "tidal_client", None)
if not (tidal_client and tidal_client.is_authenticated()): if not (tidal_client and tidal_client.is_authenticated()):
return return
td_result = tidal_client.search_track(artist_name, track_title) td_result = _call_source_lookup("Tidal track", tidal_client.search_track, artist_name, track_title)
if td_result and _names_match(td_result.get("title", ""), track_title): if td_result and _names_match(td_result.get("title", ""), track_title):
td_track_id = td_result.get("id") td_track_id = td_result.get("id")
if td_track_id: if td_track_id:
@ -394,7 +424,7 @@ def _process_tidal_source(pp: dict, metadata: dict, cfg, runtime, track_title: s
if isinstance(td_artist, dict) and td_artist.get("id"): if isinstance(td_artist, dict) and td_artist.get("id"):
pp["id_tags"]["TIDAL_ARTIST_ID"] = str(td_artist["id"]) pp["id_tags"]["TIDAL_ARTIST_ID"] = str(td_artist["id"])
if td_track_id: if td_track_id:
td_details = tidal_client.get_track(str(td_track_id)) td_details = _call_source_lookup("Tidal track details", tidal_client.get_track, str(td_track_id))
if td_details: if td_details:
pp["tidal_isrc"] = td_details.get("isrc") pp["tidal_isrc"] = td_details.get("isrc")
td_copyright = td_details.get("copyright") td_copyright = td_details.get("copyright")
@ -408,8 +438,6 @@ def _process_tidal_source(pp: dict, metadata: dict, cfg, runtime, track_title: s
td_release = str(td_album.get("release_date", "") or td_album.get("releaseDate", "") or "") td_release = str(td_album.get("release_date", "") or td_album.get("releaseDate", "") or "")
if len(td_release) >= 4 and td_release[:4].isdigit(): if len(td_release) >= 4 and td_release[:4].isdigit():
pp["release_year"] = td_release[:4] pp["release_year"] = td_release[:4]
except Exception as exc:
logger.error("Tidal lookup failed (non-fatal): %s", exc)
def _process_qobuz_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_qobuz_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -418,12 +446,11 @@ def _process_qobuz_source(pp: dict, metadata: dict, cfg, runtime, track_title: s
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
qobuz_worker = getattr(runtime, "qobuz_enrichment_worker", None) qobuz_worker = getattr(runtime, "qobuz_enrichment_worker", None)
qz_client = qobuz_worker.client if qobuz_worker else None qz_client = qobuz_worker.client if qobuz_worker else None
if not (qz_client and qz_client.is_authenticated()): if not (qz_client and qz_client.is_authenticated()):
return return
qz_result = qz_client.search_track(artist_name, track_title) qz_result = _call_source_lookup("Qobuz track", qz_client.search_track, artist_name, track_title)
if qz_result: if qz_result:
qz_performer = qz_result.get("performer") or {} qz_performer = qz_result.get("performer") or {}
if not isinstance(qz_performer, dict): if not isinstance(qz_performer, dict):
@ -460,8 +487,6 @@ def _process_qobuz_source(pp: dict, metadata: dict, cfg, runtime, track_title: s
qz_release = str(_dt.datetime.utcfromtimestamp(qz_ts).year) qz_release = str(_dt.datetime.utcfromtimestamp(qz_ts).year)
if len(qz_release) >= 4 and qz_release[:4].isdigit(): if len(qz_release) >= 4 and qz_release[:4].isdigit():
pp["release_year"] = qz_release[:4] pp["release_year"] = qz_release[:4]
except Exception as exc:
logger.error("Qobuz lookup failed (non-fatal): %s", exc)
def _process_lastfm_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_lastfm_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -470,12 +495,11 @@ def _process_lastfm_source(pp: dict, metadata: dict, cfg, runtime, track_title:
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
lastfm_worker = getattr(runtime, "lastfm_worker", None) lastfm_worker = getattr(runtime, "lastfm_worker", None)
lf_client = lastfm_worker.client if lastfm_worker else None lf_client = lastfm_worker.client if lastfm_worker else None
if not lf_client: if not lf_client:
return return
lf_result = lf_client.get_track_info(artist_name, track_title) lf_result = _call_source_lookup("Last.fm track", lf_client.get_track_info, artist_name, track_title)
if lf_result: if lf_result:
lf_url = lf_result.get("url") lf_url = lf_result.get("url")
if lf_url: if lf_url:
@ -487,8 +511,6 @@ def _process_lastfm_source(pp: dict, metadata: dict, cfg, runtime, track_title:
pp["lastfm_tags"] = [tag.get("name", "") for tag in tag_list if isinstance(tag, dict) and tag.get("name")] pp["lastfm_tags"] = [tag.get("name", "") for tag in tag_list if isinstance(tag, dict) and tag.get("name")]
elif isinstance(tag_list, dict) and tag_list.get("name"): elif isinstance(tag_list, dict) and tag_list.get("name"):
pp["lastfm_tags"] = [tag_list["name"]] pp["lastfm_tags"] = [tag_list["name"]]
except Exception as exc:
logger.error("Last.fm lookup failed (non-fatal): %s", exc)
def _process_genius_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_genius_source(pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:
@ -497,7 +519,6 @@ def _process_genius_source(pp: dict, metadata: dict, cfg, runtime, track_title:
if not track_title or not artist_name: if not track_title or not artist_name:
return return
try:
import core.genius_client as _genius_module import core.genius_client as _genius_module
if time.time() < _genius_module._rate_limit_until: if time.time() < _genius_module._rate_limit_until:
@ -507,7 +528,7 @@ def _process_genius_source(pp: dict, metadata: dict, cfg, runtime, track_title:
g_client = genius_worker.client if genius_worker else None g_client = genius_worker.client if genius_worker else None
if not g_client: if not g_client:
return return
g_result = g_client.search_song(artist_name, track_title) g_result = _call_source_lookup("Genius track", g_client.search_song, artist_name, track_title)
if g_result: if g_result:
g_id = g_result.get("id") g_id = g_result.get("id")
if g_id: if g_id:
@ -515,8 +536,6 @@ def _process_genius_source(pp: dict, metadata: dict, cfg, runtime, track_title:
g_url = g_result.get("url") g_url = g_result.get("url")
if g_url: if g_url:
pp["genius_url"] = g_url pp["genius_url"] = g_url
except Exception as exc:
logger.error("Genius lookup failed (non-fatal): %s", exc)
def _process_source_enrichment(source_name: str, pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None: def _process_source_enrichment(source_name: str, pp: dict, metadata: dict, cfg, runtime, track_title: str, artist_name: str) -> None:

View file

@ -4,6 +4,7 @@ from __future__ import annotations
import threading import threading
import time import time
from functools import wraps
from typing import Any, Dict, Optional from typing import Any, Dict, Optional
matched_context_lock = threading.Lock() matched_context_lock = threading.Lock()
@ -20,6 +21,17 @@ activity_feed_lock = threading.Lock()
_activity_toast_emitter = None _activity_toast_emitter = None
def caller_must_hold_tasks_lock(func):
"""Best-effort guard for helpers that mutate download_tasks in place."""
@wraps(func)
def wrapper(*args, **kwargs):
if not tasks_lock.locked():
raise RuntimeError(f"{func.__name__}() requires tasks_lock to be held by the caller")
return func(*args, **kwargs)
return wrapper
def set_activity_toast_emitter(emitter) -> None: def set_activity_toast_emitter(emitter) -> None:
"""Set the WebSocket-style emitter used by add_activity_item.""" """Set the WebSocket-style emitter used by add_activity_item."""
global _activity_toast_emitter global _activity_toast_emitter
@ -50,6 +62,7 @@ def add_activity_item(icon, title, subtitle, time_ago="Now", show_toast=True):
return activity_item return activity_item
@caller_must_hold_tasks_lock
def mark_task_completed(task_id: str, track_info: Optional[Dict[str, Any]] = None) -> bool: def mark_task_completed(task_id: str, track_info: Optional[Dict[str, Any]] = None) -> bool:
"""Mark a download task as completed. """Mark a download task as completed.

View file

@ -1,3 +1,5 @@
import os
import core.imports.album_naming as album_naming import core.imports.album_naming as album_naming
import core.imports.paths as import_paths import core.imports.paths as import_paths
@ -69,7 +71,7 @@ def test_get_file_path_from_template_raw_handles_quality_and_disc_placeholders(m
}, },
) )
assert folder_path == "Artist One/Album One" assert folder_path == os.path.join("Artist One", "Album One")
assert filename == "3 - Song One [FLAC 16bit]" assert filename == "3 - Song One [FLAC 16bit]"

View file

@ -1,6 +1,10 @@
from collections import OrderedDict
import types import types
import sqlite3 import sqlite3
import pytest
import requests
from core.metadata import enrichment as me from core.metadata import enrichment as me
from core.metadata import artwork as ma from core.metadata import artwork as ma
from core.metadata import source as ms from core.metadata import source as ms
@ -350,6 +354,109 @@ def test_embed_source_ids_writes_musicbrainz_release_year_and_updates_album_year
assert row["year"] == 2021 assert row["year"] == 2021
def test_musicbrainz_release_lookup_failure_does_not_poison_cache(monkeypatch):
class _FakeMBClient:
def get_release(self, mbid, includes=None):
return {}
class _FakeMBService:
def __init__(self):
self.release_calls = 0
self.mb_client = _FakeMBClient()
def match_recording(self, title, artist):
return None
def match_artist(self, artist):
return {"mbid": "artist-mbid"}
def match_release(self, album, artist):
self.release_calls += 1
if self.release_calls == 1:
raise requests.RequestException("temporary MusicBrainz outage")
return {"mbid": "release-mbid"}
monkeypatch.setattr(ms, "get_config_manager", lambda: _Config({"musicbrainz.embed_tags": True}))
monkeypatch.setattr(ms, "mb_release_cache", {})
monkeypatch.setattr(ms, "mb_release_detail_cache", {})
service = _FakeMBService()
runtime = types.SimpleNamespace(mb_worker=types.SimpleNamespace(mb_service=service))
pp = {
"id_tags": {},
"track_title": "Song One",
"artist_name": "Artist One",
"batch_artist_name": "Artist One",
"metadata": {"album": "Album One"},
"recording_mbid": None,
"artist_mbid": None,
"release_mbid": "",
"mb_genres": [],
"isrc": None,
"deezer_bpm": None,
"deezer_isrc": None,
"audiodb_mood": None,
"audiodb_style": None,
"audiodb_genre": None,
"tidal_isrc": None,
"tidal_copyright": None,
"qobuz_isrc": None,
"qobuz_copyright": None,
"qobuz_label": None,
"lastfm_tags": [],
"lastfm_url": None,
"genius_url": None,
"release_year": None,
}
metadata = {"album": "Album One", "artist": "Artist One"}
ms._process_musicbrainz_source(pp, metadata, _Config({"musicbrainz.embed_tags": True}), runtime, "Song One", "Artist One")
assert service.release_calls == 1
assert ms.mb_release_cache == {}
poisoned_norm_key = (ms.normalize_album_cache_key("Album One"), "artist one")
poisoned_exact_key = ("album one", "artist one")
ms.mb_release_cache[poisoned_norm_key] = ""
ms.mb_release_cache[poisoned_exact_key] = ""
ms._process_musicbrainz_source(pp, metadata, _Config({"musicbrainz.embed_tags": True}), runtime, "Song One", "Artist One")
assert service.release_calls == 2
def test_source_processors_do_not_swallow_programmer_errors():
class _BoomClient:
def search_track(self, artist_name, track_title):
raise ValueError("boom")
runtime = types.SimpleNamespace(deezer_worker=types.SimpleNamespace(client=_BoomClient()))
pp = {
"id_tags": {},
"batch_artist_name": "Artist One",
"release_year": None,
}
metadata = {"album": "Album One"}
with pytest.raises(ValueError):
ms._process_deezer_source(pp, metadata, _Config({"deezer.embed_tags": True}), runtime, "Song One", "Artist One")
def test_musicbrainz_caches_evict_oldest_entries():
release_cache = OrderedDict()
detail_cache = OrderedDict()
ms._bounded_cache_set(release_cache, ("album-1", "artist"), "release-1", 2)
ms._bounded_cache_set(release_cache, ("album-2", "artist"), "release-2", 2)
assert list(release_cache.keys()) == [("album-1", "artist"), ("album-2", "artist")]
assert ms._bounded_cache_get(release_cache, ("album-1", "artist")) == "release-1"
ms._bounded_cache_set(release_cache, ("album-3", "artist"), "release-3", 2)
assert list(release_cache.keys()) == [("album-1", "artist"), ("album-3", "artist")]
ms._bounded_cache_set(detail_cache, "release-1", {"title": "One"}, 1)
ms._bounded_cache_set(detail_cache, "release-2", {"title": "Two"}, 1)
assert list(detail_cache.keys()) == ["release-2"]
def test_enhance_file_metadata_forwards_runtime_to_source_embedding(monkeypatch): def test_enhance_file_metadata_forwards_runtime_to_source_embedding(monkeypatch):
audio = _FakeAudio() audio = _FakeAudio()
symbols = _fake_symbols(audio) symbols = _fake_symbols(audio)

View file

@ -0,0 +1,32 @@
import pytest
import core.runtime_state as runtime_state
def test_mark_task_completed_requires_tasks_lock():
original_tasks = dict(runtime_state.download_tasks)
runtime_state.download_tasks.clear()
runtime_state.download_tasks["task-1"] = {"status": "running", "stream_processed": False}
try:
with pytest.raises(RuntimeError, match="tasks_lock"):
runtime_state.mark_task_completed("task-1")
finally:
runtime_state.download_tasks.clear()
runtime_state.download_tasks.update(original_tasks)
def test_mark_task_completed_succeeds_when_lock_held():
original_tasks = dict(runtime_state.download_tasks)
runtime_state.download_tasks.clear()
runtime_state.download_tasks["task-1"] = {"status": "running", "stream_processed": False}
try:
with runtime_state.tasks_lock:
assert runtime_state.mark_task_completed("task-1", {"name": "Song One"}) is True
assert runtime_state.download_tasks["task-1"]["status"] == "completed"
assert runtime_state.download_tasks["task-1"]["stream_processed"] is True
assert runtime_state.download_tasks["task-1"]["track_info"] == {"name": "Song One"}
finally:
runtime_state.download_tasks.clear()
runtime_state.download_tasks.update(original_tasks)

View file

@ -1214,9 +1214,7 @@ function importPageRenderMatchList() {
// Build rows // Build rows
let matchedCount = 0; let matchedCount = 0;
const rows = data.matches.map((m, idx) => { const rows = data.matches.map((m, idx) => {
const track = m.track || m.spotify_track || {}; const trackInfo = _importPageGetTrackDisplayInfo(m, idx);
const trackNumber = track.track_number ?? track.trackNumber;
const displayTrackNumber = trackNumber ? trackNumber : (idx + 1);
let file = null; let file = null;
let confidence = m.confidence; let confidence = m.confidence;
let isOverride = false; let isOverride = false;
@ -1256,8 +1254,8 @@ function importPageRenderMatchList() {
<div class="import-page-match-row ${file ? 'matched' : ''}" <div class="import-page-match-row ${file ? 'matched' : ''}"
ondragover="importPageHandleDragOver(event)" ondragleave="this.classList.remove('drag-over')" ondrop="importPageHandleDrop(event, ${idx})" ondragover="importPageHandleDragOver(event)" ondragleave="this.classList.remove('drag-over')" ondrop="importPageHandleDrop(event, ${idx})"
onclick="importPageTapAssign(${idx})"> onclick="importPageTapAssign(${idx})">
<span class="import-page-match-num">${displayTrackNumber}</span> <span class="import-page-match-num">${trackInfo.displayTrackNumber}</span>
<span class="import-page-match-track">${_esc(track.name || track.title || 'Unknown Track')}</span> <span class="import-page-match-track">${_esc(trackInfo.name)}</span>
<span class="import-page-match-file ${file ? 'has-file' : ''}"> <span class="import-page-match-file ${file ? 'has-file' : ''}">
${file ${file
? `<span class="import-page-match-file-name">${_esc(file.filename)}</span> ? `<span class="import-page-match-file-name">${_esc(file.filename)}</span>
@ -1307,6 +1305,21 @@ function importPageRenderMatchList() {
processBtn.textContent = `Process ${matchedCount} Track${matchedCount !== 1 ? 's' : ''}`; processBtn.textContent = `Process ${matchedCount} Track${matchedCount !== 1 ? 's' : ''}`;
} }
function _importPageGetTrackDisplayInfo(item, index) {
const track = item?.track || item?.spotify_track || {};
const rawTrackNumber = track.track_number ?? track.trackNumber ?? null;
const trackNumber = rawTrackNumber === null || rawTrackNumber === undefined || rawTrackNumber === ''
? null
: String(rawTrackNumber).split('/')[0].trim();
return {
track,
name: track.name || track.title || `Track ${index + 1}`,
trackNumber,
displayTrackNumber: trackNumber || String(index + 1),
};
}
// --- Album Tab: Drag and Drop --- // --- Album Tab: Drag and Drop ---
function importPageStartDrag(event, stagingFileIndex) { function importPageStartDrag(event, stagingFileIndex) {
@ -1672,7 +1685,7 @@ function _importQueueAdd(job) {
async function _importQueueRunJob(entry, job) { async function _importQueueRunJob(entry, job) {
for (let i = 0; i < job.items.length; i++) { for (let i = 0; i < job.items.length; i++) {
const itemName = job.type === 'album' const itemName = job.type === 'album'
? (job.items[i].spotify_track?.name || `Track ${i + 1}`) ? _importPageGetTrackDisplayInfo(job.items[i], i).name
: (job.items[i].title || job.items[i].filename || `File ${i + 1}`); : (job.items[i].title || job.items[i].filename || `File ${i + 1}`);
// Update status with current track info // Update status with current track info