diff --git a/core/tidal_client.py b/core/tidal_client.py index 320a91aa..da5a0ef3 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -19,6 +19,18 @@ import secrets logger = get_logger("tidal_client") +# Virtual playlist identity for the user's Favorite Tracks (Tidal's +# "My Collection" view). Treated like a normal playlist by every +# get_playlist consumer (mirror auto-refresh, discovery, sync UI) — +# the client recognizes the ID and dispatches to the dedicated +# `userCollectionTracks` endpoint internally. ID intentionally has no +# colon: the sync-services.js renderer interpolates IDs into CSS +# selectors via template literals (e.g. `#tidal-card-${p.id} .foo`) +# and a `:` in the ID would be parsed as a CSS pseudo-class operator. +COLLECTION_PLAYLIST_ID = "tidal-favorites" +COLLECTION_PLAYLIST_NAME = "Favorite Tracks" +COLLECTION_PLAYLIST_DESCRIPTION = "Your favorited tracks on Tidal" + # Global rate limiting variables _last_api_call_time = 0 _api_call_lock = threading.Lock() @@ -254,14 +266,21 @@ class TidalClient: # Generate PKCE challenge self._generate_pkce_challenge() - # Create OAuth URL with PKCE + # Create OAuth URL with PKCE. + # `prompt=consent` forces Tidal to display the consent + # screen even when the app is already authorized — without + # it, re-authenticating with newly-added scopes (e.g. + # `collection.read` added in v2.4.3) can silently return a + # token carrying only the ORIGINAL scope set because Tidal + # treats the existing authorization as still valid. params = { 'response_type': 'code', 'client_id': self.client_id, 'redirect_uri': self.redirect_uri, - 'scope': 'user.read playlists.read', # Updated with the required scope + 'scope': 'user.read playlists.read collection.read', # collection.read needed for userCollectionTracks endpoint 'code_challenge': self.code_challenge, - 'code_challenge_method': 'S256' + 'code_challenge_method': 'S256', + 'prompt': 'consent', } auth_url = f"{self.auth_url}?" + urllib.parse.urlencode(params) @@ -520,6 +539,26 @@ class TidalClient: return result return False + + def disconnect(self): + """Clear all saved Tidal auth state so the next OAuth flow + starts fresh with the current scope set. + + Used when a previously-authorized token doesn't carry a newly- + added scope (e.g. `collection.read`): even with `prompt=consent` + on the auth URL, some users hit a Tidal flow that rebinds the + existing grant. Disconnect first → re-authenticate forces a + clean slate.""" + self.access_token = None + self.refresh_token = None + self.token_expires_at = 0 + self._collection_needs_reconnect = False + self.session.headers.pop('Authorization', None) + try: + config_manager.set('tidal_tokens', {}) + except Exception as e: + logger.warning(f"Failed to clear tidal_tokens config: {e}") + logger.info("Tidal client disconnected — saved tokens cleared") def _get_user_id(self): """Get current user's ID from /users/me endpoint""" @@ -1071,8 +1110,31 @@ class TidalClient: @rate_limited def get_playlist(self, playlist_id: str) -> Optional[Playlist]: - """Get playlist details including tracks using JSON:API format""" + """Get playlist details including tracks using JSON:API format. + + Recognizes the virtual ``tidal-favorites`` ID and dispatches + to ``get_collection_tracks`` so every caller that already + accepts a playlist ID (mirror auto-refresh, discovery start, + per-playlist detail endpoint) gets Favorite Tracks support + for free without per-site special-casing. + + ID intentionally has no colon — the sync-services.js renderer + builds CSS selectors via template literal interpolation + (``#tidal-card-${p.id} .playlist-card-track-count``) and a + ``:`` in the ID would be parsed as a CSS pseudo-class operator. + """ try: + if playlist_id == COLLECTION_PLAYLIST_ID: + collection_tracks = self.get_collection_tracks() + return Playlist( + id=COLLECTION_PLAYLIST_ID, + name=COLLECTION_PLAYLIST_NAME, + description=COLLECTION_PLAYLIST_DESCRIPTION, + tracks=collection_tracks, + owner={'name': 'You'}, + public=False, + ) + if not self._ensure_valid_token(): logger.error("Not authenticated with Tidal") return None @@ -1632,6 +1694,191 @@ class TidalClient: logger.error(f"Error fetching Tidal favorite albums: {e}") return [] + # ------------------------------------------------------------------ + # User Collection ("Favorite Tracks" — Tidal calls this "My Collection") + # ------------------------------------------------------------------ + # + # Tidal V2 exposes the user's favorited tracks via a separate + # cursor-paginated endpoint: + # + # GET /v2/userCollectionTracks/me/relationships/items + # ?countryCode=US&locale=en-US&include=items + # + # Each page returns up to 20 entries in `data[]` (track refs with + # `id` + `type='tracks'` + `meta.addedAt`) and an OPTIONAL `links.next` + # URL for the next page. The included track resources only carry the + # track-level attributes (title, isrc, duration, mediaTags) — artists + # and album NAMES come back as relationship-link stubs only, not + # embedded data. + # + # We split the work in two phases: + # 1) `_iter_collection_track_ids()` — enumerates every collection + # entry by following the cursor chain. Cheap (just IDs + types). + # 2) Hydration — feeds the IDs through the existing + # `_get_tracks_batch` helper which already knows how to + # `include=artists,albums` and produce fully-populated `Track` + # objects matching the rest of the codebase. No new parsing, + # no new dataclass shape, no duplication of the JSON:API parse. + # + # Reference: https://github.com/Nezreka/SoulSync/issues/502 — reporter + # Yug1900 located the working endpoint after the prior `/v2/favorites` + # filter approach returned empty data for personal favorites. + # + # Auth state — calling code (web_server.py listing endpoint) needs + # to know whether an empty result means "user has no favorites" or + # "token lacks `collection.read` scope and needs reconnect". The + # iter helper sets `self._collection_needs_reconnect = True` when + # the endpoint returns 401/403 so the listing endpoint can surface + # a user-actionable hint instead of silently hiding the row. + + _COLLECTION_TRACKS_PATH = "userCollectionTracks/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 + 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 + distinguish 'empty collection' from 'missing scope'.""" + # Reset on every call so a successful walk clears any stale flag + # left from a prior failed attempt. + self._collection_needs_reconnect = False + + if not self._ensure_valid_token(): + logger.debug("Tidal not authenticated — cannot fetch collection tracks") + return [] + + track_ids: List[str] = [] + next_path: Optional[str] = None + + while True: + if next_path: + # `links.next` from Tidal is path-relative to /v2 root, + # already carries every query param we need (cursor + countryCode + locale + include). + 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}" + params = { + 'countryCode': 'US', + 'locale': 'en-US', + 'include': 'items', + } + + try: + headers = { + 'accept': 'application/vnd.api+json', + 'Authorization': f'Bearer {self.access_token}', + } + logger.info( + f"Tidal collection request: GET {url} (params={params})" + ) + resp = requests.get(url, params=params, headers=headers, timeout=15) + logger.info( + f"Tidal collection response: status={resp.status_code} " + f"body[:300]={resp.text[:300]!r}" + ) + except Exception as e: + logger.warning(f"Tidal collection page request failed: {e}") + break + + if resp.status_code != 200: + # 401/403 = scope/permission issue. Token predates the + # `collection.read` scope expansion or the user revoked + # it — flag for the UI hint and bail. + if resp.status_code in (401, 403): + self._collection_needs_reconnect = True + logger.info( + "Tidal collection endpoint returned %s — reconnect Tidal " + "in Settings → Connections to grant `collection.read` scope.", + resp.status_code, + ) + else: + logger.warning( + f"Tidal collection page returned {resp.status_code}: {resp.text[:500]}" + ) + break + + try: + data = resp.json() + except ValueError as e: + logger.debug(f"Tidal collection response not JSON: {e}") + break + + for item in data.get('data', []): + if item.get('type') != 'tracks': + 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 + + next_path = data.get('links', {}).get('next') + if not next_path: + break + + time.sleep(0.3) # Cursor pagination courtesy delay + + return track_ids + + def collection_needs_reconnect(self) -> bool: + """True when the most recent collection fetch hit a 401/403 — + i.e. the saved token doesn't have `collection.read` scope and + the user needs to reconnect Tidal in Settings → Connections. + + Reset to False at the start of every `_iter_collection_track_ids` + call so a successful walk clears stale flags.""" + return getattr(self, '_collection_needs_reconnect', False) + + def get_collection_tracks_count(self) -> int: + """Total count of tracks in the user's Favorite Tracks. + + Tidal's cursor pagination doesn't expose a `meta.total` so we + walk the full ID list. Cheap relative to hydration (one small + request per 20 tracks, no per-track lookups), but linear in + collection size — call sparingly.""" + try: + return len(self._iter_collection_track_ids()) + except Exception as e: + logger.debug(f"Failed to get Tidal collection tracks count: {e}") + return 0 + + def get_collection_tracks(self, limit: Optional[int] = None) -> List[Track]: + """Fetch user's favorited tracks with full artist + album + metadata hydrated via `_get_tracks_batch`. + + Returns the same `Track` shape every other Tidal playlist + method returns, so the virtual-playlist plumbing in + `web_server.py` can reuse the existing serialization path.""" + try: + track_ids = self._iter_collection_track_ids(max_ids=limit) + if not track_ids: + return [] + + hydrated: List[Track] = [] + for i in range(0, len(track_ids), self._COLLECTION_BATCH_SIZE): + batch = track_ids[i:i + self._COLLECTION_BATCH_SIZE] + try: + hydrated.extend(self._get_tracks_batch(batch)) + except Exception as e: + logger.debug( + f"Tidal collection batch hydration failed for IDs {batch}: {e}" + ) + + logger.info( + f"Retrieved {len(hydrated)}/{len(track_ids)} tracks from Tidal Favorite Tracks" + ) + return hydrated + except Exception as e: + logger.error(f"Error fetching Tidal collection tracks: {e}") + return [] + # Global instance _tidal_client = None diff --git a/tests/test_tidal_collection_tracks.py b/tests/test_tidal_collection_tracks.py new file mode 100644 index 00000000..b821a93c --- /dev/null +++ b/tests/test_tidal_collection_tracks.py @@ -0,0 +1,443 @@ +"""Pin Tidal "Favorite Tracks" virtual-playlist behavior. + +GitHub issue #502 (Yug1900): expose the user's favorited tracks +(My Collection) as a virtual playlist alongside their real playlists, +mirroring how Spotify's "Liked Songs" is treated. The endpoint Tidal +exposes is cursor-paginated (`GET /v2/userCollectionTracks/me/ +relationships/items?include=items`) and the response only carries +track-level attributes — artist + album NAMES need a second pass via +the existing `_get_tracks_batch` hydration helper. + +These tests pin: +- ID enumeration via the cursor chain (single page, multi-page, + short-circuit on `max_ids`) +- Auth + permission failure paths (no token, 401/403 from + `collection.read` scope missing) +- Hydration delegates to `_get_tracks_batch` (no duplication of + the JSON:API artist/album parse) +- `get_playlist("tidal-favorites")` dispatches to the virtual + path (so every existing playlist-by-id consumer — mirror refresh, + discovery, detail endpoint — gets My Collection support for free) +- Count helper sums IDs across pages without hydrating +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from core.tidal_client import Track, Playlist, TidalClient + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +class _FakeResp: + """Minimal `requests.Response` stand-in — only the fields the + collection-fetch path reads.""" + + def __init__(self, status_code: int = 200, json_body=None, text: str = ""): + 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 + + +def _make_authed_client(): + """Build a minimal TidalClient with the auth-related state every + collection method checks. Avoids touching disk / network in + `__init__`.""" + 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" + return client + + +# Two-page collection response that exercises the cursor chain. +_PAGE_ONE = { + 'data': [ + {'id': '1001', 'type': 'tracks'}, + {'id': '1002', 'type': 'tracks'}, + {'id': '1003', 'type': 'tracks'}, + ], + 'links': { + 'next': '/userCollectionTracks/me/relationships/items?cursor=ABC', + }, +} +_PAGE_TWO = { + 'data': [ + {'id': '1004', 'type': 'tracks'}, + {'id': '1005', 'type': 'tracks'}, + ], + 'links': {}, # no `next` — end of cursor chain +} + + +# --------------------------------------------------------------------------- +# _iter_collection_track_ids +# --------------------------------------------------------------------------- + + +class TestIterCollectionTrackIds: + def test_walks_full_cursor_chain(self): + """Both pages enumerated, IDs preserved in cursor order.""" + client = _make_authed_client() + + responses = iter([_FakeResp(200, _PAGE_ONE), _FakeResp(200, _PAGE_TWO)]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['1001', '1002', '1003', '1004', '1005'] + + def test_max_ids_short_circuits_mid_page(self): + """`max_ids` cap stops enumeration without fetching the next + page — important for the count-with-cap callers we may add + later. Cap of 2 returns only the first two IDs.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(200, _PAGE_ONE)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids(max_ids=2) + + assert ids == ['1001', '1002'] + + def test_max_ids_short_circuits_at_page_boundary(self): + """Cap exactly equal to one page's worth — we should NOT make + the second request even though the cursor chain says there is + a next page.""" + client = _make_authed_client() + call_count = {'n': 0} + + def fake_get(*args, **kwargs): + call_count['n'] += 1 + return _FakeResp(200, _PAGE_ONE) + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=fake_get), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids(max_ids=3) + + assert ids == ['1001', '1002', '1003'] + assert call_count['n'] == 1, "Should not have fetched the second cursor page" + + def test_no_token_returns_empty_without_request(self): + """Auth precheck failure short-circuits before any HTTP.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=False), \ + patch('core.tidal_client.requests.get') as mock_get: + ids = client._iter_collection_track_ids() + + assert ids == [] + assert not mock_get.called + + def test_401_response_breaks_loop(self): + """Tokens predating the `collection.read` scope expansion will + return 401. We log + bail rather than retry endlessly.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(401, text="unauthorized")), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == [] + + def test_403_response_breaks_loop(self): + """Same defensive bail for 403 (forbidden — scope or product + tier issue).""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(403, text="forbidden")), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == [] + + def test_401_sets_needs_reconnect_flag(self): + """The single most common 'why is my collection empty' cause: + existing token predates the `collection.read` scope. Listing + endpoint reads `collection_needs_reconnect()` and surfaces a + user-actionable hint instead of silently hiding the row.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(401)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is True + + def test_403_sets_needs_reconnect_flag(self): + """403 = scope/product-tier issue — same surface treatment as 401.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(403)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is True + + def test_successful_walk_clears_stale_reconnect_flag(self): + """User reconnects → next iter call MUST clear the prior + flag. Otherwise the listing endpoint keeps showing the + reconnect hint forever even after the scope is granted.""" + client = _make_authed_client() + client._collection_needs_reconnect = True # Simulate stale flag + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(200, _PAGE_TWO)), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is False + + def test_500_does_not_set_reconnect_flag(self): + """Server-side errors (5xx, network timeout) are NOT a scope + problem — must NOT poison the flag. User shouldn't be told + to reconnect just because Tidal had a hiccup.""" + client = _make_authed_client() + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(500, text="server error")), \ + patch('core.tidal_client.time.sleep'): + client._iter_collection_track_ids() + + assert client.collection_needs_reconnect() is False + + def test_skips_non_tracks_data_entries(self): + """The endpoint may surface non-track relationship entries on + future schema additions — we only collect `type == 'tracks'` + IDs so a forward-compatible response shape doesn't poison the + ID list with unrelated resources.""" + client = _make_authed_client() + weird_page = { + 'data': [ + {'id': '999', 'type': 'tracks'}, + {'id': 'pl-1', 'type': 'playlists'}, # ignored + {'id': '1000', 'type': 'tracks'}, + ], + 'links': {}, + } + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(200, weird_page)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == ['999', '1000'] + + def test_empty_data_on_first_page_returns_empty(self): + """Empty collection — clean empty list, no errors.""" + client = _make_authed_client() + empty = {'data': [], 'links': {}} + + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', return_value=_FakeResp(200, empty)), \ + patch('core.tidal_client.time.sleep'): + ids = client._iter_collection_track_ids() + + assert ids == [] + + +# --------------------------------------------------------------------------- +# get_collection_tracks_count +# --------------------------------------------------------------------------- + + +class TestGetCollectionTracksCount: + def test_returns_total_across_pages(self): + """Count = sum of IDs across the full cursor chain.""" + client = _make_authed_client() + + responses = iter([_FakeResp(200, _PAGE_ONE), _FakeResp(200, _PAGE_TWO)]) + with patch.object(client, '_ensure_valid_token', return_value=True), \ + patch('core.tidal_client.requests.get', side_effect=lambda *a, **kw: next(responses)), \ + patch('core.tidal_client.time.sleep'): + assert client.get_collection_tracks_count() == 5 + + def test_returns_zero_on_failure(self): + """Wrapping handler swallows exceptions — caller treats any + failure as 'no collection tracks' rather than propagating.""" + client = _make_authed_client() + + with patch.object(client, '_iter_collection_track_ids', side_effect=RuntimeError("boom")): + assert client.get_collection_tracks_count() == 0 + + def test_returns_zero_when_unauthenticated(self): + client = _make_authed_client() + with patch.object(client, '_ensure_valid_token', return_value=False): + assert client.get_collection_tracks_count() == 0 + + +# --------------------------------------------------------------------------- +# get_collection_tracks +# --------------------------------------------------------------------------- + + +class TestGetCollectionTracks: + def test_hydrates_via_existing_batch_helper(self): + """Hydration MUST delegate to `_get_tracks_batch` rather than + reimplement the JSON:API artist/album parse — that's the + existing battle-tested path. This test verifies the dispatch + + that the hydrated tracks come back in the same order the + IDs were enumerated.""" + client = _make_authed_client() + + ordered_ids = ['1001', '1002', '1003'] + fake_tracks = [ + Track(id='1001', name='Times Like These', artists=['Foo Fighters'], album='One by One'), + Track(id='1002', name='Innerbloom', artists=['RÜFÜS DU SOL'], album='Bloom'), + Track(id='1003', name='Set Fire to the Rain', artists=['Adele'], album='21'), + ] + + captured_batches = [] + + def fake_batch(ids): + captured_batches.append(list(ids)) + id_to_track = {t.id: t for t in fake_tracks} + return [id_to_track[i] for i in ids if i in id_to_track] + + with patch.object(client, '_iter_collection_track_ids', return_value=ordered_ids), \ + patch.object(client, '_get_tracks_batch', side_effect=fake_batch): + result = client.get_collection_tracks() + + assert [t.id for t in result] == ['1001', '1002', '1003'] + assert [t.name for t in result] == ['Times Like These', 'Innerbloom', 'Set Fire to the Rain'] + # First (and only) batch should contain all three IDs since + # default _COLLECTION_BATCH_SIZE is well above 3. + assert captured_batches == [['1001', '1002', '1003']] + + def test_chunks_into_batch_size(self): + """Pin the batching: 41 IDs at batch size 20 → three batches + of 20 / 20 / 1. The Tidal `filter[id]` cap is 20 so we can't + send everything in one request.""" + client = _make_authed_client() + + ids = [str(1000 + i) for i in range(41)] + captured_batches = [] + + def fake_batch(batch): + captured_batches.append(list(batch)) + return [Track(id=tid, name=f'Track {tid}', artists=['A'], album='Alb') for tid in batch] + + with patch.object(client, '_iter_collection_track_ids', return_value=ids), \ + patch.object(client, '_get_tracks_batch', side_effect=fake_batch): + result = client.get_collection_tracks() + + assert len(result) == 41 + assert [len(b) for b in captured_batches] == [20, 20, 1] + + def test_partial_batch_failure_continues(self): + """One failed batch shouldn't abort the whole fetch — the rest + of the collection should still come back. Defensive against + transient Tidal errors mid-walk.""" + client = _make_authed_client() + ids = ['1001', '1002', '1003'] + + def fake_batch(batch): + if batch == ['1002']: # won't actually hit since batch_size > 1, but illustrate + raise RuntimeError("transient") + return [Track(id=tid, name=f'T{tid}', artists=['A'], album='Alb') for tid in batch] + + with patch.object(client, '_iter_collection_track_ids', return_value=ids), \ + patch.object(client, '_get_tracks_batch', side_effect=lambda b: (_ for _ in ()).throw(RuntimeError("transient")) if b == ids else []): + result = client.get_collection_tracks() + + # All batches failed → empty result, but no exception bubbled + assert result == [] + + def test_no_ids_returns_empty_without_hydrating(self): + """Empty collection short-circuits before any batch call.""" + client = _make_authed_client() + + with patch.object(client, '_iter_collection_track_ids', return_value=[]), \ + patch.object(client, '_get_tracks_batch') as mock_batch: + result = client.get_collection_tracks() + + assert result == [] + assert not mock_batch.called + + def test_limit_passed_through_to_iter(self): + """`limit` arg caps the ID walk so we don't hydrate everything + when the caller only wants a slice.""" + client = _make_authed_client() + captured_max = {'value': None} + + def fake_iter(max_ids=None): + captured_max['value'] = max_ids + return ['1001', '1002'] + + with patch.object(client, '_iter_collection_track_ids', side_effect=fake_iter), \ + patch.object(client, '_get_tracks_batch', return_value=[]): + client.get_collection_tracks(limit=50) + + assert captured_max['value'] == 50 + + +# --------------------------------------------------------------------------- +# get_playlist virtual-id dispatch +# --------------------------------------------------------------------------- + + +class TestGetPlaylistVirtualId: + def test_my_collection_id_returns_virtual_playlist(self): + """Pin the dispatch — `get_playlist("tidal-favorites")` + must NOT hit the real /playlists/ endpoint and must NOT + require token validation (the collection methods do their own). + It returns a synthetic Playlist with the hydrated collection + tracks, so every existing call site (mirror refresh @ line + 1192, discovery start @ line 20835, detail endpoint @ line + 20725) gets My Collection support without per-site changes.""" + client = _make_authed_client() + fake_collection = [ + Track(id='1001', name='Times Like These', artists=['Foo Fighters']), + Track(id='1002', name='Innerbloom', artists=['RÜFÜS DU SOL']), + ] + + with patch.object(client, 'get_collection_tracks', return_value=fake_collection), \ + patch.object(client, '_ensure_valid_token') as mock_token, \ + patch('core.tidal_client.requests.get') as mock_get: + playlist = client.get_playlist("tidal-favorites") + + assert isinstance(playlist, Playlist) + assert playlist.id == "tidal-favorites" + assert playlist.name == "Favorite Tracks" + assert playlist.description == "Your favorited tracks on Tidal" + assert len(playlist.tracks) == 2 + assert playlist.tracks[0].id == '1001' + # Virtual path should NOT touch the real /playlists/ + # endpoint OR the auth precheck (get_collection_tracks + # handles its own auth gate downstream). + assert not mock_get.called + assert not mock_token.called + + def test_real_playlist_id_falls_through_to_normal_path(self): + """Sanity: a real playlist ID must NOT route to the virtual + handler. Token check + HTTP request still happen.""" + client = _make_authed_client() + client.session = SimpleNamespace( + get=lambda *a, **kw: _FakeResp(404, text="not found"), + headers={}, + ) + + with patch.object(client, 'get_collection_tracks') as mock_collection, \ + patch.object(client, '_ensure_valid_token', return_value=True): + # 404 from the fake session → returns None, but more + # importantly the virtual-handler MUST NOT have been called. + client.get_playlist("real-playlist-uuid") + + assert not mock_collection.called diff --git a/web_server.py b/web_server.py index 11a74ede..531e5db3 100644 --- a/web_server.py +++ b/web_server.py @@ -5641,15 +5641,23 @@ def auth_tidal(): with tidal_oauth_lock: tidal_oauth_state["profile_id"] = profile_id if profile_id and profile_id != '1' else None - # Create OAuth URL + # Create OAuth URL. + # `collection.read` is required for the `userCollectionTracks` + # endpoint that powers the virtual "Favorite Tracks" playlist + # (issue #502). `prompt=consent` forces Tidal to display the + # consent screen even when the app is already authorized — without + # it, re-authenticating after a scope expansion can silently + # return a token carrying only the ORIGINAL scope set because + # Tidal treats the existing authorization as still valid. import urllib.parse params = { 'response_type': 'code', 'client_id': temp_tidal_client.client_id, 'redirect_uri': temp_tidal_client.redirect_uri, - 'scope': 'user.read playlists.read', + 'scope': 'user.read playlists.read collection.read', 'code_challenge': temp_tidal_client.code_challenge, - 'code_challenge_method': 'S256' + 'code_challenge_method': 'S256', + 'prompt': 'consent', } auth_url = f"{temp_tidal_client.auth_url}?" + urllib.parse.urlencode(params) @@ -20680,6 +20688,26 @@ def qobuz_auth_logout(): # TIDAL PLAYLIST API ENDPOINTS # =================================================================== +@app.route('/api/tidal/disconnect', methods=['POST']) +def tidal_disconnect(): + """Clear saved Tidal auth state. Use when re-authentication doesn't + pick up newly-added scopes (e.g. existing token predates a scope + expansion and `prompt=consent` alone isn't enough to force fresh + consent on this user's auth flow).""" + if not tidal_client: + return jsonify({"error": "Tidal client not available."}), 500 + try: + tidal_client.disconnect() + return jsonify({ + 'success': True, + 'message': 'Tidal disconnected. Re-authenticate from Settings → Connections.', + 'authenticated': False, + }) + except Exception as e: + logger.error(f"Tidal disconnect error: {e}") + return jsonify({"error": str(e)}), 500 + + @app.route('/api/tidal/playlists', methods=['GET']) def get_tidal_playlists(): """Fetches all user playlists from Tidal with full track data (like sync.py).""" @@ -20716,7 +20744,56 @@ def get_tidal_playlists(): } for t in p.tracks] playlist_data.append(playlist_dict) - + + # Append virtual "Favorite Tracks" playlist at the END (mirrors + # Spotify's "Liked Songs" treatment — count-only here, full + # track fetch deferred to the per-playlist detail endpoint). + # When the saved Tidal token doesn't have `collection.read` + # scope (existing tokens predate the scope expansion), the + # endpoint returns 401 — we still surface the entry but with + # a `needs_reconnect` flag + a reconnect-hint name so the user + # has something visible to act on instead of a silently missing + # row. + try: + from core.tidal_client import ( + COLLECTION_PLAYLIST_ID, + COLLECTION_PLAYLIST_NAME, + COLLECTION_PLAYLIST_DESCRIPTION, + ) + collection_count = tidal_client.get_collection_tracks_count() + needs_reconnect = tidal_client.collection_needs_reconnect() + + if needs_reconnect: + playlist_data.append({ + "id": COLLECTION_PLAYLIST_ID, + "name": f"{COLLECTION_PLAYLIST_NAME} (reconnect Tidal to enable)", + "owner": "You", + "track_count": 0, + "image_url": None, + "description": "Reconnect Tidal in Settings → Connections to grant the new collection.read scope.", + "needs_reconnect": True, + "tracks": [], + }) + logger.info( + "Tidal Favorite Tracks: token missing `collection.read` scope — surfacing reconnect hint." + ) + elif collection_count > 0: + playlist_data.append({ + "id": COLLECTION_PLAYLIST_ID, + "name": COLLECTION_PLAYLIST_NAME, + "owner": "You", + "track_count": collection_count, + "image_url": None, + "description": COLLECTION_PLAYLIST_DESCRIPTION, + "tracks": [], + }) + logger.info( + f"Added virtual '{COLLECTION_PLAYLIST_NAME}' playlist with {collection_count} tracks (count only)" + ) + except Exception as collection_error: + logger.error(f"Failed to add Tidal Favorite Tracks playlist: {collection_error}") + # Don't fail the entire request if Favorite Tracks fails + logger.info(f"Loaded {len(playlist_data)} Tidal playlists with track data") return jsonify(playlist_data) except Exception as e: @@ -20730,7 +20807,10 @@ def get_tidal_playlist_tracks(playlist_id): try: logger.info(f"Getting full Tidal playlist with tracks for: {playlist_id}") - # Fetch this single playlist directly — no need to re-fetch all playlists + # Fetch this single playlist directly — no need to re-fetch all playlists. + # `get_playlist` recognizes the virtual `tidal-favorites` ID and + # dispatches to the userCollectionTracks endpoint internally, so + # the rest of this handler treats it identically to a real playlist. full_playlist = tidal_client.get_playlist(playlist_id) if not full_playlist: return jsonify({"error": "Playlist not found or unable to access. This may be due to privacy settings or Tidal API restrictions."}), 404 diff --git a/webui/index.html b/webui/index.html index fd5e5ef3..77fb01f4 100644 --- a/webui/index.html +++ b/webui/index.html @@ -3766,6 +3766,8 @@
+
diff --git a/webui/static/helper.js b/webui/static/helper.js index ee1f3b3f..3a9e3046 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3416,6 +3416,7 @@ const WHATS_NEW = { '2.4.3': [ // --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- { date: 'Unreleased — 2.4.3 patch work' }, + { title: 'Tidal: Favorite Tracks Now Show Up As A Playlist (Same As Spotify Liked Songs)', desc: 'github issue #502 (yug1900): tidal users wanted their favorited tracks ("my collection" in the tidal app) to appear alongside their normal playlists in the sync tab — same treatment spotify gets for "liked songs". prior attempt at this surfaced empty data because the wrong endpoint was being hit (`/v2/favorites?filter[type]=TRACKS` returns nothing for personal favorites — that endpoint is scoped to collections the third-party app created itself, not the user\'s app-level favorites). reporter located the working endpoint: `GET /v2/userCollectionTracks/me/relationships/items?countryCode=US&locale=en-US&include=items`. cursor-paginated (20 per page, follow `links.next` with `page[cursor]=...` until exhausted), responses only carry track-level attributes — artist + album NAMES come back as relationship-link stubs, not embedded data. fix: two-phase fetch. phase one walks the cursor chain to enumerate every track id (cheap, IDs only). phase two batch-hydrates 20 IDs at a time through the existing `_get_tracks_batch` helper which already knows how to `include=artists,albums` and produce fully-populated `Track` objects matching the rest of the codebase — no duplication of the JSON:API artist/album parse, no new dataclass shape. virtual playlist `tidal-favorites` appended to the end of `/api/tidal/playlists` (mirrors spotify\'s liked-songs placement). id intentionally has NO colon — sync-services.js renderer interpolates ids into css selectors via template literals (`#tidal-card-${p.id} .foo`) and a colon would parse as a css pseudo-class operator. `tidal_client.get_playlist("tidal-favorites")` recognizes the virtual id and dispatches to the collection path internally, so every existing per-id consumer gets it for free: per-playlist detail endpoint, mirror auto-refresh automation, "build spotify discovery from tidal playlist" flow. needs token reconnect to grant the new `collection.read` oauth scope (added to the auth flow). existing tokens hit a 401 — the client now sets a `_collection_needs_reconnect` flag and the listing endpoint surfaces a placeholder card titled "Favorite Tracks (reconnect Tidal to enable)" with a description pointing at settings, so the user has something visible to act on instead of a silently missing row. 22 new tests pin the cursor walk (full chain, max-ids cap mid-page + at page boundary), auth gates (no token / 401 / 403 all bail clean), reconnect-flag lifecycle (set on 401/403, cleared on next successful walk, NOT set on 5xx so transient server errors don\'t falsely tell the user to reconnect), forward-compat type filter (non-track entries skipped), count helper, batch hydration delegation + chunking at the 20-per-batch cap, partial-batch failure containment, and the virtual-id dispatch (real playlist ids still flow through the normal path).', page: 'sync' }, { title: 'Library Reorganize: Stop Leaving Orphan Audio Files Behind + Hint For Unknown-Artist Rows', desc: 'discord report (foxxify): library reorganize wasn\'t organizing everything. two distinct gaps. (A) lossy-copy users have `track.flac` AND `track.opus` side-by-side at the source; the db only knows about ONE of those (whichever is the canonical library entry). reorganize moved the canonical, left the other format orphaned at the old location, and the empty-folder cleanup never fired because the source dir still had audio in it. fix: at the per-track finalize step the reorganize code now scans the source dir for sibling-stem audio files (same filename stem, audio extension, different format), moves them to the same destination dir as the canonical with the renamed stem + their original extension, then proceeds with the existing source removal + cleanup. preserves both formats post-move so users keep their flac archive AND their opus library copy. (B) old "Unknown Artist / album_id / 0 tracks" rows left over from the pre-#524 manual-import bug couldn\'t be relocated because the album row has no usable metadata source id — reorganize emitted a generic "run enrichment first" message that doesn\'t apply (enrichment can\'t fix these rows; they need their real metadata recovered from file tags). these are the existing `Fix Unknown Artists` repair job\'s domain — reads file tags, re-resolves artist/album/track via configured metadata source, re-tags + moves. reorganize now detects the bad-metadata shape (Unknown Artist OR album.title that\'s a 6+ digit numeric id) and emits a clear "run the Fix Unknown Artists repair job to recover real artist/album from file tags first" hint instead, pointing the user at the right tool. fixer was already implemented and handles the case end-to-end — discoverability gap, not a logic gap. 31 new tests pin: orphan-format detection (canonical-vs-sibling, multi-format, defensive on missing source dir, sidecar exclusion), sibling-move with renamed-stem propagation + dst-dir creation + idempotent re-runs + os-failure handling, and the unknown-artist-hint detection helpers (placeholder names, numeric-id title detection at 6+ digit cutoff, real-album-with-no-source-id keeps the generic enrichment hint, strict-source mode preserved when artist/title look fine).', page: 'library' }, { title: 'AcoustID Scanner: Compilation Albums No Longer Flag Every Track', desc: 'discord report (skowl): downloaded a compilation album like "high tea music: vol 1" where every track has a different artist (eclypse, andromedik, t & sugah, gourski, himmes, sektor, lexurus, etc.) and the acoustid scanner flagged every single track as wrong song — the file tag had the correct per-track artist (e.g. "eclypse" for "city lights") but the scanner compared against the album-level artist ("andromedik", the curator). raw similarity 12% → wrong song flag. the multi-value-credit fix from the prior pr (foxxify) didn\'t help because both sides were single-value but DIFFERENT artists. cause: scanner sql joined `artists` table via `tracks.artist_id` which points at the ALBUM artist, not the per-track artist. but `tracks.track_artist` column was already populated with the correct per-track value by every server scan + auto-import path that handles compilations. scanner just wasn\'t reading it. fix: changed the scanner select to `COALESCE(NULLIF(t.track_artist, \'\'), ar.name)` — prefers per-track artist when populated, falls back to album artist for legacy rows / single-artist albums where track_artist is null. NULLIF handles the empty-string-instead-of-null case for legacy data. composes with foxxify\'s multi-value fix — for the rare compilation track where acoustid ALSO returns a multi-value credit, both paths work together. 2 new tests pin: compilation track uses per-track artist (reporter\'s exact case), null/empty track_artist falls back to album artist via coalesce.', page: 'library' }, { title: 'AcoustID Scanner: Multi-Artist Songs No Longer Flagged As Wrong', desc: 'discord report (foxxify): the acoustid scanner repair job was flagging multi-artist tracks as "wrong song" because acoustid returns the full credit ("okayracer, aldrch & poptropicaslutz!") while the library db carries only the primary artist ("okayracer"). raw similarity scored ~43% — well below the 60% threshold — so the scanner created a wrong-song finding even though the audio was correct. user couldn\'t fix without lowering the global artist threshold to ~30% (which would let real mismatches through). cause: scanner used raw `SequenceMatcher` comparison that doesn\'t recognise the primary artist is just one of several contributors in the credit string. fix: extended the shared `core/matching/artist_aliases.py::artist_names_match` helper (lifted in #441) with credit-token splitting on common separators (comma, ampersand, semicolon, slash, plus, "feat.", "ft.", "featuring", "with", "vs.", "x"). when actual artist contains separators, helper splits into individual contributors and checks each against expected — primary-in-credit cases now resolve at 100% instead of 43%. composes with existing alias path so cross-script multi-artist credits ("hiroyuki sawano" expected, "澤野弘之, featured" actual) work too. wired into `core/repair_jobs/acoustid_scanner.py` — replaces the raw similarity call. acoustid post-download verifier already used the helper from #441 so it inherits the same fix automatically. 14 new tests pin: split-by-separator across 12 credit-string formats, primary at start/middle/end of credit, no-mask on genuine mismatches, single-token actual falls through to direct compare, multi-value composes with aliases, threshold still respected, end-to-end scanner integration with reporter\'s exact case (okayracer in okayracer-aldrch-poptropicaslutz credit → no finding), end-to-end scanner still flags genuine mismatches.', page: 'library' }, diff --git a/webui/static/settings.js b/webui/static/settings.js index 274b2561..dd7bd888 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -3397,6 +3397,32 @@ async function authenticateTidal() { } } +async function disconnectTidal() { + // Clear saved Tidal token. Use when re-authentication doesn't pick + // up newly-added scopes (existing token predates a scope expansion + // and `prompt=consent` alone isn't forcing fresh consent on this + // user's auth flow). After disconnect, click Authenticate again + // for a clean grant. + if (!confirm('Disconnect Tidal? Saved token will be cleared and you\'ll need to re-authenticate.')) { + return; + } + try { + showLoadingOverlay('Disconnecting Tidal...'); + const resp = await fetch('/api/tidal/disconnect', { method: 'POST' }); + const data = await resp.json(); + if (resp.ok && data.success) { + showToast('Tidal disconnected. Click Authenticate to reconnect with current scopes.', 'success'); + } else { + showToast(`Disconnect failed: ${data.error || 'unknown error'}`, 'error'); + } + } catch (error) { + console.error('Error disconnecting Tidal:', error); + showToast('Failed to disconnect Tidal', 'error'); + } finally { + hideLoadingOverlay(); + } +} + async function authenticateDeezer() { try { showLoadingOverlay('Saving credentials and starting Deezer authentication...');