diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a216db03..dc089154 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.7.3)' + description: 'Version tag (e.g. 2.7.6)' required: true - default: '2.7.3' + default: '2.7.6' jobs: build-and-push: diff --git a/.gitignore b/.gitignore index 8857f8d5..6e78b278 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ __pycache__/ # User-specific files (auto-created by the app if missing) config/config.json +config/youtube_cookies.txt database/music_library.db database/music_library.db-shm database/music_library.db-wal diff --git a/core/deezer_client.py b/core/deezer_client.py index 6f9d96ec..64ca5141 100644 --- a/core/deezer_client.py +++ b/core/deezer_client.py @@ -117,6 +117,48 @@ def _is_full_track_payload(payload: Optional[Dict[str, Any]]) -> bool: return 'track_position' in payload and 'contributors' in payload +def resolve_album_track_positions(session, base_url, album_ids, cache=None, sleep_s=0.2): + """Build ``{str(track_id): track_position}`` for a set of Deezer album ids. + + Deezer PLAYLIST and SEARCH track objects (and even the album object's embedded + ``tracks.data``) omit ``track_position`` — only ``/album//tracks`` and + ``/track/`` carry it. So numbering playlist tracks by their playlist index + silently poisons the real album track number, which then rides onto the + downloaded file's tag. This resolves the authoritative position per album + (cache-first, best-effort — a failed album just isn't in the map).""" + import time as _time + positions: Dict[str, int] = {} + for aid in album_ids: + aid = str(aid) + at_list = None + if cache: + try: + ct = cache.get_entity('deezer', 'album_tracks', aid) + if ct and ct.get('data'): + at_list = ct['data'] + except Exception: # noqa: BLE001 - cache is best-effort + at_list = None + if at_list is None: + try: + if sleep_s: + _time.sleep(sleep_s) # respect Deezer rate limits + r = session.get(f"{base_url}/album/{aid}/tracks", params={'limit': 500}, timeout=10) + if getattr(r, 'ok', False): + at_list = (r.json() or {}).get('data', []) + if cache and at_list is not None: + try: + cache.store_entity('deezer', 'album_tracks', aid, {'data': at_list}) + except Exception as _cache_err: # noqa: BLE001 + logger.debug("album_tracks cache store failed for %s: %s", aid, _cache_err) + except Exception: # noqa: BLE001 - never let metadata resolution break the fetch + at_list = None + for at in (at_list or []): + tp = at.get('track_position') + if at.get('id') and tp: + positions[str(at['id'])] = tp + return positions + + # ==================== Dataclasses (match iTunesClient / SpotifyClient format) ==================== @dataclass @@ -1357,6 +1399,16 @@ class DeezerClient: raw_tracks.extend(page_tracks) + # Real album track positions — playlist tracks don't carry track_position, + # so numbering by playlist index would poison the downloaded file's tag. + album_ids = {str(t.get('album', {}).get('id')) for t in raw_tracks if t.get('album', {}).get('id')} + try: + from core.metadata.cache import get_metadata_cache + _cache = get_metadata_cache() + except Exception: + _cache = None + track_positions = resolve_album_track_positions(self.session, self.BASE_URL, album_ids, _cache) + # Normalize tracks tracks: List[Dict[str, Any]] = [] for i, t in enumerate(raw_tracks, start=1): @@ -1368,7 +1420,8 @@ class DeezerClient: 'artists': [artist_name], 'album': t.get('album', {}).get('title', ''), 'duration_ms': t.get('duration', 0) * 1000, - 'track_number': i, + # REAL album position; the playlist index is a last resort only. + 'track_number': track_positions.get(str(t.get('id'))) or i, }) result = { diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index 622ace52..f2368ac1 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -418,6 +418,12 @@ class DeezerDownloadClient(DownloadSourcePlugin): if aid: album_ids.add(str(aid)) album_release_dates = {} + # Deezer PLAYLIST tracks do NOT carry `track_position` (only `/track/` + # and `/album//tracks` do), so numbering them by their playlist index + # poisons the real album track number — which then rides into the wishlist + # and onto the downloaded file's tag (e.g. 'Apologize' tagged track 1 instead + # of 16). Resolve the REAL position from each album's track list (cache-first). + track_positions: Dict[str, int] = {} # str(track_id) -> album track_position try: from core.metadata.cache import get_metadata_cache cache = get_metadata_cache() @@ -430,24 +436,32 @@ class DeezerDownloadClient(DownloadSourcePlugin): cached = cache.get_entity('deezer', 'album', aid) if cached and cached.get('release_date'): album_release_dates[aid] = cached['release_date'] - continue except Exception as e: logger.debug("cache get_entity album release_date: %s", e) # Cache miss — fetch from API - try: - time.sleep(0.3) # Respect rate limits - a_resp = self._session.get(f'https://api.deezer.com/album/{aid}', timeout=10) - if a_resp.ok: - a_data = a_resp.json() - album_release_dates[aid] = a_data.get('release_date', '') - # Store in metadata cache for future use - if cache: - try: - cache.store_entity('deezer', 'album', aid, a_data) - except Exception as e: - logger.debug("cache store_entity album release_date: %s", e) - except Exception as e: - logger.debug("fetch deezer album release_date %s: %s", aid, e) + if aid not in album_release_dates: + try: + time.sleep(0.3) # Respect rate limits + a_resp = self._session.get(f'https://api.deezer.com/album/{aid}', timeout=10) + if a_resp.ok: + a_data = a_resp.json() + album_release_dates[aid] = a_data.get('release_date', '') + # Store in metadata cache for future use + if cache: + try: + cache.store_entity('deezer', 'album', aid, a_data) + except Exception as e: + logger.debug("cache store_entity album release_date: %s", e) + except Exception as e: + logger.debug("fetch deezer album release_date %s: %s", aid, e) + # Real album track positions (separate endpoint — playlist tracks AND the + # album object's embedded tracks both omit track_position). Cache-first. + try: + from core.deezer_client import resolve_album_track_positions + track_positions = resolve_album_track_positions( + self._session, 'https://api.deezer.com', album_ids, cache) + except Exception as e: + logger.debug("resolve deezer album track positions: %s", e) tracks = [] for i, t in enumerate(raw_tracks, start=1): @@ -468,7 +482,9 @@ class DeezerDownloadClient(DownloadSourcePlugin): 'id': album_id, }, 'duration_ms': t.get('duration', 0) * 1000, - 'track_number': i, + # REAL album position (resolved above); the playlist index is a last + # resort only when the album lookup failed, never the default. + 'track_number': track_positions.get(str(t.get('id'))) or t.get('track_position') or i, }) return { diff --git a/core/discovery/sync.py b/core/discovery/sync.py index aef0acaf..cee281d6 100644 --- a/core/discovery/sync.py +++ b/core/discovery/sync.py @@ -173,6 +173,32 @@ async def _database_only_find_track(spotify_track, candidate_pool=None): logger.debug("sync match cache fast-path failed: %s", e) # --- End cache fast-path --- + # Durable manual library match (#787) — survives a library rescan (the + # sync_match_cache above does not), so a user's Find & Add pairing keeps + # sticking across auto-syncs instead of being re-matched from scratch (#895 + # follow-up). Self-heals a stale library id via the stored file path. + if spotify_id: + try: + from core.artists.map import get_current_profile_id + m = db.find_manual_library_match_by_source_track_id( + get_current_profile_id(), str(spotify_id), active_server) + if m: + lib_id = m.get('library_track_id') + dt = db.get_track_by_id(lib_id) if lib_id is not None else None + if not dt and m.get('library_file_path'): + new_id = db.find_track_id_by_file_path(m['library_file_path']) + dt = db.get_track_by_id(new_id) if new_id else None + if dt: + class DatabaseTrackDurable: + def __init__(self, db_t): + self.ratingKey = db_t.id + self.title = db_t.title + self.id = db_t.id + logger.debug(f"Durable manual match hit: '{original_title}' → {lib_id}") + return DatabaseTrackDurable(dt), 1.0 + except Exception as e: + logger.debug("durable manual match fast-path failed: %s", e) + # Try each artist for artist in spotify_track.artists: if isinstance(artist, str): diff --git a/core/exports/export_sources.py b/core/exports/export_sources.py new file mode 100644 index 00000000..48db313b --- /dev/null +++ b/core/exports/export_sources.py @@ -0,0 +1,184 @@ +"""Wire the real cheapest-first sources for the export MBID waterfall (#903). + +``mbid_resolver`` is the pure waterfall; this module supplies the real I/O behind each +source and assembles the ``resolve_fn`` the export job uses: + +1. **cache** — ``recording_mbid_cache`` (persistent (artist,title)->mbid). +2. **DB** — a text-matched library track's ``tracks.musicbrainz_recording_id``. +3. **file** — ``MUSICBRAINZ_RECORDING_ID`` tag of that track's file (when the DB row had + no recording id but the file was tagged on import). +4. **MusicBrainz** — live ``match_recording(track, artist)`` (rate-limited tail). + +Every source is wrapped so any failure (missing table, unreadable file, MB timeout) returns +None — the waterfall just falls through, the export never breaks. ``build_resolve_fn`` also +writes a fresh non-cache hit back to the cache so the next export of the same song is free. +""" + +from __future__ import annotations + +import threading +from typing import Callable, Optional, Tuple + +from utils.logging_config import get_logger + +from core.exports.mbid_resolver import ( + SRC_CACHE, + SRC_DB, + SRC_FILE, + SRC_MUSICBRAINZ, + normalize_key, + resolve_recording_mbid, +) + +logger = get_logger("exports.export_sources") + + +def _db_match(artist: str, title: str) -> Tuple[Optional[str], Optional[str]]: + """Text-match a library track by (artist, title); return (recording_mbid, file_path). + Either may be None. Fail-safe — any DB error returns (None, None).""" + if not title: + return (None, None) + try: + from database.music_database import get_database + db = get_database() + conn = db._get_connection() + try: + cur = conn.cursor() + cur.execute( + "SELECT t.musicbrainz_recording_id, t.file_path " + "FROM tracks t JOIN artists a ON t.artist_id = a.id " + "WHERE LOWER(t.title) = LOWER(?) AND LOWER(a.name) = LOWER(?) " + "LIMIT 1", + (title, artist), + ) + row = cur.fetchone() + if not row: + return (None, None) + mbid = row[0] if not hasattr(row, "keys") else row["musicbrainz_recording_id"] + fpath = row[1] if not hasattr(row, "keys") else row["file_path"] + return ((mbid or None), (fpath or None)) + finally: + try: + conn.close() + except Exception: # noqa: S110 + pass + except Exception as exc: + logger.debug(f"export db_match failed for '{artist} - {title}': {exc}") + return (None, None) + + +def db_recording_mbid(artist: str, title: str) -> Optional[str]: + """Recording MBID stored on a matched library track (``musicbrainz_recording_id``).""" + return _db_match(artist, title)[0] + + +def file_recording_mbid(artist: str, title: str) -> Optional[str]: + """Recording MBID read from the matched track's file tag (set on import post-processing).""" + _mbid, fpath = _db_match(artist, title) + if not fpath: + return None + try: + from mutagen import File as MutagenFile + audio = MutagenFile(fpath) + if audio is None or not getattr(audio, "tags", None): + return None + tags = audio.tags + # ID3 UFID (MusicBrainz), Vorbis/MP4 musicbrainz_trackid, etc. + for key in ("UFID:http://musicbrainz.org", "musicbrainz_trackid", + "MUSICBRAINZ_TRACKID", "----:com.apple.iTunes:MusicBrainz Track Id"): + try: + val = tags.get(key) + except Exception: + val = None + if not val: + continue + if hasattr(val, "data"): # ID3 UFID frame + val = val.data.decode("utf-8", "ignore") + if isinstance(val, (list, tuple)): + val = val[0] if val else "" + if isinstance(val, bytes): + val = val.decode("utf-8", "ignore") + val = str(val).strip() + if val: + return val + except Exception as exc: + logger.debug(f"export file_recording_mbid failed for {fpath}: {exc}") + return None + + +_mb_service = None +_mb_service_lock = threading.Lock() + + +def _get_mb_service(): + """Shared MusicBrainzService (client + cache + DB), created lazily so importing this + module never triggers a DB/network connection on paths that don't export.""" + global _mb_service + if _mb_service is None: + with _mb_service_lock: + if _mb_service is None: + from core.musicbrainz_service import MusicBrainzService + from database.music_database import get_database + _mb_service = MusicBrainzService(get_database()) + return _mb_service + + +def musicbrainz_recording_mbid(artist: str, title: str) -> Optional[str]: + """Live MusicBrainz ``match_recording`` — the rate-limited tail.""" + if not title: + return None + try: + svc = _get_mb_service() + if not svc: + return None + result = svc.match_recording(title, artist) + if result and result.get("mbid"): + return result["mbid"] + except Exception as exc: + logger.debug(f"export musicbrainz_recording_mbid failed for '{artist} - {title}': {exc}") + return None + + +def build_resolve_fn( + *, + db_fn: Callable[[str, str], Optional[str]] = db_recording_mbid, + file_fn: Callable[[str, str], Optional[str]] = file_recording_mbid, + mb_fn: Callable[[str, str], Optional[str]] = musicbrainz_recording_mbid, + cache_lookup: Optional[Callable[[str], Optional[str]]] = None, + cache_record: Optional[Callable[[str, str], bool]] = None, +) -> Callable[[str, str], Tuple[Optional[str], Optional[str]]]: + """Assemble the export ``resolve_fn(artist, title) -> (mbid, source_label)``. + + Runs cache -> DB -> file -> MusicBrainz, and writes a fresh (non-cache) hit back to the + persistent cache. All sources are injectable so the wiring is unit-testable; defaults + use the real cache module. + """ + if cache_lookup is None or cache_record is None: + from core.exports import recording_mbid_cache as _cache + cache_lookup = cache_lookup or _cache.lookup + cache_record = cache_record or _cache.record + + def resolve_fn(artist: str, title: str) -> Tuple[Optional[str], Optional[str]]: + sources = [ + (SRC_CACHE, lambda a, t: cache_lookup(normalize_key(a, t))), + (SRC_DB, db_fn), + (SRC_FILE, file_fn), + (SRC_MUSICBRAINZ, mb_fn), + ] + mbid, label = resolve_recording_mbid(artist, title, sources) + if mbid and label and label != SRC_CACHE: + try: + cache_record(normalize_key(artist, title), mbid) + except Exception: # noqa: S110 — cache write is best-effort + pass + return (mbid, label) + + return resolve_fn + + +__all__ = [ + "build_resolve_fn", + "db_recording_mbid", + "file_recording_mbid", + "musicbrainz_recording_mbid", +] diff --git a/core/exports/jspf_export.py b/core/exports/jspf_export.py new file mode 100644 index 00000000..3d77cd16 --- /dev/null +++ b/core/exports/jspf_export.py @@ -0,0 +1,89 @@ +"""Build a JSPF playlist (ListenBrainz-compatible) from resolved SoulSync tracks. + +ListenBrainz's ``POST /1/playlist/create`` requires JSPF where **every track carries a +``identifier`` of ``https://musicbrainz.org/recording/``** — text-only +entries (title/creator alone) are rejected. So a track can only be exported once we've +resolved its MusicBrainz *recording* MBID (see ``mbid_resolver``); tracks without one are +dropped here and surfaced to the user as "unmatched". + +Pure + I/O-free: callers pass already-resolved track dicts, this returns the JSPF dict +(and a small coverage summary). The same JSPF is used for both the downloadable ``.jspf`` +file and the direct create-playlist POST, so there's one source of truth for the shape. +""" + +from __future__ import annotations + +import re +from typing import Any, Dict, List, Tuple + +MB_RECORDING_PREFIX = "https://musicbrainz.org/recording/" + +# A MusicBrainz MBID is a canonical UUID. Validate to avoid emitting garbage identifiers +# that LB would reject (or, worse, that silently point nowhere). +_UUID_RE = re.compile( + r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE +) + + +def is_valid_recording_mbid(mbid: Any) -> bool: + """True when ``mbid`` is a well-formed MusicBrainz UUID.""" + return bool(mbid) and isinstance(mbid, str) and bool(_UUID_RE.match(mbid.strip())) + + +def _track_entry(track: Dict[str, Any]) -> Dict[str, Any] | None: + """Build one JSPF track entry, or None if the track has no valid recording MBID.""" + mbid = (track.get("recording_mbid") or "").strip() if isinstance(track.get("recording_mbid"), str) else "" + if not is_valid_recording_mbid(mbid): + return None + entry: Dict[str, Any] = {"identifier": f"{MB_RECORDING_PREFIX}{mbid}"} + # Optional, human-friendly fields — LB ignores them on create but they make the + # downloaded .jspf readable and round-trippable. + if track.get("title"): + entry["title"] = str(track["title"]) + if track.get("artist"): + entry["creator"] = str(track["artist"]) + if track.get("album"): + entry["album"] = str(track["album"]) + return entry + + +def build_jspf( + title: str, + tracks: List[Dict[str, Any]], + *, + creator: str = "", +) -> Tuple[Dict[str, Any], Dict[str, int]]: + """Build a ListenBrainz-compatible JSPF dict from resolved tracks. + + ``tracks`` is an ordered list of dicts with ``recording_mbid`` (required to be + included), plus optional ``title`` / ``artist`` / ``album``. Tracks without a valid + recording MBID are skipped (LB rejects them). + + Returns ``(jspf, summary)`` where ``jspf`` is ``{"playlist": {...}}`` and ``summary`` + is ``{"total", "included", "skipped"}`` for the coverage display. + """ + jspf_tracks: List[Dict[str, Any]] = [] + for t in tracks or []: + if not isinstance(t, dict): + continue + entry = _track_entry(t) + if entry is not None: + jspf_tracks.append(entry) + + playlist: Dict[str, Any] = { + "title": (title or "SoulSync Export").strip() or "SoulSync Export", + "track": jspf_tracks, + } + if creator: + playlist["creator"] = str(creator) + + total = sum(1 for t in (tracks or []) if isinstance(t, dict)) + summary = { + "total": total, + "included": len(jspf_tracks), + "skipped": total - len(jspf_tracks), + } + return {"playlist": playlist}, summary + + +__all__ = ["build_jspf", "is_valid_recording_mbid", "MB_RECORDING_PREFIX"] diff --git a/core/exports/mbid_resolver.py b/core/exports/mbid_resolver.py new file mode 100644 index 00000000..f5b11972 --- /dev/null +++ b/core/exports/mbid_resolver.py @@ -0,0 +1,88 @@ +"""Resolve a playlist track's MusicBrainz *recording* MBID, cheapest source first. + +A ListenBrainz playlist export needs each track's recording MBID (``jspf_export``). A +SoulSync track can supply it from several places, in increasing cost: + +1. **resolution cache** — a prior (artist,title)->mbid result (persistent; reused across + playlists and runs, so the same song never costs twice). +2. **library DB** — ``tracks.musicbrainz_recording_id`` (set by the MusicBrainz + enrichment worker). +3. **file tags** — ``MUSICBRAINZ_RECORDING_ID`` written into the audio file on import + post-processing (catches tracks enriched at import but not via the worker). +4. **MusicBrainz lookup** — a live ``match_recording(artist, title)`` (rate-limited + ~1 req/s; the slow tail — only hit when 1–3 miss). + +This module is the **pure waterfall**: the caller passes ordered ``(label, fn)`` sources, +each ``fn(artist, title) -> mbid | None``, and ``resolve_recording_mbid`` returns the +first valid hit plus its label (for the live status / stats). The actual I/O (DB query, +mutagen read, MB request, cache read/write) lives in the export job that wires the real +sources — so this stays trivially unit-testable and short-circuits correctly. +""" + +from __future__ import annotations + +import re +from typing import Any, Callable, List, Optional, Tuple + +# Source labels (also used in the live-status breakdown). +SRC_CACHE = "cache" +SRC_DB = "db" +SRC_FILE = "file" +SRC_MUSICBRAINZ = "musicbrainz" +SRC_NONE = None + +_UUID_RE = re.compile( + r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE +) + +Source = Tuple[str, Callable[[str, str], Optional[str]]] + + +def _valid(mbid: Any) -> Optional[str]: + """Return the trimmed MBID if it's a well-formed UUID, else None.""" + if not isinstance(mbid, str): + return None + m = mbid.strip() + return m if _UUID_RE.match(m) else None + + +def normalize_key(artist: Any, title: Any) -> str: + """Stable cache key for an (artist, title) pair — lower, punctuation-stripped, + whitespace-collapsed — so trivial variations share a cache entry.""" + def _n(v: Any) -> str: + s = re.sub(r"[^\w\s]", "", str(v or "").lower()) + return re.sub(r"\s+", " ", s).strip() + return f"{_n(artist)}␟{_n(title)}" + + +def resolve_recording_mbid( + artist: str, + title: str, + sources: List[Source], +) -> Tuple[Optional[str], Optional[str]]: + """Walk ``sources`` in order; return ``(mbid, label)`` of the first that yields a + valid recording MBID, or ``(None, None)`` when every source misses. + + Each source is ``(label, fn)`` and ``fn(artist, title)`` returns an MBID or None. A + source that raises is treated as a miss (never aborts the waterfall) — so one flaky + lookup (e.g. a MusicBrainz timeout) can't fail the whole export. Short-circuits: a + later/expensive source isn't called once an earlier one hits. + """ + for label, fn in sources or []: + try: + mbid = _valid(fn(artist, title)) + except Exception: + mbid = None + if mbid: + return (mbid, label) + return (None, None) + + +__all__ = [ + "resolve_recording_mbid", + "normalize_key", + "SRC_CACHE", + "SRC_DB", + "SRC_FILE", + "SRC_MUSICBRAINZ", +] diff --git a/core/exports/playlist_export.py b/core/exports/playlist_export.py new file mode 100644 index 00000000..1105a09b --- /dev/null +++ b/core/exports/playlist_export.py @@ -0,0 +1,92 @@ +"""Orchestrate resolving a playlist's tracks to recording MBIDs for export (#903). + +This is the testable heart of the export job: walk the playlist's tracks, resolve each to a +MusicBrainz recording MBID via an injected ``resolve_fn`` (which the job wires to the +cache -> DB -> file -> MusicBrainz waterfall), dedup repeated songs within the run so they +only cost one resolution, build the ordered "pseudo-playlist" of resolved tracks, and tally +live stats (resolved / unmatched / per-source / deduped) for the on-card status display. + +Pure: all I/O (DB, file reads, MusicBrainz, cache) is behind ``resolve_fn`` and the optional +``on_progress`` callback, so the dedup + accounting logic is unit-testable without any +network or database. The returned ``resolved`` list feeds straight into ``jspf_export``. +""" + +from __future__ import annotations + +from typing import Any, Callable, Dict, List, Optional, Tuple + +from core.exports.mbid_resolver import normalize_key + +# resolve_fn(artist, title) -> (recording_mbid|None, source_label|None) +ResolveFn = Callable[[str, str], Tuple[Optional[str], Optional[str]]] +ProgressFn = Callable[[int, int, Dict[str, Any]], None] + + +def _field(track: Dict[str, Any], *names: str) -> str: + """First non-empty value among ``names`` (handles both playlist + LB-cache shapes).""" + for n in names: + v = track.get(n) + if v: + return str(v) + return "" + + +def resolve_playlist_tracks( + tracks: List[Dict[str, Any]], + resolve_fn: ResolveFn, + *, + on_progress: Optional[ProgressFn] = None, +) -> Dict[str, Any]: + """Resolve every track to a recording MBID and build the export pseudo-playlist. + + ``tracks`` items may use ``artist``/``artist_name`` and ``title``/``track_name`` and + ``album``/``album_name`` (both the mirrored-playlist and LB-cache shapes are accepted). + + Returns ``{"resolved": [...], "stats": {...}}`` where each resolved entry is + ``{artist, title, album, recording_mbid}`` (recording_mbid is None when unmatched), + in original order, and stats carries ``total, resolved, unmatched, deduped, + by_source`` for the live display. + """ + total = len(tracks or []) + memo: Dict[str, Tuple[Optional[str], Optional[str]]] = {} + resolved: List[Dict[str, Any]] = [] + stats: Dict[str, Any] = { + "total": total, "resolved": 0, "unmatched": 0, "deduped": 0, "by_source": {}, + } + + for i, t in enumerate(tracks or []): + if not isinstance(t, dict): + t = {} + artist = _field(t, "artist", "artist_name", "creator") + title = _field(t, "title", "track_name", "name") + album = _field(t, "album", "album_name", "release_name") + key = normalize_key(artist, title) + + if key in memo: + mbid, source = memo[key] + stats["deduped"] += 1 + fresh = False + else: + mbid, source = resolve_fn(artist, title) + memo[key] = (mbid, source) + fresh = True + + resolved.append({"artist": artist, "title": title, "album": album, "recording_mbid": mbid}) + + if mbid: + stats["resolved"] += 1 + if fresh and source: + stats["by_source"][source] = stats["by_source"].get(source, 0) + 1 + else: + stats["unmatched"] += 1 + + if on_progress is not None: + try: + on_progress(i + 1, total, stats) + except Exception: # noqa: S110 — a progress-display error must never fail the export + pass + + return {"resolved": resolved, "stats": stats} + + +__all__ = ["resolve_playlist_tracks"] diff --git a/core/exports/recording_mbid_cache.py b/core/exports/recording_mbid_cache.py new file mode 100644 index 00000000..203050b9 --- /dev/null +++ b/core/exports/recording_mbid_cache.py @@ -0,0 +1,126 @@ +"""Persistent (artist,title) -> MusicBrainz recording-MBID cache for playlist export. + +The export waterfall (``core.exports.mbid_resolver``) ends in a live MusicBrainz lookup +that's rate-limited to ~1 req/s — the slow tail of exporting a big playlist. Remembering a +resolved recording MBID ONCE means the same song never costs a second lookup, across every +future export and every playlist it appears in. + +Mirrors ``core.metadata.album_mbid_cache`` exactly: a tiny SQLite table, lazy DB accessor, +every function wrapped so any DB error degrades to a cache miss / no-op. If this module +breaks, exports still work — they just re-resolve via the live waterfall like a cold cache. +Key is the normalized ``track_key`` from ``mbid_resolver.normalize_key(artist, title)``. +""" + +from __future__ import annotations + +import threading +from typing import Optional + +from utils.logging_config import get_logger + +logger = get_logger("exports.recording_mbid_cache") + +_db_factory_lock = threading.Lock() +_db_factory = None + + +def _get_database(): + """Resolve the MusicDatabase singleton lazily; None on any failure (treated as miss).""" + global _db_factory + with _db_factory_lock: + if _db_factory is None: + try: + from database.music_database import get_database + _db_factory = get_database + except Exception as exc: + logger.warning(f"Recording-MBID cache: could not load database module: {exc}") + return None + try: + return _db_factory() + except Exception as exc: + logger.warning(f"Recording-MBID cache: database accessor failed: {exc}") + return None + + +def lookup(track_key: str) -> Optional[str]: + """Read a cached recording MBID for ``track_key``; None on miss or any DB error.""" + if not track_key: + return None + db = _get_database() + if db is None: + return None + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute( + "SELECT recording_mbid FROM mb_recording_cache WHERE track_key = ? LIMIT 1", + (track_key,), + ) + row = cursor.fetchone() + if row: + return (row[0] if not hasattr(row, "keys") else row["recording_mbid"]) or None + except Exception as exc: + logger.debug(f"Recording-MBID cache lookup failed: {exc}") + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + return None + + +def record(track_key: str, recording_mbid: str) -> bool: + """Persist ``track_key`` -> ``recording_mbid`` (idempotent). False on any failure.""" + if not track_key or not recording_mbid: + return False + db = _get_database() + if db is None: + return False + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute( + "INSERT OR REPLACE INTO mb_recording_cache " + "(track_key, recording_mbid, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP)", + (track_key, recording_mbid), + ) + conn.commit() + return True + except Exception as exc: + logger.debug(f"Recording-MBID cache record failed: {exc}") + return False + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + + +def clear_all() -> bool: + """Wipe the cache (tests / forced re-resolve).""" + db = _get_database() + if db is None: + return False + conn = None + try: + conn = db._get_connection() + cursor = conn.cursor() + cursor.execute("DELETE FROM mb_recording_cache") + conn.commit() + return True + except Exception as exc: + logger.warning(f"Recording-MBID cache clear failed: {exc}") + return False + finally: + if conn is not None: + try: + conn.close() + except Exception: # noqa: S110 — finally cleanup + pass + + +__all__ = ["lookup", "record", "clear_all"] diff --git a/core/hifi_client.py b/core/hifi_client.py index 7d085154..73ce0c69 100644 --- a/core/hifi_client.py +++ b/core/hifi_client.py @@ -165,6 +165,81 @@ def compute_new_default_pushes(all_defaults, offered, legacy_baseline, existing) return to_add, new_offered +_EXTINF_RE = re.compile(r'#EXTINF:\s*([0-9]+(?:\.[0-9]+)?)') + + +def sum_hls_segment_seconds(playlist_text: str) -> float: + """Total audio seconds an HLS media playlist actually provides — the sum of its + ``#EXTINF`` segment durations. This is the authoritative "how much audio is really + here" signal: a PREVIEW manifest serves only ~30s of segments even though the track + is full-length, so summing EXTINF catches it before we waste the download. Returns + 0.0 when the playlist has no EXTINF lines (master playlists, legacy manifests) — the + caller treats 0 as 'unknown', never as 'preview'.""" + total = 0.0 + for m in _EXTINF_RE.finditer(playlist_text or ''): + try: + total += float(m.group(1)) + except (TypeError, ValueError): + continue + return total + + +def is_short_audio(actual_seconds: float, expected_seconds: float, threshold: float = 0.8) -> bool: + """True when ``actual`` is meaningfully shorter than ``expected`` — i.e. a preview + clip or a truncated/corrupt download. Conservative: returns False whenever either + value is missing/zero (unknown ⇒ never reject), and only trips below ``threshold`` + of the expected length (previews are ~15% of full, so the margin is huge).""" + try: + a, e = float(actual_seconds or 0), float(expected_seconds or 0) + except (TypeError, ValueError): + return False + if a <= 0 or e <= 0: + return False + return a < e * threshold + + +def is_fake_lossless_bitrate(size_bytes, claimed_seconds, sample_rate, bits_per_sample, + channels, min_ratio: float = 0.30) -> bool: + """True when a 'lossless' file's data is FAR too small for its claimed length — the + fingerprint of a ~30s preview whose STREAMINFO/container was faked to the full + duration (so every length header reads 'full' and only the bitrate gives it away). + Real FLAC is ~40-75% of raw PCM; a preview padded to full length implies single-digit + %. Conservative: 0 / bad inputs return False (never reject on unknowns).""" + try: + sz, secs = float(size_bytes or 0), float(claimed_seconds or 0) + sr, bits, ch = int(sample_rate or 0), int(bits_per_sample or 0), int(channels or 0) + except (TypeError, ValueError): + return False + if sz <= 0 or secs <= 0 or sr <= 0 or bits <= 0 or ch <= 0: + return False + return (sz * 8 / secs) < (sr * bits * ch) * min_ratio + + +def parse_ffmpeg_time(stderr_text) -> float: + """The last ``time=HH:MM:SS.xx`` ffmpeg prints while decoding — the REAL decoded + length (immune to a faked container/STREAMINFO duration). 0.0 if not found.""" + last = 0.0 + for m in re.finditer(r'time=(\d+):(\d+):(\d+(?:\.\d+)?)', stderr_text or ''): + last = int(m.group(1)) * 3600 + int(m.group(2)) * 60 + float(m.group(3)) + return last + + +def is_preview_download(real_seconds, reference_seconds, *, is_lossless, size_bytes, + sample_rate, bits_per_sample, channels): + """Is a finished file a preview/truncated fake? Two independent signals, so it fires + even when the fakery declares full length at every layer: + 1. DECODED length far below the reference (the ground truth, when a decoder ran); + 2. for lossless, an impossibly-low implied bitrate (no decoder needed). + Returns ``(is_fake, reason)``.""" + if real_seconds and is_short_audio(real_seconds, reference_seconds): + return True, "decoded %.0fs of %.0fs" % (real_seconds, reference_seconds) + if is_lossless and is_fake_lossless_bitrate(size_bytes, reference_seconds, sample_rate, + bits_per_sample, channels): + kbps = (float(size_bytes) * 8 / reference_seconds / 1000) if reference_seconds else 0 + return True, "%.0fkbps lossless over %.0fs (far too low — a ~30s preview)" % (kbps, reference_seconds) + return False, "" + + # Run the new-default push at most once per process. _pushed_new_defaults = False @@ -688,7 +763,7 @@ class HiFiClient(DownloadSourcePlugin): logger.warning(f"Failed to parse HLS playlist for track {track_id}: {e}") return None - media_text = playlist_text + media_text = playlist_text # the playlist that actually carries the EXTINF segments if '#EXT-X-STREAM-INF' in playlist_text and segment_uris: playlist_uri = segment_uris[0] try: @@ -729,6 +804,9 @@ class HiFiClient(DownloadSourcePlugin): 'extension': q_info['extension'], 'codec': q_info['codec'], 'quality': quality, + # Real audio length the manifest provides (sum of EXTINF) — used to reject + # preview manifests before downloading. 0.0 = unknown (don't reject). + 'manifest_duration': sum_hls_segment_seconds(media_text), } def _get_legacy_track_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]: @@ -753,15 +831,57 @@ class HiFiClient(DownloadSourcePlugin): 'quality': quality, } + @staticmethod + def _probe_audio_seconds(path) -> float: + """Real decoded audio length of a finished file, via mutagen (already a dep). + 0.0 on any failure — the caller treats 0 as 'unknown' and never rejects on it.""" + try: + from mutagen import File as _MutagenFile + mf = _MutagenFile(str(path)) + info = getattr(mf, 'info', None) if mf is not None else None + if info is not None: + return float(getattr(info, 'length', 0) or 0) + except Exception as _probe_err: # noqa: BLE001 + logger.debug("mutagen audio-length probe failed for %s: %s", path, _probe_err) + return 0.0 + + @staticmethod + def _find_ffmpeg(): + ff = shutil.which('ffmpeg') + if ff: + return ff + cand = Path(__file__).parent.parent / 'tools' / ('ffmpeg.exe' if os.name == 'nt' else 'ffmpeg') + return str(cand) if cand.exists() else None + + def _probe_real_seconds(self, path) -> float: + """REAL decoded audio length via ffmpeg — decodes the actual frames, so it sees + through a faked STREAMINFO/container duration (a 30s preview claiming full + length decodes to 30s). 0.0 if ffmpeg is unavailable or on error.""" + ff = self._find_ffmpeg() + if not ff: + return 0.0 + try: + proc = subprocess.run( + [ff, '-hide_banner', '-nostdin', '-i', str(path), '-map', '0:a:0', '-f', 'null', '-'], + capture_output=True, text=True, timeout=180) + return parse_ffmpeg_time(proc.stderr) + except Exception: + return 0.0 + + @staticmethod + def _flac_props(path): + """(sample_rate, bits_per_sample, channels) for the bitrate sanity check, or None.""" + try: + from mutagen.flac import FLAC + si = FLAC(str(path)).info + return (si.sample_rate, si.bits_per_sample, si.channels) + except Exception: + return None + def _demux_flac(self, input_path: Path, output_path: Path) -> None: - ffmpeg = shutil.which('ffmpeg') + ffmpeg = self._find_ffmpeg() if not ffmpeg: - tools_dir = Path(__file__).parent.parent / 'tools' - ffmpeg_candidate = tools_dir / ('ffmpeg.exe' if os.name == 'nt' else 'ffmpeg') - if ffmpeg_candidate.exists(): - ffmpeg = str(ffmpeg_candidate) - else: - raise RuntimeError('ffmpeg is required to demux FLAC from MP4. Install ffmpeg and retry.') + raise RuntimeError('ffmpeg is required to demux FLAC from MP4. Install ffmpeg and retry.') try: result = subprocess.run( @@ -883,13 +1003,18 @@ class HiFiClient(DownloadSourcePlugin): MIN_AUDIO_SIZE = 100 * 1024 - # Real track duration — lets _get_hls_manifest reject 30s preview - # manifests before we download them. Best-effort: 0 disables the check. + # Expected track length, drives every preview/truncation guard here: + # * _get_hls_manifest's pre-download is_preview_playlist check + # * the pre-download is_short_audio manifest check + # * the post-download is_preview_download faked-header decode check + # Best-effort: a 0 here just disables the duration checks, never rejects. + expected_s = 0.0 try: - _info = self.get_track_info(track_id) or {} - expected_duration_s = float(_info.get('duration_s') or 0) + info = self.get_track_info(track_id) or {} + expected_s = float(info.get('duration_s') or 0) except Exception: - expected_duration_s = 0 + expected_s = 0.0 + expected_duration_s = expected_s # alias for _get_hls_manifest's param name for q_key in chain: if self.shutdown_check and self.shutdown_check(): @@ -908,6 +1033,18 @@ class HiFiClient(DownloadSourcePlugin): logger.warning(f"No HLS manifest at quality {q_key}, trying next") continue + # Preview guard #1 (pre-download): a preview manifest serves only ~30s of + # segments for a full-length track. A preview means THIS SOURCE only has a + # preview of the track — lower quality tiers are the SAME preview — so abort + # HiFi entirely and let the orchestrator fall through to the next SOURCE + # (soulseek/youtube/…), rather than landing a lower-tier preview. + manifest_s = float(manifest_info.get('manifest_duration') or 0) + if is_short_audio(manifest_s, expected_s): + logger.warning( + "HiFi has only a PREVIEW of '%s' (manifest %.0fs of %.0fs at %s) — " + "failing HiFi so the next source is tried", display_name, manifest_s, expected_s, q_key) + return None + extension = manifest_info['extension'] safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name) out_filename = f"{safe_name}.{extension}" @@ -987,6 +1124,31 @@ class HiFiClient(DownloadSourcePlugin): out_path.unlink(missing_ok=True) continue + # Preview guard #2 (post-download): the real catch. HiFi previews fake the + # FULL length in every header — manifest EXTINF, m4a moov, FLAC + # total_samples — so only the DECODED audio (or, for lossless, the + # bitrate) reveals the ~30s truth. Reference = the largest length any + # header claims (so the file's own faked claim becomes the bar its real + # audio must clear); is_preview_download decodes + bitrate-checks. + ref_s = max(expected_s, self._probe_audio_seconds(out_path)) + real_s = self._probe_real_seconds(out_path) + props = self._flac_props(out_path) if is_flac else None + fake, why = is_preview_download( + real_s, ref_s, is_lossless=is_flac, size_bytes=final_size, + sample_rate=(props[0] if props else 0), + bits_per_sample=(props[1] if props else 0), + channels=(props[2] if props else 0)) + if fake: + # A preview at this tier means the SOURCE only has a preview — every + # lower tier is the same 30s clip (and the lossy ones dodge the + # bitrate check). Abort HiFi so the orchestrator tries the next + # SOURCE, instead of cascading down into an accepted lower-tier preview. + logger.warning( + "HiFi has only a PREVIEW of '%s' (%s at %s) — failing HiFi so the " + "next source is tried", display_name, why, q_key) + out_path.unlink(missing_ok=True) + return None + logger.info(f"HiFi download complete ({q_key}): {out_path} " f"({final_size / (1024*1024):.1f} MB)") return str(out_path) diff --git a/core/imports/album_position.py b/core/imports/album_position.py new file mode 100644 index 00000000..7da1b14a --- /dev/null +++ b/core/imports/album_position.py @@ -0,0 +1,94 @@ +"""Resolve a track's position WITHIN its album's track list. + +The bug this fixes: a track auto-downloaded from the playlist pipeline / wishlist / +watchlist is identified as belonging to an album, but the per-track position is +unknown — Deezer's search/track and MusicBrainz's recording lookups don't carry a +track position (only their album endpoint does). ``detect_album_info_web`` then +leaves ``track_number = None``, the import pipeline falls through to the default-1 +floor, and the file lands as ``01/1`` even though the album is known +(``core/imports/context.py``). Verified live: e.g. Deezer says "Obelisk" is track +9 of *The Grand Mirage*, but it was tagged 1/1. + +This is the pure matcher: given the album's track list (fetched by the caller via +``core.metadata.album_tracks.get_album_tracks_for_source`` — so this stays +source-agnostic and I/O-free) plus the track's own identifiers, return its real +``(track_number, disc_number)``. Match priority is by reliability: + +1. **ISRC** — an exact recording identity; trusted immediately. +2. **source track id** — exact within this album. +3. **normalized title** — last resort. + +Returns ``(None, None)`` on no confident match, so the caller keeps its existing +behaviour (never worse than today). +""" + +from __future__ import annotations + +import re +from typing import Any, List, Optional, Tuple + + +def _norm_title(value: Any) -> str: + """Lower, strip punctuation, collapse whitespace — for tolerant title match.""" + s = re.sub(r"[^\w\s]", "", str(value or "").lower()) + return re.sub(r"\s+", " ", s).strip() + + +def _pos_int(value: Any) -> Optional[int]: + try: + n = int(value) + except (TypeError, ValueError): + return None + return n if n >= 1 else None + + +def resolve_track_position_in_album( + album_tracks: List[dict], + *, + title: str = "", + track_id: str = "", + isrc: str = "", +) -> Tuple[Optional[int], Optional[int]]: + """Return ``(track_number, disc_number)`` for this track within ``album_tracks``, + or ``(None, None)`` when no confident match is found. + + ``album_tracks`` is the list under ``get_album_tracks_for_source(...)['tracks']`` + — each entry has ``track_number`` / ``disc_number`` / ``id`` / ``name`` / ``isrc``. + Entries without a valid positive ``track_number`` are skipped. Pure: no I/O. + """ + if not album_tracks: + return (None, None) + + want_isrc = str(isrc or "").strip().upper() + want_id = str(track_id or "").strip() + want_title = _norm_title(title) + + by_id: Optional[Tuple[int, int]] = None + by_title: Optional[Tuple[int, int]] = None + + for t in album_tracks: + if not isinstance(t, dict): + continue + tn = _pos_int(t.get("track_number")) + if tn is None: + continue + dn = _pos_int(t.get("disc_number")) or 1 + + # 1) ISRC — exact recording. Win immediately. + if want_isrc and str(t.get("isrc") or "").strip().upper() == want_isrc: + return (tn, dn) + # 2) source track id — exact within the album. + if by_id is None and want_id and str(t.get("id") or "").strip() == want_id: + by_id = (tn, dn) + # 3) normalized title — last resort. + if by_title is None and want_title and _norm_title(t.get("name")) == want_title: + by_title = (tn, dn) + + if by_id is not None: + return by_id + if by_title is not None: + return by_title + return (None, None) + + +__all__ = ["resolve_track_position_in_album"] diff --git a/core/imports/context.py b/core/imports/context.py index 9ed2a909..4e15c918 100644 --- a/core/imports/context.py +++ b/core/imports/context.py @@ -433,16 +433,24 @@ def detect_album_info_web(context, artist_context=None): track_name.strip().lower(), artist_name.strip().lower(), }: + _tn = track_info.get("track_number") + _dn = track_info.get("disc_number") + # The album is identified but discovery often doesn't carry the per-track + # POSITION — Deezer's search/track and MusicBrainz's recording lookups omit + # it (only their album endpoint has it). Without a position the pipeline + # falls through to the default-1 floor and files an album track as 01/1 + # (e.g. Deezer says "Obelisk" is track 9 of The Grand Mirage). Resolve the + # REAL position from the album's own track list when we have its id. + # Fail-safe: leaves the numbers untouched on any miss, so behaviour is + # never worse than the old preserve-None-and-fall-through. + if _tn is None: + _tn, _dn = _resolve_album_position_from_source(context, artist_context, _dn) return build_import_album_info( context, album_info={ "album_name": album_name, - # Preserve missing numbers as None so the import pipeline - # can fall through to ``extract_track_number_from_filename`` - # at ``core/imports/pipeline.py:652`` instead of locking - # to track/disc 01 for every wishlist re-attempt. - "track_number": track_info.get("track_number"), - "disc_number": track_info.get("disc_number"), + "track_number": _tn, + "disc_number": _dn, "album_image_url": album_ctx.get("image_url", ""), "confidence": 0.5, }, @@ -454,6 +462,48 @@ def detect_album_info_web(context, artist_context=None): return _resolve_single_to_parent_album(context, artist_context) +def _resolve_album_position_from_source(context, artist_context, current_disc): + """Look up a track's real ``(track_number, disc_number)`` from its album's track + list, for the case where the album is known but discovery didn't carry a + position (Deezer/MusicBrainz search omit it). + + Uses the SAME album id discovery already resolved (``get_import_source_ids`` → + ``album_id``), so it re-homes the track onto its own album with no re-search and + no edition guessing. Matches by ISRC → source track id → title via the pure + ``core.imports.album_position`` seam. Returns ``(None, current_disc)`` on any + miss/error so the caller falls back exactly as before — never worse than today. + """ + try: + source = get_import_source(context) + ids = get_import_source_ids(context) + album_id = str(ids.get("album_id") or "") + if not source or not album_id: + return None, current_disc + + from core.metadata.album_tracks import get_album_tracks_for_source + payload = get_album_tracks_for_source(source, album_id) or {} + tracks = payload.get("tracks") or [] + if not tracks: + return None, current_disc + + track_info = get_import_track_info(context) + original_search = get_import_original_search(context) + title = (track_info.get("name") or original_search.get("title") or "").strip() + isrc = str(track_info.get("isrc") or original_search.get("isrc") or "") + + from core.imports.album_position import resolve_track_position_in_album + tn, dn = resolve_track_position_in_album( + tracks, title=title, track_id=str(ids.get("track_id") or ""), isrc=isrc) + if tn is not None: + logger.info("album-position: resolved '%s' to track %s/disc %s from album %s (%s)", + title, tn, dn, album_id, source) + return tn, (dn if dn is not None else current_disc) + return None, current_disc + except Exception as e: + logger.debug("album-position resolution failed: %s", e) + return None, current_disc + + def _resolve_single_to_parent_album(context, artist_context): """A single-matched track -> a promoted album_info for its parent album, or None. GATED by ``metadata_enhancement.single_to_album`` (default OFF — it's a diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 17dcd30a..0ff2d1b4 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -719,6 +719,16 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta album_info['clean_track_name'] = clean_track_name logger.info(f"[FIX] Updated album_info track_number to {track_number} for consistent metadata") + # Sync the disc number the SAME way (and via the SAME resolver) the embedded + # tag will use — otherwise the "Disc N" folder is built from album_info's + # original disc while the tag takes the per-track disc, so a disc-2/3 track + # lands in the Disc 1 folder and every disc's tracks collapse together (Sokhi). + from core.imports.track_number import resolve_disc_for_track + _resolved_disc = resolve_disc_for_track(get_import_original_search(context), album_info) + if album_info.get('disc_number') != _resolved_disc: + logger.info(f"[FIX] Updated album_info disc_number to {_resolved_disc} for consistent metadata") + album_info['disc_number'] = _resolved_disc + final_path, _ = build_final_path_for_track(context, artist_context, album_info, file_ext) logger.info(f"Resolved path: '{final_path}'") context['_final_processed_path'] = final_path diff --git a/core/imports/track_number.py b/core/imports/track_number.py index 63b53b03..808577c7 100644 --- a/core/imports/track_number.py +++ b/core/imports/track_number.py @@ -159,3 +159,53 @@ def resolve_track_number( # value the pre-fix resolver would have used. A correctly-named file # with a stale/wrong embedded tag is therefore never regressed. return _coerce_positive(embedded_track_number) + + +def normalize_disc_number(value) -> int: + """Coerce a disc value to a positive int, defaulting to 1. + + Every track in a multi-disc album MUST carry a disc number, or Jellyfin/Plex + leave the disc-less ones floating ungrouped above the disc sections (Sokhi's + "tracks 3/9/15 at the top"). Upstream sources can hand back 0, None, '', or a + non-numeric string for some tracks — especially when a track resolved to a + different edition than its siblings — and the tag-writer only wrote the disc + tag when it was truthy, so those tracks lost it entirely on the clear-then- + rewrite. Flooring to >=1 here means a track is never written disc-less. + """ + try: + n = int(value) # int, float, or clean int-string + except (TypeError, ValueError): + try: + n = int(float(str(value).strip())) # tolerate "2.0" + except (TypeError, ValueError): + return 1 + return n if n >= 1 else 1 + + +def resolve_disc_for_track(original_search, album_info) -> int: + """The disc number for a track — resolved IDENTICALLY for the 'Disc N' folder + (import pipeline) and the embedded tag (metadata.source), so the two can never + disagree. + + Sokhi: the pipeline synced the resolved track_number into album_info (so the + folder matched the tag) but never did the same for disc — the folder used + album_info's original disc (often 1) while the tag took the per-track disc + (e.g. 2/3 from a MusicBrainz multi-medium release). Result: a disc-2/3 track + landed in the Disc 1 folder, collapsing every disc's tracks into one folder. + + Returns the first VALID positive disc — the per-track search's, else the album + context's — else 1. A falsy/unknown (0/None/'') per-track disc falls through to + the album rather than flooring early. Single source of truth so both call sites + stay in lockstep.""" + for src in ((original_search or {}), (album_info or {})): + raw = src.get("disc_number") + try: + n = int(raw) + except (TypeError, ValueError): + try: + n = int(float(str(raw).strip())) + except (TypeError, ValueError): + continue + if n >= 1: + return n + return 1 diff --git a/core/library/standalone_scan.py b/core/library/standalone_scan.py new file mode 100644 index 00000000..014ca36f --- /dev/null +++ b/core/library/standalone_scan.py @@ -0,0 +1,104 @@ +"""Decision logic for the SoulSync standalone Deep Scan's untracked → Staging move. + +The standalone deep scan (``_run_soulsync_deep_scan`` in web_server) walks the +Transfer folder, diffs it against the ``soulsync`` rows in the DB, and relocates +every file it can't find a DB record for into Staging for auto-import. That's fine +when Transfer is a scratch/landing area — files arrive, get moved, imported, and +recorded, so a later scan only ever sees a few genuinely-new arrivals. + +It is a DATA-LOSS trap when the DB is empty or out of sync with disk (a volume +swap, a DB reset, external tag edits) while Transfer holds the user's real library: +a path-only diff then flags the *entire* library as "untracked" and the scan +relocates all of it (issue #904). The same failure mode the orphan detector and the +media-server deep scan already guard against (``core.library.stale_guard``) — this +path just never used the guard. + +This module is the pure, testable decision: given the Transfer file set, the DB's +known paths, and the user's "Transfer is my permanent library" preference, decide +WHICH files are untracked and WHETHER it's safe to relocate them. The web layer does +only the I/O (walk/move/delete) based on the returned plan. +""" + +from __future__ import annotations + +from typing import Iterable, Set + +from core.library.stale_guard import ( + DEFAULT_MAX_ORPHAN_FRACTION, + DEFAULT_MIN_ORPHANS, + is_implausible_orphan_flood, +) + +# Block reason codes (web layer turns these into a user-facing warning). +BLOCK_NONE = "" +BLOCK_TRANSFER_PERMANENT = "transfer_permanent" +BLOCK_DESYNC = "desync" + + +def _norm(path: str) -> str: + """Normalize a path for cross-platform comparison (Windows vs Unix separators).""" + return str(path).replace("\\", "/") + + +def diff_untracked(transfer_files: Iterable[str], db_paths: Iterable[str]) -> Set[str]: + """Files present in ``transfer_files`` but with no matching ``db_paths`` record. + + Comparison is separator-normalized, so a DB path stored with one separator style + still matches the on-disk path. Pure — no I/O. Returns the original (un-normalized) + transfer paths so the caller can act on the real filesystem entries. + """ + db_norm = {_norm(p) for p in db_paths if p} + return {f for f in transfer_files if _norm(f) not in db_norm} + + +def plan_standalone_deep_scan( + transfer_files: Iterable[str], + db_paths: Iterable[str], + *, + never_move: bool = False, + min_untracked: int = DEFAULT_MIN_ORPHANS, + max_fraction: float = DEFAULT_MAX_ORPHAN_FRACTION, +) -> dict: + """Plan the untracked → Staging move for a standalone deep scan. Pure — no I/O. + + Returns a dict: + * ``untracked`` (set[str]) — Transfer files with no DB record. + * ``move_blocked`` (bool) — True when the untracked files must NOT be relocated. + * ``block_reason`` (str) — ``BLOCK_TRANSFER_PERMANENT`` / ``BLOCK_DESYNC`` / "". + + The move is blocked when either: + * ``never_move`` is set (the user marked Transfer as their permanent library), or + * the untracked share is implausibly large (> ``min_untracked`` files AND + > ``max_fraction`` of the folder) — the empty/desynced-DB signature, where a + path-only diff would relocate the whole library. Below that floor a normal + batch of new arrivals still moves as before. + + ``move_blocked`` is only ever True when there ARE untracked files; an empty scan + or a clean library returns ``move_blocked=False`` with no reason. + """ + transfer_set = set(transfer_files) # concrete (handles generators) + dedups + untracked = diff_untracked(transfer_set, db_paths) + total = len(transfer_set) + n_untracked = len(untracked) + + if n_untracked == 0: + return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE} + + if never_move: + return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_TRANSFER_PERMANENT} + + if is_implausible_orphan_flood( + n_untracked, total, min_orphans=min_untracked, max_fraction=max_fraction + ): + return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_DESYNC} + + return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE} + + +__all__ = [ + "diff_untracked", + "plan_standalone_deep_scan", + "BLOCK_NONE", + "BLOCK_TRANSFER_PERMANENT", + "BLOCK_DESYNC", +] diff --git a/core/listenbrainz_client.py b/core/listenbrainz_client.py index 2ec6ed0e..af457f3a 100644 --- a/core/listenbrainz_client.py +++ b/core/listenbrainz_client.py @@ -158,6 +158,161 @@ class ListenBrainzClient: return True + _MAX_TRACKS_PER_ADD = 100 # ListenBrainz MAX_RECORDINGS_PER_ADD + _PLAYLIST_EXT = "https://musicbrainz.org/doc/jspf#playlist" + + def _lb_headers(self) -> Dict: + return {"Authorization": f"Token {self.token}", "Content-Type": "application/json"} + + def _add_tracks_in_batches(self, playlist_mbid: str, tracks: List[Dict]) -> int: + """Add JSPF tracks to a playlist in <=100-track batches; return how many were added.""" + added = 0 + headers = self._lb_headers() + for i in range(0, len(tracks or []), self._MAX_TRACKS_PER_ADD): + batch = tracks[i:i + self._MAX_TRACKS_PER_ADD] + try: + r = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/add", + json={"playlist": {"track": batch}}, headers=headers, + ) + if r and r.status_code in (200, 201): + added += len(batch) + else: + logger.warning(f"ListenBrainz item/add batch failed: " + f"{r.status_code if r else 'no response'}") + except Exception as e: + logger.error(f"ListenBrainz item/add error: {e}") + return added + + def get_playlist_track_count(self, playlist_mbid: str): + """Current track count of an LB playlist, or None if it can't be fetched (gone/404).""" + try: + r = self._make_request_with_retry( + "GET", f"{self.base_url}/playlist/{playlist_mbid}", + params={"fetch_metadata": "false"}, + headers={"Authorization": f"Token {self.token}"}, + ) + if r and r.status_code == 200: + return len(((r.json() or {}).get("playlist") or {}).get("track", [])) + except Exception as e: + logger.debug(f"ListenBrainz get playlist count failed: {e}") + return None + + def delete_playlist(self, playlist_mbid: str) -> bool: + """Delete an LB playlist. True on success.""" + try: + r = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/delete", headers=self._lb_headers() + ) + return bool(r and r.status_code in (200, 201)) + except Exception as e: + logger.error(f"ListenBrainz delete playlist error: {e}") + return False + + def create_playlist(self, title: str, tracks: List[Dict], public: bool = False) -> Dict: + """Create a NEW playlist on ListenBrainz and add its tracks (#903). + + ``tracks`` are JSPF track dicts — each MUST carry an ``identifier`` of the form + ``https://musicbrainz.org/recording/`` (LB rejects text-only tracks). Creates + an empty playlist for the MBID, then adds tracks in <=100 batches. Returns + ``{success, playlist_mbid, playlist_url, added, requested, error, updated}``. Never raises. + """ + result = {"success": False, "playlist_mbid": None, "playlist_url": None, + "added": 0, "requested": len(tracks or []), "error": None, "updated": False} + if not self.is_authenticated(): + result["error"] = "ListenBrainz not authenticated (no token/username)" + return result + + create_body = {"playlist": { + "title": (title or "SoulSync Export").strip() or "SoulSync Export", + "extension": {self._PLAYLIST_EXT: {"public": bool(public)}}, + }} + try: + resp = self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/create", json=create_body, headers=self._lb_headers() + ) + except Exception as e: + result["error"] = f"create request failed: {e}" + return result + if not resp or resp.status_code not in (200, 201): + result["error"] = f"create returned {resp.status_code if resp else 'no response'}" + return result + try: + playlist_mbid = (resp.json() or {}).get("playlist_mbid") + except Exception: + playlist_mbid = None + if not playlist_mbid: + result["error"] = "create succeeded but no playlist_mbid in response" + return result + + result["playlist_mbid"] = playlist_mbid + result["playlist_url"] = f"https://listenbrainz.org/playlist/{playlist_mbid}" + result["added"] = self._add_tracks_in_batches(playlist_mbid, tracks) + result["success"] = True + return result + + def update_playlist(self, playlist_mbid: str, title: str, tracks: List[Dict], public: bool = False) -> Dict: + """Replace an existing LB playlist's contents IN PLACE (stable URL/MBID) (#903). + + Verifies the playlist still exists, clears its current items, re-adds the new tracks, + and updates the title. If the playlist is gone (deleted on LB), returns success=False + with ``gone=True`` so the caller can fall back to creating a fresh one. + Returns the same shape as ``create_playlist`` plus ``updated=True``. + """ + result = {"success": False, "playlist_mbid": playlist_mbid, + "playlist_url": f"https://listenbrainz.org/playlist/{playlist_mbid}", + "added": 0, "requested": len(tracks or []), "error": None, + "updated": True, "gone": False} + if not self.is_authenticated(): + result["error"] = "ListenBrainz not authenticated (no token/username)" + return result + + count = self.get_playlist_track_count(playlist_mbid) + if count is None: + result["error"] = "playlist not found on ListenBrainz" + result["gone"] = True + return result + + headers = self._lb_headers() + # Clear existing items (one range delete from the top). + if count > 0: + try: + self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/{playlist_mbid}/item/delete", + json={"index": 0, "count": count}, headers=headers, + ) + except Exception as e: + logger.warning(f"ListenBrainz item/delete (clear) failed: {e}") + + result["added"] = self._add_tracks_in_batches(playlist_mbid, tracks) + + # Refresh the title (best-effort — content already replaced). + try: + self._make_request_with_retry( + "POST", f"{self.base_url}/playlist/edit/{playlist_mbid}", + json={"playlist": {"title": (title or "SoulSync Export").strip() or "SoulSync Export"}}, + headers=headers, + ) + except Exception as e: + logger.debug(f"ListenBrainz playlist title edit failed: {e}") + + result["success"] = True + return result + + def create_or_update_playlist(self, title: str, tracks: List[Dict], + existing_mbid: str = None, public: bool = False) -> Dict: + """Update the existing LB playlist in place when we've pushed this one before, else + create a fresh one — so re-exporting the same SoulSync playlist never duplicates it. + Falls back to create if the remembered playlist was deleted on LB.""" + if existing_mbid: + res = self.update_playlist(existing_mbid, title, tracks, public) + if res.get("success"): + return res + # Remembered playlist gone/failed -> create a new one instead of erroring out. + logger.info(f"ListenBrainz playlist {existing_mbid} unavailable for update " + f"({res.get('error')}); creating a new one.") + return self.create_playlist(title, tracks, public) + def get_playlists_created_for_user(self, count: int = 25, offset: int = 0) -> List[Dict]: """ Fetch playlists created FOR the user (recommendations, personalized playlists) diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index 758fe206..0c99fc77 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -438,10 +438,19 @@ def embed_album_art_metadata(audio_file, metadata: dict): if not image_data: art_url = metadata.get("album_art_url") - if not art_url: - logger.warning("No album art URL available for embedding.") + # Prefer the pinned release's OWN cover over a release-group / provider + # representative (usually the standard edition); fall back to art_url + # when the release has no art of its own. Keeps embedded art in sync + # with cover.jpg (same preference + fetch). + from core.metadata.caa_art import fetch_release_preferred_art + image_data, mime_type, used_url = fetch_release_preferred_art( + release_mbid, art_url, fetch_fn=_fetch_art_bytes) + if not image_data: + if not art_url and not release_mbid: + logger.warning("No album art URL available for embedding.") return False - image_data, mime_type = _fetch_art_bytes(art_url) + if release_mbid and used_url and used_url != art_url: + logger.info("Embedding release-specific art (edition match): %s", release_mbid) if not image_data: logger.error("Failed to download album art data.") @@ -560,13 +569,20 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None, art_url = images[0].get("url", "") if art_url: logger.info("Using cover art URL from album context") - if not art_url: - logger.warning("No cover art URL available for download.") + # Prefer the pinned release's OWN cover over a release-group / provider + # representative (which is usually the standard edition), falling back + # to art_url when the release has no art of its own. Upgrades to the + # source's highest resolution via _fetch_art_bytes (shared with the + # tag-embed path so cover.jpg and embedded art match). + from core.metadata.caa_art import fetch_release_preferred_art + image_data, _, used_url = fetch_release_preferred_art( + release_mbid, art_url, fetch_fn=_fetch_art_bytes) + if not image_data: + if not art_url and not release_mbid: + logger.warning("No cover art URL available for download.") return - # Upgrade to the source's highest resolution (Spotify master / - # iTunes 3000 / Deezer 1900) with a one-level fallback — shared - # with the tag-embed path so cover.jpg and embedded art match. - image_data, _ = _fetch_art_bytes(art_url) + if release_mbid and used_url and used_url != art_url: + logger.info("Using release-specific cover.jpg (edition match): %s", release_mbid) if not image_data: return diff --git a/core/metadata/caa_art.py b/core/metadata/caa_art.py new file mode 100644 index 00000000..b1a7d949 --- /dev/null +++ b/core/metadata/caa_art.py @@ -0,0 +1,74 @@ +"""Cover Art Archive helper: prefer a pinned release's OWN cover over the +release-group representative. + +On the Cover Art Archive a release-group ``front`` is a single REPRESENTATIVE +cover — CAA designates one release in the group to stand for the whole thing, +which is almost always the standard / most-common edition. So when a download +has pinned a SPECIFIC release (e.g. a "Gustave Edition" the user picked), using +the release-group cover silently swaps in the standard art. + +This helper tries the specific release's own ``/release//front`` first and +only falls back to the caller's existing URL (a release-group representative or a +provider cover) when the release has no art of its own — so it can only ever +*improve* on today's behaviour, never strip a cover that was already showing. + +Pure: the network fetch is injected, so the preference logic is unit-testable. +""" + +from __future__ import annotations + +from typing import Callable, Optional, Tuple + +COVER_ART_ARCHIVE_URL = "https://coverartarchive.org" + + +def caa_front_url(mbid: Optional[str], scope: str = "release", size: int = 1200) -> Optional[str]: + """Build a Cover Art Archive front-cover URL, or None for a falsy mbid. + ``scope`` is 'release' (a specific edition) or 'release-group' (the group's + representative). ``size`` selects the CDN thumbnail (e.g. 250/500/1200); 0 + requests the bare ``/front`` original.""" + if not mbid: + return None + if scope not in ("release", "release-group"): + scope = "release" + suffix = f"-{size}" if size else "" + return f"{COVER_ART_ARCHIVE_URL}/{scope}/{mbid}/front{suffix}" + + +def fetch_release_preferred_art( + release_mbid: Optional[str], + fallback_url: Optional[str], + *, + fetch_fn: Callable[[str], Tuple[Optional[bytes], Optional[str]]], + size: int = 1200, + min_bytes: int = 0, +) -> Tuple[Optional[bytes], Optional[str], Optional[str]]: + """Fetch the best cover, preferring the specific release's own art. + + Tries ``/release//front`` first; on any miss (no such art, + 404, or smaller than ``min_bytes``) falls back to ``fallback_url`` (a + release-group / provider cover). ``fetch_fn(url) -> (bytes|None, mime|None)``; + it is expected to return ``(None, None)`` on a 404, which is how a release + with no art of its own advances to the fallback. ``min_bytes`` defaults to 0 + (accept any non-empty image) to preserve the fallback path's prior behaviour; + callers can raise it to reject placeholder/error images. Returns + ``(bytes|None, mime|None, url_used|None)``. Never raises for a missing cover — + a failed candidate just advances to the next, so coverage never regresses.""" + candidates = [] + release_url = caa_front_url(release_mbid, "release", size) if release_mbid else None + if release_url: + candidates.append(release_url) + if fallback_url and fallback_url not in candidates: + candidates.append(fallback_url) + + for url in candidates: + try: + data, mime = fetch_fn(url) + except Exception: + data, mime = None, None + if data and len(data) > min_bytes: + return data, mime, url + return None, None, None + + +__all__ = ["COVER_ART_ARCHIVE_URL", "caa_front_url", "fetch_release_preferred_art"] diff --git a/core/metadata/enrichment.py b/core/metadata/enrichment.py index e3642419..69716a6c 100644 --- a/core/metadata/enrichment.py +++ b/core/metadata/enrichment.py @@ -131,6 +131,14 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf track_num_str = format_track_number_tag( metadata.get('track_number'), metadata.get('total_tracks') ) + # Disc number is written UNCONDITIONALLY (floored to >=1), like the + # track number above. The old code only wrote it when truthy, so a + # track whose disc came back 0/None/'' (e.g. matched to a different + # edition) lost its disc tag on the clear-then-rewrite and floated + # ungrouped above the disc sections in Jellyfin/Plex (Sokhi). + from core.imports.track_number import normalize_disc_number + _disc_num = normalize_disc_number(metadata.get('disc_number')) + disc_num_str = str(_disc_num) write_multi = cfg.get("metadata_enhancement.tags.write_multi_artist", False) artists_list = metadata.get("_artists_list", []) @@ -162,8 +170,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf if metadata.get("genre"): audio_file.tags.add(symbols.TCON(encoding=3, text=[metadata["genre"]])) audio_file.tags.add(symbols.TRCK(encoding=3, text=[track_num_str])) - if metadata.get("disc_number"): - audio_file.tags.add(symbols.TPOS(encoding=3, text=[str(metadata["disc_number"])])) + audio_file.tags.add(symbols.TPOS(encoding=3, text=[disc_num_str])) elif is_vorbis_like(audio_file, symbols): if metadata.get("title"): audio_file["title"] = [metadata["title"]] @@ -180,8 +187,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf if metadata.get("genre"): audio_file["genre"] = [metadata["genre"]] audio_file["tracknumber"] = [track_num_str] - if metadata.get("disc_number"): - audio_file["discnumber"] = [str(metadata["disc_number"])] + audio_file["discnumber"] = [disc_num_str] elif isinstance(audio_file, symbols.MP4): if metadata.get("title"): audio_file["\xa9nam"] = [metadata["title"]] @@ -198,8 +204,7 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf audio_file["trkn"] = [format_track_number_tuple( metadata.get("track_number"), metadata.get("total_tracks") )] - if metadata.get("disc_number"): - audio_file["disk"] = [(metadata["disc_number"], 0)] + audio_file["disk"] = [(_disc_num, 0)] embed_source_ids(audio_file, metadata, context, runtime=runtime) diff --git a/core/metadata/source.py b/core/metadata/source.py index ab44b241..b3b1f4ee 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1089,10 +1089,12 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di metadata["track_number"] = 1 metadata["total_tracks"] = 1 - disc_num = original_search.get("disc_number") - if disc_num is None and album_info: - disc_num = album_info.get("disc_number") - metadata["disc_number"] = disc_num if disc_num is not None else 1 + # Resolve via the SHARED resolver so the embedded tag and the "Disc N" folder + # (computed in the import pipeline from album_info) can never disagree — same + # function, same inputs. Floors to >=1 (a 0/''/non-numeric disc must not read + # as disc-less and ungroup the track in Jellyfin/Plex). + from core.imports.track_number import resolve_disc_for_track + metadata["disc_number"] = resolve_disc_for_track(original_search, album_info) if album_ctx and album_ctx.get("release_date"): release_date = _normalize_release_date_tag(album_ctx.get("release_date")) diff --git a/core/playlists/item_naming.py b/core/playlists/item_naming.py new file mode 100644 index 00000000..94e56790 --- /dev/null +++ b/core/playlists/item_naming.py @@ -0,0 +1,94 @@ +"""Optional custom naming for the FILES inside an organize-by-playlist folder. + +By default a playlist entry keeps the real library filename (the materialized +folder is a view onto Artist/Album/track.ext). A user can opt into a flat +filename template — e.g. ``$position - $artist - $title`` — so the folder sorts +and plays the way they want (most commonly: in playlist order on a dumb DAP). + +It is a **filename** template, never a path: + - it may NOT contain a path separator (``/`` or ``\\``) — it names the file, + not a folder tree, and + - it MUST contain ``$title`` — so every file has a real, non-empty name. + +Both rules are validated up front (so the Settings UI can reject a bad value +with a reason) AND re-checked at apply time, where an invalid/empty template or +an empty render falls back to the library filename. So a bad value can never +produce a broken name — the worst case is "no change from today". + +Pure logic: no DB, no config, no filesystem. The caller supplies the metadata. +""" + +from __future__ import annotations + +from typing import Optional, Tuple + +from core.imports.paths import sanitize_filename + +# Tokens a user may use in the template (for docs / UI hints). +PLAYLIST_ITEM_TOKENS = ("$position", "$artist", "$album", "$track", "$title") + + +def validate_playlist_item_template(template: Optional[str]) -> Tuple[bool, str]: + """Return ``(ok, reason)``. An empty template is VALID and means "feature off" + (keep the library filename). ``reason`` is '' when ok.""" + t = (template or "").strip() + if not t: + return True, "" # empty == disabled, not an error + if "/" in t or "\\" in t: + return False, ("Playlist file naming can't contain a folder separator " + "( / or \\ ) — it names the file, not a path.") + if "$title" not in t: + return False, "Playlist file naming must include $title so every file has a name." + return True, "" + + +def render_playlist_item_name( + template: Optional[str], + *, + title: str, + artist: str = "", + album: str = "", + track: object = None, + position: object = None, + ext: str = "", + fallback_name: str = "", +) -> str: + """Render ``template`` to a sanitized filename WITH ``ext`` appended. + + Falls back to ``fallback_name`` (the library filename) when the template is + empty/invalid or renders to nothing after sanitizing — so the result is + never broken. ``position`` is used verbatim (the caller pre-pads it for + correct sorting); ``track`` is zero-padded to two digits when numeric.""" + ok, _ = validate_playlist_item_template(template) + t = (template or "").strip() + if not ok or not t: + return fallback_name + + pos_str = "" if position is None else str(position) + if track is None: + trk_str = "" + else: + try: + trk_str = f"{int(track):02d}" + except (TypeError, ValueError): + trk_str = str(track) + + # No token is a prefix of another, so replacement order is irrelevant. + out = t + out = out.replace("$position", pos_str) + out = out.replace("$artist", str(artist or "")) + out = out.replace("$album", str(album or "")) + out = out.replace("$track", trk_str) + out = out.replace("$title", str(title or "")) + + out = sanitize_filename(out).strip() + if not out: + return fallback_name + return out + (ext or "") + + +__all__ = [ + "PLAYLIST_ITEM_TOKENS", + "validate_playlist_item_template", + "render_playlist_item_name", +] diff --git a/core/playlists/materialize.py b/core/playlists/materialize.py index f48a9001..dff26886 100644 --- a/core/playlists/materialize.py +++ b/core/playlists/materialize.py @@ -72,16 +72,25 @@ def playlist_dir_for(playlists_root: str, playlist_name: str) -> str: return candidate -def _desired_entries(playlist_dir: str, real_paths: Sequence[str]) -> "list[tuple[str, str]]": - """Map each real file to a flat destination inside ``playlist_dir``, preserving - the source filename. On a basename collision between two *different* sources, - disambiguate with a numeric suffix rather than silently overwriting.""" +def _desired_entries( + playlist_dir: str, + real_paths: Sequence[str], + dest_names: Optional[Sequence[Optional[str]]] = None, +) -> "list[tuple[str, str]]": + """Map each real file to a flat destination inside ``playlist_dir``. + + By default the source filename is preserved. ``dest_names`` (parallel to + ``real_paths``) lets a caller override the name per entry — e.g. a custom + playlist file-naming template; a falsy override falls back to the source + basename. On a name collision between two *different* sources, disambiguate + with a numeric suffix rather than silently overwriting.""" entries: list[tuple[str, str]] = [] used: dict[str, str] = {} # dest basename -> source real path - for real in real_paths: + for i, real in enumerate(real_paths): if not real: continue - base = os.path.basename(real) + override = dest_names[i] if (dest_names is not None and i < len(dest_names)) else None + base = override or os.path.basename(real) name = base stem, ext = os.path.splitext(base) counter = 1 @@ -156,6 +165,7 @@ def rebuild_playlist_folder( real_paths: Sequence[str], mode: str = DEFAULT_MODE, *, + dest_names: Optional[Sequence[Optional[str]]] = None, prune_stale: bool = True, symlink_fn: Callable[[str, str], None] = os.symlink, copy_fn: Callable[[str, str], object] = shutil.copy2, @@ -163,13 +173,15 @@ def rebuild_playlist_folder( """(Re)build ``playlists_root//`` so it contains exactly one entry per real file in ``real_paths`` — adding missing entries, leaving correct ones untouched, and (when ``prune_stale``) removing entries no longer present. + ``dest_names`` (parallel to ``real_paths``) optionally overrides each entry's + filename (custom playlist naming); falsy entries keep the source basename. Idempotent and safe to re-run any time. Filesystem ops are injectable.""" mode = normalize_mode(mode) pdir = playlist_dir_for(playlists_root, playlist_name) summary = RebuildSummary(playlist_dir=pdir, mode_requested=mode) os.makedirs(pdir, exist_ok=True) - entries = _desired_entries(pdir, real_paths) + entries = _desired_entries(pdir, real_paths, dest_names) keep = {dest for _real, dest in entries} for real, dest in entries: diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index c8121b86..c00124eb 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -155,11 +155,20 @@ def _rebuild_one_from_db(db, config_manager, playlist: dict): source IDs), resolving to disk, and rebuilding WITH prune. Because it's driven by current membership, a track that has LEFT the playlist drops out of the set and its symlink is pruned. Returns ``(playlist_name, RebuildSummary)``.""" + import os as _os + from core.library.path_resolver import resolve_library_file_path + from core.playlists.item_naming import render_playlist_item_name root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) - real_paths: List[str] = [] + item_template = ((config_manager.get("file_organization.templates", {}) or {}) + .get("playlist_item", "") or "").strip() + + # Resolve owned tracks to real paths IN PLAYLIST ORDER, keeping the metadata + # so an optional custom filename template ($position/$artist/$title/...) can + # be applied. $position is the playlist index, which is exactly this order. + resolved: List[dict] = [] seen = set() for t in (db.get_mirrored_playlist_tracks(playlist["id"]) or []): title = (t.get("track_name") or "").strip() @@ -175,9 +184,32 @@ def _rebuild_one_from_db(db, config_manager, playlist: dict): real = resolve_library_file_path(getattr(db_track, "file_path", None), config_manager=config_manager) if real and real not in seen: seen.add(real) - real_paths.append(real) + resolved.append({ + "real": real, + "title": title, + "artist": artist, + "album": (t.get("album_name") or t.get("album") or "").strip(), + "track": getattr(db_track, "track_number", None), + }) + + real_paths: List[str] = [r["real"] for r in resolved] + + dest_names = None + if item_template: + width = max(2, len(str(len(resolved)))) # zero-pad $position for correct sorting + dest_names = [ + render_playlist_item_name( + item_template, + title=r["title"], artist=r["artist"], album=r["album"], track=r["track"], + position=f"{i:0{width}d}", + ext=_os.path.splitext(r["real"])[1], + fallback_name=_os.path.basename(r["real"]), + ) + for i, r in enumerate(resolved, start=1) + ] + name = playlist.get("name") or "Unnamed Playlist" - return name, rebuild_playlist_folder(root, name, real_paths, mode) # prune_stale=True + return name, rebuild_playlist_folder(root, name, real_paths, mode, dest_names=dest_names) # prune_stale=True def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): diff --git a/core/playlists/source_refs.py b/core/playlists/source_refs.py index 4cf03534..0e915ca4 100644 --- a/core/playlists/source_refs.py +++ b/core/playlists/source_refs.py @@ -19,6 +19,35 @@ from urllib.parse import parse_qs, urlparse _SPOTIFY_ID_RE = re.compile(r"^[A-Za-z0-9]{16,32}$") + +def stable_source_track_id(track: Mapping, existing: Optional[str] = None) -> str: + """A stable per-track id for a mirrored-playlist track. + + Spotify / YouTube / Deezer tracks carry a native id. File-import (CSV / M3U / + TXT) and iTunes-only sources don't — they arrive with an empty + ``source_track_id``. The whole manual-match system (Find & Add ↔ sync) keys on + ``source_track_id``, and an empty key can neither be recorded (the persist is a + no-op) nor looked up — so a manual match on a file-import track is silently + dropped and the track re-appears as "extra" (#901). + + When a native id is present it's used verbatim. Otherwise we derive a + DETERMINISTIC id from the track's identity (artist|title|album, normalized) so + the SAME song gets the SAME id across re-imports and discovery passes — which + is exactly what the match lookup needs. Prefixed ``file:`` so it's recognizable + and never collides with a real upstream id. Returns '' only when there's no + usable identity at all (no title).""" + native = (existing if existing is not None else track.get("source_track_id")) or "" + native = str(native).strip() + if native: + return native + title = str(track.get("track_name") or track.get("name") or "").strip().lower() + if not title: + return "" + artist = str(track.get("artist_name") or track.get("artist") or "").strip().lower() + album = str(track.get("album_name") or track.get("album") or "").strip().lower() + digest = hashlib.md5(f"{artist}|{title}|{album}".encode("utf-8")).hexdigest()[:16] + return f"file:{digest}" + # Synthetic batch playlist_id prefixes that wrap a mirrored_playlists PK. # Download/discovery flows build a batch playlist_id as f"{prefix}{pk}" — e.g. # auto_mirror_ (core/automation/handlers/sync_playlist.py), youtube_mirrored_ diff --git a/core/wishlist/routes.py b/core/wishlist/routes.py index a467676c..c9536828 100644 --- a/core/wishlist/routes.py +++ b/core/wishlist/routes.py @@ -501,6 +501,10 @@ def add_album_track_to_wishlist( source_type=source_type, source_context=enhanced_source_context, profile_id=runtime.profile_id, + # Explicit user click in the album modal — must bypass + clear the + # ignore-list, even if the user previously cancelled this track + # (otherwise the add is silently dropped — carlosjfcasero, #897). + user_initiated=True, ) if success: diff --git a/core/wishlist/service.py b/core/wishlist/service.py index df3be19a..1f76f2d9 100644 --- a/core/wishlist/service.py +++ b/core/wishlist/service.py @@ -100,6 +100,7 @@ class WishlistService: source_type: str = "manual", source_context: Dict[str, Any] = None, profile_id: int = 1, + user_initiated: bool = False, ) -> bool: """ Directly add a track to the wishlist. @@ -110,6 +111,8 @@ class WishlistService: source_type: Source type ('playlist', 'album', 'manual') source_context: Additional context information profile_id: Profile to add to + user_initiated: True for an explicit user add — bypasses + clears the + ignore-list while keeping the real source_type (#874/#897). """ if track_data is None: track_data = spotify_track_data @@ -124,6 +127,7 @@ class WishlistService: source_type=source_type, source_info=source_context or {}, profile_id=profile_id, + user_initiated=user_initiated, ) def add_spotify_track_to_wishlist( diff --git a/core/youtube_cookies.py b/core/youtube_cookies.py new file mode 100644 index 00000000..c61086fe --- /dev/null +++ b/core/youtube_cookies.py @@ -0,0 +1,105 @@ +"""YouTube cookie options for yt-dlp — a browser store *or* a pasted cookies.txt. + +Settings → YouTube offers two ways to authenticate yt-dlp: + +* a **browser dropdown** (Chrome/Firefox/…) → yt-dlp ``cookiesfrombrowser``, which + reads a logged-in browser's cookie store *on the same machine as SoulSync*. Great + for local installs, useless on a headless server / Docker box (no browser there). +* a **"Paste cookies.txt"** mode → yt-dlp ``cookiefile``, a Netscape-format cookie + file the user exports (e.g. with a "Get cookies.txt LOCALLY" extension) and pastes + in. This is the only path that works for server/Docker users, and it's what makes + *private* playlists — a user's "Liked Music" (``list=LM``) — actually visible. + +This module centralises the precedence and the pasted-file validation so the live +opts (:func:`build_youtube_cookie_opts`) and the settings-save write agree, and so +the seam is unit-testable without I/O. The web layer owns *where* the file lives +(next to ``config.json``); this module only decides the opts and validates content. +""" + +from __future__ import annotations + +import os +from typing import Any, Dict + +# Sentinel dropdown value meaning "use a pasted cookies.txt file" rather than a +# browser name. Anything else non-empty is treated as a browser for cookiesfrombrowser. +PASTE_MODE = "custom" + + +def build_youtube_cookie_opts( + mode: Any, + cookiefile_path: str = "", + *, + cookiefile_exists: bool = False, +) -> Dict[str, Any]: + """Return the yt-dlp cookie options for a given Settings→YouTube ``mode``. Pure. + + * ``mode == PASTE_MODE`` → ``{'cookiefile': path}`` when the file exists, else + ``{}`` (a stale/missing path must never become a broken cookiefile arg). + * ``mode`` is any other non-empty string → ``{'cookiesfrombrowser': (mode,)}``. + * ``mode`` falsy → ``{}`` (anonymous; public playlists only). + + Precedence is structural: a browser name is never ``PASTE_MODE``, so the two + cookie sources can't both be emitted. No I/O here — the caller passes + ``cookiefile_exists`` (the ``os.path.exists`` result) so this stays pure. + """ + m = str(mode or "").strip() + if m == PASTE_MODE: + if cookiefile_path and cookiefile_exists: + return {"cookiefile": str(cookiefile_path)} + return {} + if m: + return {"cookiesfrombrowser": (m,)} + return {} + + +def looks_like_cookiefile(content: Any) -> bool: + """True when ``content`` plausibly is a Netscape/Mozilla ``cookies.txt``. + + Requires at least one real cookie row — a non-comment line with >= 6 TAB-separated + fields (domain, flag, path, secure, expiry, name[, value]). The ``# Netscape HTTP + Cookie File`` header alone is NOT enough: a header-only paste carries no auth and + would silently save a useless file. This guards the save path so pasting junk (a + URL, JSON, or just the header) is rejected up front instead of being written out + and making yt-dlp raise mid-extraction. + """ + if not content or not isinstance(content, str): + return False + for raw in content.splitlines(): + line = raw.rstrip("\n") + if not line or line.lstrip().startswith("#"): + continue + if len(line.split("\t")) >= 6: + return True + return False + + +def write_pasted_cookiefile(content: Any, dest_path: str) -> str: + """Validate + write a pasted ``cookies.txt`` to ``dest_path``. + + Returns the written path on success, or ``""`` when the content is empty / + doesn't look like a cookie file / can't be written — in which case the caller + leaves any existing file untouched (a blank save must not wipe a saved cookie). + Best-effort ``0600`` perms since the file holds live session secrets. + """ + if not looks_like_cookiefile(content): + return "" + try: + text = content if content.endswith("\n") else content + "\n" + with open(dest_path, "w", encoding="utf-8") as fh: + fh.write(text) + try: + os.chmod(dest_path, 0o600) + except OSError: + pass + return str(dest_path) + except OSError: + return "" + + +__all__ = [ + "PASTE_MODE", + "build_youtube_cookie_opts", + "looks_like_cookiefile", + "write_pasted_cookiefile", +] diff --git a/database/music_database.py b/database/music_database.py index fbfe28e6..3b6a17fb 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -993,6 +993,39 @@ class MusicDatabase: raise self._init_manual_library_match_table() + self._backfill_mirrored_track_source_ids() + + def _backfill_mirrored_track_source_ids(self) -> int: + """One-time, idempotent: assign a stable source_track_id to mirrored tracks + that have none (file-import / iTunes-only playlists imported before #901), so + their existing Find & Add matches start sticking without a manual re-import. + Only touches empty-id rows, so it's a no-op once they're filled.""" + from core.playlists.source_refs import stable_source_track_id + updated = 0 + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(""" + SELECT id, track_name, artist_name, album_name + FROM mirrored_playlist_tracks + WHERE source_track_id IS NULL OR source_track_id = '' + """) + rows = cursor.fetchall() + for r in rows: + sid = stable_source_track_id({ + 'track_name': r['track_name'], 'artist_name': r['artist_name'], + 'album_name': r['album_name']}) + if sid: + cursor.execute( + "UPDATE mirrored_playlist_tracks SET source_track_id = ? WHERE id = ?", + (sid, r['id'])) + updated += 1 + conn.commit() + if updated: + logger.info("Backfilled stable source_track_id on %d mirrored tracks (#901)", updated) + except Exception as e: + logger.error("mirrored track source_id backfill failed: %s", e) + return updated # Bump when the schema's generation meaningfully changes. Stamped into # PRAGMA user_version as a backstop indicator; nothing GATES on it yet. @@ -2053,6 +2086,31 @@ class MusicDatabase: "ON mb_album_release_cache (release_mbid)" ) + # Persistent (artist,title) -> recording MBID cache for playlist export (#903). + # The MusicBrainz tail of the export waterfall is rate-limited (~1 req/s), so a + # resolved recording MBID is remembered ONCE and reused for that song across every + # future export and playlist. Additive: empty until the export feature writes it. + cursor.execute(""" + CREATE TABLE IF NOT EXISTS mb_recording_cache ( + track_key TEXT PRIMARY KEY, + recording_mbid TEXT NOT NULL, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + ) + """) + + # Remember which external playlist a mirrored playlist was exported to, so a + # re-export UPDATES it in place instead of creating a duplicate (#903). Keyed by + # (mirrored playlist, target service) -> the target's playlist id (LB recording MBID). + cursor.execute(""" + CREATE TABLE IF NOT EXISTS playlist_export_targets ( + mirrored_playlist_id INTEGER NOT NULL, + target TEXT NOT NULL, + target_playlist_mbid TEXT NOT NULL, + updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (mirrored_playlist_id, target) + ) + """) + # Discovery artist blacklist — artists users never want to see in discovery cursor.execute(""" CREATE TABLE IF NOT EXISTS discovery_artist_blacklist ( @@ -7289,6 +7347,18 @@ class MusicDatabase: variations.append(normalized_name.title()) variations.append(normalized_name) + # Leading-"The" toggle — a leading "The" is noise for artist identity + # ("The Black Eyed Peas" == "Black Eyed Peas"). Without this, a request for + # one variant never fetches a library track filed under the other, so it + # "fails to match" and re-downloads a duplicate. Search BOTH forms; the + # confidence scorer still decides (50/50 title/artist), so this only widens + # the candidate fetch — it can't merge genuinely different artists on its own. + stripped = artist_name.strip() + if stripped.lower().startswith("the ") and stripped[4:].strip(): + variations.append(stripped[4:].strip()) # "The Black Eyed Peas" -> "Black Eyed Peas" + elif stripped: + variations.append("The " + stripped) # "Black Eyed Peas" -> "The Black Eyed Peas" + # Add more aliases here in the future if "korn" in name_lower: if "KoЯn" not in variations: @@ -8901,8 +8971,15 @@ class MusicDatabase: source_info: Dict[str, Any] = None, profile_id: int = 1, track_data: Dict[str, Any] = None, + user_initiated: bool = False, ) -> bool: - """Add a failed track to the wishlist for retry""" + """Add a failed track to the wishlist for retry. + + ``user_initiated`` marks an explicit user add (e.g. the library album + "add to wishlist" modal). Like ``source_type == 'manual'`` it bypasses + the ignore-list gate AND clears any stale ignore — but unlike changing + ``source_type`` it preserves the real provenance ('album'), which the + wishlist categorisation (Albums vs Singles) relies on (#874/#897).""" try: if track_data is not None and spotify_track_data is None: spotify_track_data = track_data @@ -8932,7 +9009,7 @@ class MusicDatabase: # clear any stale ignore so it sticks. Fail-open: any error here # must never block a legitimate wishlist add. try: - if source_type == 'manual': + if source_type == 'manual' or user_initiated: self.remove_from_wishlist_ignore(track_id, profile_id=profile_id) elif self.is_track_ignored(track_id, profile_id=profile_id): logger.info("Skipping wishlist add — track is on the ignore-list (#874): %s", track_id) @@ -13925,16 +14002,20 @@ class MusicDatabase: logger.debug("Failed to preserve mirrored playlist extra_data: %s", e) # Replace all tracks + from core.playlists.source_refs import stable_source_track_id cursor.execute("DELETE FROM mirrored_playlist_tracks WHERE playlist_id=?", (playlist_id,)) for i, t in enumerate(tracks): + # File-import / iTunes-only tracks arrive with no native id; give + # them a DETERMINISTIC one so a Find & Add manual match can be + # recorded and found (it keys on source_track_id) instead of being + # silently dropped and re-appearing as "extra" (#901). + sid = stable_source_track_id(t) extra = t.get('extra_data') if extra and not isinstance(extra, str): extra = json.dumps(extra) # Restore preserved discovery data if the incoming track doesn't have its own - if not extra: - sid = t.get('source_track_id') - if sid and sid in old_extra_map: - extra = old_extra_map[sid] + if not extra and sid and sid in old_extra_map: + extra = old_extra_map[sid] cursor.execute(""" INSERT INTO mirrored_playlist_tracks (playlist_id, position, track_name, artist_name, album_name, duration_ms, image_url, source_track_id, extra_data) @@ -13943,7 +14024,7 @@ class MusicDatabase: playlist_id, i + 1, t.get('track_name', ''), t.get('artist_name', ''), t.get('album_name', ''), t.get('duration_ms', 0), - t.get('image_url'), t.get('source_track_id'), extra + t.get('image_url'), sid or None, extra )) conn.commit() logger.info(f"Mirrored playlist '{name}' ({source}) with {len(tracks)} tracks") @@ -14110,6 +14191,42 @@ class MusicDatabase: logger.error(f"Error updating custom_name for playlist {playlist_id}: {e}") return False + def get_playlist_export_target(self, mirrored_playlist_id: int, target: str) -> Optional[str]: + """The external playlist id this mirror was last exported to (or None). #903.""" + try: + with self._get_connection() as conn: + cur = conn.cursor() + cur.execute( + "SELECT target_playlist_mbid FROM playlist_export_targets " + "WHERE mirrored_playlist_id = ? AND target = ? LIMIT 1", + (int(mirrored_playlist_id), target), + ) + row = cur.fetchone() + if row: + return (row[0] if not hasattr(row, "keys") else row["target_playlist_mbid"]) or None + except Exception as e: + logger.debug(f"get_playlist_export_target failed: {e}") + return None + + def set_playlist_export_target(self, mirrored_playlist_id: int, target: str, target_mbid: str) -> bool: + """Remember the external playlist id for this mirror (idempotent). #903.""" + if not target_mbid: + return False + try: + with self._get_connection() as conn: + cur = conn.cursor() + cur.execute( + "INSERT OR REPLACE INTO playlist_export_targets " + "(mirrored_playlist_id, target, target_playlist_mbid, updated_at) " + "VALUES (?, ?, ?, CURRENT_TIMESTAMP)", + (int(mirrored_playlist_id), target, target_mbid), + ) + conn.commit() + return True + except Exception as e: + logger.debug(f"set_playlist_export_target failed: {e}") + return False + def get_mirrored_playlist_tracks(self, playlist_id: int) -> List[Dict]: """Return all tracks for a mirrored playlist ordered by position.""" try: diff --git a/pr_description.md b/pr_description.md index 365c2dfa..501ebcc7 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,37 +1,41 @@ -# soulsync 2.7.4 — `dev` → `main` +# soulsync 2.7.6 — `dev` → `main` -patch release on top of 2.7.3. headline is **re-identify** — re-file an already-imported track under the right release without re-downloading it. +patch release on top of 2.7.5. the headline is going the *other* way with playlists — exporting them TO listenbrainz — plus youtube liked-music sync, a deep-scan data-loss guard, and a round of dashboard performance work. --- ## what's new -### re-identify a track (#889) -filed a track under the wrong release (single vs ep vs album)? there's now a ⇄ button in the library Enhanced view that lets you fix it. search any configured source (tabs, defaults to your active one), see the same song across its single / ep / album with type badges, pick the right one, and soulsync re-files the file you already have under that release — correct year, in-album track number, and art. opt to replace the original entry or keep both. +### export playlists to listenbrainz (#903) +soulsync already pulls playlists IN from everywhere — now it can push one back OUT. every mirrored-playlist card gets a 📤 export button: pick **download .jspf** (a standard playlist file you can hand-upload anywhere) or **sync to listenbrainz** (creates the playlist straight on your LB account). each track is matched to its musicbrainz *recording* id via a cheapest-first waterfall — cache → your library (`musicbrainz_recording_id`) → the file's own tag → a live musicbrainz lookup — with the result cached so the same song never costs twice. live "matching N/M · X matched" status shows on the card, and re-syncing **updates the same LB playlist in place** instead of making duplicates. tracks that can't be resolved to an MBID are skipped (LB requires them) and counted so you see the coverage. -built additively over 5 phases (hint store → import seam → multi-source search → modal → button), all riding the existing import pipeline so a no-hint import is byte-identical to before. and it can't lose your file: replace deletes the old entry only *after* the re-import lands, and never if you pick the release it's already in. +### youtube liked-music sync (#902) +you can now sync your youtube music **Liked Music** playlist (`music.youtube.com/playlist?list=LM`). it's a private playlist, so it needs auth — and the existing "read a browser's cookies" option only works when the browser is on the same machine. added a **"paste cookies.txt"** option in Settings → YouTube so server/docker installs (and anyone on a browser like Zen that yt-dlp can't read) can supply their login from anywhere. -### cleaner libraries & imports -- **#890** — track titles no longer keep the "01 - " prefix from the filename when there's no embedded title tag (which made the real track read as a false "missing"). stripped conservatively so "7 Rings" / "1-800-273-8255" / "1979" are left alone. -- **#891** — a Library Reorganize now sweeps the leftover cover.jpg / .lrc / sidecars from the old folder so it actually empties, plus an opt-in "Remove Residual Files" toggle on the Empty Folder Cleaner for the image-only folders you already have. -- **Sokhi's batch** — same-album songs group under one canonical release id (no more split discographies / mixed cover art); a single can match its parent album; a mid-enrichment crash on an art-less file no longer leaves it untagged; and a sequel digit glued to a CJK title no longer matches the wrong album. +### deep scan won't relocate your library (#904) +a standalone Deep Scan moves files it doesn't recognize into Staging for import. if the DB was empty/out of sync with disk (a volume swap, a DB reset, external tag edits), it treated your **entire** library as "unrecognized" and relocated all of it — one user lost ~1,500 tracks into Staging. now a guard refuses the move when the unrecognized share is implausibly large (the desync signature), leaves everything in place, and warns instead. plus a **"Transfer is my permanent library — never move files out"** toggle for people whose Transfer folder *is* their live library. -### quality & sources -- **#886** — AAC (.m4a) as an opt-in soulseek quality tier, ranked above mp3 / below flac. off by default; existing profiles unchanged until you enable it. -- **#887** — enrichment on Spotify Free now reads "Running (Spotify Free)" instead of wrongly showing "Not Authenticated". -- **#884** — NZBGet imports from the finished location, not the incomplete "….#NZBID" folder. -- **#885** — setting the timezone to Australia/Sydney no longer makes the cache-maintenance job loop every 5 seconds. +### dashboard performance +a pass at the "soulsync makes my GPU work hard just sitting there" complaints. the sidebar sweep animates `transform` instead of `left` (no per-frame layout), particle glows are pre-rendered sprites instead of per-frame gradients, blur radii + redundant/invisible card shadows are trimmed, and low-power machines auto-drop to performance mode. all the visible effects stay; they're just cheaper to draw. -### polish -- the artist-detail header no longer bleeds the blurred artist photo behind it. +### fixes +- **file-import manual matches stick (#901)** — a manual match on a file-imported playlist track is no longer forgotten on re-sync (the tracks now carry a stable id; existing ones are backfilled once). +- **manual match heals a stale Plex key** — a Find & Add match whose stored Plex ratingKey went stale is now re-resolved against a live Plex search instead of silently breaking. +- **multi-disc albums** — a track now files into the Disc folder that matches its own disc tag (no more disc-2 tracks landing in Disc 1), and a track is never written disc-less. +- **auto-download track numbers** — a track auto-grabbed from the playlist pipeline / wishlist / watchlist now gets its real in-album position instead of being tagged `1/1`. + +--- + +## a brief recap of what came before +2.7.5 was a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess (#895) — plus M3U import (#893), ignore-list management (#897), and per-playlist file naming. 2.7.4 was re-identify; 2.7.3 the Quality Upgrade Finder + ignore-list (#874); 2.7.2 playlist-folder mirroring + M3U export; 2.7.1 download verification + a review queue (#852); 2.7.0 made multi-user real. --- ## tests -strictly additive across the board — every new behavior is opt-in or gated so default flows are unchanged. ~100 new tests this cycle (re-identify seam, title-strip danger cases, the shared residual-file classifier, aac tier, tz scheduler, spotify-free status). full imports / matching / reorganize / auto-import suites green, ruff clean. +additive + fail-safe — new behavior is opt-in or guarded, nothing existing rewired. new seam/regression suites across the listenbrainz export (JSPF build, the MBID waterfall + dedup, the persistent cache, the LB create/update-in-place client), the youtube cookie precedence, and the #904 deep-scan guard (incl. the 1,500-file regression). the listenbrainz push + update-in-place were also validated live against a real account. relevant suites green; `ruff check` clean on touched modules. ## post-merge -- [ ] tag `v2.7.4` on `main` -- [ ] docker-publish with `version_tag: 2.7.4` +- [ ] tag `v2.7.6` on `main` +- [ ] docker-publish with `version_tag: 2.7.6` - [ ] discord announce (auto-fired by the workflow) -- [ ] reply on #889 / #890 / #891 +- [ ] reply on #902 / #903 / #904 diff --git a/services/sync_service.py b/services/sync_service.py index b48aa051..c1213f1a 100644 --- a/services/sync_service.py +++ b/services/sync_service.py @@ -28,6 +28,71 @@ def _artist_name(artist) -> str: return name return str(artist) if artist is not None else '' + +def _plex_track_file(plex_track) -> str: + """Best-effort file path of a live Plex track object (media[0].parts[0].file).""" + try: + return plex_track.media[0].parts[0].file or '' + except Exception: + return '' + + +def reresolve_manual_match_live_plex(cache_db, media_client, m, *, profile_id, + source_track_id, server_source): + """Re-resolve a manual match whose stored Plex ratingKey went stale. + + Plex re-keys tracks on a metadata refresh/optimize, and the SoulSync DB id is + itself that old ratingKey — so until a SoulSync rescan, BOTH the stored + ``library_track_id`` and the file-path self-heal land on the same dead key and + ``fetchItem`` 404s. The only live source of truth is Plex, so search it by the + matched track's metadata, disambiguate by the stored file path (so the user's + EXACT chosen track wins when several versions exist), heal the stored id, and + return the current live Plex track. Best-effort: returns the live track or + None, never raises — a manual match must never be dropped on a transient miss. + """ + try: + title = (m.get('source_title') or '').strip() + artist = (m.get('source_artist') or '').strip() + file_path = (m.get('library_file_path') or '').strip() + if not title: + return None + results = media_client.search_tracks(title, artist, limit=15) or [] + if not results: + return None + + import os as _os + want_base = _os.path.basename(file_path.replace('\\', '/')) if file_path else '' + chosen = None + if want_base: + for t in results: + live = getattr(t, '_original_plex_track', None) + p = _plex_track_file(live) if live is not None else '' + if p and _os.path.basename(p.replace('\\', '/')) == want_base: + chosen = t + break + if chosen is None: + chosen = results[0] # search_tracks already ranks artist→title; best effort + live = getattr(chosen, '_original_plex_track', None) + if live is None or not hasattr(live, 'ratingKey'): + return None + + # Heal the stored id (+ the cache, done by the caller) so the next sync is + # fast and doesn't repeat the live search. + try: + cache_db.save_manual_library_match( + profile_id, m.get('source') or 'spotify', str(source_track_id), str(live.ratingKey), + source_title=m.get('source_title'), source_artist=m.get('source_artist'), + source_album=m.get('source_album'), + server_source=server_source, + library_file_path=file_path or _plex_track_file(live), + ) + except Exception as _heal_err: + logger.debug("manual-match heal (save) failed: %s", _heal_err) + return live + except Exception: + return None + + @dataclass class SyncResult: playlist_name: str @@ -599,38 +664,81 @@ class PlaylistSyncService: spotify_id = getattr(spotify_track, 'id', '') or '' active_server = config_manager.get_active_media_server() - # --- Sync match cache fast-path --- + # --- User-confirmed match fast-path (Find & Add / manual match) --- if spotify_id: + cache_db = MusicDatabase() + + def _materialize(server_track_id): + """Turn a stored library track id into the actual server item the + sync needs (DB row for Jellyfin/Navidrome/SoulSync, Plex fetchItem).""" + if server_track_id is None: + return None + dbt = cache_db.get_track_by_id(server_track_id) + if not dbt: + return None + if server_type in ("jellyfin", "navidrome", "soulsync"): + class DbTrackFromCache: + def __init__(self, db_t): + self.ratingKey = db_t.id + self.title = db_t.title + self.id = db_t.id + return DbTrackFromCache(dbt) + try: + at = media_client.server.fetchItem(int(server_track_id)) + return at if (at and hasattr(at, 'ratingKey')) else None + except Exception: + return None + + # 1) Volatile sync_match_cache — fast, but wiped on every library rescan. try: - cache_db = MusicDatabase() cached = cache_db.read_sync_match_cache(spotify_id, active_server) if cached: - server_track_id = cached['server_track_id'] - db_track_check = cache_db.get_track_by_id(server_track_id) - if db_track_check: - if server_type in ("jellyfin", "navidrome", "soulsync"): - class DbTrackFromCache: - def __init__(self, db_t): - self.ratingKey = db_t.id - self.title = db_t.title - self.id = db_t.id - actual_track = DbTrackFromCache(db_track_check) - else: - try: - actual_track = media_client.server.fetchItem(int(server_track_id)) - if not (actual_track and hasattr(actual_track, 'ratingKey')): - actual_track = None - except Exception: - actual_track = None - - if actual_track: - logger.debug(f"Sync cache hit: '{original_title}' → server track {server_track_id}") - return actual_track, cached['confidence'] - - logger.debug(f"Sync cache stale for '{original_title}' — track {server_track_id} gone") + actual_track = _materialize(cached['server_track_id']) + if actual_track: + logger.debug(f"Sync cache hit: '{original_title}' → server track {cached['server_track_id']}") + return actual_track, cached['confidence'] + logger.debug(f"Sync cache stale for '{original_title}' — track {cached['server_track_id']} gone") except Exception as cache_err: logger.debug(f"Sync cache lookup error: {cache_err}") - # --- End cache fast-path --- + + # 2) Durable manual library match (#787) — SURVIVES a rescan (the cache + # above does not). Without this, a user's Find & Add pairing is + # re-matched from scratch on the next auto-sync after a library scan, + # so they have to Find & Add the same track again (#895 follow-up). + # Self-heals a stale library id via the stored file path. + try: + from core.artists.map import get_current_profile_id + _profile_id = get_current_profile_id() + m = cache_db.find_manual_library_match_by_source_track_id( + _profile_id, str(spotify_id), active_server) + if m: + actual_track = _materialize(m.get('library_track_id')) + if not actual_track and m.get('library_file_path'): + new_id = cache_db.find_track_id_by_file_path(m['library_file_path']) + actual_track = _materialize(new_id) + # Plex re-keys tracks on a metadata refresh, and the SoulSync + # DB id IS that ratingKey — so both lookups above can land on + # the same stale key and 404. Re-resolve against LIVE Plex by + # the matched track's metadata so a manual match is NEVER + # dropped or silently re-matched by the fuzzy path below. + if not actual_track and server_type == "plex": + actual_track = reresolve_manual_match_live_plex( + cache_db, media_client, m, + profile_id=_profile_id, source_track_id=spotify_id, + server_source=active_server) + if actual_track and spotify_id: + try: + cache_db.save_sync_match_cache( + spotify_id, original_title, _artist_name(spotify_track.artists[0]) if spotify_track.artists else '', + active_server, actual_track.ratingKey, getattr(actual_track, 'title', original_title), 1.0) + except Exception as _cache_err: + logger.debug("sync cache heal failed: %s", _cache_err) + if actual_track: + logger.info(f"Durable manual match honored for '{original_title}' → {getattr(actual_track, 'ratingKey', m.get('library_track_id'))}") + return actual_track, 1.0 + except Exception as durable_err: + logger.debug(f"Durable manual match lookup error: {durable_err}") + # --- End match fast-path --- # Try each artist (same as modal logic) for artist in spotify_track.artists: diff --git a/templates/soulsync.xml b/templates/soulsync.xml index 0b2964a7..fda29e51 100644 --- a/templates/soulsync.xml +++ b/templates/soulsync.xml @@ -13,8 +13,8 @@ Music discovery and automation platform. Find new music, curate playlists, sync libraries, and integrate with popular streaming services, Soulseek (slskd), and media servers. MediaApp:Music http://[IP]:[PORT:8008] - https://raw.githubusercontent.com/snuffomega/SoulSync_unraid/main/soulsync.xml - https://raw.githubusercontent.com/snuffomega/SoulSync_unraid/main/soulsync.png + https://raw.githubusercontent.com/Nezreka/SoulSync/main/templates/soulsync.xml + https://raw.githubusercontent.com/Nezreka/SoulSync/main/templates/soulsync.png @@ -29,6 +29,7 @@ /mnt/user/appdata/soulsync/data /mnt/user/downloads/ /mnt/user/library/ + /mnt/user/media/music-videos/ 99 100 America/New_York diff --git a/tests/database/test_mirrored_playlists.py b/tests/database/test_mirrored_playlists.py index 8da26480..c1d0b6b6 100644 --- a/tests/database/test_mirrored_playlists.py +++ b/tests/database/test_mirrored_playlists.py @@ -60,3 +60,68 @@ def test_mirror_playlist_refresh_preserves_existing_description(tmp_path): assert refreshed_id == playlist_id playlist = db.get_mirrored_playlist(playlist_id) assert playlist["description"] == "https://open.spotify.com/playlist/abc" + + +def test_file_import_tracks_get_a_stable_source_track_id(tmp_path): + # #901: file-import tracks arrive with no source_track_id; mirror_playlist must + # assign a deterministic one so a Find & Add manual match can key on it (and so + # discovery extra_data survives a re-import). + db = MusicDatabase(str(tmp_path / "music.db")) + file_tracks = [ + {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"}, + {"track_name": "I Gotta Feeling", "artist_name": "The Black Eyed Peas"}, + ] + pid = db.mirror_playlist(source="file", source_playlist_id="myfile", name="From File", + tracks=file_tracks, profile_id=1) + rows = db.get_mirrored_playlist_tracks(pid) + ids = [r["source_track_id"] for r in rows] + assert all(i and i.startswith("file:") for i in ids) # no empty ids + assert len(set(ids)) == 2 # distinct per song + + # Re-import the SAME file → SAME ids (stable), so a recorded match still keys. + db.mirror_playlist(source="file", source_playlist_id="myfile", name="From File", + tracks=list(file_tracks), profile_id=1) + rows2 = db.get_mirrored_playlist_tracks(pid) + assert [r["source_track_id"] for r in rows2] == ids + + +def test_native_ids_still_used_verbatim(tmp_path): + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="spotify", source_playlist_id="sp", name="Sp", + tracks=[{"track_name": "S", "artist_name": "A", "source_track_id": "spotify123"}], + profile_id=1) + rows = db.get_mirrored_playlist_tracks(pid) + assert rows[0]["source_track_id"] == "spotify123" # native id untouched + + +def test_backfill_fills_existing_empty_ids_idempotently(tmp_path): + # #901 backfill: a file-import playlist mirrored BEFORE the fix has empty-id rows. + # The backfill assigns the SAME stable ids a fresh import would, so existing + # Find & Add matches start working without a re-import. + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="file", source_playlist_id="old", name="Old", + tracks=[{"track_name": "Slow Ride", "artist_name": "Foghat"}], profile_id=1) + # simulate a pre-fix row: blank out the id + with db._get_connection() as conn: + conn.execute("UPDATE mirrored_playlist_tracks SET source_track_id = '' WHERE playlist_id = ?", (pid,)) + conn.commit() + + n = db._backfill_mirrored_track_source_ids() + assert n == 1 + rows = db.get_mirrored_playlist_tracks(pid) + from core.playlists.source_refs import stable_source_track_id + assert rows[0]["source_track_id"] == stable_source_track_id( + {"track_name": "Slow Ride", "artist_name": "Foghat"}) # same id a fresh import gives + + # idempotent — second run touches nothing + assert db._backfill_mirrored_track_source_ids() == 0 + + +def test_backfill_leaves_native_ids_untouched(tmp_path): + db = MusicDatabase(str(tmp_path / "music.db")) + pid = db.mirror_playlist(source="spotify", source_playlist_id="sp", name="Sp", + tracks=[{"track_name": "S", "artist_name": "A", "source_track_id": "spotify123"}], + profile_id=1) + db._backfill_mirrored_track_source_ids() + rows = db.get_mirrored_playlist_tracks(pid) + assert rows[0]["source_track_id"] == "spotify123" diff --git a/tests/discovery/test_sync_database_only_matcher.py b/tests/discovery/test_sync_database_only_matcher.py index b28b63d2..d987e788 100644 --- a/tests/discovery/test_sync_database_only_matcher.py +++ b/tests/discovery/test_sync_database_only_matcher.py @@ -24,11 +24,13 @@ def test_matcher_signature_accepts_candidate_pool(): def _run(track, **kw): fake_db = MagicMock() fake_db.read_sync_match_cache.return_value = None + fake_db.find_manual_library_match_by_source_track_id.return_value = None fake_db.check_track_exists.return_value = (None, 0.0) fake_cm = MagicMock() fake_cm.get_active_media_server.return_value = "plex" with patch("database.music_database.MusicDatabase", return_value=fake_db), \ - patch("config.settings.config_manager", fake_cm): + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): return asyncio.run(sync_mod._database_only_find_track(track, **kw)) @@ -42,10 +44,62 @@ def test_returns_match_when_db_has_it(): track = SimpleNamespace(name="HUMBLE.", artists=["Kendrick Lamar"], id="sp2") fake_db = MagicMock() fake_db.read_sync_match_cache.return_value = None + fake_db.find_manual_library_match_by_source_track_id.return_value = None fake_db.check_track_exists.return_value = (SimpleNamespace(id="t1", title="HUMBLE."), 0.95) fake_cm = MagicMock() fake_cm.get_active_media_server.return_value = "plex" with patch("database.music_database.MusicDatabase", return_value=fake_db), \ - patch("config.settings.config_manager", fake_cm): + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): match, conf = asyncio.run(sync_mod._database_only_find_track(track, candidate_pool={})) assert conf == 0.95 and match.id == "t1" + + +# ── durable manual match (#787) survives a rescan that wipes sync_match_cache ── +# (#895 follow-up: Find & Add was forgotten on the next auto-sync after a library +# scan, because the matcher only consulted the volatile cache.) + +def _run_with_db(track, fake_db): + fake_cm = MagicMock() + fake_cm.get_active_media_server.return_value = "plex" + with patch("database.music_database.MusicDatabase", return_value=fake_db), \ + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): + return asyncio.run(sync_mod._database_only_find_track(track, candidate_pool={})) + + +def test_durable_match_used_when_volatile_cache_is_empty(): + track = SimpleNamespace(name="Valió la Pena - Salsa Version", artists=["Marc Anthony"], id="sp16") + dt = SimpleNamespace(id="t99", title="Valió la Pena (Salsa Version)") + db = MagicMock() + db.read_sync_match_cache.return_value = None # cache wiped by a rescan + db.check_track_exists.return_value = (None, 0.0) # fuzzy would FAIL + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "t99", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "t99" else None + match, conf = _run_with_db(track, db) + assert conf == 1.0 and match.id == "t99" # manual pick honored, not re-matched + + +def test_durable_match_self_heals_a_stale_library_id(): + track = SimpleNamespace(name="X", artists=["Y"], id="sp1") + dt = SimpleNamespace(id="newid", title="X") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.check_track_exists.return_value = (None, 0.0) + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "staleid", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "newid" else None # stale id misses + db.find_track_id_by_file_path.return_value = "newid" # re-resolve via path + match, conf = _run_with_db(track, db) + assert conf == 1.0 and match.id == "newid" + db.find_track_id_by_file_path.assert_called_once_with("/m/x.flac") + + +def test_no_durable_match_falls_through_to_fuzzy(): + track = SimpleNamespace(name="X", artists=["Y"], id="sp1") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.find_manual_library_match_by_source_track_id.return_value = None + db.check_track_exists.return_value = (None, 0.0) + assert _run_with_db(track, db) == (None, 0.0) diff --git a/tests/exports/__init__.py b/tests/exports/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/exports/test_export_sources.py b/tests/exports/test_export_sources.py new file mode 100644 index 00000000..bb019ea7 --- /dev/null +++ b/tests/exports/test_export_sources.py @@ -0,0 +1,59 @@ +"""Export source wiring (#903): waterfall order + cache write-back. + +build_resolve_fn assembles cache -> DB -> file -> MusicBrainz and writes a fresh +(non-cache) hit back to the cache. Pins: a cache hit short-circuits everything and is +NOT re-written; a DB/MB hit IS written back; misses fall through; the resolving label is +returned. +""" + +from __future__ import annotations + +from core.exports.export_sources import build_resolve_fn +from core.exports.mbid_resolver import SRC_CACHE, SRC_DB, SRC_MUSICBRAINZ + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" + + +def _wire(db=None, file=None, mb=None, cache=None): + recorded = {} + store = dict(cache or {}) + fn = build_resolve_fn( + db_fn=lambda a, t: (db or {}).get((a, t)), + file_fn=lambda a, t: (file or {}).get((a, t)), + mb_fn=lambda a, t: (mb or {}).get((a, t)), + cache_lookup=lambda k: store.get(k), + cache_record=lambda k, m: recorded.__setitem__(k, m) or True, + ) + return fn, recorded + + +def test_cache_hit_short_circuits_and_is_not_rewritten(): + from core.exports.mbid_resolver import normalize_key + fn, recorded = _wire( + cache={normalize_key("A", "T"): MBID}, + db={("A", "T"): "should-not-reach"}, + ) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_CACHE) + assert recorded == {} # cache hit -> no write-back + + +def test_db_hit_is_written_back_to_cache(): + from core.exports.mbid_resolver import normalize_key + fn, recorded = _wire(db={("A", "T"): MBID}) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_DB) + assert recorded == {normalize_key("A", "T"): MBID} # fresh hit cached for next time + + +def test_falls_through_to_musicbrainz_and_caches(): + fn, recorded = _wire(db={}, file={}, mb={("A", "T"): MBID}) + mbid, label = fn("A", "T") + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + assert list(recorded.values()) == [MBID] + + +def test_all_miss_returns_none_and_no_write(): + fn, recorded = _wire() + assert fn("A", "T") == (None, None) + assert recorded == {} diff --git a/tests/exports/test_jspf_export.py b/tests/exports/test_jspf_export.py new file mode 100644 index 00000000..684265d5 --- /dev/null +++ b/tests/exports/test_jspf_export.py @@ -0,0 +1,72 @@ +"""JSPF builder for ListenBrainz playlist export (#903). + +LB's create-playlist requires every track to carry a recording-MBID identifier; text-only +tracks are rejected. These pin the shape (top-level {"playlist": {...}}, string identifier +in the exact MB recording URL form), that tracks without a valid MBID are dropped, and that +the coverage summary counts included vs skipped. +""" + +from __future__ import annotations + +from core.exports.jspf_export import MB_RECORDING_PREFIX, build_jspf, is_valid_recording_mbid + +MBID_A = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID_B = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def test_valid_mbid_check(): + assert is_valid_recording_mbid(MBID_A) is True + assert is_valid_recording_mbid("not-a-uuid") is False + assert is_valid_recording_mbid("") is False + assert is_valid_recording_mbid(None) is False + + +def test_top_level_shape_and_identifier_format(): + jspf, summary = build_jspf("My Playlist", [ + {"recording_mbid": MBID_A, "title": "Gold", "artist": "Spandau Ballet", "album": "True"}, + ]) + assert set(jspf.keys()) == {"playlist"} + pl = jspf["playlist"] + assert pl["title"] == "My Playlist" + assert len(pl["track"]) == 1 + t = pl["track"][0] + # identifier is a STRING in the exact MB recording form (per the LB/JSPF spec) + assert t["identifier"] == f"{MB_RECORDING_PREFIX}{MBID_A}" + assert isinstance(t["identifier"], str) + assert t["title"] == "Gold" + assert t["creator"] == "Spandau Ballet" # artist -> creator + assert t["album"] == "True" + assert summary == {"total": 1, "included": 1, "skipped": 0} + + +def test_tracks_without_valid_mbid_are_dropped(): + jspf, summary = build_jspf("P", [ + {"recording_mbid": MBID_A, "title": "Keep"}, + {"recording_mbid": "", "title": "No MBID"}, + {"recording_mbid": "garbage", "title": "Bad MBID"}, + {"title": "Missing key entirely"}, + ]) + assert [t["title"] for t in jspf["playlist"]["track"]] == ["Keep"] + assert summary == {"total": 4, "included": 1, "skipped": 3} + + +def test_order_is_preserved(): + jspf, _ = build_jspf("P", [ + {"recording_mbid": MBID_A, "title": "first"}, + {"recording_mbid": MBID_B, "title": "second"}, + ]) + assert [t["title"] for t in jspf["playlist"]["track"]] == ["first", "second"] + + +def test_optional_fields_omitted_when_absent(): + jspf, _ = build_jspf("P", [{"recording_mbid": MBID_A}]) + t = jspf["playlist"]["track"][0] + assert "title" not in t and "creator" not in t and "album" not in t + + +def test_creator_and_title_defaults(): + jspf, summary = build_jspf("", [], creator="SoulSync") + assert jspf["playlist"]["title"] == "SoulSync Export" # blank -> default + assert jspf["playlist"]["creator"] == "SoulSync" + assert jspf["playlist"]["track"] == [] + assert summary == {"total": 0, "included": 0, "skipped": 0} diff --git a/tests/exports/test_listenbrainz_create_playlist.py b/tests/exports/test_listenbrainz_create_playlist.py new file mode 100644 index 00000000..72021e80 --- /dev/null +++ b/tests/exports/test_listenbrainz_create_playlist.py @@ -0,0 +1,160 @@ +"""ListenBrainz create_playlist client method (#903). + +Pins the create -> batched item/add flow against a mocked network: large playlists are +added in <=100 batches (LB's MAX_RECORDINGS_PER_ADD), the new playlist MBID/URL are +returned, and failures are reported (never raised) so the export job can surface them. +""" + +from __future__ import annotations + +from core.listenbrainz_client import ListenBrainzClient + + +class _Resp: + def __init__(self, status, body=None): + self.status_code = status + self._body = body or {} + + def json(self): + return self._body + + +def _client(): + # token='' (not None) skips the network token-validation in __init__; then fake auth. + c = ListenBrainzClient(token="") + c.token = "tok" + c.username = "user" + return c + + +def _tracks(n): + return [{"identifier": f"https://musicbrainz.org/recording/{i:08d}-0000-0000-0000-000000000000"} + for i in range(n)] + + +def test_create_then_batched_add(monkeypatch): + c = _client() + calls = [] + + def fake_req(method, url, **kw): + calls.append(url) + if url.endswith("/playlist/create"): + return _Resp(200, {"status": "ok", "playlist_mbid": "PL-MBID"}) + return _Resp(200, {"status": "ok"}) + + monkeypatch.setattr(c, "_make_request_with_retry", fake_req) + res = c.create_playlist("My Playlist", _tracks(250)) + + assert res["success"] is True + assert res["playlist_mbid"] == "PL-MBID" + assert res["playlist_url"] == "https://listenbrainz.org/playlist/PL-MBID" + assert res["added"] == 250 + # 1 create + 3 add batches (100 + 100 + 50) + assert sum(1 for u in calls if u.endswith("/playlist/create")) == 1 + assert sum(1 for u in calls if "/item/add" in u) == 3 + + +def test_not_authenticated_is_reported_not_raised(): + c = ListenBrainzClient(token="") # no username -> not authenticated + res = c.create_playlist("x", _tracks(1)) + assert res["success"] is False + assert "authenticated" in (res["error"] or "") + + +def test_create_failure_reported(monkeypatch): + c = _client() + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(400, {})) + res = c.create_playlist("x", _tracks(5)) + assert res["success"] is False + assert res["playlist_mbid"] is None + assert "400" in (res["error"] or "") + + +def test_create_ok_but_partial_add_failure(monkeypatch): + c = _client() + + def fake_req(method, url, **kw): + if url.endswith("/playlist/create"): + return _Resp(200, {"playlist_mbid": "PL"}) + return _Resp(500, {}) # every add fails + + monkeypatch.setattr(c, "_make_request_with_retry", fake_req) + res = c.create_playlist("x", _tracks(10)) + # Playlist WAS created -> success True, but added=0 (honest partial report) + assert res["success"] is True + assert res["playlist_mbid"] == "PL" + assert res["added"] == 0 + + +# ── update-in-place + create-or-update (#903 no-duplicate re-export) ────────── + +def _route(existing_count=None): + """Build a fake _make_request_with_retry; records calls. existing_count=None -> GET 404.""" + calls = [] + def fake(method, url, **kw): + calls.append((method, url, kw.get("json"))) + if method == "GET" and "/playlist/" in url: + if existing_count is None: + return _Resp(404, {}) + return _Resp(200, {"playlist": {"track": [{} for _ in range(existing_count)]}}) + if url.endswith("/playlist/create"): + return _Resp(200, {"playlist_mbid": "NEW-MBID"}) + return _Resp(200, {}) # item/add, item/delete, edit, delete + return fake, calls + + +def test_update_playlist_clears_then_re_adds_same_mbid(monkeypatch): + c = _client() + fake, calls = _route(existing_count=3) + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.update_playlist("EXIST-MBID", "New Title", _tracks(5)) + assert res["success"] is True and res["updated"] is True + assert res["playlist_mbid"] == "EXIST-MBID" # stable URL/MBID + assert res["added"] == 5 + urls = [u for _, u, _ in calls] + assert any("/item/delete" in u for u in urls) # cleared existing + assert any("/item/add" in u for u in urls) # re-added + assert any("/playlist/edit/EXIST-MBID" in u for u in urls) # title refreshed + + +def test_update_playlist_reports_gone(monkeypatch): + c = _client() + fake, _ = _route(existing_count=None) # GET 404 -> gone + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.update_playlist("DEAD-MBID", "T", _tracks(2)) + assert res["success"] is False + assert res["gone"] is True + + +def test_create_or_update_updates_when_existing(monkeypatch): + c = _client() + fake, _ = _route(existing_count=2) + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(4), existing_mbid="EXIST") + assert res["updated"] is True and res["playlist_mbid"] == "EXIST" + + +def test_create_or_update_falls_back_to_create_when_gone(monkeypatch): + c = _client() + fake, _ = _route(existing_count=None) # remembered playlist deleted on LB + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(4), existing_mbid="DEAD") + assert res["success"] is True + assert res["updated"] is False # fell back to create + assert res["playlist_mbid"] == "NEW-MBID" + + +def test_create_or_update_creates_when_no_existing(monkeypatch): + c = _client() + fake, _ = _route() + monkeypatch.setattr(c, "_make_request_with_retry", fake) + res = c.create_or_update_playlist("T", _tracks(3), existing_mbid=None) + assert res["playlist_mbid"] == "NEW-MBID" and res["updated"] is False + + +def test_delete_playlist(monkeypatch): + c = _client() + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(200, {})) + assert c.delete_playlist("MBID") is True + monkeypatch.setattr(c, "_make_request_with_retry", lambda *a, **k: _Resp(404, {})) + assert c.delete_playlist("MBID") is False diff --git a/tests/exports/test_mbid_resolver.py b/tests/exports/test_mbid_resolver.py new file mode 100644 index 00000000..9be12a1d --- /dev/null +++ b/tests/exports/test_mbid_resolver.py @@ -0,0 +1,75 @@ +"""MBID resolution waterfall for playlist export (#903). + +Pins: cheapest-source-first short-circuit (don't hit MusicBrainz when the DB has it), +invalid MBIDs are treated as misses, a raising source doesn't abort the export, and the +resolving source label is reported (for the live status breakdown). +""" + +from __future__ import annotations + +from core.exports.mbid_resolver import ( + SRC_CACHE, + SRC_DB, + SRC_MUSICBRAINZ, + normalize_key, + resolve_recording_mbid, +) + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID2 = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def _src(label, value): + return (label, lambda a, t: value) + + +def test_returns_first_hit_with_label(): + mbid, label = resolve_recording_mbid("A", "T", [_src(SRC_DB, MBID), _src(SRC_MUSICBRAINZ, MBID2)]) + assert (mbid, label) == (MBID, SRC_DB) + + +def test_short_circuits_expensive_sources(): + called = {"mb": False} + def mb(a, t): + called["mb"] = True + return MBID2 + mbid, label = resolve_recording_mbid("A", "T", [_src(SRC_CACHE, MBID), (SRC_MUSICBRAINZ, mb)]) + assert (mbid, label) == (MBID, SRC_CACHE) + assert called["mb"] is False # cache hit -> MusicBrainz never queried + + +def test_falls_through_misses_to_later_source(): + mbid, label = resolve_recording_mbid("A", "T", [ + _src(SRC_CACHE, None), + _src(SRC_DB, ""), + _src(SRC_MUSICBRAINZ, MBID2), + ]) + assert (mbid, label) == (MBID2, SRC_MUSICBRAINZ) + + +def test_invalid_mbid_is_a_miss(): + mbid, label = resolve_recording_mbid("A", "T", [ + _src(SRC_DB, "not-a-uuid"), + _src(SRC_MUSICBRAINZ, MBID), + ]) + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + + +def test_raising_source_does_not_abort(): + def boom(a, t): + raise RuntimeError("MusicBrainz timeout") + mbid, label = resolve_recording_mbid("A", "T", [ + (SRC_DB, boom), + _src(SRC_MUSICBRAINZ, MBID), + ]) + assert (mbid, label) == (MBID, SRC_MUSICBRAINZ) + + +def test_all_miss_returns_none(): + assert resolve_recording_mbid("A", "T", [_src(SRC_DB, None), _src(SRC_MUSICBRAINZ, None)]) == (None, None) + assert resolve_recording_mbid("A", "T", []) == (None, None) + + +def test_normalize_key_is_stable_across_variations(): + assert normalize_key("The Beatles", "Hey Jude!") == normalize_key("the beatles", "hey jude") + assert normalize_key("A", "X") != normalize_key("B", "X") diff --git a/tests/exports/test_playlist_export.py b/tests/exports/test_playlist_export.py new file mode 100644 index 00000000..d0af07e5 --- /dev/null +++ b/tests/exports/test_playlist_export.py @@ -0,0 +1,74 @@ +"""Playlist export orchestrator (#903): dedup + stats + progress accounting. + +Pins: repeated songs resolve once (deduped), per-source breakdown counts only fresh +resolutions, unmatched tracks carry recording_mbid=None but stay in order, alternate +field shapes (artist_name/track_name) are accepted, and a throwing progress callback +never fails the export. +""" + +from __future__ import annotations + +from core.exports.playlist_export import resolve_playlist_tracks + +MBID = "e8f9b188-f819-4e43-ab0f-4bd26ce9ff56" +MBID2 = "8f3471b5-7e6a-4c1f-9c1a-2b2b2b2b2b2b" + + +def test_resolves_and_keeps_order_and_unmatched(): + table = {("A", "T1"): (MBID, "db"), ("A", "T2"): (None, None)} + rf = lambda a, t: table.get((a, t), (None, None)) + out = resolve_playlist_tracks( + [{"artist": "A", "title": "T1"}, {"artist": "A", "title": "T2"}], rf + ) + res = out["resolved"] + assert [r["title"] for r in res] == ["T1", "T2"] + assert res[0]["recording_mbid"] == MBID + assert res[1]["recording_mbid"] is None + assert out["stats"]["resolved"] == 1 + assert out["stats"]["unmatched"] == 1 + assert out["stats"]["by_source"] == {"db": 1} + + +def test_dedup_resolves_repeated_song_once(): + calls = {"n": 0} + def rf(a, t): + calls["n"] += 1 + return (MBID, "musicbrainz") + tracks = [{"artist": "A", "title": "Song"}, {"artist": "a", "title": "song"}, # same (normalized) + {"artist": "A", "title": "Song"}] + out = resolve_playlist_tracks(tracks, rf) + assert calls["n"] == 1 # resolve_fn called once for 3 identical + assert out["stats"]["deduped"] == 2 + assert out["stats"]["resolved"] == 3 # all three tracks still get the mbid + assert out["stats"]["by_source"] == {"musicbrainz": 1} # counted once (fresh only) + assert all(r["recording_mbid"] == MBID for r in out["resolved"]) + + +def test_accepts_alternate_field_names(): + rf = lambda a, t: (MBID, "db") if (a, t) == ("Artist", "Track") else (None, None) + out = resolve_playlist_tracks( + [{"artist_name": "Artist", "track_name": "Track", "album_name": "Alb"}], rf + ) + r = out["resolved"][0] + assert r["artist"] == "Artist" and r["title"] == "Track" and r["album"] == "Alb" + assert r["recording_mbid"] == MBID + + +def test_progress_called_per_track_and_safe_when_throwing(): + seen = [] + def prog(done, total, stats): + seen.append((done, total)) + raise RuntimeError("display blew up") + out = resolve_playlist_tracks( + [{"artist": "A", "title": "x"}, {"artist": "B", "title": "y"}], + lambda a, t: (MBID, "db"), + on_progress=prog, + ) + assert seen == [(1, 2), (2, 2)] # called each track despite raising + assert out["stats"]["resolved"] == 2 # export still completed + + +def test_empty_playlist(): + out = resolve_playlist_tracks([], lambda a, t: (None, None)) + assert out["resolved"] == [] + assert out["stats"]["total"] == 0 diff --git a/tests/imports/test_album_position.py b/tests/imports/test_album_position.py new file mode 100644 index 00000000..d22b7ab7 --- /dev/null +++ b/tests/imports/test_album_position.py @@ -0,0 +1,119 @@ +"""A track auto-downloaded from the playlist pipeline / wishlist / watchlist is +identified as belonging to an album, but Deezer's search/track and MusicBrainz's +recording lookups don't carry a track POSITION — so detect_album_info_web left +track_number=None, the pipeline floored it to 1, and album tracks landed as 01/1 +(verified live: Deezer says "Obelisk" is track 9 of The Grand Mirage, tagged 1/1). + +The fix resolves the real position from the album's OWN track list. These pin the +pure matcher and the (fail-safe) integration wrapper. +""" + +from __future__ import annotations + +import pytest + +from core.imports.album_position import resolve_track_position_in_album + + +def _album_12(): + # a realistic 12-track album payload shape (get_album_tracks_for_source -> 'tracks') + return [ + {"id": f"t{i}", "name": n, "track_number": i, "disc_number": 1, + "isrc": f"ISRC{i:03d}"} + for i, n in enumerate( + ["Intro", "Drift", "Mirage", "Haze", "Pulse", "Glow", "Echo", "Tide", + "Obelisk", "Comet", "Dawn", "Outro"], start=1) + ] + + +# ── pure matcher ───────────────────────────────────────────────────────────── + +def test_resolves_position_by_title(): + tn, dn = resolve_track_position_in_album(_album_12(), title="Obelisk") + assert (tn, dn) == (9, 1) + + +def test_title_match_is_case_and_punctuation_insensitive(): + tracks = [{"id": "t1", "name": "Lueur Déclinante!!!", "track_number": 3, "disc_number": 1}] + tn, _ = resolve_track_position_in_album(tracks, title="lueur déclinante") + assert tn == 3 + + +def test_resolves_by_isrc_exactly(): + tn, dn = resolve_track_position_in_album(_album_12(), isrc="isrc009") # case-insensitive + assert (tn, dn) == (9, 1) + + +def test_resolves_by_track_id(): + tn, _ = resolve_track_position_in_album(_album_12(), track_id="t9") + assert tn == 9 + + +def test_isrc_beats_id_beats_title_on_conflict(): + # craft a list where ISRC, id, and title each point at a DIFFERENT track + tracks = [ + {"id": "byid", "name": "other", "track_number": 2, "disc_number": 1, "isrc": "X"}, + {"id": "z", "name": "WANT", "track_number": 3, "disc_number": 1, "isrc": "Y"}, + {"id": "z2", "name": "other2", "track_number": 4, "disc_number": 1, "isrc": "WANTISRC"}, + ] + # all three signals provided -> ISRC wins (track 4) + tn, _ = resolve_track_position_in_album(tracks, title="WANT", track_id="byid", isrc="wantisrc") + assert tn == 4 + # no ISRC -> id wins (track 2) + tn, _ = resolve_track_position_in_album(tracks, title="WANT", track_id="byid") + assert tn == 2 + # only title -> title wins (track 3) + tn, _ = resolve_track_position_in_album(tracks, title="want") + assert tn == 3 + + +def test_carries_disc_number(): + tracks = [{"id": "t1", "name": "B-Side", "track_number": 2, "disc_number": 2}] + assert resolve_track_position_in_album(tracks, title="B-Side") == (2, 2) + + +def test_no_match_returns_none(): + assert resolve_track_position_in_album(_album_12(), title="Not On This Album") == (None, None) + assert resolve_track_position_in_album([], title="x") == (None, None) + assert resolve_track_position_in_album(None, title="x") == (None, None) + + +def test_skips_entries_without_a_valid_position(): + tracks = [ + {"id": "t1", "name": "Obelisk", "track_number": 0}, # 0 -> skip + {"id": "t2", "name": "Obelisk", "track_number": None}, # None -> skip + {"id": "t3", "name": "Obelisk", "track_number": "junk"}, # junk -> skip + ] + assert resolve_track_position_in_album(tracks, title="Obelisk") == (None, None) + + +# ── integration wrapper (fail-safe, real album lookup) ─────────────────────── + +def test_wrapper_resolves_from_source_album(monkeypatch): + import core.imports.context as ctx + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", + lambda source, album_id: {"tracks": _album_12()}) + context = {"source": "deezer", + "track_info": {"id": "t9", "name": "Obelisk", "deezer_album_id": "232620572"}} + tn, dn = ctx._resolve_album_position_from_source(context, {}, 1) + assert tn == 9 and dn == 1 + + +def test_wrapper_is_failsafe_on_empty_or_missing(monkeypatch): + import core.imports.context as ctx + # no album id at all -> keeps current disc, no number + assert ctx._resolve_album_position_from_source({"source": "deezer", "track_info": {}}, {}, 1) == (None, 1) + # fetcher returns nothing -> fail-safe + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", + lambda source, album_id: {"tracks": []}) + context = {"source": "deezer", "track_info": {"name": "Obelisk", "deezer_album_id": "x"}} + assert ctx._resolve_album_position_from_source(context, {}, 2) == (None, 2) + + +def test_wrapper_never_raises_on_fetch_error(monkeypatch): + import core.imports.context as ctx + def _boom(source, album_id): + raise RuntimeError("deezer down") + monkeypatch.setattr("core.metadata.album_tracks.get_album_tracks_for_source", _boom) + context = {"source": "deezer", "track_info": {"name": "Obelisk", "deezer_album_id": "x"}} + assert ctx._resolve_album_position_from_source(context, {}, 1) == (None, 1) diff --git a/tests/imports/test_normalize_disc_number.py b/tests/imports/test_normalize_disc_number.py new file mode 100644 index 00000000..c7f392e5 --- /dev/null +++ b/tests/imports/test_normalize_disc_number.py @@ -0,0 +1,69 @@ +"""Sokhi: some tracks in a multi-disc album got a null disc in Jellyfin and floated +ungrouped above the disc sections. Root cause: the tag-writer only wrote the disc +tag when disc_number was truthy, and upstream a 0 / None / '' (esp. when a track +matched a different edition than its siblings) slipped through — so on the +clear-then-rewrite those tracks lost their disc entirely. normalize_disc_number +floors any value to >=1 so a track is never written disc-less.""" + +from __future__ import annotations + +import pytest + +from core.imports.track_number import normalize_disc_number + + +@pytest.mark.parametrize("value,expected", [ + (1, 1), (2, 2), (4, 4), + ("1", 1), ("3", 3), (" 2 ", 2), + (0, 1), ("0", 1), # the bug: 0 must floor to 1, not vanish + (None, 1), ("", 1), (" ", 1), + (-1, 1), ("-2", 1), # negatives floor to 1 + ("abc", 1), ("1/4", 1), # non-numeric -> 1 (never raises) + (2.0, 2), # float-ish via str() +]) +def test_normalize_disc_number(value, expected): + assert normalize_disc_number(value) == expected + + +def test_valid_multidisc_values_preserved(): + # a real disc on a 4xLP must survive untouched + for d in (1, 2, 3, 4): + assert normalize_disc_number(d) == d + + +# ── resolve_disc_for_track: the FOLDER and the TAG must use the same disc ────── + +from core.imports.track_number import resolve_disc_for_track + + +def test_resolve_disc_prefers_per_track_search_then_album(): + # per-track disc wins (this is the value the tag uses) — so the folder, which + # now calls the SAME resolver with the SAME inputs, lands on the same disc. + assert resolve_disc_for_track({"disc_number": 3}, {"disc_number": 1}) == 3 + # falls back to album context when the per-track search has none + assert resolve_disc_for_track({}, {"disc_number": 2}) == 2 + assert resolve_disc_for_track({"disc_number": None}, {"disc_number": 2}) == 2 + # both missing -> floored default 1 + assert resolve_disc_for_track({}, {}) == 1 + assert resolve_disc_for_track(None, None) == 1 + + +def test_resolve_disc_floors_bad_values(): + assert resolve_disc_for_track({"disc_number": 0}, {"disc_number": 5}) == 5 # 0 is falsy -> fall to album + assert resolve_disc_for_track({"disc_number": "2"}, {}) == 2 + assert resolve_disc_for_track({"disc_number": "junk"}, {}) == 1 + + +def test_folder_and_tag_resolve_identically(): + # the regression that matters: given the same (original_search, album_info), + # source.py (tag) and the pipeline (folder) get the IDENTICAL disc. + cases = [ + ({"disc_number": 2}, {"disc_number": 1}), # Sokhi's case: per-track 2, album 1 + ({"disc_number": 3}, {"disc_number": 1}), + ({}, {"disc_number": 1}), + ({"disc_number": 0}, {"disc_number": 1}), + ] + for osrch, ainfo in cases: + folder_disc = resolve_disc_for_track(osrch, ainfo) # what the pipeline writes to album_info + tag_disc = resolve_disc_for_track(osrch, ainfo) # what source.py writes to the tag + assert folder_disc == tag_disc diff --git a/tests/library/test_standalone_scan.py b/tests/library/test_standalone_scan.py new file mode 100644 index 00000000..a0781c9a --- /dev/null +++ b/tests/library/test_standalone_scan.py @@ -0,0 +1,115 @@ +"""Standalone Deep Scan planner — the #904 data-loss guard. + +The scan relocates Transfer files the DB doesn't know about into Staging. With a +path-only diff, an empty/desynced DB makes the WHOLE library look untracked and the +scan moved all of it (reporter lost ~1,500 tracks into Staging). These pin the guard: +a normal batch of new arrivals still moves; an implausibly large untracked share (the +desync signature) or a 'permanent library' opt-out blocks the move instead. +""" + +from __future__ import annotations + +from core.library.standalone_scan import ( + BLOCK_DESYNC, + BLOCK_NONE, + BLOCK_TRANSFER_PERMANENT, + diff_untracked, + plan_standalone_deep_scan, +) + + +def _files(prefix, n, start=0): + return {f"{prefix}/track{i}.flac" for i in range(start, start + n)} + + +# ── diff_untracked (pure path diff) ────────────────────────────────────────── + +def test_diff_basic(): + transfer = {"/m/a.flac", "/m/b.flac", "/m/c.flac"} + db = {"/m/a.flac", "/m/b.flac"} + assert diff_untracked(transfer, db) == {"/m/c.flac"} + + +def test_diff_is_separator_normalized(): + # DB stored a Windows-style path; the on-disk path uses forward slashes → still a match + transfer = {"/m/Artist/x.flac"} + db = {"\\m\\Artist\\x.flac"} + assert diff_untracked(transfer, db) == set() + + +def test_diff_all_untracked_when_db_empty(): + transfer = _files("/lib", 5) + assert diff_untracked(transfer, set()) == transfer + + +# ── plan: normal (move allowed) ────────────────────────────────────────────── + +def test_clean_library_not_blocked(): + transfer = _files("/lib", 1000) + plan = plan_standalone_deep_scan(transfer, transfer) # all known + assert plan["untracked"] == set() + assert plan["move_blocked"] is False + assert plan["block_reason"] == BLOCK_NONE + + +def test_normal_new_arrivals_move(): + # DB knows 990; 10 new files dropped in → small share, moves as before + known = _files("/lib", 990) + transfer = known | _files("/lib", 10, start=990) + plan = plan_standalone_deep_scan(transfer, known) + assert len(plan["untracked"]) == 10 + assert plan["move_blocked"] is False + + +def test_small_fresh_import_under_floor_moves(): + # A tiny brand-new folder (under the absolute floor) isn't second-guessed + transfer = _files("/lib", 5) + plan = plan_standalone_deep_scan(transfer, set()) + assert len(plan["untracked"]) == 5 + assert plan["move_blocked"] is False + + +# ── plan: the #904 guard (move blocked) ────────────────────────────────────── + +def test_regression_904_empty_db_full_library_blocks(): + # Empty DB + a real 1,500-track library → 100% untracked → BLOCKED, nothing moved + transfer = _files("/library", 1500) + plan = plan_standalone_deep_scan(transfer, set()) + assert len(plan["untracked"]) == 1500 + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_DESYNC + + +def test_majority_untracked_blocks(): + # 600 of 1000 unknown (60%, over the 50% line) → desync, blocked + known = _files("/lib", 400) + transfer = known | _files("/lib", 600, start=400) + plan = plan_standalone_deep_scan(transfer, known) + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_DESYNC + + +def test_minority_untracked_just_under_threshold_moves(): + # 400 of 1000 unknown (40%, under 50%) → still treated as a batch, not a desync + known = _files("/lib", 600) + transfer = known | _files("/lib", 400, start=600) + plan = plan_standalone_deep_scan(transfer, known) + assert plan["move_blocked"] is False + + +def test_never_move_blocks_even_small_sets(): + # Permanent-library opt-out: block regardless of fraction + known = _files("/lib", 990) + transfer = known | _files("/lib", 10, start=990) + plan = plan_standalone_deep_scan(transfer, known, never_move=True) + assert len(plan["untracked"]) == 10 + assert plan["move_blocked"] is True + assert plan["block_reason"] == BLOCK_TRANSFER_PERMANENT + + +def test_never_move_with_nothing_untracked_is_not_blocked(): + # Nothing to move → not a "blocked" outcome even with the toggle on + transfer = _files("/lib", 100) + plan = plan_standalone_deep_scan(transfer, transfer, never_move=True) + assert plan["untracked"] == set() + assert plan["move_blocked"] is False diff --git a/tests/playlists/test_playlist_item_naming.py b/tests/playlists/test_playlist_item_naming.py new file mode 100644 index 00000000..fd9b38ab --- /dev/null +++ b/tests/playlists/test_playlist_item_naming.py @@ -0,0 +1,121 @@ +"""Custom file naming for organize-by-playlist folders. + +The materialized playlist folder used to be stuck with the library filename. +A user can now opt into a flat filename template (e.g. "$position - $artist - +$title"). It's a FILENAME, not a path — validated so it can't make folders or +broken names, and it falls back to the library filename on anything invalid. +""" + +from __future__ import annotations + +import os + +import pytest + +from core.playlists.item_naming import ( + render_playlist_item_name, + validate_playlist_item_template, +) +from core.playlists.materialize import rebuild_playlist_folder + + +# ── validation ────────────────────────────────────────────────────────────── + +def test_empty_template_is_valid_means_off(): + assert validate_playlist_item_template("") == (True, "") + assert validate_playlist_item_template(" ") == (True, "") + assert validate_playlist_item_template(None) == (True, "") + + +def test_slash_is_rejected_no_folder_structure(): + ok, why = validate_playlist_item_template("$artist/$title") + assert ok is False and "separator" in why + ok, why = validate_playlist_item_template("$artist\\$title") + assert ok is False + + +def test_must_contain_title(): + ok, why = validate_playlist_item_template("$position - $artist") + assert ok is False and "$title" in why + + +def test_valid_flat_template_passes(): + assert validate_playlist_item_template("$position - $artist - $title") == (True, "") + + +# ── rendering ─────────────────────────────────────────────────────────────── + +def test_renders_tokens_and_keeps_extension(): + out = render_playlist_item_name( + "$position - $artist - $title", + title="One More Time", artist="Daft Punk", position="01", ext=".flac", + fallback_name="x.flac") + assert out == "01 - Daft Punk - One More Time.flac" + + +def test_track_is_zero_padded_album_is_optional(): + out = render_playlist_item_name( + "$track - $title", title="Genesis", track=5, ext=".mp3", fallback_name="x.mp3") + assert out == "05 - Genesis.mp3" + + +def test_invalid_template_falls_back_to_library_name(): + # slash / missing-title / empty all fall back — never a broken name + for bad in ("$artist/$title", "$artist - $position", ""): + assert render_playlist_item_name( + bad, title="T", artist="A", ext=".flac", fallback_name="orig.flac") == "orig.flac" + + +def test_garbage_title_still_yields_a_safe_name_not_broken(): + # a title made of separators is sanitized to a safe (ugly) name with the + # extension intact — never a broken name and never a path. + out = render_playlist_item_name( + "$title", title="/////", ext=".flac", fallback_name="orig.flac") + assert out.endswith(".flac") and "/" not in out and "\\" not in out + + +def test_rendered_name_can_never_contain_a_separator(): + out = render_playlist_item_name( + "$artist - $title", title="AC/DC Song", artist="AC/DC", ext=".flac", fallback_name="x.flac") + assert "/" not in out and "\\" not in out + + +# ── end-to-end through the real folder builder ────────────────────────────── + +def _touch(p): + os.makedirs(os.path.dirname(p), exist_ok=True) + with open(p, "wb") as f: + f.write(b"\x00") + + +def test_rebuild_uses_dest_names_when_given(tmp_path): + lib = tmp_path / "lib" + a = str(lib / "Artist A" / "05 - Song A.flac") + b = str(lib / "Artist B" / "02 - Song B.flac") + _touch(a); _touch(b) + root = str(tmp_path / "Playlists") + summary = rebuild_playlist_folder( + root, "My Mix", [a, b], "copy", + dest_names=["01 - Artist A - Song A.flac", "02 - Artist B - Song B.flac"]) + got = sorted(os.listdir(summary.playlist_dir)) + assert got == ["01 - Artist A - Song A.flac", "02 - Artist B - Song B.flac"] + + +def test_rebuild_without_dest_names_keeps_basename(tmp_path): + # back-compat: default behavior unchanged + a = str(tmp_path / "lib" / "05 - Song A.flac") + _touch(a) + summary = rebuild_playlist_folder(str(tmp_path / "PL"), "Mix", [a], "copy") + assert os.listdir(summary.playlist_dir) == ["05 - Song A.flac"] + + +def test_rebuild_disambiguates_colliding_dest_names(tmp_path): + # two different sources, same templated name (e.g. template "$title" + dup title) + a = str(tmp_path / "lib" / "a" / "x.flac") + b = str(tmp_path / "lib" / "b" / "y.flac") + _touch(a); _touch(b) + summary = rebuild_playlist_folder( + str(tmp_path / "PL"), "Mix", [a, b], "copy", + dest_names=["Song.flac", "Song.flac"]) + got = sorted(os.listdir(summary.playlist_dir)) + assert got == ["Song (2).flac", "Song.flac"] diff --git a/tests/playlists/test_stable_source_track_id.py b/tests/playlists/test_stable_source_track_id.py new file mode 100644 index 00000000..10c9af72 --- /dev/null +++ b/tests/playlists/test_stable_source_track_id.py @@ -0,0 +1,45 @@ +"""#901: a manual match (Find & Add) on a file-import playlist track was silently +dropped and the track re-appeared as "extra". Root cause: file-import / iTunes-only +tracks arrive with an EMPTY source_track_id, and the whole manual-match system keys +on it — an empty key can't be persisted (no-op) or looked up. Fix: derive a stable +deterministic id from the track's identity so matches stick like they do for +Spotify/YouTube (which carry native ids). +""" + +from __future__ import annotations + +from core.playlists.source_refs import stable_source_track_id + + +def test_native_id_is_used_verbatim(): + assert stable_source_track_id({"source_track_id": "2fdfsGuqb6SBX5ocoBWHUd"}) == "2fdfsGuqb6SBX5ocoBWHUd" + # explicit existing wins over the dict + assert stable_source_track_id({"source_track_id": "x"}, existing="y") == "y" + + +def test_file_track_gets_a_deterministic_prefixed_id(): + t = {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"} + a = stable_source_track_id(t) + assert a.startswith("file:") and len(a) == len("file:") + 16 + # SAME song → SAME id across calls/re-imports (what the match lookup needs) + assert stable_source_track_id(dict(t)) == a + + +def test_identity_is_case_and_field_insensitive_but_distinguishes_songs(): + base = {"track_name": "Slow Ride", "artist_name": "Foghat", "album_name": "Fool for the City"} + same = {"name": "slow ride", "artist": "FOGHAT", "album": "fool for the city"} # alt field names + case + assert stable_source_track_id(base) == stable_source_track_id(same) + # a different song gets a different id + assert stable_source_track_id(base) != stable_source_track_id( + {"track_name": "I Just Want to Make Love to You", "artist_name": "Foghat"}) + + +def test_empty_id_when_no_title(): + assert stable_source_track_id({"artist_name": "Foghat"}) == "" + assert stable_source_track_id({}) == "" + + +def test_never_collides_with_a_real_upstream_id(): + # the file: prefix keeps synthetic ids out of the spotify/youtube id space + fid = stable_source_track_id({"track_name": "x", "artist_name": "y"}) + assert ":" in fid and not fid.replace("file:", "").startswith("file") diff --git a/tests/sync/test_reresolve_live_plex.py b/tests/sync/test_reresolve_live_plex.py new file mode 100644 index 00000000..5de3bf49 --- /dev/null +++ b/tests/sync/test_reresolve_live_plex.py @@ -0,0 +1,108 @@ +"""Plex re-keys tracks on a metadata refresh, so a durable manual match's stored +ratingKey (and the SoulSync DB id, which IS that ratingKey) can both go stale at +once — every DB-side lookup lands on the same dead key and fetchItem 404s, so the +manually-matched track gets dropped on sync (wolf39us). The fix re-resolves the +match against LIVE Plex by the matched track's metadata, disambiguated by the +stored file path so the user's EXACT chosen track wins, and heals the stored id. +""" + +from __future__ import annotations + +from services.sync_service import reresolve_manual_match_live_plex + + +class _PlexTrack: + def __init__(self, rating_key, file): + self.ratingKey = rating_key + self.title = f"track-{rating_key}" + + class _Part: + def __init__(self, f): self.file = f + class _Media: + def __init__(self, f): self.parts = [_Part(f)] + self.media = [_Media(file)] + + +class _TrackInfo: + def __init__(self, plex_track): + self._original_plex_track = plex_track + + +class _MediaClient: + def __init__(self, results): + self._results = results + self.calls = [] + def search_tracks(self, title, artist, limit=15): + self.calls.append((title, artist)) + return self._results + + +class _CacheDb: + def __init__(self): + self.healed = [] + def save_manual_library_match(self, profile_id, source, source_track_id, library_track_id, **meta): + self.healed.append({"id": library_track_id, "source_track_id": source_track_id, **meta}) + return True + + +_MATCH = { + "source": "spotify", "source_title": "It's the End of the World", + "source_artist": "R.E.M.", "source_album": "Document", + "library_file_path": "/music/REM/Document/05 - Its the End.flac", + "library_track_id": 39161, # stale +} + + +def test_picks_the_track_matching_the_stored_file_path(): + # Two live candidates (a different version + the real one); the stored file + # path must select the user's exact track, with its CURRENT ratingKey. + wrong = _TrackInfo(_PlexTrack(50001, "/music/REM/Live/05 - Its the End (Live).flac")) + right = _TrackInfo(_PlexTrack(39167, "/music/REM/Document/05 - Its the End.flac")) + mc = _MediaClient([wrong, right]) + db = _CacheDb() + live = reresolve_manual_match_live_plex( + db, mc, _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") + assert live is not None and live.ratingKey == 39167 # current key, not stale 39161 + assert mc.calls == [("It's the End of the World", "R.E.M.")] + # healed the stored id to the fresh ratingKey + assert db.healed and db.healed[0]["id"] == "39167" + assert db.healed[0]["source_track_id"] == "sp1" + + +def test_basename_match_handles_server_vs_local_path(): + # stored path is a local path; the Plex part.file is a container path — same basename. + m = dict(_MATCH, library_file_path="D:\\Music\\REM\\05 - Its the End.flac") + right = _TrackInfo(_PlexTrack(39167, "/data/Music/REM/05 - Its the End.flac")) + live = reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([right]), m, profile_id=1, source_track_id="sp1", server_source="plex") + assert live.ratingKey == 39167 + + +def test_no_file_match_falls_back_to_top_result(): + only = _TrackInfo(_PlexTrack(40000, "/music/somewhere/else.flac")) + live = reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([only]), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") + assert live.ratingKey == 40000 # never drop a manual match — best-effort top hit + + +def test_no_results_returns_none_and_does_not_heal(): + db = _CacheDb() + assert reresolve_manual_match_live_plex( + db, _MediaClient([]), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") is None + assert db.healed == [] + + +def test_missing_title_returns_none(): + m = dict(_MATCH, source_title="") + assert reresolve_manual_match_live_plex( + _CacheDb(), _MediaClient([_TrackInfo(_PlexTrack(1, "/x.flac"))]), m, + profile_id=1, source_track_id="sp1", server_source="plex") is None + + +def test_never_raises_on_a_broken_media_client(): + class _Boom: + def search_tracks(self, *a, **k): + raise RuntimeError("plex down") + # a transient Plex error must not bubble — the caller falls through, not crash. + assert reresolve_manual_match_live_plex( + _CacheDb(), _Boom(), _MATCH, profile_id=1, source_track_id="sp1", server_source="plex") is None diff --git a/tests/sync/test_sync_durable_match.py b/tests/sync/test_sync_durable_match.py new file mode 100644 index 00000000..0c0abb4e --- /dev/null +++ b/tests/sync/test_sync_durable_match.py @@ -0,0 +1,57 @@ +"""The REAL playlist sync matcher (PlaylistSyncService._find_track_in_media_server) +must honor a durable Find & Add / manual match when the volatile sync_match_cache +has been wiped by a library rescan — otherwise the manual pick is re-matched from +scratch on the next auto-sync (#895 follow-up). Jellyfin server-type avoids Plex +fetchItem mocking; the durable block is server-agnostic.""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from services.sync_service import PlaylistSyncService + + +def _service(): + svc = PlaylistSyncService(spotify_client=MagicMock(), download_orchestrator=MagicMock(), + media_server_engine=MagicMock()) + client = MagicMock(); client.is_connected.return_value = True + svc._get_active_media_client = lambda: (client, "jellyfin") + svc._cancelled = False + return svc + + +def _run(svc, db): + cm = MagicMock(); cm.get_active_media_server.return_value = "jellyfin" + track = SimpleNamespace(name="Valió la Pena - Salsa Version", artists=["Marc Anthony"], id="sp16") + with patch("database.music_database.MusicDatabase", return_value=db), \ + patch("config.settings.config_manager", cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): + return asyncio.run(svc._find_track_in_media_server(track, candidate_pool={})) + + +def test_durable_match_used_when_volatile_cache_wiped(): + dt = SimpleNamespace(id="t99", title="Valió la Pena (Salsa Version)") + db = MagicMock() + db.read_sync_match_cache.return_value = None # rescan wiped the cache + db.check_track_exists.return_value = (None, 0.0) # fuzzy would FAIL + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "t99", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "t99" else None + match, conf = _run(svc=_service(), db=db) + assert conf == 1.0 and match.id == "t99" # manual pick honored across the rescan + + +def test_durable_match_self_heals_stale_library_id(): + dt = SimpleNamespace(id="newid", title="X") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.check_track_exists.return_value = (None, 0.0) + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "staleid", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "newid" else None # stale id gone + db.find_track_id_by_file_path.return_value = "newid" + match, conf = _run(svc=_service(), db=db) + assert conf == 1.0 and match.id == "newid" + db.find_track_id_by_file_path.assert_called_once_with("/m/x.flac") diff --git a/tests/test_artist_the_prefix_variations.py b/tests/test_artist_the_prefix_variations.py new file mode 100644 index 00000000..29670912 --- /dev/null +++ b/tests/test_artist_the_prefix_variations.py @@ -0,0 +1,90 @@ +"""Leading-"The" duplicate fix. + +A user wanted "I Gotta Feeling" by "The Black Eyed Peas" but owned it under +"Black Eyed Peas" (or vice-versa). The dedup gate (check_track_exists) fetches +candidates via _get_artist_variations(), which had no "The" toggle — so the +owned track was never fetched, the request "failed to match", and a duplicate +was downloaded. The toggle widens the fetch to both forms; the scorer still +decides, so it can't merge genuinely different artists. +""" + +from __future__ import annotations + +from database.music_database import MusicDatabase + + +def _variations(name): + db = object.__new__(MusicDatabase) # no DB / network init needed + return db._get_artist_variations(name) + + +def test_leading_the_is_stripped_to_search_the_bare_form(): + v = _variations("The Black Eyed Peas") + assert "Black Eyed Peas" in v # owned-bare form now gets fetched + assert "The Black Eyed Peas" in v # original kept + + +def test_bare_name_also_searches_the_the_prefixed_form(): + v = _variations("Black Eyed Peas") + assert "The The Black Eyed Peas" not in v # no double-"The" + assert "The Black Eyed Peas" in v # the "The"-variant gets fetched + assert "Black Eyed Peas" in v + + +def test_the_band_named_just_the_never_produces_an_empty_search(): + # "The" alone must not collapse to an empty artist search (which would match + # the entire library). Adding "The The" is harmless — the scorer still gates. + v = _variations("The") + assert "" not in v + assert "The" in v + + +def test_a_leading_the_word_is_required_not_a_mid_word_the(): + # "Theory of a Deadman" starts with "The" but not the WORD "The" — it must not + # be stripped mid-word. (It does still get the harmless "The "-prefixed widen.) + v = _variations("Theory of a Deadman") + assert "ory of a Deadman" not in v # NOT mangled mid-word + assert "Theory of a Deadman" in v + + +def test_the_toggle_lands_the_match_through_the_real_scorer(): + # End-to-end on the confidence scorer: requesting one variant against the + # other owned variant must clear the 0.8 dedup threshold (50/50 title/artist + # → 1.0*0.5 + 0.882*0.5 = 0.94), so it's recognized as already owned. + db = object.__new__(MusicDatabase) + + class _Track: + title = "I Gotta Feeling" + artist_name = "Black Eyed Peas" + track_artist = "Black Eyed Peas" + album = "The E.N.D." + + conf = db._calculate_track_confidence("I Gotta Feeling", "The Black Eyed Peas", _Track()) + assert conf >= 0.8, conf + + # …and the reverse direction too. + class _Track2: + title = "I Gotta Feeling" + artist_name = "The Black Eyed Peas" + track_artist = "The Black Eyed Peas" + album = "The E.N.D." + + conf2 = db._calculate_track_confidence("I Gotta Feeling", "Black Eyed Peas", _Track2()) + assert conf2 >= 0.8, conf2 + + +def test_toggle_does_not_falsely_merge_different_the_artists(): + # "The Police" and "Police" are arguably the same band, but "The Weeknd" vs + # "Weeknd" etc. — the toggle only WIDENS the fetch; the scorer still gates. + # A clearly different artist must not score as a match just because both + # share no "The". (Title differs too — this is the real safety net.) + db = object.__new__(MusicDatabase) + + class _Other: + title = "Some Other Song" + artist_name = "Black Eyed Peas" + track_artist = "Black Eyed Peas" + album = "Whatever" + + conf = db._calculate_track_confidence("I Gotta Feeling", "The Killers", _Other()) + assert conf < 0.8, conf diff --git a/tests/test_caa_release_art.py b/tests/test_caa_release_art.py new file mode 100644 index 00000000..29351481 --- /dev/null +++ b/tests/test_caa_release_art.py @@ -0,0 +1,85 @@ +"""Special-edition cover art: prefer the pinned release's OWN cover over the +release-group representative. + +A MusicBrainz release-group 'front' on the Cover Art Archive is a single +representative cover (usually the standard edition), so a special edition (e.g. +"Gustave Edition") was getting the standard art. The download/embed art paths now +try the specific release's own cover first and fall back to the group/provider +URL only when the release has none — so coverage never regresses. +""" + +from __future__ import annotations + +from core.metadata.caa_art import caa_front_url, fetch_release_preferred_art + + +def test_caa_front_url_scopes_and_size(): + assert caa_front_url("abc", "release") == "https://coverartarchive.org/release/abc/front-1200" + assert caa_front_url("rg", "release-group") == "https://coverartarchive.org/release-group/rg/front-1200" + assert caa_front_url("abc", "release", size=250).endswith("/front-250") + assert caa_front_url("abc", "release", size=0).endswith("/abc/front") + assert caa_front_url("", "release") is None + assert caa_front_url(None) is None + # unknown scope coerces to release + assert "/release/x/" in caa_front_url("x", "bogus") + + +def _fetcher(table): + """table: {url: bytes|None}. Returns (bytes, mime) or (None, None).""" + calls = [] + def fetch(url): + calls.append(url) + data = table.get(url) + return (data, "image/jpeg") if data else (None, None) + fetch.calls = calls + return fetch + + +def test_prefers_release_specific_art_over_fallback(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: b"X" * 5000, fb: b"Y" * 5000}) + data, mime, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"X" * 5000 and used == rel + assert fetch.calls[0] == rel # release tried FIRST + + +def test_falls_back_when_release_has_no_own_art(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: None, fb: b"Y" * 5000}) # release 404s + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"Y" * 5000 and used == fb # never regresses: keeps the old cover + assert fetch.calls == [rel, fb] # tried release, then fell back + + +def test_no_release_mbid_uses_fallback_directly(): + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({fb: b"Y" * 5000}) + data, _, used = fetch_release_preferred_art(None, fb, fetch_fn=fetch) + assert data and used == fb + assert fetch.calls == [fb] # no wasted release lookup + + +def test_tiny_image_is_treated_as_a_miss(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: b"tiny", fb: b"Y" * 5000}) # release art under min_bytes + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch, min_bytes=1000) + assert used == fb + + +def test_nothing_available_returns_none(): + fetch = _fetcher({}) + assert fetch_release_preferred_art("REL", None, fetch_fn=fetch) == (None, None, None) + assert fetch_release_preferred_art(None, None, fetch_fn=fetch) == (None, None, None) + + +def test_fetch_exception_is_treated_as_miss_not_fatal(): + fb = "https://provider.example/standard.jpg" + def fetch(url): + if "release" in url: + raise RuntimeError("network boom") + return b"Y" * 5000, "image/jpeg" + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"Y" * 5000 and used == fb # exception on release → fell back safely diff --git a/tests/test_deezer_track_positions.py b/tests/test_deezer_track_positions.py new file mode 100644 index 00000000..b0ed79b7 --- /dev/null +++ b/tests/test_deezer_track_positions.py @@ -0,0 +1,60 @@ +"""Deezer playlist tracks must carry the REAL album track_position, not their +playlist index — otherwise the downloaded file is tagged with the wrong track +number (e.g. 'Apologize' from Shock Value tagged track 1 instead of 16).""" + +from __future__ import annotations + +from core.deezer_client import resolve_album_track_positions + + +class _Resp: + def __init__(self, data, ok=True): + self._d, self.ok = data, ok + + def json(self): + return self._d + + +class _Session: + """Fake requests session returning /album//tracks payloads.""" + + def __init__(self, by_album, fail_for=()): + self.by_album, self.fail_for, self.calls = by_album, set(fail_for), [] + + def get(self, url, params=None, timeout=None): + aid = url.rstrip('/').split('/')[-2] # …/album//tracks + self.calls.append(aid) + if aid in self.fail_for: + return _Resp(None, ok=False) + return _Resp({'data': self.by_album.get(aid, [])}) + + +def test_maps_track_id_to_real_album_position(): + sess = _Session({'119606': [ + {'id': 100, 'track_position': 16}, {'id': 101, 'track_position': 2}]}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'119606'}, sleep_s=0) + assert pos == {'100': 16, '101': 2} # real positions, not 1/2 enumerate + + +def test_cache_first_skips_the_network(): + class _Cache: + def __init__(self): self.stored = {} + def get_entity(self, src, kind, aid): + return {'data': [{'id': 7, 'track_position': 9}]} if kind == 'album_tracks' else None + def store_entity(self, *a, **k): pass + sess = _Session({}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'42'}, cache=_Cache(), sleep_s=0) + assert pos == {'7': 9} and sess.calls == [] # served from cache, no HTTP + + +def test_failed_album_is_simply_absent_not_fatal(): + sess = _Session({'1': [{'id': 5, 'track_position': 3}]}, fail_for={'2'}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'1', '2'}, sleep_s=0) + assert pos == {'5': 3} # album 2 failed → just missing + + +def test_zero_position_is_ignored(): + # Deezer sometimes returns 0/None for odd entries — don't poison the map with them + sess = _Session({'1': [{'id': 5, 'track_position': 0}, {'id': 6}, {'id': 7, 'track_position': 4}]}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'1'}, sleep_s=0) + assert pos == {'7': 4} diff --git a/tests/test_hifi_preview_guard.py b/tests/test_hifi_preview_guard.py new file mode 100644 index 00000000..ed085933 --- /dev/null +++ b/tests/test_hifi_preview_guard.py @@ -0,0 +1,218 @@ +"""HiFi sometimes serves a PREVIEW manifest (~30s of segments) for a full-length +track, which slipped through the old 100KB-only size floor. The duration guards +catch it: a preview manifest is way shorter than the real track length.""" + +from __future__ import annotations + +from core.hifi_client import sum_hls_segment_seconds, is_short_audio + + +_FULL = """#EXTM3U +#EXT-X-VERSION:6 +#EXT-X-MAP:URI="init.mp4" +#EXTINF:10.0, +seg0.mp4 +#EXTINF:10.0, +seg1.mp4 +#EXTINF:9.5, +seg2.mp4 +#EXT-X-ENDLIST +""" + +_PREVIEW = """#EXTM3U +#EXTINF:15.0, +p0.mp4 +#EXTINF:15.0, +p1.mp4 +#EXT-X-ENDLIST +""" + + +def test_sums_extinf_segment_durations(): + assert sum_hls_segment_seconds(_FULL) == 29.5 # 10 + 10 + 9.5 + assert sum_hls_segment_seconds(_PREVIEW) == 30.0 # the preview's true length + + +def test_no_extinf_is_unknown_zero(): + assert sum_hls_segment_seconds("#EXTM3U\nseg.mp4\n") == 0.0 + assert sum_hls_segment_seconds("") == 0.0 + + +def test_preview_is_flagged_short_against_full_track(): + # Save Your Tears ~215s; a 30s preview manifest is obviously short + assert is_short_audio(30.0, 215.0) is True + + +def test_full_length_download_is_not_flagged(): + assert is_short_audio(213.0, 215.0) is False # ~1% trim → fine + assert is_short_audio(215.0, 215.0) is False + + +def test_unknown_durations_never_reject(): + assert is_short_audio(0, 215) is False # couldn't probe → don't reject + assert is_short_audio(30, 0) is False # expected unknown → don't reject + assert is_short_audio(0, 0) is False + + +def test_legitimately_short_track_is_kept(): + # a real 40s interlude: actual ≈ expected → not a preview + assert is_short_audio(40.0, 41.0) is False + + +def test_threshold_boundary(): + assert is_short_audio(79, 100) is True # below 80% + assert is_short_audio(85, 100) is False # above 80% + + +# ── integration: the guards actually wire into _download_sync ──────────────── +import pytest +import core.hifi_client as hc + + +class _Cfg: + """Stub config so _download_sync just takes its defaults (no DB).""" + def get(self, key, default=None): + return default + + +def _bare_client(tmp_path): + c = object.__new__(hc.HiFiClient) # skip __init__ (DB / network) + c.download_path = tmp_path + c._engine = None + c.shutdown_check = None + return c + + +def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} # real track length + tiers = [] + c._get_hls_manifest = lambda tid, quality='lossless': ( + tiers.append(quality) or + {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 30.0}) # preview at EVERY tier + c._download_segment_with_retry = lambda url: pytest.fail("downloaded a preview segment!") + + result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') + assert result is None # → orchestrator falls back + # A preview means the SOURCE only has a preview — every lower tier is the same clip, + # so it must ABORT on the first preview, not cascade down into a lower-tier preview. + assert tiers == ['lossless'] + + +def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} + c._get_hls_manifest = lambda tid, quality='lossless': { + 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 215.0} # full length → must NOT skip + seg_calls = [] + + def _seg(url): + seg_calls.append(url) + raise RuntimeError("stop after the gate") + c._download_segment_with_retry = _seg + + c._download_sync('dl1', 12345, 'x') + assert seg_calls # it got PAST the preview gate to download + + +def test_download_sync_aborts_on_a_faked_full_length_file_no_tier_cascade(tmp_path, monkeypatch): + # The real #895 case: manifest + container claim FULL length, but the finished file + # decodes to 30s. It must abort HiFi (return None) on the first tier — NOT drop to the + # lossy 'high' tier (the same 30s preview, which dodges the bitrate check). + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} + tiers = [] + c._get_hls_manifest = lambda tid, quality='lossless': ( + tiers.append(quality) or + {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 215.0}) + c._download_segment_with_retry = lambda url: b'\x00' * 200_000 # > MIN_AUDIO_SIZE + c._demux_flac = lambda i, o: o.write_bytes(b'\x00' * 200_000) # produce the 'flac' + c._probe_real_seconds = lambda p: 30.0 # decodes to 30s + c._probe_audio_seconds = lambda p: 215.0 # faked container claim + c._flac_props = lambda p: (44100, 16, 2) + + result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') + assert result is None and tiers == ['lossless'] # aborted, did NOT try 'high' + + +def test_download_sync_does_not_reject_when_track_length_unknown(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 0} # expected unknown + c._get_hls_manifest = lambda tid, quality='lossless': { + 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 30.0} # short, but expected is unknown + seg_calls = [] + c._download_segment_with_retry = lambda url: (seg_calls.append(url), (_ for _ in ()).throw(RuntimeError("stop")))[0] + + c._download_sync('dl1', 12345, 'x') + assert seg_calls # unknown length → no rejection, proceeds + + +# ── faked-header previews: claim full length everywhere, only ~30s of real audio ── +# (real numbers measured from issue #895's files: every "lossless" FLAC was a 30s +# preview with STREAMINFO total_samples faked to the full length.) +from core.hifi_client import is_fake_lossless_bitrate, is_preview_download, parse_ffmpeg_time + + +def test_real_issue895_files_are_all_flagged_by_bitrate(): + # (size_bytes, claimed_seconds) for the actual files — 16-bit/44.1kHz stereo FLAC. + samples = [ + (4_080_000, 216), # Save Your Tears (151 kbps claimed) + (6_770_000, 150), # I Ain't Worried (362 kbps — the highest, nearest the line) + (2_240_000, 326), # Lose Yourself (55 kbps) + (4_190_000, 285), # The Real Slim Shady + (4_910_000, 170), # APT + ] + for size, secs in samples: + assert is_fake_lossless_bitrate(size, secs, 44100, 16, 2) is True, (size, secs) + + +def test_real_full_lossless_is_not_flagged(): + # a genuine 16/44.1 lossless track is ~700-1100 kbps → well above the floor + assert is_fake_lossless_bitrate(25_000_000, 216, 44100, 16, 2) is False # ~926 kbps + assert is_fake_lossless_bitrate(12_000_000, 216, 44100, 16, 2) is False # ~444 kbps, still real + + +def test_bitrate_check_is_conservative_on_unknowns(): + assert is_fake_lossless_bitrate(0, 216, 44100, 16, 2) is False + assert is_fake_lossless_bitrate(4_080_000, 0, 44100, 16, 2) is False + assert is_fake_lossless_bitrate(4_080_000, 216, 0, 0, 0) is False + + +def test_is_preview_download_decode_path(): + # decoded 30s of a claimed 216s → fake, regardless of bitrate + fake, why = is_preview_download(30.0, 216.0, is_lossless=False, size_bytes=99_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake and "decoded 30s of 216s" in why + + +def test_is_preview_download_bitrate_path_when_no_decoder(): + # real_seconds=0 (no ffmpeg) → fall back to the lossless bitrate check + fake, why = is_preview_download(0.0, 216.0, is_lossless=True, size_bytes=4_080_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake and "kbps lossless" in why + + +def test_is_preview_download_passes_a_real_file(): + fake, _ = is_preview_download(214.0, 216.0, is_lossless=True, size_bytes=25_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake is False + + +def test_is_preview_download_lossy_no_decoder_is_not_flagged(): + # a lossy tier (mp3/m4a) with no decode info → can't bitrate-check → don't reject + fake, _ = is_preview_download(0.0, 216.0, is_lossless=False, size_bytes=2_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake is False + + +def test_parse_ffmpeg_time_reads_the_last_progress_line(): + stderr = "frame= ... time=00:00:12.34 bitrate=...\nframe= ... time=00:00:30.05 bitrate=..." + assert abs(parse_ffmpeg_time(stderr) - 30.05) < 0.01 + assert parse_ffmpeg_time("no time here") == 0.0 diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index fbb3332e..17648790 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -59,8 +59,12 @@ class _RebuildDB: class _Cfg: - def __init__(self, root, mode="symlink"): - self._d = {"playlists.materialize_path": root, "playlists.materialize_mode": mode} + def __init__(self, root, mode="symlink", item_template=""): + self._d = { + "playlists.materialize_path": root, + "playlists.materialize_mode": mode, + "file_organization.templates": {"playlist_item": item_template}, + } def get(self, key, default=None): return self._d.get(key, default) @@ -266,3 +270,30 @@ def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): assert name == "Mix" and s.linked == 1 # only A owned; Gone skipped assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() assert not (tmp_path / "Playlists" / "Off").exists() + + +def test_playlist_item_template_renames_entries(tmp_path: Path): + """The custom-naming opt-in: a configured playlist_item template renames the + files INSIDE the playlist folder (real library file untouched), with $position + coming straight from playlist order.""" + a = (tmp_path / "Music" / "Artist X" / "07 - A.flac") + a.parent.mkdir(parents=True, exist_ok=True) + a.write_bytes(b"audio") + db = _RebuildDB({"A": str(a)}) + cfg = _Cfg(str(tmp_path / "Playlists"), mode="copy", item_template="$position - $title") + results = rebuild_organized_playlists_from_db(db, cfg, profile_id=1) + assert results[0][0] == "Mix" + mix = tmp_path / "Playlists" / "Mix" + assert sorted(p.name for p in mix.iterdir()) == ["01 - A.flac"] # templated, NOT "07 - A.flac" + + +def test_empty_playlist_item_template_keeps_library_filename(tmp_path: Path): + """Back-compat: with no template configured, entries keep the library filename.""" + a = (tmp_path / "Music" / "Artist X" / "07 - A.flac") + a.parent.mkdir(parents=True, exist_ok=True) + a.write_bytes(b"audio") + db = _RebuildDB({"A": str(a)}) + cfg = _Cfg(str(tmp_path / "Playlists"), mode="copy") # item_template="" (default) + rebuild_organized_playlists_from_db(db, cfg, profile_id=1) + mix = tmp_path / "Playlists" / "Mix" + assert sorted(p.name for p in mix.iterdir()) == ["07 - A.flac"] # unchanged diff --git a/tests/test_youtube_cookies.py b/tests/test_youtube_cookies.py new file mode 100644 index 00000000..0d7d3a04 --- /dev/null +++ b/tests/test_youtube_cookies.py @@ -0,0 +1,100 @@ +"""Settings → YouTube cookie options: browser store vs a pasted cookies.txt. + +#902: syncing a YouTube *Music* "Liked Music" playlist (list=LM) needs auth, and on +a server/Docker box there's no local browser for cookiesfrombrowser to read — so we +let users paste a cookies.txt (yt-dlp cookiefile). These pin the precedence (so the +two cookie sources can never both be emitted), the paste validation (junk must not be +written out and break yt-dlp), and the fail-safe write (a blank save never wipes a +saved file). +""" + +from __future__ import annotations + +from core.youtube_cookies import ( + PASTE_MODE, + build_youtube_cookie_opts, + looks_like_cookiefile, + write_pasted_cookiefile, +) + +NETSCAPE = ( + "# Netscape HTTP Cookie File\n" + ".youtube.com\tTRUE\t/\tTRUE\t1999999999\tLOGIN_INFO\tsecretvalue\n" + ".youtube.com\tTRUE\t/\tTRUE\t1999999999\tSID\tanother\n" +) + + +# ── precedence (pure opts) ────────────────────────────────────────────────── + +def test_empty_mode_is_anonymous(): + assert build_youtube_cookie_opts("") == {} + assert build_youtube_cookie_opts(None) == {} + + +def test_browser_mode_uses_cookiesfrombrowser(): + assert build_youtube_cookie_opts("firefox") == {"cookiesfrombrowser": ("firefox",)} + + +def test_paste_mode_uses_cookiefile_when_present(): + opts = build_youtube_cookie_opts(PASTE_MODE, "/cfg/youtube_cookies.txt", cookiefile_exists=True) + assert opts == {"cookiefile": "/cfg/youtube_cookies.txt"} + + +def test_paste_mode_without_a_real_file_is_anonymous_not_broken(): + # stale/missing path must NOT become a cookiefile arg yt-dlp would choke on + assert build_youtube_cookie_opts(PASTE_MODE, "/cfg/gone.txt", cookiefile_exists=False) == {} + assert build_youtube_cookie_opts(PASTE_MODE, "", cookiefile_exists=True) == {} + + +def test_sources_are_mutually_exclusive(): + # a browser name is never PASTE_MODE, so cookiefile + cookiesfrombrowser can't co-occur + for mode in ("chrome", "firefox", PASTE_MODE, ""): + opts = build_youtube_cookie_opts(mode, "/x.txt", cookiefile_exists=True) + assert not ("cookiefile" in opts and "cookiesfrombrowser" in opts) + + +# ── paste validation ──────────────────────────────────────────────────────── + +def test_accepts_netscape_header_and_cookie_rows(): + assert looks_like_cookiefile(NETSCAPE) is True + # no header but a valid tab-separated cookie row still counts + assert looks_like_cookiefile(".youtube.com\tTRUE\t/\tTRUE\t123\tSID\tv") is True + + +def test_rejects_junk_paste(): + assert looks_like_cookiefile("") is False + assert looks_like_cookiefile(" ") is False + assert looks_like_cookiefile(None) is False + assert looks_like_cookiefile("https://music.youtube.com/playlist?list=LM") is False + assert looks_like_cookiefile('{"cookies": []}') is False + assert looks_like_cookiefile("# Netscape HTTP Cookie File\n# only comments\n") is False + + +# ── fail-safe write ───────────────────────────────────────────────────────── + +def test_write_persists_valid_cookiefile(tmp_path): + dest = tmp_path / "youtube_cookies.txt" + out = write_pasted_cookiefile(NETSCAPE, str(dest)) + assert out == str(dest) + assert dest.read_text().startswith("# Netscape HTTP Cookie File") + + +def test_write_appends_trailing_newline(tmp_path): + dest = tmp_path / "c.txt" + write_pasted_cookiefile(NETSCAPE.rstrip("\n"), str(dest)) + assert dest.read_text().endswith("\n") + + +def test_write_refuses_junk_and_leaves_no_file(tmp_path): + dest = tmp_path / "c.txt" + assert write_pasted_cookiefile("not a cookie file", str(dest)) == "" + assert not dest.exists() + + +def test_write_refuses_junk_without_clobbering_existing(tmp_path): + # a blank/garbage save must NOT wipe a previously-saved cookie file + dest = tmp_path / "c.txt" + write_pasted_cookiefile(NETSCAPE, str(dest)) + before = dest.read_text() + assert write_pasted_cookiefile("", str(dest)) == "" + assert dest.read_text() == before diff --git a/tests/wishlist/test_wishlist_ignore.py b/tests/wishlist/test_wishlist_ignore.py index 783511f2..023b6113 100644 --- a/tests/wishlist/test_wishlist_ignore.py +++ b/tests/wishlist/test_wishlist_ignore.py @@ -146,6 +146,26 @@ def test_gate_blocks_auto_readd_but_manual_bypasses_and_clears(db): assert db.is_track_ignored("t1") is False +def test_user_initiated_add_bypasses_and_clears_keeping_source_type(db): + # #897 / carlosjfcasero: a user manually adds an album track they had + # previously cancelled. It must bypass the gate AND clear the ignore — but + # WITHOUT pretending to be source_type='manual' (the album modal sends + # source_type='album', which the Albums/Singles categorisation relies on, + # and which an automatic path like repair_worker also legitimately uses). + track = _track("t7") + db.add_to_wishlist_ignore("t7", "Owned Song", "Owned Artist", REASON_CANCELLED) + # An automatic 'album' add (e.g. repair_worker) is still correctly blocked. + assert db.add_to_wishlist(track, source_type="album") is False + assert db.is_track_ignored("t7") is True + # The explicit user click (user_initiated) goes through and clears the ignore, + # while the stored source_type stays 'album'. + assert db.add_to_wishlist(track, source_type="album", user_initiated=True) is True + assert db.is_track_ignored("t7") is False + # Provenance preserved: the stored row is still source_type='album', NOT 'manual'. + row = next(r for r in db.get_wishlist_tracks() if str(r.get("spotify_track_id")) == "t7") + assert row.get("source_type") == "album" + + def test_gate_failopen_when_ignore_table_errors(db, monkeypatch): # If the ignore check raises, the add must still succeed (never block). monkeypatch.setattr(db, "is_track_ignored", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) diff --git a/web_server.py b/web_server.py index 0e3f955b..abab67c6 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version — single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each dev→main release. -_SOULSYNC_BASE_VERSION = "2.7.4" +_SOULSYNC_BASE_VERSION = "2.7.6" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" @@ -3126,6 +3126,22 @@ def handle_settings(): f"in Manage Profiles first.", "members_without_password": _stranded}), 400 + # YouTube pasted cookies.txt (server/Docker path): pull it out BEFORE the + # generic persist so the raw cookie blob never lands in config.json — it's + # secret + bulky. We validate up front and store only a file path. + _yt_in = new_settings.get('youtube') + _yt_paste = _yt_in.pop('cookies_paste', None) if isinstance(_yt_in, dict) else None + if _yt_paste is not None and str(_yt_paste).strip(): + from core.youtube_cookies import looks_like_cookiefile, write_pasted_cookiefile + if not looks_like_cookiefile(_yt_paste): + return jsonify({"success": False, + "error": "That doesn't look like a cookies.txt file. Export it " + "with a 'Get cookies.txt LOCALLY' browser extension and " + "paste the whole file."}), 400 + _cookie_path = str(config_manager.config_path.parent / "youtube_cookies.txt") + if write_pasted_cookiefile(_yt_paste, _cookie_path): + config_manager.set('youtube.cookies_file', _cookie_path) + if 'active_media_server' in new_settings: config_manager.set_active_media_server(new_settings['active_media_server']) @@ -14980,15 +14996,20 @@ def clean_youtube_artist(artist_string): def _youtube_cookie_opts(): """yt-dlp cookie options matching the rest of the app (Settings → YouTube). - Per-video extraction needs these to get past YouTube's bot checks.""" - opts = {} + + Per-video extraction needs these to get past YouTube's bot checks, and private + playlists (a user's "Liked Music", list=LM) need them to be visible at all. The + dropdown is either a browser name (cookiesfrombrowser, local installs) or the + PASTE_MODE sentinel, in which case we point yt-dlp at the pasted cookies.txt that + server/Docker users supply. Precedence + emptiness live in core.youtube_cookies.""" + from core.youtube_cookies import build_youtube_cookie_opts try: - cb = config_manager.get('youtube.cookies_browser', '') - if cb: - opts['cookiesfrombrowser'] = (cb,) + mode = config_manager.get('youtube.cookies_browser', '') + path = config_manager.get('youtube.cookies_file', '') + exists = bool(path) and os.path.exists(path) + return build_youtube_cookie_opts(mode, path, cookiefile_exists=exists) except Exception: # noqa: S110 - cookie config is best-effort; resolve still works without it - pass - return opts + return {} def _fetch_youtube_video_artist(video_id, cookie_opts): @@ -16431,16 +16452,40 @@ def _run_soulsync_deep_scan(): logger.info(f"[SoulSync Deep Scan] {len(db_paths)} tracks in soulsync DB") - # Phase 3: Find untracked files (in Transfer but not in DB) - untracked = transfer_files - db_paths - # Also check with normalized paths (Windows vs Unix separators) - if untracked: - db_paths_normalized = {p.replace('\\', '/') for p in db_paths} - untracked = {f for f in untracked if f.replace('\\', '/') not in db_paths_normalized} + # Phase 3: Plan the untracked → Staging move, with the data-loss guard (#904). + # A path-only diff treats EVERY file the DB doesn't know about as "a new arrival + # to relocate". When the DB is empty/out of sync with disk (volume swap, DB reset, + # external tag edits) but Transfer holds the real library, that flags the whole + # library as untracked and relocates all of it. The planner refuses the move when + # the untracked share is implausibly large (the desync signature) or when the user + # marked Transfer as their permanent library — leaving files in place and warning. + from core.library.standalone_scan import ( + plan_standalone_deep_scan, BLOCK_TRANSFER_PERMANENT, BLOCK_DESYNC, + ) + never_move = bool(config_manager.get('import.transfer_is_permanent', False)) + plan = plan_standalone_deep_scan(transfer_files, db_paths, never_move=never_move) + untracked = plan['untracked'] + move_blocked = plan['move_blocked'] + block_reason = plan['block_reason'] - # Phase 4: Move untracked files to Staging for auto-import + # Phase 4: Move untracked files to Staging for auto-import — unless guarded. moved_count = 0 - if untracked and os.path.isdir(staging_path): + blocked_count = 0 + if untracked and move_blocked: + blocked_count = len(untracked) + if block_reason == BLOCK_TRANSFER_PERMANENT: + warn = (f"Deep scan: {blocked_count} file(s) in Transfer aren't in the database, " + f"but Transfer is marked your permanent library — nothing was moved.") + else: # BLOCK_DESYNC + pct = round(100 * blocked_count / max(1, len(transfer_files))) + warn = (f"Deep scan STOPPED to protect your library: {blocked_count} of " + f"{len(transfer_files)} files in Transfer ({pct}%) aren't in the database. " + f"That usually means the database is out of sync with disk, not that you " + f"have {blocked_count} new files — so NOTHING was moved. Re-sync/import " + f"before scanning, or enable 'Transfer is my permanent library'.") + logger.warning(f"[SoulSync Deep Scan] {warn}") + add_activity_item("", "SoulSync Deep Scan — move blocked", warn, "Now") + elif untracked and os.path.isdir(staging_path): _db_update_phase_callback('moving_untracked') for file_path in untracked: try: @@ -16472,6 +16517,23 @@ def _run_soulsync_deep_scan(): stale_track_ids.append(db_path) stale_count += 1 + # Guard the deletes the same way as the move (#904): if a desync blocked the + # move, the DB<->disk mapping is unreliable, so os.path.exists may be lying for + # every file — don't delete rows. Also independently skip when the stale share + # is implausibly large (storage unreachable / remount), mirroring the orphan guard. + from core.library.stale_guard import is_implausible_stale_removal + if move_blocked and block_reason == BLOCK_DESYNC: + if stale_track_ids: + logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count} 'stale' " + f"records — move was blocked for desync, mapping is unreliable.") + stale_track_ids = [] + stale_count = 0 + elif is_implausible_stale_removal(stale_count, len(db_paths)): + logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count}/{len(db_paths)} " + f"'stale' records — implausibly large share, storage likely unreachable.") + stale_track_ids = [] + stale_count = 0 + # Remove stale records if stale_track_ids: try: @@ -16504,9 +16566,11 @@ def _run_soulsync_deep_scan(): summary = f"Deep scan complete: {len(transfer_files)} files scanned" if moved_count > 0: summary += f", {moved_count} untracked files moved to Staging" + if blocked_count > 0: + summary += f", {blocked_count} untracked files LEFT IN PLACE (move blocked — see warning)" if stale_count > 0: summary += f", {stale_count} stale records removed" - if moved_count == 0 and stale_count == 0: + if moved_count == 0 and blocked_count == 0 and stale_count == 0: summary += " — library is clean" logger.info(f"[SoulSync Deep Scan] {summary}") @@ -27178,6 +27242,117 @@ def export_watchlist(): return jsonify({"success": False, "error": str(e)}), 500 +# ── Playlist export to ListenBrainz / JSPF (#903) ──────────────────────────── +# Resolve each mirrored-playlist track to a MusicBrainz recording MBID (cache -> DB -> +# file tag -> live MB), build a JSPF, and either hand it back as a download or create the +# playlist directly on ListenBrainz. Runs in a background thread with live status the +# mirrored-playlist card polls. Entirely additive — new routes + an in-memory job registry. +_playlist_export_jobs = {} +_playlist_export_jobs_lock = threading.Lock() + + +def _run_playlist_export(job_id, playlist_id, title, mode): + job = _playlist_export_jobs[job_id] + try: + from core.exports.export_sources import build_resolve_fn + from core.exports.playlist_export import resolve_playlist_tracks + from core.exports.jspf_export import build_jspf + + db = get_database() + tracks = db.get_mirrored_playlist_tracks(int(playlist_id)) + job['total'] = len(tracks) + job['phase'] = 'resolving' + + resolve_fn = build_resolve_fn() + + def on_progress(done, total, stats): + job['done'] = done + job['stats'] = dict(stats) + + out = resolve_playlist_tracks(tracks, resolve_fn, on_progress=on_progress) + jspf, summary = build_jspf(title, out['resolved'], creator='SoulSync') + job['summary'] = summary + job['jspf'] = jspf + job['stats'] = out['stats'] + + if mode == 'push': + job['phase'] = 'pushing' + from core.listenbrainz_client import ListenBrainzClient + client = ListenBrainzClient() + # Re-export updates the same LB playlist in place instead of duplicating it (#903). + existing = db.get_playlist_export_target(int(playlist_id), 'listenbrainz') + res = client.create_or_update_playlist(title, jspf['playlist']['track'], existing_mbid=existing) + job['push'] = res + if res.get('success'): + if res.get('playlist_mbid'): + db.set_playlist_export_target(int(playlist_id), 'listenbrainz', res['playlist_mbid']) + job['phase'] = 'done' + else: + job['phase'] = 'error' + job['error'] = res.get('error') or 'ListenBrainz push failed' + else: + job['phase'] = 'done' + except Exception as e: + logger.error(f"[Playlist Export] job {job_id} failed: {e}") + job['phase'] = 'error' + job['error'] = str(e) + + +@app.route('/api/playlists//export/listenbrainz', methods=['POST']) +def start_playlist_export_listenbrainz(playlist_id): + """Start a background export of a mirrored playlist to ListenBrainz/JSPF. + + Body: {"mode": "download"|"push"} (default "download"). Returns {job_id} to poll.""" + try: + body = request.get_json(silent=True) or {} + mode = 'push' if body.get('mode') == 'push' else 'download' + db = get_database() + meta = db.get_mirrored_playlist(int(playlist_id)) or {} + title = (meta.get('name') or meta.get('title') or 'SoulSync Export').strip() or 'SoulSync Export' + + import uuid + job_id = uuid.uuid4().hex + with _playlist_export_jobs_lock: + _playlist_export_jobs[job_id] = { + 'job_id': job_id, 'playlist_id': str(playlist_id), 'title': title, + 'mode': mode, 'phase': 'starting', 'done': 0, 'total': 0, + 'stats': {}, 'error': None, + } + t = threading.Thread(target=_run_playlist_export, args=(job_id, playlist_id, title, mode), daemon=True) + t.start() + return jsonify({"success": True, "job_id": job_id}) + except Exception as e: + logger.error(f"Playlist export start failed: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + + +@app.route('/api/playlists/export/status/', methods=['GET']) +def playlist_export_status(job_id): + """Live status for an export job (polled by the mirrored-playlist card).""" + job = _playlist_export_jobs.get(job_id) + if not job: + return jsonify({"success": False, "error": "unknown job"}), 404 + # Don't ship the full JSPF on every poll — just status + coverage. + out = {k: v for k, v in job.items() if k != 'jspf'} + out['has_download'] = bool(job.get('jspf')) + return jsonify({"success": True, "job": out}) + + +@app.route('/api/playlists/export/download/', methods=['GET']) +def playlist_export_download(job_id): + """Download a completed export's JSPF file.""" + job = _playlist_export_jobs.get(job_id) + if not job or not job.get('jspf'): + return jsonify({"success": False, "error": "no export available"}), 404 + import json as _json + safe = re.sub(r'[^\w\-]+', '_', (job.get('title') or 'playlist')).strip('_') or 'playlist' + return Response( + _json.dumps(job['jspf'], indent=2, ensure_ascii=False), + mimetype='application/jspf+json', + headers={'Content-Disposition': f'attachment; filename="{safe}.jspf"'}, + ) + + @app.route('/api/library/artists/export', methods=['GET']) def export_library_artists(): """Export the WHOLE library artist roster (name + every source id/url we have, diff --git a/webui/index.html b/webui/index.html index d9c8a301..808324a3 100644 --- a/webui/index.html +++ b/webui/index.html @@ -1917,8 +1917,8 @@
📄
Drop your file here
or click to browse
-
Supported: CSV, TSV, TXT
- +
Supported: CSV, TSV, TXT, M3U, M3U8
+
@@ -1929,6 +1929,10 @@ TXT One track per line (e.g. Artist - Title). Format and separator can be adjusted after upload.
+
+ M3U / M3U8 + Standard playlist files. Artist, title and duration are read automatically from #EXTINF lines (or the file name for simple playlists). +
@@ -4969,10 +4973,24 @@ +
If YouTube shows "Sign in to confirm you're not a bot", select your browser here. - SoulSync will use your browser's YouTube cookies to authenticate. + SoulSync will use your browser's YouTube cookies to authenticate. Reading a browser + only works when that browser is on the same machine as SoulSync — on a + server/Docker box, choose Paste cookies.txt instead. +
+ +
@@ -5458,6 +5476,13 @@ Variables: $playlist, $albumartist, $artist, $artistletter, $title, $year, $quality (filename only). Use ${var} to append text
+
+ + + Renames the files inside each "Organize by Playlist" folder (your real library files are never touched). Filename only — no folders. Variables: $position (playlist order), $artist, $album, $track, $title. Must include $title and cannot contain "/". Example: $position - $artist - $title → 01 - Daft Punk - One More Time.flac. Empty = keep the original library filename. +
+
+ +
+ +
+
+ Off by default. Turn this on if your Transfer folder is your + real, permanent library (served by Navidrome/Plex/etc.) rather than a scratch + drop area. A Deep Scan then never relocates files it doesn't recognize out to + Staging — it leaves them in place and just reports them. Even with this off, a + Deep Scan now refuses to move files when the database looks badly out of sync + with disk, as a safety guard against wiping a library (issue #904). +
@@ -7179,6 +7219,10 @@
+

