From a4e5a5ab314e10f0141dd6af0a1e1b08728a12fc Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 28 Feb 2020 23:18:22 +0100 Subject: [PATCH 1/4] Register configuration correctly with application using 'ConfigureAppConfiguration()' in WebHostBuilder Without this, the correct instance of IConfiguration is not injected into services that rely on it --- Jellyfin.Server/Program.cs | 80 ++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 1dd598236b..1ca6a8a77f 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -101,10 +101,12 @@ namespace Jellyfin.Server // $JELLYFIN_LOG_DIR needs to be set for the logger configuration manager Environment.SetEnvironmentVariable("JELLYFIN_LOG_DIR", appPaths.LogDirectoryPath); - IConfiguration appConfig = await CreateConfiguration(appPaths).ConfigureAwait(false); - - CreateLogger(appConfig, appPaths); + // Create an instance of the application configuration to use for application startup + await InitLoggingConfigFile(appPaths).ConfigureAwait(false); + IConfiguration appConfig = CreateAppConfiguration(appPaths); + // Initialize logging framework + InitializeLoggingFramework(appConfig, appPaths); _logger = _loggerFactory.CreateLogger("Main"); // Log uncaught exceptions to the logging instead of std error @@ -176,15 +178,15 @@ namespace Jellyfin.Server ServiceCollection serviceCollection = new ServiceCollection(); await appHost.InitAsync(serviceCollection).ConfigureAwait(false); - var host = CreateWebHostBuilder(appHost, serviceCollection).Build(); + var webHost = CreateWebHostBuilder(appHost, serviceCollection, appPaths).Build(); // A bit hacky to re-use service provider since ASP.NET doesn't allow a custom service collection. - appHost.ServiceProvider = host.Services; + appHost.ServiceProvider = webHost.Services; appHost.FindParts(); try { - await host.StartAsync().ConfigureAwait(false); + await webHost.StartAsync().ConfigureAwait(false); } catch { @@ -220,7 +222,7 @@ namespace Jellyfin.Server } } - private static IWebHostBuilder CreateWebHostBuilder(ApplicationHost appHost, IServiceCollection serviceCollection) + private static IWebHostBuilder CreateWebHostBuilder(ApplicationHost appHost, IServiceCollection serviceCollection, IApplicationPaths appPaths) { return new WebHostBuilder() .UseKestrel(options => @@ -260,6 +262,7 @@ namespace Jellyfin.Server } } }) + .ConfigureAppConfiguration(config => config.ConfigureAppConfiguration(appPaths)) .UseContentRoot(appHost.ContentRoot) .ConfigureServices(services => { @@ -432,37 +435,56 @@ namespace Jellyfin.Server return new ServerApplicationPaths(dataDir, logDir, configDir, cacheDir, webDir); } - private static async Task CreateConfiguration(IApplicationPaths appPaths) + /// + /// Initialize the logging configuration file using the bundled resource file as a default if it doesn't exist + /// already. + /// + private static async Task InitLoggingConfigFile(IApplicationPaths appPaths) { - const string ResourcePath = "Jellyfin.Server.Resources.Configuration.logging.json"; + // Do nothing if the config file already exists string configPath = Path.Combine(appPaths.ConfigurationDirectoryPath, "logging.json"); - - if (!File.Exists(configPath)) + if (File.Exists(configPath)) { - // For some reason the csproj name is used instead of the assembly name - await using Stream? resource = typeof(Program).Assembly.GetManifestResourceStream(ResourcePath); - if (resource == null) - { - throw new InvalidOperationException( - string.Format( - CultureInfo.InvariantCulture, - "Invalid resource path: '{0}'", - ResourcePath)); - } - - await using Stream dst = File.Open(configPath, FileMode.CreateNew); - await resource.CopyToAsync(dst).ConfigureAwait(false); + return; } + // Get a stream of the resource contents + // NOTE: The .csproj name is used instead of the assembly name in the resource path + const string ResourcePath = "Jellyfin.Server.Resources.Configuration.logging.json"; + await using Stream? resource = typeof(Program).Assembly.GetManifestResourceStream(ResourcePath); + + // Fail if the resource does not exist + if (resource == null) + { + throw new InvalidOperationException( + string.Format(CultureInfo.InvariantCulture, "Invalid resource path: '{0}'", ResourcePath)); + } + + // Copy the resource contents to the expected file path for the config file + await using Stream dst = File.Open(configPath, FileMode.CreateNew); + await resource.CopyToAsync(dst).ConfigureAwait(false); + } + + private static IConfiguration CreateAppConfiguration(IApplicationPaths appPaths) + { return new ConfigurationBuilder() - .SetBasePath(appPaths.ConfigurationDirectoryPath) - .AddInMemoryCollection(ConfigurationOptions.Configuration) - .AddJsonFile("logging.json", false, true) - .AddEnvironmentVariables("JELLYFIN_") + .ConfigureAppConfiguration(appPaths) .Build(); } - private static void CreateLogger(IConfiguration configuration, IApplicationPaths appPaths) + private static IConfigurationBuilder ConfigureAppConfiguration(this IConfigurationBuilder config, IApplicationPaths appPaths) + { + return config + .SetBasePath(appPaths.ConfigurationDirectoryPath) + .AddInMemoryCollection(ConfigurationOptions.Configuration) + .AddJsonFile("logging.json", false, true) + .AddEnvironmentVariables("JELLYFIN_"); + } + + /// + /// Initialize Serilog using configuration and fall back to defaults on failure. + /// + private static void InitializeLoggingFramework(IConfiguration configuration, IApplicationPaths appPaths) { try { From 48f81180726df5a0ef6a64572a4277f3c6af6f9c Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 28 Feb 2020 23:28:15 +0100 Subject: [PATCH 2/4] Do not save a reference to the startup config in ApplicationHost --- Emby.Server.Implementations/ApplicationHost.cs | 18 +++++++----------- Jellyfin.Server/CoreAppHost.cs | 7 ++----- Jellyfin.Server/Program.cs | 9 ++++----- MediaBrowser.Common/IApplicationHost.cs | 6 ++++-- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 8ea188724a..f3abf2a3ef 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -328,8 +328,6 @@ namespace Emby.Server.Implementations private IMediaSourceManager MediaSourceManager { get; set; } - private readonly IConfiguration _configuration; - /// /// Gets the installation manager. /// @@ -367,11 +365,8 @@ namespace Emby.Server.Implementations IStartupOptions options, IFileSystem fileSystem, IImageEncoder imageEncoder, - INetworkManager networkManager, - IConfiguration configuration) + INetworkManager networkManager) { - _configuration = configuration; - XmlSerializer = new MyXmlSerializer(); NetworkManager = networkManager; @@ -587,7 +582,8 @@ namespace Emby.Server.Implementations } } - public async Task InitAsync(IServiceCollection serviceCollection) + /// + public async Task InitAsync(IServiceCollection serviceCollection, IConfiguration startupConfig) { HttpPort = ServerConfigurationManager.Configuration.HttpServerPortNumber; HttpsPort = ServerConfigurationManager.Configuration.HttpsPortNumber; @@ -620,7 +616,7 @@ namespace Emby.Server.Implementations DiscoverTypes(); - await RegisterResources(serviceCollection).ConfigureAwait(false); + await RegisterResources(serviceCollection, startupConfig).ConfigureAwait(false); ContentRoot = ServerConfigurationManager.Configuration.DashboardSourcePath; if (string.IsNullOrEmpty(ContentRoot)) @@ -659,7 +655,7 @@ namespace Emby.Server.Implementations /// /// Registers resources that classes will depend on /// - protected async Task RegisterResources(IServiceCollection serviceCollection) + protected async Task RegisterResources(IServiceCollection serviceCollection, IConfiguration startupConfig) { serviceCollection.AddMemoryCache(); @@ -762,7 +758,7 @@ namespace Emby.Server.Implementations ProcessFactory, LocalizationManager, () => SubtitleEncoder, - _configuration, + startupConfig, StartupOptions.FFmpegPath); serviceCollection.AddSingleton(MediaEncoder); @@ -784,7 +780,7 @@ namespace Emby.Server.Implementations this, LoggerFactory.CreateLogger(), ServerConfigurationManager, - _configuration, + startupConfig, NetworkManager, JsonSerializer, XmlSerializer, diff --git a/Jellyfin.Server/CoreAppHost.cs b/Jellyfin.Server/CoreAppHost.cs index 8b4b61e290..ed5968ad6f 100644 --- a/Jellyfin.Server/CoreAppHost.cs +++ b/Jellyfin.Server/CoreAppHost.cs @@ -23,23 +23,20 @@ namespace Jellyfin.Server /// The to be used by the . /// The to be used by the . /// The to be used by the . - /// The to be used by the . public CoreAppHost( ServerApplicationPaths applicationPaths, ILoggerFactory loggerFactory, StartupOptions options, IFileSystem fileSystem, IImageEncoder imageEncoder, - INetworkManager networkManager, - IConfiguration configuration) + INetworkManager networkManager) : base( applicationPaths, loggerFactory, options, fileSystem, imageEncoder, - networkManager, - configuration) + networkManager) { } diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 1ca6a8a77f..7ac3b0efb4 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -103,10 +103,10 @@ namespace Jellyfin.Server // Create an instance of the application configuration to use for application startup await InitLoggingConfigFile(appPaths).ConfigureAwait(false); - IConfiguration appConfig = CreateAppConfiguration(appPaths); + IConfiguration startupConfig = CreateAppConfiguration(appPaths); // Initialize logging framework - InitializeLoggingFramework(appConfig, appPaths); + InitializeLoggingFramework(startupConfig, appPaths); _logger = _loggerFactory.CreateLogger("Main"); // Log uncaught exceptions to the logging instead of std error @@ -171,12 +171,11 @@ namespace Jellyfin.Server options, new ManagedFileSystem(_loggerFactory.CreateLogger(), appPaths), GetImageEncoder(appPaths), - new NetworkManager(_loggerFactory.CreateLogger()), - appConfig); + new NetworkManager(_loggerFactory.CreateLogger())); try { ServiceCollection serviceCollection = new ServiceCollection(); - await appHost.InitAsync(serviceCollection).ConfigureAwait(false); + await appHost.InitAsync(serviceCollection, startupConfig).ConfigureAwait(false); var webHost = CreateWebHostBuilder(appHost, serviceCollection, appPaths).Build(); diff --git a/MediaBrowser.Common/IApplicationHost.cs b/MediaBrowser.Common/IApplicationHost.cs index 68a24aabaa..0e282cf538 100644 --- a/MediaBrowser.Common/IApplicationHost.cs +++ b/MediaBrowser.Common/IApplicationHost.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Threading.Tasks; using MediaBrowser.Common.Plugins; using MediaBrowser.Model.Updates; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; namespace MediaBrowser.Common @@ -121,11 +122,12 @@ namespace MediaBrowser.Common void RemovePlugin(IPlugin plugin); /// - /// Inits this instance. + /// Initializes this instance. /// /// The service collection. + /// The configuration to use for initialization. /// A task. - Task InitAsync(IServiceCollection serviceCollection); + Task InitAsync(IServiceCollection serviceCollection, IConfiguration startupConfig); /// /// Creates the instance. From 189f005846af9a8f960f6a79cecc8a65feb0bfa6 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 28 Feb 2020 23:35:53 +0100 Subject: [PATCH 3/4] Remove IConfiguration from service collection This does not appear to be used anywhere and the web host already handles injecting this as a special case anyways --- Emby.Server.Implementations/ApplicationHost.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index f3abf2a3ef..eee32520f0 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -664,8 +664,6 @@ namespace Emby.Server.Implementations serviceCollection.AddSingleton(ApplicationPaths); - serviceCollection.AddSingleton(_configuration); - serviceCollection.AddSingleton(JsonSerializer); serviceCollection.AddSingleton(LoggerFactory); From 7e3b6768f81c1cef6889bd7c500ba563aa13a67c Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Sun, 1 Mar 2020 12:10:32 +0100 Subject: [PATCH 4/4] Add NuGet reference to config abstractions in MediaBrowser.Common project --- MediaBrowser.Common/MediaBrowser.Common.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/MediaBrowser.Common/MediaBrowser.Common.csproj b/MediaBrowser.Common/MediaBrowser.Common.csproj index 3da8644040..04e0ee67a2 100644 --- a/MediaBrowser.Common/MediaBrowser.Common.csproj +++ b/MediaBrowser.Common/MediaBrowser.Common.csproj @@ -12,6 +12,7 @@ +