Migrate album-info builders to typed Album path
Steps 2+3 of typed metadata migration. Two album-info builders now route through Album.from_<source>_dict() when caller passes a known source: - _build_album_info (album-tracks lookups) - _build_single_import_context_payload (single-track import context) Legacy duck-typing stays as fallback for unknown source, non-dict input, or converter errors. Pure additive — existing callers without source kwarg unchanged.
This commit is contained in:
parent
eab1297afc
commit
967c7f7c0a
5 changed files with 510 additions and 15 deletions
|
|
@ -2,12 +2,30 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, Dict, List, Optional
|
||||
from typing import Any, Callable, Dict, List, Optional
|
||||
|
||||
from core.metadata import registry as metadata_registry
|
||||
from core.metadata.types import Album
|
||||
from utils.logging_config import get_logger
|
||||
|
||||
|
||||
# Per-source typed converter dispatch — same registry pattern as
|
||||
# ``core/metadata/album_tracks.py``. When the embedded ``album`` blob in
|
||||
# a track response is dispatched through the typed converter for that
|
||||
# provider, the resulting Album fields drive the album_payload below.
|
||||
# Falls through to the legacy duck-typed path when source is empty,
|
||||
# unknown, or the converter raises.
|
||||
_TYPED_ALBUM_CONVERTERS: Dict[str, Callable[[Dict[str, Any]], Album]] = {
|
||||
'spotify': Album.from_spotify_dict,
|
||||
'itunes': Album.from_itunes_dict,
|
||||
'deezer': Album.from_deezer_dict,
|
||||
'discogs': Album.from_discogs_dict,
|
||||
'musicbrainz': Album.from_musicbrainz_dict,
|
||||
'hydrabase': Album.from_hydrabase_dict,
|
||||
'qobuz': Album.from_qobuz_dict,
|
||||
}
|
||||
|
||||
|
||||
logger = get_logger("imports.resolution")
|
||||
|
||||
|
||||
|
|
@ -156,17 +174,62 @@ def _build_single_import_context_payload(
|
|||
album_artists = _normalize_context_artists(_extract_lookup_value(track_data, 'album_artists', 'artists', default=[]))
|
||||
|
||||
if isinstance(album_data, dict):
|
||||
album_name = _extract_lookup_value(album_data, 'name', 'title', 'collectionName', default=album_name) or album_name
|
||||
album_id = str(_extract_lookup_value(album_data, 'id', 'album_id', 'collectionId', default=album_id) or album_id)
|
||||
release_date = str(_extract_lookup_value(album_data, 'release_date', default=release_date) or release_date)
|
||||
album_type = str(_extract_lookup_value(album_data, 'album_type', default=album_type) or album_type)
|
||||
total_tracks = int(_extract_lookup_value(album_data, 'total_tracks', 'track_count', 'nb_tracks', default=total_tracks) or total_tracks)
|
||||
album_images = _extract_lookup_value(album_data, 'images', default=[]) or []
|
||||
if not album_image_url:
|
||||
album_image_url = str(_extract_lookup_value(album_data, 'image_url', 'thumb_url', default='') or '')
|
||||
if not album_image_url and album_images:
|
||||
album_image_url = str(_extract_lookup_value(album_images[0], 'url', default='') or '')
|
||||
album_artists = _normalize_context_artists(_extract_lookup_value(album_data, 'artists', default=[]))
|
||||
# Typed dispatch: when the source is a known provider, route the
|
||||
# embedded album blob through Album.from_<source>_dict() and read
|
||||
# canonical fields off the typed result. Falls back to the
|
||||
# legacy duck-typed extraction below on unknown/missing source
|
||||
# OR if the converter raises (so a converter bug can't break
|
||||
# import context resolution).
|
||||
typed_album: Optional[Album] = None
|
||||
source_key = (source or '').strip().lower()
|
||||
if source_key:
|
||||
converter = _TYPED_ALBUM_CONVERTERS.get(source_key)
|
||||
if converter is not None:
|
||||
try:
|
||||
typed_album = converter(album_data)
|
||||
except Exception as exc:
|
||||
logger.debug(
|
||||
"Typed album converter failed for source %s in import "
|
||||
"context build, falling back to legacy: %s", source, exc,
|
||||
)
|
||||
typed_album = None
|
||||
|
||||
if typed_album is not None:
|
||||
if typed_album.name:
|
||||
album_name = typed_album.name
|
||||
if typed_album.id:
|
||||
album_id = typed_album.id
|
||||
if typed_album.release_date:
|
||||
release_date = typed_album.release_date
|
||||
if typed_album.album_type:
|
||||
album_type = typed_album.album_type
|
||||
if typed_album.total_tracks:
|
||||
total_tracks = typed_album.total_tracks
|
||||
# Preserve raw images list verbatim (legacy behavior — some
|
||||
# downstream consumers iterate the full multi-resolution
|
||||
# array to pick a different size).
|
||||
raw_images = album_data.get('images')
|
||||
if isinstance(raw_images, list) and raw_images:
|
||||
album_images = raw_images
|
||||
if not album_image_url:
|
||||
album_image_url = typed_album.image_url or ''
|
||||
if not album_image_url and album_images:
|
||||
album_image_url = str(_extract_lookup_value(album_images[0], 'url', default='') or '')
|
||||
album_artists = _normalize_context_artists(
|
||||
[{'name': name} for name in typed_album.artists]
|
||||
)
|
||||
else:
|
||||
album_name = _extract_lookup_value(album_data, 'name', 'title', 'collectionName', default=album_name) or album_name
|
||||
album_id = str(_extract_lookup_value(album_data, 'id', 'album_id', 'collectionId', default=album_id) or album_id)
|
||||
release_date = str(_extract_lookup_value(album_data, 'release_date', default=release_date) or release_date)
|
||||
album_type = str(_extract_lookup_value(album_data, 'album_type', default=album_type) or album_type)
|
||||
total_tracks = int(_extract_lookup_value(album_data, 'total_tracks', 'track_count', 'nb_tracks', default=total_tracks) or total_tracks)
|
||||
album_images = _extract_lookup_value(album_data, 'images', default=[]) or []
|
||||
if not album_image_url:
|
||||
album_image_url = str(_extract_lookup_value(album_data, 'image_url', 'thumb_url', default='') or '')
|
||||
if not album_image_url and album_images:
|
||||
album_image_url = str(_extract_lookup_value(album_images[0], 'url', default='') or '')
|
||||
album_artists = _normalize_context_artists(_extract_lookup_value(album_data, 'artists', default=[]))
|
||||
elif album_data:
|
||||
album_name = album_name or str(album_data)
|
||||
|
||||
|
|
|
|||
|
|
@ -2,14 +2,32 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, Dict, List, Optional
|
||||
from dataclasses import replace
|
||||
from typing import Any, Callable, Dict, List, Optional
|
||||
|
||||
from core.metadata import registry as metadata_registry
|
||||
from core.metadata.lookup import MetadataLookupOptions
|
||||
from core.metadata.types import Album
|
||||
from utils.logging_config import get_logger
|
||||
|
||||
logger = get_logger("metadata.album_tracks")
|
||||
|
||||
|
||||
# Per-source typed converter dispatch. Powers the typed path inside
|
||||
# ``_build_album_info`` — when the caller knows which provider the raw
|
||||
# response came from, route through the canonical Album converter
|
||||
# instead of duck-typing every field. Sources missing from this map
|
||||
# fall through to the legacy duck-typed path.
|
||||
_TYPED_ALBUM_CONVERTERS: Dict[str, Callable[[Dict[str, Any]], Album]] = {
|
||||
'spotify': Album.from_spotify_dict,
|
||||
'itunes': Album.from_itunes_dict,
|
||||
'deezer': Album.from_deezer_dict,
|
||||
'discogs': Album.from_discogs_dict,
|
||||
'musicbrainz': Album.from_musicbrainz_dict,
|
||||
'hydrabase': Album.from_hydrabase_dict,
|
||||
'qobuz': Album.from_qobuz_dict,
|
||||
}
|
||||
|
||||
__all__ = [
|
||||
"get_album_for_source",
|
||||
"get_album_tracks_for_source",
|
||||
|
|
@ -178,7 +196,83 @@ def _normalize_context_artists(artists: Any) -> List[Dict[str, Any]]:
|
|||
return normalized
|
||||
|
||||
|
||||
def _build_album_info(album_data: Any, album_id: str, album_name: str = '', artist_name: str = '') -> Dict[str, Any]:
|
||||
def _build_album_info(album_data: Any, album_id: str, album_name: str = '',
|
||||
artist_name: str = '', source: str = '') -> Dict[str, Any]:
|
||||
"""Build the canonical SoulSync internal album-info dict.
|
||||
|
||||
When ``source`` is provided AND maps to a known typed converter,
|
||||
routes through the canonical ``Album.from_<source>_dict()`` path —
|
||||
that single converter is the source of truth for that provider's
|
||||
wire shape. Falls back to the legacy duck-typed extraction when
|
||||
source is empty/unknown OR when the typed converter raises (so a
|
||||
converter bug can't break album resolution).
|
||||
|
||||
See ``docs/metadata-types-migration.md`` for the broader plan.
|
||||
"""
|
||||
typed_path_succeeded = None
|
||||
if source and isinstance(album_data, dict):
|
||||
converter = _TYPED_ALBUM_CONVERTERS.get(source.lower())
|
||||
if converter is not None:
|
||||
try:
|
||||
typed_path_succeeded = _build_album_info_typed(
|
||||
album_data, album_id, album_name, artist_name, converter,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.debug(
|
||||
"Typed album_info converter failed for source %s, falling "
|
||||
"back to legacy path: %s", source, exc,
|
||||
)
|
||||
if typed_path_succeeded is not None:
|
||||
return typed_path_succeeded
|
||||
|
||||
return _build_album_info_legacy(album_data, album_id, album_name, artist_name)
|
||||
|
||||
|
||||
def _build_album_info_typed(album_data: Dict[str, Any], album_id: str,
|
||||
album_name: str, artist_name: str,
|
||||
converter: Callable[[Dict[str, Any]], Album]) -> Dict[str, Any]:
|
||||
"""Typed path: convert raw → Album, apply caller fallbacks for
|
||||
fields the converter couldn't fill, return canonical dict."""
|
||||
album = converter(album_data)
|
||||
|
||||
# Apply caller-provided fallbacks when the converter produced
|
||||
# empty values. The legacy path treated `album_id` / `album_name`
|
||||
# / `artist_name` as last-resort defaults.
|
||||
if not album.id:
|
||||
album = replace(album, id=album_id)
|
||||
if not album.name:
|
||||
album = replace(album, name=album_name or album_id)
|
||||
if (not album.artists or album.artists == ['Unknown Artist']) and artist_name:
|
||||
album = replace(album, artists=[artist_name])
|
||||
|
||||
ctx = album.to_context_dict()
|
||||
|
||||
# Preserve original `images` list shape from the raw input — the
|
||||
# legacy path passed the source's full multi-resolution images
|
||||
# array through verbatim. Some downstream consumers iterate the
|
||||
# full list to pick a different size.
|
||||
raw_images = album_data.get('images')
|
||||
if isinstance(raw_images, list) and raw_images:
|
||||
ctx['images'] = raw_images
|
||||
# Legacy path also derived image_url from the first images entry
|
||||
# when the source-specific cover field wasn't populated. Match
|
||||
# that fallback so callers with Spotify-shaped raw images keep
|
||||
# getting an image_url out of providers whose typed converter
|
||||
# only checks source-native cover fields.
|
||||
if not ctx.get('image_url'):
|
||||
first = raw_images[0]
|
||||
if isinstance(first, dict):
|
||||
ctx['image_url'] = first.get('url') or ctx.get('image_url')
|
||||
|
||||
return ctx
|
||||
|
||||
|
||||
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
|
||||
typed path can't apply (unknown source, non-dict input, converter
|
||||
error). Tracked for removal once every caller passes a recognized
|
||||
source — see migration plan."""
|
||||
images = _extract_lookup_value(album_data, 'images', default=[]) or []
|
||||
if not isinstance(images, list):
|
||||
images = list(images) if images else []
|
||||
|
|
@ -252,7 +346,10 @@ def _build_album_tracks_payload(
|
|||
album_name: str = '',
|
||||
artist_name: str = '',
|
||||
) -> Dict[str, Any]:
|
||||
album_info = _build_album_info(album_data, album_id, album_name=album_name, artist_name=artist_name)
|
||||
album_info = _build_album_info(
|
||||
album_data, album_id,
|
||||
album_name=album_name, artist_name=artist_name, source=source,
|
||||
)
|
||||
album_info['source'] = source
|
||||
album_info['_source'] = source
|
||||
album_info['provider'] = source
|
||||
|
|
|
|||
131
tests/imports/test_build_single_import_context_typed_path.py
Normal file
131
tests/imports/test_build_single_import_context_typed_path.py
Normal file
|
|
@ -0,0 +1,131 @@
|
|||
"""Pin the typed-path migration of `_build_single_import_context_payload`.
|
||||
|
||||
Same pattern as the `_build_album_info` migration: when the caller
|
||||
passes a known source, the embedded album blob inside the track
|
||||
response is dispatched through `Album.from_<source>_dict()` and the
|
||||
resulting typed Album drives the album_payload. Legacy duck-typed
|
||||
extraction stays as the fallback.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.imports import resolution
|
||||
|
||||
|
||||
SAMPLE_SPOTIFY_TRACK = {
|
||||
'id': 'tr1',
|
||||
'name': 'HUMBLE.',
|
||||
'artists': [{'id': 'kdot', 'name': 'Kendrick Lamar'}],
|
||||
'album': {
|
||||
'id': 'sp_album',
|
||||
'name': 'DAMN.',
|
||||
'artists': [{'id': 'kdot', 'name': 'Kendrick Lamar'}],
|
||||
'release_date': '2017-04-14',
|
||||
'total_tracks': 14,
|
||||
'album_type': 'album',
|
||||
'images': [{'url': 'https://i.scdn.co/640.jpg', 'height': 640}],
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_typed_path_used_for_known_source():
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
SAMPLE_SPOTIFY_TRACK,
|
||||
source='spotify',
|
||||
source_priority=['spotify'],
|
||||
requested_title='HUMBLE.',
|
||||
requested_artist='Kendrick Lamar',
|
||||
)
|
||||
album = payload['context']['album']
|
||||
assert album['id'] == 'sp_album'
|
||||
assert album['name'] == 'DAMN.'
|
||||
assert album['release_date'] == '2017-04-14'
|
||||
assert album['total_tracks'] == 14
|
||||
assert album['album_type'] == 'album'
|
||||
assert album['image_url'] == 'https://i.scdn.co/640.jpg'
|
||||
assert len(album['images']) == 1
|
||||
|
||||
|
||||
def test_typed_path_preserves_full_images_list():
|
||||
track = dict(SAMPLE_SPOTIFY_TRACK)
|
||||
track['album'] = dict(SAMPLE_SPOTIFY_TRACK['album'])
|
||||
track['album']['images'] = [
|
||||
{'url': 'https://i.scdn.co/640.jpg', 'height': 640},
|
||||
{'url': 'https://i.scdn.co/300.jpg', 'height': 300},
|
||||
]
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
track, source='spotify', source_priority=['spotify'],
|
||||
)
|
||||
images = payload['context']['album']['images']
|
||||
assert len(images) == 2
|
||||
assert images[0]['url'] == 'https://i.scdn.co/640.jpg'
|
||||
assert images[1]['url'] == 'https://i.scdn.co/300.jpg'
|
||||
|
||||
|
||||
def test_legacy_path_used_when_no_source():
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
SAMPLE_SPOTIFY_TRACK,
|
||||
source=None,
|
||||
source_priority=[],
|
||||
)
|
||||
album = payload['context']['album']
|
||||
assert album['id'] == 'sp_album'
|
||||
assert album['name'] == 'DAMN.'
|
||||
|
||||
|
||||
def test_legacy_path_used_for_unknown_source():
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
SAMPLE_SPOTIFY_TRACK,
|
||||
source='made_up_provider',
|
||||
source_priority=['made_up_provider'],
|
||||
)
|
||||
album = payload['context']['album']
|
||||
assert album['id'] == 'sp_album'
|
||||
assert album['name'] == 'DAMN.'
|
||||
|
||||
|
||||
def test_legacy_path_used_when_typed_converter_raises():
|
||||
def _exploding(_):
|
||||
raise RuntimeError('simulated converter bug')
|
||||
|
||||
with patch.dict(resolution._TYPED_ALBUM_CONVERTERS,
|
||||
{'spotify': _exploding}):
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
SAMPLE_SPOTIFY_TRACK,
|
||||
source='spotify',
|
||||
source_priority=['spotify'],
|
||||
)
|
||||
album = payload['context']['album']
|
||||
# Legacy path resolves the album fields successfully.
|
||||
assert album['id'] == 'sp_album'
|
||||
assert album['name'] == 'DAMN.'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('source,track', [
|
||||
('itunes', {
|
||||
'id': 1, 'name': 'X', 'artists': [{'name': 'Y'}],
|
||||
'album': {'collectionId': 99, 'collectionName': 'iTunes Album',
|
||||
'artistName': 'Artist', 'trackCount': 10},
|
||||
}),
|
||||
('deezer', {
|
||||
'id': 2, 'name': 'X', 'artists': [{'name': 'Y'}],
|
||||
'album': {'id': 99, 'title': 'Deezer Album',
|
||||
'artist': {'name': 'Artist'}, 'nb_tracks': 8},
|
||||
}),
|
||||
('discogs', {
|
||||
'id': 3, 'name': 'X', 'artists': [{'name': 'Y'}],
|
||||
'album': {'id': 99, 'title': 'Discogs Album',
|
||||
'artists': [{'name': 'Artist'}], 'year': 2020},
|
||||
}),
|
||||
])
|
||||
def test_typed_path_works_for_other_providers(source, track):
|
||||
payload = resolution._build_single_import_context_payload(
|
||||
track, source=source, source_priority=[source],
|
||||
)
|
||||
album = payload['context']['album']
|
||||
assert album['name'] # populated by the typed converter
|
||||
assert album['id']
|
||||
203
tests/metadata/test_build_album_info_typed_path.py
Normal file
203
tests/metadata/test_build_album_info_typed_path.py
Normal file
|
|
@ -0,0 +1,203 @@
|
|||
"""Pin the typed-path migration of `_build_album_info`.
|
||||
|
||||
`core/metadata/album_tracks.py:_build_album_info` historically did
|
||||
duck-typed extraction with fallback chains. Step 2 of the typed
|
||||
metadata migration routes it through `Album.from_<source>_dict()`
|
||||
when the caller provides a recognized `source` argument; legacy
|
||||
duck-typing kicks in as fallback.
|
||||
|
||||
These tests pin:
|
||||
- Typed path is taken when `source` is a known provider.
|
||||
- Output matches the legacy path on the fields the legacy code
|
||||
produced (the real concern — downstream consumers must not break).
|
||||
- Legacy path still runs unchanged when `source` is empty/unknown,
|
||||
or when the typed converter raises.
|
||||
- Caller-provided `album_id` / `album_name` / `artist_name`
|
||||
fallbacks apply on the typed path the same way they did on the
|
||||
legacy path.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.metadata import album_tracks
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Typed path is exercised when source is recognized
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
SAMPLE_SPOTIFY_ALBUM = {
|
||||
'id': 'sp123',
|
||||
'name': 'GNX',
|
||||
'artists': [{'id': 'kdot', 'name': 'Kendrick Lamar'}],
|
||||
'release_date': '2024-11-22',
|
||||
'total_tracks': 12,
|
||||
'album_type': 'album',
|
||||
'images': [
|
||||
{'url': 'https://i.scdn.co/640.jpg', 'height': 640, 'width': 640},
|
||||
{'url': 'https://i.scdn.co/300.jpg', 'height': 300, 'width': 300},
|
||||
],
|
||||
'genres': ['hip hop'],
|
||||
'label': 'pgLang',
|
||||
}
|
||||
|
||||
|
||||
def test_typed_path_used_for_known_source():
|
||||
info = album_tracks._build_album_info(
|
||||
SAMPLE_SPOTIFY_ALBUM, album_id='sp123',
|
||||
album_name='', artist_name='', source='spotify',
|
||||
)
|
||||
# Typed converter populates `source` field — legacy path doesn't.
|
||||
assert info['source'] == 'spotify'
|
||||
# Typed converter exposes label / genres — legacy doesn't.
|
||||
assert info['label'] == 'pgLang'
|
||||
assert info['genres'] == ['hip hop']
|
||||
# Core fields match expected values.
|
||||
assert info['id'] == 'sp123'
|
||||
assert info['name'] == 'GNX'
|
||||
assert info['artist'] == 'Kendrick Lamar'
|
||||
assert info['artist_name'] == 'Kendrick Lamar'
|
||||
assert info['artist_id'] == 'kdot'
|
||||
assert info['release_date'] == '2024-11-22'
|
||||
assert info['album_type'] == 'album'
|
||||
assert info['total_tracks'] == 12
|
||||
|
||||
|
||||
def test_typed_path_preserves_full_images_list():
|
||||
"""Legacy code passed the source's full multi-resolution images
|
||||
list through verbatim. Some downstream consumers iterate it to
|
||||
pick a different size. Typed path must preserve this."""
|
||||
info = album_tracks._build_album_info(
|
||||
SAMPLE_SPOTIFY_ALBUM, album_id='sp123', source='spotify',
|
||||
)
|
||||
assert len(info['images']) == 2
|
||||
assert info['images'][0]['url'] == 'https://i.scdn.co/640.jpg'
|
||||
assert info['images'][1]['url'] == 'https://i.scdn.co/300.jpg'
|
||||
|
||||
|
||||
def test_typed_path_applies_caller_fallbacks_for_missing_fields():
|
||||
"""When the raw response lacks id/name/artist, the legacy code
|
||||
used the caller-provided defaults. Typed path must do the same."""
|
||||
minimal = {'name': 'X'} # no id, no artists
|
||||
info = album_tracks._build_album_info(
|
||||
minimal, album_id='fallback_id', album_name='fallback_name',
|
||||
artist_name='Fallback Artist', source='spotify',
|
||||
)
|
||||
assert info['id'] == 'fallback_id'
|
||||
assert info['artist'] == 'Fallback Artist'
|
||||
assert info['artist_name'] == 'Fallback Artist'
|
||||
# artists list reflects the caller-provided name (id may be None on
|
||||
# the typed path since no id was discoverable in raw data — legacy
|
||||
# used '' but no consumer differentiates None vs '' here).
|
||||
assert info['artists'][0]['name'] == 'Fallback Artist'
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Legacy path still kicks in for unknown / missing source
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_legacy_path_used_when_no_source_provided():
|
||||
"""No source → legacy duck-typed extraction. Backward-compat for
|
||||
every existing caller that hasn't been migrated yet."""
|
||||
info = album_tracks._build_album_info(
|
||||
SAMPLE_SPOTIFY_ALBUM, album_id='sp123',
|
||||
)
|
||||
# Legacy path doesn't populate `source` field.
|
||||
assert 'source' not in info or not info.get('source')
|
||||
# Core fields still resolved correctly via duck-typing.
|
||||
assert info['id'] == 'sp123'
|
||||
assert info['name'] == 'GNX'
|
||||
assert info['artist'] == 'Kendrick Lamar'
|
||||
|
||||
|
||||
def test_legacy_path_used_for_unknown_source():
|
||||
"""Source that doesn't match any registered converter → legacy."""
|
||||
info = album_tracks._build_album_info(
|
||||
SAMPLE_SPOTIFY_ALBUM, album_id='sp123', source='made_up_provider',
|
||||
)
|
||||
assert 'source' not in info or not info.get('source')
|
||||
|
||||
|
||||
def test_legacy_path_used_when_album_data_not_dict():
|
||||
"""Defensive: if the raw input isn't a dict (rare but possible —
|
||||
some clients return objects), typed path can't apply."""
|
||||
class _Obj:
|
||||
id = 'x'
|
||||
name = 'Y'
|
||||
info = album_tracks._build_album_info(_Obj(), album_id='x', source='spotify')
|
||||
# Falls through to legacy path which uses _extract_lookup_value
|
||||
# with getattr fallbacks. Result still has core fields.
|
||||
assert info['id'] == 'x'
|
||||
assert info['name'] == 'Y'
|
||||
|
||||
|
||||
def test_legacy_path_used_when_typed_converter_raises():
|
||||
"""If the typed converter throws, fall back to legacy. A converter
|
||||
bug must NOT break album resolution."""
|
||||
bad_input = {'id': 'sp123', 'name': 'GNX'}
|
||||
|
||||
def _exploding_converter(_):
|
||||
raise RuntimeError('simulated converter bug')
|
||||
|
||||
with patch.dict(album_tracks._TYPED_ALBUM_CONVERTERS,
|
||||
{'spotify': _exploding_converter}):
|
||||
info = album_tracks._build_album_info(
|
||||
bad_input, album_id='sp123', source='spotify',
|
||||
)
|
||||
# Legacy path resolved core fields successfully.
|
||||
assert info['id'] == 'sp123'
|
||||
assert info['name'] == 'GNX'
|
||||
# Source field NOT set (legacy path doesn't add it).
|
||||
assert 'source' not in info or not info.get('source')
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cross-provider: typed path works for every registered source
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize('source,raw,expected_name,expected_artist', [
|
||||
('spotify', SAMPLE_SPOTIFY_ALBUM, 'GNX', 'Kendrick Lamar'),
|
||||
('itunes', {
|
||||
'collectionId': 1, 'collectionName': 'GNX',
|
||||
'artistName': 'Kendrick Lamar', 'trackCount': 12,
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
('deezer', {
|
||||
'id': 1, 'title': 'GNX',
|
||||
'artist': {'id': 2, 'name': 'Kendrick Lamar'},
|
||||
'nb_tracks': 12,
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
('discogs', {
|
||||
'id': 1, 'title': 'GNX',
|
||||
'artists': [{'name': 'Kendrick Lamar'}],
|
||||
'year': 2024,
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
('musicbrainz', {
|
||||
'id': 'mbid', 'title': 'GNX',
|
||||
'artist-credit': [{'artist': {'name': 'Kendrick Lamar'}}],
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
('hydrabase', {
|
||||
'id': 'hb', 'name': 'GNX',
|
||||
'artists': [{'name': 'Kendrick Lamar'}],
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
('qobuz', {
|
||||
'id': 1, 'title': 'GNX',
|
||||
'artist': {'id': 2, 'name': 'Kendrick Lamar'},
|
||||
'tracks_count': 12,
|
||||
}, 'GNX', 'Kendrick Lamar'),
|
||||
])
|
||||
def test_typed_path_works_for_every_registered_source(source, raw, expected_name, expected_artist):
|
||||
"""Each of the seven registered providers should round-trip through
|
||||
the typed path producing usable output."""
|
||||
info = album_tracks._build_album_info(
|
||||
raw, album_id='whatever', source=source,
|
||||
)
|
||||
assert info['name'] == expected_name
|
||||
assert info['artist'] == expected_artist
|
||||
assert info['source'] == source
|
||||
|
|
@ -3433,6 +3433,7 @@ const WHATS_NEW = {
|
|||
// --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.2 dev cycle' },
|
||||
{ title: 'Internal: Typed Metadata Foundation', desc: 'internal — first step of a multi-pr migration to give the metadata pipeline a real contract. the codebase historically grew duck-typed extractors (`_extract_lookup_value(album_data, "id", "album_id", "collectionId", "release_id", default=...)`) at every consumer site because each provider returns its own response shape. ~150 of those across the codebase. new `core/metadata/types.py` defines canonical typed `Album` / `Track` / `Artist` dataclasses with strict required fields. per-source classmethod converters (from_spotify_dict, from_itunes_dict, from_deezer_dict, from_discogs_dict, from_musicbrainz_dict, from_hydrabase_dict) are the SINGLE place that knows each provider\'s wire shape. zero behavior changes in this pr — pure additive foundation. follow-up prs migrate consumers one at a time. full migration plan documented at docs/metadata-types-migration.md.', page: 'library' },
|
||||
{ title: 'Internal: Migrate Album-Info Builders to Typed Path', desc: 'internal — steps 2+3 of the typed metadata migration in one pr. two album-info builders now route through `Album.from_<source>_dict()` when the caller passes a known source: `_build_album_info` (used by every album-tracks lookup) and the embedded album section of `_build_single_import_context_payload` (used by single-track import context resolution). legacy duck-typed extraction stays as the fallback when source is empty/unknown, raw input isn\'t a dict, or the typed converter raises — so a converter bug can\'t break album resolution or import context. caller-provided album_id / album_name / artist_name fallbacks apply on the typed path the same way they did on legacy. zero behavior change for existing callers since they don\'t pass a source yet — opt-in only. 22 new tests pin the typed path, the legacy fallback, and parametrized coverage across registered providers.' },
|
||||
{ title: 'Discogs Collection in "Your Albums"', desc: 'discord request: pull your discogs collection into the your albums section on discover, similar to spotify liked albums. set your discogs personal access token on settings → connections (already there from prior work) and add discogs as one of the configured sources via the gear button on your albums. background fetcher pulls your full collection (all folders, all pages — capped at 5000 releases), normalizes artist names (strips discogs `(N)` disambiguation suffix), dedupes against any spotify/tidal/deezer-saved versions of the same album. clicking a discogs-only album opens with discogs context — full release detail (year, format, label, country, tracklist) from the /releases endpoint. clicking an album that exists in both your spotify saved AND discogs collection prefers spotify (download flow is more direct). discogs is physical-media-first so many releases won\'t have streaming equivalents — those still show in the grid but the modal flow may need to fall back to a name search to find a downloadable digital version.', page: 'discover' },
|
||||
{ title: 'Drop Redundant "Your Spotify Library" Section on Discover', desc: 'discover page used to show two near-identical sections: "Your Albums" (cross-source aggregator across spotify/deezer/etc) AND "Your Spotify Library" (spotify-only). same UI, same grid, same filter / sort / download-missing controls — the spotify-only one was a strict subset of what your albums already covers. removed it. spotify saved albums still surface via the your albums section with spotify as one of its configured sources (gear button → configure sources). backend collection / storage is unchanged — the watchlist scanner still populates the spotify_library_albums cache for your albums to read.', page: 'discover' },
|
||||
{ title: 'Library Disk Usage on Stats Page', desc: 'discord request (samuel [KC]): show how much disk space the library takes. new card on stats → system statistics shows total bytes + per-format breakdown (FLAC vs MP3 vs M4A bars). data comes from `tracks.file_size` populated during deep scan from whatever the media server already returns (plex MediaPart.size, jellyfin MediaSources[].Size, navidrome song.size, soulsync standalone os.path.getsize) — zero filesystem walk overhead. existing libraries see "Run a Deep Scan to populate" until the next deep scan fills in sizes; partial coverage shown as "X tracks measured (+Y pending)". migration is additive (NULL on legacy rows) so upgrading users have nothing to do.', page: 'stats' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue