From f59ab11c4374b8b795e0457485aba1be783a66bb Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 21:00:18 +1000 Subject: [PATCH] fix(db): materialise the rating list before updating MigrateRatingLevels issued an ExecuteUpdate while the SELECT DISTINCT reader was still open on the same connection. SQLite tolerates that, Npgsql does not, so the AppInitialisation stage aborted and every PostgreSQL instance crash-looped on startup. - read the distinct ratings into a list before the update loop - cover the migration against a real PostgreSQL, with NULL and empty ratings --- .../20260302090000_MigrateRatingLevels.cs | 4 +- .../Migrations/PostgreSqlUpgradeTests.cs | 97 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/Jellyfin.Server/Migrations/Routines/20260302090000_MigrateRatingLevels.cs b/Jellyfin.Server/Migrations/Routines/20260302090000_MigrateRatingLevels.cs index ed92c34aa3..df8a9cceb2 100644 --- a/Jellyfin.Server/Migrations/Routines/20260302090000_MigrateRatingLevels.cs +++ b/Jellyfin.Server/Migrations/Routines/20260302090000_MigrateRatingLevels.cs @@ -36,7 +36,9 @@ internal class MigrateRatingLevels : IDatabaseMigrationRoutine _logger.LogInformation("Recalculating parental rating levels based on rating string."); using var context = _provider.CreateDbContext(); using var transaction = context.Database.BeginTransaction(); - var ratings = context.BaseItems.AsNoTracking().Select(e => e.OfficialRating).Distinct(); + // Read the whole list up front: the updates below run on the same connection, and a provider + // that cannot multiplex commands rejects them while the reader is still open. + var ratings = context.BaseItems.AsNoTracking().Select(e => e.OfficialRating).Distinct().ToList(); foreach (var rating in ratings) { if (string.IsNullOrEmpty(rating)) diff --git a/tests/Jellyfin.Server.Tests/Migrations/PostgreSqlUpgradeTests.cs b/tests/Jellyfin.Server.Tests/Migrations/PostgreSqlUpgradeTests.cs index 9f7ac1481a..26506f8ebb 100644 --- a/tests/Jellyfin.Server.Tests/Migrations/PostgreSqlUpgradeTests.cs +++ b/tests/Jellyfin.Server.Tests/Migrations/PostgreSqlUpgradeTests.cs @@ -14,9 +14,12 @@ using Jellyfin.Server.Migrations.Routines; using Jellyfin.Server.ServerSetupApp; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; +using MediaBrowser.Model.Entities; +using MediaBrowser.Model.Globalization; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Moq; using Npgsql; @@ -43,6 +46,9 @@ public sealed class PostgreSqlUpgradeTests : IAsyncLifetime private const string DanglingOwnerItemId = "44444444-4444-4444-4444-444444444444"; private const string MalformedOwnerItemId = "55555555-5555-5555-5555-555555555555"; private const string PlaceholderOwnedItemId = "66666666-6666-6666-6666-666666666666"; + private const string UnratedItemId = "77777777-7777-7777-7777-777777777777"; + private const string EmptyRatingItemId = "88888888-8888-8888-8888-888888888888"; + private const string RatedItemId = "99999999-9999-9999-9999-999999999999"; private const string AliceId = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; private const string BobId = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"; @@ -248,6 +254,39 @@ public sealed class PostgreSqlUpgradeTests : IAsyncLifetime Assert.Equal(1L, await ScalarAsync(dataSource, $"""SELECT count(*) FROM "BaseItems" WHERE "Id" = '{PlaceholderOwnedItemId}'""", cancellationToken)); } + [Fact] + public async Task MigrateRatingLevels_RecalculatesEveryRating() + { + var cancellationToken = TestContext.Current.CancellationToken; + var connectionString = await CreateDatabaseAsync("rating_levels", cancellationToken); + + await using var dataSource = new NpgsqlDataSourceBuilder(connectionString).Build(); + await MigrateAsync(dataSource, null, cancellationToken); + + // A NULL and an empty rating are what make the migration issue an update while it is still + // walking the distinct rating list, which a connection that cannot multiplex commands rejects. + await InsertRatedItemAsync(dataSource, UnratedItemId, null, cancellationToken); + await InsertRatedItemAsync(dataSource, EmptyRatingItemId, string.Empty, cancellationToken); + await InsertRatedItemAsync(dataSource, RatedItemId, "PG-13", cancellationToken); + + var localizationManager = new Mock(); + localizationManager + .Setup(manager => manager.GetRatingScore("PG-13", null)) + .Returns(new ParentalRatingScore(13, 2)); + + new MigrateRatingLevels( + new SingleContextFactory(dataSource), + new NullStartupLogger(), + localizationManager.Object).Perform(); + + Assert.Null(await InheritedRatingAsync(dataSource, UnratedItemId, "InheritedParentalRatingValue", cancellationToken)); + Assert.Null(await InheritedRatingAsync(dataSource, UnratedItemId, "InheritedParentalRatingSubValue", cancellationToken)); + Assert.Null(await InheritedRatingAsync(dataSource, EmptyRatingItemId, "InheritedParentalRatingValue", cancellationToken)); + Assert.Null(await InheritedRatingAsync(dataSource, EmptyRatingItemId, "InheritedParentalRatingSubValue", cancellationToken)); + Assert.Equal(13, await InheritedRatingAsync(dataSource, RatedItemId, "InheritedParentalRatingValue", cancellationToken)); + Assert.Equal(2, await InheritedRatingAsync(dataSource, RatedItemId, "InheritedParentalRatingSubValue", cancellationToken)); + } + [Fact] public async Task UniqueUsernameIndex_CannotBeAppliedBeforeTheUsernameCodeMigration() { @@ -411,6 +450,34 @@ public sealed class PostgreSqlUpgradeTests : IAsyncLifetime private static string Literal(string? value) => value is null ? "NULL" : $"'{value}'"; + /// + /// Adds an item carrying a rating and an already populated inherited rating, so that clearing it is visible. + /// + private static Task InsertRatedItemAsync( + NpgsqlDataSource dataSource, + string itemId, + string? officialRating, + CancellationToken cancellationToken) + => ExecuteAsync( + dataSource, + $""" + INSERT INTO "BaseItems" ("Id", "Type", "IsMovie", "IsLocked", "IsSeries", "IsRepeat", "IsInMixedFolder", + "IsFolder", "IsVirtualItem", "Name", "Path", "OfficialRating", + "InheritedParentalRatingValue", "InheritedParentalRatingSubValue") + VALUES ('{itemId}', 'MediaBrowser.Controller.Entities.Movies.Movie', true, false, false, false, false, false, false, + 'Rated {itemId}', '/media/{itemId}.mkv', {Literal(officialRating)}, 99, 98); + """, + cancellationToken); + + private static async Task InheritedRatingAsync(NpgsqlDataSource dataSource, string itemId, string column, CancellationToken cancellationToken) + { + var value = await NullableScalarAsync( + dataSource, + $"""SELECT "{column}" FROM "BaseItems" WHERE "Id" = '{itemId}'""", + cancellationToken).ConfigureAwait(false); + return (int?)value; + } + /// /// Builds a database that looks like one written by the previous build: only the baseline migration applied, and /// data that only the old column types could hold. @@ -555,6 +622,36 @@ public sealed class PostgreSqlUpgradeTests : IAsyncLifetime return new NpgsqlConnectionStringBuilder(_container.GetConnectionString()) { Database = name }.ConnectionString; } + /// + /// Stands in for the startup logger. Moq cannot proxy a logger whose category is an internal migration. + /// + private sealed class NullStartupLogger : IStartupLogger + { + public StartupLogTopic? Topic => null; + + public IDisposable? BeginScope(TState state) + where TState : notnull + => NullLogger.Instance.BeginScope(state); + + public bool IsEnabled(LogLevel logLevel) => false; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + } + + IStartupLogger IStartupLogger.BeginGroup(FormattableString logEntry) => this; + + IStartupLogger IStartupLogger.BeginGroup(FormattableString logEntry) => new NullStartupLogger(); + + IStartupLogger IStartupLogger.BeginGroup(FormattableString logEntry) => this; + + IStartupLogger IStartupLogger.With(Microsoft.Extensions.Logging.ILogger logger) => this; + + IStartupLogger IStartupLogger.With(Microsoft.Extensions.Logging.ILogger logger) => new NullStartupLogger(); + + IStartupLogger IStartupLogger.With(Microsoft.Extensions.Logging.ILogger logger) => this; + } + private sealed class SingleContextFactory : IDbContextFactory { private readonly NpgsqlDataSource _dataSource; -- 2.47.3