Merge pull request #497 from Nezreka/refactor/media-server-engine

Refactor/media server engine
This commit is contained in:
BoulderBadgeDad 2026-05-06 09:07:43 -07:00 committed by GitHub
commit 549c885f02
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 1929 additions and 375 deletions

View file

@ -39,8 +39,7 @@ class SyncDeps:
"""Bundle of cross-cutting deps the sync worker needs."""
config_manager: Any
sync_service: Any
plex_client: Any
jellyfin_client: Any
media_server_engine: Any
automation_engine: Any
run_async: Callable[..., Any]
record_sync_history_start: Callable
@ -227,8 +226,9 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p
# Check sync service components
logger.info(f" spotify_client: {sync_service.spotify_client is not None}")
logger.info(f" deps.plex_client: {sync_service.plex_client is not None}")
logger.info(f" deps.jellyfin_client: {sync_service.jellyfin_client is not None}")
_ms_engine = getattr(sync_service, '_engine', None)
logger.info(f" plex_client: {(_ms_engine.client('plex') if _ms_engine else None) is not None}")
logger.info(f" jellyfin_client: {(_ms_engine.client('jellyfin') if _ms_engine else None) is not None}")
# Check media server connection before starting
from config.settings import config_manager
@ -404,11 +404,12 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p
try:
active_server = deps.config_manager.get_active_media_server()
logger.info(f"[PLAYLIST IMAGE] active_server={active_server}")
if active_server == 'plex' and deps.plex_client:
ok = deps.plex_client.set_playlist_image(playlist_name, playlist_image_url)
_engine = deps.media_server_engine
if active_server == 'plex' and _engine and _engine.client('plex'):
ok = _engine.client('plex').set_playlist_image(playlist_name, playlist_image_url)
logger.info(f"[PLAYLIST IMAGE] Plex upload result: {ok}")
elif active_server in ('jellyfin', 'emby') and deps.jellyfin_client:
ok = deps.jellyfin_client.set_playlist_image(playlist_name, playlist_image_url)
elif active_server in ('jellyfin', 'emby') and _engine and _engine.client('jellyfin'):
ok = _engine.client('jellyfin').set_playlist_image(playlist_name, playlist_image_url)
logger.info(f"[PLAYLIST IMAGE] Jellyfin upload result: {ok}")
# Navidrome doesn't support custom playlist images
except Exception as img_err:

View file

@ -1,33 +1,19 @@
import requests
import time
from typing import List, Optional, Dict, Any
from dataclasses import dataclass
from datetime import datetime
import json
from utils.logging_config import get_logger
from config.settings import config_manager
# Shared dataclasses live in the neutral media_server package — every
# server client used to define a near-identical XTrackInfo /
# XPlaylistInfo. Lifted to one canonical type so consumers (matching
# engine, sync service) get a single import.
from core.media_server.types import TrackInfo, PlaylistInfo
logger = get_logger("jellyfin_client")
@dataclass
class JellyfinTrackInfo:
id: str
title: str
artist: str
album: str
duration: int
track_number: Optional[int] = None
year: Optional[int] = None
rating: Optional[float] = None
@dataclass
class JellyfinPlaylistInfo:
id: str
title: str
description: Optional[str]
duration: int
leaf_count: int
tracks: List[JellyfinTrackInfo]
class JellyfinArtist:
"""Wrapper class to mimic Plex artist object interface"""
@ -146,7 +132,10 @@ class JellyfinTrack:
return self._client.get_album_by_id(self._album_id)
return None
class JellyfinClient:
from core.media_server.contract import MediaServerClient
class JellyfinClient(MediaServerClient):
def __init__(self):
self.base_url: Optional[str] = None
self.api_key: Optional[str] = None
@ -1201,7 +1190,7 @@ class JellyfinClient:
return stats
def get_all_playlists(self) -> List[JellyfinPlaylistInfo]:
def get_all_playlists(self) -> List[PlaylistInfo]:
"""Get all playlists from Jellyfin server"""
if not self.ensure_connection():
return []
@ -1218,7 +1207,7 @@ class JellyfinClient:
playlists = []
for item in response.get('Items', []):
playlist_info = JellyfinPlaylistInfo(
playlist_info = PlaylistInfo(
id=item.get('Id', ''),
title=item.get('Name', 'Unknown Playlist'),
description=item.get('Overview'),
@ -1235,7 +1224,7 @@ class JellyfinClient:
logger.error(f"Error getting playlists from Jellyfin: {e}")
return []
def get_playlist_by_name(self, name: str) -> Optional[JellyfinPlaylistInfo]:
def get_playlist_by_name(self, name: str) -> Optional[PlaylistInfo]:
"""Get a specific playlist by name"""
playlists = self.get_all_playlists()
for playlist in playlists:

View file

@ -19,13 +19,17 @@ logger = get_logger("listening_stats_worker")
class ListeningStatsWorker:
"""Background worker that polls media servers for play data."""
def __init__(self, database, config_manager, plex_client=None,
jellyfin_client=None, navidrome_client=None):
def __init__(self, database, config_manager, media_server_engine=None):
"""Initialize the worker.
``media_server_engine`` owns the per-server clients (Plex /
Jellyfin / Navidrome). The worker resolves the active server's
client through ``self._engine.client(name)`` instead of holding
per-server kwargs.
"""
self.db = database
self.config_manager = config_manager
self.plex_client = plex_client
self.jellyfin_client = jellyfin_client
self.navidrome_client = navidrome_client
self._engine = media_server_engine
# Worker state
self.running = False
@ -145,13 +149,11 @@ class ListeningStatsWorker:
logger.info(f"Polling {active_server} for listening data...")
self.current_item = f"Polling {active_server}..."
client = None
if active_server == 'plex' and self.plex_client:
client = self.plex_client
elif active_server == 'jellyfin' and self.jellyfin_client:
client = self.jellyfin_client
elif active_server == 'navidrome' and self.navidrome_client:
client = self.navidrome_client
client = self._engine.client(active_server) if self._engine else None
# SoulSync standalone has no listening data; only the three
# streaming servers contribute. Mirror the legacy guard here.
if active_server not in ('plex', 'jellyfin', 'navidrome'):
client = None
if not client:
logger.warning(f"No client available for active server: {active_server}")

View file

@ -7,7 +7,10 @@ from utils.logging_config import get_logger
from config.settings import config_manager
from core.spotify_client import Track as SpotifyTrack
from core.plex_client import PlexTrackInfo
from core.media_server.types import TrackInfo
# TrackResult / AlbumResult moved out of core.soulseek_client into the
# neutral download_plugins package (download PR's Gap 1 lift). Import
# from the new location.
from core.download_plugins.types import TrackResult, AlbumResult
@ -16,7 +19,7 @@ logger = get_logger("matching_engine")
@dataclass
class MatchResult:
spotify_track: SpotifyTrack
plex_track: Optional[PlexTrackInfo]
plex_track: Optional[TrackInfo]
confidence: float
match_type: str
@ -302,7 +305,7 @@ class MusicMatchingEngine:
confidence = (title_score * 0.60) + (artist_score * 0.30) + (duration_score * 0.10)
return confidence, "standard_match"
def calculate_match_confidence(self, spotify_track: SpotifyTrack, plex_track: PlexTrackInfo) -> Tuple[float, str]:
def calculate_match_confidence(self, spotify_track: SpotifyTrack, plex_track: TrackInfo) -> Tuple[float, str]:
"""Calculates a confidence score using a prioritized model, starting with a strict 'core' title check."""
return self.score_track_match(
source_title=spotify_track.name,
@ -313,7 +316,7 @@ class MusicMatchingEngine:
candidate_duration_ms=plex_track.duration if plex_track.duration else 0
)
def find_best_match(self, spotify_track: SpotifyTrack, plex_tracks: List[PlexTrackInfo]) -> MatchResult:
def find_best_match(self, spotify_track: SpotifyTrack, plex_tracks: List[TrackInfo]) -> MatchResult:
"""Finds the best Plex track match from a list of candidates."""
best_match = None
best_confidence = 0.0

View file

@ -0,0 +1,39 @@
"""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. 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.
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.
Note: only ``MediaServerClient`` is re-exported here. The engine +
registry are NOT importing the registry triggers eager imports
of every per-server client class, and those clients now inherit
``MediaServerClient`` (Cin-1), so re-exporting them here would
form a circular import the moment a client tried to resolve its
base class. Import them directly from their submodules:
from core.media_server.engine import MediaServerEngine
from core.media_server.registry import build_default_registry
"""
from core.media_server.contract import MediaServerClient
__all__ = [
"MediaServerClient",
]

View file

@ -0,0 +1,110 @@
"""Canonical contract for media server clients.
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.
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 four calls. This file just makes that
implicit contract explicit + the conformance test pins it.
"""
from __future__ import annotations
from typing import Any, List, Protocol, runtime_checkable
@runtime_checkable
class MediaServerClient(Protocol):
"""Structural contract every media server client must satisfy.
``runtime_checkable`` lets ``isinstance(client, MediaServerClient)``
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 — required, every server implements
# ------------------------------------------------------------------
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 implements
# ------------------------------------------------------------------
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."""
...
# ---------------------------------------------------------------------------
# Required method set — pinned by the conformance test. Mirrors the
# Protocol body exactly so static + runtime contracts can't drift.
# ---------------------------------------------------------------------------
REQUIRED_METHODS = {
'is_connected',
'ensure_connection',
'get_all_artists',
'get_all_album_ids',
}
# 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',
'create_playlist',
'update_playlist',
'copy_playlist',
'get_all_playlists',
'get_playlist_by_name',
'get_play_history',
'get_track_play_counts',
'update_artist_genres',
'update_artist_poster',
'update_album_poster',
'update_artist_biography',
'update_track_metadata',
)

209
core/media_server/engine.py Normal file
View file

@ -0,0 +1,209 @@
"""MediaServerEngine — central registry-backed access to media server clients.
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.
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
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 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, Optional
from utils.logging_config import get_logger
from core.media_server.contract import MediaServerClient
from core.media_server.registry import MediaServerRegistry, build_default_registry
logger = get_logger("media_server.engine")
class MediaServerEngine:
"""Registry-backed access to the per-server media clients.
Owns the per-server client instances + a small set of generic
accessors (``client(name)`` / ``active_client()`` /
``configured_clients()`` / ``reload_config(name)``) so call sites
don't reach for separate per-server globals. The one cross-server
dispatch wrapper kept on the engine ``is_connected()``
backs the dashboard status indicators that have multiple call
sites; everything else dispatches per-server in the call site
itself, reaching the relevant client through ``engine.client(name)``.
"""
def __init__(
self,
registry: Optional[MediaServerRegistry] = None,
active_server_resolver=None,
clients: Optional[Dict[str, Any]] = None,
) -> None:
"""Initialize the engine.
Args:
registry: Plugin registry. Defaults to the four built-in
servers (Plex, Jellyfin, Navidrome, SoulSync).
active_server_resolver: Callable returning the current
active server name (e.g. ``'plex'``). Defaults to
``config_manager.get_active_media_server``. Tests
inject a custom resolver to switch active server
without touching real config.
clients: Pre-built {name: client_instance} dict. When
provided, the engine wraps these instances directly
instead of asking the registry to construct fresh
ones. web_server.py uses this so the engine
shares the same client objects as the
pre-existing global variables (no double-init).
"""
self.registry = registry if registry is not None else build_default_registry()
if clients is not None:
# Wrap pre-built instances (production case from web_server.py
# init). Skip registry.initialize() — we already have the
# instances, hand them off via the registry's public
# set_instance(name, client) method so internal storage stays
# encapsulated.
for name, client in clients.items():
self.registry.set_instance(name, client)
# Mark any registered-but-not-supplied as failed init so
# active_client() returns None for them.
for name in self.registry.names():
if self.registry.get(name) is None and name not in clients:
self.registry.set_instance(name, None)
else:
self.registry.initialize()
if active_server_resolver is None:
from config.settings import config_manager
active_server_resolver = config_manager.get_active_media_server
self._resolve_active = active_server_resolver
# ------------------------------------------------------------------
# Client lookup — generic accessors that replace per-server
# attribute reaches in callers.
# ------------------------------------------------------------------
def client(self, name: str) -> Optional[MediaServerClient]:
"""Return the client instance for the given server name, or
None if it's not registered / failed to initialize. Used by
callers that need a server-specific method beyond the
contract surface."""
return self.registry.get(name)
@property
def active_server(self) -> str:
"""The currently-selected media server name."""
return self._resolve_active()
def active_client(self) -> Optional[MediaServerClient]:
"""The client for the currently-active server."""
return self.registry.get(self.active_server)
def is_connected(self) -> bool:
"""Active server's connection state. False if no active
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
try:
return client.is_connected()
except Exception as exc:
logger.warning("%s is_connected raised: %s", self.active_server, exc)
return False
def configured_clients(self) -> Dict[str, MediaServerClient]:
"""Return ``{name: client}`` for every server that's both
registered AND reports ``is_connected() == True``. Replaces
the legacy per-server `if X and X.is_connected(): ...`
chains in web_server.py.
``is_connected`` is in REQUIRED_METHODS, so every client the
registry yields here implements it no hasattr guard needed.
"""
result: Dict[str, MediaServerClient] = {}
for name, client in self.registry.all_clients():
try:
if client.is_connected():
result[name] = client
except Exception as exc:
logger.warning("%s is_connected raised in configured_clients: %s", name, exc)
return result
def reload_config(self, name: Optional[str] = None) -> bool:
"""Reload config on a single server (or every server when
``name`` is None). Generic dispatch caller passes the name
instead of reaching for ``plex_client.reload_config()``
/ ``jellyfin_client.reload_config()`` directly. Servers
without a ``reload_config`` method are silently skipped.
"""
names = [name] if name else list(self.registry.names())
ok = True
for n in names:
client = self.client(n)
if client is None or not hasattr(client, 'reload_config'):
continue
try:
client.reload_config()
except Exception as exc:
logger.warning("%s reload_config failed: %s", n, exc)
ok = False
return ok
# ---------------------------------------------------------------------------
# Singleton accessor — mirrors the get_metadata_engine() /
# get_download_orchestrator() pattern so callers that don't need a
# custom registry use this instead of instantiating MediaServerEngine
# directly. web_server.py constructs the singleton at startup and
# installs it via ``set_media_server_engine`` so the factory + the
# global handle share state.
# ---------------------------------------------------------------------------
_default_engine: Optional['MediaServerEngine'] = None
def get_media_server_engine() -> 'MediaServerEngine':
"""Return (lazily creating) the process-wide MediaServerEngine
singleton. Mirrors the ``get_metadata_engine()`` /
``get_download_orchestrator()`` shape."""
global _default_engine
if _default_engine is None:
_default_engine = MediaServerEngine()
return _default_engine
def set_media_server_engine(engine: Optional['MediaServerEngine']) -> None:
"""Set the process-wide singleton. Used by web_server.py at boot
to install the engine it constructs (with the pre-built per-client
instances) as the default for callers reaching via
``get_media_server_engine()``."""
global _default_engine
_default_engine = engine

View file

@ -0,0 +1,142 @@
"""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
that every dispatch site reached individually.
Server-specific dispatch chains in web_server.py (playlist add /
remove / replace, per-server metadata sync, etc.) still hand-branch
on ``active_server == X`` because the work each server does at those
sites is genuinely different but they reach the per-server CLIENT
through ``engine.client(name)`` (which goes through this registry)
instead of separate globals.
Adding a new server (Subsonic / Emby) = one ``register`` call here +
the new client class.
"""
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
def set_instance(self, name: str, instance: Optional[MediaServerClient]) -> None:
"""Stash a pre-built client instance into the registry without
running the spec's factory. Used by callers (web_server.py at
boot) that already constructed the per-server clients and want
the engine to wrap those exact instances rather than build new
ones. Replaces the old pattern of reaching into ``_instances``
directly from the engine."""
self._instances[name] = instance
@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

132
core/media_server/types.py Normal file
View file

@ -0,0 +1,132 @@
"""Shared dataclasses for the media-server contract.
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. 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
from dataclasses import dataclass, field
from typing import TYPE_CHECKING, List, Optional
if TYPE_CHECKING:
# plexapi types — only loaded when type-checking; runtime
# paths through from_plex_track accept whatever PlexClient
# passes through.
from plexapi.audio import Track as PlexTrack
from plexapi.playlist import Playlist as PlexPlaylist
@dataclass
class TrackInfo:
"""Canonical track-shape returned by media server clients.
Plex, Jellyfin, and Navidrome each defined their own near-identical
``XTrackInfo`` dataclass (SoulSync standalone uses richer per-track
wrappers and doesn't surface this exact shape). Lifted to one
canonical type here so consumers (matching engine, sync service,
library scanners) get a single import.
"""
id: str
title: str
artist: str
album: str
duration: int
track_number: Optional[int] = None
year: Optional[int] = None
rating: Optional[float] = None
# ------------------------------------------------------------------
# Per-server constructors — mirror Cin's metadata Album.from_X_dict
# pattern. Each one knows ONE server's wire shape.
# ------------------------------------------------------------------
@classmethod
def from_plex_track(cls, track: 'PlexTrack') -> 'TrackInfo':
"""Build a TrackInfo from a plexapi PlexTrack.
Defensive: tracks may be missing artist or album metadata in
Plex (especially fan-uploaded content); fall back to
"Unknown Artist" / "Unknown Album" instead of raising.
"""
# Imported lazily so this module stays import-light. plexapi
# is heavy and pulls in network + ssl deps just to define
# exception types.
from plexapi.exceptions import NotFound
try:
artist_title = track.artist().title if track.artist() else "Unknown Artist"
except (NotFound, AttributeError):
artist_title = "Unknown Artist"
try:
album_title = track.album().title if track.album() else "Unknown Album"
except (NotFound, AttributeError):
album_title = "Unknown Album"
return cls(
id=str(track.ratingKey),
title=track.title,
artist=artist_title,
album=album_title,
duration=track.duration,
track_number=track.trackNumber,
year=track.year,
rating=track.userRating,
)
@dataclass
class PlaylistInfo:
"""Canonical playlist-shape returned by every media server client.
Same lift rationale as ``TrackInfo`` every server defined the
same five-field dataclass + a list of tracks.
"""
id: str
title: str
description: Optional[str]
duration: int
leaf_count: int
tracks: List[TrackInfo] = field(default_factory=list)
# ------------------------------------------------------------------
# Per-server constructors
# ------------------------------------------------------------------
@classmethod
def from_plex_playlist(cls, playlist: 'PlexPlaylist') -> 'PlaylistInfo':
"""Build a PlaylistInfo from a plexapi Playlist. Skips items
that aren't audio tracks (Plex playlists can mix media types
in theory, though music libraries shouldn't)."""
from plexapi.audio import Track as PlexTrack
tracks: List[TrackInfo] = []
for item in playlist.items():
if isinstance(item, PlexTrack):
tracks.append(TrackInfo.from_plex_track(item))
return cls(
id=str(playlist.ratingKey),
title=playlist.title,
description=playlist.summary,
duration=playlist.duration,
leaf_count=playlist.leafCount,
tracks=tracks,
)

View file

@ -2,33 +2,19 @@ import requests
import hashlib
import secrets
from typing import List, Optional, Dict, Any
from dataclasses import dataclass
from datetime import datetime
import json
from utils.logging_config import get_logger
from config.settings import config_manager
# Shared dataclasses live in the neutral media_server package — every
# server client used to define a near-identical XTrackInfo /
# XPlaylistInfo. Lifted to one canonical type so consumers (matching
# engine, sync service) get a single import.
from core.media_server.types import TrackInfo, PlaylistInfo
logger = get_logger("navidrome_client")
@dataclass
class NavidromeTrackInfo:
id: str
title: str
artist: str
album: str
duration: int
track_number: Optional[int] = None
year: Optional[int] = None
rating: Optional[float] = None
@dataclass
class NavidromePlaylistInfo:
id: str
title: str
description: Optional[str]
duration: int
leaf_count: int
tracks: List[NavidromeTrackInfo]
class NavidromeArtist:
"""Wrapper class to mimic Plex artist object interface"""
@ -150,7 +136,10 @@ class NavidromeTrack:
return self._client.get_album_by_id(self._album_id)
return None
class NavidromeClient:
from core.media_server.contract import MediaServerClient
class NavidromeClient(MediaServerClient):
def __init__(self):
self.base_url: Optional[str] = None
self.username: Optional[str] = None
@ -824,7 +813,7 @@ class NavidromeClient:
return {}
return {}
def get_all_playlists(self) -> List[NavidromePlaylistInfo]:
def get_all_playlists(self) -> List[PlaylistInfo]:
"""Get all playlists from Navidrome server"""
if not self.ensure_connection():
return []
@ -838,7 +827,7 @@ class NavidromeClient:
playlists_data = response.get('playlists', {}).get('playlist', [])
for playlist_data in playlists_data:
playlist_info = NavidromePlaylistInfo(
playlist_info = PlaylistInfo(
id=playlist_data.get('id', ''),
title=playlist_data.get('name', 'Unknown Playlist'),
description=playlist_data.get('comment'),
@ -855,7 +844,7 @@ class NavidromeClient:
logger.error(f"Error getting playlists from Navidrome: {e}")
return []
def get_playlist_by_name(self, name: str) -> Optional[NavidromePlaylistInfo]:
def get_playlist_by_name(self, name: str) -> Optional[PlaylistInfo]:
"""Get a specific playlist by name"""
playlists = self.get_all_playlists()
for playlist in playlists:
@ -976,7 +965,7 @@ class NavidromeClient:
logger.error(f"Error getting tracks for playlist {playlist_id}: {e}")
return []
def get_playlists_by_name(self, name: str) -> List[NavidromePlaylistInfo]:
def get_playlists_by_name(self, name: str) -> List[PlaylistInfo]:
"""Get all playlists matching a specific name (case-insensitive)"""
matches = []
playlists = self.get_all_playlists()
@ -1140,7 +1129,7 @@ class NavidromeClient:
self._track_cache.clear()
logger.info("Navidrome client cache cleared")
def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[NavidromeTrackInfo]:
def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[TrackInfo]:
"""Search for tracks using Navidrome search API"""
if not self.ensure_connection():
logger.warning("Navidrome not connected. Cannot perform search.")
@ -1166,7 +1155,7 @@ class NavidromeClient:
search_result = response.get('searchResult3', {})
for track_data in search_result.get('song', []):
track_info = NavidromeTrackInfo(
track_info = TrackInfo(
id=track_data.get('id', ''),
title=track_data.get('title', ''),
artist=track_data.get('artist', ''),

View file

@ -4,7 +4,6 @@ from plexapi.audio import Track as PlexTrack, Album as PlexAlbum, Artist as Plex
from plexapi.playlist import Playlist as PlexPlaylist
from plexapi.exceptions import PlexApiException, NotFound
from typing import List, Optional, Dict, Any
from dataclasses import dataclass
import requests
from datetime import datetime, timedelta
import re
@ -12,69 +11,17 @@ from utils.logging_config import get_logger
from config.settings import config_manager
import threading
# Shared dataclasses live in the neutral media_server package now —
# every server client used to define a near-identical XTrackInfo /
# XPlaylistInfo. Lifted to one canonical type so consumers (matching
# engine, sync service, etc.) get a single import.
from core.media_server.types import TrackInfo, PlaylistInfo
from core.media_server.contract import MediaServerClient
logger = get_logger("plex_client")
@dataclass
class PlexTrackInfo:
id: str
title: str
artist: str
album: str
duration: int
track_number: Optional[int] = None
year: Optional[int] = None
rating: Optional[float] = None
@classmethod
def from_plex_track(cls, track: PlexTrack) -> 'PlexTrackInfo':
# Gracefully handle tracks that might be missing artist or album metadata in Plex
try:
artist_title = track.artist().title if track.artist() else "Unknown Artist"
except (NotFound, AttributeError):
artist_title = "Unknown Artist"
try:
album_title = track.album().title if track.album() else "Unknown Album"
except (NotFound, AttributeError):
album_title = "Unknown Album"
return cls(
id=str(track.ratingKey),
title=track.title,
artist=artist_title,
album=album_title,
duration=track.duration,
track_number=track.trackNumber,
year=track.year,
rating=track.userRating
)
@dataclass
class PlexPlaylistInfo:
id: str
title: str
description: Optional[str]
duration: int
leaf_count: int
tracks: List[PlexTrackInfo]
@classmethod
def from_plex_playlist(cls, playlist: PlexPlaylist) -> 'PlexPlaylistInfo':
tracks = []
for item in playlist.items():
if isinstance(item, PlexTrack):
tracks.append(PlexTrackInfo.from_plex_track(item))
return cls(
id=str(playlist.ratingKey),
title=playlist.title,
description=playlist.summary,
duration=playlist.duration,
leaf_count=playlist.leafCount,
tracks=tracks
)
class PlexClient:
class PlexClient(MediaServerClient):
def __init__(self):
self.server: Optional[PlexServer] = None
self.music_library: Optional[MusicSection] = None
@ -268,7 +215,7 @@ class PlexClient:
"""Check if both server is connected AND music library is selected."""
return self.server is not None and self.music_library is not None
def get_all_playlists(self) -> List[PlexPlaylistInfo]:
def get_all_playlists(self) -> List[PlaylistInfo]:
if not self.ensure_connection():
logger.error("Not connected to Plex server")
return []
@ -278,7 +225,7 @@ class PlexClient:
try:
for playlist in self.server.playlists():
if getattr(playlist, 'playlistType', None) == 'audio':
playlist_info = PlexPlaylistInfo.from_plex_playlist(playlist)
playlist_info = PlaylistInfo.from_plex_playlist(playlist)
playlists.append(playlist_info)
logger.info(f"Retrieved {len(playlists)} audio playlists")
@ -288,14 +235,14 @@ class PlexClient:
logger.error(f"Error fetching playlists: {e}")
return []
def get_playlist_by_name(self, name: str) -> Optional[PlexPlaylistInfo]:
def get_playlist_by_name(self, name: str) -> Optional[PlaylistInfo]:
if not self.ensure_connection():
return None
try:
playlist = self.server.playlist(name)
if getattr(playlist, 'playlistType', None) == 'audio':
return PlexPlaylistInfo.from_plex_playlist(playlist)
return PlaylistInfo.from_plex_playlist(playlist)
return None
except NotFound:
@ -311,14 +258,14 @@ class PlexClient:
return False
try:
# Handle both PlexTrackInfo objects and actual Plex track objects
# Handle both TrackInfo objects and actual Plex track objects
plex_tracks = []
for track in tracks:
if hasattr(track, 'ratingKey'):
# This is already a Plex track object
plex_tracks.append(track)
elif hasattr(track, '_original_plex_track'):
# This is a PlexTrackInfo object with stored original track reference
# This is a TrackInfo object with stored original track reference
original_track = track._original_plex_track
if original_track is not None:
plex_tracks.append(original_track)
@ -326,7 +273,7 @@ class PlexClient:
else:
logger.warning(f"Stored track reference is None for: {track.title} by {track.artist}")
elif hasattr(track, 'title'):
# Fallback: This is a PlexTrackInfo object, need to find the actual track
# Fallback: This is a TrackInfo object, need to find the actual track
plex_track = self._find_track(track.title, track.artist, track.album)
if plex_track:
plex_tracks.append(plex_track)
@ -448,7 +395,7 @@ class PlexClient:
logger.error(f"Error copying playlist '{source_name}' to '{target_name}': {e}")
return False
def update_playlist(self, playlist_name: str, tracks: List[PlexTrackInfo]) -> bool:
def update_playlist(self, playlist_name: str, tracks: List[TrackInfo]) -> bool:
if not self.ensure_connection():
return False
@ -519,7 +466,7 @@ class PlexClient:
logger.error(f"Error searching for track '{title}' by '{artist}': {e}")
return None
def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[PlexTrackInfo]:
def search_tracks(self, title: str, artist: str, limit: int = 15) -> List[TrackInfo]:
"""
Searches for tracks using an efficient, multi-stage "early exit" strategy.
It stops and returns results as soon as candidates are found.
@ -554,7 +501,7 @@ class PlexClient:
# --- Early Exit: If Stage 1 found results, stop here ---
if candidate_tracks:
logger.info(f"Found {len(candidate_tracks)} candidates in Stage 1. Exiting early.")
tracks = [PlexTrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
tracks = [TrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
# Store references to original tracks for playlist creation
for i, track_info in enumerate(tracks):
if i < len(candidate_tracks):
@ -573,7 +520,7 @@ class PlexClient:
# --- Early Exit: If Stage 2 found results, stop here ---
if candidate_tracks:
logger.info(f"Found {len(candidate_tracks)} candidates in Stage 2. Exiting early.")
tracks = [PlexTrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
tracks = [TrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
# Store references to original tracks for playlist creation
for i, track_info in enumerate(tracks):
if i < len(candidate_tracks):
@ -587,7 +534,7 @@ class PlexClient:
# Removed to prevent false positives where tracks with same title
# but different artists are incorrectly matched
tracks = [PlexTrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
tracks = [TrackInfo.from_plex_track(track) for track in candidate_tracks[:limit]]
# Store references to original tracks for playlist creation
for i, track_info in enumerate(tracks):

View file

@ -190,7 +190,10 @@ class SoulSyncArtist:
return self._albums
class SoulSyncClient:
from core.media_server.contract import MediaServerClient
class SoulSyncClient(MediaServerClient):
"""Filesystem-based media server client for standalone SoulSync operation.
Scans the Transfer folder recursively, reads audio file tags, and

View file

@ -19,16 +19,21 @@ class WebScanManager:
- Progress tracking and status reporting
"""
def __init__(self, media_clients, delay_seconds: int = 60):
def __init__(self, media_server_engine, delay_seconds: int = 60):
"""
Initialize the web scan manager.
Args:
media_clients: Dict containing plex_client, jellyfin_client, navidrome_client
media_server_engine: MediaServerEngine that owns the per-server
clients. Replaces the legacy ``media_clients`` dict the
manager now resolves the active server's client through
``self._engine.client(name)`` instead of a hand-keyed
dict that drifted out of sync with the engine's source
of truth.
delay_seconds: Debounce delay in seconds (default 60s)
"""
self.delay = delay_seconds
self.media_clients = media_clients
self._engine = media_server_engine
self._timer = None
self._scan_in_progress = False
self._downloads_during_scan = False
@ -44,29 +49,19 @@ class WebScanManager:
logger.info(f"WebScanManager initialized with {delay_seconds}s debounce delay")
def _get_active_media_client(self):
"""Get the active media client based on config settings"""
"""Get the active media client through the engine."""
try:
from config.settings import config_manager
active_server = config_manager.get_active_media_server()
server_client_map = {
'jellyfin': 'jellyfin_client',
'navidrome': 'navidrome_client',
'plex': 'plex_client',
'soulsync': 'soulsync_library_client',
}
if not self._engine:
logger.error("Web scan manager has no engine reference")
return None, None
# Try to get the configured active server
if active_server in server_client_map:
client_key = server_client_map[active_server]
client = self.media_clients.get(client_key)
if client and hasattr(client, 'is_connected') and client.is_connected():
return client, active_server
else:
logger.warning(f"{active_server.title()} client not connected — scan skipped")
return None, None
logger.error("No active media server configured for scanning")
client = self._engine.client(active_server)
if client and hasattr(client, 'is_connected') and client.is_connected():
return client, active_server
logger.warning(f"{(active_server or 'unknown').title()} client not connected — scan skipped")
return None, None
except Exception as e:

View file

@ -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)

View file

@ -4,9 +4,7 @@ from dataclasses import dataclass
from datetime import datetime
from utils.logging_config import get_logger
from core.spotify_client import SpotifyClient, Playlist as SpotifyPlaylist, Track as SpotifyTrack
from core.plex_client import PlexClient, PlexTrackInfo
from core.jellyfin_client import JellyfinClient
from core.navidrome_client import NavidromeClient
from core.media_server.types import TrackInfo
from core.download_orchestrator import DownloadOrchestrator
from core.matching_engine import MusicMatchingEngine, MatchResult
@ -44,17 +42,35 @@ class SyncProgress:
failed_tracks: int = 0
class PlaylistSyncService:
def __init__(self, spotify_client: SpotifyClient, plex_client: PlexClient, download_orchestrator: DownloadOrchestrator, jellyfin_client: JellyfinClient = None, navidrome_client = None):
def __init__(self, spotify_client: SpotifyClient, download_orchestrator: DownloadOrchestrator, media_server_engine=None):
"""Initialize the sync service.
``media_server_engine`` is the central MediaServerEngine that owns
the per-server clients (Plex / Jellyfin / Navidrome / SoulSync).
Replaces the legacy per-server kwargs (plex_client / jellyfin_client
/ navidrome_client) all media-server access now goes through
``self._engine.client(name)`` so swapping the active server doesn't
need a service rebuild.
``download_orchestrator`` (formerly ``soulseek_client``) is the
DownloadOrchestrator that owns the per-source download clients
rename + retype landed via the download refactor PR.
"""
self.spotify_client = spotify_client
self.plex_client = plex_client
self.jellyfin_client = jellyfin_client
self.navidrome_client = navidrome_client
self._engine = media_server_engine
self.download_orchestrator = download_orchestrator
self.progress_callbacks = {} # Playlist-specific progress callbacks
self.syncing_playlists = set() # Track multiple syncing playlists
self._cancelled = False
self.matching_engine = MusicMatchingEngine()
def _media_client(self, name: str):
"""Resolve a per-server client through the engine, or None when the
engine isn't wired (defensive — every production path passes one)."""
if self._engine is None:
return None
return self._engine.client(name)
def _get_active_media_client(self, profile_id=None):
"""Get the active media client based on config settings.
@ -68,26 +84,27 @@ class PlaylistSyncService:
active_server = config_manager.get_active_media_server()
if active_server == "jellyfin":
if not self.jellyfin_client:
client = self._media_client('jellyfin')
if not client:
logger.error("Jellyfin client not provided to sync service")
return None, "jellyfin"
# Apply per-profile Jellyfin library if set
if profile_id:
self._apply_profile_library(profile_id, 'jellyfin', self.jellyfin_client)
return self.jellyfin_client, "jellyfin"
self._apply_profile_library(profile_id, 'jellyfin', client)
return client, "jellyfin"
elif active_server == "navidrome":
if not self.navidrome_client:
client = self._media_client('navidrome')
if not client:
logger.error("Navidrome client not provided to sync service")
return None, "navidrome"
return self.navidrome_client, "navidrome"
return client, "navidrome"
else: # Default to Plex
# Apply per-profile Plex library if set
if profile_id:
self._apply_profile_library(profile_id, 'plex', self.plex_client)
return self.plex_client, "plex"
client = self._media_client('plex')
if profile_id and client:
self._apply_profile_library(profile_id, 'plex', client)
return client, "plex"
except Exception as e:
logger.error(f"Error determining active media server: {e}")
return self.plex_client, "plex" # Fallback to Plex
return self._media_client('plex'), "plex" # Fallback to Plex
def _apply_profile_library(self, profile_id, server_type, client):
"""Apply per-profile library selection to a media client if configured."""
@ -441,7 +458,7 @@ class PlaylistSyncService:
self.clear_progress_callback(playlist.name)
self._cancelled = False
async def _find_track_in_media_server(self, spotify_track: SpotifyTrack) -> Tuple[Optional[PlexTrackInfo], float]:
async def _find_track_in_media_server(self, spotify_track: SpotifyTrack) -> Tuple[Optional[TrackInfo], float]:
"""Find a track using the same improved database matching as Download Missing Tracks modal"""
try:
# Check active media server connection

View file

@ -35,6 +35,15 @@ class _FakeMediaClient:
return self._connected
class _FakeMediaServerEngine:
"""Stand-in for MediaServerEngine — only the bits SyncDeps needs."""
def __init__(self, plex=None, jellyfin=None, navidrome=None):
self._clients = {'plex': plex, 'jellyfin': jellyfin, 'navidrome': navidrome}
def client(self, name):
return self._clients.get(name)
class _FakeSyncService:
def __init__(self, *, media_client=None, server_type='plex',
sync_result=None, raise_on_sync=None,
@ -44,8 +53,12 @@ class _FakeSyncService:
self._sync_result = sync_result or _FakeSyncResult()
self._raise_on_sync = raise_on_sync
self.spotify_client = object() if spotify_client else None
self.plex_client = object() if plex_client else None
self.jellyfin_client = object() if jellyfin_client else None
# The sync_service exposes the engine so the discovery worker
# can introspect per-server clients via self._engine.client(name).
self._engine = _FakeMediaServerEngine(
plex=object() if plex_client else None,
jellyfin=object() if jellyfin_client else None,
)
self.progress_callback = None
self.progress_playlist_name = None
self.cleared_callbacks = []
@ -130,8 +143,10 @@ def _build_deps(
return ds.SyncDeps(
config_manager=config or _FakeConfig(),
sync_service=sync_service or _FakeSyncService(media_client=_FakeMediaClient()),
plex_client=plex or _FakePlex(),
jellyfin_client=jellyfin or _FakeJellyfin(),
media_server_engine=_FakeMediaServerEngine(
plex=plex or _FakePlex(),
jellyfin=jellyfin or _FakeJellyfin(),
),
automation_engine=automation or _FakeAutomationEngine(),
run_async=run_async or _run_async_sync,
record_sync_history_start=record_sync_history_start or (lambda **kw: None),

View file

View file

@ -0,0 +1,72 @@
"""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.registry 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}"
)
@pytest.mark.parametrize('server_name', ['plex', 'jellyfin', 'navidrome', 'soulsync'])
def test_server_class_explicitly_inherits_contract(server_name):
"""Per Cin's standard from the download refactor: clients must
explicitly inherit ``MediaServerClient`` so the contract conformance
is obvious from reading the class declaration. Structural
typing alone (which would still pass `hasattr` checks) leaves
the contract invisible to anyone reading the code drift in a
future client class wouldn't fail at the contract boundary."""
from core.media_server.contract import MediaServerClient
classes = _import_server_classes()
cls = classes[server_name]
assert issubclass(cls, MediaServerClient), (
f"{cls.__name__} must explicitly inherit MediaServerClient — "
f"structural typing isn't enough"
)

View file

@ -0,0 +1,302 @@
"""Tests for MediaServerEngine cross-server dispatch."""
from __future__ import annotations
from unittest.mock import MagicMock
import pytest
from core.media_server.engine import MediaServerEngine
from core.media_server.registry import MediaServerRegistry, ServerSpec
class _FakeClient:
"""Stand-in client supporting all required + optional methods.
Tests selectively override per-test to assert dispatch behavior."""
def __init__(self, name='fake', connected=True):
self.name = name
self._connected = connected
self._artists = []
self._album_ids = set()
self._search_results = []
self._scan_triggered = False
def is_connected(self):
return self._connected
def ensure_connection(self):
return self._connected
def get_all_artists(self):
return self._artists
def get_all_album_ids(self):
return self._album_ids
def search_tracks(self, title, artist, limit=15):
return self._search_results
def trigger_library_scan(self):
self._scan_triggered = True
return True
def is_library_scanning(self):
return False
def get_library_stats(self):
return {'artists': 0, 'albums': 0, 'tracks': 0}
def get_recently_added_albums(self, max_results=400):
return []
@pytest.fixture
def make_engine():
"""Build an engine with mock clients. Test passes a dict of
name client; engine wires them via a registry + custom
active-server resolver."""
def _make(clients_by_name, active='plex'):
registry = MediaServerRegistry()
for name, client in clients_by_name.items():
registry.register(ServerSpec(
name=name,
factory=lambda c=client: c,
display_name=name.title(),
))
return MediaServerEngine(
registry=registry,
active_server_resolver=lambda: active,
)
return _make
# ---------------------------------------------------------------------------
# Active-server resolution
# ---------------------------------------------------------------------------
def test_active_server_property_reflects_resolver(make_engine):
plex = _FakeClient('plex')
jelly = _FakeClient('jellyfin')
engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='jellyfin')
assert engine.active_server == 'jellyfin'
assert engine.active_client() is jelly
def test_client_lookup_by_name(make_engine):
plex = _FakeClient('plex')
engine = make_engine({'plex': plex}, active='plex')
assert engine.client('plex') is plex
assert engine.client('made_up') is None
# ---------------------------------------------------------------------------
# Required-method dispatch
# ---------------------------------------------------------------------------
def test_is_connected_routes_to_active_client(make_engine):
plex = _FakeClient('plex', connected=True)
jelly = _FakeClient('jellyfin', connected=False)
engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='jellyfin')
assert engine.is_connected() is False # follows jellyfin
engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='plex')
assert engine.is_connected() is True # follows plex
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 False instead of raising."""
registry = MediaServerRegistry()
registry.register(ServerSpec(
name='broken',
factory=lambda: (_ for _ in ()).throw(RuntimeError("init failed")),
display_name='Broken',
))
registry.initialize() # captures the exception
engine = MediaServerEngine(registry=registry, active_server_resolver=lambda: 'broken')
assert engine.is_connected() is False
assert engine.active_client() is None
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"))
engine = make_engine({'plex': plex}, active='plex')
assert engine.is_connected() is False
# ---------------------------------------------------------------------------
# Generic accessors (Cin's standard from the download refactor)
# ---------------------------------------------------------------------------
def test_configured_clients_only_returns_connected_servers(make_engine):
"""Replaces the legacy per-server `if X and X.is_connected(): ...`
chains in web_server.py. Single call returns the dict."""
plex = _FakeClient('plex', connected=True)
jelly = _FakeClient('jellyfin', connected=False)
soulsync = _FakeClient('soulsync', connected=True)
engine = make_engine(
{'plex': plex, 'jellyfin': jelly, 'soulsync': soulsync},
active='plex',
)
result = engine.configured_clients()
assert set(result.keys()) == {'plex', 'soulsync'}
assert result['plex'] is plex
assert result['soulsync'] is soulsync
def test_configured_clients_skips_clients_whose_is_connected_raises(make_engine):
"""Defensive: a single broken is_connected() must not crash the
iteration. Healthy clients still come back."""
healthy = _FakeClient('plex', connected=True)
broken = _FakeClient('jellyfin')
broken.is_connected = MagicMock(side_effect=RuntimeError("boom"))
engine = make_engine({'plex': healthy, 'jellyfin': broken}, active='plex')
result = engine.configured_clients()
assert 'plex' in result
assert 'jellyfin' not in result
def test_reload_config_dispatches_to_named_server(make_engine):
"""Generic dispatch — caller passes server name instead of
reaching for plex_client.reload_config() directly."""
class _ReloadablePlex(_FakeClient):
def __init__(self):
super().__init__('plex')
self.reload_called = False
def reload_config(self):
self.reload_called = True
plex = _ReloadablePlex()
soulsync = _FakeClient('soulsync') # No reload_config method
engine = make_engine({'plex': plex, 'soulsync': soulsync}, active='plex')
assert engine.reload_config('plex') is True
assert plex.reload_called is True
def test_reload_config_skips_clients_without_method(make_engine):
"""Servers that don't expose reload_config are skipped silently
(return True)."""
soulsync = _FakeClient('soulsync')
engine = make_engine({'soulsync': soulsync}, active='soulsync')
assert engine.reload_config('soulsync') is True
def test_reload_config_with_no_args_reloads_every_server(make_engine):
"""When called with no name, hits every registered server that
exposes reload_config."""
class _ReloadableClient(_FakeClient):
def __init__(self, name):
super().__init__(name)
self.reload_called = False
def reload_config(self):
self.reload_called = True
plex = _ReloadableClient('plex')
jelly = _ReloadableClient('jellyfin')
engine = make_engine({'plex': plex, 'jellyfin': jelly}, active='plex')
engine.reload_config()
assert plex.reload_called is True
assert jelly.reload_called is True
# ---------------------------------------------------------------------------
# Singleton factory (matches get_metadata_engine() / get_download_orchestrator())
# ---------------------------------------------------------------------------
def test_get_media_server_engine_returns_set_singleton(make_engine):
"""When set_media_server_engine has been called (web_server.py
does this at boot), get_media_server_engine returns the installed
instance instead of building a fresh one with the default registry."""
from core.media_server.engine import (
get_media_server_engine,
set_media_server_engine,
)
engine = make_engine({'plex': _FakeClient('plex')}, active='plex')
set_media_server_engine(engine)
try:
assert get_media_server_engine() is engine
finally:
set_media_server_engine(None)
# ---------------------------------------------------------------------------
# Empty-engine fallback (web_server.py boot resilience)
# ---------------------------------------------------------------------------
def test_engine_with_empty_clients_dict_is_safe_to_use():
"""web_server.py falls back to ``MediaServerEngine(clients={})`` if
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 accessor instead of raising."""
registry = MediaServerRegistry()
registry.register(ServerSpec(
name='plex', factory=lambda: _FakeClient('plex'), display_name='Plex',
))
engine = MediaServerEngine(
registry=registry,
clients={}, # No pre-built clients passed.
active_server_resolver=lambda: 'plex',
)
# client(name) returns None for every server — engine doesn't crash.
assert engine.client('plex') is None
assert engine.client('jellyfin') is None
# Active-client lookup + is_connected gracefully handle the empty
# case without raising.
assert engine.active_client() is None
assert engine.is_connected() is False
# configured_clients() returns empty dict cleanly.
assert engine.configured_clients() == {}
# ---------------------------------------------------------------------------
# Registry encapsulation (engine reaches via public set_instance,
# not the private _instances dict)
# ---------------------------------------------------------------------------
def test_engine_uses_registry_set_instance_not_private_attr():
"""Engine constructor with ``clients=`` must hand instances off
via the registry's public ``set_instance(name, client)`` method
rather than reaching into ``registry._instances`` directly. Pin
by giving the registry a ``set_instance`` spy and verifying the
engine calls it."""
plex = _FakeClient('plex')
registry = MediaServerRegistry()
registry.register(ServerSpec(
name='plex', factory=lambda: _FakeClient('plex'), display_name='Plex',
))
set_instance_calls = []
original = registry.set_instance
def _spy(name, client):
set_instance_calls.append((name, client))
original(name, client)
registry.set_instance = _spy
MediaServerEngine(
registry=registry,
clients={'plex': plex},
active_server_resolver=lambda: 'plex',
)
assert ('plex', plex) in set_instance_calls

View file

@ -0,0 +1,104 @@
"""Phase A pinning tests for JellyfinClient.
Pin the OBSERVABLE BEHAVIOR the engine will dispatch through after
Phase B/C. Jellyfin's connection-readiness check is stricter than
Plex (requires base_url + api_key + user_id + music_library_id all
present), and get_all_artists has a side effect of pre-populating
caches.
"""
from __future__ import annotations
from unittest.mock import MagicMock, patch
import pytest
from core.jellyfin_client import JellyfinClient
@pytest.fixture
def jellyfin_client():
"""A bare JellyfinClient with no real connection."""
client = JellyfinClient.__new__(JellyfinClient)
client.base_url = None
client.api_key = None
client.user_id = None
client.music_library_id = None
client._connection_attempted = False
client._is_connecting = False
# Initialize the various caches the real __init__ sets up
client._album_cache = {}
client._track_cache = {}
client._artist_cache = {}
client._cache_populated = False
client._metadata_only_mode = False
return client
# ---------------------------------------------------------------------------
# is_connected
# ---------------------------------------------------------------------------
def test_is_connected_returns_false_when_no_credentials(jellyfin_client):
"""Pinning: any of base_url / api_key / user_id / music_library_id
being None False. All four must be set."""
with patch.object(jellyfin_client, 'ensure_connection', return_value=False):
assert jellyfin_client.is_connected() is False
def test_is_connected_returns_true_only_when_all_four_set(jellyfin_client):
"""Pinning: requires base_url AND api_key AND user_id AND
music_library_id. Setting only some still False."""
jellyfin_client._connection_attempted = True
# Only some — still False
jellyfin_client.base_url = 'http://j'
jellyfin_client.api_key = 'k'
jellyfin_client.user_id = 'u'
jellyfin_client.music_library_id = None
assert jellyfin_client.is_connected() is False
# All four — True
jellyfin_client.music_library_id = 'lib'
assert jellyfin_client.is_connected() is True
# ---------------------------------------------------------------------------
# get_all_artists / get_all_album_ids
# ---------------------------------------------------------------------------
def test_get_all_artists_returns_empty_when_not_connected(jellyfin_client):
with patch.object(jellyfin_client, 'ensure_connection', return_value=False):
assert jellyfin_client.get_all_artists() == []
def test_get_all_artists_returns_empty_when_no_music_library_id(jellyfin_client):
"""Pinning: even with ensure_connection True, no music_library_id
selected empty (don't crash)."""
jellyfin_client.music_library_id = None
with patch.object(jellyfin_client, 'ensure_connection', return_value=True):
assert jellyfin_client.get_all_artists() == []
def test_get_all_album_ids_returns_set(jellyfin_client):
"""Pinning: returns a set of string Jellyfin GUID ids. Same shape
semantic as Plex (set of strings) engine extraction depends on
uniform set type."""
jellyfin_client._connection_attempted = True
jellyfin_client.base_url = 'http://j'
jellyfin_client.api_key = 'k'
jellyfin_client.user_id = 'u'
jellyfin_client.music_library_id = 'lib'
fake_response_pages = [
{'Items': [{'Id': 'guid-1'}, {'Id': 'guid-2'}], 'TotalRecordCount': 2},
]
with patch.object(jellyfin_client, 'ensure_connection', return_value=True), \
patch.object(jellyfin_client, '_make_request', side_effect=fake_response_pages):
result = jellyfin_client.get_all_album_ids()
assert isinstance(result, set)
assert result == {'guid-1', 'guid-2'}

View file

@ -0,0 +1,79 @@
"""Phase A pinning tests for NavidromeClient.
Pin the OBSERVABLE BEHAVIOR the engine will dispatch through. Auth
shape uses base_url + username + password (no token like Plex).
Both get_all_artists + get_all_album_ids return empty when the
client isn't connected.
"""
from __future__ import annotations
from unittest.mock import MagicMock, patch
import pytest
from core.navidrome_client import NavidromeClient
@pytest.fixture
def nav_client():
"""A bare NavidromeClient with no real connection."""
client = NavidromeClient.__new__(NavidromeClient)
client.base_url = None
client.username = None
client.password = None
client._connection_attempted = False
client._is_connecting = False
client._artist_cache = {}
client._album_cache = {}
client._track_cache = {}
client._folder_album_ids = None
client._progress_callback = None
client.music_folder_id = None
return client
def test_is_connected_returns_false_without_credentials(nav_client):
with patch.object(nav_client, 'ensure_connection', return_value=False):
assert nav_client.is_connected() is False
def test_is_connected_returns_true_when_all_three_set(nav_client):
"""Pinning: requires base_url AND username AND password.
No token model (vs Plex). Salt is generated per-request."""
nav_client._connection_attempted = True
nav_client.base_url = 'http://nav'
nav_client.username = 'u'
nav_client.password = 'p'
assert nav_client.is_connected() is True
def test_get_all_artists_returns_empty_when_not_connected(nav_client):
with patch.object(nav_client, 'ensure_connection', return_value=False):
assert nav_client.get_all_artists() == []
def test_get_all_album_ids_returns_set(nav_client):
"""Pinning: returns a set of string Navidrome ids. Same shape
semantic as Plex/Jellyfin (set of strings) engine extraction
depends on uniform set type."""
nav_client._connection_attempted = True
nav_client.base_url = 'http://nav'
nav_client.username = 'u'
nav_client.password = 'p'
# Navidrome uses paginated getAlbumList2 — first page returns
# 2 albums, second returns empty (terminates loop).
# _make_request unwraps the subsonic-response envelope and
# returns the body. get_all_album_ids reads response.albumList2.album.
page_responses = [
{'albumList2': {'album': [{'id': 'nav-1'}, {'id': 'nav-2'}]}},
{'albumList2': {'album': []}},
]
with patch.object(nav_client, 'ensure_connection', return_value=True), \
patch.object(nav_client, '_make_request', side_effect=page_responses):
result = nav_client.get_all_album_ids()
assert isinstance(result, set)
assert result == {'nav-1', 'nav-2'}

View file

@ -0,0 +1,113 @@
"""Phase A pinning tests for PlexClient.
Pin the OBSERVABLE BEHAVIOR the engine will dispatch through after
Phase B/C. The web_server.py dispatch sites + DatabaseUpdateWorker
read these methods generically they must keep their current shape
through the engine refactor.
Plex's surface is wider than the contract requires (additional
playlist / metadata writeback methods), but pinning ALL of them
turns into ~30 tests. Focused on the methods the dispatch sites
actually call: is_connected, is_fully_configured, ensure_connection,
get_all_artists, get_all_album_ids, search_tracks, trigger_library_scan,
is_library_scanning, get_library_stats.
"""
from __future__ import annotations
from unittest.mock import MagicMock, patch
import pytest
from core.plex_client import PlexClient
@pytest.fixture
def plex_client():
"""A bare PlexClient with no real connection. Tests that need
a connected state set client.server = MagicMock() directly."""
client = PlexClient.__new__(PlexClient)
client.server = None
client.music_library = None
client._connection_attempted = False
client._is_connecting = False
client._last_connection_check = 0
client._connection_check_interval = 30
return client
# ---------------------------------------------------------------------------
# is_connected
# ---------------------------------------------------------------------------
def test_is_connected_returns_false_when_no_server(plex_client):
"""Pinning: no server object → False. Dashboard status indicators
+ endpoint guards depend on this."""
plex_client.server = None
# Patch ensure_connection to avoid network call
with patch.object(plex_client, 'ensure_connection', return_value=False):
assert plex_client.is_connected() is False
def test_is_connected_returns_true_when_server_present(plex_client):
"""Pinning: a non-None server object → True (even if music_library
is unset). is_fully_configured() is the stricter check."""
plex_client.server = MagicMock()
plex_client._connection_attempted = True
assert plex_client.is_connected() is True
def test_is_fully_configured_requires_server_and_music_library(plex_client):
"""Pinning: is_fully_configured == True only when BOTH server AND
music_library are set. Used by playlist-sync gating."""
plex_client.server = MagicMock()
plex_client.music_library = None
assert plex_client.is_fully_configured() is False
plex_client.music_library = MagicMock()
assert plex_client.is_fully_configured() is True
# ---------------------------------------------------------------------------
# get_all_artists / get_all_album_ids
# ---------------------------------------------------------------------------
def test_get_all_artists_returns_empty_when_not_connected(plex_client):
"""Pinning: ensure_connection returning False → empty list (not
exception). Caller iterates the result, never raising."""
with patch.object(plex_client, 'ensure_connection', return_value=False):
assert plex_client.get_all_artists() == []
def test_get_all_artists_iterates_searchartists_when_connected(plex_client):
"""Pinning: when connected, get_all_artists calls
music_library.searchArtists() and returns the result list."""
fake_artists = [MagicMock(title='A'), MagicMock(title='B')]
plex_client.server = MagicMock()
plex_client.music_library = MagicMock()
plex_client.music_library.searchArtists.return_value = fake_artists
with patch.object(plex_client, 'ensure_connection', return_value=True):
result = plex_client.get_all_artists()
assert result == fake_artists
def test_get_all_album_ids_returns_set_of_string_ratingkeys(plex_client):
"""Pinning: returns a set (not a list) of string ratingKey values.
DatabaseUpdateWorker uses set membership for diff-detection.
NOTE: Plex stores ratingKey as int but the method coerces to str
so the set semantics match other servers (Jellyfin GUIDs, Navidrome
string ids). Engine refactor must preserve."""
fake_albums = [MagicMock(ratingKey=1), MagicMock(ratingKey=2),
MagicMock(ratingKey=3)]
plex_client.server = MagicMock()
plex_client.music_library = MagicMock()
plex_client.music_library.albums.return_value = fake_albums
with patch.object(plex_client, 'ensure_connection', return_value=True):
result = plex_client.get_all_album_ids()
assert isinstance(result, set)
assert result == {'1', '2', '3'}

View file

@ -0,0 +1,90 @@
"""Phase A pinning tests for SoulSyncClient (standalone library mode).
Pin the OBSERVABLE BEHAVIOR the engine will dispatch through.
SoulSync standalone is the structurally-different one no auth /
no API / no library scan. is_connected just checks `transfer_path`
is a directory. Filesystem walk happens via _get_cached_scan().
"""
from __future__ import annotations
import os
from unittest.mock import patch
import pytest
from core.soulsync_client import SoulSyncClient
@pytest.fixture
def ss_client(tmp_path):
"""A bare SoulSyncClient pointed at a temp directory."""
client = SoulSyncClient.__new__(SoulSyncClient)
client._transfer_path = str(tmp_path)
client._progress_callback = None
client._cache = None
client._cache_time = 0
client._cache_ttl = 300
client._last_scan_time = None
return client
# ---------------------------------------------------------------------------
# is_connected / ensure_connection
# ---------------------------------------------------------------------------
def test_is_connected_true_when_transfer_path_exists(ss_client, tmp_path):
"""Pinning: filesystem-based connectivity. is_connected just
checks os.path.isdir(transfer_path)."""
assert ss_client.is_connected() is True # tmp_path exists
def test_is_connected_false_when_transfer_path_missing(ss_client):
"""Pinning: missing transfer_path → False. The standalone case
where the user hasn't configured a folder yet."""
ss_client._transfer_path = '/nonexistent/path/that/does/not/exist'
assert ss_client.is_connected() is False
def test_ensure_connection_reloads_config_then_checks_path(ss_client, tmp_path):
"""Pinning: ensure_connection re-reads config (so the user
changing the transfer_path takes effect without a process
restart) and then checks the path. Returns True iff the path
is a directory."""
with patch.object(ss_client, '_reload_config') as fake_reload:
result = ss_client.ensure_connection()
fake_reload.assert_called_once()
assert result is True
# ---------------------------------------------------------------------------
# get_all_artists / get_all_album_ids
# ---------------------------------------------------------------------------
def test_get_all_artists_uses_cached_scan(ss_client):
"""Pinning: get_all_artists delegates to _get_cached_scan which
enforces the 5-min TTL. Engine extraction must preserve."""
fake_artists = [object(), object(), object()]
with patch.object(ss_client, '_get_cached_scan', return_value=fake_artists):
result = ss_client.get_all_artists()
assert result == fake_artists
def test_get_all_album_ids_returns_set(ss_client):
"""Pinning: returns a set of MD5-hashed string ids. Same uniform
set semantics as Plex/Jellyfin/Navidrome."""
fake_album = type('FakeAlbum', (), {'ratingKey': 'hash-1'})()
fake_album2 = type('FakeAlbum', (), {'ratingKey': 'hash-2'})()
# Real SoulSyncArtist exposes albums() method (not _albums attr).
fake_artist = type('FakeArtist', (), {
'albums': lambda self: [fake_album, fake_album2],
})()
with patch.object(ss_client, '_get_cached_scan', return_value=[fake_artist]):
result = ss_client.get_all_album_ids()
assert isinstance(result, set)
assert result == {'hash-1', 'hash-2'}

View file

@ -570,7 +570,7 @@ IS_SHUTTING_DOWN = False
# Each client is initialized independently so one failure doesn't take down everything.
# Previously, a single exception set ALL clients to None, breaking the entire app.
logger.info("Initializing SoulSync services for Web UI...")
spotify_client = plex_client = jellyfin_client = navidrome_client = soulsync_library_client = download_orchestrator = tidal_client = matching_engine = sync_service = web_scan_manager = None
spotify_client = download_orchestrator = tidal_client = matching_engine = sync_service = web_scan_manager = media_server_engine = None
try:
spotify_client = get_spotify_client()
@ -578,30 +578,63 @@ try:
except Exception as e:
logger.error(f" Spotify client failed to initialize: {e}")
try:
plex_client = PlexClient()
logger.info(" Plex client initialized")
except Exception as e:
logger.error(f" Plex client failed to initialize: {e}")
try:
jellyfin_client = JellyfinClient()
logger.info(" Jellyfin client initialized")
except Exception as e:
logger.error(f" Jellyfin client failed to initialize: {e}")
def _safe_init_media_client(factory, name):
"""Build a media-server client, capturing per-server init failures
so one broken server doesn't take the engine down with it.
try:
navidrome_client = NavidromeClient()
logger.info(" Navidrome client initialized")
except Exception as e:
logger.error(f" Navidrome client failed to initialize: {e}")
Logs the exception class so the boot log distinguishes config errors
(ConnectionError, AuthenticationError) from genuine import / dependency
failures the broad ``Exception`` catch is intentional (any failure
on this code path means the user can't use that server, and we want
the other three to keep working) but the log makes the cause
diagnosable."""
try:
instance = factory()
logger.info(f" {name} client initialized")
return instance
except Exception as exc:
logger.error(
f" {name} client failed to initialize: {type(exc).__name__}: {exc}",
exc_info=True,
)
return None
# Build the MediaServerEngine. The engine OWNS the per-server client
# instances — no separate web_server.py globals (Cin's standard from
# the download refactor: drop redundant access paths). All callers go
# through media_server_engine.client('<name>').
try:
from core.media_server.engine import MediaServerEngine, set_media_server_engine
from core.soulsync_client import SoulSyncClient
soulsync_library_client = SoulSyncClient()
logger.info(" SoulSync library client initialized")
media_server_engine = MediaServerEngine(clients={
'plex': _safe_init_media_client(PlexClient, "Plex"),
'jellyfin': _safe_init_media_client(JellyfinClient, "Jellyfin"),
'navidrome': _safe_init_media_client(NavidromeClient, "Navidrome"),
'soulsync': _safe_init_media_client(SoulSyncClient, "SoulSync library"),
})
# Install as process-wide singleton so callers reaching via
# get_media_server_engine() see the same instance web_server.py
# constructs at boot. Matches the metadata + download engine
# patterns.
set_media_server_engine(media_server_engine)
logger.info(" Media server engine initialized")
except Exception as e:
logger.error(f" SoulSync library client failed to initialize: {e}")
logger.error(f" Media server engine failed to initialize: {e}", exc_info=True)
# Fallback: empty engine so downstream `engine.client('plex')`
# returns None instead of AttributeError'ing on a None engine
# global. Pre-refactor each per-server client global had its own
# try/except so engine failure didn't take down dispatch sites;
# this preserves that resilience. If the fallback ALSO fails
# (e.g. the import itself broke), media_server_engine stays as
# the None initialized at the top of the module.
try:
from core.media_server.engine import MediaServerEngine, set_media_server_engine
media_server_engine = MediaServerEngine(clients={})
set_media_server_engine(media_server_engine)
except Exception as fallback_exc:
logger.error(f" Empty-engine fallback also failed: {fallback_exc}", exc_info=True)
try:
download_orchestrator = DownloadOrchestrator()
@ -626,7 +659,7 @@ except Exception as e:
logger.error(f" Matching engine failed to initialize: {e}")
try:
sync_service = PlaylistSyncService(spotify_client, plex_client, download_orchestrator, jellyfin_client, navidrome_client)
sync_service = PlaylistSyncService(spotify_client, download_orchestrator, media_server_engine=media_server_engine)
logger.info(" Playlist sync service initialized")
except Exception as e:
logger.error(f" Playlist sync service failed to initialize: {e}")
@ -645,13 +678,7 @@ if download_orchestrator and hasattr(download_orchestrator, 'registry'):
# Initialize web scan manager for automatic post-download scanning
try:
media_clients = {
'plex_client': plex_client,
'jellyfin_client': jellyfin_client,
'navidrome_client': navidrome_client,
'soulsync_library_client': soulsync_library_client,
}
web_scan_manager = WebScanManager(media_clients, delay_seconds=60)
web_scan_manager = WebScanManager(media_server_engine, delay_seconds=60)
logger.info(" Web scan manager initialized")
except Exception as e:
logger.error(f" Web scan manager failed to initialize: {e}")
@ -3461,23 +3488,14 @@ def get_status():
metadata_status = get_metadata_status_snapshot(spotify_client=spotify_client)
# Test media server - use EXISTING instances (they have internal caching)
# Media server clients already cache connection checks internally
# Test media server — engine reads active_server config + dispatches
# to the right client (with internal connection caching). Engine
# returns False safely if the active client is None / not registered.
if current_time - _status_cache_timestamps['media_server'] > STATUS_CACHE_TTL:
media_server_start = time.time()
media_server_status = False
if active_server == "plex" and plex_client:
# Use existing instance - has 30s internal connection cache
media_server_status = plex_client.is_connected()
elif active_server == "jellyfin" and jellyfin_client:
# Use existing instance - has internal connection caching
media_server_status = jellyfin_client.is_connected()
elif active_server == "navidrome" and navidrome_client:
# Use existing instance
media_server_status = navidrome_client.is_connected()
elif active_server == "soulsync":
# Standalone mode — always connected if Transfer folder exists
media_server_status = soulsync_library_client.is_connected() if soulsync_library_client else False
media_server_status = (
media_server_engine.is_connected() if media_server_engine else False
)
media_server_response_time = (time.time() - media_server_start) * 1000
_status_cache['media_server'] = {
'connected': media_server_status,
@ -4142,12 +4160,12 @@ def handle_settings():
# Reload service clients with new settings (guard against None from partial init)
if spotify_client:
spotify_client.reload_config()
if plex_client:
plex_client.server = None
if jellyfin_client:
jellyfin_client.reload_config()
if navidrome_client:
navidrome_client.reload_config()
if media_server_engine.client('plex'):
media_server_engine.client('plex').server = None
if media_server_engine.client('jellyfin'):
media_server_engine.client('jellyfin').reload_config()
if media_server_engine.client('navidrome'):
media_server_engine.client('navidrome').reload_config()
# Reload orchestrator settings (download source mode, hybrid_primary, etc.)
if download_orchestrator:
download_orchestrator.reload_settings()
@ -5111,8 +5129,8 @@ def clear_plex_library_preference():
from database.music_database import MusicDatabase
db = MusicDatabase()
db.set_preference('plex_music_library', '')
if plex_client:
plex_client.music_library = None
if media_server_engine.client('plex'):
media_server_engine.client('plex').music_library = None
return jsonify({"success": True, "message": "Plex library preference cleared."})
except Exception as e:
logger.error(f"Error clearing Plex library preference: {e}")
@ -5123,7 +5141,7 @@ def clear_plex_library_preference():
def get_plex_music_libraries():
"""Get list of all available music libraries from Plex"""
try:
libraries = plex_client.get_available_music_libraries()
libraries = media_server_engine.client('plex').get_available_music_libraries()
# Get currently selected library
from database.music_database import MusicDatabase
@ -5132,8 +5150,8 @@ def get_plex_music_libraries():
# Get the currently active library name
current_library = None
if plex_client.music_library:
current_library = plex_client.music_library.title
if media_server_engine.client('plex').music_library:
current_library = media_server_engine.client('plex').music_library.title
return jsonify({
"success": True,
@ -5155,7 +5173,7 @@ def select_plex_music_library():
if not library_name:
return jsonify({"success": False, "error": "No library name provided"}), 400
success = plex_client.set_music_library_by_name(library_name)
success = media_server_engine.client('plex').set_music_library_by_name(library_name)
if success:
add_activity_item("", "Library Selected", f"Plex music library set to: {library_name}", "Now")
@ -5171,7 +5189,7 @@ def select_plex_music_library():
def get_jellyfin_users():
"""Get list of Jellyfin users that have music libraries"""
try:
users = jellyfin_client.get_available_users()
users = media_server_engine.client('jellyfin').get_available_users()
# Get currently selected user
from database.music_database import MusicDatabase
@ -5180,9 +5198,9 @@ def get_jellyfin_users():
# Determine the current user name from user_id
current_user = None
if jellyfin_client.user_id:
if media_server_engine.client('jellyfin').user_id:
for u in users:
if u['id'] == jellyfin_client.user_id:
if u['id'] == media_server_engine.client('jellyfin').user_id:
current_user = u['name']
break
@ -5206,7 +5224,7 @@ def select_jellyfin_user():
if not username:
return jsonify({"success": False, "error": "No username provided"}), 400
success = jellyfin_client.set_user_by_name(username)
success = media_server_engine.client('jellyfin').set_user_by_name(username)
if success:
add_activity_item("", "User Selected", f"Jellyfin user set to: {username}", "Now")
@ -5222,7 +5240,7 @@ def select_jellyfin_user():
def get_jellyfin_music_libraries():
"""Get list of all available music libraries from Jellyfin"""
try:
libraries = jellyfin_client.get_available_music_libraries()
libraries = media_server_engine.client('jellyfin').get_available_music_libraries()
# Get currently selected library
from database.music_database import MusicDatabase
@ -5231,10 +5249,10 @@ def get_jellyfin_music_libraries():
# Get the currently active library name (match Plex behavior)
current_library = None
if jellyfin_client.music_library_id:
if media_server_engine.client('jellyfin').music_library_id:
# Look up library name from ID
for lib in libraries:
if lib['key'] == jellyfin_client.music_library_id:
if lib['key'] == media_server_engine.client('jellyfin').music_library_id:
current_library = lib['title']
break
@ -5258,7 +5276,7 @@ def select_jellyfin_music_library():
if not library_name:
return jsonify({"success": False, "error": "No library name provided"}), 400
success = jellyfin_client.set_music_library_by_name(library_name)
success = media_server_engine.client('jellyfin').set_music_library_by_name(library_name)
if success:
add_activity_item("", "Library Selected", f"Jellyfin music library set to: {library_name}", "Now")
@ -5274,19 +5292,19 @@ def select_jellyfin_music_library():
def get_navidrome_music_folders():
"""Get list of available music folders from Navidrome"""
try:
if not navidrome_client:
if not media_server_engine.client('navidrome'):
return jsonify({"success": False, "error": "Navidrome client not configured"}), 400
folders = navidrome_client.get_music_folders()
folders = media_server_engine.client('navidrome').get_music_folders()
from database.music_database import MusicDatabase
db = MusicDatabase()
selected_folder = db.get_preference('navidrome_music_folder')
current_folder = None
if navidrome_client.music_folder_id:
if media_server_engine.client('navidrome').music_folder_id:
for f in folders:
if f['key'] == navidrome_client.music_folder_id:
if f['key'] == media_server_engine.client('navidrome').music_folder_id:
current_folder = f['title']
break
@ -5307,7 +5325,7 @@ def select_navidrome_music_folder():
data = request.get_json()
folder_name = data.get('folder_name', '')
success = navidrome_client.set_music_folder_by_name(folder_name)
success = media_server_engine.client('navidrome').set_music_folder_by_name(folder_name)
if success:
if folder_name:
@ -6868,7 +6886,7 @@ def enhanced_search_library_check():
data = request.get_json() or {}
result = _search_library_check.check_library_presence(
database=get_database(),
plex_client=plex_client,
plex_client=media_server_engine.client('plex') if media_server_engine else None,
config_manager=config_manager,
profile_id=get_current_profile_id(),
albums=data.get('albums', []),
@ -8035,9 +8053,9 @@ def test_automation_workflow():
results['media_clients'] = {'active_server': active_server}
for client_name, client in [
('plex', plex_client),
('jellyfin', jellyfin_client),
('navidrome', navidrome_client)
('plex', media_server_engine.client('plex')),
('jellyfin', media_server_engine.client('jellyfin')),
('navidrome', media_server_engine.client('navidrome'))
]:
try:
is_connected = client.is_connected() if client else False
@ -9295,15 +9313,9 @@ def get_artist_enhanced_detail(artist_id):
if album.get('thumb_url'):
album['thumb_url'] = fix_artist_image_url(album['thumb_url'])
# Include server type for sync option
# Include server type for sync option — engine routes to active client.
active_server = config_manager.get_active_media_server()
server_connected = False
if active_server == 'plex':
server_connected = plex_client.is_connected()
elif active_server == 'jellyfin':
server_connected = jellyfin_client.is_connected()
elif active_server == 'navidrome':
server_connected = navidrome_client.is_connected()
server_connected = media_server_engine.is_connected() if media_server_engine else False
result['server_type'] = active_server if server_connected else None
return jsonify(result)
@ -9551,13 +9563,7 @@ def get_track_tag_preview(track_id):
# Include server type so frontend can offer server sync option
active_server = config_manager.get_active_media_server()
server_connected = False
if active_server == 'plex':
server_connected = plex_client.is_connected()
elif active_server == 'jellyfin':
server_connected = jellyfin_client.is_connected()
elif active_server == 'navidrome':
server_connected = navidrome_client.is_connected()
server_connected = media_server_engine.is_connected() if media_server_engine else False
return jsonify({
"success": True,
@ -9658,15 +9664,9 @@ def get_batch_tag_preview():
results.append(entry)
# Server type info
# Server type info — engine routes to active client.
active_server = config_manager.get_active_media_server()
server_connected = False
if active_server == 'plex':
server_connected = plex_client.is_connected()
elif active_server == 'jellyfin':
server_connected = jellyfin_client.is_connected()
elif active_server == 'navidrome':
server_connected = navidrome_client.is_connected()
server_connected = media_server_engine.is_connected() if media_server_engine else False
return jsonify({
"success": True,
@ -10817,7 +10817,7 @@ def _sync_tracks_to_server(track_rows, server_type):
if track_data.get('year'):
metadata['year'] = track_data['year']
if metadata:
success = plex_client.update_track_metadata(str(track_data['id']), metadata)
success = media_server_engine.client('plex').update_track_metadata(str(track_data['id']), metadata)
if success:
result['synced'] += 1
else:
@ -10832,7 +10832,7 @@ def _sync_tracks_to_server(track_rows, server_type):
elif server_type == 'jellyfin':
# Jellyfin: just trigger a library scan once after all file writes
try:
success = jellyfin_client.trigger_library_scan()
success = media_server_engine.client('jellyfin').trigger_library_scan()
if success:
result['synced'] = len(track_rows)
else:
@ -10860,8 +10860,8 @@ def _resolve_library_file_path(file_path):
# Also check the media server's music library path (handles Docker↔host path mismatch)
library_dirs = set()
try:
if plex_client and plex_client.server and plex_client.music_library:
for loc in plex_client.music_library.locations:
if media_server_engine.client('plex') and media_server_engine.client('plex').server and media_server_engine.client('plex').music_library:
for loc in media_server_engine.client('plex').music_library.locations:
library_dirs.add(loc)
except Exception:
pass
@ -11533,9 +11533,9 @@ def redownload_search_metadata(track_id):
if thumb_url and not thumb_url.startswith('http'):
_ab = ''
_at = ''
if plex_client and plex_client.server:
_ab = getattr(plex_client.server, '_baseurl', '') or ''
_at = getattr(plex_client.server, '_token', '') or ''
if media_server_engine.client('plex') and media_server_engine.client('plex').server:
_ab = getattr(media_server_engine.client('plex').server, '_baseurl', '') or ''
_at = getattr(media_server_engine.client('plex').server, '_token', '') or ''
if not _ab:
_pc = config_manager.get_plex_config()
_ab = (_pc.get('base_url', '') or '').rstrip('/')
@ -11826,12 +11826,12 @@ def sync_artist_library(artist_id):
if server_source:
media_client = None
if server_source == 'plex' and plex_client and plex_client.server:
media_client = plex_client
elif server_source == 'jellyfin' and jellyfin_client:
media_client = jellyfin_client
elif server_source == 'navidrome' and navidrome_client:
media_client = navidrome_client
if server_source == 'plex' and media_server_engine.client('plex') and media_server_engine.client('plex').server:
media_client = media_server_engine.client('plex')
elif server_source == 'jellyfin' and media_server_engine.client('jellyfin'):
media_client = media_server_engine.client('jellyfin')
elif server_source == 'navidrome' and media_server_engine.client('navidrome'):
media_client = media_server_engine.client('navidrome')
if media_client:
try:
@ -15114,11 +15114,11 @@ def _run_db_update_task(full_refresh, server_type):
media_client = None
if server_type == "plex":
media_client = plex_client
media_client = media_server_engine.client('plex')
elif server_type == "jellyfin":
media_client = jellyfin_client
media_client = media_server_engine.client('jellyfin')
elif server_type == "navidrome":
media_client = navidrome_client
media_client = media_server_engine.client('navidrome')
if not media_client:
_db_update_error_callback(f"Media client for '{server_type}' not available.")
@ -15160,11 +15160,11 @@ def _run_deep_scan_task(server_type):
media_client = None
if server_type == "plex":
media_client = plex_client
media_client = media_server_engine.client('plex')
elif server_type == "jellyfin":
media_client = jellyfin_client
media_client = media_server_engine.client('jellyfin')
elif server_type == "navidrome":
media_client = navidrome_client
media_client = media_server_engine.client('navidrome')
elif server_type == "soulsync":
# SoulSync standalone deep scan: find untracked files → move to Staging,
# remove stale DB records where files no longer exist on disk
@ -18037,10 +18037,10 @@ def get_server_playlists():
return jsonify({"success": False, "error": "No media server configured"}), 400
playlists_data = []
if active_server == 'plex' and plex_client and plex_client.is_connected():
if active_server == 'plex' and media_server_engine.client('plex') and media_server_engine.client('plex').is_connected():
# Use raw Plex API to get playlist metadata without fetching all tracks
try:
raw_playlists = plex_client.server.playlists()
raw_playlists = media_server_engine.client('plex').server.playlists()
logger.info(f"[ServerPlaylists] Plex returned {len(raw_playlists)} total playlists")
for playlist in raw_playlists:
if getattr(playlist, 'playlistType', None) == 'audio':
@ -18053,22 +18053,22 @@ def get_server_playlists():
except Exception as e:
logger.error(f"[ServerPlaylists] Error fetching Plex playlists: {e}", exc_info=True)
return jsonify({"success": False, "error": f"Plex error: {str(e)}"}), 500
elif active_server == 'jellyfin' and jellyfin_client and jellyfin_client.is_connected():
for pl in jellyfin_client.get_all_playlists():
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin') and media_server_engine.client('jellyfin').is_connected():
for pl in media_server_engine.client('jellyfin').get_all_playlists():
playlists_data.append({
'id': pl.id,
'name': pl.title,
'track_count': pl.leaf_count,
})
elif active_server == 'navidrome' and navidrome_client and navidrome_client.is_connected():
for pl in navidrome_client.get_all_playlists():
elif active_server == 'navidrome' and media_server_engine.client('navidrome') and media_server_engine.client('navidrome').is_connected():
for pl in media_server_engine.client('navidrome').get_all_playlists():
playlists_data.append({
'id': pl.id,
'name': pl.title,
'track_count': pl.leaf_count,
})
else:
logger.warning(f"[ServerPlaylists] Server '{active_server}' not connected. plex_client={plex_client is not None}, jellyfin_client={jellyfin_client is not None}, navidrome_client={navidrome_client is not None}")
logger.warning(f"[ServerPlaylists] Server '{active_server}' not connected. plex_client={media_server_engine.client('plex') is not None}, jellyfin_client={media_server_engine.client('jellyfin') is not None}, navidrome_client={media_server_engine.client('navidrome') is not None}")
return jsonify({"success": False, "error": f"{active_server} not connected"}), 400
return jsonify({"success": True, "server_type": active_server, "playlists": playlists_data})
@ -18086,24 +18086,30 @@ def get_server_playlist_tracks(playlist_id):
# Get tracks from server
server_tracks = []
if active_server == 'plex' and plex_client:
if active_server == 'plex' and media_server_engine.client('plex'):
try:
# Try by ID first, fall back to name lookup (ID changes when playlist is recreated)
raw_playlist = None
try:
raw_playlist = plex_client.server.fetchItem(int(playlist_id))
raw_playlist = media_server_engine.client('plex').server.fetchItem(int(playlist_id))
except Exception:
pass
if not raw_playlist and playlist_name:
try:
raw_playlist = plex_client.server.playlist(playlist_name)
raw_playlist = media_server_engine.client('plex').server.playlist(playlist_name)
except Exception:
pass
if not raw_playlist:
logger.warning(
f"[ServerPlaylistTracks] Plex playlist not found by "
f"id={playlist_id} or name='{playlist_name}'"
f"compare view will show every source track as Find & Add"
)
if raw_playlist:
if not playlist_name:
playlist_name = raw_playlist.title
plex_base = getattr(plex_client.server, '_baseurl', '') or ''
plex_token = getattr(plex_client.server, '_token', '') or ''
plex_base = getattr(media_server_engine.client('plex').server, '_baseurl', '') or ''
plex_token = getattr(media_server_engine.client('plex').server, '_token', '') or ''
if not plex_base:
# Fallback: get from config
_pc = config_manager.get_plex_config()
@ -18128,9 +18134,9 @@ def get_server_playlist_tracks(playlist_id):
})
except Exception as e:
logger.error(f"[ServerPlaylistTracks] Plex error: {e}", exc_info=True)
elif active_server == 'jellyfin' and jellyfin_client:
tracks = jellyfin_client.get_playlist_tracks(playlist_id)
jf_base = jellyfin_client.base_url or ''
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
tracks = media_server_engine.client('jellyfin').get_playlist_tracks(playlist_id)
jf_base = media_server_engine.client('jellyfin').base_url or ''
for t in (tracks or []):
raw = t._data if hasattr(t, '_data') else {}
artists = raw.get('Artists', [])
@ -18145,8 +18151,8 @@ def get_server_playlist_tracks(playlist_id):
'duration': t.duration,
'thumb': thumb,
})
elif active_server == 'navidrome' and navidrome_client:
tracks = navidrome_client.get_playlist_tracks(playlist_id)
elif active_server == 'navidrome' and media_server_engine.client('navidrome'):
tracks = media_server_engine.client('navidrome').get_playlist_tracks(playlist_id)
for t in (tracks or []):
raw = t._data if hasattr(t, '_data') else {}
# Navidrome cover art via Subsonic API
@ -18171,9 +18177,9 @@ def get_server_playlist_tracks(playlist_id):
# Build server art URL prefix for resolving relative thumb paths
_art_prefix = ''
_art_suffix = ''
if active_server == 'plex' and plex_client and plex_client.server:
_ab = getattr(plex_client.server, '_baseurl', '') or ''
_at = getattr(plex_client.server, '_token', '') or ''
if active_server == 'plex' and media_server_engine.client('plex') and media_server_engine.client('plex').server:
_ab = getattr(media_server_engine.client('plex').server, '_baseurl', '') or ''
_at = getattr(media_server_engine.client('plex').server, '_token', '') or ''
if not _ab:
_pc = config_manager.get_plex_config()
_ab = (_pc.get('base_url', '') or '').rstrip('/')
@ -18356,11 +18362,22 @@ def server_playlist_replace_track(playlist_id):
active_server = config_manager.get_active_media_server()
if active_server == 'plex' and plex_client:
# Use raw Plex API - fetch playlist directly
if active_server == 'plex' and media_server_engine.client('plex'):
# ID-first, name-fallback (Plex deletes + recreates on edit
# so the cached rating key can be stale).
plex_server = media_server_engine.client('plex').server
raw_playlist = None
try:
raw_playlist = plex_client.server.playlist(playlist_name)
raw_playlist = plex_server.fetchItem(int(playlist_id))
except Exception:
pass
if not raw_playlist and playlist_name:
try:
raw_playlist = plex_server.playlist(playlist_name)
except Exception:
pass
if not raw_playlist:
logger.warning(f"[ServerPlaylist] replace-track: playlist not found by id={playlist_id} or name='{playlist_name}'")
return jsonify({"success": False, "error": "Playlist not found on server"}), 404
# Build new track list with replacement
@ -18368,7 +18385,7 @@ def server_playlist_replace_track(playlist_id):
replaced = False
for item in raw_playlist.items():
if str(item.ratingKey) == str(old_track_id) and not replaced:
new_item = plex_client.server.fetchItem(int(new_track_id))
new_item = media_server_engine.client('plex').server.fetchItem(int(new_track_id))
if new_item:
new_tracks.append(new_item)
replaced = True
@ -18381,13 +18398,13 @@ def server_playlist_replace_track(playlist_id):
# Delete old and recreate directly (avoid update_playlist's backup logic)
raw_playlist.delete()
from plexapi.playlist import Playlist
new_pl = Playlist.create(plex_client.server, playlist_name, items=new_tracks)
new_pl = Playlist.create(media_server_engine.client('plex').server, playlist_name, items=new_tracks)
return jsonify({"success": True, "message": "Track replaced", "new_playlist_id": str(new_pl.ratingKey)})
else:
return jsonify({"success": False, "error": "Old track not found in playlist"}), 404
elif active_server == 'jellyfin' and jellyfin_client:
current_tracks = jellyfin_client.get_playlist_tracks(playlist_id)
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
current_tracks = media_server_engine.client('jellyfin').get_playlist_tracks(playlist_id)
new_track_ids = []
replaced = False
for t in (current_tracks or []):
@ -18400,12 +18417,12 @@ def server_playlist_replace_track(playlist_id):
if replaced:
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in new_track_ids]
jellyfin_client.update_playlist(playlist_name, new_track_objs)
media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs)
return jsonify({"success": True, "message": "Track replaced"})
return jsonify({"success": False, "error": "Old track not found"}), 404
elif active_server == 'navidrome' and navidrome_client:
current_tracks = navidrome_client.get_playlist_tracks(playlist_id)
elif active_server == 'navidrome' and media_server_engine.client('navidrome'):
current_tracks = media_server_engine.client('navidrome').get_playlist_tracks(playlist_id)
new_track_ids = []
replaced = False
for t in (current_tracks or []):
@ -18418,7 +18435,7 @@ def server_playlist_replace_track(playlist_id):
if replaced:
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in new_track_ids]
navidrome_client.create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
media_server_engine.client('navidrome').create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
return jsonify({"success": True, "message": "Track replaced"})
return jsonify({"success": False, "error": "Old track not found"}), 404
@ -18444,12 +18461,26 @@ def server_playlist_add_track(playlist_id):
active_server = config_manager.get_active_media_server()
if active_server == 'plex' and plex_client:
if active_server == 'plex' and media_server_engine.client('plex'):
# ID-first, name-fallback — Plex deletes + recreates playlists
# on edit so the rating key the frontend cached can be stale.
# The GET tracks endpoint uses the same lookup chain.
plex_server = media_server_engine.client('plex').server
raw_playlist = None
try:
raw_playlist = plex_client.server.playlist(playlist_name)
raw_playlist = plex_server.fetchItem(int(playlist_id))
except Exception:
pass
if not raw_playlist and playlist_name:
try:
raw_playlist = plex_server.playlist(playlist_name)
except Exception:
pass
if not raw_playlist:
logger.warning(f"[ServerPlaylist] add-track: playlist not found by id={playlist_id} or name='{playlist_name}'")
return jsonify({"success": False, "error": "Playlist not found"}), 404
new_item = plex_client.server.fetchItem(int(track_id))
new_item = plex_server.fetchItem(int(track_id))
if not new_item:
return jsonify({"success": False, "error": "Track not found on server"}), 404
@ -18476,22 +18507,22 @@ def server_playlist_add_track(playlist_id):
logger.info(f"[ServerPlaylist] Added track to playlist, playlist ID: {new_id}")
return jsonify({"success": True, "message": "Track added", "new_playlist_id": new_id})
elif active_server == 'jellyfin' and jellyfin_client:
current_tracks = jellyfin_client.get_playlist_tracks(playlist_id) or []
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
current_tracks = media_server_engine.client('jellyfin').get_playlist_tracks(playlist_id) or []
track_ids = [str(t.ratingKey) for t in current_tracks]
pos = max(0, min(int(position), len(track_ids))) if position is not None else len(track_ids)
track_ids.insert(pos, track_id)
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in track_ids]
jellyfin_client.update_playlist(playlist_name, new_track_objs)
media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs)
return jsonify({"success": True, "message": "Track added"})
elif active_server == 'navidrome' and navidrome_client:
current_tracks = navidrome_client.get_playlist_tracks(playlist_id) or []
elif active_server == 'navidrome' and media_server_engine.client('navidrome'):
current_tracks = media_server_engine.client('navidrome').get_playlist_tracks(playlist_id) or []
track_ids = [str(t.ratingKey) for t in current_tracks]
pos = max(0, min(int(position), len(track_ids))) if position is not None else len(track_ids)
track_ids.insert(pos, track_id)
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in track_ids]
navidrome_client.create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
media_server_engine.client('navidrome').create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
return jsonify({"success": True, "message": "Track added"})
return jsonify({"success": False, "error": f"Unsupported server: {active_server}"}), 400
@ -18515,10 +18546,22 @@ def server_playlist_remove_track(playlist_id):
active_server = config_manager.get_active_media_server()
if active_server == 'plex' and plex_client:
if active_server == 'plex' and media_server_engine.client('plex'):
# ID-first, name-fallback (Plex deletes + recreates on edit
# so the cached rating key can be stale).
plex_server = media_server_engine.client('plex').server
raw_playlist = None
try:
raw_playlist = plex_client.server.playlist(playlist_name)
raw_playlist = plex_server.fetchItem(int(playlist_id))
except Exception:
pass
if not raw_playlist and playlist_name:
try:
raw_playlist = plex_server.playlist(playlist_name)
except Exception:
pass
if not raw_playlist:
logger.warning(f"[ServerPlaylist] remove-track: playlist not found by id={playlist_id} or name='{playlist_name}'")
return jsonify({"success": False, "error": "Playlist not found"}), 404
# Rebuild without the target track
@ -18529,26 +18572,26 @@ def server_playlist_remove_track(playlist_id):
raw_playlist.delete()
if new_items:
from plexapi.playlist import Playlist
new_pl = Playlist.create(plex_client.server, playlist_name, items=new_items)
new_pl = Playlist.create(media_server_engine.client('plex').server, playlist_name, items=new_items)
return jsonify({"success": True, "message": "Track removed", "new_playlist_id": str(new_pl.ratingKey)})
return jsonify({"success": True, "message": "Track removed (playlist now empty)"})
elif active_server == 'jellyfin' and jellyfin_client:
current_tracks = jellyfin_client.get_playlist_tracks(playlist_id) or []
elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'):
current_tracks = media_server_engine.client('jellyfin').get_playlist_tracks(playlist_id) or []
new_ids = [str(t.ratingKey) for t in current_tracks if str(t.ratingKey) != str(remove_track_id)]
if len(new_ids) == len(current_tracks):
return jsonify({"success": False, "error": "Track not found in playlist"}), 404
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in new_ids]
jellyfin_client.update_playlist(playlist_name, new_track_objs)
media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs)
return jsonify({"success": True, "message": "Track removed"})
elif active_server == 'navidrome' and navidrome_client:
current_tracks = navidrome_client.get_playlist_tracks(playlist_id) or []
elif active_server == 'navidrome' and media_server_engine.client('navidrome'):
current_tracks = media_server_engine.client('navidrome').get_playlist_tracks(playlist_id) or []
new_ids = [str(t.ratingKey) for t in current_tracks if str(t.ratingKey) != str(remove_track_id)]
if len(new_ids) == len(current_tracks):
return jsonify({"success": False, "error": "Track not found in playlist"}), 404
new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in new_ids]
navidrome_client.create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
media_server_engine.client('navidrome').create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id)
return jsonify({"success": True, "message": "Track removed"})
return jsonify({"success": False, "error": f"Unsupported server: {active_server}"}), 400
@ -18572,9 +18615,9 @@ def library_search_tracks():
# Build thumb URL resolver for this server
_art_prefix = ''
_art_suffix = ''
if active_server == 'plex' and plex_client and plex_client.server:
_ab = getattr(plex_client.server, '_baseurl', '') or ''
_at = getattr(plex_client.server, '_token', '') or ''
if active_server == 'plex' and media_server_engine.client('plex') and media_server_engine.client('plex').server:
_ab = getattr(media_server_engine.client('plex').server, '_baseurl', '') or ''
_at = getattr(media_server_engine.client('plex').server, '_token', '') or ''
if not _ab:
_pc = config_manager.get_plex_config()
_ab = (_pc.get('base_url', '') or '').rstrip('/')
@ -23251,8 +23294,7 @@ def _build_sync_deps():
return _discovery_sync.SyncDeps(
config_manager=config_manager,
sync_service=sync_service,
plex_client=plex_client,
jellyfin_client=jellyfin_client,
media_server_engine=media_server_engine,
automation_engine=automation_engine,
run_async=run_async,
record_sync_history_start=_record_sync_history_start,
@ -23384,12 +23426,12 @@ def test_database_access():
# Test media clients
logger.info(" Media clients status:")
logger.info(f" plex_client: {plex_client is not None}")
if plex_client:
logger.info(f" plex_client.is_connected(): {plex_client.is_connected()}")
logger.info(f" jellyfin_client: {jellyfin_client is not None}")
if jellyfin_client:
logger.info(f" jellyfin_client.is_connected(): {jellyfin_client.is_connected()}")
logger.info(f" media_server_engine.client('plex'): {media_server_engine.client('plex') is not None}")
if media_server_engine.client('plex'):
logger.info(f" media_server_engine.client('plex').is_connected(): {media_server_engine.client('plex').is_connected()}")
logger.info(f" media_server_engine.client('jellyfin'): {media_server_engine.client('jellyfin') is not None}")
if media_server_engine.client('jellyfin'):
logger.info(f" media_server_engine.client('jellyfin').is_connected(): {media_server_engine.client('jellyfin').is_connected()}")
return jsonify({
"success": True,
@ -23398,8 +23440,8 @@ def test_database_access():
"database_initialized": db is not None,
"database_stats": stats,
"active_server": active_server,
"plex_connected": plex_client.is_connected() if plex_client else False,
"jellyfin_connected": jellyfin_client.is_connected() if jellyfin_client else False,
"plex_connected": media_server_engine.client('plex').is_connected() if media_server_engine.client('plex') else False,
"jellyfin_connected": media_server_engine.client('jellyfin').is_connected() if media_server_engine.client('jellyfin') else False,
}
})
@ -29118,17 +29160,17 @@ def start_metadata_update():
# Get appropriate media client - Support all three servers
if active_server == "jellyfin":
media_client = jellyfin_client
media_client = media_server_engine.client('jellyfin')
if not media_client:
add_activity_item("", "Metadata Update", "Jellyfin client not available", "Now")
return jsonify({"success": False, "error": "Jellyfin client not available"}), 400
elif active_server == "navidrome":
media_client = navidrome_client
media_client = media_server_engine.client('navidrome')
if not media_client:
add_activity_item("", "Metadata Update", "Navidrome client not available", "Now")
return jsonify({"success": False, "error": "Navidrome client not available"}), 400
else: # plex
media_client = plex_client
media_client = media_server_engine.client('plex')
if not media_client:
add_activity_item("", "Metadata Update", "Plex client not available", "Now")
return jsonify({"success": False, "error": "Plex client not available"}), 400
@ -32833,9 +32875,7 @@ try:
listening_stats_worker = ListeningStatsWorker(
database=listening_stats_db,
config_manager=config_manager,
plex_client=plex_client,
jellyfin_client=jellyfin_client,
navidrome_client=navidrome_client,
media_server_engine=media_server_engine,
)
listening_stats_worker.start()
logger.info("Listening stats worker initialized and started")

View file

@ -3432,6 +3432,8 @@ const WHATS_NEW = {
'2.4.2': [
// --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.2 dev cycle' },
{ title: 'Internal: Media Server Engine Cin/JohnBaumb Pass', desc: 'internal — applied the same architectural cleanups the download engine PR went through to the media server engine PR before review. (1) every server client (Plex / Jellyfin / Navidrome / SoulSync) now explicitly inherits `MediaServerClient` instead of relying on structural typing — drift in any class fails at the conformance test boundary. (2) generic accessors on the engine: `configured_clients()` (replaces per-server `if X and X.is_connected(): clients[name] = X` chains in web_server.py) and `reload_config(name=None)` (generic dispatch instead of per-client reload calls). (3) singleton factory: `get_media_server_engine()` / `set_media_server_engine()` matching the metadata + download engine shape. web_server.py boots via `set_media_server_engine(...)` so factory + global handle share state. (4) ~70 direct `plex_client.X` / `jellyfin_client.X` / `navidrome_client.X` / `soulsync_library_client.X` attribute reaches in web_server.py migrated to `media_server_engine.client(\'<name>\').X`. ~60 standalone refs (truthy checks, media_client assignments, source-name tuples) also routed through the engine. (5) the per-server `plex_client` / `jellyfin_client` / `navidrome_client` / `soulsync_library_client` globals in web_server.py are gone entirely — engine owns the client instances now, every caller reaches via `media_server_engine.client(\'<name>\')`. four multi-client consumers (`PlaylistSyncService`, `ListeningStatsWorker`, `WebScanManager`, discovery `SyncDeps`) refactored to take the engine instead of separate per-server kwargs. (6) `TrackInfo` and `PlaylistInfo` lifted out of `core/plex_client.py` / `jellyfin_client.py` / `navidrome_client.py` (each was defining a near-identical copy) into the neutral `core/media_server/types.py` module — same lift Cin caught on the download `TrackResult`/`AlbumResult`/`DownloadStatus` situation. consumers (matching engine, sync service) get one import. zero behavior change.' },
{ title: 'Internal: Media Server Engine Foundation', desc: 'internal — companion to the download engine refactor. introduces a media-server engine + plugin contract on top of the four server clients (plex / jellyfin / navidrome / soulsync standalone). pre-refactor web_server.py held four separate per-server client globals that every dispatch site reached individually. new `core/media_server/` package provides `MediaServerEngine` that owns the per-server clients + a small set of generic accessors (`client(name)`, `active_client()`, `configured_clients()`, `reload_config(name)`) so call sites use one canonical lookup pattern. plugin contract requires the four methods every server actually implements (is_connected, ensure_connection, get_all_artists, get_all_album_ids) — methods that exist on most-but-not-all servers (search_tracks, trigger_library_scan, get_library_stats, etc.) are listed in `KNOWN_PER_SERVER_METHODS` for discoverability and reached directly via `engine.client(name).<method>` since there\'s no uniform safe-default that fits every method. honest scope: the four uniform-shape `is_connected` dispatches were lifted into `engine.is_connected()` (the one cross-server wrapper kept on the engine); the ~18 server-specific chains that do genuinely different per-server work (playlist track replace, metadata sync, scan strategies) stay explicit at the call site per the "lift what\'s truly shared" standard, but reach the per-server client through the engine. 42 tests pin: per-server observable behavior (4 server pinning files, 20 tests), engine surface + accessor contracts (15 tests), structural conformance + explicit-inheritance (9 tests). zero behavior change for users.' },
{ title: 'Drop SoundCloud Preview Snippets at Search Time', desc: 'soundcloud serves a ~30s preview clip for tracks that are gated behind go+ / login (very common for major-label uploads — official content basically doesn\'t exist on soundcloud, so what shows up is bootlegs, fan uploads, type beats, and these 30s previews). yt-dlp accepts the preview as the download payload, the post-download integrity check catches the duration mismatch and quarantines the file, but the user just sees "all candidates failed" with no explanation. previews also showed up in the candidate-review modal where clicking one bypassed validation and downloaded the same broken file. now `filter_soundcloud_previews` drops these candidates at every entry point — auto-search scoring, modal-cache fallback, AND the not-found raw-results path — so previews never reach the matcher OR the user. drops candidates < 35s or below half the expected duration, gated on expected being non-trivially long (>60s) so genuine short tracks still pass. also fixed a silent regression in the hybrid-fallback path where the per-source attribute removal left `getattr(orch, \'youtube\', None)` returning None for every source — fallback never fired. now resolves through the orchestrator\'s `client(name)` accessor.', page: 'downloads' },
{ title: 'Internal: Move Shared Download Dataclasses + Singleton Boot Path', desc: 'internal — two architectural cleanups on top of the download engine refactor. (1) `TrackResult`, `AlbumResult`, `DownloadStatus`, `SearchResult` lived in `core/soulseek_client.py` for historical reasons (they grew up there as the soulseek-only types and got exported when other download sources were added). every plugin imported these from the soulseek module just to satisfy the contract — coupling 8 clients to a sibling source for type imports only. moved them to `core/download_plugins/types.py` (the neutral plugin package) and updated all 14 import sites across deezer/hifi/lidarr/qobuz/soundcloud/tidal/youtube clients + the engine + matching engine + redownload + tests. clean break, no backward-compat re-export. (2) `web_server.py` now boots the orchestrator via `set_download_orchestrator(DownloadOrchestrator())` so the singleton factory + boot path share state — `get_download_orchestrator()` returns the same instance the global handle points at instead of lazily building a separate one. matches cin\'s `get_metadata_engine()` pattern.' },
{ title: 'Internal: Rename `soulseek_client` Global → `download_orchestrator`', desc: 'internal — followup cleanup. the global handle in web_server.py was named `soulseek_client` for historical reasons (the orchestrator was originally just the soulseek client and grew downstream sources around it), but the type has long been `DownloadOrchestrator` not `SoulseekClient`. renamed the global + every parameter/attribute that carried the legacy name across web_server.py, api/, core/downloads/*, core/search/*, core/streaming/*, services/sync_service.py, and the test fixtures (`MasterDeps.soulseek_client` → `download_orchestrator`, `init(soulseek_client_obj)` → `init(download_orchestrator_obj)`, etc). module path `core.soulseek_client` and class `SoulseekClient` (the actual soulseek-only client) are unchanged — only the orchestrator handle renamed. ~250 references touched, suite green.' },