From d6a1c8413c6a213f6e579246c1b85aad9b028b3a Mon Sep 17 00:00:00 2001 From: Christopher Young Date: Tue, 30 Sep 2025 12:27:25 -0600 Subject: [PATCH 1/7] fixed logic in HasAccessToQueue. If we receive a null response from IsVisibleStandalone then it should be false --- Emby.Server.Implementations/SyncPlay/Group.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs index c2e834ad58..0095ba0a37 100644 --- a/Emby.Server.Implementations/SyncPlay/Group.cs +++ b/Emby.Server.Implementations/SyncPlay/Group.cs @@ -206,7 +206,7 @@ namespace Emby.Server.Implementations.SyncPlay foreach (var itemId in queue) { var item = _libraryManager.GetItemById(itemId); - if (!item.IsVisibleStandalone(user)) + if (!item?.IsVisibleStandalone(user) ?? false) { return false; } From 91b2b7fc3dfe23fdc01834f2f1364e9f8bd98fe4 Mon Sep 17 00:00:00 2001 From: Christopher Young Date: Wed, 8 Oct 2025 12:27:46 -0600 Subject: [PATCH 2/7] added tests --- Emby.Server.Implementations/SyncPlay/Group.cs | 3 +- .../SyncPlay/GroupTests.cs | 85 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs index 0095ba0a37..1142a2f0af 100644 --- a/Emby.Server.Implementations/SyncPlay/Group.cs +++ b/Emby.Server.Implementations/SyncPlay/Group.cs @@ -206,7 +206,8 @@ namespace Emby.Server.Implementations.SyncPlay foreach (var itemId in queue) { var item = _libraryManager.GetItemById(itemId); - if (!item?.IsVisibleStandalone(user) ?? false) + + if (!item?.IsVisibleStandalone(user) ?? true) { return false; } diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs new file mode 100644 index 0000000000..f011d941cc --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs @@ -0,0 +1,85 @@ +using System; +using System.Collections.Generic; +using Emby.Server.Implementations.SyncPlay; +using Jellyfin.Database.Implementations.Entities; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Session; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.SyncPlay +{ + public class GroupTests + { + [Fact] + public void HasAccessToPlayQueue_ReturnsTrue_WhenItemsAreVisible() + { + // Arrange + var mockLogger = new Mock>(); + var mockLoggerFactory = new Mock(); + mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + + var mockUserManager = new Mock(); + var mockSessionManager = new Mock(); + var mockLibraryManager = new Mock(); + + var mockItem = new Mock(); + mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + + mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(mockItem.Object); + + var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + + var itemId = Guid.NewGuid(); + var playlist = new List { itemId }; + group.PlayQueue.Reset(); + group.PlayQueue.SetPlaylist(playlist); + + Assert.Single(group.PlayQueue.GetPlaylist()); + Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); + + var user = new User("test-user", "auth-provider", "pwdreset-provider"); + + var result = group.HasAccessToPlayQueue(user); + + Assert.True(result); + } + + [Fact] + public void HasAccessToPlayQueue_ReturnsFalse_WhenLibraryReturnsNullForItem() + { + // Arrange + var mockLogger = new Mock>(); + var mockLoggerFactory = new Mock(); + mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + + var mockUserManager = new Mock(); + var mockSessionManager = new Mock(); + var mockLibraryManager = new Mock(); + + var mockItem = new Mock(); + mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + + mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns((BaseItem?)null); + Assert.Null( + mockLibraryManager.Object.GetItemById(Guid.NewGuid())); + var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + + var itemId = Guid.NewGuid(); + var playlist = new List { itemId }; + group.PlayQueue.Reset(); + group.PlayQueue.SetPlaylist(playlist); + + Assert.Single(group.PlayQueue.GetPlaylist()); + Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); + + var user = new User("test-user", "auth-provider", "pwdreset-provider"); + + var result = group.HasAccessToPlayQueue(user); + + Assert.False(result); + } + } +} From c08b1a4595da47e16ae57829b4e95fcaffe81e8b Mon Sep 17 00:00:00 2001 From: pokreman06 <112423673+pokreman06@users.noreply.github.com> Date: Fri, 10 Oct 2025 10:35:46 -0600 Subject: [PATCH 3/7] Update Emby.Server.Implementations/SyncPlay/Group.cs Co-authored-by: Bond-009 --- Emby.Server.Implementations/SyncPlay/Group.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs index 1142a2f0af..38a0018a70 100644 --- a/Emby.Server.Implementations/SyncPlay/Group.cs +++ b/Emby.Server.Implementations/SyncPlay/Group.cs @@ -207,7 +207,7 @@ namespace Emby.Server.Implementations.SyncPlay { var item = _libraryManager.GetItemById(itemId); - if (!item?.IsVisibleStandalone(user) ?? true) + if (item is null || !item.IsVisibleStandalone(user)) { return false; } From 790220ef6b041c3b906a2f9dbaea947e0c59d9a4 Mon Sep 17 00:00:00 2001 From: pokreman06 <112423673+pokreman06@users.noreply.github.com> Date: Fri, 10 Oct 2025 10:38:29 -0600 Subject: [PATCH 4/7] Update tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs Co-authored-by: Bond-009 --- .../Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs index f011d941cc..44ff31cb8d 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs @@ -16,7 +16,6 @@ namespace Jellyfin.Server.Implementations.Tests.SyncPlay [Fact] public void HasAccessToPlayQueue_ReturnsTrue_WhenItemsAreVisible() { - // Arrange var mockLogger = new Mock>(); var mockLoggerFactory = new Mock(); mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); From b09c9655fd768172518f2958cc50b0b6ea3fa109 Mon Sep 17 00:00:00 2001 From: pokreman06 <112423673+pokreman06@users.noreply.github.com> Date: Fri, 10 Oct 2025 10:38:36 -0600 Subject: [PATCH 5/7] Update tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs Co-authored-by: Bond-009 --- .../Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs index 44ff31cb8d..1f62d07ab5 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs @@ -49,7 +49,6 @@ namespace Jellyfin.Server.Implementations.Tests.SyncPlay [Fact] public void HasAccessToPlayQueue_ReturnsFalse_WhenLibraryReturnsNullForItem() { - // Arrange var mockLogger = new Mock>(); var mockLoggerFactory = new Mock(); mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); From 438d992c8b0522f6a17f437ee991c8ef6808d749 Mon Sep 17 00:00:00 2001 From: Christopher Young Date: Sat, 8 Nov 2025 07:30:58 -0700 Subject: [PATCH 6/7] Fixed group tests to use a file scoped namespace --- .../SyncPlay/GroupTests.cs | 99 +++++++++---------- 1 file changed, 49 insertions(+), 50 deletions(-) diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs index 1f62d07ab5..d854301358 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs @@ -9,75 +9,74 @@ using Microsoft.Extensions.Logging; using Moq; using Xunit; -namespace Jellyfin.Server.Implementations.Tests.SyncPlay +namespace Jellyfin.Server.Implementations.Tests.SyncPlay; + +public class GroupTests { - public class GroupTests + [Fact] + public void HasAccessToPlayQueue_ReturnsTrue_WhenItemsAreVisible() { - [Fact] - public void HasAccessToPlayQueue_ReturnsTrue_WhenItemsAreVisible() - { - var mockLogger = new Mock>(); - var mockLoggerFactory = new Mock(); - mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + var mockLogger = new Mock>(); + var mockLoggerFactory = new Mock(); + mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); - var mockUserManager = new Mock(); - var mockSessionManager = new Mock(); - var mockLibraryManager = new Mock(); + var mockUserManager = new Mock(); + var mockSessionManager = new Mock(); + var mockLibraryManager = new Mock(); - var mockItem = new Mock(); - mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + var mockItem = new Mock(); + mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); - mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(mockItem.Object); + mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(mockItem.Object); - var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); - var itemId = Guid.NewGuid(); - var playlist = new List { itemId }; - group.PlayQueue.Reset(); - group.PlayQueue.SetPlaylist(playlist); + var itemId = Guid.NewGuid(); + var playlist = new List { itemId }; + group.PlayQueue.Reset(); + group.PlayQueue.SetPlaylist(playlist); - Assert.Single(group.PlayQueue.GetPlaylist()); - Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); + Assert.Single(group.PlayQueue.GetPlaylist()); + Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); - var user = new User("test-user", "auth-provider", "pwdreset-provider"); + var user = new User("test-user", "auth-provider", "pwdreset-provider"); - var result = group.HasAccessToPlayQueue(user); + var result = group.HasAccessToPlayQueue(user); - Assert.True(result); - } + Assert.True(result); + } - [Fact] - public void HasAccessToPlayQueue_ReturnsFalse_WhenLibraryReturnsNullForItem() - { - var mockLogger = new Mock>(); - var mockLoggerFactory = new Mock(); - mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + [Fact] + public void HasAccessToPlayQueue_ReturnsFalse_WhenLibraryReturnsNullForItem() + { + var mockLogger = new Mock>(); + var mockLoggerFactory = new Mock(); + mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); - var mockUserManager = new Mock(); - var mockSessionManager = new Mock(); - var mockLibraryManager = new Mock(); + var mockUserManager = new Mock(); + var mockSessionManager = new Mock(); + var mockLibraryManager = new Mock(); - var mockItem = new Mock(); - mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + var mockItem = new Mock(); + mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); - mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns((BaseItem?)null); - Assert.Null( - mockLibraryManager.Object.GetItemById(Guid.NewGuid())); - var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns((BaseItem?)null); + Assert.Null( + mockLibraryManager.Object.GetItemById(Guid.NewGuid())); + var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); - var itemId = Guid.NewGuid(); - var playlist = new List { itemId }; - group.PlayQueue.Reset(); - group.PlayQueue.SetPlaylist(playlist); + var itemId = Guid.NewGuid(); + var playlist = new List { itemId }; + group.PlayQueue.Reset(); + group.PlayQueue.SetPlaylist(playlist); - Assert.Single(group.PlayQueue.GetPlaylist()); - Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); + Assert.Single(group.PlayQueue.GetPlaylist()); + Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); - var user = new User("test-user", "auth-provider", "pwdreset-provider"); + var user = new User("test-user", "auth-provider", "pwdreset-provider"); - var result = group.HasAccessToPlayQueue(user); + var result = group.HasAccessToPlayQueue(user); - Assert.False(result); - } + Assert.False(result); } } From 88602ce90530e89668c55df69b70b4f14bdc9e8b Mon Sep 17 00:00:00 2001 From: Christopher Young Date: Sat, 8 Nov 2025 12:35:37 -0700 Subject: [PATCH 7/7] Refactored GroupTests. Removed duplicate mock object declarations --- .../SyncPlay/GroupTests.cs | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs index d854301358..bd9e680cd9 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/GroupTests.cs @@ -13,24 +13,35 @@ namespace Jellyfin.Server.Implementations.Tests.SyncPlay; public class GroupTests { + public GroupTests() + { + var mockLogger = new Mock>(); + MockLoggerFactory = new Mock(); + MockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + + MockUserManager = new Mock(); + MockSessionManager = new Mock(); + MockLibraryManager = new Mock(); + MockItem = new Mock(); + MockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + } + + private Mock MockLoggerFactory { get; } + + private Mock MockUserManager { get; } + + private Mock MockSessionManager { get; } + + private Mock MockLibraryManager { get; } + + private Mock MockItem { get; } + [Fact] public void HasAccessToPlayQueue_ReturnsTrue_WhenItemsAreVisible() { - var mockLogger = new Mock>(); - var mockLoggerFactory = new Mock(); - mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); - - var mockUserManager = new Mock(); - var mockSessionManager = new Mock(); - var mockLibraryManager = new Mock(); - - var mockItem = new Mock(); - mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); - - mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(mockItem.Object); - - var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + MockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(MockItem.Object); + var group = new Emby.Server.Implementations.SyncPlay.Group(MockLoggerFactory.Object, MockUserManager.Object, MockSessionManager.Object, MockLibraryManager.Object); var itemId = Guid.NewGuid(); var playlist = new List { itemId }; group.PlayQueue.Reset(); @@ -40,7 +51,6 @@ public class GroupTests Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); var user = new User("test-user", "auth-provider", "pwdreset-provider"); - var result = group.HasAccessToPlayQueue(user); Assert.True(result); @@ -49,22 +59,11 @@ public class GroupTests [Fact] public void HasAccessToPlayQueue_ReturnsFalse_WhenLibraryReturnsNullForItem() { - var mockLogger = new Mock>(); - var mockLoggerFactory = new Mock(); - mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + MockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns((BaseItem?)null); - var mockUserManager = new Mock(); - var mockSessionManager = new Mock(); - var mockLibraryManager = new Mock(); - - var mockItem = new Mock(); - mockItem.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); - - mockLibraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns((BaseItem?)null); - Assert.Null( - mockLibraryManager.Object.GetItemById(Guid.NewGuid())); - var group = new Emby.Server.Implementations.SyncPlay.Group(mockLoggerFactory.Object, mockUserManager.Object, mockSessionManager.Object, mockLibraryManager.Object); + Assert.Null(MockLibraryManager.Object.GetItemById(Guid.NewGuid())); + var group = new Emby.Server.Implementations.SyncPlay.Group(MockLoggerFactory.Object, MockUserManager.Object, MockSessionManager.Object, MockLibraryManager.Object); var itemId = Guid.NewGuid(); var playlist = new List { itemId }; group.PlayQueue.Reset(); @@ -74,7 +73,6 @@ public class GroupTests Assert.Equal(itemId, group.PlayQueue.GetPlaylist()[0].ItemId); var user = new User("test-user", "auth-provider", "pwdreset-provider"); - var result = group.HasAccessToPlayQueue(user); Assert.False(result);