diff --git a/core/itunes_client.py b/core/itunes_client.py index 0d34b6db..cdf54f5b 100644 --- a/core/itunes_client.py +++ b/core/itunes_client.py @@ -739,7 +739,23 @@ class iTunesClient: cache = get_metadata_cache() cached = cache.get_entity('itunes', 'album', f"{album_id}_tracks") if cached and cached.get('items'): - return cached + # #918 follow-up: a tracks entry cached BEFORE the limit=200 fix is truncated + # to 50 and survives in the persistent cache (30-day TTL), so every window that + # loads this album from cache still shows 50 — not just the one path that was + # re-fetched fresh. Self-heal: entries written by the fixed fetch carry + # `_complete`; a legacy entry without it is re-validated against the album's + # known trackCount and re-fetched if it's short. (trackCount comes from the + # collection metadata and is unaffected by the tracks-limit bug.) + if cached.get('_complete'): + return cached + album_meta = cache.get_entity('itunes', 'album', str(album_id)) + expected = (album_meta or {}).get('trackCount') + if not (isinstance(expected, int) and expected > len(cached['items'])): + return cached + logger.info( + "iTunes album %s tracks cache looks truncated (%d cached < %d trackCount) — refetching", + album_id, len(cached['items']), expected, + ) # #918: the iTunes Lookup API returns only 50 related entities unless `limit` is # passed (max 200), so albums >50 tracks were truncated in the download window. @@ -851,7 +867,11 @@ class iTunesClient: 'items': tracks, 'total': len(tracks), 'limit': len(tracks), - 'next': None + 'next': None, + # Marks this entry as fetched with the limit=200 query (#918) so the + # stale-cache self-heal above trusts it and never re-fetches in a loop — + # important for region-restricted albums where len(tracks) < trackCount. + '_complete': True, } # Cache the album tracks listing diff --git a/tests/test_itunes_album_tracks_limit.py b/tests/test_itunes_album_tracks_limit.py index 1fa36e40..fdb921f9 100644 --- a/tests/test_itunes_album_tracks_limit.py +++ b/tests/test_itunes_album_tracks_limit.py @@ -44,3 +44,113 @@ def test_get_album_tracks_requests_limit_200(monkeypatch): assert captured.get('entity') == 'song' assert captured.get('id') == '123' assert result is not None + assert result.get('_complete') is True # fresh fetch is marked complete + + +# ── #918 follow-up: self-heal a stale truncated cache ───────────────────────── +# The metadata cache is persistent (30-day TTL), so a tracks entry written before +# the limit=200 fix stays truncated at 50 and is served to EVERY window (e.g. the +# Standard-view add-album modal) until it expires. get_album_tracks must detect a +# legacy entry shorter than the album's known trackCount and re-fetch. + +class _StubCache: + """Holds entities so cache hits/stores can be asserted.""" + def __init__(self, entities=None): + self.entities = dict(entities or {}) + self.stored = {} + + def get_entity(self, source, entity_type, entity_id): + return self.entities.get((source, entity_type, entity_id)) + + def store_entity(self, source, entity_type, entity_id, data): + self.stored[(source, entity_type, entity_id)] = data + + def store_entities_bulk(self, *a, **k): + pass + + +def _collection(track_count): + return {'wrapperType': 'collection', 'collectionId': 123, 'collectionName': 'Big OST', + 'artistName': 'Composer', 'trackCount': track_count, 'artworkUrl100': 'http://x/100x100bb.jpg'} + + +def _track(n): + return {'wrapperType': 'track', 'kind': 'song', 'trackId': n, 'trackName': f'T{n}', + 'trackNumber': n, 'discNumber': 1, 'artistName': 'Composer', 'trackTimeMillis': 1000} + + +def _full_results(n): + return [_collection(n)] + [_track(i) for i in range(1, n + 1)] + + +def _items(n): + return [{'id': str(i)} for i in range(n)] + + +def test_stale_truncated_legacy_cache_is_refetched(monkeypatch): + """A 50-item legacy entry (no _complete) for a 70-track album → re-fetch full.""" + client = ic.iTunesClient(country='US') + cache = _StubCache({ + ('itunes', 'album', '123_tracks'): {'items': _items(50), 'total': 50}, # legacy, truncated + ('itunes', 'album', '123'): _collection(70), # real trackCount=70 + }) + monkeypatch.setattr(ic, 'get_metadata_cache', lambda: cache) + calls = {'n': 0} + + def fake_lookup(**params): + calls['n'] += 1 + return _full_results(70) + + monkeypatch.setattr(client, '_lookup', fake_lookup) + + result = client.get_album_tracks('123') + + assert calls['n'] == 1 # re-fetched (didn't trust the stale 50) + assert len(result['items']) == 70 + assert result['_complete'] is True + assert cache.stored[('itunes', 'album', '123_tracks')]['_complete'] is True # healed in cache + + +def test_complete_cache_is_trusted_no_refetch(monkeypatch): + """A _complete entry is returned as-is even if shorter than trackCount + (region-restricted album) — must NOT loop re-fetching.""" + client = ic.iTunesClient(country='US') + complete = {'items': _items(50), 'total': 50, '_complete': True} + cache = _StubCache({ + ('itunes', 'album', '123_tracks'): complete, + ('itunes', 'album', '123'): _collection(70), + }) + monkeypatch.setattr(ic, 'get_metadata_cache', lambda: cache) + + def boom(**params): + raise AssertionError('must not re-fetch a _complete entry') + + monkeypatch.setattr(client, '_lookup', boom) + + assert client.get_album_tracks('123') is complete + + +def test_legacy_complete_cache_not_refetched(monkeypatch): + """A legacy entry whose length already meets trackCount is fine — no re-fetch.""" + client = ic.iTunesClient(country='US') + legacy = {'items': _items(30), 'total': 30} # no _complete, but complete by count + cache = _StubCache({ + ('itunes', 'album', '123_tracks'): legacy, + ('itunes', 'album', '123'): _collection(30), + }) + monkeypatch.setattr(ic, 'get_metadata_cache', lambda: cache) + monkeypatch.setattr(client, '_lookup', lambda **p: (_ for _ in ()).throw(AssertionError('no refetch'))) + + assert client.get_album_tracks('123') is legacy + + +def test_legacy_cache_without_album_meta_is_trusted(monkeypatch): + """trackCount unknown (album meta not cached) → trust the cache, don't re-fetch + (safe fallback; no regression for direct get_album_tracks callers).""" + client = ic.iTunesClient(country='US') + legacy = {'items': _items(50), 'total': 50} # no _complete, no album meta + cache = _StubCache({('itunes', 'album', '123_tracks'): legacy}) + monkeypatch.setattr(ic, 'get_metadata_cache', lambda: cache) + monkeypatch.setattr(client, '_lookup', lambda **p: (_ for _ in ()).throw(AssertionError('no refetch'))) + + assert client.get_album_tracks('123') is legacy