From ea654f664ee04a306c7c2ef34a6a2336e4c96773 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 4 May 2026 22:19:52 -0700 Subject: [PATCH] Cin-1: Make DownloadSourcePlugin inheritance explicit on every client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cin's review feedback: the plugin contract was discoverable only from the registry, not from the client files themselves. Reading `youtube_client.py` cold gave no signal that the class participates in the DownloadSourcePlugin contract. Every download client class now inherits DownloadSourcePlugin explicitly: - SoulseekClient(DownloadSourcePlugin) - YouTubeClient(DownloadSourcePlugin) - TidalDownloadClient(DownloadSourcePlugin) - QobuzClient(DownloadSourcePlugin) - HiFiClient(DownloadSourcePlugin) - DeezerDownloadClient(DownloadSourcePlugin) - SoundcloudClient(DownloadSourcePlugin) - LidarrDownloadClient(DownloadSourcePlugin) Adjustments: - core/download_plugins/base.py — moved TrackResult/AlbumResult/ DownloadStatus imports under TYPE_CHECKING since they're only used in type annotations. Without this, clients inheriting the contract create a circular import. - core/download_plugins/__init__.py — drops DownloadPluginRegistry re-export. Importing the package no longer triggers the registry's eager client imports (which would also be circular for clients that import from the package). Callers that need the registry import it directly: `from core.download_plugins.registry import DownloadPluginRegistry`. Suite still green (335 download tests). --- core/deezer_download_client.py | 5 ++++- core/download_plugins/__init__.py | 13 +++++++++++-- core/download_plugins/base.py | 15 ++++++++++----- core/hifi_client.py | 5 ++++- core/lidarr_download_client.py | 5 ++++- core/qobuz_client.py | 5 ++++- core/soulseek_client.py | 5 ++++- core/soundcloud_client.py | 5 ++++- core/tidal_download_client.py | 5 ++++- core/youtube_client.py | 5 ++++- 10 files changed, 53 insertions(+), 15 deletions(-) diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index f08f1175..81b0840b 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -79,7 +79,10 @@ def _decrypt_chunk(chunk: bytes, key: bytes) -> bytes: ) from exc -class DeezerDownloadClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class DeezerDownloadClient(DownloadSourcePlugin): """Deezer download client using ARL token authentication.""" def __init__(self, download_path: str = None): diff --git a/core/download_plugins/__init__.py b/core/download_plugins/__init__.py index bbe3b8df..ed6a79b6 100644 --- a/core/download_plugins/__init__.py +++ b/core/download_plugins/__init__.py @@ -17,9 +17,18 @@ See `core/download_plugins/base.py` for the protocol contract and """ from core.download_plugins.base import DownloadSourcePlugin -from core.download_plugins.registry import DownloadPluginRegistry + +# NOTE: DownloadPluginRegistry is intentionally NOT re-exported here. +# Importing the registry triggers eager imports of every client class +# (see registry.py for why eager — test fixtures inject mock modules +# at collection time and we need real bindings before that). Clients +# inherit from DownloadSourcePlugin (Cin's review feedback — visible +# contract conformance), so importing the package via ``from +# core.download_plugins import DownloadSourcePlugin`` from a client +# file would create a circular import if registry came along for the +# ride. Callers that need the registry import it directly: +# from core.download_plugins.registry import DownloadPluginRegistry __all__ = [ "DownloadSourcePlugin", - "DownloadPluginRegistry", ] diff --git a/core/download_plugins/base.py b/core/download_plugins/base.py index f1c7fc0c..5523beeb 100644 --- a/core/download_plugins/base.py +++ b/core/download_plugins/base.py @@ -27,12 +27,17 @@ makes the implicit contract explicit so: from __future__ import annotations -from typing import List, Optional, Protocol, Tuple, runtime_checkable +from typing import TYPE_CHECKING, List, Optional, Protocol, Tuple, runtime_checkable -# Soulseek client owns the canonical TrackResult / AlbumResult / DownloadStatus -# dataclasses — every other source already imports from there. Keep it that way -# until a future PR lifts those into a neutral location. -from core.soulseek_client import AlbumResult, DownloadStatus, TrackResult +# Soulseek client owns the canonical TrackResult / AlbumResult / +# DownloadStatus dataclasses — every other source already imports +# from there. We only need them for type annotations on this +# protocol; using TYPE_CHECKING avoids a circular import once the +# clients themselves inherit from DownloadSourcePlugin (Cin's +# review feedback — clients explicitly declare conformance instead +# of relying on structural typing). +if TYPE_CHECKING: + from core.soulseek_client import AlbumResult, DownloadStatus, TrackResult @runtime_checkable diff --git a/core/hifi_client.py b/core/hifi_client.py index ea9876de..9a7dfe4c 100644 --- a/core/hifi_client.py +++ b/core/hifi_client.py @@ -86,7 +86,10 @@ DEFAULT_INSTANCES = [ ] -class HiFiClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class HiFiClient(DownloadSourcePlugin): """ HiFi API client for searching and downloading lossless music. Uses public hifi-api instances (Tidal backend) — no auth required. diff --git a/core/lidarr_download_client.py b/core/lidarr_download_client.py index 8a71b179..0d92cdd9 100644 --- a/core/lidarr_download_client.py +++ b/core/lidarr_download_client.py @@ -33,7 +33,10 @@ from core.soulseek_client import TrackResult, AlbumResult, DownloadStatus logger = get_logger("lidarr_client") -class LidarrDownloadClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class LidarrDownloadClient(DownloadSourcePlugin): """Lidarr download client — uses Lidarr as a download source for Usenet/torrent content. Implements the same interface as SoulseekClient, QobuzClient, TidalDownloadClient diff --git a/core/qobuz_client.py b/core/qobuz_client.py index a06a9031..983dce69 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -104,7 +104,10 @@ QOBUZ_QUALITY_MAP = { } -class QobuzClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class QobuzClient(DownloadSourcePlugin): """ Qobuz download client using Qobuz REST API. Provides search, matching, and download capabilities as a drop-in alternative to Soulseek/YouTube/Tidal. diff --git a/core/soulseek_client.py b/core/soulseek_client.py index e69c9439..bd76b4f9 100644 --- a/core/soulseek_client.py +++ b/core/soulseek_client.py @@ -188,7 +188,10 @@ class DownloadStatus: time_remaining: Optional[int] = None file_path: Optional[str] = None -class SoulseekClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class SoulseekClient(DownloadSourcePlugin): def __init__(self): self.base_url: Optional[str] = None self.api_key: Optional[str] = None diff --git a/core/soundcloud_client.py b/core/soundcloud_client.py index c0362859..0d640985 100644 --- a/core/soundcloud_client.py +++ b/core/soundcloud_client.py @@ -82,7 +82,10 @@ def _sanitize_filename(name: str) -> str: return cleaned or 'soundcloud_track' -class SoundcloudClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class SoundcloudClient(DownloadSourcePlugin): """SoundCloud download client built on yt-dlp's SoundCloud extractor. Mirrors the public surface of TidalDownloadClient / QobuzClient so the diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 725a6bee..c2ac6760 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -92,7 +92,10 @@ HLS_QUALITY_MAP = { HLS_MAP_TAG_RE = re.compile(r'#EXT-X-MAP:.*URI="([^"]+)"') -class TidalDownloadClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class TidalDownloadClient(DownloadSourcePlugin): """ Tidal download client using tidalapi. Provides search, matching, and download capabilities as a drop-in alternative to YouTube/Soulseek. diff --git a/core/youtube_client.py b/core/youtube_client.py index 3bf0bbe7..b3cd7270 100644 --- a/core/youtube_client.py +++ b/core/youtube_client.py @@ -91,7 +91,10 @@ class YouTubeSearchResult: self.parsed_artist = self.channel -class YouTubeClient: +from core.download_plugins.base import DownloadSourcePlugin + + +class YouTubeClient(DownloadSourcePlugin): """ YouTube download client using yt-dlp. Provides search, matching, and download capabilities with full Spotify metadata integration.