Phase 0: Add MediaServerClient contract + registry

`core/media_server/` package with the Protocol contract that
every media server client (Plex, Jellyfin, Navidrome, SoulSync
standalone) satisfies, plus the registry that holds them.

Required methods conservatively limited to the four every server
truly implements today: is_connected, ensure_connection,
get_all_artists, get_all_album_ids. Other generic methods
(search_tracks, trigger_library_scan, get_recently_added_albums,
etc.) are listed as OPTIONAL — present on most servers but not
all (SoulSync has no library-scan API since it walks the filesystem
directly; Jellyfin uses a different search shape). Phase B's
engine adapters route around the gaps with per-server fallback
instead of forcing every client to declare a no-op stub.

Same registry shape as the download plugin registry — single
source of truth for which servers exist + name resolution. Adding
a 5th server (Subsonic, Emby, etc.) becomes one register call
plus the new client class.

5 conformance tests pin every server class implements every
required method. Plan doc at docs/media-server-engine-refactor-plan.md.

Pure additive — no consumer routes through the contract or
registry yet. Suite still green (1921 passed).
pull/497/head
Broque Thomas 3 weeks ago
parent 0eaac77627
commit f702196dca

@ -0,0 +1,23 @@
"""Media server engine — central dispatch for 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.
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.
See ``docs/media-server-engine-refactor-plan.md`` for the full
phased plan.
"""
from core.media_server.contract import MediaServerClient
from core.media_server.registry import MediaServerRegistry, build_default_registry
__all__ = ["MediaServerClient", "MediaServerRegistry", "build_default_registry"]

@ -0,0 +1,148 @@
"""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.
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 /
SoulSyncClient grew the same shape independently because every
caller needed the same calls. This file just makes the implicit
contract explicit.
"""
from __future__ import annotations
from typing import Any, Dict, List, Optional, Protocol, runtime_checkable
@runtime_checkable
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.
"""
# ------------------------------------------------------------------
# Connection / lifecycle
# ------------------------------------------------------------------
def is_connected(self) -> bool:
"""Cheap probe — does the client have a live connection /
token / session right now? Used by the dashboard status
indicators + endpoint guards."""
...
def ensure_connection(self) -> bool:
"""Re-auth or reconnect if needed. May make a network call.
Returns True if connection is usable after the call."""
...
# ------------------------------------------------------------------
# Library reads (required — every server must support these)
# ------------------------------------------------------------------
def get_all_artists(self) -> List[Any]:
"""Return every artist the server knows about. Each item is
a server-specific wrapper object (PlexArtist, JellyfinArtist,
NavidromeArtist, SoulSyncArtist) caller treats them
opaquely."""
...
def get_all_album_ids(self) -> set:
"""Return the set of every album ID in the library. ID
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.
# ---------------------------------------------------------------------------
# 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',
'get_all_artists',
'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 = {
'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',
}

@ -0,0 +1,127 @@
"""Media server plugin registry.
Single source of truth for which servers exist, what their canonical
names are, and which client class implements each. Replaces the
historic web_server.py pattern of holding 4 separate client globals
+ 33 hand-maintained ``if active_server == 'plex' / 'jellyfin' / ...``
dispatch sites.
Adding a new server (e.g. Subsonic, Emby) becomes one ``register``
call here + the new client class. Web server dispatch stays put.
"""
from __future__ import annotations
from dataclasses import dataclass, field
from typing import Callable, Dict, Iterator, List, Optional, Tuple
from utils.logging_config import get_logger
from core.media_server.contract import MediaServerClient
# Eager imports for the same import-order reason the download plugin
# registry uses them (some integration tests inject mock modules into
# sys.modules at collection time; lazy import would bind to the mock).
from core.jellyfin_client import JellyfinClient
from core.navidrome_client import NavidromeClient
from core.plex_client import PlexClient
from core.soulsync_client import SoulSyncClient
logger = get_logger("media_server.registry")
@dataclass(frozen=True)
class ServerSpec:
"""Static descriptor for a media server. ``factory`` is the
zero-arg callable that builds the client (each server has its
own setup chain Plex pulls token from config, Jellyfin reads
user_id, etc.)."""
name: str
factory: Callable[[], MediaServerClient]
display_name: str
aliases: Tuple[str, ...] = field(default_factory=tuple)
class MediaServerRegistry:
"""Holds the live client instances + name → instance lookup.
Two-phase construction (mirrors the download plugin registry):
1. Specs registered cheaply (just stores callable refs).
2. ``initialize()`` calls each factory once. Failures captured
in ``init_failures`` so one broken server doesn't take down
the orchestrator.
"""
def __init__(self) -> None:
self._specs: Dict[str, ServerSpec] = {}
self._instances: Dict[str, Optional[MediaServerClient]] = {}
self._init_failures: List[str] = []
def register(self, spec: ServerSpec) -> None:
if spec.name in self._specs:
raise ValueError(f"Server already registered: {spec.name}")
self._specs[spec.name] = spec
def initialize(self) -> None:
for spec in self._specs.values():
try:
instance = spec.factory()
self._instances[spec.name] = instance
except Exception as exc:
logger.error("%s media server client failed to initialize: %s", spec.display_name, exc)
self._init_failures.append(spec.display_name)
self._instances[spec.name] = None
@property
def init_failures(self) -> List[str]:
return list(self._init_failures)
def get(self, name: str) -> Optional[MediaServerClient]:
if not name:
return None
if name in self._instances:
return self._instances[name]
for spec in self._specs.values():
if name in spec.aliases:
return self._instances.get(spec.name)
return None
def get_spec(self, name: str) -> Optional[ServerSpec]:
if name in self._specs:
return self._specs[name]
for spec in self._specs.values():
if name in spec.aliases:
return spec
return None
def display_name(self, name: str) -> str:
spec = self.get_spec(name)
return spec.display_name if spec else name
def names(self) -> List[str]:
return list(self._specs.keys())
def all_clients(self) -> Iterator[Tuple[str, MediaServerClient]]:
"""Yield (name, client) for every successfully-initialized
server. Used by cross-server operations."""
for name, instance in self._instances.items():
if instance is not None:
yield name, instance
def build_default_registry() -> MediaServerRegistry:
"""Construct the registry with SoulSync's four built-in media
servers. Called once during MediaServerEngine construction.
Adding a server (e.g. Subsonic, Emby) = one ``register`` call
here + the new client class. No dispatch-site changes required.
"""
registry = MediaServerRegistry()
registry.register(ServerSpec(name='plex', factory=PlexClient, display_name='Plex'))
registry.register(ServerSpec(name='jellyfin', factory=JellyfinClient, display_name='Jellyfin'))
registry.register(ServerSpec(name='navidrome', factory=NavidromeClient, display_name='Navidrome'))
registry.register(ServerSpec(name='soulsync', factory=SoulSyncClient, display_name='SoulSync Library'))
return registry

