Auto-import: SoulSync standalone library writes server-quality rows
# Background
SoulSync standalone is meant to be a full replacement for Plex /
Jellyfin / Navidrome — files imported via auto-import (or any other
import path) should land in the database with the same field richness
a media-server scan would write. They weren't.
# Gaps fixed
The auto-import worker built a context dict for each track and handed
it to `_post_process_matched_download` (the same callback the regular
download flow uses). That dict was missing three things downstream
needed:
1. **No `source` field anywhere.** `record_soulsync_library_entry`
reads `get_import_source(context)` to pick the source-aware ID
columns (`spotify_track_id` / `deezer_id` / `itunes_track_id` /
etc.) on the artists / albums / tracks rows. With no source, the
resolver returned an empty string → `get_library_source_id_columns("")`
returned an empty dict → the `UPDATE tracks SET <source>_id = ?`
blocks were silently skipped. Result: every auto-imported track
landed with NULL on every source-id column. Watchlist scans
(which match by stable source IDs to detect "this track is already
in library") couldn't recognise these rows and would re-download
them on the next pass.
2. **No `_download_username='auto_import'`.** Both
`record_library_history_download` and `record_download_provenance`
default to "Soulseek" when no `username` is in the context. Every
staging-folder import was being labelled as a Soulseek download
in library history + provenance — false signal in the UI.
3. **No per-recording IDs (`isrc`, `musicbrainz_recording_id`) on
track_info.** The Navidrome scanner already writes
`musicbrainz_recording_id` directly to the tracks row when present.
Picard-tagged libraries always carry MBID; metadata sources
(Spotify via MusicBrainz enrichment, Deezer, etc.) carry ISRC.
Auto-import had access to both via the metadata-source response
but didn't propagate them — so the soulsync row went in with
NULL on both columns.
# Changes
**`core/auto_import_worker.py` — `_process_matches`:**
- Top-level `'source': source` (from `identification['source']`)
- `'_download_username': 'auto_import'`
- `track_info['isrc']`, `track_info['musicbrainz_recording_id']` —
pulled from the per-track payload returned by the metadata source
- `track_info['album_id']` — back-reference so source-aware ID
resolution works on sources whose API nests album under
`track.album.id` rather than `track.album_id`
- `spotify_artist['id']` now correctly carries the artist's source ID
(was `identification['album_id']`, a copy-paste bug from the
original implementation that made artist-id resolution fall back
to fuzzy matching)
- `spotify_album['artists'][0]['id']` carries artist source ID for
the same resolution path
**`core/imports/side_effects.py`:**
- `record_library_history_download` source_map: add
`"auto_import": "Auto-Import"` — tags imported tracks correctly
- `record_download_provenance` source_service: add
`"auto_import": "auto_import"` — provenance shows real source
- `record_soulsync_library_entry` track INSERT: now includes
`musicbrainz_recording_id` + `isrc` columns (matches
`insert_or_update_media_track`'s shape for Navidrome /
Plex / Jellyfin scans). Both default to NULL when not present.
# Behavior preserved
- Files still land in the same library template path (no path-build
change)
- Other media-server flows (Plex / Jellyfin / Navidrome users)
unaffected — `record_soulsync_library_entry` still gates on
`get_active_media_server() == "soulsync"`. Auto-import on those
servers continues to drop the file in the library folder + emits
`batch_complete` for the scan-trigger automation, same as before.
- Direct downloads (search → Download button) unaffected — they
already passed `source` + `username` correctly.
# Tests added
`tests/imports/test_auto_import_context_shape.py` (8 tests, new file):
- Worker context carries `source` for every metadata source
(parametrised across spotify / deezer / itunes / discogs)
- `_download_username='auto_import'` set unconditionally
- ISRC + MBID propagate from track payload to track_info when present
- ISRC + MBID default to empty string when absent (downstream
normalises to NULL at write time)
- track_info includes album-id back-reference
`tests/imports/test_import_side_effects.py` (4 new tests + 2 schema
column adds):
- `record_soulsync_library_entry` writes mbid + isrc columns when
present in track_info
- Deezer source maps to deezer_id column (regression case for
source-aware column resolver)
- `record_library_history_download` labels `_download_username=
'auto_import'` as "Auto-Import" not "Soulseek"
- `record_download_provenance` registers source_service as
"auto_import" not "soulseek"
# Verification
- 8/8 new context-shape tests pass
- 6/6 side-effects tests pass (4 new + 2 existing)
- 207 imports tests pass
- 2359 full suite passes (+12 from baseline 2347, no regressions)
- 1 pre-existing flake (`test_watchdog_warns_about_stuck_workers`,
passes in isolation, unrelated to this change)
- Ruff clean
This commit is contained in:
parent
eb68873ec9
commit
8493be207e
5 changed files with 523 additions and 7 deletions
|
|
@ -1526,23 +1526,61 @@ class AutoImportWorker:
|
|||
continue
|
||||
|
||||
try:
|
||||
# Build context matching the manual import format
|
||||
# Build context matching the manual import format.
|
||||
#
|
||||
# The post-process pipeline (`_post_process_matched_download`
|
||||
# → `record_soulsync_library_entry`) reads `source` to pick
|
||||
# the right source-id columns on artists/albums/tracks,
|
||||
# and reads `_download_username` to label the row in
|
||||
# library history + provenance. Without these the SoulSync
|
||||
# standalone library lands the file but leaves
|
||||
# `spotify_track_id` / `deezer_id` / etc. NULL and tags the
|
||||
# provenance row as "Soulseek" (the default fallback).
|
||||
# SoulSync standalone is a full server replacement, so the
|
||||
# row must carry the same field richness as a Plex/Jellyfin/
|
||||
# Navidrome scan would write.
|
||||
context_key = f"auto_import_{candidate.folder_hash}_{track_number}"
|
||||
# Album-level identifiers from the metadata source response.
|
||||
# `album_data['id']` is the source-native album id (e.g.
|
||||
# spotify album id, deezer album id). Identification fed it
|
||||
# into `identification['album_id']` already; prefer the
|
||||
# album_data version since it's authoritative when both
|
||||
# are present.
|
||||
source_album_id = album_data.get('id') or identification.get('album_id') or ''
|
||||
# ISRC + MusicBrainz Recording ID — propagated by the
|
||||
# metadata layer (`_build_album_track_entry`) so files
|
||||
# tagged with these IDs can match later watchlist scans
|
||||
# without relying on fuzzy title comparison.
|
||||
track_isrc = track.get('isrc', '') or ''
|
||||
track_mbid = track.get('musicbrainz_recording_id', '') or track.get('mbid', '') or ''
|
||||
context = {
|
||||
# Top-level `source` is the canonical signal that the
|
||||
# imports pipeline reads via `get_import_source()`.
|
||||
# `get_library_source_id_columns(source)` then picks
|
||||
# the right column on artists/albums/tracks for the
|
||||
# source-aware UPDATE.
|
||||
'source': source,
|
||||
# `_download_username` is read by
|
||||
# `record_library_history_download` +
|
||||
# `record_download_provenance` to label the row.
|
||||
# 'auto_import' maps to "Auto-Import" / "auto_import"
|
||||
# in those source maps so the UI doesn't show every
|
||||
# imported file as "Soulseek".
|
||||
'_download_username': 'auto_import',
|
||||
'spotify_artist': {
|
||||
'id': identification.get('album_id') or 'auto_import',
|
||||
'id': identification.get('artist_id') or '',
|
||||
'name': artist_name,
|
||||
'genres': [],
|
||||
},
|
||||
'spotify_album': {
|
||||
'id': album_data.get('id') or identification.get('album_id') or '',
|
||||
'id': source_album_id,
|
||||
'name': album_name,
|
||||
'release_date': release_date,
|
||||
'total_tracks': album_data.get('total_tracks', match_result.get('total_tracks', 0)),
|
||||
'total_discs': total_discs,
|
||||
'image_url': image_url,
|
||||
'images': album_data.get('images', [{'url': image_url}] if image_url else []),
|
||||
'artists': [{'name': artist_name}],
|
||||
'artists': [{'name': artist_name, 'id': identification.get('artist_id') or ''}],
|
||||
'album_type': album_data.get('album_type', 'album'),
|
||||
},
|
||||
'track_info': {
|
||||
|
|
@ -1553,6 +1591,14 @@ class AutoImportWorker:
|
|||
'duration_ms': track.get('duration_ms', 0),
|
||||
'artists': track.get('artists', [{'name': artist_name}]),
|
||||
'uri': track.get('uri', ''),
|
||||
# Album-id back-reference + per-recording IDs so
|
||||
# `get_import_source_ids` can resolve them onto
|
||||
# the right column even when the source's API
|
||||
# nests them under `album.id` rather than
|
||||
# `track.album_id`.
|
||||
'album_id': source_album_id,
|
||||
'isrc': track_isrc,
|
||||
'musicbrainz_recording_id': track_mbid,
|
||||
},
|
||||
'original_search_result': {
|
||||
'title': track_name,
|
||||
|
|
|
|||
|
|
@ -89,6 +89,12 @@ def record_library_history_download(context: Dict[str, Any]) -> None:
|
|||
"deezer_dl": "Deezer",
|
||||
"lidarr": "Lidarr",
|
||||
"soundcloud": "SoundCloud",
|
||||
# Auto-import isn't a download source, but flows through the
|
||||
# same post-process pipeline (file lands → record provenance
|
||||
# + history → write to library DB). Tagging it as "Auto-Import"
|
||||
# in history avoids mislabeling staging-folder imports as
|
||||
# Soulseek downloads.
|
||||
"auto_import": "Auto-Import",
|
||||
}
|
||||
download_source = source_map.get(username, "Soulseek")
|
||||
|
||||
|
|
@ -161,6 +167,13 @@ def record_download_provenance(context: Dict[str, Any]) -> None:
|
|||
"deezer_dl": "deezer",
|
||||
"lidarr": "lidarr",
|
||||
"soundcloud": "soundcloud",
|
||||
# Auto-import: surfaced in provenance so the redownload modal
|
||||
# can tell the user "this came from staging on <date>" instead
|
||||
# of falsely listing soulseek as the source. The underlying
|
||||
# metadata source (spotify / deezer / itunes) is recorded
|
||||
# separately via the source-aware ID columns on the tracks
|
||||
# row itself.
|
||||
"auto_import": "auto_import",
|
||||
}.get(username, "soulseek")
|
||||
|
||||
ti = context.get("track_info") or context.get("search_result") or {}
|
||||
|
|
@ -416,14 +429,28 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
|
|||
if ta_name and ta_name.lower() != artist_name.lower():
|
||||
track_artist = ta_name
|
||||
|
||||
# Per-recording identifiers — scanner picks `musicbrainz_recording_id`
|
||||
# off the Navidrome track wrapper; auto-import has the same field
|
||||
# available from the metadata-source response (Spotify exposes
|
||||
# `musicbrainz_recording_id` via the MusicBrainz client, Picard-
|
||||
# tagged files surface it via `_read_file_tags`). `isrc` is even
|
||||
# better signal for cross-source dedup — it's the per-recording
|
||||
# ID labels embed in the audio. Both land in dedicated columns
|
||||
# so the watchlist scanner's stable-ID match path recognises
|
||||
# auto-imported tracks the next time the user adds the artist
|
||||
# to a watchlist.
|
||||
track_mbid = (track_info.get("musicbrainz_recording_id") or "").strip().lower() or None
|
||||
track_isrc = (track_info.get("isrc") or "").strip().upper() or None
|
||||
|
||||
cursor.execute("SELECT id FROM tracks WHERE file_path = ?", (final_path,))
|
||||
if not cursor.fetchone():
|
||||
cursor.execute(
|
||||
"""
|
||||
INSERT INTO tracks (id, album_id, artist_id, title, track_number,
|
||||
duration, file_path, bitrate, file_size, track_artist, server_source,
|
||||
duration, file_path, bitrate, file_size, track_artist,
|
||||
musicbrainz_recording_id, isrc, server_source,
|
||||
created_at, updated_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'soulsync', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
|
||||
""",
|
||||
(
|
||||
track_id,
|
||||
|
|
@ -436,6 +463,8 @@ def record_soulsync_library_entry(context: Dict[str, Any], artist_context: Dict[
|
|||
bitrate,
|
||||
file_size,
|
||||
track_artist,
|
||||
track_mbid,
|
||||
track_isrc,
|
||||
),
|
||||
)
|
||||
track_source_col = source_columns.get("track")
|
||||
|
|
|
|||
255
tests/imports/test_auto_import_context_shape.py
Normal file
255
tests/imports/test_auto_import_context_shape.py
Normal file
|
|
@ -0,0 +1,255 @@
|
|||
"""Pin the post-process context dict the auto-import worker hands to
|
||||
``_post_process_matched_download``.
|
||||
|
||||
Background
|
||||
----------
|
||||
|
||||
Auto-import doesn't write to the SoulSync standalone DB itself —
|
||||
it routes every matched track through the same
|
||||
``_post_process_matched_download`` callback the regular download
|
||||
flow uses. The pipeline downstream (``record_soulsync_library_entry``,
|
||||
``record_library_history_download``, ``record_download_provenance``)
|
||||
reads:
|
||||
|
||||
- ``context["source"]`` — picks the right source-id columns
|
||||
(``spotify_track_id`` / ``deezer_id`` / ``itunes_track_id`` / etc.)
|
||||
- ``context["_download_username"]`` — labels the row in library
|
||||
history + provenance ("Auto-Import" instead of falling back to the
|
||||
Soulseek default).
|
||||
- ``context["track_info"]["musicbrainz_recording_id"]`` and
|
||||
``context["track_info"]["isrc"]`` — per-recording IDs that land on
|
||||
the dedicated ``musicbrainz_recording_id`` / ``isrc`` track columns.
|
||||
|
||||
If the worker drops any of these, the soulsync library row gets
|
||||
written but with NULL on every source-id column, and library history
|
||||
mislabels every imported file as a Soulseek download. SoulSync
|
||||
standalone is meant to be a full server replacement so it must reach
|
||||
parity with what a Plex / Jellyfin / Navidrome scan would write. These
|
||||
tests pin that contract at the worker boundary.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any, Dict, List
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@dataclass
|
||||
class _FakeCandidate:
|
||||
path: str
|
||||
name: str
|
||||
audio_files: List[str] = field(default_factory=list)
|
||||
disc_structure: Dict[int, List[str]] = field(default_factory=dict)
|
||||
folder_hash: str = "fake-hash"
|
||||
is_single: bool = False
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def worker_with_capture(tmp_path):
|
||||
"""Worker whose ``process_callback`` captures the per-track context
|
||||
dict so the test can assert on its shape directly."""
|
||||
from core.auto_import_worker import AutoImportWorker
|
||||
|
||||
captured: List[Dict[str, Any]] = []
|
||||
fake_db = MagicMock()
|
||||
fake_cfg = MagicMock()
|
||||
fake_cfg.get.side_effect = lambda key, default=None: default
|
||||
|
||||
def _capture(_key, ctx, _path):
|
||||
captured.append(ctx)
|
||||
|
||||
worker = AutoImportWorker(
|
||||
database=fake_db,
|
||||
staging_path=str(tmp_path),
|
||||
transfer_path=str(tmp_path / "transfer"),
|
||||
process_callback=_capture,
|
||||
config_manager=fake_cfg,
|
||||
automation_engine=None,
|
||||
)
|
||||
worker._captured = captured
|
||||
return worker
|
||||
|
||||
|
||||
def _make_match_result(source: str, track_count: int = 1) -> Dict[str, Any]:
|
||||
return {
|
||||
"matches": [], # filled by tests
|
||||
"unmatched_files": [],
|
||||
"total_tracks": track_count,
|
||||
"matched_count": track_count,
|
||||
"confidence": 0.95,
|
||||
"album_data": {
|
||||
"id": "ALBUM-ID-FROM-SOURCE",
|
||||
"total_tracks": track_count,
|
||||
"album_type": "album",
|
||||
"release_date": "2024-01-01",
|
||||
"images": [{"url": "https://img.example/cover.jpg"}],
|
||||
"artists": [{"name": "A", "id": "ARTIST-ID-FROM-SOURCE"}],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _make_identification(source: str = "deezer") -> Dict[str, Any]:
|
||||
return {
|
||||
"source": source,
|
||||
"artist_name": "A",
|
||||
"artist_id": "ARTIST-ID-FROM-SOURCE",
|
||||
"album_name": "Album",
|
||||
"album_id": "ALBUM-ID-FROM-SOURCE",
|
||||
"image_url": "https://img.example/cover.jpg",
|
||||
"release_date": "2024-01-01",
|
||||
"method": "tags",
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# context["source"] propagation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize("source", ["spotify", "deezer", "itunes", "discogs"])
|
||||
def test_context_carries_source(worker_with_capture, tmp_path, source):
|
||||
"""Worker must propagate ``identification['source']`` onto the
|
||||
top-level context. Without it, ``record_soulsync_library_entry``
|
||||
can't pick a source column and writes the row with NULL on every
|
||||
source-id field."""
|
||||
f = tmp_path / "01.flac"
|
||||
f.write_bytes(b"audio")
|
||||
cand = _FakeCandidate(path=str(tmp_path), name="Album")
|
||||
ident = _make_identification(source=source)
|
||||
mr = _make_match_result(source, 1)
|
||||
mr["matches"] = [{
|
||||
"track": {"id": "t1", "name": "Track", "track_number": 1,
|
||||
"disc_number": 1, "duration_ms": 200000,
|
||||
"artists": [{"name": "A"}]},
|
||||
"file": str(f), "confidence": 0.95,
|
||||
}]
|
||||
|
||||
worker_with_capture._process_matches(cand, ident, mr)
|
||||
|
||||
ctx = worker_with_capture._captured[0]
|
||||
assert ctx["source"] == source, (
|
||||
f"Expected context['source']={source!r}, got {ctx.get('source')!r}. "
|
||||
f"Without this, soulsync library writes the row with NULL on "
|
||||
f"{source}_track_id."
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Auto-import labels: history + provenance must NOT default to Soulseek
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_context_marks_download_username_as_auto_import(worker_with_capture, tmp_path):
|
||||
"""``_download_username='auto_import'`` is what triggers the
|
||||
"Auto-Import" / "auto_import" branch in side_effects.py source maps.
|
||||
Without it, every imported file is labelled as a Soulseek download
|
||||
in library history + provenance — false signal in the UI."""
|
||||
f = tmp_path / "01.flac"
|
||||
f.write_bytes(b"audio")
|
||||
cand = _FakeCandidate(path=str(tmp_path), name="Album")
|
||||
ident = _make_identification("spotify")
|
||||
mr = _make_match_result("spotify", 1)
|
||||
mr["matches"] = [{
|
||||
"track": {"id": "t1", "name": "Track", "track_number": 1,
|
||||
"disc_number": 1, "duration_ms": 200000,
|
||||
"artists": [{"name": "A"}]},
|
||||
"file": str(f), "confidence": 0.95,
|
||||
}]
|
||||
|
||||
worker_with_capture._process_matches(cand, ident, mr)
|
||||
|
||||
ctx = worker_with_capture._captured[0]
|
||||
assert ctx.get("_download_username") == "auto_import"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Per-recording IDs flow through to track_info
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_context_propagates_isrc_and_mbid_when_present(worker_with_capture, tmp_path):
|
||||
"""When the metadata-source response carries per-recording IDs
|
||||
(Picard-tagged libraries always have MBID, MusicBrainz-enriched
|
||||
Spotify carries ISRC), they must end up on
|
||||
context['track_info']['isrc'] / ['musicbrainz_recording_id'] so
|
||||
the soulsync library row writes them onto dedicated columns."""
|
||||
f = tmp_path / "01.flac"
|
||||
f.write_bytes(b"audio")
|
||||
cand = _FakeCandidate(path=str(tmp_path), name="Album")
|
||||
ident = _make_identification("spotify")
|
||||
mr = _make_match_result("spotify", 1)
|
||||
mr["matches"] = [{
|
||||
"track": {
|
||||
"id": "spotify-track-id",
|
||||
"name": "Track",
|
||||
"track_number": 1,
|
||||
"disc_number": 1,
|
||||
"duration_ms": 200000,
|
||||
"artists": [{"name": "A"}],
|
||||
"isrc": "USABC1234567",
|
||||
"musicbrainz_recording_id": "abcd1234-mbid-uuid-form",
|
||||
},
|
||||
"file": str(f), "confidence": 0.95,
|
||||
}]
|
||||
|
||||
worker_with_capture._process_matches(cand, ident, mr)
|
||||
|
||||
ti = worker_with_capture._captured[0]["track_info"]
|
||||
assert ti["isrc"] == "USABC1234567"
|
||||
assert ti["musicbrainz_recording_id"] == "abcd1234-mbid-uuid-form"
|
||||
|
||||
|
||||
def test_context_per_recording_ids_default_empty_when_missing(worker_with_capture, tmp_path):
|
||||
"""Missing IDs default to empty string, NOT to None — the side-
|
||||
effects layer normalises to None at write time. Empty string keeps
|
||||
the field present in the dict so downstream code that does
|
||||
`track_info.get("isrc")` doesn't have to special-case missing keys."""
|
||||
f = tmp_path / "01.flac"
|
||||
f.write_bytes(b"audio")
|
||||
cand = _FakeCandidate(path=str(tmp_path), name="Album")
|
||||
ident = _make_identification("deezer")
|
||||
mr = _make_match_result("deezer", 1)
|
||||
mr["matches"] = [{
|
||||
"track": {"id": "111", "name": "Track", "track_number": 1,
|
||||
"disc_number": 1, "duration_ms": 200000,
|
||||
"artists": [{"name": "A"}]}, # no isrc / mbid
|
||||
"file": str(f), "confidence": 0.95,
|
||||
}]
|
||||
|
||||
worker_with_capture._process_matches(cand, ident, mr)
|
||||
|
||||
ti = worker_with_capture._captured[0]["track_info"]
|
||||
assert ti.get("isrc") == ""
|
||||
assert ti.get("musicbrainz_recording_id") == ""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Album back-reference on track_info
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_track_info_includes_album_id_back_reference(worker_with_capture, tmp_path):
|
||||
"""`get_import_source_ids` reads `track_info.album_id` as one of the
|
||||
fallback paths for resolving the album-source-id. Without the back
|
||||
reference, sources whose API response nests album under
|
||||
`track.album.id` fall through and the soulsync row writes NULL on
|
||||
the album-source-id column."""
|
||||
f = tmp_path / "01.flac"
|
||||
f.write_bytes(b"audio")
|
||||
cand = _FakeCandidate(path=str(tmp_path), name="Album")
|
||||
ident = _make_identification("spotify")
|
||||
mr = _make_match_result("spotify", 1)
|
||||
mr["matches"] = [{
|
||||
"track": {"id": "t1", "name": "Track", "track_number": 1,
|
||||
"disc_number": 1, "duration_ms": 200000,
|
||||
"artists": [{"name": "A"}]},
|
||||
"file": str(f), "confidence": 0.95,
|
||||
}]
|
||||
|
||||
worker_with_capture._process_matches(cand, ident, mr)
|
||||
|
||||
ti = worker_with_capture._captured[0]["track_info"]
|
||||
assert ti.get("album_id") == "ALBUM-ID-FROM-SOURCE"
|
||||
|
|
@ -61,10 +61,13 @@ def _make_soulsync_db():
|
|||
bitrate INTEGER,
|
||||
file_size INTEGER,
|
||||
track_artist TEXT,
|
||||
musicbrainz_recording_id TEXT,
|
||||
isrc TEXT,
|
||||
server_source TEXT,
|
||||
created_at TEXT,
|
||||
updated_at TEXT,
|
||||
spotify_track_id TEXT
|
||||
spotify_track_id TEXT,
|
||||
deezer_id TEXT
|
||||
)
|
||||
"""
|
||||
)
|
||||
|
|
@ -204,3 +207,185 @@ def test_record_soulsync_library_entry_ignores_numeric_spotify_ids(tmp_path, mon
|
|||
assert artist_row["spotify_artist_id"] is None
|
||||
assert album_row["spotify_album_id"] is None
|
||||
assert track_row["spotify_track_id"] is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SoulSync standalone parity — auto-import / direct download must write the
|
||||
# same field richness a Plex/Jellyfin/Navidrome scan would write. Pin the
|
||||
# per-recording identifier columns (`musicbrainz_recording_id`, `isrc`)
|
||||
# AND the source-aware ID columns (`deezer_id`, etc.) for non-Spotify
|
||||
# sources so dev work can't silently drop them.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_record_soulsync_library_entry_writes_mbid_and_isrc(tmp_path, monkeypatch):
|
||||
"""Per-recording IDs land on the tracks row when the metadata source
|
||||
provides them (Picard-tagged libraries, MusicBrainz-enriched
|
||||
Spotify, etc.). Without this, watchlist re-download checks fall
|
||||
back to fuzzy name matching and re-download tracks the user
|
||||
already has."""
|
||||
conn = _make_soulsync_db()
|
||||
fake_db = _FakeDB(conn)
|
||||
final_path = tmp_path / "track.flac"
|
||||
final_path.write_bytes(b"audio")
|
||||
|
||||
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
|
||||
monkeypatch.setattr(
|
||||
side_effects,
|
||||
"_get_config_manager",
|
||||
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
|
||||
)
|
||||
import core.genre_filter as genre_filter
|
||||
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
|
||||
|
||||
context = {
|
||||
"source": "spotify",
|
||||
"artist": {"id": "sp-artist", "name": "Picard Artist"},
|
||||
"album": {
|
||||
"id": "sp-album", "name": "Tagged Album",
|
||||
"release_date": "2022-01-01", "total_tracks": 10,
|
||||
},
|
||||
"track_info": {
|
||||
"id": "sp-track", "name": "Tagged Track",
|
||||
"track_number": 3, "duration_ms": 195000,
|
||||
"artists": [{"name": "Picard Artist"}],
|
||||
# Per-recording IDs — read by Mutagen from MUSICBRAINZ_TRACKID
|
||||
# tag (Picard) or surfaced from the metadata source's response.
|
||||
"musicbrainz_recording_id": "abcd1234-mbid-uuid-form",
|
||||
"isrc": "USABC1234567",
|
||||
},
|
||||
"original_search_result": {"title": "Tagged Track"},
|
||||
"_final_processed_path": str(final_path),
|
||||
}
|
||||
artist_context = {"name": "Picard Artist", "genres": []}
|
||||
album_info = {"is_album": True, "album_name": "Tagged Album", "track_number": 3}
|
||||
|
||||
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
|
||||
|
||||
row = conn.execute("SELECT musicbrainz_recording_id, isrc FROM tracks").fetchone()
|
||||
assert row["musicbrainz_recording_id"] == "abcd1234-mbid-uuid-form"
|
||||
assert row["isrc"] == "USABC1234567"
|
||||
|
||||
|
||||
def test_record_soulsync_library_entry_handles_deezer_source(tmp_path, monkeypatch):
|
||||
"""Deezer source maps all three (artist/album/track) IDs onto the
|
||||
`deezer_id` column. Verify the source-aware column resolver routes
|
||||
correctly — a regression here means deezer-primary users get
|
||||
soulsync rows with no source ID at all."""
|
||||
conn = _make_soulsync_db()
|
||||
fake_db = _FakeDB(conn)
|
||||
final_path = tmp_path / "track.flac"
|
||||
final_path.write_bytes(b"audio")
|
||||
|
||||
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
|
||||
monkeypatch.setattr(
|
||||
side_effects,
|
||||
"_get_config_manager",
|
||||
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
|
||||
)
|
||||
import core.genre_filter as genre_filter
|
||||
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
|
||||
|
||||
context = {
|
||||
"source": "deezer",
|
||||
"artist": {"id": "12345", "name": "DZ Artist"},
|
||||
"album": {"id": "67890", "name": "DZ Album", "total_tracks": 5},
|
||||
"track_info": {
|
||||
"id": "111213",
|
||||
"name": "DZ Track",
|
||||
"track_number": 1,
|
||||
"duration_ms": 180000,
|
||||
"artists": [{"name": "DZ Artist"}],
|
||||
},
|
||||
"original_search_result": {"title": "DZ Track"},
|
||||
"_final_processed_path": str(final_path),
|
||||
}
|
||||
artist_context = {"name": "DZ Artist", "genres": []}
|
||||
album_info = {"is_album": True, "album_name": "DZ Album", "track_number": 1}
|
||||
|
||||
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
|
||||
|
||||
track_row = conn.execute("SELECT deezer_id FROM tracks").fetchone()
|
||||
# Deezer source map writes the track's source-id onto the deezer_id
|
||||
# column (same column name the artist + album use; deezer doesn't
|
||||
# split per-entity-type ID columns the way Spotify / iTunes do).
|
||||
assert track_row["deezer_id"] == "111213"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Auto-import labelling — library history + provenance must show
|
||||
# "Auto-Import" / "auto_import" instead of falling back to "Soulseek".
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_history_db():
|
||||
conn = sqlite3.connect(":memory:")
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute(
|
||||
"""
|
||||
CREATE TABLE library_history (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
event_type TEXT, title TEXT, artist_name TEXT, album_name TEXT,
|
||||
quality TEXT, file_path TEXT, thumb_url TEXT, download_source TEXT,
|
||||
source_track_id TEXT, source_track_title TEXT, source_filename TEXT,
|
||||
acoustid_result TEXT, source_artist TEXT, created_at TEXT
|
||||
)
|
||||
"""
|
||||
)
|
||||
return conn
|
||||
|
||||
|
||||
def test_library_history_labels_auto_import(monkeypatch):
|
||||
"""Auto-import sets `_download_username='auto_import'`; history row
|
||||
must read 'Auto-Import' instead of falling back to 'Soulseek'."""
|
||||
conn = _make_history_db()
|
||||
|
||||
captured = {}
|
||||
|
||||
class _DBStub:
|
||||
def add_library_history_entry(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
monkeypatch.setattr(side_effects, "get_database", lambda: _DBStub())
|
||||
|
||||
context = {
|
||||
"_download_username": "auto_import",
|
||||
"track_info": {
|
||||
"name": "Auto-Imported Track",
|
||||
"artists": [{"name": "Some Artist"}],
|
||||
"album": "Some Album",
|
||||
"id": "abc",
|
||||
},
|
||||
"original_search_result": {},
|
||||
"_final_processed_path": "/library/some-album/01.flac",
|
||||
}
|
||||
side_effects.record_library_history_download(context)
|
||||
assert captured["download_source"] == "Auto-Import"
|
||||
assert captured["title"] == "Auto-Imported Track"
|
||||
|
||||
|
||||
def test_provenance_labels_auto_import(monkeypatch):
|
||||
"""Same gate for provenance: `_download_username='auto_import'`
|
||||
must register the provenance row as `auto_import` (lowercase /
|
||||
canonical), not the `soulseek` fallback default."""
|
||||
captured = {}
|
||||
|
||||
class _DBStub:
|
||||
def record_track_download(self, **kwargs):
|
||||
captured.update(kwargs)
|
||||
|
||||
monkeypatch.setattr(side_effects, "get_database", lambda: _DBStub())
|
||||
|
||||
context = {
|
||||
"_download_username": "auto_import",
|
||||
"track_info": {
|
||||
"name": "Auto-Imported Track",
|
||||
"artists": [{"name": "Some Artist"}],
|
||||
"album": "Some Album",
|
||||
"id": "abc",
|
||||
},
|
||||
"original_search_result": {},
|
||||
"_final_processed_path": "/library/some-album/01.flac",
|
||||
}
|
||||
side_effects.record_download_provenance(context)
|
||||
assert captured.get("source_service") == "auto_import"
|
||||
|
|
|
|||
|
|
@ -3416,6 +3416,7 @@ const WHATS_NEW = {
|
|||
'2.4.3': [
|
||||
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.3 patch work' },
|
||||
{ title: 'Auto-Import: SoulSync Standalone Library Now Gets Full Server-Quality Rows', desc: 'soulsync standalone is meant to be a full replacement for plex / jellyfin / navidrome — the imported tracks should land in the db with the same field richness a media server scan would write. they weren\'t. the auto-import context dict (the payload it handed to the post-process pipeline) had no `source` field anywhere, so `record_soulsync_library_entry` couldn\'t pick the right source-id column on the new tracks/albums/artists rows. result: every auto-imported track landed with NULL on `spotify_track_id` / `deezer_id` / `itunes_track_id` / etc. — watchlist scans (which match by stable source IDs) couldn\'t recognise these tracks as already in library and would re-download them on the next pass. fixed by threading `identification[\'source\']` onto the top-level context, plus per-recording IDs (`isrc`, `musicbrainz_recording_id`) onto track_info so picard-tagged libraries land their per-recording metadata directly. also added `_download_username=\'auto_import\'` so library history shows "Auto-Import" instead of mislabeling every staging import as "Soulseek" (the fallback default), and an "auto_import" → "Auto-Import" mapping in the source-map dicts at side_effects.py to honour it. record_soulsync_library_entry tracks INSERT now also writes `musicbrainz_recording_id` + `isrc` columns directly (matches the navidrome scanner write path). 14 new tests pin: auto-import context carries source for every metadata source (spotify/deezer/itunes/discogs), `_download_username=auto_import`, isrc + mbid pass-through to track_info, album-id back-reference on track_info, soulsync library writes mbid + isrc to dedicated columns, deezer source maps to deezer_id column, library history + provenance use Auto-Import / auto_import labels.', page: 'import' },
|
||||
{ title: 'Auto-Import: Process Multiple Albums At Once', desc: 'auto-import used to process one album at a time. drop 5 albums into staging → wait for the first to fully finish (identify + match + every track post-processed) before the second one even starts. on a slow network or with a big batch this means 30+ minutes of staring at "Processing AlbumOne" while the others sit untouched. now there\'s a small bounded thread pool (3 workers by default, configurable) — up to 3 albums process in parallel, the queue moves through the rest as workers free up. clicking "Scan Now" multiple times no longer spawns extra unbounded scan threads — every trigger (timer + manual button) routes through one shared scan lock so duplicate triggers no-op instead of stacking up. live progress widget on the auto-import card now lists EACH in-flight album with its own track index/total/name instead of one shared scalar that the parallel workers used to stomp on each other. graceful shutdown: stopping the worker waits for in-flight pool work to finish before reporting stopped — no half-moved files or partial DB writes mid-album. stats counters (`scanned` / `auto_processed` / `pending_review` / `failed`) now use a lock so parallel workers don\'t lose increments under load. 17 new tests pin: pool size config, scan lock dedup, executor dispatch + bounded parallelism, cross-trigger candidate dedup, graceful shutdown, per-candidate UI state isolation across parallel workers, stats counter thread-safety, and snapshot consistency.', page: 'import' },
|
||||
{ title: 'Manual Search In The Failed-Track Candidates Modal', desc: 'when a download fails or returns "not found" the user can already click the status cell to open a modal showing whatever search candidates the auto-search left over and pick a different one. that modal now ALSO has a manual search bar. type any query, hit search, get a fresh round of results from the download sources without having to start the whole download flow over from the search page. solves the case where the auto-query was bad (featured artist not in title, parentheticals like "(remastered 2019)" tripping the matcher, slight artist-name variants) but the file genuinely exists on the source. source picker is smart per download mode: single-source mode (soulseek-only / youtube-only / etc) shows a "searching X" label, no dropdown; hybrid mode shows a dropdown with "all sources" default plus every configured source — picking "all" runs parallel searches across all of them and tags each result row with its source badge. only configured sources show up; unconfigured ones are hidden. results stream in as each source completes via NDJSON instead of blocking on the slowest source — the table starts populating the moment the first source returns. clicking a result reuses the existing retry-download flow → same path, same acoustid verification on the file when it lands, no shortcut around the safety net. additive in the truest sense: the existing modal layout / candidates table / download buttons are byte-identical when the user doesn\'t use manual search. backend extends the candidates endpoint with `download_mode` + `available_sources` + a `source` field per candidate (purely additive — old fields untouched), and adds a new `POST /api/downloads/task/<id>/manual-search` that streams NDJSON (one header line, one source_results line per source as completed, one done terminator) so the frontend renderer can append rows incrementally. 11 tests pin the streaming contract: query length / source whitelist / task 404 validation, single-source dispatch, parallel "all" dispatch, one-event-per-source streaming shape, unconfigured-source skip + reject, header metadata, and per-source exception isolation (one source raising emits a `source_error` event but doesn\'t fail the stream).', page: 'downloads' },
|
||||
{ title: 'Manual Picks Don\'t Auto-Retry Anymore (And The Modal Always Opens)', desc: 'three follow-on fixes to the manual-search feature once people started actually using it. (1) when the user picked a candidate and that download failed (e.g. soundcloud 404 on a stale track url), the auto-retry monitor would treat it like any other failed auto-attempt — yank the task back to "searching" and pick a different candidate. felt completely wrong from the user\'s perspective: "i picked THIS one, why is it searching for something else?" now manual picks are tagged with a `_user_manual_pick` flag and the auto-retry path bails on it. failure surfaces to the user instead of getting silently fallen-back. (2) non-soulseek manual picks (youtube / tidal / qobuz / hifi / deezer / soundcloud / lidarr) were getting stuck at "downloading 0%" forever even after their engine reported terminal failure. cause: status polling went into a "let monitor handle retry" branch that never fired because manual picks bail on retry — task was orphaned in downloading state. fix: when the engine reports Errored on a manual pick, mark the task failed directly, don\'t defer to the monitor. plus an engine-state fallback path covers the rare race where the orchestrator\'s pre-populated transfer lookup is missing the entry. (3) failed / not_found rows were only clickable when the auto-search had cached candidates — but the whole point of opening the modal now is to RUN a manual search, which doesn\'t need pre-existing candidates. now every failed / not_found / cancelled row opens the modal regardless. (4) one nasty deadlock fix in the process: the new "mark failed" path was synchronously calling `on_download_completed` while holding `tasks_lock`, which itself re-acquires the same lock — `threading.Lock` is non-reentrant so the polling thread wedged forever. while wedged the lock was held → every other endpoint that needed it (including /candidates → can\'t open OTHER modals) hung waiting. moved completion callbacks onto a daemon thread so the lock releases first. (5) manual download worker now runs on its own dedicated thread instead of competing with the batch\'s 3-worker `missing_download_executor` pool — saturated batches no longer queue manual picks indefinitely. all changes are scoped to manual picks only via the `_user_manual_pick` flag — auto-attempt flow is byte-identical to before. 17 unit tests pin the gate behavior (status engine fallback / monitor retry skip / IF-branch failure transition / auto-attempt skip).', page: 'downloads' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue