diff --git a/core/tidal_client.py b/core/tidal_client.py index d0d22099..c24bc321 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -1455,244 +1455,14 @@ class TidalClient: logger.error(f"Error getting Tidal user info: {e}") return None - def get_favorite_artists(self, limit: int = 200) -> list: - """Fetch user's favorite artists from Tidal. - Returns list of dicts with tidal_id, name, image_url.""" - try: - if not self._ensure_valid_token(): - logger.debug("Tidal not authenticated — cannot fetch favorites") - return [] - - user_id, api_version = self._get_user_id() - if not user_id: - logger.warning("Could not get Tidal user ID for favorites") - return [] - - artists = [] - - if api_version == 'v2': - # V2 API: /v2/favorites with filter - offset = 0 - while len(artists) < limit: - try: - headers = self.session.headers.copy() - headers['accept'] = 'application/vnd.api+json' - resp = requests.get( - f"{self.base_url}/favorites", - params={ - 'countryCode': 'US', - 'filter[user.id]': user_id, - 'filter[type]': 'ARTISTS', - 'include': 'artists', - 'page[limit]': min(50, limit - len(artists)), - 'page[offset]': offset - }, - headers=headers, timeout=15 - ) - if resp.status_code != 200: - logger.debug(f"Tidal V2 favorites returned {resp.status_code}, trying V1") - break - data = resp.json() - # Parse included artists - included = data.get('included', []) - if not included: - items = data.get('data', []) - if not items: - break - # Try to extract from data items directly - for item in items: - attrs = item.get('attributes', {}) - name = attrs.get('name', '') - if name: - img = None - img_data = item.get('relationships', {}).get('image', {}).get('data', {}) - if isinstance(img_data, dict) and img_data.get('id'): - img = f"https://resources.tidal.com/images/{img_data['id'].replace('-', '/')}/750x750.jpg" - artists.append({'tidal_id': item.get('id', ''), 'name': name, 'image_url': img}) - else: - for inc in included: - if inc.get('type') == 'artists': - attrs = inc.get('attributes', {}) - img = None - img_rel = inc.get('relationships', {}).get('image', {}).get('data', {}) - if isinstance(img_rel, dict) and img_rel.get('id'): - img = f"https://resources.tidal.com/images/{img_rel['id'].replace('-', '/')}/750x750.jpg" - artists.append({ - 'tidal_id': str(inc.get('id', '')), - 'name': attrs.get('name', ''), - 'image_url': img, - }) - if not data.get('links', {}).get('next'): - break - offset += 50 - import time - time.sleep(0.5) - except Exception as e: - logger.debug(f"Tidal V2 favorites error: {e}") - break - - # Fallback to V1 API if V2 returned nothing - if not artists: - try: - offset = 0 - while len(artists) < limit: - resp = self.session.get( - f"{self.alt_base_url}/users/{user_id}/favorites/artists", - params={'countryCode': 'US', 'limit': min(50, limit - len(artists)), 'offset': offset}, - timeout=15 - ) - if resp.status_code != 200: - logger.debug(f"Tidal V1 favorites returned {resp.status_code}") - break - data = resp.json() - items = data.get('items', []) - if not items: - break - for item in items: - a = item.get('item', item) - img_id = (a.get('picture') or '').replace('-', '/') - img = f"https://resources.tidal.com/images/{img_id}/750x750.jpg" if img_id else None - artists.append({ - 'tidal_id': str(a.get('id', '')), - 'name': a.get('name', ''), - 'image_url': img, - }) - total = data.get('totalNumberOfItems', 0) - offset += len(items) - if offset >= total: - break - import time - time.sleep(0.5) - except Exception as e: - logger.debug(f"Tidal V1 favorites error: {e}") - - logger.info(f"Retrieved {len(artists)} favorite artists from Tidal") - return artists - except Exception as e: - logger.error(f"Error fetching Tidal favorite artists: {e}") - return [] - - def get_favorite_albums(self, limit: int = 200) -> list: - """Fetch user's favorite albums from Tidal. - Returns list of dicts with tidal_id, album_name, artist_name, image_url, release_date, total_tracks.""" - try: - if not self._ensure_valid_token(): - logger.debug("Tidal not authenticated — cannot fetch favorite albums") - return [] - - user_id, api_version = self._get_user_id() - if not user_id: - logger.warning("Could not get Tidal user ID for favorite albums") - return [] - - albums = [] - - if api_version == 'v2': - offset = 0 - while len(albums) < limit: - try: - headers = self.session.headers.copy() - headers['accept'] = 'application/vnd.api+json' - resp = requests.get( - f"{self.base_url}/favorites", - params={ - 'countryCode': 'US', - 'filter[user.id]': user_id, - 'filter[type]': 'ALBUMS', - 'include': 'albums', - 'page[limit]': min(50, limit - len(albums)), - 'page[offset]': offset - }, - headers=headers, timeout=15 - ) - if resp.status_code != 200: - logger.debug(f"Tidal V2 favorite albums returned {resp.status_code}, trying V1") - break - data = resp.json() - included = data.get('included', []) - items = included if included else data.get('data', []) - if not items: - break - for item in items: - if included and item.get('type') not in ('albums', 'album'): - continue - attrs = item.get('attributes', {}) - title = attrs.get('title', '') - if not title: - continue - img = None - img_rel = item.get('relationships', {}).get('image', {}).get('data', {}) - if isinstance(img_rel, dict) and img_rel.get('id'): - img = f"https://resources.tidal.com/images/{img_rel['id'].replace('-', '/')}/750x750.jpg" - artist_name = '' - artist_rel = attrs.get('artists', [{}]) - if artist_rel and isinstance(artist_rel, list): - artist_name = artist_rel[0].get('name', '') if isinstance(artist_rel[0], dict) else '' - albums.append({ - 'tidal_id': str(item.get('id', '')), - 'album_name': title, - 'artist_name': artist_name, - 'image_url': img, - 'release_date': attrs.get('releaseDate', ''), - 'total_tracks': attrs.get('numberOfTracks', 0), - }) - if not data.get('links', {}).get('next'): - break - offset += 50 - import time - time.sleep(0.5) - except Exception as e: - logger.debug(f"Tidal V2 favorite albums error: {e}") - break - - # Fallback to V1 API - if not albums: - try: - offset = 0 - while len(albums) < limit: - resp = self.session.get( - f"{self.alt_base_url}/users/{user_id}/favorites/albums", - params={'countryCode': 'US', 'limit': min(50, limit - len(albums)), 'offset': offset}, - timeout=15 - ) - if resp.status_code != 200: - logger.debug(f"Tidal V1 favorite albums returned {resp.status_code}") - break - data = resp.json() - items = data.get('items', []) - if not items: - break - for item in items: - a = item.get('item', item) - img_id = (a.get('cover') or '').replace('-', '/') - img = f"https://resources.tidal.com/images/{img_id}/750x750.jpg" if img_id else None - artist_name = '' - if isinstance(a.get('artist'), dict): - artist_name = a['artist'].get('name', '') - elif isinstance(a.get('artists'), list) and a['artists']: - artist_name = a['artists'][0].get('name', '') - albums.append({ - 'tidal_id': str(a.get('id', '')), - 'album_name': a.get('title', ''), - 'artist_name': artist_name, - 'image_url': img, - 'release_date': a.get('releaseDate', ''), - 'total_tracks': a.get('numberOfTracks', 0), - }) - total = data.get('totalNumberOfItems', 0) - offset += len(items) - if offset >= total: - break - import time - time.sleep(0.5) - except Exception as e: - logger.debug(f"Tidal V1 favorite albums error: {e}") - - logger.info(f"Retrieved {len(albums)} favorite albums from Tidal") - return albums - except Exception as e: - logger.error(f"Error fetching Tidal favorite albums: {e}") - return [] + # `get_favorite_artists` and `get_favorite_albums` were defined here + # against the legacy `/v2/favorites?filter[type]=...` endpoint with a + # V1 fallback. Both paths are dead in 2026: V2 returns 404 for + # personal favorites (it's scoped to third-party-app-created + # collections only), and V1 returns 403 because modern OAuth tokens + # carry `collection.read` instead of the legacy `r_usr` scope V1 + # demands. Replaced by the V2 user-collection endpoints below — see + # the "Favorited albums + artists" section near the end of this class. # ------------------------------------------------------------------ # User Collection ("Favorite Tracks" — Tidal calls this "My Collection") @@ -1732,15 +1502,23 @@ class TidalClient: # a user-actionable hint instead of silently hiding the row. _COLLECTION_TRACKS_PATH = "userCollectionTracks/me/relationships/items" + _COLLECTION_ALBUMS_PATH = "userCollectionAlbums/me/relationships/items" + _COLLECTION_ARTISTS_PATH = "userCollectionArtists/me/relationships/items" _COLLECTION_BATCH_SIZE = 20 # Tidal `filter[id]` page cap @rate_limited - def _iter_collection_track_ids(self, max_ids: Optional[int] = None) -> List[str]: - """Walk the cursor-paginated collection endpoint and return the - list of track IDs in the user's Favorite Tracks. - - ``max_ids`` caps the walk early — used by callers that only - need a count or a partial list. Returns ``[]`` when not + def _iter_collection_resource_ids(self, path: str, expected_type: str, + max_ids: Optional[int] = None) -> List[str]: + """Walk a cursor-paginated collection endpoint and return the + list of resource IDs (tracks / albums / artists). + + Generic across all three favorited-resource endpoints — the + only differences between them are the path segment and the + ``type`` field on each ``data[]`` entry. Pagination, auth, + scope-failure detection, and the diagnostic logging are + identical. + + ``max_ids`` caps the walk early. Returns ``[]`` when not authenticated or when the endpoint refuses (e.g. token without ``collection.read`` scope). On 401/403 also flips ``self._collection_needs_reconnect = True`` so the caller can @@ -1750,10 +1528,10 @@ class TidalClient: self._collection_needs_reconnect = False if not self._ensure_valid_token(): - logger.debug("Tidal not authenticated — cannot fetch collection tracks") + logger.debug("Tidal not authenticated — cannot fetch collection %s", expected_type) return [] - track_ids: List[str] = [] + ids: List[str] = [] next_path: Optional[str] = None while True: @@ -1763,7 +1541,7 @@ class TidalClient: url = next_path if next_path.startswith('http') else f"https://openapi.tidal.com/v2{next_path}" params = None else: - url = f"{self.base_url}/{self._COLLECTION_TRACKS_PATH}" + url = f"{self.base_url}/{path}" params = { 'countryCode': 'US', 'locale': 'en-US', @@ -1811,13 +1589,13 @@ class TidalClient: break for item in data.get('data', []): - if item.get('type') != 'tracks': + if item.get('type') != expected_type: continue - tid = item.get('id') - if tid: - track_ids.append(str(tid)) - if max_ids is not None and len(track_ids) >= max_ids: - return track_ids + rid = item.get('id') + if rid: + ids.append(str(rid)) + if max_ids is not None and len(ids) >= max_ids: + return ids next_path = data.get('links', {}).get('next') if not next_path: @@ -1825,7 +1603,25 @@ class TidalClient: time.sleep(0.3) # Cursor pagination courtesy delay - return track_ids + return ids + + def _iter_collection_track_ids(self, max_ids: Optional[int] = None) -> List[str]: + """Favorited tracks — thin wrapper over the generic walker.""" + return self._iter_collection_resource_ids( + self._COLLECTION_TRACKS_PATH, 'tracks', max_ids, + ) + + def _iter_collection_album_ids(self, max_ids: Optional[int] = None) -> List[str]: + """Favorited albums — thin wrapper over the generic walker.""" + return self._iter_collection_resource_ids( + self._COLLECTION_ALBUMS_PATH, 'albums', max_ids, + ) + + def _iter_collection_artist_ids(self, max_ids: Optional[int] = None) -> List[str]: + """Favorited artists — thin wrapper over the generic walker.""" + return self._iter_collection_resource_ids( + self._COLLECTION_ARTISTS_PATH, 'artists', max_ids, + ) def collection_needs_reconnect(self) -> bool: """True when the most recent collection fetch hit a 401/403 — @@ -1879,6 +1675,224 @@ class TidalClient: logger.error(f"Error fetching Tidal collection tracks: {e}") return [] + # ------------------------------------------------------------------ + # Favorited albums + artists — V2 collection endpoints + # ------------------------------------------------------------------ + # + # Same problem the tracks side hit on issue #502: the prior + # `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoints are + # deprecated (404) and the V1 fallback (`/v1/users//favorites/ + # albums|artists`) returns 403 because modern OAuth tokens with + # `collection.read` scope don't have the legacy `r_usr` scope V1 + # requires. Discord-reported symptom: Discover → Your Albums (and + # Your Artists) section shows nothing for Tidal users regardless + # of how many albums/artists they've favorited. + # + # Fix mirrors the tracks path: + # 1) Cursor-walk `/v2/userCollection{Albums|Artists}/me/relationships/items` + # via `_iter_collection_album_ids` / `_iter_collection_artist_ids` + # (lifted into the generic `_iter_collection_resource_ids` helper). + # 2) Batch-hydrate via `/v2/{albums|artists}?filter[id]=...&include=...` + # with single-request fan-out (artists+coverArt for albums, + # profileArt for artists). Parses JSON:API `included[]` for + # artist names + image URLs. + # + # Public surface preserves the existing return shape — list of + # dicts matching what `database.upsert_liked_album` / + # `upsert_liked_artist` consume — so web_server.py callers + # (`/api/discover/your-albums-fetch` and equivalent) stay + # byte-identical. + + @rate_limited + def _get_albums_batch(self, album_ids: List[str]) -> List[Dict[str, Any]]: + """Batch-fetch album metadata + cover art + artist names in + one request via JSON:API extended-include semantics. Returns + list of dicts matching `database.upsert_liked_album` kwargs.""" + if not album_ids: + return [] + try: + params = { + 'countryCode': 'US', + 'include': 'artists,coverArt', + 'filter[id]': ','.join(album_ids), + } + headers = {'accept': 'application/vnd.api+json'} + resp = self.session.get( + f"{self.base_url}/albums", + params=params, headers=headers, timeout=15, + ) + if resp.status_code != 200: + logger.debug( + f"Tidal albums batch returned {resp.status_code}: {resp.text[:200]}" + ) + return [] + + data = resp.json() + artists_by_id, artworks_by_id = self._build_included_maps(data.get('included', [])) + + results: List[Dict[str, Any]] = [] + for item in data.get('data', []): + if item.get('type') != 'albums': + continue + attrs = item.get('attributes', {}) + rels = item.get('relationships', {}) + results.append({ + 'tidal_id': str(item.get('id', '')), + 'album_name': attrs.get('title', '') or '', + 'artist_name': self._first_artist_name(rels, artists_by_id), + 'image_url': self._first_artwork_url(rels.get('coverArt', {}), artworks_by_id), + 'release_date': attrs.get('releaseDate', '') or '', + 'total_tracks': int(attrs.get('numberOfItems') or 0), + }) + return results + except Exception as e: + logger.debug(f"Tidal _get_albums_batch error: {e}") + return [] + + @rate_limited + def _get_artists_batch(self, artist_ids: List[str]) -> List[Dict[str, Any]]: + """Batch-fetch artist metadata + profile image. Returns list + of dicts matching the prior `get_favorite_artists` shape + (`tidal_id`, `name`, `image_url`).""" + if not artist_ids: + return [] + try: + params = { + 'countryCode': 'US', + 'include': 'profileArt', + 'filter[id]': ','.join(artist_ids), + } + headers = {'accept': 'application/vnd.api+json'} + resp = self.session.get( + f"{self.base_url}/artists", + params=params, headers=headers, timeout=15, + ) + if resp.status_code != 200: + logger.debug( + f"Tidal artists batch returned {resp.status_code}: {resp.text[:200]}" + ) + return [] + + data = resp.json() + _, artworks_by_id = self._build_included_maps(data.get('included', [])) + + results: List[Dict[str, Any]] = [] + for item in data.get('data', []): + if item.get('type') != 'artists': + continue + attrs = item.get('attributes', {}) + rels = item.get('relationships', {}) + results.append({ + 'tidal_id': str(item.get('id', '')), + 'name': attrs.get('name', '') or '', + 'image_url': self._first_artwork_url(rels.get('profileArt', {}), artworks_by_id), + }) + return results + except Exception as e: + logger.debug(f"Tidal _get_artists_batch error: {e}") + return [] + + @staticmethod + def _build_included_maps(included: List[Dict[str, Any]]): + """Index a JSON:API `included[]` array by resource type so the + per-resource lookup in batch-hydrate is O(1) per relationship + ref rather than O(n).""" + artists_by_id: Dict[str, Dict[str, Any]] = {} + artworks_by_id: Dict[str, Dict[str, Any]] = {} + for inc in included: + inc_id = str(inc.get('id', '')) + if not inc_id: + continue + inc_type = inc.get('type') + if inc_type == 'artists': + artists_by_id[inc_id] = inc + elif inc_type == 'artworks': + artworks_by_id[inc_id] = inc + return artists_by_id, artworks_by_id + + @staticmethod + def _first_artist_name(relationships: Dict[str, Any], + artists_by_id: Dict[str, Dict[str, Any]]) -> str: + """Resolve the primary artist name from a relationships block + + included-artists map. Returns '' if not resolvable so the + upsert path doesn't trip on None.""" + artist_refs = relationships.get('artists', {}).get('data', []) + if not artist_refs: + return '' + first_id = str(artist_refs[0].get('id', '')) + artist_obj = artists_by_id.get(first_id, {}) + return artist_obj.get('attributes', {}).get('name', '') or '' + + @staticmethod + def _first_artwork_url(artwork_relationship: Dict[str, Any], + artworks_by_id: Dict[str, Dict[str, Any]]) -> Optional[str]: + """Resolve the largest cover/profile image URL from an artwork + relationship + included-artworks map. Tidal returns files + largest-first so picking files[0] gets the highest-resolution + variant (typically 1280×1280).""" + refs = artwork_relationship.get('data', []) + if not refs: + return None + first_id = str(refs[0].get('id', '')) + artwork = artworks_by_id.get(first_id, {}) + files = artwork.get('attributes', {}).get('files', []) + if not files: + return None + return files[0].get('href') + + def get_favorite_albums(self, limit: int = 200) -> List[Dict[str, Any]]: + """Fetch user's favorited albums via the V2 user-collection + endpoint. Replaces the prior `/v2/favorites` + V1-fallback + path which is now dead (V2 endpoint deprecated, V1 returns + 403 for modern OAuth tokens lacking `r_usr` scope). + + Returns list of dicts matching `database.upsert_liked_album` + kwargs — the discover.py 'Your Albums' aggregator iterates + these and writes them to the liked_albums table.""" + try: + album_ids = self._iter_collection_album_ids(max_ids=limit) + if not album_ids: + return [] + + results: List[Dict[str, Any]] = [] + for i in range(0, len(album_ids), self._COLLECTION_BATCH_SIZE): + batch = album_ids[i:i + self._COLLECTION_BATCH_SIZE] + results.extend(self._get_albums_batch(batch)) + + logger.info( + f"Retrieved {len(results)}/{len(album_ids)} favorite albums from Tidal" + ) + return results + except Exception as e: + logger.error(f"Error fetching Tidal favorite albums: {e}") + return [] + + def get_favorite_artists(self, limit: int = 200) -> List[Dict[str, Any]]: + """Fetch user's favorited artists via the V2 user-collection + endpoint. Replaces the prior `/v2/favorites` + V1-fallback + path (dead for the same reason as `get_favorite_albums`). + + Returns list of dicts matching the prior shape (`tidal_id`, + `name`, `image_url`) so web_server.py's `/api/discover/ + your-artists-fetch` aggregator path stays byte-identical.""" + try: + artist_ids = self._iter_collection_artist_ids(max_ids=limit) + if not artist_ids: + return [] + + results: List[Dict[str, Any]] = [] + for i in range(0, len(artist_ids), self._COLLECTION_BATCH_SIZE): + batch = artist_ids[i:i + self._COLLECTION_BATCH_SIZE] + results.extend(self._get_artists_batch(batch)) + + logger.info( + f"Retrieved {len(results)}/{len(artist_ids)} favorite artists from Tidal" + ) + return results + except Exception as e: + logger.error(f"Error fetching Tidal favorite artists: {e}") + return [] + # Global instance _tidal_client = None diff --git a/tests/test_tidal_favorite_albums_artists.py b/tests/test_tidal_favorite_albums_artists.py new file mode 100644 index 00000000..32bee28a --- /dev/null +++ b/tests/test_tidal_favorite_albums_artists.py @@ -0,0 +1,442 @@ +"""Pin Tidal favorite albums + artists fetch via V2 user-collection +endpoints. + +Discord report: Discover → Your Albums section showed nothing for +Tidal users regardless of how many albums they'd favorited. Audit +found `get_favorite_albums` (and `get_favorite_artists`) called the +deprecated `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoint +which returns 404 for personal favorites — that endpoint is scoped +to collections the third-party app created itself, not the user's +app-level favorites. The V1 fallback (`/v1/users//favorites/...`) +returns 403 for modern OAuth tokens because they carry +`collection.read` instead of the legacy `r_usr` scope. + +Fix: rewire to the same V2 user-collection cursor-paginated +endpoints we shipped for tracks (issue #502): + - `/v2/userCollectionAlbums/me/relationships/items` + - `/v2/userCollectionArtists/me/relationships/items` + +Plus per-resource batch hydration via `/v2/{albums|artists}` with +extended-include semantics (`include=artists,coverArt` for albums, +`include=profileArt` for artists) so artist names + image URLs come +back in a single request per batch instead of N+1 lookups. + +These tests pin: + - Cursor walkers dispatch correct path + type to the generic + `_iter_collection_resource_ids` helper + - Batch hydrators parse JSON:API `data[]` + `included[]` into the + legacy return shape that `database.upsert_liked_album` / + `upsert_liked_artist` consume — preserves byte-identical wiring + in `web_server.py`'s discover aggregator + - Image URL resolution picks largest variant from artwork files[] + - Artist-name resolution falls through to '' when relationships + are missing (so the upsert path doesn't trip on None) + - Empty-input + HTTP-error paths return [] without raising +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from core.tidal_client import TidalClient + + +def _make_client(): + """Bare TidalClient with auth state primed — no real connection. + Mirrors the helper in test_tidal_collection_tracks.py.""" + client = TidalClient.__new__(TidalClient) + client.access_token = "fake-token" + client.token_expires_at = 9_999_999_999 + client.base_url = "https://openapi.tidal.com/v2" + client.alt_base_url = "https://api.tidal.com/v1" + client.session = MagicMock() + return client + + +class _FakeResp: + def __init__(self, status_code=200, json_body=None, text=""): + self.status_code = status_code + self._body = json_body if json_body is not None else {} + self.text = text or str(self._body) + + def json(self): + return self._body + + +# --------------------------------------------------------------------------- +# Cursor-walker dispatch +# --------------------------------------------------------------------------- + + +class TestCollectionWalkerDispatch: + def test_album_iter_passes_album_path_and_type(self): + """`_iter_collection_album_ids` must dispatch to the generic + walker with the albums path + 'albums' expected_type. If the + wrapper drifts (e.g. typoed path) the IDs come back empty.""" + client = _make_client() + with patch.object(client, '_iter_collection_resource_ids', + return_value=['111', '222']) as mock_walk: + ids = client._iter_collection_album_ids(max_ids=50) + + mock_walk.assert_called_once_with( + 'userCollectionAlbums/me/relationships/items', 'albums', 50, + ) + assert ids == ['111', '222'] + + def test_artist_iter_passes_artist_path_and_type(self): + client = _make_client() + with patch.object(client, '_iter_collection_resource_ids', + return_value=['17275']) as mock_walk: + ids = client._iter_collection_artist_ids() + + mock_walk.assert_called_once_with( + 'userCollectionArtists/me/relationships/items', 'artists', None, + ) + assert ids == ['17275'] + + +# --------------------------------------------------------------------------- +# Helper: included map + relationship resolution +# --------------------------------------------------------------------------- + + +class TestIncludedMaps: + def test_build_included_maps_groups_by_type(self): + included = [ + {'id': 'a1', 'type': 'artists', 'attributes': {'name': 'Foo'}}, + {'id': 'art1', 'type': 'artworks', 'attributes': {'files': []}}, + {'id': 'a2', 'type': 'artists', 'attributes': {'name': 'Bar'}}, + {'id': 'unknown1', 'type': 'something_else'}, + {'type': 'artworks'}, # missing id — should be skipped + ] + artists, artworks = TidalClient._build_included_maps(included) + assert set(artists.keys()) == {'a1', 'a2'} + assert set(artworks.keys()) == {'art1'} + assert artists['a1']['attributes']['name'] == 'Foo' + + def test_first_artist_name_resolves_from_map(self): + artists_map = {'a1': {'attributes': {'name': 'Eminem'}}} + rels = {'artists': {'data': [{'id': 'a1', 'type': 'artists'}]}} + assert TidalClient._first_artist_name(rels, artists_map) == 'Eminem' + + def test_first_artist_name_empty_when_no_refs(self): + """Defensive: relationships block missing or empty → '' so + upsert path doesn't trip on None.""" + assert TidalClient._first_artist_name({}, {}) == '' + assert TidalClient._first_artist_name( + {'artists': {'data': []}}, {} + ) == '' + + def test_first_artist_name_empty_when_unknown_id(self): + """Artist ref points at an ID not in included map — fall + through to '' rather than crash.""" + rels = {'artists': {'data': [{'id': 'missing'}]}} + artists_map = {'other': {'attributes': {'name': 'X'}}} + assert TidalClient._first_artist_name(rels, artists_map) == '' + + def test_first_artwork_url_picks_first_file(self): + """Tidal returns artwork files largest-first. Picking files[0] + gets the highest-resolution variant (typically 1280×1280).""" + artworks_map = { + 'art1': {'attributes': {'files': [ + {'href': 'https://big.jpg', 'meta': {'width': 1280}}, + {'href': 'https://small.jpg', 'meta': {'width': 320}}, + ]}} + } + rel = {'data': [{'id': 'art1', 'type': 'artworks'}]} + url = TidalClient._first_artwork_url(rel, artworks_map) + assert url == 'https://big.jpg' + + def test_first_artwork_url_none_when_no_relationship(self): + assert TidalClient._first_artwork_url({}, {}) is None + assert TidalClient._first_artwork_url({'data': []}, {}) is None + + def test_first_artwork_url_none_when_no_files(self): + """Defensive: artwork resource exists but has no files array. + Return None rather than IndexError.""" + artworks_map = {'art1': {'attributes': {'files': []}}} + rel = {'data': [{'id': 'art1'}]} + assert TidalClient._first_artwork_url(rel, artworks_map) is None + + +# --------------------------------------------------------------------------- +# Batch hydration — albums +# --------------------------------------------------------------------------- + + +_ALBUM_BATCH_RESPONSE = { + 'data': [ + { + 'id': '141121273', + 'type': 'albums', + 'attributes': { + 'title': 'Mr. Morale & The Big Steppers', + 'releaseDate': '2022-05-13', + 'numberOfItems': 18, + }, + 'relationships': { + 'artists': {'data': [{'id': '5034248', 'type': 'artists'}]}, + 'coverArt': {'data': [{'id': 'cover-uuid', 'type': 'artworks'}]}, + }, + }, + { + 'id': '999', + 'type': 'albums', + 'attributes': {'title': 'Album Without Artist or Cover'}, + 'relationships': {}, + }, + ], + 'included': [ + { + 'id': '5034248', 'type': 'artists', + 'attributes': {'name': 'Kendrick Lamar'}, + }, + { + 'id': 'cover-uuid', 'type': 'artworks', + 'attributes': {'files': [ + {'href': 'https://resources.tidal.com/images/cover/1280x1280.jpg'}, + ]}, + }, + ], +} + + +class TestGetAlbumsBatch: + def test_parses_full_album_response(self): + client = _make_client() + client.session.get = MagicMock( + return_value=_FakeResp(200, _ALBUM_BATCH_RESPONSE) + ) + results = client._get_albums_batch(['141121273', '999']) + + assert len(results) == 2 + # First album — full attributes resolved from included + first = results[0] + assert first['tidal_id'] == '141121273' + assert first['album_name'] == 'Mr. Morale & The Big Steppers' + assert first['artist_name'] == 'Kendrick Lamar' + assert first['image_url'] == 'https://resources.tidal.com/images/cover/1280x1280.jpg' + assert first['release_date'] == '2022-05-13' + assert first['total_tracks'] == 18 + # Second album — missing relationships fall through to defaults + second = results[1] + assert second['album_name'] == 'Album Without Artist or Cover' + assert second['artist_name'] == '' + assert second['image_url'] is None + assert second['release_date'] == '' + assert second['total_tracks'] == 0 + + def test_empty_input_returns_empty_without_request(self): + client = _make_client() + client.session.get = MagicMock() + results = client._get_albums_batch([]) + assert results == [] + client.session.get.assert_not_called() + + def test_http_error_returns_empty(self): + client = _make_client() + client.session.get = MagicMock( + return_value=_FakeResp(500, text='server error') + ) + results = client._get_albums_batch(['111']) + assert results == [] + + def test_skips_data_entries_with_wrong_type(self): + """Forward-compat: response shape might surface non-album + resources alongside the request — only collect entries whose + type is 'albums'.""" + client = _make_client() + client.session.get = MagicMock(return_value=_FakeResp(200, { + 'data': [ + {'id': '1', 'type': 'albums', 'attributes': {'title': 'A'}, 'relationships': {}}, + {'id': '2', 'type': 'tracks', 'attributes': {'title': 'Skip Me'}}, + ], + 'included': [], + })) + results = client._get_albums_batch(['1', '2']) + assert len(results) == 1 + assert results[0]['album_name'] == 'A' + + def test_filter_id_param_is_comma_joined(self): + """The Tidal API expects `filter[id]=a,b,c` — verify our + param construction. Drift here would break batching against + production silently.""" + client = _make_client() + captured_params = {} + + def fake_get(url, params=None, headers=None, timeout=None): + captured_params.update(params or {}) + return _FakeResp(200, {'data': [], 'included': []}) + + client.session.get = MagicMock(side_effect=fake_get) + client._get_albums_batch(['111', '222', '333']) + assert captured_params['filter[id]'] == '111,222,333' + assert captured_params['include'] == 'artists,coverArt' + + +# --------------------------------------------------------------------------- +# Batch hydration — artists +# --------------------------------------------------------------------------- + + +_ARTIST_BATCH_RESPONSE = { + 'data': [ + { + 'id': '17275', + 'type': 'artists', + 'attributes': {'name': 'Eminem'}, + 'relationships': { + 'profileArt': {'data': [{'id': 'profile-uuid', 'type': 'artworks'}]}, + }, + }, + ], + 'included': [ + { + 'id': 'profile-uuid', 'type': 'artworks', + 'attributes': {'files': [ + {'href': 'https://resources.tidal.com/images/profile/750x750.jpg'}, + ]}, + }, + ], +} + + +class TestGetArtistsBatch: + def test_parses_full_artist_response(self): + client = _make_client() + client.session.get = MagicMock( + return_value=_FakeResp(200, _ARTIST_BATCH_RESPONSE) + ) + results = client._get_artists_batch(['17275']) + + assert len(results) == 1 + assert results[0]['tidal_id'] == '17275' + assert results[0]['name'] == 'Eminem' + assert results[0]['image_url'] == 'https://resources.tidal.com/images/profile/750x750.jpg' + + def test_empty_input_returns_empty_without_request(self): + client = _make_client() + client.session.get = MagicMock() + assert client._get_artists_batch([]) == [] + client.session.get.assert_not_called() + + def test_http_error_returns_empty(self): + client = _make_client() + client.session.get = MagicMock( + return_value=_FakeResp(404, text='not found') + ) + assert client._get_artists_batch(['17275']) == [] + + def test_filter_id_and_include_params(self): + client = _make_client() + captured = {} + + def fake_get(url, params=None, headers=None, timeout=None): + captured.update(params or {}) + return _FakeResp(200, {'data': [], 'included': []}) + + client.session.get = MagicMock(side_effect=fake_get) + client._get_artists_batch(['17275', '721']) + assert captured['filter[id]'] == '17275,721' + assert captured['include'] == 'profileArt' + + +# --------------------------------------------------------------------------- +# Public methods — orchestrator behavior +# --------------------------------------------------------------------------- + + +class TestGetFavoriteAlbums: + def test_walks_then_batches_then_returns(self): + """End-to-end: iter returns IDs, batch hydrates them, result + is the concatenation. Backward-compatible shape preserved + for `database.upsert_liked_album` callers.""" + client = _make_client() + with patch.object(client, '_iter_collection_album_ids', + return_value=['1', '2', '3']) as mock_iter, \ + patch.object(client, '_get_albums_batch', + return_value=[ + {'tidal_id': '1', 'album_name': 'A', + 'artist_name': 'X', 'image_url': 'u', + 'release_date': '2020', 'total_tracks': 10}, + {'tidal_id': '2', 'album_name': 'B', + 'artist_name': 'Y', 'image_url': None, + 'release_date': '', 'total_tracks': 0}, + ]) as mock_batch: + results = client.get_favorite_albums(limit=100) + + mock_iter.assert_called_once_with(max_ids=100) + # Single batch call since 3 IDs fit in one BATCH_SIZE chunk (20) + assert mock_batch.call_count == 1 + assert len(results) == 2 + assert results[0]['tidal_id'] == '1' + # Verify shape compatibility with upsert_liked_album kwargs + expected_keys = {'tidal_id', 'album_name', 'artist_name', + 'image_url', 'release_date', 'total_tracks'} + assert set(results[0].keys()) == expected_keys + + def test_no_ids_returns_empty_without_batch(self): + client = _make_client() + with patch.object(client, '_iter_collection_album_ids', return_value=[]), \ + patch.object(client, '_get_albums_batch') as mock_batch: + assert client.get_favorite_albums() == [] + mock_batch.assert_not_called() + + def test_chunks_into_batch_size(self): + """41 IDs at BATCH_SIZE 20 → three batches of 20/20/1. + Tidal's filter[id] cap is the per-request limit; orchestrator + must respect it.""" + client = _make_client() + ids = [str(i) for i in range(41)] + captured_batches = [] + + def fake_batch(batch): + captured_batches.append(list(batch)) + return [{'tidal_id': b, 'album_name': f'A{b}', 'artist_name': '', + 'image_url': None, 'release_date': '', 'total_tracks': 0} + for b in batch] + + with patch.object(client, '_iter_collection_album_ids', return_value=ids), \ + patch.object(client, '_get_albums_batch', side_effect=fake_batch): + results = client.get_favorite_albums() + + assert len(results) == 41 + assert [len(b) for b in captured_batches] == [20, 20, 1] + + +class TestGetFavoriteArtists: + def test_walks_then_batches(self): + client = _make_client() + with patch.object(client, '_iter_collection_artist_ids', + return_value=['17275']) as mock_iter, \ + patch.object(client, '_get_artists_batch', + return_value=[{'tidal_id': '17275', 'name': 'Eminem', + 'image_url': 'https://eminem.jpg'}]) as mock_batch: + results = client.get_favorite_artists(limit=200) + + mock_iter.assert_called_once_with(max_ids=200) + mock_batch.assert_called_once() + assert len(results) == 1 + assert results[0]['name'] == 'Eminem' + # Backward-compat shape — exactly the keys the prior + # implementation returned + assert set(results[0].keys()) == {'tidal_id', 'name', 'image_url'} + + def test_no_ids_returns_empty(self): + client = _make_client() + with patch.object(client, '_iter_collection_artist_ids', return_value=[]), \ + patch.object(client, '_get_artists_batch') as mock_batch: + assert client.get_favorite_artists() == [] + mock_batch.assert_not_called() + + def test_swallows_iter_exception_returns_empty(self): + """Defensive: if the cursor walker blows up mid-page, the + public method should return [] (no partial corruption of the + liked-artists table).""" + client = _make_client() + with patch.object(client, '_iter_collection_artist_ids', + side_effect=RuntimeError('boom')): + assert client.get_favorite_artists() == [] diff --git a/webui/static/helper.js b/webui/static/helper.js index 2c525056..2cce02dd 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3416,6 +3416,7 @@ const WHATS_NEW = { '2.5.1': [ // --- post-release patch work on the 2.5.1 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- { date: 'Unreleased — 2.5.1 patch work' }, + { title: 'Tidal Favorite Albums + Artists Now Show Up On Discover', desc: 'discover → your albums (and your artists) was returning nothing for tidal users regardless of how many albums/artists they\'d favorited. cause: `get_favorite_albums` and `get_favorite_artists` were calling the deprecated `/v2/favorites?filter[type]=ALBUMS|ARTISTS` endpoint, which returns 404 for personal favorites — that endpoint is scoped to collections the third-party app created itself, not the user\'s app-level favorites. the V1 fallback was also dead because modern OAuth tokens carry `collection.read` instead of the legacy `r_usr` scope V1 requires (returns 403). same root cause as the favorited tracks fix from #502. fix: rewire to the working V2 user-collection endpoints — `/v2/userCollectionAlbums/me/relationships/items` and `/v2/userCollectionArtists/me/relationships/items` — using the same cursor-paginated pattern shipped for tracks. ID enumeration lifted into a generic `_iter_collection_resource_ids(path, expected_type, max_ids)` helper so tracks/albums/artists all share one walker (~80 lines deduped). batch hydration via `/v2/{albums|artists}?filter[id]=...&include=...` with extended JSON:API include semantics — single request returns 20 albums + their artists + cover artworks all in `included[]`, parsed via two static helpers (`_first_artist_name`, `_first_artwork_url`) that map relationship refs to the included map. cover/profile images pick `files[0]` (largest variant Tidal returns, typically 1280×1280). public methods preserve the prior return shape so the discover aggregator in web_server.py stays byte-identical. 24 new tests pin: cursor-walker dispatch (correct path + type), included-map building, artist + artwork relationship resolution (full + missing + unknown-id), batch hydration parse for albums + artists, empty-input + HTTP-error short-circuits, BATCH_SIZE chunking (41 IDs → 20/20/1), end-to-end orchestrator behavior.', page: 'discover' }, { title: 'Server Playlist Sync: Append Mode (Stop Overwriting User-Added Tracks)', desc: 'discord report (cjfc, 2026-04-26): syncing a spotify playlist to your server overwrote anything you\'d manually added to the server-side playlist. now there\'s a per-sync mode picker next to the Sync button on the playlist details modal: "Replace" (default, current behavior — delete + recreate) or "Append only" (preserve existing, only add tracks not already there). useful when the source platform caps playlist size (spotify 100-track limit) and you\'re manually building beyond it on the server. each server client (plex / jellyfin / navidrome) gets a new `append_to_playlist(name, tracks)` method that uses the server\'s native append api — plex `addItems`, jellyfin `POST /Playlists//Items`, navidrome subsonic `updatePlaylist?songIdToAdd=...`. no delete-recreate, no backup playlist created in append mode (preserves playlist creation date + metadata + non-soulsync-managed tracks). dedup-by-id ensures we never add a track that\'s already on the playlist (matched by ratingKey for plex, jellyfin guid id for jellyfin, song id for navidrome — server-native identity, not fuzzy title+artist match). falls back to `create_playlist` when the playlist doesn\'t exist yet (first sync). sync_service dispatches via the new mode flag through /api/sync/start; soulsync standalone has no playlist methods at all so the dispatch falls back to update_playlist with a warning log when append is requested against it. 15 new tests pin: missing playlist → create delegation, dedup filtering (existing ids skipped), short-circuit on no-new-tracks (no api call), failure paths return False without raising, contract listing for each server client.', page: 'sync' }, ], '2.5.0': [