Canonical album version — Stage 3: Reorganizer prefers pinned canonical (read)
_resolve_source now prefers the album's pinned canonical (source, album_id) when set, before the source-priority walk. So once an album's canonical is resolved, reorganize agrees with Track Number Repair (Stage 4) and stops mislabelling a standard album as deluxe (#767-Bug2). Gated + side-effect-free: only changes behavior for albums that already carry a canonical (none do until the populate step runs), an explicit user source pick (strict_source) still wins over the canonical, and a failed canonical fetch falls through to today's priority walk. So this stage is behavior-neutral until canonical is populated. Tests: tests/test_reorganize_canonical_source.py (4) — canonical preferred over priority, fetch-failure falls back, strict_source ignores canonical, no-canonical unchanged. 113 reorganize-orchestrator/tag-source/unknown-artist tests still pass (no regression).
This commit is contained in:
parent
43878b4d3d
commit
ecdfde03c6
2 changed files with 94 additions and 0 deletions
|
|
@ -227,6 +227,25 @@ def _resolve_source(album_data: dict, primary_source: str, strict_source: bool =
|
|||
"""
|
||||
source_ids = _extract_source_ids(album_data)
|
||||
|
||||
# #765: if a canonical release was pinned for this album (best-fit to the
|
||||
# user's actual files), prefer it — so reorganize agrees with Track Number
|
||||
# Repair and stops mislabelling standard albums as deluxe (#767-Bug2). Gated
|
||||
# on the album row carrying a canonical, and skipped when the user explicitly
|
||||
# picked a source in the modal (strict_source) — their choice wins. Falls
|
||||
# through to the priority walk if the canonical fetch fails.
|
||||
if not strict_source:
|
||||
c_source = album_data.get('canonical_source')
|
||||
c_id = album_data.get('canonical_album_id')
|
||||
if c_source and c_id:
|
||||
try:
|
||||
api_album = get_album_for_source(c_source, c_id)
|
||||
api_tracks = get_album_tracks_for_source(c_source, c_id)
|
||||
items = _normalize_album_tracks(api_tracks)
|
||||
if items and api_album:
|
||||
return c_source, api_album, items
|
||||
except Exception as e:
|
||||
logger.warning(f"[Reorganize] canonical {c_source} lookup raised: {e}")
|
||||
|
||||
if strict_source:
|
||||
sources_to_try = [primary_source] if primary_source else []
|
||||
else:
|
||||
|
|
|
|||
75
tests/test_reorganize_canonical_source.py
Normal file
75
tests/test_reorganize_canonical_source.py
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
"""_resolve_source honors a pinned canonical release (#765 Stage 3, read side).
|
||||
|
||||
Gated + side-effect-free: only changes behavior for albums that already carry a
|
||||
canonical_source/canonical_album_id, and an explicit user source pick
|
||||
(strict_source) still wins. No canonical -> byte-identical to before.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import core.library_reorganize as lr
|
||||
|
||||
|
||||
def _patch_fetch(monkeypatch, tracklists):
|
||||
"""tracklists: {(source, album_id): items_or_None}. Patches the album +
|
||||
tracklist fetchers and the normaliser (pass-through)."""
|
||||
def get_album(source, aid):
|
||||
return {"name": f"{source}:{aid}"} if tracklists.get((source, aid)) else None
|
||||
|
||||
def get_tracks(source, aid):
|
||||
return tracklists.get((source, aid))
|
||||
|
||||
monkeypatch.setattr(lr, "get_album_for_source", get_album)
|
||||
monkeypatch.setattr(lr, "get_album_tracks_for_source", get_tracks)
|
||||
monkeypatch.setattr(lr, "_normalize_album_tracks", lambda items: items or [])
|
||||
monkeypatch.setattr(lr, "get_source_priority", lambda primary: ["spotify", "itunes", "deezer"])
|
||||
|
||||
|
||||
def test_canonical_source_preferred_over_priority(monkeypatch):
|
||||
# Album has spotify (priority winner) AND a pinned canonical = deezer.
|
||||
_patch_fetch(monkeypatch, {
|
||||
("spotify", "sp1"): [{"name": "x"}],
|
||||
("deezer", "dz1"): [{"name": "y"}],
|
||||
})
|
||||
album_data = {
|
||||
"spotify_album_id": "sp1", "deezer_id": "dz1",
|
||||
"canonical_source": "deezer", "canonical_album_id": "dz1",
|
||||
}
|
||||
source, api_album, items = lr._resolve_source(album_data, "spotify")
|
||||
assert source == "deezer" # canonical beats the priority walk
|
||||
|
||||
|
||||
def test_canonical_fetch_failure_falls_back_to_priority(monkeypatch):
|
||||
# Canonical points at musicbrainz but that fetch yields nothing -> fall back.
|
||||
_patch_fetch(monkeypatch, {
|
||||
("spotify", "sp1"): [{"name": "x"}],
|
||||
# no entry for ('musicbrainz', 'mb1') -> get_tracks returns None
|
||||
})
|
||||
album_data = {
|
||||
"spotify_album_id": "sp1",
|
||||
"canonical_source": "musicbrainz", "canonical_album_id": "mb1",
|
||||
}
|
||||
source, _, _ = lr._resolve_source(album_data, "spotify")
|
||||
assert source == "spotify" # fell back to priority
|
||||
|
||||
|
||||
def test_strict_source_ignores_canonical(monkeypatch):
|
||||
# User explicitly picked spotify in the modal — their choice wins over canonical.
|
||||
_patch_fetch(monkeypatch, {
|
||||
("spotify", "sp1"): [{"name": "x"}],
|
||||
("deezer", "dz1"): [{"name": "y"}],
|
||||
})
|
||||
album_data = {
|
||||
"spotify_album_id": "sp1", "deezer_id": "dz1",
|
||||
"canonical_source": "deezer", "canonical_album_id": "dz1",
|
||||
}
|
||||
source, _, _ = lr._resolve_source(album_data, "spotify", strict_source=True)
|
||||
assert source == "spotify"
|
||||
|
||||
|
||||
def test_no_canonical_unchanged(monkeypatch):
|
||||
# No canonical set -> identical to legacy priority resolution.
|
||||
_patch_fetch(monkeypatch, {("spotify", "sp1"): [{"name": "x"}]})
|
||||
album_data = {"spotify_album_id": "sp1"}
|
||||
source, _, _ = lr._resolve_source(album_data, "spotify")
|
||||
assert source == "spotify"
|
||||
Loading…
Reference in a new issue