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
This commit is contained in:
@@ -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))
|
||||
|
||||
@@ -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<long>(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<ILocalizationManager>();
|
||||
localizationManager
|
||||
.Setup(manager => manager.GetRatingScore("PG-13", null))
|
||||
.Returns(new ParentalRatingScore(13, 2));
|
||||
|
||||
new MigrateRatingLevels(
|
||||
new SingleContextFactory(dataSource),
|
||||
new NullStartupLogger<MigrateRatingLevels>(),
|
||||
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}'";
|
||||
|
||||
/// <summary>
|
||||
/// Adds an item carrying a rating and an already populated inherited rating, so that clearing it is visible.
|
||||
/// </summary>
|
||||
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<int?> 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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Stands in for the startup logger. Moq cannot proxy a logger whose category is an internal migration.
|
||||
/// </summary>
|
||||
private sealed class NullStartupLogger<TCategory> : IStartupLogger<TCategory>
|
||||
{
|
||||
public StartupLogTopic? Topic => null;
|
||||
|
||||
public IDisposable? BeginScope<TState>(TState state)
|
||||
where TState : notnull
|
||||
=> NullLogger.Instance.BeginScope(state);
|
||||
|
||||
public bool IsEnabled(LogLevel logLevel) => false;
|
||||
|
||||
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
|
||||
{
|
||||
}
|
||||
|
||||
IStartupLogger IStartupLogger.BeginGroup(FormattableString logEntry) => this;
|
||||
|
||||
IStartupLogger<TOther> IStartupLogger.BeginGroup<TOther>(FormattableString logEntry) => new NullStartupLogger<TOther>();
|
||||
|
||||
IStartupLogger<TCategory> IStartupLogger<TCategory>.BeginGroup(FormattableString logEntry) => this;
|
||||
|
||||
IStartupLogger IStartupLogger.With(Microsoft.Extensions.Logging.ILogger logger) => this;
|
||||
|
||||
IStartupLogger<TOther> IStartupLogger.With<TOther>(Microsoft.Extensions.Logging.ILogger logger) => new NullStartupLogger<TOther>();
|
||||
|
||||
IStartupLogger<TCategory> IStartupLogger<TCategory>.With(Microsoft.Extensions.Logging.ILogger logger) => this;
|
||||
}
|
||||
|
||||
private sealed class SingleContextFactory : IDbContextFactory<JellyfinDbContext>
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
|
||||
Reference in New Issue
Block a user