From 0ff869dfcd4ab527dccc975c9be414d1c050a90d Mon Sep 17 00:00:00 2001 From: ZeusCraft10 Date: Mon, 5 Jan 2026 21:08:26 -0500 Subject: fix: Handle unknown item types gracefully in DeserializeBaseItem When querying items with recursive=true, items with types from removed plugins would cause a 500 error. Now these items are skipped with a warning log instead of throwing an exception. Fixes #15945 --- .../Item/BaseItemRepository.cs | 40 +++++++----- .../Migrations/Routines/MigrateLibraryDb.cs | 7 ++- .../Item/BaseItemRepositoryTests.cs | 72 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryTests.cs diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs index 85ab00a2b..b7f1c23e0 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs @@ -277,7 +277,7 @@ public sealed class BaseItemRepository dbQuery = ApplyQueryPaging(dbQuery, filter); dbQuery = ApplyNavigations(dbQuery, filter); - result.Items = dbQuery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).ToArray(); + result.Items = dbQuery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).Where(dto => dto is not null).ToArray()!; result.StartIndex = filter.StartIndex ?? 0; return result; } @@ -297,7 +297,7 @@ public sealed class BaseItemRepository dbQuery = ApplyQueryPaging(dbQuery, filter); dbQuery = ApplyNavigations(dbQuery, filter); - return dbQuery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).ToArray(); + return dbQuery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).Where(dto => dto is not null).ToArray()!; } /// @@ -341,7 +341,7 @@ public sealed class BaseItemRepository mainquery = ApplyNavigations(mainquery, filter); - return mainquery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).ToArray(); + return mainquery.AsEnumerable().Where(e => e is not null).Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)).Where(dto => dto is not null).ToArray()!; } /// @@ -1159,7 +1159,7 @@ public sealed class BaseItemRepository return type.GetCustomAttribute() == null; } - private BaseItemDto DeserializeBaseItem(BaseItemEntity baseItemEntity, bool skipDeserialization = false) + private BaseItemDto? DeserializeBaseItem(BaseItemEntity baseItemEntity, bool skipDeserialization = false) { ArgumentNullException.ThrowIfNull(baseItemEntity, nameof(baseItemEntity)); if (_serverConfigurationManager?.Configuration is null) @@ -1182,11 +1182,19 @@ public sealed class BaseItemRepository /// Logger. /// The application server Host. /// If only mapping should be processed. - /// A mapped BaseItem. - /// Will be thrown if an invalid serialisation is requested. - public static BaseItemDto DeserializeBaseItem(BaseItemEntity baseItemEntity, ILogger logger, IServerApplicationHost? appHost, bool skipDeserialization = false) + /// A mapped BaseItem, or null if the item type is unknown. + public static BaseItemDto? DeserializeBaseItem(BaseItemEntity baseItemEntity, ILogger logger, IServerApplicationHost? appHost, bool skipDeserialization = false) { - var type = GetType(baseItemEntity.Type) ?? throw new InvalidOperationException("Cannot deserialize unknown type."); + var type = GetType(baseItemEntity.Type); + if (type is null) + { + logger.LogWarning( + "Skipping item {ItemId} with unknown type '{ItemType}'. This may indicate a removed plugin or database corruption.", + baseItemEntity.Id, + baseItemEntity.Type); + return null; + } + BaseItemDto? dto = null; if (TypeRequiresDeserialization(type) && baseItemEntity.Data is not null && !skipDeserialization) { @@ -1353,10 +1361,9 @@ public sealed class BaseItemRepository .. resultQuery .AsEnumerable() .Where(e => e is not null) - .Select(e => - { - return (DeserializeBaseItem(e.item, filter.SkipDeserialization), e.itemCount); - }) + .Select(e => (Item: DeserializeBaseItem(e.item, filter.SkipDeserialization), e.itemCount)) + .Where(e => e.Item is not null) + .Select(e => (e.Item!, e.itemCount)) ]; } else @@ -1367,10 +1374,9 @@ public sealed class BaseItemRepository .. query .AsEnumerable() .Where(e => e is not null) - .Select(e => - { - return (DeserializeBaseItem(e, filter.SkipDeserialization), null); - }) + .Select(e => (Item: DeserializeBaseItem(e, filter.SkipDeserialization), ItemCounts: (ItemCounts?)null)) + .Where(e => e.Item is not null) + .Select(e => (e.Item!, e.ItemCounts)) ]; } @@ -2671,6 +2677,6 @@ public sealed class BaseItemRepository .Where(e => artistNames.Contains(e.Name)) .ToArray(); - return artists.GroupBy(e => e.Name).ToDictionary(e => e.Key!, e => e.Select(f => DeserializeBaseItem(f)).Cast().ToArray()); + return artists.GroupBy(e => e.Name).ToDictionary(e => e.Key!, e => e.Select(f => DeserializeBaseItem(f)).Where(dto => dto is not null).Cast().ToArray()); } } diff --git a/Jellyfin.Server/Migrations/Routines/MigrateLibraryDb.cs b/Jellyfin.Server/Migrations/Routines/MigrateLibraryDb.cs index d221d1853..4b1e53a35 100644 --- a/Jellyfin.Server/Migrations/Routines/MigrateLibraryDb.cs +++ b/Jellyfin.Server/Migrations/Routines/MigrateLibraryDb.cs @@ -1247,8 +1247,11 @@ internal class MigrateLibraryDb : IDatabaseMigrationRoutine } var baseItem = BaseItemRepository.DeserializeBaseItem(entity, _logger, null, false); - var dataKeys = baseItem.GetUserDataKeys(); - userDataKeys.AddRange(dataKeys); + if (baseItem is not null) + { + var dataKeys = baseItem.GetUserDataKeys(); + userDataKeys.AddRange(dataKeys); + } return (entity, userDataKeys.ToArray()); } diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryTests.cs new file mode 100644 index 000000000..c450cbb0e --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/BaseItemRepositoryTests.cs @@ -0,0 +1,72 @@ +using System; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +public class BaseItemRepositoryTests +{ + [Fact] + public void DeserializeBaseItem_WithUnknownType_ReturnsNull() + { + // Arrange + var entity = new BaseItemEntity + { + Id = Guid.NewGuid(), + Type = "NonExistent.Plugin.CustomItemType" + }; + + // Act + var result = BaseItemRepository.DeserializeBaseItem(entity, NullLogger.Instance, null, false); + + // Assert + Assert.Null(result); + } + + [Fact] + public void DeserializeBaseItem_WithUnknownType_LogsWarning() + { + // Arrange + var entity = new BaseItemEntity + { + Id = Guid.NewGuid(), + Type = "NonExistent.Plugin.CustomItemType" + }; + var loggerMock = new Mock(); + + // Act + BaseItemRepository.DeserializeBaseItem(entity, loggerMock.Object, null, false); + + // Assert + loggerMock.Verify( + x => x.Log( + LogLevel.Warning, + It.IsAny(), + It.Is((v, t) => v.ToString()!.Contains("unknown type", StringComparison.OrdinalIgnoreCase)), + It.IsAny(), + It.IsAny>()), + Times.Once); + } + + [Fact] + public void DeserializeBaseItem_WithKnownType_ReturnsItem() + { + // Arrange + var entity = new BaseItemEntity + { + Id = Guid.NewGuid(), + Type = "MediaBrowser.Controller.Entities.Movies.Movie" + }; + + // Act + var result = BaseItemRepository.DeserializeBaseItem(entity, NullLogger.Instance, null, false); + + // Assert + Assert.NotNull(result); + } +} -- cgit v1.2.3