Import from File

-

Import track lists from CSV, TSV, or plain text files. Drag and drop a file or click to browse. SoulSync parses the file, lets you preview and map columns, then creates a mirrored playlist for discovery and download.

+

Import track lists from CSV, TSV, M3U/M3U8, or plain text files. Drag and drop a file or click to browse. SoulSync parses the file, lets you preview and map columns, then creates a mirrored playlist for discovery and download.

  • CSV/TSV: Auto-detects columns; map Artist, Title, and Album from dropdowns
    • +
  • M3U/M3U8: Read automatically — artist, title and duration come from #EXTINF lines (or the file name for simple playlists). Round-trips with SoulSync's own M3U export
  • Text files: One track per line; choose Artist-Title or Title-Artist order and separator (dash, tab, pipe, etc.)
  • Preview parsed tracks before importing
  • Name your playlist and it becomes a mirrored playlist for sync
    • @@ -1308,7 +1309,7 @@ const DOCS_SECTIONS = [

Import from Text File

-

Import track lists from CSV, TSV, or TXT files. Upload a file with columns for artist, album, and track title:

+

Import track lists from CSV, TSV, TXT, or M3U/M3U8 files. Upload a file with columns for artist, album, and track title (M3U playlists are read automatically):

  1. Click Import from File and select your text file
  2. Choose the separator (comma, tab, or pipe)
    3. diff --git a/webui/static/helper.js b/webui/static/helper.js index 4763f5c4..16673477 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -722,8 +722,8 @@ const HELPER_CONTENT = { }, '.sync-tab-button[data-tab="import-file"]': { title: 'Import from File', - description: 'Import track lists from CSV, TSV, or plain text files. Drag and drop or browse for a file, map columns, then create a playlist for sync.', - tips: ['Supports CSV, TSV, and plain text (one track per line)', 'Column mapping for CSV/TSV files', 'Creates a mirrored playlist for persistent state'], + description: 'Import track lists from CSV, TSV, M3U/M3U8, or plain text files. Drag and drop or browse for a file, map columns, then create a playlist for sync.', + tips: ['Supports CSV, TSV, M3U/M3U8, and plain text (one track per line)', 'M3U/M3U8 is read automatically (artist, title, duration from #EXTINF)', 'Column mapping for CSV/TSV files', 'Creates a mirrored playlist for persistent state'], docsId: 'sync-import-file' }, '.sync-tab-button[data-tab="mirrored"]': { @@ -3404,16 +3404,17 @@ function closeHelperSearch() { const WHATS_NEW = { // Convention: keep only the CURRENT release here, plus a single brief // "Earlier versions" summary entry. Don't accumulate old per-version blocks. - '2.7.4': [ - { date: 'June 2026 — 2.7.4 release' }, - { title: 'Re-identify a track (#889)', desc: 'filed a track under the wrong release? a new ⇄ button in the library Enhanced view lets you re-identify it — search any source (tabs, defaults to your active one), see the same song across its single / EP / album with type badges, pick the right one, and soulsync re-files the file you already have under that release with the correct year, in-album track number, and art. replace the original or keep both.', page: 'artists' }, - { title: 'Track titles no longer keep the "01 - " (#890)', desc: 'files with no embedded title tag used to import as "01 - Song Title" (the filename stem) — which never matched the canonical "Song Title", so the real track showed as a false "missing". the number prefix is now stripped, conservatively, so titles like "7 Rings" or "1-800-273-8255" are left alone.', page: 'library' }, - { title: 'Clear dead cover-art folders (#891)', desc: 'a Library Reorganize that moves an album now sweeps the leftover cover.jpg / .lrc / sidecars from the old folder so it actually empties. plus an opt-in "Remove Residual Files" toggle on the Empty Folder Cleaner clears the image-/sidecar-only folders you already have.', page: 'tools' }, - { title: 'AAC as an opt-in quality tier (#886)', desc: 'soulseek downloads can now include AAC (.m4a) as a selectable quality tier, ranked above MP3 and below FLAC. purely additive — off by default; every existing profile behaves exactly as before until you enable it.', page: 'settings' }, - { title: 'Spotify Free enrichment status (#887)', desc: 'if you run enrichment on Spotify Free (no spotify auth), the dashboard button now reads "Running (Spotify Free)" instead of wrongly showing "Not Authenticated".', page: 'dashboard' }, - { title: 'Cleaner album imports (Sokhi)', desc: 'songs from the same album now group under one canonical release id (no more split discographies or mixed cover art), a single can be matched to its parent album, a mid-enrichment crash on an art-less file no longer leaves it untagged, and a sequel digit glued to a CJK title no longer matches the wrong album.', page: 'library' }, - { title: 'More fixes', desc: 'NZBGet imports from the finished location instead of the incomplete "….#NZBID" folder (#884); setting your timezone to Australia/Sydney no longer makes the cache-maintenance job loop every 5 seconds (#885); and the artist-detail header no longer bleeds the blurred artist photo behind it.', page: 'downloads' }, - { title: 'Earlier versions', desc: '2.7.3 added the Quality Upgrade Finder (find + upgrade tracks you own in worse quality than available), a wishlist ignore-list (#874), quarantine duplicate-grouping (#876), the "Track 01" position fix, and Tidal discovery/favorites fixes (#867, #880). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.' }, + '2.7.6': [ + { date: 'June 2026 — 2.7.6 release' }, + { title: 'Export playlists to ListenBrainz (#903)', desc: 'soulsync already pulls playlists IN — now it can push one OUT. every mirrored-playlist card gets a 📤 export button: download a standard .jspf file, or sync the playlist straight to your ListenBrainz account. each track is matched to its musicbrainz recording id (cache → your library → file tag → live musicbrainz), with live "matching N/M" status on the card. re-syncing updates the SAME LB playlist in place instead of duplicating it.', page: 'sync' }, + { title: 'YouTube Liked Music sync (#902)', desc: 'you can now sync your youtube music "Liked Music" playlist (music.youtube.com/playlist?list=LM). it\'s private, so it needs auth — added a "paste cookies.txt" option in Settings → YouTube so server/docker installs (or browsers yt-dlp can\'t read, like Zen) can supply their login from anywhere.', page: 'settings' }, + { title: 'Deep Scan won\'t relocate your library (#904)', desc: 'a standalone Deep Scan moves unrecognized files into Staging. if the DB was empty/out of sync with disk, it treated your ENTIRE library as unrecognized and relocated all of it. now a guard refuses the move when the unrecognized share is implausibly large, leaves files in place, and warns — plus a "Transfer is my permanent library — never move files out" toggle.', page: 'settings' }, + { title: 'Dashboard performance', desc: 'a pass at "soulsync works my GPU hard just sitting there". the sidebar sweep animates transform not left, particle glows are cached sprites, blur radii + redundant/invisible card shadows are trimmed, and low-power machines auto-drop to performance mode. the effects all stay — they\'re just cheaper to draw.', page: 'dashboard' }, + { title: 'File-import manual matches stick (#901)', desc: 'a manual match on a file-imported playlist track is no longer forgotten on re-sync — the tracks now carry a stable id (existing ones backfilled once), so your pick survives.', page: 'sync' }, + { title: 'Manual match heals a stale Plex key', desc: 'a Find & Add match whose stored Plex ratingKey went stale is now re-resolved against a live Plex search instead of silently breaking on the next sync.', page: 'sync' }, + { title: 'Multi-disc albums', desc: 'a track now files into the Disc folder that matches its own disc tag (no more disc-2 tracks landing in Disc 1), and a track is never written disc-less.', page: 'downloads' }, + { title: 'Auto-download track numbers', desc: 'a track auto-grabbed from the playlist pipeline / wishlist / watchlist now gets its real in-album position instead of being tagged 1/1.', page: 'downloads' }, + { title: 'Earlier versions', desc: '2.7.5 was a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess (#895), M3U import (#893), ignore-list management (#897), per-playlist file naming. 2.7.4 added re-identify; 2.7.3 the Quality Upgrade Finder + ignore-list (#874); 2.7.2 playlist-folder mirroring + M3U export; 2.7.1 download verification + a review queue (#852); 2.7.0 made multi-user real.' }, ], }; @@ -3444,38 +3445,54 @@ const WHATS_NEW = { // usage_note?: 'optional hint shown at the bottom' } const VERSION_MODAL_SECTIONS = [ { - title: "Re-identify a track (#889)", - description: "filed a track under the wrong release? re-identify it from the library without re-downloading — soulsync re-files the file you already have under the release you pick.", + title: "Export playlists to ListenBrainz (#903)", + description: "soulsync already pulls playlists IN from everywhere — now it can push one back OUT.", features: [ - "a ⇄ button in the Enhanced library view opens a search across any configured source (tabs, defaults to your active one)", - "see the same song across its single / EP / album, each with a type badge, and pick the right collection", - "it re-files under that release with the correct year, in-album track number, and album art", - "replace the original entry or keep both — and it can never delete the file if you pick the release it's already in", + "📤 export button on every mirrored-playlist card — download a standard .jspf file, or sync the playlist straight onto your ListenBrainz account", + "each track is matched to its musicbrainz recording id via a cheapest-first waterfall — cache → your library → the file's own tag → a live musicbrainz lookup — and the result is cached so the same song never costs twice", + "live \"matching N/M · X matched\" status on the card, and re-syncing updates the SAME LB playlist in place instead of making duplicates", ], - usage_note: "Library → an artist → Enhanced view → ⇄ on a track", + usage_note: "find it on the 📤 button when you hover a mirrored playlist card. tracks that can't be resolved to a musicbrainz id are skipped (LB requires them) and counted, so you see the coverage.", }, { - title: "Cleaner libraries & imports", - description: "a batch of fixes that keep the library tidy and matchable.", + title: "YouTube Liked Music sync (#902)", + description: "sync your youtube music \"Liked Music\" playlist (music.youtube.com/playlist?list=LM).", features: [ - "#890 — track titles no longer keep the \"01 - \" from a filename (which caused false \"missing\" tracks); stripped conservatively so \"7 Rings\" / \"1-800-273-8255\" are left alone", - "#891 — a reorganize now sweeps leftover cover.jpg / .lrc from the old folder, plus an opt-in \"Remove Residual Files\" toggle clears image-only folders you already have", - "Sokhi — same-album songs group under one canonical release (no split discographies / mixed cover art); a single can match its parent album; a mid-enrichment crash no longer leaves a file untagged; a CJK-title sequel-digit no longer matches the wrong album", + "it's a private playlist, so it needs auth — the existing browser-cookie option only works when the browser is on the same machine as soulsync", + "added a \"paste cookies.txt\" option in Settings → YouTube so server/docker installs (or browsers yt-dlp can't read, like Zen) can supply their login from anywhere", ], }, { - title: "Quality & sources", - description: "more control over downloads, plus a couple of source fixes.", + title: "Deep Scan won't relocate your library (#904)", + description: "a standalone Deep Scan moves unrecognized files into Staging — which went very wrong when the DB was out of sync with disk.", features: [ - "#886 — AAC (.m4a) as an opt-in soulseek quality tier, ranked above MP3 and below FLAC; off by default so nothing changes until you enable it", - "#887 — enrichment on Spotify Free now reads \"Running (Spotify Free)\" instead of \"Not Authenticated\"", - "#884 — NZBGet imports from the finished location, not the incomplete \"….#NZBID\" folder", - "#885 — Australia/Sydney timezone no longer makes the cache-maintenance job loop every 5 seconds", + "if the DB was empty/desynced, a scan treated your ENTIRE library as unrecognized and relocated all of it (one user lost ~1,500 tracks into Staging)", + "now a guard refuses the move when the unrecognized share is implausibly large (the desync signature), leaves everything in place, and warns instead", + "new \"Transfer is my permanent library — never move files out\" toggle for people whose Transfer folder IS their live library", ], }, { - title: "Earlier in 2.7.3 / 2.7.2 / 2.7.1 / 2.7.0", - description: "2.7.3 added the Quality Upgrade Finder (find + upgrade tracks you own in worse quality than available), a wishlist ignore-list (#874), quarantine duplicate-grouping (#876), the \"Track 01\" position fix, and Tidal discovery/favorites fixes (#867, #880). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification (acoustid checks every download) + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.", + title: "Dashboard performance + fixes", + description: "lighter dashboard, and a handful of matching/import fixes.", + features: [ + "dashboard — the sidebar sweep animates transform not left, particle glows are cached sprites, blur/shadow excess is trimmed, and low-power machines auto-drop to performance mode (the effects all stay, just cheaper to draw)", + "#901 — a manual match on a file-imported playlist track now survives re-sync (stable ids, existing ones backfilled)", + "a Find & Add match whose stored Plex key went stale is re-resolved against a live Plex search instead of breaking", + "multi-disc albums file each track in the Disc folder matching its tag (no disc-less / wrong-disc tracks); auto-grabbed tracks get their real in-album number instead of 1/1", + ], + }, + { + title: "Earlier in 2.7.5", + description: "a fix-heavy cycle — matching & artwork accuracy plus a few quality-of-life features.", + features: [ + "special-edition cover art (a \"Gustave Edition\" uses its OWN cover), deezer track numbers, and the \"The\" duplicate fix", + "HiFi 30-second previews disguised as full songs are caught and rejected (#895)", + "import M3U / M3U8 playlists (#893), name files inside playlist folders, find & add remembered, ignore-list management (#897), Unraid template fixes (#899)", + ], + }, + { + title: "Earlier in 2.7.4 / 2.7.3 / 2.7.2 / 2.7.1 / 2.7.0", + description: "2.7.4 added re-identify (re-file an imported track under the right release without re-downloading) plus library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874); 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification (acoustid checks every download) + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.", features: [], }, ]; diff --git a/webui/static/init.js b/webui/static/init.js index a8a11b98..aea68fab 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -213,6 +213,47 @@ function applyReduceEffects(enabled) { // Bootstrap accent and reduce-effects from localStorage instantly (prevents flash) (function () { + // Auto performance mode on likely-weak hardware. Only acts when this device has + // NO stored preference yet (null) — so it runs at most once and never overrides + // a choice the user (or a prior auto-run) made. Device-scoped via localStorage on + // purpose: a weak laptop shouldn't flip the server setting for the user's other + // machines. Mobile already disables these effects elsewhere, so skip it here. + if (localStorage.getItem('soulsync-reduce-effects') === null) { + const ua = navigator.userAgent || ''; + const isMobile = window.innerWidth <= 768 || /Mobi|Android|iPhone|iPad|iPod/i.test(ua); + const cores = navigator.hardwareConcurrency || 0; // widely supported + const mem = navigator.deviceMemory || 0; // Chromium only; 0 elsewhere + // Conservative — avoid flagging capable machines: <=2 cores, or <=2GB, or a + // low-mid box that's low on BOTH (<=4 cores AND <=4GB). A 4-core/8GB laptop + // (mem>4) is NOT flagged; Firefox/Safari (mem unknown) only trip on <=2 cores. + const weak = !isMobile && ( + (cores > 0 && cores <= 2) || + (mem > 0 && mem <= 2) || + (cores > 0 && cores <= 4 && mem > 0 && mem <= 4) + ); + if (weak) { + localStorage.setItem('soulsync-reduce-effects', '1'); + window._autoPerfModeApplied = true; // show the explainer toast once the UI is up + } + } + + if (window._autoPerfModeApplied) { + // Toast lives in downloads.js (loaded separately) — retry until it's defined. + const fireToast = (tries) => { + if (typeof showToast === 'function') { + showToast('Performance mode is on — this looks like a lower-power device. ' + + 'Turn effects back on in Settings → Appearance.', 'info'); + } else if (tries < 40) { + setTimeout(() => fireToast(tries + 1), 250); + } + }; + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', () => fireToast(0)); + } else { + fireToast(0); + } + } + if (localStorage.getItem('soulsync-reduce-effects') === '1') { document.body.classList.add('reduce-effects'); window._reduceEffectsActive = true; diff --git a/webui/static/particles.js b/webui/static/particles.js index 2b446806..4869bfdb 100644 --- a/webui/static/particles.js +++ b/webui/static/particles.js @@ -101,6 +101,27 @@ // Each preset: { count, init(p, i), update(p, time, i), draw(p, ctx, accent, time), // optional: initExtras(), drawGlobal(ctx, particles, accent, time, extras) } + // Pre-rendered glow sprite. A dashboard particle's glow is a radial gradient + // (its accent colour -> transparent) of a SIZE that's fixed for the particle's + // life — the only thing changing each frame is the pulse ALPHA. Since canvas + // multiplies fillStyle/image alpha by ctx.globalAlpha, we can bake the gradient + // ONCE into an offscreen canvas at full alpha and drawImage it each frame with + // globalAlpha = pulse, instead of building a fresh createRadialGradient + arc-fill + // for all 50 particles every frame. Output is pixel-identical; just far cheaper. + function buildGlowSprite(col, glowSize) { + const s = Math.max(1, Math.ceil(glowSize)); + const c = document.createElement('canvas'); + c.width = s * 2; + c.height = s * 2; + const g = c.getContext('2d'); + const grad = g.createRadialGradient(s, s, 0, s, s, s); + grad.addColorStop(0, `rgba(${col}, 1)`); + grad.addColorStop(1, 'rgba(0,0,0,0)'); + g.fillStyle = grad; + g.fillRect(0, 0, s * 2, s * 2); + return c; + } + const PRESETS = { // ── DASHBOARD — network nodes, connections, data packets, shooting stars ── @@ -130,16 +151,20 @@ const pulse = 0.5 + 0.5 * Math.sin(time * 1.5 + p.phase); const col = shiftAccent(accent, p.hueShift); - // Glow — hubs get bigger, brighter glow + // Glow — hubs get bigger, brighter glow. Cached sprite blit (alpha at + // blit time); pixel-identical to the old per-frame radial gradient. const glowSize = p.isHub ? p.radius * 6 : p.radius * 4; const glowAlpha = p.isHub ? (0.18 + pulse * 0.12) : (0.10 + pulse * 0.07); - const glow = ctx.createRadialGradient(p.x, p.y, 0, p.x, p.y, glowSize); - glow.addColorStop(0, `rgba(${col}, ${glowAlpha})`); - glow.addColorStop(1, 'rgba(0,0,0,0)'); - ctx.beginPath(); - ctx.arc(p.x, p.y, glowSize, 0, Math.PI * 2); - ctx.fillStyle = glow; - ctx.fill(); + if (!p._glowCanvas || p._glowCol !== col || p._glowR !== glowSize) { + p._glowCanvas = buildGlowSprite(col, glowSize); // rebuilt only on accent change + p._glowCol = col; + p._glowR = glowSize; + } + // Multiply by the incoming globalAlpha so transition fades match exactly. + const _glowPrevAlpha = ctx.globalAlpha; + ctx.globalAlpha = _glowPrevAlpha * glowAlpha; + ctx.drawImage(p._glowCanvas, p.x - glowSize, p.y - glowSize, glowSize * 2, glowSize * 2); + ctx.globalAlpha = _glowPrevAlpha; // Core const coreAlpha = p.isHub ? (0.5 + pulse * 0.3) : (0.3 + pulse * 0.2); diff --git a/webui/static/settings.js b/webui/static/settings.js index 6bcffe17..937f7793 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1215,6 +1215,25 @@ async function loadSettingsData() { // Populate YouTube settings document.getElementById('youtube-cookies-browser').value = settings.youtube?.cookies_browser || ''; document.getElementById('youtube-download-delay').value = settings.youtube?.download_delay ?? 3; + // Show the cookies.txt paste box only in "custom" mode. We never echo the + // stored cookie back to the UI (it's secret + lives in a file, not config); + // if one is already saved, say so via placeholder so a blank save won't wipe it. + const _ytCookieSel = document.getElementById('youtube-cookies-browser'); + const _ytPasteBox = document.getElementById('youtube-cookies-paste'); + const _ytPasteGroup = document.getElementById('youtube-cookies-paste-group'); + if (_ytCookieSel && _ytPasteGroup) { + const _toggleYtPaste = () => { + _ytPasteGroup.style.display = _ytCookieSel.value === 'custom' ? '' : 'none'; + }; + if (_ytPasteBox && settings.youtube?.cookies_file) { + _ytPasteBox.placeholder = 'A cookies.txt is saved. Paste again to replace it, or leave blank to keep it.'; + } + _toggleYtPaste(); + if (!_ytCookieSel.dataset.pasteToggleBound) { + _ytCookieSel.addEventListener('change', _toggleYtPaste); + _ytCookieSel.dataset.pasteToggleBound = '1'; + } + } // Update UI based on download source mode updateDownloadSourceUI(); @@ -1266,6 +1285,7 @@ async function loadSettingsData() { document.getElementById('template-album-path').value = settings.file_organization?.templates?.album_path || '$albumartist/$albumartist - $album/$track - $title'; document.getElementById('template-single-path').value = settings.file_organization?.templates?.single_path || '$artist/$artist - $title/$title'; document.getElementById('template-playlist-path').value = settings.file_organization?.templates?.playlist_path || '$playlist/$artist - $title'; + document.getElementById('template-playlist-item').value = settings.file_organization?.templates?.playlist_item || ''; document.getElementById('template-video-path').value = settings.file_organization?.templates?.video_path || '$artist/$title-video'; document.getElementById('disc-label').value = settings.file_organization?.disc_label || 'Disc'; document.getElementById('collab-artist-mode').value = settings.file_organization?.collab_artist_mode || 'first'; @@ -1315,6 +1335,8 @@ async function loadSettingsData() { document.getElementById('import-replace-lower-quality').checked = settings.import?.replace_lower_quality === true; const _folderArtistEl = document.getElementById('import-folder-artist-override'); if (_folderArtistEl) _folderArtistEl.checked = settings.import?.folder_artist_override === true; + const _transferPermEl = document.getElementById('import-transfer-permanent'); + if (_transferPermEl) _transferPermEl.checked = settings.import?.transfer_is_permanent === true; // Populate M3U Export settings document.getElementById('m3u-export-enabled').checked = settings.m3u_export?.enabled === true; @@ -1362,8 +1384,14 @@ async function loadSettingsData() { if (workerOrbsCheckbox) workerOrbsCheckbox.checked = workerOrbsEnabled; applyWorkerOrbsSetting(workerOrbsEnabled); - // Reduce effects toggle - const reduceEffects = settings.ui_appearance?.reduce_effects === true; // default false + // Reduce effects toggle. This flag is device-scoped: localStorage (set by the + // live toggle and by weak-hardware auto-detect) is the source of truth for THIS + // machine; the server value is only the cross-device default used when this + // device has never chosen. Prefer localStorage when present so opening Settings + // doesn't clobber an auto-enabled (or manually-set) per-device choice. + const serverReduce = settings.ui_appearance?.reduce_effects === true; // default false + const localReduce = localStorage.getItem('soulsync-reduce-effects'); // '1' | '0' | null + const reduceEffects = localReduce !== null ? (localReduce === '1') : serverReduce; const reduceCheckbox = document.getElementById('reduce-effects-enabled'); if (reduceCheckbox) reduceCheckbox.checked = reduceEffects; applyReduceEffects(reduceEffects); @@ -2907,6 +2935,21 @@ async function saveSettings(quiet = false) { } } + // Validate the optional "Playlist File Naming" template before saving: it's a + // filename (no path separator) and must include $title — mirrors the server-side + // rule so a broken value can't be stored. Empty = feature off (allowed). + const _plItemTpl = (document.getElementById('template-playlist-item')?.value || '').trim(); + if (_plItemTpl) { + if (_plItemTpl.includes('/') || _plItemTpl.includes('\\')) { + showToast('Playlist File Naming can\'t contain a folder separator ( / or \\ ) — it names the file, not a path.', 'error'); + return; + } + if (!_plItemTpl.includes('$title')) { + showToast('Playlist File Naming must include $title so every file has a name.', 'error'); + return; + } + } + const settings = { active_media_server: activeServer, spotify: { @@ -3105,6 +3148,7 @@ async function saveSettings(quiet = false) { album_path: document.getElementById('template-album-path').value, single_path: document.getElementById('template-single-path').value, playlist_path: document.getElementById('template-playlist-path').value, + playlist_item: document.getElementById('template-playlist-item').value, video_path: document.getElementById('template-video-path').value } }, @@ -3139,6 +3183,7 @@ async function saveSettings(quiet = false) { quality_filter_enabled: document.getElementById('import-quality-filter-enabled')?.checked !== false, replace_lower_quality: document.getElementById('import-replace-lower-quality').checked, folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true, + transfer_is_permanent: document.getElementById('import-transfer-permanent')?.checked === true, staging_path: document.getElementById('staging-path').value || './Staging' }, playlists: { @@ -3171,6 +3216,9 @@ async function saveSettings(quiet = false) { youtube: { cookies_browser: document.getElementById('youtube-cookies-browser').value, download_delay: parseInt(document.getElementById('youtube-download-delay').value) || 3, + // Raw cookies.txt blob — backend validates, writes it to a file, and stores + // only the path (never echoed back). Blank = keep any already-saved file. + cookies_paste: document.getElementById('youtube-cookies-paste')?.value || '', }, security: { require_pin_on_launch: document.getElementById('security-require-pin')?.checked || false, diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index 395ba528..922e6e0d 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -41,8 +41,8 @@ function _initImportFileTab() { function _importFileRead(file) { const ext = file.name.split('.').pop().toLowerCase(); - if (!['csv', 'tsv', 'txt'].includes(ext)) { - showToast('Unsupported file type. Use CSV, TSV, or TXT.', 'error'); + if (!['csv', 'tsv', 'txt', 'm3u', 'm3u8'].includes(ext)) { + showToast('Unsupported file type. Use CSV, TSV, TXT, M3U, or M3U8.', 'error'); return; } @@ -50,7 +50,8 @@ function _importFileRead(file) { reader.onload = (e) => { _importFileState.rawText = e.target.result; _importFileState.fileName = file.name; - _importFileState.fileType = (ext === 'txt') ? 'text' : 'csv'; + _importFileState.fileType = (ext === 'm3u' || ext === 'm3u8') ? 'm3u' + : (ext === 'txt') ? 'text' : 'csv'; _importFileParseAndPreview(); }; reader.readAsText(file); @@ -103,6 +104,66 @@ function _importFileParseCsv(text, delimiter) { return { headers, rows }; } +// Parse an M3U / M3U8 playlist into track objects. Handles both the simple form +// (one media path per line) and the extended form (#EXTINF:, - +// followed by the path). SoulSync's own export is extended M3U and replaces the path +// of an un-located track with a "# MISSING: ..." comment — those are exactly the +// tracks a user imports to go match/download, so a pending #EXTINF is flushed even +// when no path line follows it. Returns { tracks, playlistName }. +function _importFileParseM3u(text) { + const lines = (text || '').split(/\r?\n/); + const tracks = []; + let playlistName = ''; + let pending = null; // { duration_ms, artist, title } from the last #EXTINF + + function splitArtistTitle(s) { + const i = s.indexOf(' - '); + return i !== -1 + ? { artist: s.slice(0, i).trim(), title: s.slice(i + 3).trim() } + : { artist: '', title: s.trim() }; + } + function pushTrack(artist, title, duration_ms) { + if (!title && !artist) return; + tracks.push({ track_name: title || '', artist_name: artist || '', album_name: '', duration_ms: duration_ms || 0 }); + } + function flushPending() { + if (pending) { pushTrack(pending.artist, pending.title, pending.duration_ms); pending = null; } + } + + for (const raw of lines) { + const line = raw.trim(); + if (!line) continue; + if (line.charAt(0) === '#') { + if (/^#EXTINF:/i.test(line)) { + flushPending(); // a prior #EXTINF whose path was missing still counts + const rest = line.slice(8); // after "#EXTINF:" + const comma = rest.indexOf(','); + const secs = parseFloat(comma === -1 ? rest : rest.slice(0, comma)); + const meta = comma === -1 ? '' : rest.slice(comma + 1).trim(); + const { artist, title } = splitArtistTitle(meta); + pending = { duration_ms: (!isNaN(secs) && secs > 0) ? Math.round(secs * 1000) : 0, artist, title }; + } else if (/^#PLAYLIST:/i.test(line)) { + playlistName = line.slice(line.indexOf(':') + 1).trim(); + } + // ignore #EXTM3U, #GENERATED, "# MISSING:", #EXTALB, and other directives + continue; + } + // A non-# line is a media path/URL — the entry for the pending #EXTINF, if any. + if (pending && (pending.title || pending.artist)) { + pushTrack(pending.artist, pending.title, pending.duration_ms); + pending = null; + } else { + // Simple M3U with no #EXTINF: derive artist/title from the file name. + pending = null; + const base = (line.split(/[\\/]/).pop() || line).replace(/\.[^.]+$/, ''); + const { artist, title } = splitArtistTitle(base); + pushTrack(artist, title, 0); + } + } + flushPending(); // trailing #EXTINF with no following path + return { tracks, playlistName }; +} + function _importFileAutoMapColumns(headers) { const map = {}; const lowerHeaders = headers.map(h => h.toLowerCase().trim()); @@ -139,7 +200,14 @@ function _importFileParseAndPreview() { const state = _importFileState; const text = state.rawText; - if (state.fileType === 'text') { + if (state.fileType === 'm3u') { + // M3U/M3U8 is self-describing — no column mapping or order/separator needed. + const { tracks, playlistName } = _importFileParseM3u(text); + state.rows = tracks; // already track objects + state.headers = []; + state.columnMap = {}; + state.m3uPlaylistName = playlistName || ''; + } else if (state.fileType === 'text') { // Plain text: one track per line const lines = text.split(/\r?\n/).filter(l => l.trim()); state.rows = lines; @@ -163,7 +231,15 @@ function _importFileBuildTracks() { const state = _importFileState; state.parsedTracks = []; - if (state.fileType === 'text') { + if (state.fileType === 'm3u') { + // Tracks were already parsed by _importFileParseM3u; just copy them through. + state.parsedTracks = state.rows.map(t => ({ + track_name: t.track_name || '', + artist_name: t.artist_name || '', + album_name: t.album_name || '', + duration_ms: t.duration_ms || 0 + })); + } else if (state.fileType === 'text') { const orderEl = document.getElementById('import-file-text-order'); const sepEl = document.getElementById('import-file-text-separator'); const order = orderEl ? orderEl.value : 'artist-title'; @@ -249,10 +325,12 @@ function _importFileRenderPreview() { _importFileRenderColumnMapping(); } - // Pre-fill playlist name from filename (strip extension) + // Pre-fill playlist name: prefer the M3U's own #PLAYLIST: directive, else the filename. const nameInput = document.getElementById('import-file-playlist-name'); if (nameInput && !nameInput.value) { - nameInput.value = state.fileName.replace(/\.[^.]+$/, ''); + nameInput.value = (state.fileType === 'm3u' && state.m3uPlaylistName) + ? state.m3uPlaylistName + : state.fileName.replace(/\.[^.]+$/, ''); } // Update button state const btn = document.getElementById('import-file-import-btn'); @@ -523,6 +601,7 @@ function renderMirroredCard(p, container) { <button class="mirrored-card-pipeline" onclick="event.stopPropagation(); runMirroredPlaylistPipeline(${p.id}, '${_escJs(p.name)}')" title="Refresh, discover, sync, and queue missing tracks">Auto-Sync</button> <button class="mirrored-card-rename" onclick="event.stopPropagation(); editMirroredCustomName(${p.id}, '${_escJs(p.name)}', '${_escJs(p.custom_name || '')}')" title="Rename (changes the name shown here and used when syncing)">✏️</button> <button class="mirrored-card-link" onclick="event.stopPropagation(); editMirroredSourceRef(${p.id}, '${_escJs(p.name)}', '${_escJs(p.source)}', '${_escJs(sourceRef)}')" title="Edit original playlist link">🔗</button> + <button class="mirrored-card-export" onclick="event.stopPropagation(); exportMirroredPlaylist(${p.id}, '${_escJs(p.display_name || p.name)}')" title="Export to ListenBrainz / JSPF">📤</button> <button class="mirrored-card-delete" onclick="event.stopPropagation(); deleteMirroredPlaylist(${p.id}, '${_escJs(p.name)}')" title="Delete mirror">✕</button> `; card.addEventListener('click', () => { @@ -566,6 +645,110 @@ function renderMirroredCard(p, container) { } } +// ── Playlist export to ListenBrainz / JSPF (#903) ─────────────────────────── +// Export button on the mirrored-playlist card -> pick a destination -> background job +// resolves each track to a MusicBrainz recording MBID (cache/DB/file/MusicBrainz) and either +// downloads the .jspf or pushes the playlist straight to ListenBrainz, with live status on the card. +function exportMirroredPlaylist(playlistId, name) { + const existing = document.getElementById('pl-export-modal'); + if (existing) existing.remove(); + const overlay = document.createElement('div'); + overlay.id = 'pl-export-modal'; + overlay.style.cssText = 'position:fixed;inset:0;z-index:10000;display:flex;align-items:center;justify-content:center;background:rgba(0,0,0,0.6);backdrop-filter:blur(4px);'; + overlay.innerHTML = ` + <div style="width:420px;max-width:92vw;background:linear-gradient(135deg,rgba(26,26,30,0.98),rgba(18,18,22,0.99));border:1px solid rgba(var(--accent-rgb),0.25);border-radius:18px;padding:22px;box-shadow:0 18px 50px rgba(0,0,0,0.55);"> + <div style="font-size:16px;font-weight:700;color:#fff;margin-bottom:4px;">Export playlist</div> + <div style="font-size:13px;color:rgba(255,255,255,0.55);margin-bottom:18px;">${_esc(name)} → ListenBrainz</div> + <button class="pl-export-choice" data-mode="push" style="width:100%;text-align:left;margin-bottom:10px;padding:13px 15px;border-radius:12px;border:1px solid rgba(var(--accent-rgb),0.35);background:rgba(var(--accent-rgb),0.12);color:#fff;cursor:pointer;"> + <div style="font-weight:600;">Sync to ListenBrainz</div> + <div style="font-size:12px;color:rgba(255,255,255,0.55);">Create the playlist directly on your ListenBrainz account (needs your LB token).</div> + </button> + <button class="pl-export-choice" data-mode="download" style="width:100%;text-align:left;margin-bottom:16px;padding:13px 15px;border-radius:12px;border:1px solid rgba(255,255,255,0.1);background:rgba(255,255,255,0.04);color:#fff;cursor:pointer;"> + <div style="font-weight:600;">Download .jspf file</div> + <div style="font-size:12px;color:rgba(255,255,255,0.55);">Save a JSPF playlist you can upload to ListenBrainz manually.</div> + </button> + <div style="font-size:11.5px;color:rgba(255,255,255,0.4);line-height:1.5;">Each track is matched to its MusicBrainz recording ID (cached → library → file tag → MusicBrainz). Tracks without an ID can't be included — you'll see how many matched.</div> + <div style="text-align:right;margin-top:14px;"><button onclick="document.getElementById('pl-export-modal').remove()" style="background:none;border:none;color:rgba(255,255,255,0.5);cursor:pointer;font-size:13px;">Cancel</button></div> + </div>`; + overlay.addEventListener('click', (e) => { if (e.target === overlay) overlay.remove(); }); + overlay.querySelectorAll('.pl-export-choice').forEach(btn => { + btn.addEventListener('click', () => { + const mode = btn.dataset.mode; + overlay.remove(); + _startPlaylistExport(playlistId, mode, name); + }); + }); + document.body.appendChild(overlay); +} + +async function _startPlaylistExport(playlistId, mode, name) { + _setExportStatus(playlistId, `<span style="color:#a78bfa;">Starting export…</span>`); + try { + const resp = await fetch(`/api/playlists/${playlistId}/export/listenbrainz`, { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ mode }), + }); + const data = await resp.json(); + if (!data.success || !data.job_id) { + _setExportStatus(playlistId, `<span style="color:#ef4444;">Export failed to start</span>`); + return; + } + _pollPlaylistExport(data.job_id, playlistId, mode, name); + } catch (e) { + _setExportStatus(playlistId, `<span style="color:#ef4444;">Export error</span>`); + } +} + +async function _pollPlaylistExport(jobId, playlistId, mode, name) { + try { + const resp = await fetch(`/api/playlists/export/status/${jobId}`); + const data = await resp.json(); + const job = data.job || {}; + const st = job.stats || {}; + if (job.phase === 'resolving') { + const pct = job.total ? Math.round(100 * (job.done || 0) / job.total) : 0; + _setExportStatus(playlistId, `<span style="color:#38bdf8;">Matching ${job.done || 0}/${job.total || 0} (${pct}%)${st.resolved != null ? ` · ${st.resolved} matched` : ''}</span>`); + } else if (job.phase === 'pushing') { + _setExportStatus(playlistId, `<span style="color:#a78bfa;">Pushing to ListenBrainz…</span>`); + } else if (job.phase === 'done') { + const sum = job.summary || {}; + const cov = `${sum.included || 0}/${sum.total || 0} matched${sum.skipped ? ` · ${sum.skipped} unmatched` : ''}`; + if (mode === 'download') { + window.location = `/api/playlists/export/download/${jobId}`; + _setExportStatus(playlistId, `<span style="color:#22c55e;">Downloaded · ${cov}</span>`, 8000); + } else { + const url = (job.push && job.push.playlist_url) || ''; + _setExportStatus(playlistId, `<span style="color:#22c55e;">Synced to ListenBrainz · ${cov}</span>` + (url ? ` <a href="${url}" target="_blank" style="color:#38bdf8;">view</a>` : ''), 12000); + if (typeof showToast === 'function') showToast(`Playlist synced to ListenBrainz (${cov})`, 'success'); + } + return; + } else if (job.phase === 'error') { + _setExportStatus(playlistId, `<span style="color:#ef4444;">${_esc(job.error || 'Export failed')}</span>`, 10000); + return; + } + setTimeout(() => _pollPlaylistExport(jobId, playlistId, mode, name), 1000); + } catch (e) { + setTimeout(() => _pollPlaylistExport(jobId, playlistId, mode, name), 2000); + } +} + +function _setExportStatus(playlistId, html, autoHideMs) { + // Inject into the card's existing .card-meta line (same approach as the pipeline phase + // indicator) so the status sits inline and never disrupts the card's flex row. + const card = document.getElementById(`mirrored-card-${playlistId}`); + if (!card) return; + const meta = card.querySelector('.card-meta'); + if (!meta) return; + let span = meta.querySelector('.export-status-span'); + if (!span) { + span = document.createElement('span'); + span.className = 'export-status-span'; + meta.appendChild(span); + } + span.innerHTML = html; + if (autoHideMs) setTimeout(() => { if (span && span.parentNode) span.remove(); }, autoHideMs); +} + function updateMirroredCardPhase(urlHash, phase) { // Update the state phase (updateYouTubeCardPhase skips this for mirrored playlists due to no cardElement) const state = youtubePlaylistStates[urlHash]; diff --git a/webui/static/style.css b/webui/static/style.css index 93342888..d176e84d 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -82,11 +82,11 @@ body { border-top: 1px solid rgba(255, 255, 255, 0.12); border-bottom-right-radius: 24px; - /* Soft floating shadow with inner glow */ + /* Soft floating shadow with inner glow — dropped the 0 4px 16px duplicate and the + 0 0 60px accent glow (60px blur at 6% opacity: nearly invisible but a costly + shadow pass). One outer + the two insets read the same. */ box-shadow: - 0 8px 32px rgba(0, 0, 0, 0.3), - 0 4px 16px rgba(0, 0, 0, 0.2), - 0 0 60px rgba(var(--accent-rgb), 0.06), + 0 8px 30px rgba(0, 0, 0, 0.34), inset 0 1px 0 rgba(255, 255, 255, 0.1), inset 0 -1px 0 rgba(0, 0, 0, 0.2); @@ -108,7 +108,10 @@ body { position: fixed; pointer-events: none; border-radius: 50%; - filter: blur(40px); + /* The radial-gradient already fades to transparent at 70%, so a smaller blur + still reads as a soft aura — but shrinks the composited (orb + feather) + bounding box and the blur work. 40px -> 28px: ~imperceptible, cheaper fill. */ + filter: blur(28px); will-change: transform, opacity; } @@ -307,11 +310,9 @@ body.reduce-effects .sidebar::after { gap: 8px; position: sticky; top: 0; - /* Above the nav (the sidebar gives its children z-index:1) so nav items - scrolling up sit BEHIND the header — i.e. in its backdrop, where the - blur below can actually act on them. Without this they paint in front - and backdrop-filter has nothing to blur. */ - z-index: 2; + /* Same stacking level as the nav (z-index:1) — sits in the normal sidebar + flow rather than forced above everything. */ + z-index: 1; overflow: hidden; flex-shrink: 0; @@ -319,8 +320,11 @@ body.reduce-effects .sidebar::after { accent-gradient top now read as a soft blur instead of bleeding through sharply. (Only visible where the background is translucent — the opaque base toward the bottom still stops any bleed.) */ - backdrop-filter: blur(28px) saturate(1.3); - -webkit-backdrop-filter: blur(28px) saturate(1.3); + /* Backdrop blur is re-evaluated whenever content drifts behind it (the + ambient orbs do). Cost scales with radius — 28px -> 18px keeps the frosted + read while cutting the per-frame blur work by ~a third. */ + backdrop-filter: blur(18px) saturate(1.3); + -webkit-backdrop-filter: blur(18px) saturate(1.3); /* Subtle inner glow */ box-shadow: inset 0 1px 0 rgba(255, 255, 255, 0.1); @@ -342,14 +346,20 @@ body.reduce-effects .sidebar::after { rgba(var(--accent-rgb), 0.06) 60%, transparent 100% ); + /* Animate transform (compositor-only) instead of `left` (which forced a + layout recalc every frame). The element width is 60% of the header, so + translateX(400%) = 240% of header width — the same travel `left:-100%` + → `left:140%` produced. Pixel-identical path, no per-frame layout. */ + transform: translateX(0); + will-change: transform, opacity; animation: sidebar-header-sweep 8s ease-in-out infinite; pointer-events: none; } @keyframes sidebar-header-sweep { - 0%, 100% { left: -100%; opacity: 0; } + 0%, 100% { transform: translateX(0); opacity: 0; } 10% { opacity: 1; } - 50% { left: 140%; opacity: 1; } + 50% { transform: translateX(400%); opacity: 1; } 60% { opacity: 0; } } @@ -8390,10 +8400,11 @@ body.helper-mode-active #dashboard-activity-feed:hover { border: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); margin: 20px; - /* Soft floating shadow */ + /* Soft floating shadow — one outer layer instead of two stacked (this is a + near-fullscreen element, so each shadow pass is expensive); the single + slightly-stronger blur reads the same. */ box-shadow: - 0 8px 32px rgba(0, 0, 0, 0.3), - 0 4px 16px rgba(0, 0, 0, 0.2), + 0 8px 28px rgba(0, 0, 0, 0.34), inset 0 1px 0 rgba(255, 255, 255, 0.08); } @@ -9049,8 +9060,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { } .service-card { - background: rgba(18, 18, 22, 0.9); - backdrop-filter: blur(12px); + /* backdrop-filter dropped: the 90% opaque bg made blur(12px) invisible (x3 cards, + pure GPU waste). Bumped to 0.97 so the now-unblurred sliver is imperceptible. */ + background: rgba(18, 18, 22, 0.97); border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.06); padding: 20px 22px; @@ -9821,8 +9833,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { } .stat-card-dashboard { - background: rgba(18, 18, 22, 0.9); - backdrop-filter: blur(12px); + /* backdrop-filter dropped: 90% opaque bg made blur(12px) invisible (x3, GPU waste); + 0.97 keeps it looking solid. */ + background: rgba(18, 18, 22, 0.97); border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.06); padding: 20px; @@ -10228,9 +10241,9 @@ body.helper-mode-active #dashboard-activity-feed:hover { .activity-feed-container { /* Premium glassmorphic foundation */ background: linear-gradient(135deg, - rgba(26, 26, 26, 0.95) 0%, - rgba(18, 18, 18, 0.98) 100%); - backdrop-filter: blur(12px) saturate(1.1); + rgba(26, 26, 26, 0.97) 0%, + rgba(18, 18, 18, 0.99) 100%); + /* backdrop-filter dropped: 95-99% opaque bg already hid the blur (GPU waste). */ border-radius: 16px; border: 1px solid rgba(255, 255, 255, 0.08); border-top: 1px solid rgba(255, 255, 255, 0.12); @@ -14672,6 +14685,7 @@ body.helper-mode-active #dashboard-activity-feed:hover { .mirrored-card-delete, .mirrored-card-clear, .mirrored-card-link, +.mirrored-card-export, .mirrored-card-rename { background: rgba(255, 255, 255, 0.06); border: 1px solid rgba(255, 255, 255, 0.08); @@ -14693,11 +14707,25 @@ body.helper-mode-active #dashboard-activity-feed:hover { .mirrored-playlist-card:hover .mirrored-card-delete, .mirrored-playlist-card:hover .mirrored-card-clear, .mirrored-playlist-card:hover .mirrored-card-link, +.mirrored-playlist-card:hover .mirrored-card-export, .mirrored-playlist-card:hover .mirrored-card-rename, .mirrored-playlist-card:hover .mirrored-card-pipeline { opacity: 1; } +.mirrored-card-export:hover { + color: rgba(var(--accent-rgb), 1); + background: rgba(var(--accent-rgb), 0.15); + border-color: rgba(var(--accent-rgb), 0.3); + transform: scale(1.1); +} + +/* Live export status, injected inline into the card's .card-meta line. */ +.export-status-span { + font-size: inherit; + white-space: nowrap; +} + .mirrored-card-delete:hover { color: #ff4444; background: rgba(255, 68, 68, 0.15);