Merge pull request #17370 from zerafachris/fix/item-update-null-optional-fields

fix: don't throw ArgumentNullException on partial UpdateItem payloads (#17366)

2 files changed, 105 insertions, 11 deletions

diff --git a/Jellyfin.Api/Controllers/ItemUpdateController.cs b/Jellyfin.Api/Controllers/ItemUpdateController.cs
index e26264d292..36c82cf461 100644
--- a/Jellyfin.Api/Controllers/ItemUpdateController.cs
+++ b/Jellyfin.Api/Controllers/ItemUpdateController.cs
@@ -236,7 +236,7 @@ public class ItemUpdateController : BaseJellyfinApiController
         return NoContent();
     }
 
-    private async Task UpdateItem(BaseItemDto request, BaseItem item)
+    internal async Task UpdateItem(BaseItemDto request, BaseItem item)
     {
         item.Name = request.Name;
         item.ForcedSortName = request.ForcedSortName;
@@ -250,7 +250,11 @@ public class ItemUpdateController : BaseJellyfinApiController
         item.IndexNumber = request.IndexNumber;
         item.ParentIndexNumber = request.ParentIndexNumber;
         item.Overview = request.Overview;
-        item.Genres = request.Genres.Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+
+        if (request.Genres is not null)
+        {
+            item.Genres = request.Genres.Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+        }
 
         if (item is Episode episode)
         {
@@ -293,10 +297,20 @@ public class ItemUpdateController : BaseJellyfinApiController
         item.CustomRating = request.CustomRating;
 
         var currentTags = item.Tags;
-        var newTags = request.Tags.Select(t => t.Trim()).Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
-        var removedTags = currentTags.Except(newTags).ToList();
-        var addedTags = newTags.Except(currentTags).ToList();
-        item.Tags = newTags;
+        List<string> removedTags;
+        List<string> addedTags;
+        if (request.Tags is not null)
+        {
+            var newTags = request.Tags.Select(t => t.Trim()).Distinct(StringComparer.OrdinalIgnoreCase).ToArray();
+            removedTags = currentTags.Except(newTags).ToList();
+            addedTags = newTags.Except(currentTags).ToList();
+            item.Tags = newTags;
+        }
+        else
+        {
+            removedTags = [];
+            addedTags = [];
+        }
 
         if (item is Series rseries)
         {
@@ -412,15 +426,18 @@ public class ItemUpdateController : BaseJellyfinApiController
             item.RunTimeTicks = request.RunTimeTicks;
         }
 
-        foreach (var pair in request.ProviderIds.ToList())
+        if (request.ProviderIds is not null)
         {
-            if (string.IsNullOrEmpty(pair.Value))
+            foreach (var pair in request.ProviderIds.ToList())
             {
-                request.ProviderIds.Remove(pair.Key);
+                if (string.IsNullOrEmpty(pair.Value))
+                {
+                    request.ProviderIds.Remove(pair.Key);
+                }
             }
-        }
 
-        item.ProviderIds = request.ProviderIds;
+            item.ProviderIds = request.ProviderIds;
+        }
 
         if (item is Video video)
         {
diff --git a/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs b/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs
new file mode 100644
index 0000000000..1a91efe4f2
--- /dev/null
+++ b/tests/Jellyfin.Api.Tests/Controllers/ItemUpdateControllerTests.cs
@@ -0,0 +1,77 @@
+using System;
+using System.Threading.Tasks;
+using Jellyfin.Api.Controllers;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Providers;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Globalization;
+using MediaBrowser.Model.IO;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Api.Tests.Controllers;
+
+public class ItemUpdateControllerTests
+{
+    private readonly ItemUpdateController _subject;
+
+    public ItemUpdateControllerTests()
+    {
+        _subject = new ItemUpdateController(
+            Mock.Of<IFileSystem>(),
+            Mock.Of<ILibraryManager>(),
+            Mock.Of<IProviderManager>(),
+            Mock.Of<ILocalizationManager>(),
+            Mock.Of<IServerConfigurationManager>());
+    }
+
+    [Fact]
+    public async Task UpdateItem_WhenOnlyTagsFieldSupplied_DoesNotThrowAndAppliesTags()
+    {
+        // Regression test for https://github.com/jellyfin/jellyfin/issues/17366
+        // A partial update payload that only sets "Tags" leaves every other
+        // BaseItemDto collection property null (they have no default
+        // initializer). Genres and ProviderIds used to be fed straight into
+        // Distinct()/ToList() without a null check, so this call used to throw
+        // ArgumentNullException before the fix below was applied.
+        var movie = new Movie();
+        var request = new BaseItemDto
+        {
+            Tags = new[] { "new-tag-1", "new-tag-2" }
+        };
+
+        await InvokeUpdateItem(request, movie);
+
+        Assert.Equal(new[] { "new-tag-1", "new-tag-2" }, movie.Tags);
+        Assert.Empty(movie.Genres);
+        Assert.Empty(movie.ProviderIds);
+    }
+
+    [Fact]
+    public async Task UpdateItem_WhenGenresAndProviderIdsOmitted_LeavesExistingValuesUnchanged()
+    {
+        var movie = new Movie
+        {
+            Genres = new[] { "Action" }
+        };
+        movie.ProviderIds["Imdb"] = "tt1234567";
+
+        var request = new BaseItemDto
+        {
+            Tags = Array.Empty<string>()
+        };
+
+        await InvokeUpdateItem(request, movie);
+
+        Assert.Equal(new[] { "Action" }, movie.Genres);
+        Assert.Equal("tt1234567", movie.ProviderIds["Imdb"]);
+    }
+
+    private Task InvokeUpdateItem(BaseItemDto request, BaseItem item)
+    {
+        return _subject.UpdateItem(request, item);
+    }
+}