From 7519c3d50c5ce3f01e05a4430fa9bbb94e77cac4 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 4 May 2026 23:14:05 -0700 Subject: [PATCH] Cin-5: Drop per-source attrs from orchestrator Removed the eight backward-compat attribute aliases on the orchestrator (soulseek, youtube, tidal, qobuz, hifi, deezer_dl, lidarr, soundcloud). External callers and the orchestrator's own internals now reach clients through the generic alias-aware client(name) accessor. - core/downloads/{master,monitor,validation}.py: migrated to client(). Monitor's per-source aggregation loop replaced with a single engine.get_all_downloads() call. - core/search/{orchestrator,stream}.py: migrated; stream.py drops the hand-built mode-to-client dict. - web_server.py: migrated /api/deezer/arl-* + tidal client lookup. - core/download_orchestrator.py: internal self.soulseek / self.deezer_dl reaches now route through self.client(); attr assignments dropped from __init__; module docstring updated. - Test fakes (_FakeSoulseek, _FakeSoulseekWithYT) expose client(name) instead of stuffing per-source attributes. - Conformance test re-pinned to the client() accessor contract. --- core/download_orchestrator.py | 56 +++++++++---------- core/downloads/master.py | 2 +- core/downloads/monitor.py | 20 +++---- core/downloads/validation.py | 2 +- core/qobuz_client.py | 2 +- core/search/orchestrator.py | 7 ++- core/search/stream.py | 12 +--- tests/downloads/test_download_orchestrator.py | 14 +---- tests/search/test_search_orchestrator.py | 5 +- tests/search/test_search_stream.py | 17 ++++-- .../test_download_orchestrator_soundcloud.py | 40 ++++++------- tests/test_download_plugin_conformance.py | 20 +++---- tests/test_qobuz_credential_sync.py | 2 +- web_server.py | 17 ++---- webui/static/helper.js | 1 + 15 files changed, 100 insertions(+), 117 deletions(-) diff --git a/core/download_orchestrator.py b/core/download_orchestrator.py index 8cdb3148..88ccfc90 100644 --- a/core/download_orchestrator.py +++ b/core/download_orchestrator.py @@ -14,10 +14,9 @@ Supports eight modes: The orchestrator dispatches through ``core.download_plugins.registry`` instead of hardcoded per-source ``[self.soulseek, self.youtube, ...]`` -lists. The ``self.`` attributes are preserved for backward -compatibility — anything outside this file that reaches in for a -specific client (e.g. ``orchestrator.soulseek._make_request`` for -Soulseek-specific internals) keeps working unchanged. +lists. External callers reach individual clients via the generic +``orchestrator.client('')`` accessor (alias-aware), not direct +attribute access. """ import asyncio @@ -57,19 +56,6 @@ class DownloadOrchestrator: self.registry.initialize() self._init_failures = self.registry.init_failures - # Backward-compat attribute aliases so existing callers like - # ``orchestrator.soulseek._make_request(...)`` keep working - # unchanged. The registry is the source of truth for dispatch; - # these are convenience handles for source-specific internals. - self.soulseek = self.registry.get('soulseek') - self.youtube = self.registry.get('youtube') - self.tidal = self.registry.get('tidal') - self.qobuz = self.registry.get('qobuz') - self.hifi = self.registry.get('hifi') - self.deezer_dl = self.registry.get('deezer') - self.lidarr = self.registry.get('lidarr') - self.soundcloud = self.registry.get('soundcloud') - # Engine — owns cross-source state, threading, search retry, # rate-limits, fallback. Built in subsequent phases. For Phase # B it's just an empty registry of plugins so future phases @@ -106,15 +92,17 @@ class DownloadOrchestrator: self.hybrid_order = config_manager.get('download_source.hybrid_order', ['hifi', 'youtube', 'soulseek']) # Reload underlying client configs (SLSKD URL, API key, etc.) - if self.soulseek: - self.soulseek._setup_client() + soulseek = self.client('soulseek') + if soulseek: + soulseek._setup_client() logger.info("Soulseek client config reloaded") # Reconnect Deezer if ARL changed deezer_arl = config_manager.get('deezer_download.arl', '') - if deezer_arl and self.deezer_dl: - self.deezer_dl.reconnect(deezer_arl) - self.deezer_dl._quality = config_manager.get('deezer_download.quality', 'flac') + deezer_dl = self.client('deezer_dl') + if deezer_arl and deezer_dl: + deezer_dl.reconnect(deezer_arl) + deezer_dl._quality = config_manager.get('deezer_download.quality', 'flac') # Reload download path for all clients that cache it. # Soulseek owns the path config and is reloaded above; every @@ -381,7 +369,8 @@ class DownloadOrchestrator: elif is_streaming: filtered_results = tracks else: - filtered_results = self.soulseek.filter_results_by_quality_preference(tracks) if self.soulseek else tracks + soulseek = self.client('soulseek') + filtered_results = soulseek.filter_results_by_quality_preference(tracks) if soulseek else tracks if not filtered_results: logger.warning(f"No suitable quality results found for: {query}") @@ -459,9 +448,10 @@ class DownloadOrchestrator: True if successful """ # This is Soulseek-specific, so only call on Soulseek client - if not self.soulseek: + soulseek = self.client('soulseek') + if not soulseek: return False - return await self.soulseek.signal_download_completion(download_id, username, remove) + return await soulseek.signal_download_completion(download_id, username, remove) async def clear_all_completed_downloads(self) -> bool: """Clear completed downloads from every source. Delegates @@ -485,9 +475,10 @@ class DownloadOrchestrator: Returns: API response """ - if not self.soulseek: + soulseek = self.client('soulseek') + if not soulseek: raise RuntimeError("Soulseek client not available (failed to initialize)") - return await self.soulseek._make_request(method, endpoint, **kwargs) + return await soulseek._make_request(method, endpoint, **kwargs) async def _make_direct_request(self, method: str, endpoint: str, **kwargs): """ @@ -502,9 +493,10 @@ class DownloadOrchestrator: Returns: API response """ - if not self.soulseek: + soulseek = self.client('soulseek') + if not soulseek: raise RuntimeError("Soulseek client not available (failed to initialize)") - return await self.soulseek._make_direct_request(method, endpoint, **kwargs) + return await soulseek._make_direct_request(method, endpoint, **kwargs) async def clear_all_searches(self) -> bool: """ @@ -513,7 +505,8 @@ class DownloadOrchestrator: Returns: True if successful """ - return await self.soulseek.clear_all_searches() if self.soulseek else True + soulseek = self.client('soulseek') + return await soulseek.clear_all_searches() if soulseek else True async def maintain_search_history_with_buffer(self, keep_searches: int = 50, trigger_threshold: int = 200) -> bool: """ @@ -526,7 +519,8 @@ class DownloadOrchestrator: Returns: True if successful """ - return await self.soulseek.maintain_search_history_with_buffer(keep_searches, trigger_threshold) if self.soulseek else True + soulseek = self.client('soulseek') + return await soulseek.maintain_search_history_with_buffer(keep_searches, trigger_threshold) if soulseek else True async def cancel_all_downloads(self) -> bool: """Cancel and remove all downloads from all sources. diff --git a/core/downloads/master.py b/core/downloads/master.py index e5c081d4..ff073637 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -372,7 +372,7 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma _sr.info(f"[Album Pre-flight] Searching for '{artist_name} {album_name}'") logger.info(f"[Album Pre-flight] Searching Soulseek for complete album: '{artist_name} - {album_name}'") - slsk = deps.soulseek_client.soulseek if hasattr(deps.soulseek_client, 'soulseek') else deps.soulseek_client + slsk = deps.soulseek_client.client('soulseek') if hasattr(deps.soulseek_client, 'client') else deps.soulseek_client # Try multiple query variations (banned keywords in artist/album name can return 0 results) album_queries = [f"{artist_name} {album_name}"] diff --git a/core/downloads/monitor.py b/core/downloads/monitor.py index 8f2bd06c..c66ad504 100644 --- a/core/downloads/monitor.py +++ b/core/downloads/monitor.py @@ -254,7 +254,8 @@ class WebUIDownloadMonitor: # Get Soulseek downloads from API transfers_data = None - if soulseek_active and soulseek_client and getattr(soulseek_client, 'soulseek', None) and soulseek_client.soulseek.base_url: + _slsk = soulseek_client.client('soulseek') if soulseek_client and hasattr(soulseek_client, 'client') else None + if soulseek_active and _slsk and _slsk.base_url: transfers_data = run_async(soulseek_client._make_request('GET', 'transfers/downloads')) if transfers_data: for user_data in transfers_data: @@ -266,18 +267,15 @@ class WebUIDownloadMonitor: key = _make_context_key(username, file_info.get('filename', '')) live_transfers[key] = file_info - # Also get non-Soulseek downloads (YouTube/Tidal/Qobuz/HiFi/Deezer/Lidarr) - # Call each client directly to avoid redundant slskd API call through orchestrator + # Also get non-Soulseek downloads via the engine — single + # cross-source aggregation, no per-source iteration. try: all_downloads = [] - for _dl_client in [soulseek_client.youtube, soulseek_client.tidal, soulseek_client.qobuz, - soulseek_client.hifi, soulseek_client.deezer_dl, soulseek_client.lidarr, - getattr(soulseek_client, 'soundcloud', None)]: - if _dl_client: - try: - all_downloads.extend(run_async(_dl_client.get_all_downloads())) - except Exception: - pass + if soulseek_client and hasattr(soulseek_client, 'engine'): + try: + all_downloads = run_async(soulseek_client.engine.get_all_downloads()) + except Exception: + pass for download in all_downloads: key = _make_context_key(download.username, download.filename) # Convert DownloadStatus to transfer dict format for monitor compatibility diff --git a/core/downloads/validation.py b/core/downloads/validation.py index 2b046739..5f8fced7 100644 --- a/core/downloads/validation.py +++ b/core/downloads/validation.py @@ -152,7 +152,7 @@ def get_valid_candidates(results, spotify_track, query): else: # Filter by user's quality profile before artist verification (Soulseek only) # Use existing soulseek_client to avoid re-initializing (which accesses download_path filesystem) - quality_filtered_candidates = soulseek_client.soulseek.filter_results_by_quality_preference(initial_candidates) + quality_filtered_candidates = soulseek_client.client('soulseek').filter_results_by_quality_preference(initial_candidates) # IMPORTANT: Respect empty results from quality filter # If user has strict quality requirements (e.g., FLAC-only with fallback disabled), diff --git a/core/qobuz_client.py b/core/qobuz_client.py index 983dce69..0b021983 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -529,7 +529,7 @@ class QobuzClient(DownloadSourcePlugin): """Pull session state from config without making a network probe. SoulSync runs two ``QobuzClient`` instances side by side — one wired - through ``soulseek_client.qobuz`` for the auth-flow endpoints, and a + through ``soulseek_client.client('qobuz')`` for the auth-flow endpoints, and a second owned by the enrichment worker for thread safety. When the user logs in via ``/api/qobuz/auth/login`` or ``/api/qobuz/auth/token`` only the auth-flow instance's in-memory state is updated; the worker's diff --git a/core/search/orchestrator.py b/core/search/orchestrator.py index 1c2cb4c1..ea8eb5af 100644 --- a/core/search/orchestrator.py +++ b/core/search/orchestrator.py @@ -289,10 +289,11 @@ def run_enhanced_search(query: str, requested_source: str, deps: SearchDeps) -> # --------------------------------------------------------------------------- def resolve_youtube_videos_client(deps: SearchDeps): - """Return the soulseek_client.youtube subclient or None when unavailable.""" - if not deps.soulseek_client: + """Return the YouTube download client (used for music-video search) + via the orchestrator's generic accessor, or None when unavailable.""" + if not deps.soulseek_client or not hasattr(deps.soulseek_client, 'client'): return None - return getattr(deps.soulseek_client, 'youtube', None) + return deps.soulseek_client.client('youtube') def stream_youtube_videos(query: str, youtube_client, run_async: Callable) -> Iterator[str]: diff --git a/core/search/stream.py b/core/search/stream.py index cae298f3..3b0293de 100644 --- a/core/search/stream.py +++ b/core/search/stream.py @@ -118,15 +118,9 @@ def stream_search_track( queries = _build_stream_queries(track_name, artist_name, effective_mode) - stream_clients = { - 'youtube': soulseek_client.youtube, - 'tidal': soulseek_client.tidal, - 'qobuz': soulseek_client.qobuz, - 'hifi': soulseek_client.hifi, - 'deezer_dl': soulseek_client.deezer_dl, - 'lidarr': soulseek_client.lidarr, - } - stream_client = stream_clients.get(effective_mode) + # Map mode name to canonical registry name (legacy 'deezer_dl' + # alias resolves to 'deezer' via the orchestrator's registry). + stream_client = soulseek_client.client(effective_mode) use_direct_client = stream_client is not None max_peer_queue = config_manager.get('soulseek.max_peer_queue', 0) or 0 diff --git a/tests/downloads/test_download_orchestrator.py b/tests/downloads/test_download_orchestrator.py index 6902f80c..84117068 100644 --- a/tests/downloads/test_download_orchestrator.py +++ b/tests/downloads/test_download_orchestrator.py @@ -51,14 +51,6 @@ def _build_orchestrator(**clients): orch = DownloadOrchestrator.__new__(DownloadOrchestrator) orch.registry = registry orch._init_failures = registry.init_failures - orch.soulseek = registry.get('soulseek') - orch.youtube = registry.get('youtube') - orch.tidal = registry.get('tidal') - orch.qobuz = registry.get('qobuz') - orch.hifi = registry.get('hifi') - orch.deezer_dl = registry.get('deezer') - orch.lidarr = registry.get('lidarr') - orch.soundcloud = registry.get('soundcloud') # Engine — orchestrator delegates per-source query/cancel # methods to it, so the test fixture must build one and # register every mock plugin under its canonical name. @@ -87,8 +79,8 @@ def test_clear_all_completed_downloads_ignores_unconfigured_clients(): result = _run_async(orch.clear_all_completed_downloads()) assert result is True - assert orch.soulseek.clear_calls == 1 - assert orch.youtube.clear_calls == 0 + assert orch.client('soulseek').clear_calls == 1 + assert orch.client('youtube').clear_calls == 0 def test_clear_all_completed_downloads_propagates_configured_failures(): @@ -99,7 +91,7 @@ def test_clear_all_completed_downloads_propagates_configured_failures(): result = _run_async(orch.clear_all_completed_downloads()) assert result is False - assert orch.soulseek.clear_calls == 1 + assert orch.client('soulseek').clear_calls == 1 # --------------------------------------------------------------------------- diff --git a/tests/search/test_search_orchestrator.py b/tests/search/test_search_orchestrator.py index 25082404..2c3cfa4b 100644 --- a/tests/search/test_search_orchestrator.py +++ b/tests/search/test_search_orchestrator.py @@ -462,7 +462,10 @@ class _FakeYouTube: class _FakeSoulseekWithYT: def __init__(self, youtube): - self.youtube = youtube + self._youtube = youtube + + def client(self, name): + return self._youtube if name == 'youtube' else None def test_resolve_youtube_videos_returns_subclient(): diff --git a/tests/search/test_search_stream.py b/tests/search/test_search_stream.py index fc12f724..c62e9c54 100644 --- a/tests/search/test_search_stream.py +++ b/tests/search/test_search_stream.py @@ -53,15 +53,20 @@ class _FakeStreamClient: class _FakeSoulseek: def __init__(self, youtube=None, tidal=None, qobuz=None, hifi=None, deezer_dl=None, lidarr=None, results_per_query=None): - self.youtube = youtube - self.tidal = tidal - self.qobuz = qobuz - self.hifi = hifi - self.deezer_dl = deezer_dl - self.lidarr = lidarr + self._clients = { + 'youtube': youtube, + 'tidal': tidal, + 'qobuz': qobuz, + 'hifi': hifi, + 'deezer_dl': deezer_dl, + 'lidarr': lidarr, + } self._results = results_per_query or {} self.search_calls = [] + def client(self, name): + return self._clients.get(name) + async def search(self, query, timeout=15): self.search_calls.append(query) return self._results.get(query, ([], [])) diff --git a/tests/test_download_orchestrator_soundcloud.py b/tests/test_download_orchestrator_soundcloud.py index 43d240df..a3d1ab0a 100644 --- a/tests/test_download_orchestrator_soundcloud.py +++ b/tests/test_download_orchestrator_soundcloud.py @@ -39,18 +39,18 @@ def orchestrator() -> DownloadOrchestrator: def test_orchestrator_constructs_soundcloud_client(orchestrator: DownloadOrchestrator) -> None: - assert orchestrator.soundcloud is not None - assert isinstance(orchestrator.soundcloud, SoundcloudClient) + assert orchestrator.client('soundcloud') is not None + assert isinstance(orchestrator.client('soundcloud'), SoundcloudClient) def test_client_lookup_resolves_soundcloud(orchestrator: DownloadOrchestrator) -> None: - """Verify the dict-based name → client lookup includes SoundCloud.""" - assert orchestrator._client('soundcloud') is orchestrator.soundcloud + """Verify the registry-backed name → client lookup includes SoundCloud.""" + assert orchestrator.client('soundcloud') is orchestrator.registry.get('soundcloud') def test_client_lookup_returns_none_for_unknown(orchestrator: DownloadOrchestrator) -> None: """Sanity: unknown sources don't somehow resolve to SoundCloud.""" - assert orchestrator._client('made_up') is None + assert orchestrator.client('made_up') is None # --------------------------------------------------------------------------- @@ -80,7 +80,7 @@ def test_download_routes_soundcloud_username_to_client(orchestrator: DownloadOrc async def _fake_download(username, filename, file_size=0): return sentinel - with patch.object(orchestrator.soundcloud, 'download', side_effect=_fake_download) as mock_dl: + with patch.object(orchestrator.client('soundcloud'), 'download', side_effect=_fake_download) as mock_dl: result = _run(orchestrator.download( 'soundcloud', '999||https://soundcloud.com/x/y||Display', @@ -93,13 +93,13 @@ def test_download_routes_soundcloud_username_to_client(orchestrator: DownloadOrc def test_download_unknown_username_still_falls_to_soulseek(orchestrator: DownloadOrchestrator) -> None: """Adding SoundCloud must not change the legacy Soulseek-fallback behavior for unrecognized usernames.""" - if orchestrator.soulseek is None: + if orchestrator.client('soulseek') is None: pytest.skip("Soulseek client unavailable in this environment") async def _fake_soulseek_download(username, filename, file_size=0): return 'soulseek-id' - with patch.object(orchestrator.soulseek, 'download', side_effect=_fake_soulseek_download) as mock_dl: + with patch.object(orchestrator.client('soulseek'), 'download', side_effect=_fake_soulseek_download) as mock_dl: result = _run(orchestrator.download('some_random_user', 'file.mp3', 0)) assert result == 'soulseek-id' mock_dl.assert_called_once() @@ -122,8 +122,8 @@ def test_hybrid_search_iterates_soundcloud_when_in_order(orchestrator: DownloadO async def _fake_search(query, timeout=None, progress_callback=None): return ([fake_track], []) - with patch.object(orchestrator.soundcloud, 'search', side_effect=_fake_search), \ - patch.object(orchestrator.soundcloud, 'is_configured', return_value=True): + with patch.object(orchestrator.client('soundcloud'), 'search', side_effect=_fake_search), \ + patch.object(orchestrator.client('soundcloud'), 'is_configured', return_value=True): tracks, albums = _run(orchestrator.search("any query")) assert tracks == [fake_track] @@ -136,7 +136,7 @@ def test_hybrid_search_skips_unconfigured_soundcloud(orchestrator: DownloadOrche orchestrator.mode = 'hybrid' orchestrator.hybrid_order = ['soundcloud', 'soulseek'] - if orchestrator.soulseek is None: + if orchestrator.client('soulseek') is None: pytest.skip("Soulseek client unavailable in this environment") soulseek_track = MagicMock() @@ -145,9 +145,9 @@ def test_hybrid_search_skips_unconfigured_soundcloud(orchestrator: DownloadOrche async def _fake_soulseek_search(query, timeout=None, progress_callback=None): return ([soulseek_track], []) - with patch.object(orchestrator.soundcloud, 'is_configured', return_value=False), \ - patch.object(orchestrator.soulseek, 'is_configured', return_value=True), \ - patch.object(orchestrator.soulseek, 'search', side_effect=_fake_soulseek_search): + with patch.object(orchestrator.client('soundcloud'), 'is_configured', return_value=False), \ + patch.object(orchestrator.client('soulseek'), 'is_configured', return_value=True), \ + patch.object(orchestrator.client('soulseek'), 'search', side_effect=_fake_soulseek_search): tracks, _ = _run(orchestrator.search("any")) assert tracks == [soulseek_track] @@ -166,7 +166,7 @@ def test_get_all_downloads_walks_soundcloud(orchestrator: DownloadOrchestrator) async def _fake_get_all(): return [fake_status] - with patch.object(orchestrator.soundcloud, 'get_all_downloads', side_effect=_fake_get_all): + with patch.object(orchestrator.client('soundcloud'), 'get_all_downloads', side_effect=_fake_get_all): all_dl = _run(orchestrator.get_all_downloads()) assert any(d is fake_status for d in all_dl) @@ -180,7 +180,7 @@ def test_get_download_status_finds_soundcloud_id(orchestrator: DownloadOrchestra async def _fake_get_status(download_id): return fake_status if download_id == 'sc-2' else None - with patch.object(orchestrator.soundcloud, 'get_download_status', side_effect=_fake_get_status): + with patch.object(orchestrator.client('soundcloud'), 'get_download_status', side_effect=_fake_get_status): result = _run(orchestrator.get_download_status('sc-2')) assert result is fake_status @@ -192,7 +192,7 @@ def test_cancel_routes_soundcloud_username(orchestrator: DownloadOrchestrator) - async def _fake_cancel(download_id, username=None, remove=False): return True - with patch.object(orchestrator.soundcloud, 'cancel_download', side_effect=_fake_cancel) as mock_cancel: + with patch.object(orchestrator.client('soundcloud'), 'cancel_download', side_effect=_fake_cancel) as mock_cancel: ok = _run(orchestrator.cancel_download('sc-3', username='soundcloud')) assert ok is True mock_cancel.assert_called_once() @@ -210,7 +210,7 @@ def test_clear_completed_walks_soundcloud(orchestrator: DownloadOrchestrator) -> async def _fake_clear(): return True - with patch.object(orchestrator.soundcloud, 'clear_all_completed_downloads', side_effect=_fake_clear) as mock_clear: + with patch.object(orchestrator.client('soundcloud'), 'clear_all_completed_downloads', side_effect=_fake_clear) as mock_clear: _run(orchestrator.clear_all_completed_downloads()) mock_clear.assert_called_once() @@ -228,8 +228,8 @@ def test_soundcloud_only_mode_uses_soundcloud(orchestrator: DownloadOrchestrator async def _fake_search(query, timeout=None, progress_callback=None): return ([MagicMock(username='soundcloud')], []) - with patch.object(orchestrator.soundcloud, 'search', side_effect=_fake_search) as mock_sc, \ - patch.object(orchestrator.soulseek, 'search', side_effect=AssertionError("soulseek must not be searched")): + with patch.object(orchestrator.client('soundcloud'), 'search', side_effect=_fake_search) as mock_sc, \ + patch.object(orchestrator.client('soulseek'), 'search', side_effect=AssertionError("soulseek must not be searched")): tracks, _ = _run(orchestrator.search("any")) assert len(tracks) == 1 diff --git a/tests/test_download_plugin_conformance.py b/tests/test_download_plugin_conformance.py index 2cb341bb..2b0f4e9d 100644 --- a/tests/test_download_plugin_conformance.py +++ b/tests/test_download_plugin_conformance.py @@ -147,17 +147,17 @@ def test_plugin_class_async_methods_are_coroutines(plugin_name): def test_orchestrator_uses_registry_for_dispatch(): - """The orchestrator must hold a registry reference and the - backward-compat ``self.`` attributes must point at the - SAME instances the registry returned. Anything that reaches in - for ``orchestrator.soulseek`` and any future code that uses - ``orchestrator.registry.get('soulseek')`` should be looking at - the same object.""" + """The orchestrator must hold a registry reference and the generic + ``client(name)`` accessor must return the same instances the + registry holds. Per-source attribute aliases (``orchestrator.soulseek`` + etc.) were removed in favor of ``orchestrator.client('soulseek')``; + the legacy alias name (``deezer_dl``) still resolves to the canonical + deezer plugin via the registry's alias map.""" from core.download_orchestrator import DownloadOrchestrator orchestrator = DownloadOrchestrator() assert hasattr(orchestrator, 'registry') - assert orchestrator.soulseek is orchestrator.registry.get('soulseek') - assert orchestrator.youtube is orchestrator.registry.get('youtube') - assert orchestrator.deezer_dl is orchestrator.registry.get('deezer') - assert orchestrator.lidarr is orchestrator.registry.get('lidarr') + assert orchestrator.client('soulseek') is orchestrator.registry.get('soulseek') + assert orchestrator.client('youtube') is orchestrator.registry.get('youtube') + assert orchestrator.client('deezer_dl') is orchestrator.registry.get('deezer') + assert orchestrator.client('lidarr') is orchestrator.registry.get('lidarr') diff --git a/tests/test_qobuz_credential_sync.py b/tests/test_qobuz_credential_sync.py index e1089062..156db419 100644 --- a/tests/test_qobuz_credential_sync.py +++ b/tests/test_qobuz_credential_sync.py @@ -6,7 +6,7 @@ Settings showed "Connected: (Active)" but underneath an error yellow even after a successful login. Root cause: SoulSync runs two QobuzClient instances side by side — one -through ``soulseek_client.qobuz`` for the auth-flow endpoints, and a +through ``soulseek_client.client('qobuz')`` for the auth-flow endpoints, and a second owned by the enrichment worker thread for thread safety. Login only updated the first instance's in-memory state. The dashboard's "configured" check (and the connection-test step) read the worker diff --git a/web_server.py b/web_server.py index b341a550..47c7c759 100644 --- a/web_server.py +++ b/web_server.py @@ -20054,9 +20054,10 @@ def _get_tidal_download_client(): """Get Tidal download client from the orchestrator, with helpful error if unavailable.""" if not soulseek_client: raise RuntimeError("Download orchestrator not initialized — check startup logs for errors") - if not hasattr(soulseek_client, 'tidal') or not soulseek_client.tidal: + tidal = soulseek_client.client("tidal") if hasattr(soulseek_client, 'client') else None + if not tidal: raise RuntimeError("Tidal download client not available — ensure tidalapi is installed") - return soulseek_client.client("tidal") + return tidal @app.route('/api/tidal/download/auth/start', methods=['POST']) def tidal_download_auth_start(): @@ -21124,9 +21125,7 @@ def _get_metadata_fallback_client(): def get_deezer_arl_status(): """Check if Deezer ARL is configured and authenticated.""" try: - deezer_dl = None - if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl: - deezer_dl = soulseek_client.client("deezer_dl") + deezer_dl = soulseek_client.client("deezer_dl") if soulseek_client and hasattr(soulseek_client, 'client') else None if deezer_dl and deezer_dl.is_authenticated(): user_data = deezer_dl._user_data or {} return jsonify({ @@ -21143,9 +21142,7 @@ def get_deezer_arl_status(): def get_deezer_arl_playlists(): """Fetch user playlists via Deezer ARL authentication (like /api/spotify/playlists).""" try: - deezer_dl = None - if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl: - deezer_dl = soulseek_client.client("deezer_dl") + deezer_dl = soulseek_client.client("deezer_dl") if soulseek_client and hasattr(soulseek_client, 'client') else None if not deezer_dl or not deezer_dl.is_authenticated(): return jsonify({'error': 'Deezer ARL not authenticated. Configure your ARL token in Settings > Downloads.'}), 401 @@ -21173,9 +21170,7 @@ def get_deezer_arl_playlists(): def get_deezer_arl_playlist_tracks(playlist_id): """Fetch full playlist with tracks via ARL (like /api/spotify/playlist/).""" try: - deezer_dl = None - if soulseek_client and hasattr(soulseek_client, 'deezer_dl') and soulseek_client.deezer_dl: - deezer_dl = soulseek_client.client("deezer_dl") + deezer_dl = soulseek_client.client("deezer_dl") if soulseek_client and hasattr(soulseek_client, 'client') else None if not deezer_dl or not deezer_dl.is_authenticated(): return jsonify({'error': 'Deezer ARL not authenticated.'}), 401 diff --git a/webui/static/helper.js b/webui/static/helper.js index 063c95bb..9e0f0beb 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3432,6 +3432,7 @@ const WHATS_NEW = { '2.4.2': [ // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- { date: 'Unreleased — 2.4.2 dev cycle' }, + { title: 'Internal: Drop Backward-Compat Per-Source Attrs', desc: 'internal — followup to cin\'s download engine review. removed the `orchestrator.soulseek` / `.youtube` / `.tidal` / `.qobuz` / `.hifi` / `.deezer_dl` / `.lidarr` / `.soundcloud` attribute aliases that were preserved for backward compat. external callers (core/downloads/, core/search/, web_server.py) all migrated to the generic `orchestrator.client(\'\')` accessor — alias-aware (legacy `deezer_dl` resolves to canonical `deezer`), single source of truth via the registry. the orchestrator\'s own internal `self.soulseek` / `self.deezer_dl` reaches also routed through `client()` so the only place that knows about per-source identity is the registry. test fakes updated to expose `client(name)` instead of stuffing attributes; conformance test pinned to the new accessor contract. zero behavior change — just cleaner shape.' }, { title: 'Internal: Download Engine Review Followup', desc: 'internal — three correctness fixes on top of the download engine refactor, all flagged in cin\'s pr review. (1) `engine.cancel_download(source_hint=\'deezer_dl\')` was silently routing deezer cancels to soulseek because the legacy alias never made it to the engine\'s plugin map — only the registry knew about it. fix: aliases now flow through `register_plugin` and `get_plugin` / `cancel_download` resolve them to the canonical name. (2) `_resolve_source_chain` filtered hybrid_order against canonical registry names only, so any user with `deezer_dl` in their config quietly dropped deezer from hybrid mode. fix: orchestrator normalizes through `registry.get_spec()` first. (3) the worker\'s terminal write was a read-then-write split — a cancel landing between the snapshot and the update could be overwritten back to errored / completed. fix: new atomic `update_record_unless_state` on the engine holds `state_lock` across the check + write; both `_mark_terminal` AND the success path use it now. also added generic `client(name)` / `configured_clients()` / `reload_instances(name?)` accessors on the orchestrator + a `get/set_download_orchestrator()` singleton matching cin\'s `get_metadata_engine()` shape, and migrated 30 external `soulseek_client.` reaches in web_server.py to `client("")`. 18 new tests pin every fix.' }, { title: 'Internal: Typed Metadata Foundation', desc: 'internal — first step of a multi-pr migration to give the metadata pipeline a real contract. the codebase historically grew duck-typed extractors (`_extract_lookup_value(album_data, "id", "album_id", "collectionId", "release_id", default=...)`) at every consumer site because each provider returns its own response shape. ~150 of those across the codebase. new `core/metadata/types.py` defines canonical typed `Album` / `Track` / `Artist` dataclasses with strict required fields. per-source classmethod converters (from_spotify_dict, from_itunes_dict, from_deezer_dict, from_discogs_dict, from_musicbrainz_dict, from_hydrabase_dict) are the SINGLE place that knows each provider\'s wire shape. zero behavior changes in this pr — pure additive foundation. follow-up prs migrate consumers one at a time. full migration plan documented at docs/metadata-types-migration.md.', page: 'library' }, { title: 'Internal: Migrate Album-Info Builders to Typed Path', desc: 'internal — steps 2+3 of the typed metadata migration in one pr. two album-info builders now route through `Album.from__dict()` when the caller passes a known source: `_build_album_info` (used by every album-tracks lookup) and the embedded album section of `_build_single_import_context_payload` (used by single-track import context resolution). legacy duck-typed extraction stays as the fallback when source is empty/unknown, raw input isn\'t a dict, or the typed converter raises — so a converter bug can\'t break album resolution or import context. caller-provided album_id / album_name / artist_name fallbacks apply on the typed path the same way they did on legacy. zero behavior change for existing callers since they don\'t pass a source yet — opt-in only. 22 new tests pin the typed path, the legacy fallback, and parametrized coverage across registered providers.' },