Merge pull request #17395 from paoloantinori/fix/userdata-null-user-nre-master

Avoid NRE when sorting by user-dependent keys without a user

5 files changed, 103 insertions, 22 deletions

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index f684f4dca3..52569ed66a 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -2315,9 +2315,13 @@ namespace Emby.Server.Implementations.Library
         {
             var comparer = Comparers.FirstOrDefault(c => name == c.Type);
 
-            // If it requires a user, create a new one, and assign the user
             if (comparer is IUserBaseItemComparer)
             {
+                if (user is null)
+                {
+                    throw new ArgumentException($"Sort key '{name}' requires a user, but none was provided.");
+                }
+
                 var userComparer = (IUserBaseItemComparer)Activator.CreateInstance(comparer.GetType())!; // only null for Nullable<T> instances
 
                 userComparer.User = user;
diff --git a/Emby.Server.Implementations/Library/UserDataManager.cs b/Emby.Server.Implementations/Library/UserDataManager.cs
index f5c41e5670..0680046c11 100644
--- a/Emby.Server.Implementations/Library/UserDataManager.cs
+++ b/Emby.Server.Implementations/Library/UserDataManager.cs
@@ -352,6 +352,7 @@ namespace Emby.Server.Implementations.Library
         /// <inheritdoc />
         public UserItemData? GetUserData(User user, BaseItem item)
         {
+            ArgumentNullException.ThrowIfNull(user);
             var row = ResolveUserDataRow(item, item.UserData?.Where(e => e.UserId.Equals(user.Id)));
             return row is not null ? Map(row) : new UserItemData()
             {
diff --git a/Emby.Server.Implementations/Sorting/DateLastMediaAddedComparer.cs b/Emby.Server.Implementations/Sorting/DateLastMediaAddedComparer.cs
index f10e7fcbb7..4159f8cf7d 100644
--- a/Emby.Server.Implementations/Sorting/DateLastMediaAddedComparer.cs
+++ b/Emby.Server.Implementations/Sorting/DateLastMediaAddedComparer.cs
@@ -3,35 +3,15 @@
 
 using System;
 using Jellyfin.Data.Enums;
-using Jellyfin.Database.Implementations.Entities;
 using MediaBrowser.Controller.Entities;
-using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.Sorting;
 using MediaBrowser.Model.Querying;
 
 namespace Emby.Server.Implementations.Sorting
 {
-    public class DateLastMediaAddedComparer : IUserBaseItemComparer
+    public class DateLastMediaAddedComparer : IBaseItemComparer
     {
         /// <summary>
-        /// Gets or sets the user.
-        /// </summary>
-        /// <value>The user.</value>
-        public User User { get; set; }
-
-        /// <summary>
-        /// Gets or sets the user manager.
-        /// </summary>
-        /// <value>The user manager.</value>
-        public IUserManager UserManager { get; set; }
-
-        /// <summary>
-        /// Gets or sets the user data manager.
-        /// </summary>
-        /// <value>The user data manager.</value>
-        public IUserDataManager UserDataManager { get; set; }
-
-        /// <summary>
         /// Gets the name.
         /// </summary>
         /// <value>The name.</value>
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManagerSortTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManagerSortTests.cs
new file mode 100644
index 0000000000..65ec41291d
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/LibraryManagerSortTests.cs
@@ -0,0 +1,89 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using AutoFixture;
+using AutoFixture.AutoMoq;
+using Emby.Naming.Common;
+using Emby.Server.Implementations.Library;
+using Emby.Server.Implementations.Sorting;
+using Jellyfin.Data.Enums;
+using Jellyfin.Database.Implementations.Enums;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Audio;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Persistence;
+using MediaBrowser.Controller.Resolvers;
+using MediaBrowser.Controller.Sorting;
+using MediaBrowser.Model.IO;
+using Moq;
+using Xunit;
+using BaseItem = MediaBrowser.Controller.Entities.BaseItem;
+
+namespace Jellyfin.Server.Implementations.Tests.Library;
+
+public class LibraryManagerSortTests
+{
+    [Fact]
+    public void Sort_UserDependentKey_NullUser_ThrowsArgumentException()
+    {
+        var libraryManager = CreateLibraryManager(
+            new IBaseItemComparer[] { new PlayCountComparer(), new SortNameComparer() });
+
+        BaseItem[] items =
+        {
+            new Audio { Name = "Zulu", SortName = "Zulu", Id = Guid.NewGuid() },
+            new Audio { Name = "Alpha", SortName = "Alpha", Id = Guid.NewGuid() },
+        };
+
+        Assert.Throws<ArgumentException>(() => libraryManager.Sort(
+            items,
+            user: null,
+            new[] { (ItemSortBy.PlayCount, SortOrder.Descending) }).ToArray());
+    }
+
+    [Fact]
+    public void Sort_DateLastContentAdded_NullUser_OrdersByDateNotSortName()
+    {
+        var libraryManager = CreateLibraryManager(
+            new IBaseItemComparer[] { new DateLastMediaAddedComparer(), new SortNameComparer() });
+
+        BaseItem[] items =
+        {
+            MakeFolder("Alpha", new DateTime(2026, 1, 1)),
+            MakeFolder("Mike", new DateTime(2025, 1, 1)),
+            MakeFolder("Zulu", new DateTime(2024, 1, 1))
+        };
+
+        var sorted = libraryManager.Sort(
+            items,
+            user: null,
+            new[] { (ItemSortBy.DateLastContentAdded, SortOrder.Descending) }).ToArray();
+
+        Assert.Equal(new[] { "Alpha", "Mike", "Zulu" }, sorted.Select(i => i.Name));
+    }
+
+    private static Folder MakeFolder(string name, DateTime dateLastMediaAdded)
+        => new() { Name = name, Id = Guid.NewGuid(), DateLastMediaAdded = dateLastMediaAdded };
+
+    private static Emby.Server.Implementations.Library.LibraryManager CreateLibraryManager(IReadOnlyCollection<IBaseItemComparer> comparers)
+    {
+        var fixture = new Fixture().Customize(new AutoMoqCustomization());
+        fixture.Register(() => new NamingOptions());
+        var configMock = fixture.Freeze<Mock<IServerConfigurationManager>>();
+        configMock.Setup(c => c.ApplicationPaths.ProgramDataPath).Returns("/data");
+        BaseItem.ConfigurationManager ??= configMock.Object;
+        var itemRepository = fixture.Freeze<Mock<IItemRepository>>();
+        itemRepository.Setup(i => i.RetrieveItem(It.IsAny<Guid>())).Returns<BaseItem>(null);
+        var fileSystemMock = fixture.Freeze<Mock<IFileSystem>>();
+        fileSystemMock.Setup(f => f.GetFileInfo(It.IsAny<string>())).Returns<string>(path => new FileSystemMetadata { FullName = path });
+
+        return fixture.Build<Emby.Server.Implementations.Library.LibraryManager>().Do(s => s.AddParts(
+                fixture.Create<IEnumerable<IResolverIgnoreRule>>(),
+                fixture.Create<IEnumerable<IItemResolver>>(),
+                fixture.Create<IEnumerable<IIntroProvider>>(),
+                comparers,
+                fixture.Create<IEnumerable<ILibraryPostScanTask>>()))
+            .Create();
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
index bd14ca008d..ba3127bc08 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/UserDataManagerTests.cs
@@ -206,4 +206,11 @@ public sealed class UserDataManagerTests : IDisposable
         Assert.Equal(222, result[fossilItem.Id].PlaybackPositionTicks);
         Assert.Equal(333, result[retiredItem.Id].PlaybackPositionTicks);
     }
+
+    [Fact]
+    public void GetUserData_NullUser_ThrowsArgumentNullException()
+    {
+        var item = CreateAudioBook();
+        Assert.Throws<ArgumentNullException>(() => _userDataManager.GetUserData(null!, item));
+    }
 }