From 3634dca83f4f2878c7278c27ff16e1d69ac6c4da Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 4 May 2026 12:59:04 -0700 Subject: [PATCH] B3: Orchestrator delegates query methods to engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_all_downloads`, `get_download_status`, `cancel_download`, and `clear_all_completed_downloads` on the orchestrator are now thin pass-throughs to the engine. The plugin-iteration logic lives in one place (the engine) instead of duplicated across orchestrator methods. Source-hint routing semantics preserved verbatim — engine.cancel treats streaming-source names as direct routes and unknown names as Soulseek peer usernames, exactly like the legacy orchestrator did. Per-plugin exceptions still get swallowed defensively. Test fixture `_build_orchestrator` now constructs an engine and registers every mock plugin so the helper-built orchestrators have the same wiring as production. Suite still green (2012 passed). Zero behavior change for users. --- core/download_orchestrator.py | 93 +++---------------- tests/downloads/test_download_orchestrator.py | 7 ++ 2 files changed, 22 insertions(+), 78 deletions(-) diff --git a/core/download_orchestrator.py b/core/download_orchestrator.py index 41f34eb3..5f776fab 100644 --- a/core/download_orchestrator.py +++ b/core/download_orchestrator.py @@ -389,70 +389,21 @@ class DownloadOrchestrator: return await soulseek.download(username, filename, file_size) async def get_all_downloads(self) -> List[DownloadStatus]: - """ - Get all active downloads from all sources. - - Returns: - List of DownloadStatus objects - """ - all_downloads = [] - for _, client in self.registry.all_plugins(): - try: - all_downloads.extend(await client.get_all_downloads()) - except Exception: - pass - return all_downloads + """Aggregated view across every source. Delegates to the + engine, which iterates registered plugins.""" + return await self.engine.get_all_downloads() async def get_download_status(self, download_id: str) -> Optional[DownloadStatus]: - """ - Get status of a specific download. - - Args: - download_id: Download ID to query - - Returns: - DownloadStatus object or None if not found - """ - for _, client in self.registry.all_plugins(): - try: - status = await client.get_download_status(download_id) - if status: - return status - except Exception: - pass - - return None + """Find a download by id across every source. Delegates to + the engine.""" + return await self.engine.get_download_status(download_id) async def cancel_download(self, download_id: str, username: str = None, remove: bool = False) -> bool: - """ - Cancel an active download. - - Args: - download_id: Download ID to cancel - username: Username hint (optional). Streaming source name - (e.g. ``'youtube'``) routes directly to that source; - anything else routes to Soulseek as a peer username. - remove: Whether to remove from active downloads - - Returns: - True if cancelled successfully - """ - spec = self.registry.get_spec(username) if username else None - if spec is not None and spec.name != 'soulseek': - client = self.registry.get(spec.name) - return await client.cancel_download(download_id, username, remove) if client else False - elif username: - soulseek = self.registry.get('soulseek') - return await soulseek.cancel_download(download_id, username, remove) if soulseek else False - - # No hint — try every source - for _, client in self.registry.all_plugins(): - try: - if await client.cancel_download(download_id, username, remove): - return True - except Exception: - pass - return False + """Cancel an active download. Delegates to the engine, which + handles source-hint routing (streaming source name → direct + plugin, unknown name → Soulseek as peer username, no hint → + try every plugin).""" + return await self.engine.cancel_download(download_id, username, remove) async def signal_download_completion(self, download_id: str, username: str, remove: bool = True) -> bool: """ @@ -472,24 +423,10 @@ class DownloadOrchestrator: return await self.soulseek.signal_download_completion(download_id, username, remove) async def clear_all_completed_downloads(self) -> bool: - """ - Clear all completed downloads from every source. - - Returns: - True if successful - """ - results = [] - for name, client in self.registry.all_plugins(): - if hasattr(client, "is_configured") and not client.is_configured(): - logger.debug("Skipping %s clear_all_completed_downloads (not configured)", name) - continue - try: - results.append(await client.clear_all_completed_downloads()) - except Exception as exc: - logger.warning("%s clear_all_completed_downloads failed: %s", name, exc) - results.append(False) - - return all(results) if results else True + """Clear completed downloads from every source. Delegates + to the engine, which skips unconfigured plugins and treats + per-plugin failures as False (not an exception).""" + return await self.engine.clear_all_completed_downloads() # ===== Soulseek-specific methods (for backwards compatibility) ===== # These are internal methods that some parts of the codebase use directly diff --git a/tests/downloads/test_download_orchestrator.py b/tests/downloads/test_download_orchestrator.py index 1ceb0283..f0467c06 100644 --- a/tests/downloads/test_download_orchestrator.py +++ b/tests/downloads/test_download_orchestrator.py @@ -1,3 +1,4 @@ +from core.download_engine import DownloadEngine from core.download_orchestrator import DownloadOrchestrator from core.download_plugins.registry import DownloadPluginRegistry, PluginSpec @@ -58,6 +59,12 @@ def _build_orchestrator(**clients): 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. + orch.engine = DownloadEngine() + for source_name, plugin in registry.all_plugins(): + orch.engine.register_plugin(source_name, plugin) return orch