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.
This commit is contained in:
Broque Thomas
parent
d0eac87601
commit
7519c3d50c
15 changed files with 100 additions and 117 deletions
|
|
@ -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.<source>`` 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('<name>')`` 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.
|
||||
|
|
|
|||
|
|
@ -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}"]
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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]:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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, ([], []))
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.<source>`` 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')
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ Settings showed "Connected: <username> (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
|
||||
|
|
|
|||
|
|
@ -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/<id>)."""
|
||||
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
|
||||
|
||||
|
|
|
|||
|
|
@ -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(\'<source>\')` 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.<source>` reaches in web_server.py to `client("<source>")`. 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_<source>_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.' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue