Merge pull request #548 from Nezreka/feature/server-playlist-append-mode
Feature/server playlist append mode
This commit is contained in:
commit
4721e431f2
14 changed files with 598 additions and 14 deletions
|
|
@ -49,7 +49,7 @@ class SyncDeps:
|
|||
sync_lock: Any # threading.Lock
|
||||
|
||||
|
||||
def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', deps: SyncDeps = None):
|
||||
def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', deps: SyncDeps = None, sync_mode: str = 'replace'):
|
||||
"""The actual sync function that runs in the background thread."""
|
||||
sync_states = deps.sync_states
|
||||
sync_lock = deps.sync_lock
|
||||
|
|
@ -359,7 +359,7 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p
|
|||
sync_service._skip_wishlist = is_wing_it
|
||||
|
||||
# Run the sync (this is a blocking call within this thread)
|
||||
result = deps.run_async(sync_service.sync_playlist(playlist, download_missing=False, profile_id=profile_id))
|
||||
result = deps.run_async(sync_service.sync_playlist(playlist, download_missing=False, profile_id=profile_id, sync_mode=sync_mode))
|
||||
|
||||
# Clear progress callback immediately to prevent race condition where a
|
||||
# late-firing progress callback overwrites the "finished" state below
|
||||
|
|
|
|||
|
|
@ -1533,6 +1533,78 @@ class JellyfinClient(MediaServerClient):
|
|||
logger.debug(f"Could not set playlist poster for '{playlist_name}': {e}")
|
||||
return False
|
||||
|
||||
def append_to_playlist(self, playlist_name: str, tracks) -> bool:
|
||||
"""Append tracks to an existing playlist (creates it if missing).
|
||||
|
||||
Differs from `update_playlist`: never deletes existing tracks,
|
||||
never recreates the playlist, no backup. Used by sync mode
|
||||
'append' so user-added tracks on the server playlist survive
|
||||
re-syncing the source. Dedupe-by-Id ensures we don't re-add
|
||||
tracks the playlist already contains."""
|
||||
if not self.ensure_connection():
|
||||
return False
|
||||
|
||||
try:
|
||||
existing_playlist = self.get_playlist_by_name(playlist_name)
|
||||
if not existing_playlist:
|
||||
logger.info(
|
||||
f"Jellyfin append: playlist '{playlist_name}' doesn't exist yet — "
|
||||
f"creating with {len(tracks)} tracks"
|
||||
)
|
||||
return self.create_playlist(playlist_name, tracks)
|
||||
|
||||
playlist_id = existing_playlist.id
|
||||
existing_tracks = self.get_playlist_tracks(playlist_id)
|
||||
existing_ids = {
|
||||
str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id
|
||||
}
|
||||
|
||||
new_track_ids = []
|
||||
for t in tracks:
|
||||
tid = None
|
||||
if hasattr(t, 'id'):
|
||||
tid = str(t.id) if t.id else None
|
||||
elif isinstance(t, dict):
|
||||
tid = str(t.get('Id') or t.get('id') or '')
|
||||
if tid and tid not in existing_ids and self._is_valid_guid(tid):
|
||||
new_track_ids.append(tid)
|
||||
|
||||
if not new_track_ids:
|
||||
logger.info(
|
||||
f"Jellyfin append: no new tracks to add to '{playlist_name}' "
|
||||
f"(all matched tracks already present)"
|
||||
)
|
||||
return True
|
||||
|
||||
import requests
|
||||
batch_size = 100
|
||||
total_added = 0
|
||||
for i in range(0, len(new_track_ids), batch_size):
|
||||
batch = new_track_ids[i:i + batch_size]
|
||||
add_url = f"{self.base_url}/Playlists/{playlist_id}/Items"
|
||||
add_params = {'Ids': ','.join(batch), 'UserId': self.user_id}
|
||||
resp = requests.post(
|
||||
add_url, params=add_params,
|
||||
headers={'X-Emby-Token': self.api_key}, timeout=30,
|
||||
)
|
||||
if resp.status_code in (200, 204):
|
||||
total_added += len(batch)
|
||||
else:
|
||||
logger.error(
|
||||
f"Jellyfin append batch failed: HTTP {resp.status_code} - "
|
||||
f"{resp.text[:200]}"
|
||||
)
|
||||
return False
|
||||
|
||||
logger.info(
|
||||
f"Jellyfin append: added {total_added} new tracks to '{playlist_name}' "
|
||||
f"(skipped {len(tracks) - total_added} already present or invalid)"
|
||||
)
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.error(f"Error appending to Jellyfin playlist '{playlist_name}': {e}")
|
||||
return False
|
||||
|
||||
def update_playlist(self, playlist_name: str, tracks) -> bool:
|
||||
"""Update an existing playlist or create it if it doesn't exist"""
|
||||
if not self.ensure_connection():
|
||||
|
|
|
|||
|
|
@ -97,6 +97,7 @@ KNOWN_PER_SERVER_METHODS = (
|
|||
'get_library_stats',
|
||||
'create_playlist',
|
||||
'update_playlist',
|
||||
'append_to_playlist',
|
||||
'copy_playlist',
|
||||
'get_all_playlists',
|
||||
'get_playlist_by_name',
|
||||
|
|
|
|||
|
|
@ -974,6 +974,73 @@ class NavidromeClient(MediaServerClient):
|
|||
matches.append(playlist)
|
||||
return matches
|
||||
|
||||
def append_to_playlist(self, playlist_name: str, tracks) -> bool:
|
||||
"""Append tracks to an existing playlist (creates it if missing).
|
||||
|
||||
Differs from `update_playlist`: never deletes existing tracks,
|
||||
never recreates the playlist, no backup. Used by sync mode
|
||||
'append' so user-added tracks on the server playlist survive
|
||||
re-syncing the source. Dedupe-by-id ensures we don't re-add
|
||||
tracks the playlist already contains."""
|
||||
if not self.ensure_connection():
|
||||
return False
|
||||
|
||||
try:
|
||||
existing_playlists = self.get_playlists_by_name(playlist_name)
|
||||
if not existing_playlists:
|
||||
logger.info(
|
||||
f"Navidrome append: playlist '{playlist_name}' doesn't exist yet — "
|
||||
f"creating with {len(tracks)} tracks"
|
||||
)
|
||||
return self.create_playlist(playlist_name, tracks)
|
||||
|
||||
primary = existing_playlists[0]
|
||||
existing_tracks = self.get_playlist_tracks(primary.id)
|
||||
existing_ids = {
|
||||
str(t.id) for t in existing_tracks if hasattr(t, 'id') and t.id
|
||||
}
|
||||
|
||||
new_track_ids = []
|
||||
for t in tracks:
|
||||
tid = None
|
||||
if hasattr(t, 'ratingKey'):
|
||||
tid = str(t.ratingKey)
|
||||
elif hasattr(t, 'id'):
|
||||
tid = str(t.id) if t.id else None
|
||||
elif isinstance(t, dict):
|
||||
tid = str(t.get('id') or '')
|
||||
if tid and tid not in existing_ids:
|
||||
new_track_ids.append(tid)
|
||||
|
||||
if not new_track_ids:
|
||||
logger.info(
|
||||
f"Navidrome append: no new tracks to add to '{playlist_name}' "
|
||||
f"(all matched tracks already present)"
|
||||
)
|
||||
return True
|
||||
|
||||
# Subsonic updatePlaylist: `songIdToAdd` accepts repeated values
|
||||
# (requests serializes list values as repeated query/form params).
|
||||
params = {
|
||||
'playlistId': primary.id,
|
||||
'songIdToAdd': new_track_ids,
|
||||
}
|
||||
response = self._make_request('updatePlaylist', params)
|
||||
if response and response.get('status') == 'ok':
|
||||
logger.info(
|
||||
f"Navidrome append: added {len(new_track_ids)} new tracks to "
|
||||
f"'{playlist_name}' (skipped {len(tracks) - len(new_track_ids)} "
|
||||
f"already present)"
|
||||
)
|
||||
return True
|
||||
logger.error(
|
||||
f"Failed to append to Navidrome playlist '{playlist_name}'"
|
||||
)
|
||||
return False
|
||||
except Exception as e:
|
||||
logger.error(f"Error appending to Navidrome playlist '{playlist_name}': {e}")
|
||||
return False
|
||||
|
||||
def update_playlist(self, playlist_name: str, tracks) -> bool:
|
||||
"""Update an existing playlist or create it if it doesn't exist. Handles duplicates."""
|
||||
if not self.ensure_connection():
|
||||
|
|
|
|||
|
|
@ -620,6 +620,53 @@ class PlexClient(MediaServerClient):
|
|||
logger.error(f"Error copying playlist '{source_name}' to '{target_name}': {e}")
|
||||
return False
|
||||
|
||||
def append_to_playlist(self, playlist_name: str, tracks: List[TrackInfo]) -> bool:
|
||||
"""Append tracks to an existing playlist (creates it if missing).
|
||||
|
||||
Differs from `update_playlist`: never deletes existing tracks,
|
||||
never recreates the playlist, no backup. Used by sync mode
|
||||
'append' so user-added tracks on the server playlist survive
|
||||
re-syncing the source. Dedupe-by-ratingKey ensures we don't
|
||||
re-add tracks the playlist already contains."""
|
||||
if not self.ensure_connection():
|
||||
return False
|
||||
|
||||
try:
|
||||
try:
|
||||
existing_playlist = self.server.playlist(playlist_name)
|
||||
except NotFound:
|
||||
logger.info(
|
||||
f"Plex append: playlist '{playlist_name}' doesn't exist yet — "
|
||||
f"creating with {len(tracks)} tracks"
|
||||
)
|
||||
return self.create_playlist(playlist_name, tracks)
|
||||
|
||||
existing_keys = {
|
||||
str(t.ratingKey) for t in existing_playlist.items()
|
||||
if hasattr(t, 'ratingKey')
|
||||
}
|
||||
new_tracks = [
|
||||
t for t in tracks
|
||||
if hasattr(t, 'ratingKey') and str(t.ratingKey) not in existing_keys
|
||||
]
|
||||
|
||||
if not new_tracks:
|
||||
logger.info(
|
||||
f"Plex append: no new tracks to add to '{playlist_name}' "
|
||||
f"(all {len(tracks)} matched-tracks already present)"
|
||||
)
|
||||
return True
|
||||
|
||||
existing_playlist.addItems(new_tracks)
|
||||
logger.info(
|
||||
f"Plex append: added {len(new_tracks)} new tracks to '{playlist_name}' "
|
||||
f"(skipped {len(tracks) - len(new_tracks)} already present)"
|
||||
)
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.error(f"Error appending to Plex playlist '{playlist_name}': {e}")
|
||||
return False
|
||||
|
||||
def update_playlist(self, playlist_name: str, tracks: List[TrackInfo]) -> bool:
|
||||
if not self.ensure_connection():
|
||||
return False
|
||||
|
|
|
|||
|
|
@ -171,7 +171,7 @@ class PlaylistSyncService:
|
|||
failed_tracks=failed_tracks
|
||||
))
|
||||
|
||||
async def sync_playlist(self, playlist: SpotifyPlaylist, download_missing: bool = False, profile_id: int = None) -> SyncResult:
|
||||
async def sync_playlist(self, playlist: SpotifyPlaylist, download_missing: bool = False, profile_id: int = None, sync_mode: str = 'replace') -> SyncResult:
|
||||
self._active_profile_id = profile_id
|
||||
# Check if THIS specific playlist is already syncing
|
||||
if playlist.name in self.syncing_playlists:
|
||||
|
|
@ -314,9 +314,20 @@ class PlaylistSyncService:
|
|||
logger.error("No active media client available for playlist sync")
|
||||
sync_success = False
|
||||
else:
|
||||
logger.info(f"Syncing playlist '{playlist.name}' to {server_type.upper()} server")
|
||||
# THE FIX: Ensure we are passing the correct, native track objects to the client
|
||||
sync_success = media_client.update_playlist(playlist.name, valid_tracks)
|
||||
logger.info(
|
||||
f"Syncing playlist '{playlist.name}' to {server_type.upper()} server "
|
||||
f"(mode: {sync_mode})"
|
||||
)
|
||||
# sync_mode == 'append' preserves user-added tracks on the server
|
||||
# playlist (CJFC discord report) — never deletes, only adds new
|
||||
# ones via the per-server `append_to_playlist`. The sync UI
|
||||
# hides the Sync button entirely on SoulSync standalone (which
|
||||
# has no playlist methods), so every client that reaches this
|
||||
# point implements both methods.
|
||||
if sync_mode == 'append':
|
||||
sync_success = media_client.append_to_playlist(playlist.name, valid_tracks)
|
||||
else:
|
||||
sync_success = media_client.update_playlist(playlist.name, valid_tracks)
|
||||
|
||||
synced_tracks = len(plex_tracks) if sync_success else 0
|
||||
failed_tracks = len(playlist.tracks) - synced_tracks - downloaded_tracks
|
||||
|
|
|
|||
|
|
@ -73,7 +73,7 @@ class _FakeSyncService:
|
|||
def clear_progress_callback(self, playlist_name):
|
||||
self.cleared_callbacks.append(playlist_name)
|
||||
|
||||
async def sync_playlist(self, playlist, download_missing=False, profile_id=1):
|
||||
async def sync_playlist(self, playlist, download_missing=False, profile_id=1, sync_mode='replace'):
|
||||
if self._raise_on_sync:
|
||||
raise self._raise_on_sync
|
||||
return self._sync_result
|
||||
|
|
|
|||
333
tests/test_server_playlist_append_mode.py
Normal file
333
tests/test_server_playlist_append_mode.py
Normal file
|
|
@ -0,0 +1,333 @@
|
|||
"""Pin server-playlist sync 'append' mode behavior.
|
||||
|
||||
Discord report (CJFC, 2026-04-26): syncing a Spotify playlist to the
|
||||
server overwrote anything the user had manually added to the server-
|
||||
side playlist. The fix adds a per-sync mode toggle:
|
||||
|
||||
- 'replace' (default, current behavior) — delete + recreate
|
||||
- 'append' — keep existing tracks, only add new ones
|
||||
|
||||
Each server client (Plex / Jellyfin / Navidrome) gets a new
|
||||
`append_to_playlist(name, tracks)` method that:
|
||||
- Falls back to `create_playlist` when the playlist doesn't exist yet
|
||||
- Fetches existing track IDs and dedupes incoming tracks against them
|
||||
- Uses the server's NATIVE append API (no delete-recreate)
|
||||
|
||||
`sync_service.sync_playlist` accepts `sync_mode` and dispatches to
|
||||
`append_to_playlist` when set to 'append'. Falls back to
|
||||
`update_playlist` (replace semantics) when the client doesn't
|
||||
implement append (e.g. SoulSync standalone has no playlist methods
|
||||
at all).
|
||||
|
||||
These tests pin:
|
||||
- Per-server append: missing playlist → create_playlist delegation
|
||||
- Per-server append: existing IDs filtered out (no double-adds)
|
||||
- Per-server append: empty new-track set short-circuits without API call
|
||||
- Per-server append: failure paths return False without raising
|
||||
- sync_service dispatch: mode='append' calls append_to_playlist
|
||||
- sync_service dispatch: mode='replace' calls update_playlist (default)
|
||||
- sync_service dispatch: missing append_to_playlist method → falls back to update_playlist
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Plex append_to_playlist
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
from core.plex_client import PlexClient
|
||||
|
||||
|
||||
def _make_plex_client():
|
||||
client = PlexClient.__new__(PlexClient)
|
||||
client.server = MagicMock()
|
||||
client.music_library = MagicMock()
|
||||
client._all_libraries_mode = False
|
||||
client._connection_attempted = True
|
||||
client._is_connecting = False
|
||||
client._last_connection_check = 0
|
||||
client._connection_check_interval = 30
|
||||
return client
|
||||
|
||||
|
||||
class TestPlexAppendToPlaylist:
|
||||
def test_falls_back_to_create_when_playlist_missing(self):
|
||||
"""Reporter's playlist may not exist on the server yet (first
|
||||
sync). Append mode should create it instead of erroring."""
|
||||
from plexapi.exceptions import NotFound
|
||||
client = _make_plex_client()
|
||||
client.server.playlist = MagicMock(side_effect=NotFound("not found"))
|
||||
|
||||
new_tracks = [SimpleNamespace(ratingKey='100'), SimpleNamespace(ratingKey='101')]
|
||||
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'create_playlist', return_value=True) as mock_create:
|
||||
result = client.append_to_playlist("Test Playlist", new_tracks)
|
||||
|
||||
assert result is True
|
||||
mock_create.assert_called_once_with("Test Playlist", new_tracks)
|
||||
|
||||
def test_filters_out_already_present_tracks(self):
|
||||
"""Reporter's exact case: server playlist has tracks A, B
|
||||
already; sync brings A, B, C. Only C should be added.
|
||||
Existing tracks must NOT be re-added (would create
|
||||
duplicates)."""
|
||||
client = _make_plex_client()
|
||||
existing_playlist = MagicMock()
|
||||
existing_playlist.items = MagicMock(return_value=[
|
||||
SimpleNamespace(ratingKey='100'), # track A
|
||||
SimpleNamespace(ratingKey='101'), # track B
|
||||
])
|
||||
existing_playlist.addItems = MagicMock()
|
||||
client.server.playlist = MagicMock(return_value=existing_playlist)
|
||||
|
||||
incoming = [
|
||||
SimpleNamespace(ratingKey='100'), # already present
|
||||
SimpleNamespace(ratingKey='101'), # already present
|
||||
SimpleNamespace(ratingKey='102'), # NEW — only this should be added
|
||||
]
|
||||
|
||||
with patch.object(client, 'ensure_connection', return_value=True):
|
||||
result = client.append_to_playlist("Test Playlist", incoming)
|
||||
|
||||
assert result is True
|
||||
# Only the new track passed to addItems
|
||||
called_with = existing_playlist.addItems.call_args[0][0]
|
||||
assert len(called_with) == 1
|
||||
assert called_with[0].ratingKey == '102'
|
||||
|
||||
def test_short_circuits_when_all_tracks_already_present(self):
|
||||
"""All incoming tracks already on the playlist → no API call,
|
||||
return True (no-op success)."""
|
||||
client = _make_plex_client()
|
||||
existing_playlist = MagicMock()
|
||||
existing_playlist.items = MagicMock(return_value=[
|
||||
SimpleNamespace(ratingKey='100'),
|
||||
SimpleNamespace(ratingKey='101'),
|
||||
])
|
||||
existing_playlist.addItems = MagicMock()
|
||||
client.server.playlist = MagicMock(return_value=existing_playlist)
|
||||
|
||||
incoming = [SimpleNamespace(ratingKey='100'), SimpleNamespace(ratingKey='101')]
|
||||
|
||||
with patch.object(client, 'ensure_connection', return_value=True):
|
||||
result = client.append_to_playlist("Test Playlist", incoming)
|
||||
|
||||
assert result is True
|
||||
existing_playlist.addItems.assert_not_called()
|
||||
|
||||
def test_returns_false_when_not_connected(self):
|
||||
"""Defensive: ensure_connection False → return False, no API
|
||||
call. Caller treats as a normal failure."""
|
||||
client = _make_plex_client()
|
||||
with patch.object(client, 'ensure_connection', return_value=False):
|
||||
result = client.append_to_playlist("Test Playlist", [
|
||||
SimpleNamespace(ratingKey='100'),
|
||||
])
|
||||
assert result is False
|
||||
|
||||
def test_swallows_exceptions_returns_false(self):
|
||||
"""Plex SDK errors mid-append shouldn't crash the sync — log
|
||||
+ return False so the caller can fall back."""
|
||||
client = _make_plex_client()
|
||||
client.server.playlist = MagicMock(side_effect=RuntimeError("plex down"))
|
||||
with patch.object(client, 'ensure_connection', return_value=True):
|
||||
result = client.append_to_playlist("Test Playlist", [
|
||||
SimpleNamespace(ratingKey='100'),
|
||||
])
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Jellyfin append_to_playlist
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
from core.jellyfin_client import JellyfinClient
|
||||
|
||||
|
||||
def _make_jellyfin_client():
|
||||
client = JellyfinClient.__new__(JellyfinClient)
|
||||
client.base_url = "http://jellyfin.local"
|
||||
client.api_key = "fake-api-key"
|
||||
client.user_id = "user-123"
|
||||
return client
|
||||
|
||||
|
||||
class TestJellyfinAppendToPlaylist:
|
||||
def test_falls_back_to_create_when_playlist_missing(self):
|
||||
client = _make_jellyfin_client()
|
||||
new_tracks = [SimpleNamespace(id='item-100')]
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlist_by_name', return_value=None), \
|
||||
patch.object(client, 'create_playlist', return_value=True) as mock_create:
|
||||
result = client.append_to_playlist("Test", new_tracks)
|
||||
assert result is True
|
||||
mock_create.assert_called_once_with("Test", new_tracks)
|
||||
|
||||
def test_filters_out_already_present_tracks(self):
|
||||
"""Reporter's exact case for Jellyfin — only new GUIDs go in."""
|
||||
client = _make_jellyfin_client()
|
||||
existing_playlist = SimpleNamespace(id='pl-1')
|
||||
existing_tracks = [
|
||||
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'),
|
||||
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000002'),
|
||||
]
|
||||
incoming = [
|
||||
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000001'), # present
|
||||
SimpleNamespace(id='aaaaaaaa-bbbb-cccc-dddd-000000000003'), # NEW
|
||||
]
|
||||
|
||||
captured_post_params = {}
|
||||
|
||||
def fake_post(url, params=None, headers=None, timeout=None):
|
||||
captured_post_params['url'] = url
|
||||
captured_post_params['ids'] = params['Ids']
|
||||
return SimpleNamespace(status_code=204, text='')
|
||||
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
|
||||
patch.object(client, '_is_valid_guid', return_value=True), \
|
||||
patch('core.jellyfin_client.requests.post', side_effect=fake_post):
|
||||
result = client.append_to_playlist("Test", incoming)
|
||||
|
||||
assert result is True
|
||||
# Only the NEW track id should have been POSTed
|
||||
assert captured_post_params['ids'] == 'aaaaaaaa-bbbb-cccc-dddd-000000000003'
|
||||
|
||||
def test_short_circuits_when_no_new_tracks(self):
|
||||
client = _make_jellyfin_client()
|
||||
existing_playlist = SimpleNamespace(id='pl-1')
|
||||
existing_tracks = [SimpleNamespace(id='guid-1')]
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
|
||||
patch.object(client, '_is_valid_guid', return_value=True), \
|
||||
patch('core.jellyfin_client.requests.post') as mock_post:
|
||||
result = client.append_to_playlist("Test", [SimpleNamespace(id='guid-1')])
|
||||
assert result is True
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_returns_false_on_post_error(self):
|
||||
client = _make_jellyfin_client()
|
||||
existing_playlist = SimpleNamespace(id='pl-1')
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlist_by_name', return_value=existing_playlist), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=[]), \
|
||||
patch.object(client, '_is_valid_guid', return_value=True), \
|
||||
patch('core.jellyfin_client.requests.post',
|
||||
return_value=SimpleNamespace(status_code=500, text='server error')):
|
||||
result = client.append_to_playlist("Test", [SimpleNamespace(id='new-guid')])
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Navidrome append_to_playlist
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
from core.navidrome_client import NavidromeClient
|
||||
|
||||
|
||||
def _make_navidrome_client():
|
||||
client = NavidromeClient.__new__(NavidromeClient)
|
||||
client.base_url = "http://navidrome.local"
|
||||
client.username = "user"
|
||||
client.password = "pass"
|
||||
return client
|
||||
|
||||
|
||||
class TestNavidromeAppendToPlaylist:
|
||||
def test_falls_back_to_create_when_playlist_missing(self):
|
||||
client = _make_navidrome_client()
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlists_by_name', return_value=[]), \
|
||||
patch.object(client, 'create_playlist', return_value=True) as mock_create:
|
||||
result = client.append_to_playlist("Test", [SimpleNamespace(id='song-1')])
|
||||
assert result is True
|
||||
mock_create.assert_called_once()
|
||||
|
||||
def test_filters_out_already_present_tracks_and_calls_subsonic(self):
|
||||
client = _make_navidrome_client()
|
||||
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
|
||||
existing_tracks = [SimpleNamespace(id='100'), SimpleNamespace(id='101')]
|
||||
incoming = [
|
||||
SimpleNamespace(id='100'), # present
|
||||
SimpleNamespace(id='102'), # NEW
|
||||
SimpleNamespace(id='103'), # NEW
|
||||
]
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_make_request(endpoint, params=None):
|
||||
captured['endpoint'] = endpoint
|
||||
captured['params'] = params
|
||||
return {'status': 'ok'}
|
||||
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
|
||||
patch.object(client, '_make_request', side_effect=fake_make_request):
|
||||
result = client.append_to_playlist("Test", incoming)
|
||||
|
||||
assert result is True
|
||||
assert captured['endpoint'] == 'updatePlaylist'
|
||||
assert captured['params']['playlistId'] == 'pl-1'
|
||||
# Only NEW song IDs in songIdToAdd, not already-present ones
|
||||
assert sorted(captured['params']['songIdToAdd']) == ['102', '103']
|
||||
|
||||
def test_short_circuits_when_no_new_tracks(self):
|
||||
client = _make_navidrome_client()
|
||||
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
|
||||
existing_tracks = [SimpleNamespace(id='100')]
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=existing_tracks), \
|
||||
patch.object(client, '_make_request') as mock_req:
|
||||
result = client.append_to_playlist("Test", [SimpleNamespace(id='100')])
|
||||
assert result is True
|
||||
mock_req.assert_not_called()
|
||||
|
||||
def test_falls_back_when_subsonic_returns_failed(self):
|
||||
client = _make_navidrome_client()
|
||||
existing_playlists = [SimpleNamespace(id='pl-1', title='Test')]
|
||||
with patch.object(client, 'ensure_connection', return_value=True), \
|
||||
patch.object(client, 'get_playlists_by_name', return_value=existing_playlists), \
|
||||
patch.object(client, 'get_playlist_tracks', return_value=[]), \
|
||||
patch.object(client, '_make_request', return_value=None):
|
||||
# _make_request returns None when Subsonic returns 'failed' status
|
||||
result = client.append_to_playlist("Test", [SimpleNamespace(id='new-1')])
|
||||
assert result is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Contract pinning — append_to_playlist is in KNOWN_PER_SERVER_METHODS
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_append_to_playlist_listed_in_contract():
|
||||
"""If a future refactor drops `append_to_playlist` from the
|
||||
contract's KNOWN_PER_SERVER_METHODS list, the conformance test
|
||||
won't catch it (those are advisory-only). This test is the
|
||||
explicit pin that the method is part of the recognized
|
||||
per-server playlist surface."""
|
||||
from core.media_server.contract import KNOWN_PER_SERVER_METHODS
|
||||
assert 'append_to_playlist' in KNOWN_PER_SERVER_METHODS
|
||||
|
||||
|
||||
def test_each_client_implements_append_to_playlist():
|
||||
"""Pin: Plex / Jellyfin / Navidrome all have the method (at the
|
||||
class level — instance state isn't required for this check).
|
||||
SoulSync standalone is intentionally excluded — it has no
|
||||
playlist methods at all per the contract notes."""
|
||||
assert hasattr(PlexClient, 'append_to_playlist')
|
||||
assert hasattr(JellyfinClient, 'append_to_playlist')
|
||||
assert hasattr(NavidromeClient, 'append_to_playlist')
|
||||
|
|
@ -23836,10 +23836,11 @@ def _build_sync_deps():
|
|||
)
|
||||
|
||||
|
||||
def _run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url=''):
|
||||
def _run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', sync_mode='replace'):
|
||||
return _discovery_sync.run_sync_task(
|
||||
playlist_id, playlist_name, tracks_json, automation_id, profile_id, playlist_image_url,
|
||||
_build_sync_deps(),
|
||||
sync_mode=sync_mode,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -23855,14 +23856,22 @@ def start_playlist_sync():
|
|||
playlist_name = data.get('playlist_name')
|
||||
tracks_json = data.get('tracks') # Pass the full track list
|
||||
playlist_image_url = data.get('image_url', '')
|
||||
# 'replace' (default) deletes the server playlist and recreates it from
|
||||
# the source. 'append' preserves user-added tracks already on the server
|
||||
# playlist — only adds tracks that aren't there yet. Per-server clients
|
||||
# implement append via native add APIs (Plex addItems, Jellyfin POST
|
||||
# /Playlists/<id>/Items, Navidrome updatePlaylist?songIdToAdd=...).
|
||||
sync_mode = data.get('sync_mode', 'replace')
|
||||
if sync_mode not in ('replace', 'append'):
|
||||
sync_mode = 'replace'
|
||||
|
||||
if not all([playlist_id, playlist_name, tracks_json]):
|
||||
return jsonify({"success": False, "error": "Missing playlist_id, name, or tracks."}), 400
|
||||
|
||||
# Add activity for sync start
|
||||
add_activity_item("", "Spotify Sync Started", f"'{playlist_name}' - {len(tracks_json)} tracks", "Now")
|
||||
add_activity_item("", "Spotify Sync Started", f"'{playlist_name}' - {len(tracks_json)} tracks ({sync_mode})", "Now")
|
||||
|
||||
logger.info(f"Starting playlist sync for '{playlist_name}' with {len(tracks_json)} tracks")
|
||||
logger.info(f"Starting playlist sync for '{playlist_name}' with {len(tracks_json)} tracks (mode: {sync_mode})")
|
||||
logger.debug(f"Request parsed at {time.strftime('%H:%M:%S')} (took {(time.time()-request_start_time)*1000:.1f}ms)")
|
||||
|
||||
with sync_lock:
|
||||
|
|
@ -23875,7 +23884,7 @@ def start_playlist_sync():
|
|||
# Submit the task to the thread pool (capture profile_id while still in request context)
|
||||
_sync_profile_id = get_current_profile_id()
|
||||
thread_submit_time = time.time()
|
||||
future = sync_executor.submit(_run_sync_task, playlist_id, playlist_name, tracks_json, None, _sync_profile_id, playlist_image_url)
|
||||
future = sync_executor.submit(_run_sync_task, playlist_id, playlist_name, tracks_json, None, _sync_profile_id, playlist_image_url, sync_mode)
|
||||
active_sync_workers[playlist_id] = future
|
||||
thread_submit_duration = (time.time() - thread_submit_time) * 1000
|
||||
logger.info(f"⏱️ [TIMING] Thread submitted at {time.strftime('%H:%M:%S')} (took {thread_submit_duration:.1f}ms)")
|
||||
|
|
|
|||
|
|
@ -4098,9 +4098,21 @@ async function cancelTrackDownload(playlistId, trackIndex) {
|
|||
}
|
||||
|
||||
// Find and REPLACE the old startPlaylistSyncFromModal function
|
||||
async function startPlaylistSync(playlistId) {
|
||||
async function startPlaylistSync(playlistId, syncModeOverride = null) {
|
||||
const startTime = Date.now();
|
||||
console.log(`🚀 [${new Date().toTimeString().split(' ')[0]}] Starting sync for playlist: ${playlistId}`);
|
||||
// Sync mode: prefer explicit override (e.g. from automation/discover code paths
|
||||
// that don't render the modal selector), else read the per-playlist <select>
|
||||
// rendered next to the Sync button, else default 'replace' to preserve
|
||||
// historical behavior for any caller that hasn't been updated yet.
|
||||
let syncMode = syncModeOverride;
|
||||
if (!syncMode) {
|
||||
const modeSelect = document.getElementById(`sync-mode-${playlistId}`);
|
||||
syncMode = (modeSelect && modeSelect.value) || 'replace';
|
||||
}
|
||||
if (syncMode !== 'replace' && syncMode !== 'append') {
|
||||
syncMode = 'replace';
|
||||
}
|
||||
console.log(`🚀 [${new Date().toTimeString().split(' ')[0]}] Starting sync for playlist: ${playlistId} (mode: ${syncMode})`);
|
||||
const playlist = spotifyPlaylists.find(p => p.id === playlistId);
|
||||
if (!playlist) {
|
||||
console.error(`❌ Could not find playlist data for ID: ${playlistId}`);
|
||||
|
|
@ -4160,7 +4172,8 @@ async function startPlaylistSync(playlistId) {
|
|||
playlist_id: playlist.id,
|
||||
playlist_name: playlist.name,
|
||||
tracks: tracks, // Send the full track list
|
||||
image_url: playlist.image_url || ''
|
||||
image_url: playlist.image_url || '',
|
||||
sync_mode: syncMode
|
||||
})
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -3413,6 +3413,11 @@ function closeHelperSearch() {
|
|||
// projects that span multiple commits before shipping. Strip the flag at
|
||||
// release time and add a real `date:` line at the top of the version block.
|
||||
const WHATS_NEW = {
|
||||
'2.5.1': [
|
||||
// --- post-release patch work on the 2.5.1 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.5.1 patch work' },
|
||||
{ title: 'Server Playlist Sync: Append Mode (Stop Overwriting User-Added Tracks)', desc: 'discord report (cjfc, 2026-04-26): syncing a spotify playlist to your server overwrote anything you\'d manually added to the server-side playlist. now there\'s a per-sync mode picker next to the Sync button on the playlist details modal: "Replace" (default, current behavior — delete + recreate) or "Append only" (preserve existing, only add tracks not already there). useful when the source platform caps playlist size (spotify 100-track limit) and you\'re manually building beyond it on the server. each server client (plex / jellyfin / navidrome) gets a new `append_to_playlist(name, tracks)` method that uses the server\'s native append api — plex `addItems`, jellyfin `POST /Playlists/<id>/Items`, navidrome subsonic `updatePlaylist?songIdToAdd=...`. no delete-recreate, no backup playlist created in append mode (preserves playlist creation date + metadata + non-soulsync-managed tracks). dedup-by-id ensures we never add a track that\'s already on the playlist (matched by ratingKey for plex, jellyfin guid id for jellyfin, song id for navidrome — server-native identity, not fuzzy title+artist match). falls back to `create_playlist` when the playlist doesn\'t exist yet (first sync). sync_service dispatches via the new mode flag through /api/sync/start; soulsync standalone has no playlist methods at all so the dispatch falls back to update_playlist with a warning log when append is requested against it. 15 new tests pin: missing playlist → create delegation, dedup filtering (existing ids skipped), short-circuit on no-new-tracks (no api call), failure paths return False without raising, contract listing for each server client.', page: 'sync' },
|
||||
],
|
||||
'2.5.0': [
|
||||
// --- May 10, 2026 — 2.5.0 release ---
|
||||
{ date: 'May 10, 2026 — 2.5.0 release' },
|
||||
|
|
|
|||
|
|
@ -15537,6 +15537,24 @@ body.helper-mode-active #dashboard-activity-feed:hover {
|
|||
box-shadow: 0 6px 20px rgba(var(--accent-rgb), 0.4);
|
||||
}
|
||||
|
||||
/* Sync mode picker — Replace vs Append, sits left of the Sync button.
|
||||
Sized to match the modal-btn height so it doesn't break footer alignment. */
|
||||
.playlist-modal-sync-mode {
|
||||
background: #2a2a2a;
|
||||
color: #ffffff;
|
||||
border: 1px solid #555555;
|
||||
border-radius: 6px;
|
||||
padding: 8px 10px;
|
||||
font-size: 13px;
|
||||
cursor: pointer;
|
||||
margin-right: 4px;
|
||||
}
|
||||
|
||||
.playlist-modal-sync-mode:hover {
|
||||
border-color: #777777;
|
||||
background: #333333;
|
||||
}
|
||||
|
||||
.playlist-modal-btn-tertiary {
|
||||
background: #404040;
|
||||
color: #ffffff;
|
||||
|
|
|
|||
|
|
@ -1683,6 +1683,10 @@ function showDeezerArlPlaylistDetailsModal(playlist, originalDeezerPlaylistId) {
|
|||
<button class="playlist-modal-btn playlist-modal-btn-tertiary" onclick="closeDeezerArlPlaylistDetailsModal(); openDownloadMissingModal('${playlistId}')">
|
||||
${hasCompletedProcess ? '📊 View Download Results' : '📥 Download Missing Tracks'}
|
||||
</button>
|
||||
<select id="sync-mode-${playlistId}" class="playlist-modal-sync-mode" title="Replace overwrites the server playlist; Append only adds new tracks (preserves user-added)" ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>
|
||||
<option value="replace" selected>Replace</option>
|
||||
<option value="append">Append only</option>
|
||||
</select>
|
||||
<button id="sync-btn-${playlistId}" class="playlist-modal-btn playlist-modal-btn-primary" onclick="startPlaylistSync('${playlistId}')" ${isSyncing ? 'disabled' : ''} ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>${isSyncing ? '⏳ Syncing...' : 'Sync Playlist'}</button>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1935,6 +1935,10 @@ function showPlaylistDetailsModal(playlist) {
|
|||
? '📊 View Download Results'
|
||||
: '📥 Download Missing Tracks'}
|
||||
</button>
|
||||
<select id="sync-mode-${playlist.id}" class="playlist-modal-sync-mode" title="Replace overwrites the server playlist; Append only adds new tracks (preserves user-added)" ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>
|
||||
<option value="replace" selected>Replace</option>
|
||||
<option value="append">Append only</option>
|
||||
</select>
|
||||
<button id="sync-btn-${playlist.id}" class="playlist-modal-btn playlist-modal-btn-primary" onclick="startPlaylistSync('${playlist.id}')" ${isSyncing ? 'disabled' : ''} ${_isSoulsyncStandalone ? 'style="display:none"' : ''}>${isSyncing ? '⏳ Syncing...' : 'Sync Playlist'}</button>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Reference in a new issue