@ -0,0 +1,159 @@
# Media Server Engine Refactor Plan
## Goal
Same playbook as the download engine refactor, applied to media servers. Replace 33 `if active_server == 'plex' / 'jellyfin' / 'navidrome' / 'soulsync'` dispatch sites in web_server.py with a central `MediaServerEngine` that owns server selection + cross-server query dispatch. Each per-server client stays as-is for its protocol-specific work (Plex API SDK, Jellyfin REST, Navidrome OpenSubsonic, SoulSync filesystem walk).
**The smell:** Adding a 5th server (recent SoulSync standalone showed how) requires hunting through 33 web_server.py dispatch sites + DatabaseUpdateWorker branching + UI sync-button visibility hacks. Plex assumptions had leaked into nominally generic code paths.
## Architecture target
```
┌───────────┐ ┌──────────────────┐
│ feature │ ─search/scan/sync─▶ ┌──│ MediaServerEngine │ ─▶ Plex (PlexAPI SDK)
│ │ ◀── results ────── │ │ ◆ active selection│ ─▶ Jellyfin (REST)
└───────────┘ │ │ ◆ dispatch │ ─▶ Navidrome (Subsonic)
│ │ ◆ shared shape │ ─▶ SoulSync (filesystem)
│ └──────────────────┘
│ (no need for per-server thread pool — server APIs
│ are sync-call shaped, not stream-shaped like downloads)
```
## What clients keep (per-server, legitimately)
- **Auth** — Plex token, Jellyfin API key + user ID + library ID, Navidrome user/pass + salt, SoulSync filesystem path
- **Wire protocol** — PlexAPI SDK objects vs Jellyfin REST `/Items` vs Navidrome XML/JSON Subsonic vs filesystem walk
- **ID schemes** — Plex ratingKey (int), Jellyfin GUID (hex), Navidrome string, SoulSync MD5
- **Connection state** — each client owns its session / token / connection object
- **Cache strategy** — Jellyfin's aggressive pre-cache, Navidrome's folder filter, SoulSync's 5-min TTL
- **Wrapper objects**`JellyfinTrack`, `NavidromeTrack`, etc. stay in client modules
## What moves into the engine
| Today (web_server.py duplicated) | Tomorrow (MediaServerEngine, single dispatch site) |
|---|---|
| `if active_server == 'plex': plex_client.X() elif 'jellyfin': jellyfin_client.X() ...` | `engine.X()` — engine reads `active_server` once, routes |
| Status check 4-way branch | `engine.is_connected()` |
| Library scan trigger 3-way | `engine.trigger_library_scan()` |
| Search dispatch in 3 sites | `engine.search_tracks(title, artist)` |
| Play history 4-way | `engine.get_play_history()` |
| Metadata writeback 4-way (genre / poster / bio) | `engine.update_artist_genres()` etc. — engine routes, plugin no-ops where unsupported |
| `DatabaseUpdateWorker.server_type` branching | `engine` injected, no per-server branching |
## Plugin contract (ABC, narrower than the download contract)
```python
class MediaServerClient(ABC):
# Connection
@abstractmethod
def is_connected(self) -> bool: ...
@abstractmethod
def ensure_connection(self) -> bool: ...
# Library reads (required)
@abstractmethod
def get_all_artists(self): ...
@abstractmethod
def get_all_album_ids(self): ...
@abstractmethod
def search_tracks(self, title, artist, limit=15): ...
@abstractmethod
def get_recently_added_albums(self, max_results=400): ...
# Library writes (required for the scan endpoints — may no-op for SoulSync)
@abstractmethod
def trigger_library_scan(self) -> bool: ...
@abstractmethod
def is_library_scanning(self) -> bool: ...
# Playlist sync (optional — Navidrome stub returns True)
def create_playlist(self, name, tracks) -> bool: return True
def update_playlist(self, name, tracks) -> bool: return True
def copy_playlist(self, source, dest_name) -> bool: return True
# Analytics (optional)
def get_play_history(self, limit=500) -> list: return []
def get_track_play_counts(self) -> dict: return {}
# Metadata writeback (optional — clients no-op where API doesn't support it)
def update_artist_genres(self, artist, genres) -> bool: return True
def update_artist_poster(self, artist, image_data) -> bool: return True
def update_album_poster(self, album, image_data) -> bool: return True
def update_artist_biography(self, artist) -> bool: return True
```
## Phased commit plan
### Phase 0 — Foundation
- **0.1** Plugin contract (`core/media_server/contract.py`) + ABC
- **0.2** Registry + dispatch helpers (`core/media_server/registry.py`)
- **0.3** Conformance tests — every registered client satisfies the ABC
### Phase A — Behavior pinning
- **A1** Pin Plex client surface (status check, search, scan trigger, metadata writeback shape)
- **A2** Pin Jellyfin client surface
- **A3** Pin Navidrome client surface
- **A4** Pin SoulSync standalone client surface
### Phase B — Engine skeleton
- **B1** `MediaServerEngine` skeleton — holds clients, exposes `active_server` selection
- **B2** Engine dispatch methods (`is_connected`, `search_tracks`, `trigger_library_scan`, `is_library_scanning`, etc.)
- **B3** Engine-level conformance tests
### Phase C — Migrate web_server.py dispatch sites
Each commit migrates a logical cluster (status, scan, playlist, metadata, history) so the diff per commit is small + reviewable.
- **C1** Status / connection-check sites
- **C2** Library scan trigger + scanning-state polling
- **C3** Search dispatch (3 sites)
- **C4** Playlist sync / apply / suggest
- **C5** Play history + track play counts
- **C6** Metadata writeback (genres / posters / bio)
### Phase D — DatabaseUpdateWorker
- **D1** Inject engine into `DatabaseUpdateWorker`
- **D2** Strip `self.server_type` branching (use `engine.get_active_client_type()` where shape genuinely differs)
### Phase E — Cleanup + ship
- **E1** Drop unused imports / dead code
- **E2** WHATS_NEW + PR description
**Total estimated:** 15-18 commits. ~600 LOC moved, ~200 LOC deleted.
## Risk profile
**Low:**
- Phase 0 (pure additive — new contract, no existing code touched)
- Phase A (tests only)
- Phase B (engine exists but nothing routes through it yet)
**Medium:**
- Phase C (each commit changes 5-10 dispatch sites in web_server.py — cross-cutting)
- Phase D (DatabaseUpdateWorker is a hot path during library refresh)
**Mitigation:**
- Phase A pinning catches per-client behavior drift
- Phase B engine tested in isolation before C wires it in
- Phase C commits are small + reversible (one cluster per commit)
- Suite green at every commit
## What's NOT in this PR
- Unifying track ID schemes (Plex int vs Jellyfin GUID vs Navidrome string vs SoulSync MD5) — separate data model refactor
- Merging Plex/Jellyfin/Navidrome wrapper objects (each tied to its API)
- Extracting common metadata writeback logic — too server-specific
- Adding new server types (Subsonic, MusicBrainz local) — out of scope; this PR makes them easier later
## Coordination with other work
- Independent of the download engine refactor (PR #494) — different subsystem
- Cin's metadata engine work also independent
- No collisions expected
## Compatibility commitments
- All existing config keys preserved
- All existing API endpoints preserved
- Plex/Jellyfin/Navidrome/SoulSync clients keep their public methods (engine wraps, doesn't replace)
- Active-server config (`server.active`) unchanged
- `DatabaseUpdateWorker.server_type` available for external readers (deprecated but not removed)

@ -0,0 +1,54 @@
"""Pin the structural conformance of every media server client to
``MediaServerClient``. Mirrors the download plugin conformance test
shape class-level checks (not instance-level) so importing the
test doesn't drag every server's heavy auth init into the test
collection phase.
"""
from __future__ import annotations
import pytest
from core.media_server.contract import REQUIRED_METHODS
def _import_server_classes():
"""Import every server client class lazily inside tests so
auth-init-heavy modules (Plex, Jellyfin) aren't imported at test
collection."""
from core.jellyfin_client import JellyfinClient
from core.navidrome_client import NavidromeClient
from core.plex_client import PlexClient
from core.soulsync_client import SoulSyncClient
return {
'plex': PlexClient,
'jellyfin': JellyfinClient,
'navidrome': NavidromeClient,
'soulsync': SoulSyncClient,
}
def test_default_registry_registers_all_four_servers():
"""Smoke check that the foundation registry knows about every
server SoulSync historically dispatched to."""
from core.media_server import build_default_registry
registry = build_default_registry()
expected = {'plex', 'jellyfin', 'navidrome', 'soulsync'}
assert set(registry.names()) == expected
@pytest.mark.parametrize('server_name', ['plex', 'jellyfin', 'navidrome', 'soulsync'])
def test_server_class_has_all_required_methods(server_name):
"""Every registered server class exposes every required protocol
method by name. Diagnostic-friendly: tells you WHICH method is
missing when a new server is added without all the required
methods."""
classes = _import_server_classes()
cls = classes[server_name]
missing = [m for m in REQUIRED_METHODS if not hasattr(cls, m)]
assert not missing, (
f"{server_name} ({cls.__name__}) missing required methods: {missing}"
)
Loading…
Cancel
Save