diff --git a/core/downloads/candidates.py b/core/downloads/candidates.py index d9f20777..1edca30d 100644 --- a/core/downloads/candidates.py +++ b/core/downloads/candidates.py @@ -205,6 +205,21 @@ def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, 'artists': _fallback_album_artists } + # #915: parity with Reorganize / manual Enrich. If the album context is lean + # (no release_date) and the user's PRIMARY metadata source isn't Spotify, hydrate + # it from that source — the same place a reorganize reads — so the download's + # $year folder, release_date and album_type match instead of dropping the year / + # defaulting to YYYY-01-01 and forcing a manual reorganize afterwards. + try: + from core.downloads.track_metadata_backfill import backfill_album_context_from_source + from core.metadata import registry as _meta_registry + from core.metadata.album_tracks import get_album_for_source as _get_album_for_source + backfill_album_context_from_source( + spotify_album_context, _meta_registry.get_primary_source(), _get_album_for_source, + ) + except Exception as _bf_err: # noqa: BLE001 — never let backfill break a download + logger.debug("[Context] primary-source album backfill skipped: %s", _bf_err) + download_payload = candidate.__dict__ username = download_payload.get('username') diff --git a/core/downloads/track_metadata_backfill.py b/core/downloads/track_metadata_backfill.py index da7fac0e..82474d65 100644 --- a/core/downloads/track_metadata_backfill.py +++ b/core/downloads/track_metadata_backfill.py @@ -93,6 +93,56 @@ def _backfill_album_context( album_context['image_url'] = first['url'] +# Placeholder album ids used when no real source album id is known — never queryable. +_SENTINEL_ALBUM_IDS = {'explicit_album', 'from_sync_modal', ''} + + +def backfill_album_context_from_source( + album_context: Dict[str, Any], + primary_source: Optional[str], + get_album_for_source_fn: Any, +) -> bool: + """Hydrate a lean album context from the user's PRIMARY metadata source (#915). + + Post-processing's only album backfill (:func:`hydrate_download_metadata`) goes through + ``spotify_client.get_track_details`` — Spotify-only. An iTunes/Deezer-primary user's + download therefore kept a lean context (no ``release_date``), so the path dropped the + ``$year`` and the date defaulted to ``YYYY-01-01`` — until they ran a Reorganize, which + reads the full album from the PRIMARY source. This closes that gap by doing the same: + fetch the full album from the primary source and backfill, so a download's pathing/tags + match what a later reorganize would produce. + + ``get_album_for_source_fn(source, album_id)`` is injected (the real one is + ``core.metadata.album_tracks.get_album_for_source``) so this stays pure + testable. + No-op when: the context is already complete; the primary source is spotify (the existing + track-details path covers it); or no real source album id is present. Returns True when + it filled anything. Never raises — a backfill failure must not break a download. + """ + if not isinstance(album_context, dict) or not _album_is_lean(album_context): + return False + if not primary_source or primary_source == 'spotify': + return False + album_id = album_context.get('id') + if not album_id or str(album_id) in _SENTINEL_ALBUM_IDS: + return False + try: + album = get_album_for_source_fn(primary_source, str(album_id)) + except Exception as e: # noqa: BLE001 — defensive: never let backfill break a download + logger.warning("[Context] primary-source (%s) album backfill failed: %s", primary_source, e) + return False + if not isinstance(album, dict): + return False + before = album_context.get('release_date') + _backfill_album_context(album_context, {'album': album}) + if album_context.get('release_date') and album_context.get('release_date') != before: + logger.info( + "[Context] Hydrated lean album context from primary source %s " + "(release_date=%r, total_tracks=%r)", + primary_source, album_context.get('release_date'), album_context.get('total_tracks'), + ) + return True + + def hydrate_download_metadata( track: Any, track_info: Any, @@ -172,4 +222,5 @@ def hydrate_download_metadata( __all__ = [ 'ResolvedTrackMetadata', 'hydrate_download_metadata', + 'backfill_album_context_from_source', ] diff --git a/tests/downloads/test_track_metadata_backfill.py b/tests/downloads/test_track_metadata_backfill.py index 99f77351..6eb74ed5 100644 --- a/tests/downloads/test_track_metadata_backfill.py +++ b/tests/downloads/test_track_metadata_backfill.py @@ -507,5 +507,73 @@ def test_api_called_at_most_once_per_invocation(): assert client.get_track_details.call_count == 1 +# ── #915: source-aware album-context backfill (parity with Reorganize/Enrich) ── +from core.downloads.track_metadata_backfill import backfill_album_context_from_source # noqa: E402 + + +def _lean_ctx(album_id="itunes-123"): + return {"id": album_id, "name": "Big OST", "release_date": "", "total_tracks": 0, "album_type": "album"} + + +def _itunes_album(**over): + base = {"id": "itunes-123", "name": "Big OST", "release_date": "2024-04-17", + "total_tracks": 70, "album_type": "album"} + base.update(over) + return base + + +def test_backfill_hydrates_lean_context_from_primary_source(): + ctx = _lean_ctx() + calls = [] + + def get_album(source, album_id): + calls.append((source, album_id)) + return _itunes_album() + + assert backfill_album_context_from_source(ctx, "itunes", get_album) is True + assert ctx["release_date"] == "2024-04-17" # real date, not YYYY-01-01 + assert ctx["total_tracks"] == 70 + assert calls == [("itunes", "itunes-123")] # queried the PRIMARY source with the album id + + +def test_backfill_noop_when_context_already_complete(): + ctx = {"id": "itunes-123", "release_date": "2024-04-17", "total_tracks": 70, "album_type": "album"} + called = [] + backfill_album_context_from_source(ctx, "itunes", lambda *a: called.append(a)) + assert called == [] # complete -> no fetch + assert ctx["release_date"] == "2024-04-17" + + +def test_backfill_noop_for_spotify_primary(): + # Spotify is covered by hydrate_download_metadata's get_track_details path. + called = [] + assert backfill_album_context_from_source(_lean_ctx(), "spotify", lambda *a: called.append(a)) is False + assert called == [] + + +def test_backfill_noop_for_sentinel_album_id(): + called = [] + for sentinel in ("explicit_album", "from_sync_modal", ""): + backfill_album_context_from_source(_lean_ctx(sentinel), "itunes", lambda *a: called.append(a)) + assert called == [] # no real id -> never queries + + +def test_backfill_leaves_context_lean_when_source_returns_nothing(): + ctx = _lean_ctx() + backfill_album_context_from_source(ctx, "itunes", lambda *a: None) + assert ctx["release_date"] == "" # still lean, but no crash + + +def test_backfill_swallows_source_errors(): + ctx = _lean_ctx() + + def boom(*_a): + raise RuntimeError("itunes down") + + # Must not raise — a backfill failure cannot break a download. + assert backfill_album_context_from_source(ctx, "itunes", boom) is False + assert ctx["release_date"] == "" + + if __name__ == '__main__': pytest.main([__file__, '-v'])