Skip to content

Commit 9b61665

Browse files
committed
mediaserver incosisntency fix
1 parent 6c0147e commit 9b61665

3 files changed

Lines changed: 104 additions & 9 deletions

File tree

tasks/mediaserver/jellyfin.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -478,14 +478,7 @@ def get_playlist_by_name(playlist_name):
478478

479479

480480
def create_playlist(base_name, item_ids):
481-
url = f"{_jellyfin_base_url()}/Playlists"
482-
body = {"Name": base_name, "Ids": item_ids, "UserId": _jellyfin_user_id()}
483-
try:
484-
r = requests.post(url, headers=_jellyfin_headers_from_creds(), json=body, timeout=REQUESTS_TIMEOUT)
485-
if r.ok:
486-
logger.info("Created Jellyfin playlist '%s'", base_name)
487-
except Exception:
488-
logger.exception("Exception creating Jellyfin playlist '%s'", base_name)
481+
return _create_fresh_playlist(base_name, item_ids)
489482

490483

491484
def get_all_playlists():

tasks/mediaserver/plex.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,9 +560,10 @@ def _create_playlist_batched(title, item_ids, user_creds=None):
560560

561561
def create_playlist(base_name, item_ids):
562562
try:
563-
_create_playlist_batched(base_name, list(item_ids))
563+
return _create_playlist_batched(base_name, list(item_ids))
564564
except Exception:
565565
logger.exception("Exception creating Plex playlist '%s'", base_name)
566+
return None
566567

567568

568569
def create_instant_playlist(playlist_name, item_ids, user_creds=None):

