From 6489244bcc6898db8b6e54082b7c866aa3f6ccb4 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Tue, 5 May 2026 22:36:05 -0700 Subject: [PATCH] =?UTF-8?q?MS=20Cin/JohnBaumb=20honesty=20pass=20=E2=80=94?= =?UTF-8?q?=20drop=20dead=20wrappers,=20sync=20contract=20to=20reality?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-review audit found premature abstraction + lying docstrings. Cut what isn't used, made the rest match what's actually shipped. (1) Engine: dropped 7 cross-server dispatch wrappers that had ZERO production callers (ensure_connection / get_all_artists / get_all_album_ids / search_tracks / trigger_library_scan / is_library_scanning / get_library_stats / get_recently_added_albums). Every consumer reaches the active client directly via sync_service._get_active_media_client() or engine.client(name). Engine surface shrinks to client(name) / active_client() / active_server / is_connected() (the one wrapper that has callers — 4 dashboard status sites) / configured_clients() / reload_config(). ~150 lines deleted, 5 dead-method tests removed. (2) Contract Protocol body trimmed to match REQUIRED_METHODS exactly (is_connected, ensure_connection, get_all_artists, get_all_album_ids). The other 5 methods that were declared in the Protocol "required" section weren't actually required — Plex doesn't implement get_recently_added_albums, Jellyfin doesn't implement search_tracks, SoulSync doesn't implement most of them. Static contract now matches runtime conformance test. Optional methods moved to a KNOWN_PER_SERVER_METHODS data-only listing with audited per-server coverage notes — discoverability without false promises. (3) Engine module docstring + __init__.py docstring no longer overclaim "33+ chains collapsed" — only 4 uniform-shape chains were collapsed; ~18 server-specific chains stay explicit per the "lift what's truly shared" standard. Phrasing now matches reality. (4) types.py docstring claimed TrackInfo.from_jellyfin_dict and TrackInfo.from_navidrome_dict exist as classmethods. They don't — only from_plex_track / from_plex_playlist do. Jellyfin and Navidrome construct TrackInfo inline at their call sites today. Docstring now honest about that + flags the lift as a clean followup. (5) Engine line 95 comment "backward-compat for source-specific reaches" was misleading — there is no legacy alternative being preserved; engine.client(name) IS the canonical access pattern. Section header rewritten. Tests: 2121 pass (was 2126; -5 dead-method pin tests). --- core/media_server/__init__.py | 28 +++-- core/media_server/contract.py | 110 ++++++------------ core/media_server/engine.py | 181 ++++++------------------------ core/media_server/types.py | 28 ++--- tests/media_server/test_engine.py | 81 ++----------- 5 files changed, 114 insertions(+), 314 deletions(-) diff --git a/core/media_server/__init__.py b/core/media_server/__init__.py index 3c125ede..0dc6269c 100644 --- a/core/media_server/__init__.py +++ b/core/media_server/__init__.py @@ -1,17 +1,23 @@ -"""Media server engine — central dispatch for the per-server clients -(Plex, Jellyfin, Navidrome, SoulSync standalone). +"""Media server engine — central registry-backed access to the +per-server clients (Plex, Jellyfin, Navidrome, SoulSync standalone). Companion to the download engine refactor — same architectural -shape applied to the read-side of the library. The orchestrator -historically had 33+ ``if active_server == 'plex' / 'jellyfin' / -...`` dispatch sites in web_server.py. This package replaces those -with a single ``MediaServerEngine.method()`` call per operation. +shape applied to the read-side of the library. Pre-refactor +web_server.py held four separate per-server globals +(``plex_client`` / ``jellyfin_client`` / ``navidrome_client`` / +``soulsync_library_client``) that every dispatch site reached +individually. This package replaces those globals with a single +engine that owns the client instances + a generic +``engine.client(name)`` accessor. -Per-server clients keep their protocol-specific work (Plex's -PlexAPI SDK, Jellyfin's REST endpoints, Navidrome's OpenSubsonic -API, SoulSync's filesystem walk). The engine just routes by -``active_server`` config + provides a uniform shape for the calls -``web_server.py`` makes generically. +The 18-or-so ``if active_server == 'plex' / 'jellyfin' / ...`` +chains in web_server.py that do server-specific work (Plex raw +playlist API vs Jellyfin / Navidrome client methods returning +different shapes) stay explicit at the call site per the "lift +what's truly shared" standard — but they reach the per-server +client through ``engine.client(name)`` rather than the legacy +globals. The four uniform-shape ``is_connected`` chains were the +only ones genuinely shared and are now ``engine.is_connected()``. See ``docs/media-server-engine-refactor-plan.md`` for the full phased plan. diff --git a/core/media_server/contract.py b/core/media_server/contract.py index 986b4108..d051482a 100644 --- a/core/media_server/contract.py +++ b/core/media_server/contract.py @@ -1,29 +1,25 @@ """Canonical contract for media server clients. -Narrow on purpose. Only declares the methods the orchestrator -dispatches generically across all servers. Server-specific extras -(Plex's `set_music_library_by_name`, Jellyfin's user picker, -Navidrome's music folder filter, SoulSync's filesystem rescan) -stay on the underlying client and are accessed through the -registry's typed accessor — same pattern as the download -plugin contract. +Narrow on purpose. Protocol body declares ONLY the methods every +registered client actually implements today — keeps the static +contract honest. Server-specific extras (Plex's +``set_music_library_by_name``, Jellyfin's user picker, Navidrome's +music folder filter, SoulSync's filesystem rescan) and methods that +most-but-not-all servers implement (``search_tracks`` on Plex / +Navidrome but not Jellyfin; ``get_recently_added_albums`` on +Jellyfin / Navidrome / SoulSync but not Plex) stay off the Protocol +and are reached through ``engine.client(name)`` directly. -Every required method must be implemented by every registered -client. Optional methods have default no-op implementations so -servers without that capability (e.g. Navidrome's metadata -writeback stubs, SoulSync's playlist sync N/A) don't have to -declare a no-op explicitly. - -The contract is a Protocol (structural typing) rather than an -ABC — existing PlexClient / JellyfinClient / NavidromeClient / +The contract is a Protocol (structural typing) rather than an ABC — +existing PlexClient / JellyfinClient / NavidromeClient / SoulSyncClient grew the same shape independently because every -caller needed the same calls. This file just makes the implicit -contract explicit. +caller needed the same four calls. This file just makes that +implicit contract explicit + the conformance test pins it. """ from __future__ import annotations -from typing import Any, Dict, List, Optional, Protocol, runtime_checkable +from typing import Any, List, Protocol, runtime_checkable @runtime_checkable @@ -31,14 +27,13 @@ class MediaServerClient(Protocol): """Structural contract every media server client must satisfy. ``runtime_checkable`` lets ``isinstance(client, MediaServerClient)`` - work for the conformance test, but it ONLY checks method names — - not signatures. The conformance test in - ``tests/media_server/test_conformance.py`` does the deeper - signature check. + work, but it ONLY checks method names — not signatures. The + conformance test in ``tests/media_server/test_conformance.py`` + does the deeper class-level check via REQUIRED_METHODS. """ # ------------------------------------------------------------------ - # Connection / lifecycle + # Connection / lifecycle — required, every server implements # ------------------------------------------------------------------ def is_connected(self) -> bool: @@ -53,7 +48,7 @@ class MediaServerClient(Protocol): ... # ------------------------------------------------------------------ - # Library reads (required — every server must support these) + # Library reads — required, every server implements # ------------------------------------------------------------------ def get_all_artists(self) -> List[Any]: @@ -68,51 +63,12 @@ class MediaServerClient(Protocol): format is server-native — caller doesn't introspect.""" ... - def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[Any]: - """Search the server's library for tracks matching the title - + artist. Used by playlist sync, download matching, and the - general search UI. Each item is a server-specific TrackInfo - wrapper.""" - ... - - def get_recently_added_albums(self, max_results: int = 400) -> List[Any]: - """Recently-added view — used by the Discover page + - watchlist scanner. Sorted by add timestamp descending.""" - ... - - # ------------------------------------------------------------------ - # Library writes — scan triggers - # ------------------------------------------------------------------ - - def trigger_library_scan(self) -> bool: - """Ask the server to scan its music library. Some servers - (SoulSync standalone) walk the filesystem themselves; some - (Plex / Jellyfin / Navidrome) hit a server-side scan API.""" - ... - - def is_library_scanning(self) -> bool: - """True if a scan is currently running. Polled by the - scan-progress UI.""" - ... - - def get_library_stats(self) -> Dict[str, int]: - """Counts of artists / albums / tracks. Used by the - dashboard system-stats card.""" - ... - # --------------------------------------------------------------------------- -# Required + optional method names — used by the conformance tests to -# check structural conformance without the proxy weight of dataclasses. +# Required method set — pinned by the conformance test. Mirrors the +# Protocol body exactly so static + runtime contracts can't drift. # --------------------------------------------------------------------------- -# Conservative requirement set — only methods every one of the four -# servers actually implements today. Audited by the conformance test. -# Other methods (search_tracks, trigger_library_scan, etc.) exist on -# most servers but not all (e.g. SoulSync has no library scan API -# because it walks the filesystem directly). Phase B's engine -# adapters handle those with per-server fallback rather than forcing -# every client to declare a no-op stub. REQUIRED_METHODS = { 'is_connected', 'ensure_connection', @@ -120,29 +76,35 @@ REQUIRED_METHODS = { 'get_all_album_ids', } -# Methods declared on the protocol but NOT enforced — the -# conformance test does NOT fail if a client lacks one. Engine -# adapters route around the gaps. Listed here so future contributors -# know what the engine expects to find when present. -OPTIONAL_METHODS = { +# Methods that exist on SOME servers but NOT all, listed here for +# discoverability. The conformance test does NOT enforce these. Callers +# that need one reach the per-server client directly via +# ``engine.client(name).`` rather than going through the engine, +# since the engine has no uniform safe-default that fits every method. +# +# Coverage today (audited 2026-05): +# search_tracks: Plex ✓, Navidrome ✓, Jellyfin ✗, SoulSync ✗ +# get_recently_added_albums: Jellyfin ✓, Navidrome ✓, SoulSync ✓, Plex ✗ (uses recentlyAdded() on music library) +# trigger_library_scan / is_library_scanning: Plex ✓, Jellyfin ✓, Navidrome ✓, SoulSync ✗ (filesystem walks in-process) +# get_library_stats: Plex ✓, Jellyfin ✓, Navidrome ✓, SoulSync ✗ +# create_playlist / update_playlist / get_all_playlists / etc: Plex ✓, Jellyfin ✓, Navidrome ✓, SoulSync ✗ +# update_artist_*, update_album_poster, update_track_metadata: Plex ✓, Jellyfin partial, Navidrome stubs, SoulSync ✗ +KNOWN_PER_SERVER_METHODS = ( 'search_tracks', 'get_recently_added_albums', 'trigger_library_scan', 'is_library_scanning', 'get_library_stats', - # Playlist sync (Plex / Jellyfin / Navidrome implement; SoulSync no-op) 'create_playlist', 'update_playlist', 'copy_playlist', 'get_all_playlists', 'get_playlist_by_name', - # Analytics 'get_play_history', 'get_track_play_counts', - # Metadata writeback (Plex full support; Jellyfin partial; Navidrome stubs; SoulSync N/A) 'update_artist_genres', 'update_artist_poster', 'update_album_poster', 'update_artist_biography', 'update_track_metadata', -} +) diff --git a/core/media_server/engine.py b/core/media_server/engine.py index 52b96b2a..b8c4f3c3 100644 --- a/core/media_server/engine.py +++ b/core/media_server/engine.py @@ -1,33 +1,40 @@ -"""MediaServerEngine — central dispatch for media server operations. +"""MediaServerEngine — central registry-backed access to media server clients. -Replaces the *uniform-shape* dispatch chains in web_server.py -(``is_connected``, ``get_all_artists``, etc. — anything where every -server returns the same shape and the only branching was on -``active_server == X``). Each such operation is now one -``engine.method()`` call that: +Honest scope: the engine OWNS the per-server client instances and +exposes a small set of generic accessors so callers don't need +per-server attribute reaches. Most actual cross-server dispatch in +web_server.py (playlist add / remove / replace, per-server metadata +sync, deep scan with server-specific cache strategies) is genuinely +different per server and stays explicit in the call site — the +engine just provides the canonical client lookup so those sites +reach via ``engine.client(name)`` instead of separate globals. -1. Reads the ``server.active`` config to find the current target. -2. Looks up the registered client. -3. Calls the corresponding method (with safe per-server fallbacks - for methods that don't exist on every client — e.g. SoulSync - has no library-scan API). +Surface: +- ``client(name)`` / ``active_client()`` — name → client lookup +- ``active_server`` — config-driven active server name +- ``is_connected()`` — only cross-server dispatch with real callers + today (dashboard status indicators); kept as the canonical example +- ``configured_clients()`` — replaces the legacy per-server + ``if X and X.is_connected()`` chains in web_server.py +- ``reload_config(name=None)`` — generic dispatch instead of + per-client reload calls -Server-specific dispatch sites (Plex's raw playlist API, Jellyfin / -Navidrome client methods returning different shapes) stay explicit -in web_server.py per the "lift what's truly shared" standard. They -reach individual clients via ``engine.client(name)`` rather than -the per-server globals — same generic-accessor pattern as the -download orchestrator. +Per-method engine wrappers for ``get_all_artists`` / ``search_tracks`` +/ ``trigger_library_scan`` / etc. were on an earlier draft but had no +production callers — every consumer reaches the active client directly +through ``sync_service._get_active_media_client()`` or +``engine.client(name)`` and calls the per-server method itself. Cut +per the "no premature abstraction" standard. -Engine itself is constructed once during web_server.py init and -held as a process-wide singleton via -``set_media_server_engine`` / ``get_media_server_engine``, mirroring -the metadata + download engine factory shape. +Engine is constructed once during web_server.py init and held as a +process-wide singleton via ``set_media_server_engine`` / +``get_media_server_engine``, mirroring the metadata + download +engine factory shape. """ from __future__ import annotations -from typing import Any, Dict, List, Optional +from typing import Any, Dict, Optional from utils.logging_config import get_logger @@ -92,7 +99,8 @@ class MediaServerEngine: self._resolve_active = active_server_resolver # ------------------------------------------------------------------ - # Direct client access (backward-compat for source-specific reaches) + # Client lookup — generic accessors that replace per-server + # attribute reaches in callers. # ------------------------------------------------------------------ def client(self, name: str) -> Optional[MediaServerClient]: @@ -111,13 +119,12 @@ class MediaServerEngine: """The client for the currently-active server.""" return self.registry.get(self.active_server) - # ------------------------------------------------------------------ - # Cross-server dispatch — required methods (always present) - # ------------------------------------------------------------------ - def is_connected(self) -> bool: """Active server's connection state. False if no active - client (registered but failed to initialize).""" + client (registered but failed to initialize). The dashboard + status indicators + endpoint guards rely on this — the only + cross-server dispatch wrapper kept on the engine because it + actually has callers.""" client = self.active_client() if client is None: return False @@ -127,124 +134,6 @@ class MediaServerEngine: logger.warning("%s is_connected raised: %s", self.active_server, exc) return False - def ensure_connection(self) -> bool: - """Re-auth or reconnect the active server. Returns True if - usable after the call.""" - client = self.active_client() - if client is None: - return False - try: - return client.ensure_connection() - except Exception as exc: - logger.warning("%s ensure_connection raised: %s", self.active_server, exc) - return False - - def get_all_artists(self) -> List[Any]: - """Active server's full artist list. Empty list if not - connected or call fails.""" - client = self.active_client() - if client is None: - return [] - try: - return client.get_all_artists() - except Exception as exc: - logger.warning("%s get_all_artists raised: %s", self.active_server, exc) - return [] - - def get_all_album_ids(self) -> set: - """Active server's album-ID set. Empty set if not connected - or call fails.""" - client = self.active_client() - if client is None: - return set() - try: - return client.get_all_album_ids() - except Exception as exc: - logger.warning("%s get_all_album_ids raised: %s", self.active_server, exc) - return set() - - # ------------------------------------------------------------------ - # Optional methods — engine routes if the client implements them, - # returns a safe default otherwise (mirrors the legacy web_server.py - # branches that special-cased SoulSync / Navidrome). - # ------------------------------------------------------------------ - - def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[Any]: - """Search the active server's library. Returns empty list - for servers that don't implement search_tracks (SoulSync - standalone reads filesystem; no live search API).""" - client = self.active_client() - if client is None or not hasattr(client, 'search_tracks'): - return [] - try: - return client.search_tracks(title, artist, limit) - except Exception as exc: - logger.warning("%s search_tracks raised: %s", self.active_server, exc) - return [] - - def trigger_library_scan(self) -> bool: - """Trigger a server-side library scan. No-op (returns True) - for SoulSync standalone — filesystem walks happen in-process.""" - client = self.active_client() - if client is None: - return False - if not hasattr(client, 'trigger_library_scan'): - return True - try: - return client.trigger_library_scan() - except Exception as exc: - logger.warning("%s trigger_library_scan raised: %s", self.active_server, exc) - return False - - def is_library_scanning(self) -> bool: - """True if the active server is currently scanning. Always - False for SoulSync standalone.""" - client = self.active_client() - if client is None or not hasattr(client, 'is_library_scanning'): - return False - try: - return client.is_library_scanning() - except Exception as exc: - logger.warning("%s is_library_scanning raised: %s", self.active_server, exc) - return False - - def get_library_stats(self) -> Dict[str, int]: - """Counts of artists / albums / tracks. Default empty dict - if the server doesn't implement (SoulSync standalone).""" - client = self.active_client() - if client is None or not hasattr(client, 'get_library_stats'): - return {} - try: - return client.get_library_stats() - except Exception as exc: - logger.warning("%s get_library_stats raised: %s", self.active_server, exc) - return {} - - def get_recently_added_albums(self, max_results: int = 400) -> List[Any]: - """Recently-added albums view. Plex uses a different name; - engine routes to whichever method the active server has.""" - client = self.active_client() - if client is None: - return [] - # Plex uses recentlyAdded() on the music library object, not - # a top-level method. SoulSync, Jellyfin, Navidrome all - # expose get_recently_added_albums directly. - if hasattr(client, 'get_recently_added_albums'): - try: - return client.get_recently_added_albums(max_results) - except Exception as exc: - logger.warning( - "%s get_recently_added_albums raised: %s", - self.active_server, exc, - ) - return [] - return [] - - # ------------------------------------------------------------------ - # Generic accessors — replace per-server attribute reaches in - # callers (Cin's standard from the download refactor). - # ------------------------------------------------------------------ - def configured_clients(self) -> Dict[str, MediaServerClient]: """Return ``{name: client}`` for every server that's both registered AND reports ``is_connected() == True``. Replaces diff --git a/core/media_server/types.py b/core/media_server/types.py index 7d48e1c5..9467d8e1 100644 --- a/core/media_server/types.py +++ b/core/media_server/types.py @@ -1,19 +1,21 @@ """Shared dataclasses for the media-server contract. -Plex / Jellyfin / Navidrome / SoulSync clients all surfaced near- -identical ``XTrackInfo`` and ``XPlaylistInfo`` shapes (id, title, -artist, album, duration, track_number, year, optional rating) -because every consumer needed the same shape downstream. The -per-server class names were a copy-paste artifact, not a real -contract difference. +Plex / Jellyfin / Navidrome clients all surfaced near-identical +``XTrackInfo`` and ``XPlaylistInfo`` shapes (id, title, artist, +album, duration, track_number, year, optional rating) because every +consumer needed the same shape downstream. The per-server class +names were a copy-paste artifact, not a real contract difference. -This module owns the canonical types. Per-server constructors live -on the unified class as classmethods (``TrackInfo.from_plex_track``, -``TrackInfo.from_jellyfin_dict``, ``TrackInfo.from_navidrome_dict``) -— same pattern Cin used for ``Album.from_spotify_dict`` etc. on the -metadata engine. Heavy server SDK types (``PlexTrack``, dict -shapes) imported under TYPE_CHECKING so this module stays -import-light. +This module owns the canonical types. Plex's existing classmethod +constructors (``TrackInfo.from_plex_track``, +``PlaylistInfo.from_plex_playlist``) live here. Jellyfin and +Navidrome currently construct ``TrackInfo`` inline at their call +sites — lifting those into matching ``from_jellyfin_dict`` / +``from_navidrome_dict`` classmethods is a clean followup but isn't +needed for the dataclass unification this module ships. + +Heavy server SDK types (``PlexTrack``) imported under TYPE_CHECKING +so this module stays import-light. """ from __future__ import annotations diff --git a/tests/media_server/test_engine.py b/tests/media_server/test_engine.py index eb0be123..262fbe1e 100644 --- a/tests/media_server/test_engine.py +++ b/tests/media_server/test_engine.py @@ -107,20 +107,9 @@ def test_is_connected_routes_to_active_client(make_engine): assert engine.is_connected() is True # follows plex -def test_get_all_album_ids_returns_active_clients_set(make_engine): - plex = _FakeClient('plex') - plex._album_ids = {'p-1', 'p-2'} - jelly = _FakeClient('jellyfin') - jelly._album_ids = {'j-1'} - engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='plex') - assert engine.get_all_album_ids() == {'p-1', 'p-2'} - engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='jellyfin') - assert engine.get_all_album_ids() == {'j-1'} - - -def test_engine_returns_safe_defaults_when_active_client_failed_to_init(make_engine): +def test_engine_is_connected_returns_false_when_active_client_failed_to_init(): """When the active client failed to initialize (registry stored - None), the engine returns safe defaults instead of raising.""" + None), the engine returns False instead of raising.""" registry = MediaServerRegistry() registry.register(ServerSpec( name='broken', @@ -131,65 +120,18 @@ def test_engine_returns_safe_defaults_when_active_client_failed_to_init(make_eng engine = MediaServerEngine(registry=registry, active_server_resolver=lambda: 'broken') assert engine.is_connected() is False - assert engine.get_all_artists() == [] - assert engine.get_all_album_ids() == set() - assert engine.search_tracks('t', 'a') == [] - assert engine.trigger_library_scan() is False - assert engine.is_library_scanning() is False + assert engine.active_client() is None -def test_engine_swallows_per_method_exceptions(make_engine): - """A method that raises must NOT propagate to the dispatch - site — engine returns the safe default instead, mirroring the - legacy web_server.py defensive try/except chains.""" +def test_is_connected_swallows_exception_from_client(make_engine): + """If the client's is_connected raises, engine returns False + instead of propagating — dashboard status indicators stay + responsive even if a server is misbehaving.""" plex = _FakeClient('plex') plex.is_connected = MagicMock(side_effect=RuntimeError("boom")) - plex.get_all_album_ids = MagicMock(side_effect=RuntimeError("boom")) engine = make_engine({'plex': plex}, active='plex') assert engine.is_connected() is False - assert engine.get_all_album_ids() == set() - - -# --------------------------------------------------------------------------- -# Optional-method dispatch (engine returns safe default when missing) -# --------------------------------------------------------------------------- - - -class _MinimalClient: - """Stand-in for SoulSync standalone — only the required methods, - NO optional methods. Used to assert engine routes around missing - optional methods with safe defaults.""" - - def is_connected(self): return True - def ensure_connection(self): return True - def get_all_artists(self): return [] - def get_all_album_ids(self): return set() - - -def test_search_tracks_returns_empty_when_client_lacks_method(make_engine): - """SoulSync standalone has no search_tracks — engine returns - [] instead of raising AttributeError.""" - engine = make_engine({'soulsync': _MinimalClient()}, active='soulsync') - assert engine.search_tracks('t', 'a') == [] - - -def test_trigger_library_scan_returns_true_when_client_lacks_method(make_engine): - """SoulSync has no trigger_library_scan (filesystem walks - happen in-process). Engine no-ops with True so callers don't - treat it as a failure.""" - engine = make_engine({'soulsync': _MinimalClient()}, active='soulsync') - assert engine.trigger_library_scan() is True - - -def test_is_library_scanning_returns_false_when_client_lacks_method(make_engine): - engine = make_engine({'soulsync': _MinimalClient()}, active='soulsync') - assert engine.is_library_scanning() is False - - -def test_get_library_stats_returns_empty_dict_when_client_lacks_method(make_engine): - engine = make_engine({'soulsync': _MinimalClient()}, active='soulsync') - assert engine.get_library_stats() == {} # --------------------------------------------------------------------------- @@ -306,7 +248,7 @@ def test_engine_with_empty_clients_dict_is_safe_to_use(): full engine init raises — preserves the resilience the per-server globals had pre-refactor (each one had its own try/except so engine failure didn't take down dispatch sites). Pin the contract: empty - engine still answers safely on every method instead of raising.""" + engine still answers safely on every accessor instead of raising.""" registry = MediaServerRegistry() registry.register(ServerSpec( name='plex', factory=lambda: _FakeClient('plex'), display_name='Plex', @@ -320,11 +262,10 @@ def test_engine_with_empty_clients_dict_is_safe_to_use(): # client(name) returns None for every server — engine doesn't crash. assert engine.client('plex') is None assert engine.client('jellyfin') is None - # Cross-server methods return safe defaults instead of raising. + # Active-client lookup + is_connected gracefully handle the empty + # case without raising. + assert engine.active_client() is None assert engine.is_connected() is False - assert engine.get_all_artists() == [] - assert engine.get_all_album_ids() == set() - assert engine.search_tracks('t', 'a') == [] # configured_clients() returns empty dict cleanly. assert engine.configured_clients() == {}