Resolve migration routine loggers from the application container
This commit is contained in:
@@ -29,7 +29,7 @@ internal class MigrateLibraryUserData : IAsyncMigrationRoutine
|
||||
private readonly IDbContextFactory<JellyfinDbContext> _provider;
|
||||
|
||||
public MigrateLibraryUserData(
|
||||
IStartupLogger<MigrateLibraryDb> startupLogger,
|
||||
IStartupLogger<MigrateLibraryUserData> startupLogger,
|
||||
IDbContextFactory<JellyfinDbContext> provider,
|
||||
IServerApplicationPaths paths)
|
||||
{
|
||||
|
||||
@@ -24,7 +24,7 @@ internal class ReseedFolderFlag : IAsyncMigrationRoutine
|
||||
private readonly IDbContextFactory<JellyfinDbContext> _provider;
|
||||
|
||||
public ReseedFolderFlag(
|
||||
IStartupLogger<MigrateLibraryDb> startupLogger,
|
||||
IStartupLogger<ReseedFolderFlag> startupLogger,
|
||||
IDbContextFactory<JellyfinDbContext> provider,
|
||||
IServerApplicationPaths paths)
|
||||
{
|
||||
|
||||
@@ -4,7 +4,6 @@ using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Jellyfin.Server.ServerSetupApp;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging;
|
||||
|
||||
namespace Jellyfin.Server.Migrations.Stages;
|
||||
|
||||
@@ -23,127 +22,49 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta
|
||||
|
||||
public async Task Perform(IServiceProvider? serviceProvider, IStartupLogger logger, CancellationToken cancellationToken)
|
||||
{
|
||||
#pragma warning disable CS0618 // Type or member is obsolete
|
||||
if (typeof(IMigrationRoutine).IsAssignableFrom(MigrationType))
|
||||
{
|
||||
if (serviceProvider is null)
|
||||
{
|
||||
((IMigrationRoutine)Activator.CreateInstance(MigrationType)!).Perform();
|
||||
}
|
||||
else
|
||||
{
|
||||
var migrationServices = new MigrationServiceProvider(serviceProvider, logger);
|
||||
await using (migrationServices.ConfigureAwait(false))
|
||||
{
|
||||
((IMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).Perform();
|
||||
}
|
||||
}
|
||||
#pragma warning restore CS0618 // Type or member is obsolete
|
||||
}
|
||||
else if (typeof(IAsyncMigrationRoutine).IsAssignableFrom(MigrationType))
|
||||
{
|
||||
if (serviceProvider is null)
|
||||
{
|
||||
await ((IAsyncMigrationRoutine)Activator.CreateInstance(MigrationType)!).PerformAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
else
|
||||
{
|
||||
var migrationServices = new MigrationServiceProvider(serviceProvider, logger);
|
||||
await using (migrationServices.ConfigureAwait(false))
|
||||
{
|
||||
await ((IAsyncMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).PerformAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
if (!IsMigrationRoutine(MigrationType))
|
||||
{
|
||||
throw new InvalidOperationException($"The type {MigrationType} does not implement either IMigrationRoutine or IAsyncMigrationRoutine and is not a valid migration type");
|
||||
}
|
||||
|
||||
if (serviceProvider is null)
|
||||
{
|
||||
await RunAsync(Activator.CreateInstance(MigrationType)!, cancellationToken).ConfigureAwait(false);
|
||||
return;
|
||||
}
|
||||
|
||||
// The routine runs against a scope of the applications own container. Copying the application service
|
||||
// descriptors into a child container instead would make that child container the owner of every singleton it
|
||||
// forwards, so disposing it after the migration would also dispose the applications own instance of services
|
||||
// like the ProviderManager and leave the server broken until the next restart.
|
||||
var scope = serviceProvider.CreateAsyncScope();
|
||||
await using (scope.ConfigureAwait(false))
|
||||
{
|
||||
// Nests everything the routine logs through an injected IStartupLogger under the migrations own topic.
|
||||
using (StartupLogger.BeginAmbientTopic(logger.Topic))
|
||||
{
|
||||
await RunAsync(ActivatorUtilities.CreateInstance(scope.ServiceProvider, MigrationType), cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Provides the services a migration routine is constructed with.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// This overlays the migration scoped logging services onto a scope of the application container. Copying the
|
||||
/// application service descriptors into a child container instead would make that child container the owner of
|
||||
/// every singleton it forwards, so disposing it after the migration would also dispose the applications own
|
||||
/// instance of services like the <c>ProviderManager</c> and leave the server broken until the next restart.
|
||||
/// </remarks>
|
||||
private sealed class MigrationServiceProvider : IServiceProvider, IServiceProviderIsService, IAsyncDisposable
|
||||
// The obsolete IMigrationRoutine is still implemented by every routine that predates the async interface, so
|
||||
// the members that have to touch it are grouped here behind a single suppression.
|
||||
#pragma warning disable CS0618 // Type or member is obsolete
|
||||
private static bool IsMigrationRoutine(Type migrationType)
|
||||
{
|
||||
private readonly AsyncServiceScope _scope;
|
||||
private readonly IStartupLogger _logger;
|
||||
private readonly IServiceProviderIsService? _isService;
|
||||
|
||||
public MigrationServiceProvider(IServiceProvider serviceProvider, IStartupLogger logger)
|
||||
{
|
||||
_scope = serviceProvider.CreateAsyncScope();
|
||||
_logger = logger;
|
||||
_isService = _scope.ServiceProvider.GetService<IServiceProviderIsService>();
|
||||
}
|
||||
|
||||
public object? GetService(Type serviceType)
|
||||
{
|
||||
if (serviceType == typeof(IServiceProvider))
|
||||
{
|
||||
return this;
|
||||
}
|
||||
|
||||
if (serviceType == typeof(IServiceProviderIsService))
|
||||
{
|
||||
return _isService is null ? null : this;
|
||||
}
|
||||
|
||||
if (serviceType == typeof(IStartupLogger))
|
||||
{
|
||||
return _logger;
|
||||
}
|
||||
|
||||
if (serviceType == typeof(StartupLogTopic))
|
||||
{
|
||||
return _logger.Topic;
|
||||
}
|
||||
|
||||
if (IsCategoryLogger(serviceType))
|
||||
{
|
||||
var category = serviceType.GenericTypeArguments[0];
|
||||
var baseLogger = _scope.ServiceProvider.GetRequiredService(typeof(ILogger<>).MakeGenericType(category));
|
||||
return Activator.CreateInstance(typeof(NestedStartupLogger<>).MakeGenericType(category), baseLogger, _logger.Topic);
|
||||
}
|
||||
|
||||
return _scope.ServiceProvider.GetService(serviceType);
|
||||
}
|
||||
|
||||
public bool IsService(Type serviceType)
|
||||
{
|
||||
if (serviceType == typeof(IServiceProvider)
|
||||
|| serviceType == typeof(IServiceProviderIsService)
|
||||
|| serviceType == typeof(IStartupLogger)
|
||||
|| serviceType == typeof(StartupLogTopic)
|
||||
|| IsCategoryLogger(serviceType))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
return _isService?.IsService(serviceType) ?? false;
|
||||
}
|
||||
|
||||
public ValueTask DisposeAsync()
|
||||
{
|
||||
return _scope.DisposeAsync();
|
||||
}
|
||||
|
||||
private static bool IsCategoryLogger(Type serviceType)
|
||||
{
|
||||
return serviceType.IsConstructedGenericType && serviceType.GetGenericTypeDefinition() == typeof(IStartupLogger<>);
|
||||
}
|
||||
return typeof(IMigrationRoutine).IsAssignableFrom(migrationType) || typeof(IAsyncMigrationRoutine).IsAssignableFrom(migrationType);
|
||||
}
|
||||
|
||||
private class NestedStartupLogger<TCategory> : StartupLogger<TCategory>
|
||||
private static async Task RunAsync(object routine, CancellationToken cancellationToken)
|
||||
{
|
||||
public NestedStartupLogger(ILogger logger, StartupLogTopic? topic) : base(logger, topic)
|
||||
if (routine is IMigrationRoutine migrationRoutine)
|
||||
{
|
||||
migrationRoutine.Perform();
|
||||
return;
|
||||
}
|
||||
|
||||
await ((IAsyncMigrationRoutine)routine).PerformAsync(cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
#pragma warning restore CS0618 // Type or member is obsolete
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using System;
|
||||
using System.Globalization;
|
||||
using System.Threading;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
|
||||
@@ -8,6 +9,8 @@ namespace Jellyfin.Server.ServerSetupApp;
|
||||
/// <inheritdoc/>
|
||||
public class StartupLogger : IStartupLogger
|
||||
{
|
||||
private static readonly AsyncLocal<StartupLogTopic?> _ambientTopic = new();
|
||||
|
||||
private readonly StartupLogTopic? _topic;
|
||||
|
||||
/// <summary>
|
||||
@@ -17,6 +20,7 @@ public class StartupLogger : IStartupLogger
|
||||
public StartupLogger(ILogger logger)
|
||||
{
|
||||
BaseLogger = logger;
|
||||
_topic = _ambientTopic.Value;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -39,6 +43,18 @@ public class StartupLogger : IStartupLogger
|
||||
/// </summary>
|
||||
protected ILogger BaseLogger { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Makes <paramref name="topic"/> the topic that loggers created on this execution context attach to.
|
||||
/// </summary>
|
||||
/// <param name="topic">The topic to nest newly created loggers under.</param>
|
||||
/// <returns>A scope that restores the previously ambient topic when disposed.</returns>
|
||||
internal static IDisposable BeginAmbientTopic(StartupLogTopic? topic)
|
||||
{
|
||||
var scope = new AmbientTopicScope(_ambientTopic.Value);
|
||||
_ambientTopic.Value = topic;
|
||||
return scope;
|
||||
}
|
||||
|
||||
/// <inheritdoc/>
|
||||
public IStartupLogger BeginGroup(FormattableString logEntry)
|
||||
{
|
||||
@@ -121,4 +137,19 @@ public class StartupLogger : IStartupLogger
|
||||
Topic.Children.Add(startupEntry);
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class AmbientTopicScope : IDisposable
|
||||
{
|
||||
private readonly StartupLogTopic? _previous;
|
||||
|
||||
public AmbientTopicScope(StartupLogTopic? previous)
|
||||
{
|
||||
_previous = previous;
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_ambientTopic.Value = _previous;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,7 +20,6 @@ public class CodeMigrationTests
|
||||
.RegisterStartupLogger()
|
||||
.AddSingleton<ApplicationSingleton>()
|
||||
.AddTransient<MigrationTransient>();
|
||||
services.AddSingleton(services);
|
||||
|
||||
await using var serviceProvider = services.BuildServiceProvider();
|
||||
var applicationSingleton = serviceProvider.GetRequiredService<ApplicationSingleton>();
|
||||
@@ -44,6 +43,29 @@ public class CodeMigrationTests
|
||||
Assert.Same(logger.Topic, performed.Logger.Topic);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Perform_DoesNotLeakTheMigrationTopic()
|
||||
{
|
||||
var services = new ServiceCollection()
|
||||
.AddLogging()
|
||||
.RegisterStartupLogger()
|
||||
.AddSingleton<ApplicationSingleton>()
|
||||
.AddTransient<MigrationTransient>();
|
||||
|
||||
await using var serviceProvider = services.BuildServiceProvider();
|
||||
var logger = new StartupLogger(NullLogger.Instance).BeginGroup($"Test migration");
|
||||
|
||||
var migration = new CodeMigration(
|
||||
typeof(TestMigration),
|
||||
new JellyfinMigrationAttribute("2026-09-05T10:00:00", nameof(TestMigration)),
|
||||
null);
|
||||
await migration.Perform(serviceProvider, logger, CancellationToken.None);
|
||||
|
||||
// The topic belongs to the migration that ran, so loggers resolved afterwards must not still write into it.
|
||||
Assert.Null(serviceProvider.GetRequiredService<IStartupLogger<CodeMigrationTests>>().Topic);
|
||||
Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
|
||||
}
|
||||
|
||||
private sealed class ApplicationSingleton : IDisposable
|
||||
{
|
||||
public bool IsDisposed { get; private set; }
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
using Jellyfin.Server.ServerSetupApp;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Xunit;
|
||||
|
||||
namespace Jellyfin.Server.Tests.ServerSetupApp;
|
||||
|
||||
public class StartupLoggerTests
|
||||
{
|
||||
[Fact]
|
||||
public void BeginAmbientTopic_AttachesNewLoggersToTheTopic()
|
||||
{
|
||||
var migration = new StartupLogger(NullLogger.Instance).BeginGroup($"Migration");
|
||||
|
||||
using (StartupLogger.BeginAmbientTopic(migration.Topic))
|
||||
{
|
||||
Assert.Same(migration.Topic, new StartupLogger(NullLogger.Instance).Topic);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BeginAmbientTopic_RestoresThePreviousTopic()
|
||||
{
|
||||
var root = new StartupLogger(NullLogger.Instance);
|
||||
var outer = root.BeginGroup($"Outer");
|
||||
var inner = outer.BeginGroup($"Inner");
|
||||
|
||||
Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
|
||||
|
||||
using (StartupLogger.BeginAmbientTopic(outer.Topic))
|
||||
{
|
||||
using (StartupLogger.BeginAmbientTopic(inner.Topic))
|
||||
{
|
||||
Assert.Same(inner.Topic, new StartupLogger(NullLogger.Instance).Topic);
|
||||
}
|
||||
|
||||
// Leaving a nested topic has to fall back to the enclosing one, not to the setup UI root.
|
||||
Assert.Same(outer.Topic, new StartupLogger(NullLogger.Instance).Topic);
|
||||
}
|
||||
|
||||
Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BeginGroup_KeepsAnExplicitTopicOverTheAmbientOne()
|
||||
{
|
||||
var migration = new StartupLogger(NullLogger.Instance).BeginGroup($"Migration");
|
||||
var unrelated = new StartupLogger(NullLogger.Instance).BeginGroup($"Unrelated");
|
||||
|
||||
using (StartupLogger.BeginAmbientTopic(migration.Topic))
|
||||
{
|
||||
Assert.Same(unrelated.Topic, unrelated.With(NullLogger.Instance).Topic);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user