test/unit/test_mediaserver.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,12 @@
1717
* Best-artist selection, field normalization and playlist/album/track parsing
1818
* getAllSongs pagination, list-libraries shape, and create-or-replace flows
1919
* Dispatcher validation and automatic-playlist deletion routing
20+
* Every provider's create_playlist returns the created playlist on success and
21+
None on failure, so a caller may test the result instead of assuming success
2022
"""
2123

24+
import importlib
25+
2226
import pytest
2327
from unittest.mock import Mock, MagicMock, patch
2428
import requests
@@ -2642,6 +2646,103 @@ def test_new_playlist_overflow_failure_returns_none(
26422646
assert jellyfin._create_fresh_playlist('SF', item_ids) is None
26432647

26442648

2649+
_DELEGATING_CREATORS = [
2650+
('jellyfin', '_create_fresh_playlist'),
2651+
('lyrion', '_create_playlist_batched'),
2652+
('navidrome', '_create_playlist_batched'),
2653+
('plex', '_create_playlist_batched'),
2654+
]
2655+
2656+
2657+
class TestCreatePlaylistReturnContract:
2658+
@pytest.mark.parametrize('provider,creator', _DELEGATING_CREATORS)
2659+
def test_returns_the_created_playlist_rather_than_none(self, provider, creator):
2660+
module = importlib.import_module(f'tasks.mediaserver.{provider}')
2661+
created = {'Id': 'p-1', 'Name': 'Mix'}
2662+
2663+
with patch.object(module, creator, return_value=created):
2664+
assert module.create_playlist('Mix', ['t1']) == created
2665+
2666+
@pytest.mark.parametrize('provider,creator', _DELEGATING_CREATORS)
2667+
def test_returns_none_when_the_creator_reports_failure(self, provider, creator):
2668+
module = importlib.import_module(f'tasks.mediaserver.{provider}')
2669+
2670+
with patch.object(module, creator, return_value=None):
2671+
assert module.create_playlist('Mix', ['t1']) is None
2672+
2673+
2674+
class TestJellyfinCreatePlaylist:
2675+
@patch('tasks.mediaserver.jellyfin._add_items_to_playlist', return_value=True)
2676+
@patch('tasks.mediaserver.jellyfin.requests')
2677+
@patch('tasks.mediaserver.jellyfin.config')
2678+
def test_overflow_tracks_are_added_instead_of_truncated(
2679+
self, mock_config, mock_requests, mock_add
2680+
):
2681+
from tasks.mediaserver import jellyfin
2682+
2683+
mock_config.JELLYFIN_URL = 'http://jf'
2684+
mock_config.JELLYFIN_USER_ID = 'admin-user'
2685+
mock_config.HEADERS = {'Authorization': 'MediaBrowser Token="t"'}
2686+
post_resp = MagicMock()
2687+
post_resp.json.return_value = {'Id': 'new-jf', 'Name': 'Mix'}
2688+
mock_requests.post.return_value = post_resp
2689+
item_ids = [f'song-{i}' for i in range(jellyfin.JELLYFIN_PLAYLIST_BATCH_SIZE + 5)]
2690+
2691+
result = jellyfin.create_playlist('Mix', item_ids)
2692+
2693+
assert result['Id'] == 'new-jf'
2694+
posted = mock_requests.post.call_args[1]['json']['Ids']
2695+
assert len(posted) == jellyfin.JELLYFIN_PLAYLIST_BATCH_SIZE
2696+
assert mock_add.call_args[0][1] == item_ids[jellyfin.JELLYFIN_PLAYLIST_BATCH_SIZE:]
2697+
2698+
@patch('tasks.mediaserver.jellyfin.requests')
2699+
@patch('tasks.mediaserver.jellyfin.config')
2700+
def test_server_rejection_returns_none_instead_of_silent_success(
2701+
self, mock_config, mock_requests
2702+
):
2703+
from tasks.mediaserver import jellyfin
2704+
2705+
mock_config.JELLYFIN_URL = 'http://jf'
2706+
mock_config.JELLYFIN_USER_ID = 'admin-user'
2707+
mock_config.HEADERS = {'Authorization': 'MediaBrowser Token="t"'}
2708+
post_resp = MagicMock()
2709+
post_resp.raise_for_status.side_effect = requests.exceptions.HTTPError('403')
2710+
mock_requests.post.return_value = post_resp
2711+
2712+
assert jellyfin.create_playlist('Mix', ['t1']) is None
2713+
2714+
2715+
class TestEmbyCreatePlaylistReturnContract:
2716+
@patch('tasks.mediaserver.emby.requests')
2717+
@patch('tasks.mediaserver.emby.config')
2718+
def test_returns_the_created_playlist_rather_than_none(self, mock_config, mock_requests):
2719+
from tasks.mediaserver import emby
2720+
2721+
mock_config.EMBY_URL = 'http://emby'
2722+
mock_config.EMBY_USER_ID = 'user123'
2723+
mock_config.EMBY_TOKEN = 'tok'
2724+
mock_requests.utils.quote.side_effect = lambda value: value
2725+
mock_requests.post.return_value.json.return_value = {'Id': 'new-emby', 'Name': 'Mix'}
2726+
2727+
assert emby.create_playlist('Mix', ['t1'])['Id'] == 'new-emby'
2728+
2729+
@patch('tasks.mediaserver.emby.requests')
2730+
@patch('tasks.mediaserver.emby.config')
2731+
def test_server_rejection_returns_none(self, mock_config, mock_requests):
2732+
from tasks.mediaserver import emby
2733+
2734+
mock_config.EMBY_URL = 'http://emby'
2735+
mock_config.EMBY_USER_ID = 'user123'
2736+
mock_config.EMBY_TOKEN = 'tok'
2737+
mock_requests.utils.quote.side_effect = lambda value: value
2738+
mock_requests.exceptions = requests.exceptions
2739+
mock_requests.post.return_value.raise_for_status.side_effect = (
2740+
requests.exceptions.HTTPError('403')
2741+
)
2742+
2743+
assert emby.create_playlist('Mix', ['t1']) is None
2744+
2745+
26452746
class TestEmbyCreateOrReplacePlaylist:
26462747
@patch('tasks.mediaserver.emby.requests')
26472748
@patch('tasks.mediaserver.emby.get_playlist_by_name')

0 commit comments

Comments
 (0)