Merge pull request #561 from Nezreka/fix/auto-import-multi-source-fallback
Fix/auto import multi source fallback
This commit is contained in:
commit
d10546a9bc
7 changed files with 989 additions and 71 deletions
|
|
@ -199,6 +199,53 @@ def _quality_rank(ext: str) -> int:
|
|||
return ranks.get(ext.lower(), 1)
|
||||
|
||||
|
||||
# Weight constants for `_score_album_search_result` — exposed at module
|
||||
# level so they're greppable + bumpable in one place. Pre-fix these were
|
||||
# magic numbers inline.
|
||||
_ALBUM_NAME_WEIGHT = 0.5 # title fuzzy similarity
|
||||
_ARTIST_NAME_WEIGHT = 0.2 # primary artist fuzzy similarity (skipped when target is empty)
|
||||
_TRACK_COUNT_WEIGHT = 0.3 # how close the source's track count is to the file count
|
||||
|
||||
|
||||
def _score_album_search_result(album_result, target_album: str,
|
||||
target_artist: Optional[str],
|
||||
file_count: int) -> float:
|
||||
"""Pure scoring helper for `_search_metadata_source`.
|
||||
|
||||
Weights how well an `album_result` from a metadata source's
|
||||
`search_albums` matches the search inputs. Returns float in [0.0, 1.0].
|
||||
Pre-extraction this lived inline in the loop body; lifting it out
|
||||
lets the weight math be pinned independently of the orchestrator
|
||||
(per-source iteration, exception containment, threshold check).
|
||||
|
||||
`album_result` is expected to expose:
|
||||
- `.name` (str)
|
||||
- `.artists` (list of dict-like with 'name', optional 'id') or list[str]
|
||||
- `.total_tracks` (int, optional)
|
||||
"""
|
||||
score = 0.0
|
||||
|
||||
# Album name similarity (default 50%)
|
||||
name = getattr(album_result, 'name', '') or ''
|
||||
score += _similarity(target_album, name) * _ALBUM_NAME_WEIGHT
|
||||
|
||||
# Artist similarity (default 20%) — only when target_artist provided
|
||||
if target_artist:
|
||||
artists = getattr(album_result, 'artists', None) or []
|
||||
r_artist = artists[0] if artists else ''
|
||||
if isinstance(r_artist, dict):
|
||||
r_artist = r_artist.get('name', '')
|
||||
score += _similarity(target_artist, str(r_artist)) * _ARTIST_NAME_WEIGHT
|
||||
|
||||
# Track count match (default 30%) — only when both sides have a count
|
||||
r_tracks = getattr(album_result, 'total_tracks', 0) or 0
|
||||
if r_tracks > 0 and file_count > 0:
|
||||
count_ratio = 1.0 - abs(r_tracks - file_count) / max(r_tracks, file_count)
|
||||
score += max(0.0, count_ratio) * _TRACK_COUNT_WEIGHT
|
||||
|
||||
return score
|
||||
|
||||
|
||||
class AutoImportWorker:
|
||||
"""Background worker that watches the staging folder and auto-imports music.
|
||||
|
||||
|
|
@ -1260,87 +1307,120 @@ class AutoImportWorker:
|
|||
def _search_metadata_source(self, artist: Optional[str], album: str,
|
||||
method: str, candidate: FolderCandidate,
|
||||
query: str = None) -> Optional[Dict]:
|
||||
"""Search the active metadata source for an album match."""
|
||||
"""Search configured metadata sources for an album match.
|
||||
|
||||
Iterates `get_source_priority(get_primary_source())` so primary
|
||||
is tried first and the rest are tried as fallback. Returns the
|
||||
FIRST source whose best result clears the 0.4 score threshold.
|
||||
|
||||
Pre-fix this only queried the primary, which meant indie/niche
|
||||
albums missing from the user's primary (e.g. Bandcamp releases
|
||||
not on Spotify) failed auto-import even when manual search
|
||||
could find them on Tidal/Deezer. The manual search bar at the
|
||||
bottom of the Import tab already iterates the full source
|
||||
chain via `search_import_albums` — this aligns auto-import
|
||||
with that behavior.
|
||||
"""
|
||||
try:
|
||||
from core.metadata_service import get_primary_source, get_client_for_source
|
||||
|
||||
source = get_primary_source()
|
||||
client = get_client_for_source(source)
|
||||
if not client or not hasattr(client, 'search_albums'):
|
||||
return None
|
||||
from core.metadata_service import (
|
||||
get_primary_source,
|
||||
get_source_priority,
|
||||
get_client_for_source,
|
||||
)
|
||||
|
||||
primary_source = get_primary_source()
|
||||
source_chain = get_source_priority(primary_source)
|
||||
search_query = query or (f"{artist} {album}" if artist else album)
|
||||
results = client.search_albums(search_query, limit=5)
|
||||
if not results:
|
||||
return None
|
||||
|
||||
# Score each result
|
||||
best_result = None
|
||||
best_score = 0
|
||||
for source in source_chain:
|
||||
client = get_client_for_source(source)
|
||||
if not client or not hasattr(client, 'search_albums'):
|
||||
continue
|
||||
|
||||
for r in results:
|
||||
score = 0
|
||||
# Album name similarity (50%)
|
||||
score += _similarity(album, r.name) * 0.5
|
||||
# Artist similarity (20%)
|
||||
if artist:
|
||||
r_artist = r.artists[0] if hasattr(r, 'artists') and r.artists else ''
|
||||
if isinstance(r_artist, dict):
|
||||
r_artist = r_artist.get('name', '')
|
||||
score += _similarity(artist, str(r_artist)) * 0.2
|
||||
# Track count match (30%)
|
||||
r_tracks = getattr(r, 'total_tracks', 0) or 0
|
||||
try:
|
||||
results = client.search_albums(search_query, limit=5)
|
||||
except Exception as e:
|
||||
# Per-source failures (rate limit, auth, transient HTTP)
|
||||
# shouldn't abort the fallback chain. Log + continue.
|
||||
logger.debug(
|
||||
f"Auto-import: search_albums failed on {source}: {e}"
|
||||
)
|
||||
continue
|
||||
|
||||
if not results:
|
||||
continue
|
||||
|
||||
# Score each result via the pure helper. Helper is
|
||||
# tested independently in
|
||||
# `tests/imports/test_album_search_scoring.py` so the
|
||||
# weight math is pinned at the function boundary, not
|
||||
# through the orchestrator path.
|
||||
file_count = len(candidate.audio_files)
|
||||
if r_tracks > 0 and file_count > 0:
|
||||
count_ratio = 1.0 - abs(r_tracks - file_count) / max(r_tracks, file_count)
|
||||
score += max(0, count_ratio) * 0.3
|
||||
best_result = None
|
||||
best_score = 0.0
|
||||
for r in results:
|
||||
score = _score_album_search_result(r, album, artist, file_count)
|
||||
if score > best_score:
|
||||
best_score = score
|
||||
best_result = r
|
||||
|
||||
if score > best_score:
|
||||
best_score = score
|
||||
best_result = r
|
||||
if not best_result or best_score < 0.4:
|
||||
# Primary returned weak/no match — fall through to next source
|
||||
if source != primary_source:
|
||||
logger.debug(
|
||||
f"Auto-import: {source} best score {best_score:.2f} "
|
||||
f"below threshold for '{album}', trying next source"
|
||||
)
|
||||
continue
|
||||
|
||||
if not best_result or best_score < 0.4:
|
||||
return None
|
||||
# Get image
|
||||
image_url = ''
|
||||
if hasattr(best_result, 'image_url'):
|
||||
image_url = best_result.image_url or ''
|
||||
elif hasattr(best_result, 'images') and best_result.images:
|
||||
img = best_result.images[0]
|
||||
image_url = img.get('url', '') if isinstance(img, dict) else str(img)
|
||||
|
||||
# Get image
|
||||
image_url = ''
|
||||
if hasattr(best_result, 'image_url'):
|
||||
image_url = best_result.image_url or ''
|
||||
elif hasattr(best_result, 'images') and best_result.images:
|
||||
img = best_result.images[0]
|
||||
image_url = img.get('url', '') if isinstance(img, dict) else str(img)
|
||||
r_artist = ''
|
||||
r_artist_id = ''
|
||||
if hasattr(best_result, 'artists') and best_result.artists:
|
||||
a = best_result.artists[0]
|
||||
if isinstance(a, dict):
|
||||
r_artist = a.get('name', str(a))
|
||||
# Surface the metadata-source artist ID so the
|
||||
# standalone-library write can land it on the right
|
||||
# `<source>_artist_id` column. Without this the
|
||||
# artists row gets created but with NULL on the
|
||||
# source-id, and watchlist scans can't recognise
|
||||
# the artist as already in library by stable ID.
|
||||
r_artist_id = str(a.get('id', '') or '')
|
||||
else:
|
||||
r_artist = str(a)
|
||||
|
||||
r_artist = ''
|
||||
r_artist_id = ''
|
||||
if hasattr(best_result, 'artists') and best_result.artists:
|
||||
a = best_result.artists[0]
|
||||
if isinstance(a, dict):
|
||||
r_artist = a.get('name', str(a))
|
||||
# Surface the metadata-source artist ID so the
|
||||
# standalone-library write can land it on the right
|
||||
# `<source>_artist_id` column. Without this the
|
||||
# artists row gets created but with NULL on the
|
||||
# source-id, and watchlist scans can't recognise
|
||||
# the artist as already in library by stable ID.
|
||||
r_artist_id = str(a.get('id', '') or '')
|
||||
else:
|
||||
r_artist = str(a)
|
||||
# Get release date
|
||||
release_date = getattr(best_result, 'release_date', '') or ''
|
||||
|
||||
# Get release date
|
||||
release_date = getattr(best_result, 'release_date', '') or ''
|
||||
if source != primary_source:
|
||||
logger.info(
|
||||
f"Auto-import: identified '{album}' via fallback "
|
||||
f"source {source!r} (score {best_score:.2f}, primary "
|
||||
f"{primary_source!r} returned nothing usable)"
|
||||
)
|
||||
|
||||
return {
|
||||
'album_id': best_result.id,
|
||||
'album_name': best_result.name,
|
||||
'artist_name': r_artist or artist or '',
|
||||
'artist_id': r_artist_id,
|
||||
'image_url': image_url,
|
||||
'release_date': release_date,
|
||||
'total_tracks': getattr(best_result, 'total_tracks', 0),
|
||||
'source': source,
|
||||
'method': method,
|
||||
'identification_confidence': best_score,
|
||||
}
|
||||
return {
|
||||
'album_id': best_result.id,
|
||||
'album_name': best_result.name,
|
||||
'artist_name': r_artist or artist or '',
|
||||
'artist_id': r_artist_id,
|
||||
'image_url': image_url,
|
||||
'release_date': release_date,
|
||||
'total_tracks': getattr(best_result, 'total_tracks', 0),
|
||||
'source': source,
|
||||
'method': method,
|
||||
'identification_confidence': best_score,
|
||||
}
|
||||
|
||||
return None
|
||||
|
||||
except Exception as e:
|
||||
logger.debug(f"Metadata search failed for '{album}': {e}")
|
||||
|
|
|
|||
217
tests/imports/test_album_search_scoring.py
Normal file
217
tests/imports/test_album_search_scoring.py
Normal file
|
|
@ -0,0 +1,217 @@
|
|||
"""Pin `_score_album_search_result` weight math.
|
||||
|
||||
Helper extracted from the inline scoring block inside
|
||||
`_search_metadata_source` (auto-import album identification). Lifting
|
||||
it to a pure function lets each weight be tested in isolation
|
||||
without mocking the full source-chain orchestrator.
|
||||
|
||||
Weights (constants in `core.auto_import_worker`):
|
||||
- `_ALBUM_NAME_WEIGHT` = 0.5
|
||||
- `_ARTIST_NAME_WEIGHT` = 0.2 (skipped when target_artist falsy)
|
||||
- `_TRACK_COUNT_WEIGHT` = 0.3 (skipped when either side has 0 tracks)
|
||||
|
||||
Maximum score is 1.0 when all three components match perfectly. The
|
||||
0.4 threshold in the orchestrator means a result needs at least one
|
||||
strong signal plus a partial second — pure track-count match alone
|
||||
(0.3) is below threshold.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from core.auto_import_worker import (
|
||||
_ALBUM_NAME_WEIGHT,
|
||||
_ARTIST_NAME_WEIGHT,
|
||||
_TRACK_COUNT_WEIGHT,
|
||||
_score_album_search_result,
|
||||
)
|
||||
|
||||
|
||||
def _result(name: str, artist_name: str = "", total_tracks: int = 0):
|
||||
"""Minimal album-result stub matching the shape `search_albums`
|
||||
returns. `artists` is the list-of-dicts shape every adapter uses."""
|
||||
artists = [{"name": artist_name}] if artist_name else []
|
||||
return SimpleNamespace(name=name, artists=artists, total_tracks=total_tracks)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Component weights — pinned at the boundary
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestWeightConstants:
|
||||
def test_weights_sum_to_one(self):
|
||||
"""Total weight budget = 1.0. If a weight is bumped without
|
||||
adjusting another, perfect-match score drifts above/below 1.0
|
||||
and the 0.4 threshold semantics shift silently."""
|
||||
total = _ALBUM_NAME_WEIGHT + _ARTIST_NAME_WEIGHT + _TRACK_COUNT_WEIGHT
|
||||
assert total == pytest.approx(1.0, abs=1e-9), (
|
||||
f"Weights must sum to 1.0; got {total}"
|
||||
)
|
||||
|
||||
def test_album_weight_is_dominant(self):
|
||||
"""Album name has 50% — strongest signal. If artist or track
|
||||
count weight ever exceeds album weight, the matching semantics
|
||||
flip (e.g. a wrong-album-right-count result could outscore
|
||||
a right-album-wrong-count one). Pin so a future weight tweak
|
||||
doesn't break this invariant."""
|
||||
assert _ALBUM_NAME_WEIGHT > _ARTIST_NAME_WEIGHT
|
||||
assert _ALBUM_NAME_WEIGHT > _TRACK_COUNT_WEIGHT
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Perfect match → 1.0
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPerfectMatch:
|
||||
def test_all_three_perfect_returns_one(self):
|
||||
r = _result("Test Album", "Test Artist", total_tracks=10)
|
||||
score = _score_album_search_result(r, "Test Album", "Test Artist", file_count=10)
|
||||
assert score == pytest.approx(1.0, abs=1e-9)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Album name component
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAlbumNameWeight:
|
||||
def test_exact_album_match_no_other_signals(self):
|
||||
"""Album name matches perfectly but no artist provided and
|
||||
no track count match — score is just 50%."""
|
||||
r = _result("Test Album", total_tracks=0)
|
||||
score = _score_album_search_result(r, "Test Album", target_artist=None, file_count=0)
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_completely_different_album_zero_album_component(self):
|
||||
r = _result("Totally Different", total_tracks=0)
|
||||
score = _score_album_search_result(r, "Test Album", target_artist=None, file_count=0)
|
||||
# SequenceMatcher would return some small non-zero similarity even
|
||||
# for fully different strings, so just verify it's well below 0.5
|
||||
assert score < _ALBUM_NAME_WEIGHT * 0.5
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Artist component
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestArtistWeight:
|
||||
def test_artist_match_adds_full_artist_weight(self):
|
||||
r = _result("Album", "Artist", total_tracks=0)
|
||||
with_artist = _score_album_search_result(r, "Album", "Artist", file_count=0)
|
||||
without_artist = _score_album_search_result(r, "Album", None, file_count=0)
|
||||
# Difference = artist weight
|
||||
assert with_artist - without_artist == pytest.approx(_ARTIST_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_target_artist_none_skips_artist_component(self):
|
||||
"""When target_artist is falsy (None / empty), artist weight
|
||||
contributes zero — not a penalty, not a bonus. Lets album-
|
||||
only searches (e.g. from a folder name with no artist info)
|
||||
still hit the threshold via album + track count alone."""
|
||||
r = _result("Album", "WrongArtist", total_tracks=10)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=10)
|
||||
# Album + track count perfect = 0.5 + 0.3 = 0.8 (artist weight skipped)
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT + _TRACK_COUNT_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_string_artist_not_dict_still_scored(self):
|
||||
"""Some adapters return `artists` as list-of-strings instead
|
||||
of list-of-dicts. Helper must handle both shapes."""
|
||||
r = SimpleNamespace(name="Album", artists=["Just A String"], total_tracks=0)
|
||||
score_string = _score_album_search_result(r, "Album", "Just A String", 0)
|
||||
assert score_string >= _ALBUM_NAME_WEIGHT + _ARTIST_NAME_WEIGHT - 0.05
|
||||
|
||||
def test_empty_artists_list_treats_as_no_artist(self):
|
||||
r = _result("Album", artist_name="", total_tracks=0) # no artist
|
||||
score = _score_album_search_result(r, "Album", "Some Target", file_count=0)
|
||||
# Artist sim against empty string is 0 → no artist weight contribution
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Track count component
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTrackCountWeight:
|
||||
def test_exact_track_count_match_full_weight(self):
|
||||
r = _result("Album", total_tracks=10)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=10)
|
||||
# Album (perfect) + track count (perfect) — no artist component
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT + _TRACK_COUNT_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_off_by_one_track_count_near_full(self):
|
||||
"""1 track off out of 10 → ratio = 1.0 - 1/10 = 0.9 → 0.9 * 0.3 = 0.27"""
|
||||
r = _result("Album", total_tracks=10)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=9)
|
||||
expected = _ALBUM_NAME_WEIGHT + (0.9 * _TRACK_COUNT_WEIGHT)
|
||||
assert score == pytest.approx(expected, abs=1e-9)
|
||||
|
||||
def test_bandcamp_vs_streaming_track_count_mismatch(self):
|
||||
"""Reporter's exact case: Bandcamp 7-track album vs Spotify
|
||||
4-track release. Track count ratio = 1.0 - 3/7 = ~0.571.
|
||||
With perfect album + artist match, total = 0.5 + 0.2 + 0.171
|
||||
= 0.871 → comfortably above the 0.4 threshold so the album
|
||||
still identifies despite the count mismatch."""
|
||||
r = _result("Work in Progress", "Godly the Ruler", total_tracks=4)
|
||||
score = _score_album_search_result(r, "Work in Progress", "Godly the Ruler", file_count=7)
|
||||
assert score > 0.4, (
|
||||
f"Bandcamp-vs-streaming case must still pass threshold; got {score:.3f}"
|
||||
)
|
||||
# Sanity bound — score should land around 0.87
|
||||
assert 0.85 < score < 0.90
|
||||
|
||||
def test_zero_track_count_from_source_skips_track_component(self):
|
||||
"""Some search responses don't include total_tracks. Helper
|
||||
must not penalize — just skip the track-count component."""
|
||||
r = _result("Album", total_tracks=0)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=10)
|
||||
# Only album component contributes
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_zero_file_count_skips_track_component(self):
|
||||
"""Defensive: candidate has 0 files (somehow). Don't divide
|
||||
by zero or skew the score."""
|
||||
r = _result("Album", total_tracks=10)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=0)
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_huge_mismatch_track_count_no_negative_contribution(self):
|
||||
"""File count 1, source track count 100 → ratio = 1 - 99/100
|
||||
= 0.01. Tiny but non-negative. `max(0, ...)` guards against
|
||||
any future formula change introducing a negative."""
|
||||
r = _result("Album", total_tracks=100)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=1)
|
||||
assert score >= _ALBUM_NAME_WEIGHT # at least the album component
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Edge cases — defensive
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestEdgeCases:
|
||||
def test_album_result_without_name_attribute(self):
|
||||
"""If the result somehow lacks `.name` (unusual adapter
|
||||
return), helper falls back to '' and scores 0 album sim."""
|
||||
r = SimpleNamespace(artists=[], total_tracks=0) # no `.name`
|
||||
score = _score_album_search_result(r, "Test Album", None, file_count=0)
|
||||
assert score == 0.0
|
||||
|
||||
def test_album_result_without_artists_attribute(self):
|
||||
"""If `.artists` is missing, treat as empty list."""
|
||||
r = SimpleNamespace(name="Album", total_tracks=0) # no .artists
|
||||
score = _score_album_search_result(r, "Album", "Target Artist", file_count=0)
|
||||
# Album matches perfectly; artist sim against missing is 0
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
|
||||
def test_album_result_with_none_total_tracks(self):
|
||||
"""Some adapters return None for missing total_tracks instead
|
||||
of 0. `getattr(..., 'total_tracks', 0) or 0` should handle it."""
|
||||
r = SimpleNamespace(name="Album", artists=[], total_tracks=None)
|
||||
score = _score_album_search_result(r, "Album", None, file_count=10)
|
||||
assert score == pytest.approx(_ALBUM_NAME_WEIGHT, abs=1e-9)
|
||||
232
tests/imports/test_auto_import_clear_completed_endpoint.py
Normal file
232
tests/imports/test_auto_import_clear_completed_endpoint.py
Normal file
|
|
@ -0,0 +1,232 @@
|
|||
"""Pin /api/auto-import/clear-completed behavior.
|
||||
|
||||
Reported case: Clear History button on the Import page left zombie
|
||||
rows behind — every survivor showed "⧗ Processing" from 2-9 days ago.
|
||||
Trace: `_record_in_progress` inserts a `status='processing'` row up-front
|
||||
so the UI can render the in-flight import; `_finalize_result` updates
|
||||
it to `completed`/`failed` when the import finishes. If the worker is
|
||||
killed mid-import (server restart, crash), the row never gets finalized
|
||||
and stays at `processing` forever. The endpoint's SQL delete-list
|
||||
omitted `processing`, so zombies survived every click.
|
||||
|
||||
Fix added `processing` to the delete list, BUT guards against nuking
|
||||
genuinely-live imports by intersecting against the worker's
|
||||
`_snapshot_active()` map — any folder hash currently registered there
|
||||
is excluded from the delete.
|
||||
|
||||
These tests pin:
|
||||
- `processing` rows ARE swept (no longer zombies)
|
||||
- Live `processing` rows (folder hash currently in `_active_imports`) survive
|
||||
- `pending_review` survives (user still must approve/reject)
|
||||
- `completed` / `approved` / `failed` / `needs_identification` /
|
||||
`rejected` rows still get swept (unchanged contract)
|
||||
- Count returned in the JSON response matches the actual delete count
|
||||
- Empty active set falls through to the unparameterized DELETE
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
from contextlib import contextmanager
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_test_client():
|
||||
import web_server
|
||||
web_server.app.config['TESTING'] = True
|
||||
with web_server.app.test_client() as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def seeded_db(tmp_path):
|
||||
"""Real sqlite DB with the auto_import_history table populated by
|
||||
a mix of statuses + folder hashes. Returns a (connection_factory,
|
||||
rows_seeded) tuple. The factory is a `_get_connection` lookalike
|
||||
that returns the same connection so the endpoint sees the same data."""
|
||||
db_path = str(tmp_path / 'test.db')
|
||||
conn = sqlite3.connect(db_path, check_same_thread=False)
|
||||
conn.row_factory = sqlite3.Row
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("""
|
||||
CREATE TABLE auto_import_history (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
folder_name TEXT NOT NULL,
|
||||
folder_path TEXT NOT NULL,
|
||||
folder_hash TEXT,
|
||||
status TEXT NOT NULL DEFAULT 'scanning',
|
||||
confidence REAL DEFAULT 0.0,
|
||||
album_id TEXT,
|
||||
album_name TEXT,
|
||||
artist_name TEXT,
|
||||
image_url TEXT,
|
||||
total_files INTEGER DEFAULT 0,
|
||||
matched_files INTEGER DEFAULT 0,
|
||||
match_data TEXT,
|
||||
identification_method TEXT,
|
||||
error_message TEXT,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
processed_at TIMESTAMP
|
||||
)
|
||||
""")
|
||||
seeds = [
|
||||
# (folder_name, folder_hash, status)
|
||||
('Album A', 'hash-completed-1', 'completed'),
|
||||
('Album B', 'hash-approved-1', 'approved'),
|
||||
('Album C', 'hash-failed-1', 'failed'),
|
||||
('Album D', 'hash-needsid-1', 'needs_identification'),
|
||||
('Album E', 'hash-rejected-1', 'rejected'),
|
||||
('Zombie F', 'hash-zombie-1', 'processing'), # stale
|
||||
('Zombie G', 'hash-zombie-2', 'processing'), # stale
|
||||
('Live Import H', 'hash-LIVE', 'processing'), # active — must survive
|
||||
('Awaiting Review I', 'hash-review-1', 'pending_review'), # must survive
|
||||
]
|
||||
for name, fh, status in seeds:
|
||||
cursor.execute(
|
||||
"INSERT INTO auto_import_history (folder_name, folder_path, folder_hash, status) "
|
||||
"VALUES (?, ?, ?, ?)",
|
||||
(name, f"/staging/{name}", fh, status),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
# Build a fake DB object whose `_get_connection` returns a context
|
||||
# manager wrapping the live connection (matches the endpoint's
|
||||
# `with db._get_connection() as conn` usage).
|
||||
@contextmanager
|
||||
def _conn_cm():
|
||||
yield conn
|
||||
|
||||
fake_db = MagicMock()
|
||||
fake_db._get_connection = _conn_cm
|
||||
|
||||
return fake_db, conn
|
||||
|
||||
|
||||
def _statuses_remaining(conn):
|
||||
cur = conn.cursor()
|
||||
cur.execute("SELECT folder_name, status FROM auto_import_history ORDER BY id")
|
||||
return [(row['folder_name'], row['status']) for row in cur.fetchall()]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestClearCompletedEndpoint:
|
||||
def test_sweeps_zombie_processing_rows_but_keeps_live(self, app_test_client, seeded_db, monkeypatch):
|
||||
"""The bug: a `processing` row that's been there for days is a
|
||||
zombie (server restart killed `_finalize_result`). Endpoint must
|
||||
sweep it. But a `processing` row whose folder_hash is currently
|
||||
registered in `_active_imports` is a LIVE import — must survive
|
||||
or the UI loses its in-flight row mid-run."""
|
||||
fake_db, conn = seeded_db
|
||||
# Worker reports one live import — folder_hash hash-LIVE
|
||||
fake_worker = MagicMock()
|
||||
fake_worker._snapshot_active.return_value = [{'folder_hash': 'hash-LIVE'}]
|
||||
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
|
||||
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
|
||||
|
||||
resp = app_test_client.post('/api/auto-import/clear-completed')
|
||||
assert resp.status_code == 200
|
||||
body = resp.get_json()
|
||||
assert body['success'] is True
|
||||
|
||||
survivors = _statuses_remaining(conn)
|
||||
names = {n for n, _ in survivors}
|
||||
# Live import survives
|
||||
assert 'Live Import H' in names, (
|
||||
f"Live in-flight processing row was deleted; survivors={names}"
|
||||
)
|
||||
# Pending review survives — user still must approve/reject
|
||||
assert 'Awaiting Review I' in names
|
||||
# Both zombies swept
|
||||
assert 'Zombie F' not in names
|
||||
assert 'Zombie G' not in names
|
||||
# Standard terminal-status rows swept
|
||||
for completed_name in ('Album A', 'Album B', 'Album C', 'Album D', 'Album E'):
|
||||
assert completed_name not in names, f"{completed_name} should have been deleted"
|
||||
|
||||
def test_count_in_response_matches_actual_deletes(self, app_test_client, seeded_db, monkeypatch):
|
||||
"""JSON response carries the rowcount so the UI toast can show
|
||||
accurate `Cleared N items`."""
|
||||
fake_db, conn = seeded_db
|
||||
fake_worker = MagicMock()
|
||||
fake_worker._snapshot_active.return_value = [{'folder_hash': 'hash-LIVE'}]
|
||||
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
|
||||
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
|
||||
|
||||
resp = app_test_client.post('/api/auto-import/clear-completed')
|
||||
body = resp.get_json()
|
||||
# 9 rows seeded; 7 deletable (5 terminal + 2 zombie processing);
|
||||
# 2 survive (1 live, 1 pending_review)
|
||||
assert body['count'] == 7, f"Expected 7 deletes; got {body['count']}"
|
||||
cur = conn.cursor()
|
||||
cur.execute("SELECT COUNT(*) FROM auto_import_history")
|
||||
assert cur.fetchone()[0] == 2
|
||||
|
||||
def test_empty_active_set_takes_unparameterized_path(self, app_test_client, seeded_db, monkeypatch):
|
||||
"""When no live imports are running, the SQL skips the `AND
|
||||
folder_hash NOT IN (...)` clause. Pinned because an empty
|
||||
`IN ()` is a SQL syntax error in sqlite — the branch matters."""
|
||||
fake_db, conn = seeded_db
|
||||
# Remove the live row so all `processing` rows are zombies
|
||||
cur = conn.cursor()
|
||||
cur.execute("DELETE FROM auto_import_history WHERE folder_hash = ?", ('hash-LIVE',))
|
||||
conn.commit()
|
||||
|
||||
fake_worker = MagicMock()
|
||||
fake_worker._snapshot_active.return_value = [] # nothing active
|
||||
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
|
||||
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
|
||||
|
||||
resp = app_test_client.post('/api/auto-import/clear-completed')
|
||||
assert resp.status_code == 200
|
||||
body = resp.get_json()
|
||||
assert body['success'] is True
|
||||
# 5 terminal + 2 zombie processing = 7. Pending_review (1) survives.
|
||||
assert body['count'] == 7
|
||||
|
||||
survivors = _statuses_remaining(conn)
|
||||
assert len(survivors) == 1
|
||||
assert survivors[0][1] == 'pending_review'
|
||||
|
||||
def test_worker_unavailable_returns_500(self, app_test_client, monkeypatch):
|
||||
"""If the auto-import worker isn't initialised, the endpoint
|
||||
bails early — no DB access, clear error. Pre-fix this branch
|
||||
was already in place; pinning ensures the active-hash refactor
|
||||
didn't accidentally start touching the worker before the guard."""
|
||||
monkeypatch.setattr('web_server.auto_import_worker', None)
|
||||
resp = app_test_client.post('/api/auto-import/clear-completed')
|
||||
assert resp.status_code == 500
|
||||
body = resp.get_json()
|
||||
assert body['success'] is False
|
||||
assert 'not available' in body['error'].lower()
|
||||
|
||||
def test_pending_review_always_survives(self, app_test_client, seeded_db, monkeypatch):
|
||||
"""Specific pin for the deliberate `pending_review` exclusion.
|
||||
Even when no imports are active and every other status is being
|
||||
swept, `pending_review` rows must be left alone — user-action
|
||||
required, not automatic cleanup."""
|
||||
fake_db, conn = seeded_db
|
||||
fake_worker = MagicMock()
|
||||
fake_worker._snapshot_active.return_value = []
|
||||
monkeypatch.setattr('web_server.auto_import_worker', fake_worker)
|
||||
monkeypatch.setattr('web_server.get_database', lambda: fake_db)
|
||||
|
||||
app_test_client.post('/api/auto-import/clear-completed')
|
||||
|
||||
cur = conn.cursor()
|
||||
cur.execute("SELECT COUNT(*) FROM auto_import_history WHERE status = 'pending_review'")
|
||||
assert cur.fetchone()[0] == 1, (
|
||||
"pending_review rows must never be swept by clear-completed"
|
||||
)
|
||||
|
|
@ -480,6 +480,7 @@ def test_search_metadata_source_extracts_artist_id_from_dict_artist():
|
|||
|
||||
worker = AutoImportWorker(database=MagicMock(), process_callback=lambda *a, **k: None)
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority", return_value=["spotify"]), \
|
||||
patch("core.metadata_service.get_client_for_source", return_value=fake_client):
|
||||
result = worker._search_metadata_source(
|
||||
"Test Artist", "Test Album", "tags", candidate,
|
||||
|
|
|
|||
365
tests/imports/test_auto_import_multi_source_fallback.py
Normal file
365
tests/imports/test_auto_import_multi_source_fallback.py
Normal file
|
|
@ -0,0 +1,365 @@
|
|||
"""Pin auto-import multi-source fallback for album identification.
|
||||
|
||||
Discord report (mushy, paraphrased): 16 Bandcamp albums sat in staging
|
||||
because auto-import couldn't identify them. Manual search at the
|
||||
bottom of the Import Music tab found the same albums fine via Tidal
|
||||
or Deezer — the user's primary metadata source (Spotify) just didn't
|
||||
have them.
|
||||
|
||||
Root cause: `_search_metadata_source` only queried the primary source.
|
||||
The manual `search_import_albums` path already iterates the full
|
||||
`get_source_priority(get_primary_source())` chain and breaks on first
|
||||
source that returns results. This brings auto-import to parity.
|
||||
|
||||
Fix semantics (option C — "primary first, fall through on weak"):
|
||||
- Try primary source first
|
||||
- Score result; if best ≥ 0.4 → return with that source
|
||||
- Otherwise fall through to next source in priority order
|
||||
- First source that produces a result above threshold wins
|
||||
- Returns None only if ALL sources fail / score below threshold
|
||||
|
||||
Tests pin:
|
||||
- Primary success path unchanged (returns primary result, no fallback fired)
|
||||
- Primary returns nothing → fallback fires to next source
|
||||
- Primary scores below threshold → fallback fires
|
||||
- First fallback succeeds → no further sources queried
|
||||
- All sources fail → None
|
||||
- Per-source exception is contained (doesn't abort the chain)
|
||||
- Result `source` field reflects WHICH source actually matched
|
||||
- `identification_confidence` is the score from the winning source
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.auto_import_worker import AutoImportWorker, FolderCandidate
|
||||
|
||||
|
||||
def _make_album(name: str, artist_name: str, total_tracks: int,
|
||||
album_id: str = "alb-id", artist_id: str = "art-id"):
|
||||
"""Build a fake album result matching `search_albums` return shape."""
|
||||
return SimpleNamespace(
|
||||
id=album_id,
|
||||
name=name,
|
||||
artists=[{"id": artist_id, "name": artist_name}],
|
||||
total_tracks=total_tracks,
|
||||
image_url="https://img.example/cover.jpg",
|
||||
release_date="2024-01-01",
|
||||
)
|
||||
|
||||
|
||||
def _make_worker():
|
||||
"""Bare AutoImportWorker bypassing __init__ side effects."""
|
||||
return AutoImportWorker(database=MagicMock(), process_callback=lambda *a, **k: None)
|
||||
|
||||
|
||||
def _make_candidate(file_count: int = 7, name: str = "TestAlbum"):
|
||||
"""Folder candidate with N files (no actual disk reads)."""
|
||||
return FolderCandidate(
|
||||
path=f"/staging/{name}",
|
||||
name=name,
|
||||
audio_files=[f"/staging/{name}/{i:02d}.flac" for i in range(1, file_count + 1)],
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Primary success path — fallback never fires
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPrimarySuccess:
|
||||
def test_primary_returns_strong_match_no_fallback(self):
|
||||
"""Pre-fix behavior preserved: if primary scores above 0.4,
|
||||
return its result and don't touch other sources."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=10)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.return_value = [
|
||||
_make_album("Test Album", "Test Artist", total_tracks=10),
|
||||
]
|
||||
tidal_client = MagicMock() # should NEVER be called
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source(
|
||||
"Test Artist", "Test Album", "tags", candidate,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "spotify"
|
||||
assert result["album_name"] == "Test Album"
|
||||
spotify_client.search_albums.assert_called_once()
|
||||
tidal_client.search_albums.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Primary fails — fallback fires
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFallbackOnNoResults:
|
||||
def test_primary_empty_falls_through_to_next_source(self):
|
||||
"""Reporter's exact case: Spotify doesn't have the Bandcamp
|
||||
indie album. Tidal does. Auto-import must find it via Tidal."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=7)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.return_value = [] # not on Spotify
|
||||
tidal_client = MagicMock()
|
||||
tidal_client.search_albums.return_value = [
|
||||
_make_album("Work in Progress", "Godly the Ruler", total_tracks=7,
|
||||
album_id="tidal-alb-1", artist_id="tidal-art-1"),
|
||||
]
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source(
|
||||
"Godly the Ruler", "Work in Progress", "tags", candidate,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "tidal", "Result must carry the source that actually matched"
|
||||
assert result["album_id"] == "tidal-alb-1"
|
||||
assert result["artist_id"] == "tidal-art-1"
|
||||
spotify_client.search_albums.assert_called_once()
|
||||
tidal_client.search_albums.assert_called_once()
|
||||
|
||||
|
||||
class TestFallbackOnWeakScore:
|
||||
def test_primary_below_threshold_falls_through(self):
|
||||
"""Primary returns results but none score above 0.4 (e.g.
|
||||
wrong-album false-matches). Fall through to next source for
|
||||
a stronger match."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=7)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
# Wrong album — name barely matches, no artist match, wrong track count
|
||||
spotify_client.search_albums.return_value = [
|
||||
_make_album("Different", "Wrong Artist", total_tracks=2),
|
||||
]
|
||||
deezer_client = MagicMock()
|
||||
deezer_client.search_albums.return_value = [
|
||||
_make_album("Work in Progress", "Godly the Ruler", total_tracks=7),
|
||||
]
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "deezer": deezer_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source(
|
||||
"Godly the Ruler", "Work in Progress", "tags", candidate,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "deezer"
|
||||
assert result["album_name"] == "Work in Progress"
|
||||
# Both clients called — primary returned weak, fallback picked up
|
||||
spotify_client.search_albums.assert_called_once()
|
||||
deezer_client.search_albums.assert_called_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Chain semantics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestChainSemantics:
|
||||
def test_first_fallback_success_stops_chain(self):
|
||||
"""When fallback succeeds, no further sources are queried.
|
||||
Don't waste API budget on Deezer if Tidal already gave us a
|
||||
strong result."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=10)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.return_value = []
|
||||
tidal_client = MagicMock()
|
||||
tidal_client.search_albums.return_value = [
|
||||
_make_album("Test", "Artist", total_tracks=10),
|
||||
]
|
||||
deezer_client = MagicMock() # should NEVER be called
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client,
|
||||
"tidal": tidal_client,
|
||||
"deezer": deezer_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "tidal"
|
||||
deezer_client.search_albums.assert_not_called()
|
||||
|
||||
def test_all_sources_fail_returns_none(self):
|
||||
"""If every source returns nothing or scores below threshold,
|
||||
the whole search returns None (caller proceeds to next
|
||||
identification strategy)."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=7)
|
||||
|
||||
empty_client = MagicMock()
|
||||
empty_client.search_albums.return_value = []
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
return_value=empty_client):
|
||||
result = worker._search_metadata_source(
|
||||
"Unknown Artist", "Nonexistent Album", "tags", candidate,
|
||||
)
|
||||
|
||||
assert result is None
|
||||
# All 3 sources got queried
|
||||
assert empty_client.search_albums.call_count == 3
|
||||
|
||||
def test_per_source_exception_does_not_abort_chain(self):
|
||||
"""If one source raises (rate limit, auth, transient HTTP),
|
||||
the chain continues to the next source instead of aborting
|
||||
the whole identification attempt."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=10)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.side_effect = RuntimeError("rate limit")
|
||||
tidal_client = MagicMock()
|
||||
tidal_client.search_albums.return_value = [
|
||||
_make_album("Test", "Artist", total_tracks=10),
|
||||
]
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "tidal", (
|
||||
"Primary exception must not block the fallback chain"
|
||||
)
|
||||
|
||||
def test_unconfigured_source_skipped_gracefully(self):
|
||||
"""If `get_client_for_source` returns None for a source
|
||||
(user hasn't configured it), skip and continue."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=10)
|
||||
|
||||
tidal_client = MagicMock()
|
||||
tidal_client.search_albums.return_value = [
|
||||
_make_album("Test", "Artist", total_tracks=10),
|
||||
]
|
||||
|
||||
# Spotify returns None (no client configured); Tidal works
|
||||
def client_dispatch(source, **kwargs):
|
||||
if source == "spotify":
|
||||
return None
|
||||
return {"tidal": tidal_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
|
||||
|
||||
assert result is not None
|
||||
assert result["source"] == "tidal"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Result shape preservation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestResultShape:
|
||||
def test_result_carries_correct_source_for_downstream_match(self):
|
||||
"""`_match_tracks` reads `identification['source']` to know
|
||||
which client to ask for the album's tracklist. Result MUST
|
||||
carry the source that actually matched, not the primary
|
||||
source name."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=8)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.return_value = []
|
||||
deezer_client = MagicMock()
|
||||
deezer_client.search_albums.return_value = [
|
||||
_make_album("Test", "Artist", total_tracks=8, album_id="dz-123"),
|
||||
]
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "deezer": deezer_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "deezer"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source("Artist", "Test", "tags", candidate)
|
||||
|
||||
assert result["source"] == "deezer"
|
||||
assert result["album_id"] == "dz-123", (
|
||||
"Album ID must be the Deezer ID so _match_tracks queries "
|
||||
"Deezer's get_album with the right ID format"
|
||||
)
|
||||
|
||||
def test_identification_confidence_reflects_winning_source(self):
|
||||
"""`identification_confidence` is used in the overall-confidence
|
||||
formula and the 0.9 / 0.7 cascade thresholds. It must be the
|
||||
score from the source that actually matched."""
|
||||
worker = _make_worker()
|
||||
candidate = _make_candidate(file_count=10)
|
||||
|
||||
spotify_client = MagicMock()
|
||||
spotify_client.search_albums.return_value = []
|
||||
# Perfect match on Tidal — all 3 weights at max → score = 1.0
|
||||
tidal_client = MagicMock()
|
||||
tidal_client.search_albums.return_value = [
|
||||
_make_album("Test Album", "Test Artist", total_tracks=10),
|
||||
]
|
||||
|
||||
def client_dispatch(source, **kwargs):
|
||||
return {"spotify": spotify_client, "tidal": tidal_client}.get(source)
|
||||
|
||||
with patch("core.metadata_service.get_primary_source", return_value="spotify"), \
|
||||
patch("core.metadata_service.get_source_priority",
|
||||
return_value=["spotify", "tidal"]), \
|
||||
patch("core.metadata_service.get_client_for_source",
|
||||
side_effect=client_dispatch):
|
||||
result = worker._search_metadata_source(
|
||||
"Test Artist", "Test Album", "tags", candidate,
|
||||
)
|
||||
|
||||
assert result["identification_confidence"] == pytest.approx(1.0, abs=0.01)
|
||||
|
|
@ -34625,14 +34625,35 @@ def auto_import_approve_all():
|
|||
|
||||
@app.route('/api/auto-import/clear-completed', methods=['POST'])
|
||||
def auto_import_clear_completed():
|
||||
"""Remove completed/imported items from history."""
|
||||
"""Remove completed/imported items from history.
|
||||
|
||||
`processing` rows are included so zombie entries (server restarted
|
||||
mid-import → `_record_in_progress` row never got finalized) get
|
||||
swept. Live in-flight imports are protected by intersecting against
|
||||
`_snapshot_active()` — anything currently registered in the worker's
|
||||
`_active_imports` map keeps its row. `pending_review` is left out so
|
||||
user still has to approve/reject those explicitly.
|
||||
"""
|
||||
if not auto_import_worker:
|
||||
return jsonify({"success": False, "error": "Auto-import not available"}), 500
|
||||
try:
|
||||
active_hashes = {e['folder_hash'] for e in auto_import_worker._snapshot_active()}
|
||||
db = get_database()
|
||||
with db._get_connection() as conn:
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("DELETE FROM auto_import_history WHERE status IN ('completed', 'approved', 'failed', 'needs_identification', 'rejected')")
|
||||
base_sql = (
|
||||
"DELETE FROM auto_import_history "
|
||||
"WHERE status IN ('completed', 'approved', 'failed', "
|
||||
"'needs_identification', 'rejected', 'processing')"
|
||||
)
|
||||
if active_hashes:
|
||||
placeholders = ','.join('?' * len(active_hashes))
|
||||
cursor.execute(
|
||||
f"{base_sql} AND folder_hash NOT IN ({placeholders})",
|
||||
tuple(active_hashes),
|
||||
)
|
||||
else:
|
||||
cursor.execute(base_sql)
|
||||
count = cursor.rowcount
|
||||
conn.commit()
|
||||
return jsonify({"success": True, "count": count})
|
||||
|
|
|
|||
|
|
@ -3416,6 +3416,8 @@ const WHATS_NEW = {
|
|||
'2.5.1': [
|
||||
// --- post-release patch work on the 2.5.1 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.5.1 patch work' },
|
||||
{ title: 'Import History: Clear History Button Now Clears Stuck "Processing" Rows', desc: 'noticed on the import page: clear history left zombie rows behind that all showed "⧗ processing" status from 2-9 days ago. trace: `_record_in_progress` inserts a `status=\'processing\'` row up-front so the ui can render the in-flight import while it runs, then `_finalize_result` updates it to `completed`/`failed` when the import finishes. when the server is restarted mid-import (or the worker crashes), the row never gets finalized — stays at `processing` forever. the clear-history endpoint\'s sql `DELETE ... WHERE status IN (\'completed\', \'approved\', \'failed\', \'needs_identification\', \'rejected\')` didn\'t include `processing`, so those zombies survived every click. fix: add `processing` to the delete list, but guard against nuking actually-live imports by intersecting against `_snapshot_active()` — any folder hash currently registered in the worker\'s in-memory `_active_imports` map is excluded from the delete. `pending_review` deliberately left out so user still has to approve/reject those explicitly. one endpoint touched (`/api/auto-import/clear-completed` in web_server.py). no worker changes. zombie-row pile gets swept on next click, new imports still record + update normally.', page: 'import' },
|
||||
{ title: 'Auto-Import: Falls Through To Other Metadata Sources When Primary Has No Match', desc: 'discord report (mushy): 16 bandcamp indie albums sat in staging because auto-import couldn\'t identify them. manual search at the bottom of the import music tab found the same albums fine — they just weren\'t on the user\'s primary metadata source (spotify) but existed on tidal/deezer. trace: `_search_metadata_source` in `core/auto_import_worker.py` only queried `get_primary_source()` — single source, no fallback. meanwhile `search_import_albums` (the manual search bar at the bottom of the tab) already iterated the full `get_source_priority(get_primary_source())` chain and broke on first source with results. asymmetric behavior — manual search worked, auto-import didn\'t, same album. fix: lift auto-import to use the same source-chain pattern. try primary first; if it returns nothing OR scores below the 0.4 threshold, fall through to next source in priority order. first source that produces a strong-enough match wins. result dict carries the `source` that actually matched (not the primary name), so downstream `_match_tracks` calls the right client to fetch the album\'s tracklist. defensive per-source try/except so a rate-limited or auth-failed source doesn\'t abort the chain. unconfigured sources (client=None) silently skipped. scoring math lifted to pure helper `_score_album_search_result` so weight tweaks (album 50% / artist 20% / track-count 30%) are pinned at the function boundary independent of the orchestrator. weight constants exposed at module level (`_ALBUM_NAME_WEIGHT`, `_ARTIST_NAME_WEIGHT`, `_TRACK_COUNT_WEIGHT`) — greppable, bumpable in one place. 9 integration tests + 18 scoring-helper tests. integration tests pin: primary-success path unchanged (no fallback fires, only primary client called), primary-empty falls through to next source, primary-weak-score falls through, first fallback success stops the chain (no wasted api calls on remaining sources), all-sources-fail returns None, per-source exception contained, unconfigured-source skipped gracefully, result `source` field reflects winning source, `identification_confidence` from winning source. backwards compatible — single-source users see no change (chain just has one entry).', page: 'import' },
|
||||
{ title: 'Multi-Artist Tag Settings Now Actually Work (artist_separator + feat_in_title + write_multi_artist)', desc: 'three settings on settings → metadata → tags were partially or completely unimplemented. (1) `write_multi_artist` only worked because of a never-populated `_artists_list` field — `core/metadata/source.py` built `metadata["artist"]` as a hardcoded ", "-joined string but never assigned `metadata["_artists_list"]`, so `core/metadata/enrichment.py:114` always saw an empty list and silently no-op\'d the multi-value tag write. (2) `artist_separator` (default ", ") was referenced in the UI + settings.js save path but ZERO python code read the value — every multi-artist track ended up with hardcoded ", " regardless of what the user picked. (3) `feat_in_title` (when true: pull featured artists into the title as " (feat. X, Y)" and leave only primary in the ARTIST tag — picard convention) had no implementation at all. fix in source.py: populate `_artists_list` from the search response\'s artists array, then build the ARTIST string per the user\'s settings — primary-only when feat_in_title is on (with featured names appended to title; double-append guarded for source titles that already include "feat."), else joined with the configured separator. fix in enrichment.py id3 path: writing TPE1 twice (single-string then list) was overwriting the configured separator. now keeps TPE1 as the display string and writes a separate `TXXX:Artists` frame for the multi-value list (picard convention). vorbis path was already correct (separate "artist" + "artists" keys). deezer-specific upgrade path: deezer\'s `/search` endpoint only returns the primary artist — full contributors live on `/track/<id>`. when source==deezer AND the search response had a single artist AND a track_id is available, enrichment now fetches the per-track endpoint and upgrades the artists list before tag-write. one extra API call per affected deezer track (skipped when search already returned multiple). spotify, tidal, itunes search responses already include all artists so they\'re unaffected. 29 new tests pin: `_artists_list` populated for multi/single/no-artist cases, separator drives ARTIST string (default + custom), single-artist case unaffected by either setting, feat_in_title pulls featured to title + leaves primary in ARTIST, feat_in_title no-op for single artist, double-append guard recognizes 9 source-title variants ("(feat. X)", "(Feat. X)", "(FEAT X)", "(feat X)", "(Featuring X)", "[feat. X]", "ft. X", "(ft X)", "FT. X"), word-boundary regex doesn\'t false-match substrings ("Aftermath" still gets the append), combined-settings precedence (feat_in_title wins over separator for ARTIST string but `_artists_list` carries everyone for the multi-value tag), deezer upgrade fires only when search returned single artist + track_id available, no upgrade for non-deezer sources, upgrade failure falls through to search-result list, no false-positive when /track/<id> confirms single artist.', page: 'settings' },
|
||||
{ title: 'AudioDB Enrichment: Track Worker No Longer Stuck In Infinite Retry Loop', desc: 'github issue #553: audiodb track enrichment "stuck" — constant requests, no progress, only error log was a 10s read-timeout from `lookup_track_by_id` repeating against the same track. trace: when an entity already has `audiodb_id` populated (from manual match or earlier scan) but `audiodb_match_status` is NULL, the worker tries a direct ID lookup. if it fails (returns None on timeout — audiodb\'s `track.php` endpoint is slow, 10s timeouts common), the prior code logged "preserving manual match" and returned WITHOUT marking status. row stayed NULL → queue picked it up next tick → tried direct lookup → timed out → returned → infinite loop. fix: (1) when direct lookup fails (None or exception), mark `audiodb_match_status="error"` so the queue\'s NULL-status filter stops re-picking the row on every tick. preserves the existing `audiodb_id` (no fallback to name-search guess that would overwrite a manual match). (2) extended the retry-after-cutoff queue priorities (4/5/6) to include `\'error\'` rows alongside `\'not_found\'` — same `retry_days=30` window. transient audiodb outages still recover automatically; permanently-broken IDs eventually get re-attempted once a month. only triggered for entities in the inconsistent state of `audiodb_id` set + `match_status` NULL — happy path and already-matched/already-not-found rows unchanged. 5 new tests pin: lookup-returns-none marks error (no infinite loop), lookup-raises-exception marks error, lookup-success preserves happy path, error-row-past-cutoff gets re-picked, error-row-within-cutoff stays skipped.', page: 'tools' },
|
||||
{ title: 'Docker: Container No Longer Restart-Loops On Bind-Mounted Staging Folder', desc: 'after pulling latest, the container refused to start. logs showed `mkdir: cannot create directory \'/app/Staging\': Permission denied`. cause traced back to the 2026-05-08 image-bloat fix (commit 70e1750) which changed the Dockerfile from `chown -R /app` to a scoped chown on specific subdirs (the recursive chown was duplicating the whole /app tree into a new layer and ballooning image size). side effect: `/app` itself went from soulsync:soulsync to root:root (Docker WORKDIR default), AND `/app/Staging` was left out of both the Dockerfile mkdir + chown list and only created at runtime by the entrypoint script. on rootless Docker / Podman where in-container "root" maps to a host UID, the entrypoint mkdir on `/app/Staging` could fail with EACCES depending on the bind-mount path\'s host ownership — `set -e` then aborted the script and the container restart-looped. fix: (1) Dockerfile now pre-bakes `/app/Staging` into the image alongside the other runtime mount points (mkdir + scoped chown) so the entrypoint mkdir is a guaranteed no-op even when bind-mount perms are weird. (2) entrypoint mkdir + chown both have `|| true` now so any future bind-mount permission quirk surfaces as a log line, not a restart loop. (3) new writability audit at the end of entrypoint setup — `gosu soulsync test -w` on every bind-mountable dir, logs a loud warning with the exact `chown` command to run on the host if perms mismatch the configured PUID/PGID. catches the underlying bind-mount perm issue that the restart-loop fix would otherwise mask (container starts, but auto-import / downloads write into unwritable dirs and fail silently). zero behavior change for users whose containers were already starting fine; defensive against the rootless/podman config that broke after the image-bloat refactor.', page: 'tools' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue