MS Cin/JohnBaumb honesty pass — drop dead wrappers, sync contract to reality
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).
This commit is contained in:
Broque Thomas
parent
99f527ecd1
commit
6489244bcc
5 changed files with 114 additions and 314 deletions
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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).<method>`` 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',
|
||||
}
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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() == {}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue