From 6883cd0969e40ad380d20b5b338c681767bc73f1 Mon Sep 17 00:00:00 2001 From: Enea D'Angiò Date: Fri, 3 Jul 2026 11:14:11 +0200 Subject: Fix play queue index handling in SyncPlay Three index bugs in PlayQueueManager, two of which leave PlayingItemIndex out of bounds, making every subsequent Buffering/Ready request throw and leaving the group unusable until it empties: - RemoveFromPlaylist did not compensate for removed items preceding the playing item: removing the playing item together with earlier items could select the wrong item or crash with an out-of-bounds index. - Next/Previous on an empty playlist with RepeatOne/RepeatAll reported success or set PlayingItemIndex to 0 on an empty list, crashing downstream in Group and corrupting the index. - SetPlayingItemByIndex accepted an index equal to the playlist count (latent off-by-one, callers currently pre-validate). --- .../SyncPlay/PlayQueueManagerTests.cs | 156 +++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs (limited to 'tests') diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs new file mode 100644 index 0000000000..32685556b2 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs @@ -0,0 +1,156 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using MediaBrowser.Controller.SyncPlay.Queue; +using MediaBrowser.Model.SyncPlay; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.SyncPlay; + +public class PlayQueueManagerTests +{ + private static PlayQueueManager CreateQueue(int itemCount) + { + var items = Enumerable.Range(0, itemCount).Select(_ => Guid.NewGuid()).ToList(); + var queue = new PlayQueueManager(); + queue.SetPlaylist(items); + return queue; + } + + [Fact] + public void RemoveFromPlaylist_PlayingItemAndPrecedingItemRemoved_PicksPreviousItem() + { + var queue = CreateQueue(5); + queue.SetPlayingItemByIndex(3); + + var playlist = queue.GetPlaylist(); + var expectedItemId = playlist[2].ItemId; + var toRemove = new List { playlist[0].PlaylistItemId, playlist[3].PlaylistItemId }; + var playingItemRemoved = queue.RemoveFromPlaylist(toRemove); + + Assert.True(playingItemRemoved); + Assert.Equal(3, queue.GetPlaylist().Count); + Assert.Equal(1, queue.PlayingItemIndex); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + + [Fact] + public void RemoveFromPlaylist_PlayingItemAndAllPrecedingItemsRemoved_PicksFirstRemainingItem() + { + var queue = CreateQueue(3); + queue.SetPlayingItemByIndex(2); + + var playlist = queue.GetPlaylist(); + var expectedItemId = playlist[1].ItemId; + var toRemove = new List { playlist[0].PlaylistItemId, playlist[2].PlaylistItemId }; + var playingItemRemoved = queue.RemoveFromPlaylist(toRemove); + + Assert.True(playingItemRemoved); + Assert.Single(queue.GetPlaylist()); + Assert.Equal(0, queue.PlayingItemIndex); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + + [Fact] + public void RemoveFromPlaylist_AllItemsRemoved_ResetsPlayingItem() + { + var queue = CreateQueue(2); + queue.SetPlayingItemByIndex(1); + + var toRemove = queue.GetPlaylist().Select(item => item.PlaylistItemId).ToList(); + var playingItemRemoved = queue.RemoveFromPlaylist(toRemove); + + Assert.True(playingItemRemoved); + Assert.Empty(queue.GetPlaylist()); + Assert.False(queue.IsItemPlaying()); + Assert.Equal(Guid.Empty, queue.GetPlayingItemPlaylistId()); + } + + [Fact] + public void RemoveFromPlaylist_ShuffleMode_PicksPreviousItem() + { + var queue = CreateQueue(5); + queue.SetShuffleMode(GroupShuffleMode.Shuffle); + queue.SetPlayingItemByIndex(3); + + var playlist = queue.GetPlaylist(); + var expectedItemId = playlist[2].ItemId; + var toRemove = new List { playlist[0].PlaylistItemId, playlist[3].PlaylistItemId }; + var playingItemRemoved = queue.RemoveFromPlaylist(toRemove); + + Assert.True(playingItemRemoved); + Assert.Equal(3, queue.GetPlaylist().Count); + Assert.Equal(1, queue.PlayingItemIndex); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + + [Fact] + public void RemoveFromPlaylist_PlayingItemNotRemoved_RestoresPlayingItem() + { + var queue = CreateQueue(3); + queue.SetPlayingItemByIndex(2); + + var playlist = queue.GetPlaylist(); + var expectedItemId = playlist[2].ItemId; + var toRemove = new List { playlist[0].PlaylistItemId }; + var playingItemRemoved = queue.RemoveFromPlaylist(toRemove); + + Assert.False(playingItemRemoved); + Assert.Equal(1, queue.PlayingItemIndex); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + + [Theory] + [InlineData(GroupRepeatMode.RepeatNone)] + [InlineData(GroupRepeatMode.RepeatOne)] + [InlineData(GroupRepeatMode.RepeatAll)] + public void Next_EmptyPlaylist_ReturnsFalse(GroupRepeatMode repeatMode) + { + var queue = new PlayQueueManager(); + queue.SetRepeatMode(repeatMode); + + Assert.False(queue.Next()); + Assert.False(queue.IsItemPlaying()); + Assert.Equal(Guid.Empty, queue.GetPlayingItemPlaylistId()); + } + + [Theory] + [InlineData(GroupRepeatMode.RepeatNone)] + [InlineData(GroupRepeatMode.RepeatOne)] + [InlineData(GroupRepeatMode.RepeatAll)] + public void Previous_EmptyPlaylist_ReturnsFalse(GroupRepeatMode repeatMode) + { + var queue = new PlayQueueManager(); + queue.SetRepeatMode(repeatMode); + + Assert.False(queue.Previous()); + Assert.False(queue.IsItemPlaying()); + Assert.Equal(Guid.Empty, queue.GetPlayingItemPlaylistId()); + } + + [Theory] + [InlineData(-1)] + [InlineData(2)] + [InlineData(3)] + public void SetPlayingItemByIndex_OutOfBounds_ResetsPlayingItem(int playlistIndex) + { + var queue = CreateQueue(2); + + queue.SetPlayingItemByIndex(playlistIndex); + + Assert.False(queue.IsItemPlaying()); + Assert.Equal(Guid.Empty, queue.GetPlayingItemPlaylistId()); + } + + [Fact] + public void SetPlayingItemByIndex_InBounds_SetsPlayingItem() + { + var queue = CreateQueue(2); + var expectedItemId = queue.GetPlaylist()[1].ItemId; + + queue.SetPlayingItemByIndex(1); + + Assert.True(queue.IsItemPlaying()); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } +} -- cgit v1.2.3