Fix $albumtype defaulting to album for non-Spotify sources
- legacy duck-typed builder only checked the `album_type` key; deezer uses `record_type`, tidal uses `type` (uppercase), some flattened musicbrainz shapes use `primary-type` — all defaulted to album, so EPs and singles ended up filed under Album/ in user templates that reference $albumtype - widen lookup to album_type / record_type / type / primary-type and route through new pure `_normalize_album_type` helper that case-folds + validates against the canonical token set (album / single / ep / compilation), unknown → album - typed-converter path (spotify / deezer / itunes / discogs / mb / hydrabase / qobuz) unchanged — those were already correct Discord report (CAL).
This commit is contained in:
parent
09597eb6db
commit
5eae24b8bb
3 changed files with 170 additions and 1 deletions
|
|
@ -267,6 +267,33 @@ def _build_album_info_typed(album_data: Dict[str, Any], album_id: str,
|
|||
return ctx
|
||||
|
||||
|
||||
_ALBUM_TYPE_CANONICAL = {'album', 'single', 'ep', 'compilation'}
|
||||
|
||||
|
||||
def _normalize_album_type(value: Any, default: str = 'album') -> str:
|
||||
"""Map a raw album-type value from any source to the canonical
|
||||
lowercase token the path templates use (``album`` / ``single`` /
|
||||
``ep`` / ``compilation``).
|
||||
|
||||
Different metadata sources expose the album type under different
|
||||
keys AND with different casing — Tidal returns ``ALBUM``, MB
|
||||
returns ``Album``, Deezer's ``record_type`` is already lowercase.
|
||||
Without this normalization the legacy duck-typed builder accepted
|
||||
only Spotify-shaped lowercase ``album_type``, so every other
|
||||
source's discography ended up filed under ``Album/`` regardless
|
||||
of actual type (Discord report, CAL, 2026-05-12).
|
||||
|
||||
Unknown values fall back to ``default`` rather than passing
|
||||
through — keeps stray strings out of the path template.
|
||||
"""
|
||||
if value is None:
|
||||
return default
|
||||
v = str(value).strip().lower()
|
||||
if not v:
|
||||
return default
|
||||
return v if v in _ALBUM_TYPE_CANONICAL else default
|
||||
|
||||
|
||||
def _build_album_info_legacy(album_data: Any, album_id: str,
|
||||
album_name: str, artist_name: str) -> Dict[str, Any]:
|
||||
"""Original duck-typed extraction. Kept as the fallback when the
|
||||
|
|
@ -310,7 +337,12 @@ def _build_album_info_legacy(album_data: Any, album_id: str,
|
|||
'image_url': image_url,
|
||||
'images': images,
|
||||
'release_date': _extract_lookup_value(album_data, 'release_date', default='') or '',
|
||||
'album_type': _extract_lookup_value(album_data, 'album_type', default='album') or 'album',
|
||||
'album_type': _normalize_album_type(
|
||||
_extract_lookup_value(
|
||||
album_data, 'album_type', 'record_type', 'type', 'primary-type',
|
||||
default=None,
|
||||
)
|
||||
),
|
||||
'total_tracks': _extract_lookup_value(album_data, 'total_tracks', 'track_count', default=0) or 0,
|
||||
}
|
||||
|
||||
|
|
|
|||
132
tests/metadata/test_album_type_normalization.py
Normal file
132
tests/metadata/test_album_type_normalization.py
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
"""Pin `_normalize_album_type` + the legacy fallback's multi-key
|
||||
lookup for `album_type`.
|
||||
|
||||
Discord report (CAL, 2026-05-12): downloading an artist's discography
|
||||
with `$albumtype` in the path template put every release under
|
||||
`Album/` regardless of actual type — EPs, singles, all dumped into
|
||||
`Album/`. Trace: `_build_album_info_legacy` only checked the
|
||||
`album_type` key. Different sources expose the type under different
|
||||
names (Deezer `record_type`, Tidal/MB `type` / `primary-type`, often
|
||||
uppercase). Spotify-shaped lowercase `album_type` was the only path
|
||||
that worked; everything else defaulted to `album`.
|
||||
|
||||
Fix widens the legacy lookup to check `album_type`, `record_type`,
|
||||
`type`, `primary-type` and routes the value through
|
||||
`_normalize_album_type` which lowercases, validates against the
|
||||
canonical token set, and falls back to `album` for unknowns.
|
||||
|
||||
These tests pin both the normalizer (pure helper) and the wired
|
||||
behavior in `_build_album_info_legacy` (smoke).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from core.metadata.album_tracks import (
|
||||
_build_album_info_legacy,
|
||||
_normalize_album_type,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pure helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestNormalizeAlbumType:
|
||||
@pytest.mark.parametrize('raw', ['album', 'ALBUM', 'Album', ' Album '])
|
||||
def test_album_variants_normalize_to_album(self, raw):
|
||||
assert _normalize_album_type(raw) == 'album'
|
||||
|
||||
@pytest.mark.parametrize('raw', ['single', 'SINGLE', 'Single'])
|
||||
def test_single_variants_normalize_to_single(self, raw):
|
||||
assert _normalize_album_type(raw) == 'single'
|
||||
|
||||
@pytest.mark.parametrize('raw', ['ep', 'EP', 'Ep'])
|
||||
def test_ep_variants_normalize_to_ep(self, raw):
|
||||
assert _normalize_album_type(raw) == 'ep'
|
||||
|
||||
def test_compilation_preserved(self):
|
||||
"""Spotify exposes 'compilation' as a distinct type. Preserve
|
||||
it so users with a `$albumtype` template get a separate folder
|
||||
instead of compilations getting demoted into `album/`."""
|
||||
assert _normalize_album_type('compilation') == 'compilation'
|
||||
|
||||
@pytest.mark.parametrize('raw', [None, '', ' '])
|
||||
def test_empty_inputs_return_default(self, raw):
|
||||
assert _normalize_album_type(raw) == 'album'
|
||||
|
||||
def test_unknown_value_returns_default(self):
|
||||
"""Stray strings (e.g. 'mixtape', 'box-set') don't pass through
|
||||
— they'd produce nonsense folder names. Default to album."""
|
||||
assert _normalize_album_type('mixtape') == 'album'
|
||||
assert _normalize_album_type('box-set') == 'album'
|
||||
|
||||
def test_custom_default_honored(self):
|
||||
assert _normalize_album_type('weird', default='single') == 'single'
|
||||
assert _normalize_album_type(None, default='ep') == 'ep'
|
||||
|
||||
def test_non_string_value_handled(self):
|
||||
"""Defensive: source might hand us an int / bool / dict.
|
||||
Should not crash."""
|
||||
assert _normalize_album_type(0) == 'album'
|
||||
assert _normalize_album_type({'name': 'foo'}) == 'album'
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Legacy builder — alt-key support
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLegacyBuilderAlbumTypeAltKeys:
|
||||
"""The bug was sources whose `album_type` lives under an alt key.
|
||||
Pin each known shape produces the correct canonical token."""
|
||||
|
||||
def _build(self, album_data):
|
||||
return _build_album_info_legacy(
|
||||
album_data, album_id='id1', album_name='Test', artist_name='Artist',
|
||||
)
|
||||
|
||||
def test_spotify_shape_album_type_key(self):
|
||||
info = self._build({'album_type': 'single'})
|
||||
assert info['album_type'] == 'single'
|
||||
|
||||
def test_deezer_shape_record_type_key(self):
|
||||
"""Deezer's API returns `record_type` not `album_type`. CAL's
|
||||
EPs returned `record_type='ep'` but the legacy reader missed it
|
||||
and defaulted to album."""
|
||||
info = self._build({'record_type': 'ep'})
|
||||
assert info['album_type'] == 'ep'
|
||||
|
||||
def test_tidal_shape_type_key_uppercase(self):
|
||||
"""Tidal returns `type='ALBUM'/'EP'/'SINGLE'`. Uppercase + alt
|
||||
key = double-miss before the fix."""
|
||||
info = self._build({'type': 'EP'})
|
||||
assert info['album_type'] == 'ep'
|
||||
|
||||
def test_musicbrainz_shape_primary_type_key(self):
|
||||
"""Some flattened MB shapes carry `primary-type` at the top
|
||||
level (typed path handles release-group nesting; legacy hits
|
||||
the flattened cases)."""
|
||||
info = self._build({'primary-type': 'Single'})
|
||||
assert info['album_type'] == 'single'
|
||||
|
||||
def test_album_type_wins_when_multiple_keys_present(self):
|
||||
"""When both `album_type` AND `record_type` exist, prefer the
|
||||
Spotify-canonical key. `_extract_lookup_value` checks left to
|
||||
right — pin that ordering."""
|
||||
info = self._build({'album_type': 'album', 'record_type': 'ep'})
|
||||
assert info['album_type'] == 'album'
|
||||
|
||||
def test_no_type_key_defaults_to_album(self):
|
||||
"""Source response with no type field at all → defaults to
|
||||
`album` (legacy behavior preserved for genuinely-missing data)."""
|
||||
info = self._build({'name': 'Some Album'})
|
||||
assert info['album_type'] == 'album'
|
||||
|
||||
def test_unknown_type_value_defaults_to_album(self):
|
||||
"""`type='Mixtape'` → not in canonical set → default. Prevents
|
||||
a stray value from poisoning the path template."""
|
||||
info = self._build({'type': 'Mixtape'})
|
||||
assert info['album_type'] == 'album'
|
||||
|
|
@ -3413,6 +3413,11 @@ function closeHelperSearch() {
|
|||
// projects that span multiple commits before shipping. Strip the flag at
|
||||
// release time and add a real `date:` line at the top of the version block.
|
||||
const WHATS_NEW = {
|
||||
'2.5.2': [
|
||||
// --- post-release patch work on the 2.5.2 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.5.2 patch work' },
|
||||
{ title: '$albumtype Folder Template Now Splits EPs / Singles For Non-Spotify Sources', desc: 'discord report (cal): downloading an artist\'s discography with `$albumtype` in the path template put every release under `Album/` regardless of actual type — eps, singles, all dumped into the album folder. trace: the legacy duck-typed album-info builder at `core/metadata/album_tracks.py:_build_album_info_legacy` only checked the `album_type` key. spotify uses `album_type` (lowercase) so spotify discographies worked. but deezer\'s api uses `record_type`, tidal uses `type` (uppercase ALBUM/EP/SINGLE), and some flattened musicbrainz shapes use `primary-type` — none of those matched, all defaulted to `album`. fix: widen the legacy lookup to check `album_type` / `record_type` / `type` / `primary-type` and route the value through a new pure `_normalize_album_type` helper that lowercases + validates against the canonical token set (`album` / `single` / `ep` / `compilation`) and falls back to `album` for unknowns. typed-converter path for spotify / deezer / itunes / discogs / musicbrainz / hydrabase / qobuz unchanged — they were already correct. tidal users were the main offender (no typed converter for dict-shaped tidal data). 24 new tests pin: case-insensitive normalization for each canonical type, compilation preserved (spotify supports it), unknown values default to album, defensive against none / empty / non-string inputs, multi-key precedence (`album_type` wins over `record_type`), each known source shape produces correct token.', page: 'tools' },
|
||||
],
|
||||
'2.5.1': [
|
||||
// --- May 12, 2026 — 2.5.1 release ---
|
||||
{ date: 'May 12, 2026 — 2.5.1 release' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue