diff --git a/Emby.Server.Implementations/AppBase/BaseConfigurationManager.cs b/Emby.Server.Implementations/AppBase/BaseConfigurationManager.cs index aa19948e36..a5c4f3fa8b 100644 --- a/Emby.Server.Implementations/AppBase/BaseConfigurationManager.cs +++ b/Emby.Server.Implementations/AppBase/BaseConfigurationManager.cs @@ -87,6 +87,12 @@ namespace Emby.Server.Implementations.AppBase /// The application paths. public IApplicationPaths CommonApplicationPaths { get; private set; } + /// + /// Gets or sets the bus announcing configuration writes to the other instances sharing this + /// configuration directory. Defaults to a no-op, which is the single-instance behaviour. + /// + public IConfigurationInvalidationBus InvalidationBus { get; set; } = NullConfigurationInvalidationBus.Instance; + /// /// Gets or sets the system configuration. /// @@ -169,6 +175,8 @@ namespace Emby.Server.Implementations.AppBase } OnConfigurationUpdated(); + + InvalidationBus.PublishLocalWrite(ConfigurationInvalidationScope.SystemConfiguration, null); } /// @@ -350,6 +358,29 @@ namespace Emby.Server.Implementations.AppBase } OnNamedConfigurationUpdated(key, configuration); + + InvalidationBus.PublishLocalWrite(ConfigurationInvalidationScope.NamedConfiguration, key); + } + + /// + public void InvalidateCachedConfiguration(string? key) + { + if (string.IsNullOrEmpty(key)) + { + lock (_configurationSyncLock) + { + _configuration = null; + } + + // Reloads the system configuration off the shared file as a side effect of re-deriving + // the cache path from it, then tells the in-process consumers to re-read it. + OnConfigurationUpdated(); + return; + } + + _configurations.TryRemove(key, out _); + + OnNamedConfigurationUpdated(key, GetConfiguration(key)); } /// diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 1a54565863..5f42a24825 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -706,6 +706,8 @@ namespace Emby.Server.Implementations BaseItem.UserDataManager = Resolve(); CollectionFolder.XmlSerializer = _xmlSerializer; CollectionFolder.ApplicationHost = this; + CollectionFolder.InvalidationBus = Resolve(); + ConfigurationManager.InvalidationBus = CollectionFolder.InvalidationBus; Folder.UserViewManager = Resolve(); Folder.CollectionManager = Resolve(); Folder.LimitedConcurrencyLibraryScheduler = Resolve(); @@ -790,6 +792,43 @@ namespace Emby.Server.Implementations } } + /// + /// Works out what a configuration update means for the ports this process bound at startup. + /// + /// The HTTP port this process is bound to. + /// The HTTPS port this process is bound to. + /// The HTTP port the shared configuration now carries. + /// The HTTPS port the shared configuration now carries. + /// Whether the shared configuration still marks the port as authorized. + /// Whether this update is another instance's write being applied. + /// What the update requires of this instance. + internal static PortChangeOutcome EvaluatePortChange( + int boundHttpPort, + int boundHttpsPort, + int configuredHttpPort, + int configuredHttpsPort, + bool isPortAuthorized, + bool isApplyingRemoteInvalidation) + { + // Nothing is bound yet, so nothing has gone stale. + if (boundHttpPort == 0 || boundHttpsPort == 0) + { + return default; + } + + if (configuredHttpPort == boundHttpPort && configuredHttpsPort == boundHttpsPort) + { + return default; + } + + // Whoever wrote the change, this process is still listening on a port the configuration no + // longer names, so the pending restart is reported either way. The authorization flag belongs + // to the instance that made the change: it cleared the flag along with the port, and clearing + // it again here would write shared configuration on that instance's behalf and announce it a + // second time. + return new PortChangeOutcome(true, isPortAuthorized && !isApplyingRemoteInvalidation); + } + /// /// Called when [configuration updated]. /// @@ -797,26 +836,24 @@ namespace Emby.Server.Implementations /// The instance containing the event data. private void OnConfigurationUpdated(object sender, EventArgs e) { - var requiresRestart = false; var networkConfiguration = ConfigurationManager.GetNetworkConfiguration(); - // Don't do anything if these haven't been set yet - if (HttpPort != 0 && HttpsPort != 0) - { - // Need to restart if ports have changed - if (networkConfiguration.InternalHttpPort != HttpPort - || networkConfiguration.InternalHttpsPort != HttpsPort) - { - if (ConfigurationManager.Configuration.IsPortAuthorized) - { - ConfigurationManager.Configuration.IsPortAuthorized = false; - ConfigurationManager.SaveConfiguration(); + var portChange = EvaluatePortChange( + HttpPort, + HttpsPort, + networkConfiguration.InternalHttpPort, + networkConfiguration.InternalHttpsPort, + ConfigurationManager.Configuration.IsPortAuthorized, + ConfigurationInvalidationContext.IsApplyingRemoteInvalidation); - requiresRestart = true; - } - } + if (portChange.ClearsPortAuthorization) + { + ConfigurationManager.Configuration.IsPortAuthorized = false; + ConfigurationManager.SaveConfiguration(); } + var requiresRestart = portChange.RequiresRestart; + if (ValidateSslCertificate(networkConfiguration)) { requiresRestart = true; diff --git a/Emby.Server.Implementations/Configuration/ConfigurationInvalidationSubscriber.cs b/Emby.Server.Implementations/Configuration/ConfigurationInvalidationSubscriber.cs new file mode 100644 index 0000000000..c28fbb60a3 --- /dev/null +++ b/Emby.Server.Implementations/Configuration/ConfigurationInvalidationSubscriber.cs @@ -0,0 +1,82 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller.Entities; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +namespace Emby.Server.Implementations.Configuration +{ + /// + /// Applies the configuration invalidations published by the other instances sharing this + /// configuration directory, dropping the local cache entry so the next read comes off the shared file. + /// + public sealed class ConfigurationInvalidationSubscriber : IHostedService + { + private readonly IConfigurationInvalidationBus _bus; + private readonly IConfigurationManager _configurationManager; + private readonly ILogger _logger; + + /// + /// Initializes a new instance of the class. + /// + /// The invalidation bus. + /// The configuration manager holding the cached configuration. + /// The logger. + public ConfigurationInvalidationSubscriber( + IConfigurationInvalidationBus bus, + IConfigurationManager configurationManager, + ILogger logger) + { + _bus = bus; + _configurationManager = configurationManager; + _logger = logger; + } + + /// + public Task StartAsync(CancellationToken cancellationToken) + { + _bus.Subscribe(Apply); + return Task.CompletedTask; + } + + /// + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + private void Apply(ConfigurationInvalidation invalidation) + { + try + { + // Applying re-raises the same update events a local save raises, so the in-process + // consumers re-read. The scope tells those consumers that the write was somebody else's, + // so the ones that answer an update by writing neither repeat it nor publish it back. + using var scope = ConfigurationInvalidationContext.BeginApply(); + + switch (invalidation.Scope) + { + case ConfigurationInvalidationScope.SystemConfiguration: + _configurationManager.InvalidateCachedConfiguration(null); + break; + case ConfigurationInvalidationScope.NamedConfiguration when !string.IsNullOrEmpty(invalidation.Target): + _configurationManager.InvalidateCachedConfiguration(invalidation.Target); + break; + case ConfigurationInvalidationScope.LibraryOptions when !string.IsNullOrEmpty(invalidation.Target): + CollectionFolder.InvalidateLibraryOptions(invalidation.Target); + break; + case ConfigurationInvalidationScope.AllLibraryOptions: + CollectionFolder.InvalidateAllLibraryOptions(); + break; + default: + return; + } + + _logger.LogDebug("Applied {Scope} invalidation for {Target} from {OriginId}.", invalidation.Scope, invalidation.Target, invalidation.OriginId); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to apply a {Scope} invalidation for {Target}.", invalidation.Scope, invalidation.Target); + } + } + } +} diff --git a/Emby.Server.Implementations/Configuration/RedisConfigurationInvalidationBus.cs b/Emby.Server.Implementations/Configuration/RedisConfigurationInvalidationBus.cs new file mode 100644 index 0000000000..7438b7eebd --- /dev/null +++ b/Emby.Server.Implementations/Configuration/RedisConfigurationInvalidationBus.cs @@ -0,0 +1,96 @@ +using System; +using System.Text.Json; +using Jellyfin.Extensions.Json; +using MediaBrowser.Common.Configuration; +using Microsoft.Extensions.Logging; +using StackExchange.Redis; + +namespace Emby.Server.Implementations.Configuration +{ + /// + /// A Redis pub/sub . Notices are broadcast on one channel + /// and every instance but the publisher applies them. + /// + public sealed class RedisConfigurationInvalidationBus : IConfigurationInvalidationBus + { + private const string ChannelName = "jellyfin:configinvalidation"; + + private static readonly JsonSerializerOptions _jsonOptions = JsonDefaults.Options; + + private readonly ISubscriber _subscriber; + private readonly ILogger _logger; + private readonly string _originId; + + /// + /// Initializes a new instance of the class. + /// + /// The Redis connection multiplexer. + /// The logger. + public RedisConfigurationInvalidationBus(IConnectionMultiplexer redis, ILogger logger) + { + ArgumentNullException.ThrowIfNull(redis); + + _subscriber = redis.GetSubscriber(); + _logger = logger; + _originId = Environment.GetEnvironmentVariable("JELLYFIN_INSTANCE_ID") ?? Environment.MachineName; + } + + /// + public void Publish(ConfigurationInvalidationScope scope, string? target) + { + var invalidation = new ConfigurationInvalidation + { + Scope = scope, + Target = target, + OriginId = _originId + }; + + try + { + // Fire and forget: an admin saving configuration must never wait on, or fail because of, + // the bus. The write has already reached the shared directory by this point. + _subscriber.Publish( + RedisChannel.Literal(ChannelName), + JsonSerializer.Serialize(invalidation, _jsonOptions), + CommandFlags.FireAndForget); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to publish {Scope} invalidation for {Target}; other instances keep their cached copy until they restart.", scope, target); + } + } + + /// + public void Subscribe(Action handler) + { + ArgumentNullException.ThrowIfNull(handler); + + try + { + _subscriber.Subscribe(RedisChannel.Literal(ChannelName), (_, value) => Dispatch(handler, value)); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to subscribe to configuration invalidations; this instance keeps its cached configuration until it restarts."); + } + } + + private void Dispatch(Action handler, RedisValue value) + { + try + { + var invalidation = JsonSerializer.Deserialize(value.ToString(), _jsonOptions); + if (invalidation is null || string.Equals(invalidation.OriginId, _originId, StringComparison.Ordinal)) + { + return; + } + + handler(invalidation); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to apply a configuration invalidation."); + } + } + } +} diff --git a/Emby.Server.Implementations/PortChangeOutcome.cs b/Emby.Server.Implementations/PortChangeOutcome.cs new file mode 100644 index 0000000000..78ee1b5293 --- /dev/null +++ b/Emby.Server.Implementations/PortChangeOutcome.cs @@ -0,0 +1,14 @@ +namespace Emby.Server.Implementations +{ + /// + /// What a configuration update carrying different ports requires of the instance reading it. + /// + /// + /// Whether this process is still bound to a port the shared configuration no longer names, and so has + /// to report a pending restart. + /// + /// + /// Whether this instance is the one that has to clear the port authorization flag and save it. + /// + internal readonly record struct PortChangeOutcome(bool RequiresRestart, bool ClearsPortAuthorization); +} diff --git a/Jellyfin.Server/CoreAppHost.cs b/Jellyfin.Server/CoreAppHost.cs index ee6c80ba10..9ac86bb8ed 100644 --- a/Jellyfin.Server/CoreAppHost.cs +++ b/Jellyfin.Server/CoreAppHost.cs @@ -112,6 +112,10 @@ namespace Jellyfin.Server // instance. Active by default once a Redis connection is configured, no-op otherwise. serviceCollection.AddScanLeaderLease(_startupConfig, Logger); + // Configuration invalidation bus: propagates shared-configuration and library-option writes + // to the other instances. Redis-backed when configured, no-op otherwise. + serviceCollection.AddConfigurationInvalidationBus(_startupConfig, Logger); + foreach (var type in GetExportTypes()) { serviceCollection.AddSingleton(typeof(ILyricProvider), type); diff --git a/Jellyfin.Server/Extensions/ConfigurationInvalidationServiceCollectionExtensions.cs b/Jellyfin.Server/Extensions/ConfigurationInvalidationServiceCollectionExtensions.cs new file mode 100644 index 0000000000..e2bd67880e --- /dev/null +++ b/Jellyfin.Server/Extensions/ConfigurationInvalidationServiceCollectionExtensions.cs @@ -0,0 +1,74 @@ +using System; +using Emby.Server.Implementations.Configuration; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller.MediaEncoding; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using StackExchange.Redis; + +namespace Jellyfin.Server.Extensions; + +/// +/// Extensions for registering the shared-configuration invalidation bus. +/// +public static class ConfigurationInvalidationServiceCollectionExtensions +{ + /// + /// Registers the invalidation bus, Redis-backed when a connection string is configured and no-op + /// otherwise, and the subscriber applying the notices other instances publish. + /// + /// + /// The connection string is only set for a multi-instance deployment, which is the only shape where + /// one instance can write the shared configuration directory behind another's back. + /// + /// The service collection. + /// The configuration to read the Redis connection string from. + /// The logger to report the selected bus on. + /// The updated service collection. + public static IServiceCollection AddConfigurationInvalidationBus( + this IServiceCollection serviceCollection, + IConfiguration configuration, + ILogger logger) + { + ArgumentNullException.ThrowIfNull(configuration); + ArgumentNullException.ThrowIfNull(logger); + + if (string.IsNullOrEmpty(configuration[TranscodeStoreOptions.RedisConnectionStringKey])) + { + logger.LogInformation( + "Configuration invalidation bus: {Bus}. Shared-configuration and library-visibility changes stay local to the instance that made them; set {Key} to propagate them.", + nameof(NullConfigurationInvalidationBus), + TranscodeStoreOptions.RedisConnectionStringKey); + + return serviceCollection.AddSingleton(NullConfigurationInvalidationBus.Instance); + } + + logger.LogInformation( + "Configuration invalidation bus: {Bus}. Shared-configuration and library-visibility changes propagate to every instance.", + nameof(RedisConfigurationInvalidationBus)); + + serviceCollection.AddSingleton(sp => + { + try + { + return new RedisConfigurationInvalidationBus( + sp.GetRequiredService(), + sp.GetRequiredService>()); + } + catch (Exception ex) + { + // Fail open: an unreachable Redis degrades to the single-instance behaviour of every + // instance keeping its own cached configuration, rather than aborting startup. + sp.GetRequiredService>().LogError( + ex, + "Redis is configured but unavailable, so shared-configuration changes will not propagate between instances. Check {Key}.", + TranscodeStoreOptions.RedisConnectionStringKey); + + return NullConfigurationInvalidationBus.Instance; + } + }); + + return serviceCollection.AddHostedService(); + } +} diff --git a/MediaBrowser.Common/Configuration/ConfigurationInvalidation.cs b/MediaBrowser.Common/Configuration/ConfigurationInvalidation.cs new file mode 100644 index 0000000000..ba6c884a44 --- /dev/null +++ b/MediaBrowser.Common/Configuration/ConfigurationInvalidation.cs @@ -0,0 +1,26 @@ +namespace MediaBrowser.Common.Configuration +{ + /// + /// A notice that one instance has written shared configuration, so every other instance has to drop + /// its locally cached copy and read the shared file again. + /// + public sealed class ConfigurationInvalidation + { + /// + /// Gets or sets the cache this notice refers to. + /// + public ConfigurationInvalidationScope Scope { get; set; } + + /// + /// Gets or sets what was invalidated within the scope: the configuration key for + /// , the library path for + /// , and null otherwise. + /// + public string? Target { get; set; } + + /// + /// Gets or sets the identity of the instance that published the notice, so it can ignore its own. + /// + public string? OriginId { get; set; } + } +} diff --git a/MediaBrowser.Common/Configuration/ConfigurationInvalidationBusExtensions.cs b/MediaBrowser.Common/Configuration/ConfigurationInvalidationBusExtensions.cs new file mode 100644 index 0000000000..c556c33e5d --- /dev/null +++ b/MediaBrowser.Common/Configuration/ConfigurationInvalidationBusExtensions.cs @@ -0,0 +1,30 @@ +namespace MediaBrowser.Common.Configuration +{ + /// + /// Extensions for . + /// + public static class ConfigurationInvalidationBusExtensions + { + /// + /// Announces a write this instance originated, and stays silent for a write induced by an + /// invalidation another instance published. + /// + /// + /// This is the backstop for the write path: a consumer reacting to an applied invalidation by + /// writing - including a plugin that knows nothing about the bus - cannot turn that write into a + /// notice of its own. + /// + /// The bus. + /// The cache that was written. + /// The configuration key or library path that was written, if any. + public static void PublishLocalWrite(this IConfigurationInvalidationBus bus, ConfigurationInvalidationScope scope, string? target) + { + if (ConfigurationInvalidationContext.IsApplyingRemoteInvalidation) + { + return; + } + + bus.Publish(scope, target); + } + } +} diff --git a/MediaBrowser.Common/Configuration/ConfigurationInvalidationContext.cs b/MediaBrowser.Common/Configuration/ConfigurationInvalidationContext.cs new file mode 100644 index 0000000000..57f0de4dd1 --- /dev/null +++ b/MediaBrowser.Common/Configuration/ConfigurationInvalidationContext.cs @@ -0,0 +1,57 @@ +using System; +using System.Threading; + +namespace MediaBrowser.Common.Configuration +{ + /// + /// Marks the flow of control that is applying an invalidation published by another instance, so the + /// reactions to it can tell a remote write apart from a local one. + /// + /// + /// Applying an invalidation raises the same update events a local save raises, because the in-process + /// consumers have to re-read the configuration either way. Some of those consumers answer an update by + /// writing, and that write must neither repeat what the publishing instance already did nor fan back + /// out over the bus. The flag rides the execution context, so it reaches the queued and asynchronous + /// event handlers as well as the synchronous ones. + /// + public static class ConfigurationInvalidationContext + { + private static readonly AsyncLocal _applyingRemoteInvalidation = new(); + + /// + /// Gets a value indicating whether the current flow of control is applying an invalidation + /// published by another instance rather than handling a local save. + /// + public static bool IsApplyingRemoteInvalidation => _applyingRemoteInvalidation.Value; + + /// + /// Marks the current flow of control as applying a remote invalidation until the returned scope is + /// disposed. + /// + /// The scope to dispose once the invalidation has been applied. + public static IDisposable BeginApply() => new ApplyScope(); + + private sealed class ApplyScope : IDisposable + { + private readonly bool _previous; + private bool _disposed; + + public ApplyScope() + { + _previous = _applyingRemoteInvalidation.Value; + _applyingRemoteInvalidation.Value = true; + } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + _applyingRemoteInvalidation.Value = _previous; + } + } + } +} diff --git a/MediaBrowser.Common/Configuration/ConfigurationInvalidationScope.cs b/MediaBrowser.Common/Configuration/ConfigurationInvalidationScope.cs new file mode 100644 index 0000000000..2c541a4334 --- /dev/null +++ b/MediaBrowser.Common/Configuration/ConfigurationInvalidationScope.cs @@ -0,0 +1,29 @@ +namespace MediaBrowser.Common.Configuration +{ + /// + /// Identifies which locally cached copy of the shared configuration a + /// refers to. + /// + public enum ConfigurationInvalidationScope + { + /// + /// The system configuration cached by the configuration manager. + /// + SystemConfiguration = 0, + + /// + /// A single named configuration, identified by its key. + /// + NamedConfiguration = 1, + + /// + /// The library options of a single collection folder, identified by its path. + /// + LibraryOptions = 2, + + /// + /// The library options of every collection folder, for changes to the library structure itself. + /// + AllLibraryOptions = 3 + } +} diff --git a/MediaBrowser.Common/Configuration/IConfigurationInvalidationBus.cs b/MediaBrowser.Common/Configuration/IConfigurationInvalidationBus.cs new file mode 100644 index 0000000000..e620153626 --- /dev/null +++ b/MediaBrowser.Common/Configuration/IConfigurationInvalidationBus.cs @@ -0,0 +1,28 @@ +using System; + +namespace MediaBrowser.Common.Configuration +{ + /// + /// Carries cache-invalidation notices between the instances that share one configuration directory. + /// + /// + /// The shared directory carries the content; this bus only carries the fact that it changed. Every + /// implementation is expected to fail open: a bus that cannot deliver must not throw into the write + /// path, leaving each instance on its own locally cached copy until it restarts. + /// + public interface IConfigurationInvalidationBus + { + /// + /// Announces that this instance has written shared configuration. + /// + /// The cache that was written. + /// The configuration key or library path that was written, if any. + void Publish(ConfigurationInvalidationScope scope, string? target); + + /// + /// Registers the handler invoked for notices published by other instances. + /// + /// The handler applying the invalidation locally. + void Subscribe(Action handler); + } +} diff --git a/MediaBrowser.Common/Configuration/IConfigurationManager.cs b/MediaBrowser.Common/Configuration/IConfigurationManager.cs index 18a8d3e7b7..2b8becad96 100644 --- a/MediaBrowser.Common/Configuration/IConfigurationManager.cs +++ b/MediaBrowser.Common/Configuration/IConfigurationManager.cs @@ -85,6 +85,20 @@ namespace MediaBrowser.Common.Configuration /// /// The factories. void AddParts(IEnumerable factories); + + /// + /// Drops the locally cached copy of configuration another instance has written to the shared + /// configuration directory, so the next read reloads it, and raises the local update event. + /// + /// + /// An implementation predating the invalidation bus keeps the default, which reports that it + /// cannot drop its cache rather than quietly leaving it stale. The caller applying a remote + /// notice treats that as a failed apply and logs it. + /// + /// The named configuration key, or null for the system configuration. + /// The implementation cannot drop its cached configuration. + void InvalidateCachedConfiguration(string? key) + => throw new NotSupportedException(GetType().Name + " cannot drop configuration cached from the shared configuration directory, so writes by other instances will not be picked up."); } public static class ConfigurationManagerExtensions diff --git a/MediaBrowser.Common/Configuration/NullConfigurationInvalidationBus.cs b/MediaBrowser.Common/Configuration/NullConfigurationInvalidationBus.cs new file mode 100644 index 0000000000..a93b41a2cf --- /dev/null +++ b/MediaBrowser.Common/Configuration/NullConfigurationInvalidationBus.cs @@ -0,0 +1,27 @@ +using System; + +namespace MediaBrowser.Common.Configuration +{ + /// + /// A no-op used by single-instance installs and whenever + /// the shared bus is unavailable. Every instance keeps its own cached configuration, which is the + /// behaviour of an install that has only one. + /// + public sealed class NullConfigurationInvalidationBus : IConfigurationInvalidationBus + { + /// + /// Gets the shared instance. + /// + public static NullConfigurationInvalidationBus Instance { get; } = new NullConfigurationInvalidationBus(); + + /// + public void Publish(ConfigurationInvalidationScope scope, string? target) + { + } + + /// + public void Subscribe(Action handler) + { + } + } +} diff --git a/MediaBrowser.Controller/Entities/CollectionFolder.cs b/MediaBrowser.Controller/Entities/CollectionFolder.cs index ffdc8421da..004ff8cf72 100644 --- a/MediaBrowser.Controller/Entities/CollectionFolder.cs +++ b/MediaBrowser.Controller/Entities/CollectionFolder.cs @@ -14,6 +14,7 @@ using System.Threading.Tasks; using Jellyfin.Data.Enums; using Jellyfin.Database.Implementations.Entities; using Jellyfin.Extensions.Json; +using MediaBrowser.Common.Configuration; using MediaBrowser.Controller.IO; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Providers; @@ -70,6 +71,12 @@ namespace MediaBrowser.Controller.Entities public static IServerApplicationHost ApplicationHost { get; set; } + /// + /// Gets or sets the bus announcing library option writes to the other instances sharing this + /// configuration directory. Defaults to a no-op, which is the single-instance behaviour. + /// + public static IConfigurationInvalidationBus InvalidationBus { get; set; } = NullConfigurationInvalidationBus.Instance; + [JsonIgnore] public override bool SupportsPlayedStatus => false; @@ -188,11 +195,36 @@ namespace MediaBrowser.Controller.Entities XmlSerializer.SerializeToFile(clone, GetLibraryOptionsPath(path)); LibraryOptionsUpdated?.Invoke(null, new LibraryOptionsUpdatedEventArgs(path, options)); + + InvalidationBus.PublishLocalWrite(ConfigurationInvalidationScope.LibraryOptions, path); } - public static void OnCollectionFolderChange() + /// + /// Drops the cached options of one library so the next read comes off options.xml again. + /// Applied on the instances that did not write, and so does not publish. + /// + /// The library path. + public static void InvalidateLibraryOptions(string path) + { + _libraryOptions.TryRemove(path, out _); + + LibraryOptionsUpdated?.Invoke(null, new LibraryOptionsUpdatedEventArgs(path, GetLibraryOptions(path))); + } + + /// + /// Drops every cached library option set. Applied on the instances that did not write, and so does + /// not publish. + /// + public static void InvalidateAllLibraryOptions() => _libraryOptions.Clear(); + public static void OnCollectionFolderChange() + { + InvalidateAllLibraryOptions(); + + InvalidationBus.PublishLocalWrite(ConfigurationInvalidationScope.AllLibraryOptions, null); + } + public override bool IsSaveLocalMetadataEnabled() { return true; diff --git a/src/Jellyfin.LiveTv/Recordings/RecordingsManager.cs b/src/Jellyfin.LiveTv/Recordings/RecordingsManager.cs index 62a06370da..ce463ceb3d 100644 --- a/src/Jellyfin.LiveTv/Recordings/RecordingsManager.cs +++ b/src/Jellyfin.LiveTv/Recordings/RecordingsManager.cs @@ -444,6 +444,13 @@ public sealed class RecordingsManager : IRecordingsManager, IDisposable private async void OnNamedConfigurationUpdated(object? sender, ConfigurationUpdateEventArgs e) { + // The instance that wrote the change creates the folders; racing it from here would write the + // same virtual folder a second time. + if (ConfigurationInvalidationContext.IsApplyingRemoteInvalidation) + { + return; + } + if (string.Equals(e.Key, "livetv", StringComparison.OrdinalIgnoreCase)) { await CreateRecordingFolders().ConfigureAwait(false); diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/ApplicationHostPortChangeTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/ApplicationHostPortChangeTests.cs new file mode 100644 index 0000000000..2c3e215ce0 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/ApplicationHostPortChangeTests.cs @@ -0,0 +1,81 @@ +using Emby.Server.Implementations; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// The decision ApplicationHost.OnConfigurationUpdated makes about a port change. The ports this +/// process bound are fixed for its lifetime and the pending-restart flag is per-process, so an instance +/// applying another instance's port change still has to notice its own binding went stale - while leaving +/// the authorization write, and the notice that follows it, to the instance that made the change. +/// +public static class ApplicationHostPortChangeTests +{ + /// + /// The local case, unchanged: clear the authorization flag and report the pending restart. + /// + [Fact] + public static void LocalPortChange_ClearsAuthorizationAndRequiresRestart() + { + var outcome = ApplicationHost.EvaluatePortChange(8096, 8920, 9096, 8920, true, false); + + Assert.True(outcome.RequiresRestart); + Assert.True(outcome.ClearsPortAuthorization); + } + + /// + /// The cross-instance case: the peer wrote the new port and cleared the flag with it, so this + /// instance must not write, but it is still listening on the old port and has to say so. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public static void RemotePortChange_RequiresRestartWithoutWriting(bool isPortAuthorized) + { + var outcome = ApplicationHost.EvaluatePortChange(8096, 8920, 9096, 8920, isPortAuthorized, true); + + Assert.True(outcome.RequiresRestart); + Assert.False(outcome.ClearsPortAuthorization); + } + + /// + /// A second update while a port change is already pending must not write the flag again, and the + /// binding is still stale. + /// + [Fact] + public static void LocalPortChange_WithAuthorizationAlreadyCleared_RequiresRestartWithoutWriting() + { + var outcome = ApplicationHost.EvaluatePortChange(8096, 8920, 9096, 8920, false, false); + + Assert.True(outcome.RequiresRestart); + Assert.False(outcome.ClearsPortAuthorization); + } + + /// + /// An update that leaves the ports alone is not a port change, whoever wrote it. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public static void UnchangedPorts_DoNothing(bool isApplyingRemoteInvalidation) + { + var outcome = ApplicationHost.EvaluatePortChange(8096, 8920, 8096, 8920, true, isApplyingRemoteInvalidation); + + Assert.False(outcome.RequiresRestart); + Assert.False(outcome.ClearsPortAuthorization); + } + + /// + /// Nothing is decided before the ports have been bound. + /// + [Theory] + [InlineData(0, 8920)] + [InlineData(8096, 0)] + public static void UnboundPorts_DoNothing(int boundHttpPort, int boundHttpsPort) + { + var outcome = ApplicationHost.EvaluatePortChange(boundHttpPort, boundHttpsPort, 9096, 9920, true, false); + + Assert.False(outcome.RequiresRestart); + Assert.False(outcome.ClearsPortAuthorization); + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/FakeInvalidationBusFabric.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/FakeInvalidationBusFabric.cs new file mode 100644 index 0000000000..04fcceeede --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/FakeInvalidationBusFabric.cs @@ -0,0 +1,73 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using MediaBrowser.Common.Configuration; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// An in-process stand-in for the Redis pub/sub bus: every endpoint connected to one fabric receives +/// what the others publish, and never its own notices. +/// +internal sealed class FakeInvalidationBusFabric +{ + private readonly List _endpoints = new(); + + /// + /// Connects a new instance to the fabric. + /// + /// The identity of the connecting instance. + /// The bus of that instance. + public IConfigurationInvalidationBus Connect(string originId) + { + var endpoint = new Endpoint(this, originId); + lock (_endpoints) + { + _endpoints.Add(endpoint); + } + + return endpoint; + } + + private void Broadcast(ConfigurationInvalidation invalidation) + { + Endpoint[] endpoints; + lock (_endpoints) + { + endpoints = _endpoints.ToArray(); + } + + foreach (var endpoint in endpoints.Where(e => !string.Equals(e.OriginId, invalidation.OriginId, StringComparison.Ordinal))) + { + endpoint.Deliver(invalidation); + } + } + + private sealed class Endpoint : IConfigurationInvalidationBus + { + private readonly FakeInvalidationBusFabric _fabric; + private readonly List> _handlers = new(); + + public Endpoint(FakeInvalidationBusFabric fabric, string originId) + { + _fabric = fabric; + OriginId = originId; + } + + public string OriginId { get; } + + public void Publish(ConfigurationInvalidationScope scope, string? target) + => _fabric.Broadcast(new ConfigurationInvalidation { Scope = scope, Target = target, OriginId = OriginId }); + + public void Subscribe(Action handler) + => _handlers.Add(handler); + + public void Deliver(ConfigurationInvalidation invalidation) + { + foreach (var handler in _handlers) + { + handler(invalidation); + } + } + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/LibraryVisibilityPropagationTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/LibraryVisibilityPropagationTests.cs new file mode 100644 index 0000000000..65694a0427 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/LibraryVisibilityPropagationTests.cs @@ -0,0 +1,161 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations.Configuration; +using Emby.Server.Implementations.Serialization; +using Jellyfin.Data; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Enums; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Model.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// Library options are cached in a process-wide dictionary, so the replica that did not serve the admin's +/// request is the one under test here: the other replica's write reaches the shared library directory, and +/// this one has to stop answering out of its own stale copy. +/// +/// +/// Disabling a library is an access revocation that overrides every per-user check, so a stale replica +/// keeps serving content that is supposed to be hidden from everyone. +/// +public sealed class LibraryVisibilityPropagationTests : IDisposable +{ + private readonly string _libraryPath; + private readonly MyXmlSerializer _serializer = new MyXmlSerializer(); + + /// + /// Initializes a new instance of the class. + /// + public LibraryVisibilityPropagationTests() + { + _libraryPath = Path.Combine(Path.GetTempPath(), "jf-library-prop-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_libraryPath); + + var applicationHost = new Mock(); + applicationHost.Setup(host => host.ExpandVirtualPath(It.IsAny())).Returns(path => path); + applicationHost.Setup(host => host.ReverseVirtualPath(It.IsAny())).Returns(path => path); + + CollectionFolder.XmlSerializer = _serializer; + CollectionFolder.ApplicationHost = applicationHost.Object; + } + + /// + public void Dispose() + { + CollectionFolder.InvalidationBus = NullConfigurationInvalidationBus.Instance; + CollectionFolder.InvalidateAllLibraryOptions(); + + try + { + Directory.Delete(_libraryPath, true); + } + catch (IOException) + { + } + } + + /// + /// Disabling a library on one replica has to hide it on every replica. Until it does, the ones that did + /// not serve the request keep the library visible to every user. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task LibraryDisabledOnAnotherInstance_IsNotVisibleHere() + { + var fabric = new FakeInvalidationBusFabric(); + var otherInstance = fabric.Connect("pod-a"); + await SubscribeThisInstanceAsync(fabric); + + WriteSharedOptions(enabled: true); + + var user = CreateUser(); + var library = new CollectionFolder { Path = _libraryPath, Name = "Movies" }; + + // This replica answers out of its cache from here on. + Assert.True(library.IsVisible(user)); + + // The admin disables the library on the other replica: it writes the shared directory and says so. + WriteSharedOptions(enabled: false); + otherInstance.Publish(ConfigurationInvalidationScope.LibraryOptions, _libraryPath); + + Assert.False(library.IsVisible(user)); + Assert.False(CollectionFolder.GetLibraryOptions(_libraryPath).Enabled); + } + + /// + /// A path remap made on another replica has to reach this one, or it keeps resolving media against a + /// path that is no longer the library's. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task LibraryPathRemappedOnAnotherInstance_IsSeenHere() + { + var fabric = new FakeInvalidationBusFabric(); + var otherInstance = fabric.Connect("pod-a"); + await SubscribeThisInstanceAsync(fabric); + + WriteSharedOptions(enabled: true, mediaPath: "/media/old"); + Assert.Equal("/media/old", CollectionFolder.GetLibraryOptions(_libraryPath).PathInfos[0].Path); + + WriteSharedOptions(enabled: true, mediaPath: "/media/new"); + otherInstance.Publish(ConfigurationInvalidationScope.LibraryOptions, _libraryPath); + + Assert.Equal("/media/new", CollectionFolder.GetLibraryOptions(_libraryPath).PathInfos[0].Path); + } + + /// + /// Saving library options here has to tell the other replicas, which is the half of the exchange the + /// tests above take as given. + /// + [Fact] + public void SaveLibraryOptions_AnnouncesTheLibraryToTheOtherInstances() + { + var fabric = new FakeInvalidationBusFabric(); + ConfigurationInvalidation? received = null; + + var otherInstance = fabric.Connect("pod-b"); + otherInstance.Subscribe(invalidation => received = invalidation); + CollectionFolder.InvalidationBus = fabric.Connect("pod-a"); + + CollectionFolder.SaveLibraryOptions(_libraryPath, new LibraryOptions { Enabled = false }); + + Assert.NotNull(received); + Assert.Equal(ConfigurationInvalidationScope.LibraryOptions, received.Scope); + Assert.Equal(_libraryPath, received.Target); + } + + private async Task SubscribeThisInstanceAsync(FakeInvalidationBusFabric fabric) + { + var bus = fabric.Connect("pod-b"); + CollectionFolder.InvalidationBus = bus; + + var subscriber = new ConfigurationInvalidationSubscriber( + bus, + Mock.Of(), + NullLogger.Instance); + + await subscriber.StartAsync(CancellationToken.None); + } + + private void WriteSharedOptions(bool enabled, string mediaPath = "/media") + { + // Written the way the other replica writes it, straight onto the shared directory. + var options = new LibraryOptions { Enabled = enabled, PathInfos = [new MediaPathInfo(mediaPath)] }; + _serializer.SerializeToFile(options, Path.Combine(_libraryPath, "options.xml")); + } + + private static User CreateUser() + { + var user = new User("propagation", "auth", "reset"); + user.SetPermission(PermissionKind.EnableAllFolders, true); + return user; + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/RedisConfigurationInvalidationBusTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/RedisConfigurationInvalidationBusTests.cs new file mode 100644 index 0000000000..95232a8f10 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/RedisConfigurationInvalidationBusTests.cs @@ -0,0 +1,104 @@ +using System; +using System.Threading.Tasks; +using Emby.Server.Implementations.Configuration; +using MediaBrowser.Common.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using StackExchange.Redis; +using Testcontainers.Redis; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// Round-trips through a real Redis, the transport two +/// replicas actually use to tell each other that the shared configuration directory has changed. +/// +[Trait("Category", "RequiresDocker")] +public sealed class RedisConfigurationInvalidationBusTests : IAsyncLifetime +{ + private readonly RedisContainer _container; + private IConnectionMultiplexer? _redis; + + /// + /// Initializes a new instance of the class. + /// + public RedisConfigurationInvalidationBusTests() + { + _container = new RedisBuilder("redis:7-alpine").Build(); + } + + /// + public async ValueTask InitializeAsync() + { + await _container.StartAsync(); + _redis = await ConnectionMultiplexer.ConnectAsync(_container.GetConnectionString()); + } + + /// + public async ValueTask DisposeAsync() + { + if (_redis is not null) + { + await _redis.DisposeAsync(); + } + + await _container.DisposeAsync(); + } + + /// + /// A notice published by one replica reaches the other, carrying enough to invalidate one entry. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task Publish_ReachesTheOtherInstance() + { + var received = new TaskCompletionSource(); + var instanceA = CreateBus("pod-a"); + var instanceB = CreateBus("pod-b"); + + instanceB.Subscribe(invalidation => received.TrySetResult(invalidation)); + + instanceA.Publish(ConfigurationInvalidationScope.LibraryOptions, "/media/movies"); + + var invalidation = await received.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + Assert.Equal(ConfigurationInvalidationScope.LibraryOptions, invalidation.Scope); + Assert.Equal("/media/movies", invalidation.Target); + Assert.Equal("pod-a", invalidation.OriginId); + } + + /// + /// The publishing replica has already applied the change to its own cache, so it must not act on its + /// own notice and reload what it just wrote. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task Publish_IsNotDeliveredToThePublisher() + { + var ownNotice = new TaskCompletionSource(); + var otherNotice = new TaskCompletionSource(); + var instanceA = CreateBus("pod-a"); + var instanceB = CreateBus("pod-b"); + + instanceA.Subscribe(invalidation => ownNotice.TrySetResult(invalidation)); + instanceB.Subscribe(invalidation => otherNotice.TrySetResult(invalidation)); + + instanceA.Publish(ConfigurationInvalidationScope.SystemConfiguration, null); + + // Ordering is per channel, so B having the notice means A would have had it too. + await otherNotice.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + Assert.False(ownNotice.Task.IsCompleted); + } + + private RedisConfigurationInvalidationBus CreateBus(string originId) + { + Environment.SetEnvironmentVariable("JELLYFIN_INSTANCE_ID", originId); + try + { + return new RedisConfigurationInvalidationBus(_redis!, NullLogger.Instance); + } + finally + { + Environment.SetEnvironmentVariable("JELLYFIN_INSTANCE_ID", null); + } + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/RemoteInvalidationApplyTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/RemoteInvalidationApplyTests.cs new file mode 100644 index 0000000000..6766f05276 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/RemoteInvalidationApplyTests.cs @@ -0,0 +1,205 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations; +using Emby.Server.Implementations.Configuration; +using Emby.Server.Implementations.Serialization; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Common.Net; +using MediaBrowser.Controller.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// Applying an invalidation re-raises the same update events a local save raises, and some consumers of +/// those events answer an update by writing - RecordingsManager creating the recording folders for +/// the livetv key, ApplicationHost clearing IsPortAuthorized on a port change. The +/// instance that did not write must not repeat those writes, and no write it is induced into must reach +/// the bus. +/// +public sealed class RemoteInvalidationApplyTests : IDisposable +{ + private readonly string _root; + + /// + /// Initializes a new instance of the class. + /// + public RemoteInvalidationApplyTests() + { + _root = Path.Combine(Path.GetTempPath(), "jf-config-apply-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_root); + } + + /// + public void Dispose() + { + try + { + Directory.Delete(_root, true); + } + catch (IOException) + { + } + } + + /// + /// A consumer that knows nothing about the bus - a plugin, or anything reached transitively from one - + /// can answer an applied invalidation by writing. That write must not become a notice of its own. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task NamedInvalidation_InducingAWriteOnTheReceiver_DoesNotPublishBack() + { + var fabric = new FakeInvalidationBusFabric(); + var instanceA = CreateInstance(fabric, "pod-a"); + var instanceB = await CreateSubscribedInstanceAsync(fabric, "pod-b"); + + var receivedByA = 0; + instanceA.InvalidationBus.Subscribe(_ => Interlocked.Increment(ref receivedByA)); + + var writesByB = 0; + instanceB.NamedConfigurationUpdated += (_, e) => + { + // One shot: the induced write raises the event again on this instance. + if (Interlocked.Increment(ref writesByB) > 1) + { + return; + } + + var configuration = (NetworkConfiguration)instanceB.GetConfiguration(e.Key); + configuration.PublishedServerUriBySubnet = ["10.0.0.0/8=example"]; + instanceB.SaveConfiguration(e.Key, configuration); + }; + + var updated = instanceA.GetNetworkConfiguration(); + updated.EnableRemoteAccess = false; + instanceA.SaveConfiguration(NetworkConfigurationStore.StoreKey, updated); + + Assert.Equal(0, receivedByA); + + // The point of the bus still holds: B is not left on its stale copy. + Assert.False(instanceB.GetNetworkConfiguration().EnableRemoteAccess); + } + + /// + /// The shape of RecordingsManager: a consumer that answers a named configuration update by + /// writing has to be able to tell that the write was another instance's, and skip it. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task NamedInvalidation_WithAWriteTriggeringConsumer_DoesNotDuplicateTheWrite() + { + var fabric = new FakeInvalidationBusFabric(); + var instanceA = CreateInstance(fabric, "pod-a"); + var instanceB = await CreateSubscribedInstanceAsync(fabric, "pod-b"); + + var receivedByA = 0; + instanceA.InvalidationBus.Subscribe(_ => Interlocked.Increment(ref receivedByA)); + + var writesByB = 0; + instanceB.NamedConfigurationUpdated += (_, e) => + { + if (ConfigurationInvalidationContext.IsApplyingRemoteInvalidation) + { + return; + } + + Interlocked.Increment(ref writesByB); + }; + + var updated = instanceA.GetNetworkConfiguration(); + updated.EnableRemoteAccess = false; + instanceA.SaveConfiguration(NetworkConfigurationStore.StoreKey, updated); + + Assert.Equal(0, writesByB); + Assert.Equal(0, receivedByA); + Assert.False(instanceB.GetNetworkConfiguration().EnableRemoteAccess); + + // A read-only consumer is still told, which is what the invalidation exists for. + var refreshes = 0; + instanceB.NamedConfigurationUpdated += (_, _) => Interlocked.Increment(ref refreshes); + + updated.EnableRemoteAccess = true; + instanceA.SaveConfiguration(NetworkConfigurationStore.StoreKey, updated); + + Assert.Equal(1, refreshes); + } + + /// + /// The ApplicationHost.IsPortAuthorized class of consumer: the system configuration event is + /// queued rather than raised inline, so the fix has to survive the hop onto the thread pool. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task SystemInvalidation_InducingAQueuedWriteOnTheReceiver_DoesNotPublishBack() + { + var fabric = new FakeInvalidationBusFabric(); + var instanceA = CreateInstance(fabric, "pod-a"); + var instanceB = await CreateSubscribedInstanceAsync(fabric, "pod-b"); + + var receivedByA = 0; + instanceA.InvalidationBus.Subscribe(_ => Interlocked.Increment(ref receivedByA)); + + var applied = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var handled = 0; + instanceB.ConfigurationUpdated += (_, _) => + { + if (Interlocked.Increment(ref handled) > 1) + { + return; + } + + instanceB.Configuration.IsPortAuthorized = false; + instanceB.SaveConfiguration(); + applied.TrySetResult(); + }; + + instanceA.Configuration.QuickConnectAvailable = false; + instanceA.SaveConfiguration(); + + await applied.Task.WaitAsync(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken); + + Assert.Equal(0, receivedByA); + Assert.False(instanceB.Configuration.QuickConnectAvailable); + } + + private async Task CreateSubscribedInstanceAsync(FakeInvalidationBusFabric fabric, string originId) + { + var instance = CreateInstance(fabric, originId); + var subscriber = new ConfigurationInvalidationSubscriber( + instance.InvalidationBus, + instance, + NullLogger.Instance); + + await subscriber.StartAsync(CancellationToken.None); + return instance; + } + + private ServerConfigurationManager CreateInstance(FakeInvalidationBusFabric fabric, string originId) + { + var paths = new ServerApplicationPaths( + Ensure("data"), + Ensure("log"), + Ensure("config"), + Ensure("cache"), + Ensure("web")); + + var manager = new ServerConfigurationManager(paths, NullLoggerFactory.Instance, new MyXmlSerializer()) + { + InvalidationBus = fabric.Connect(originId) + }; + + manager.AddParts([new NetworkConfigurationFactory()]); + return manager; + } + + private string Ensure(string name) + { + var path = Path.Combine(_root, name); + Directory.CreateDirectory(path); + return path; + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Configuration/SharedConfigurationPropagationTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Configuration/SharedConfigurationPropagationTests.cs new file mode 100644 index 0000000000..115efc6b06 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Configuration/SharedConfigurationPropagationTests.cs @@ -0,0 +1,147 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations; +using Emby.Server.Implementations.Configuration; +using Emby.Server.Implementations.Serialization; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Common.Net; +using MediaBrowser.Controller.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using StackExchange.Redis; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Configuration; + +/// +/// Two independently constructed instances over one configuration +/// directory are the in-process stand-in for two replicas sharing one /config mount: what either of +/// them writes, the other has to pick up without being restarted. +/// +public sealed class SharedConfigurationPropagationTests : IDisposable +{ + private readonly string _root; + + /// + /// Initializes a new instance of the class. + /// + public SharedConfigurationPropagationTests() + { + _root = Path.Combine(Path.GetTempPath(), "jf-config-prop-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_root); + } + + /// + public void Dispose() + { + try + { + Directory.Delete(_root, true); + } + catch (IOException) + { + } + } + + /// + /// A system configuration setting tightened on one replica has to hold on every other replica, not + /// only on the one that served the admin's request. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task SystemConfigurationSavedOnOneInstance_IsSeenByAnotherWithoutRestart() + { + var fabric = new FakeInvalidationBusFabric(); + var instanceA = CreateInstance(fabric, "pod-a"); + var instanceB = await CreateSubscribedInstanceAsync(fabric, "pod-b"); + + // B has the pre-change configuration in hand before A writes, as a running replica would. + Assert.True(instanceB.Configuration.QuickConnectAvailable); + + instanceA.Configuration.QuickConnectAvailable = false; + instanceA.SaveConfiguration(); + + Assert.False(instanceB.Configuration.QuickConnectAvailable); + } + + /// + /// The same has to hold for the named configurations, which are cached per key and never reloaded. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task NamedConfigurationSavedOnOneInstance_IsSeenByAnotherWithoutRestart() + { + var fabric = new FakeInvalidationBusFabric(); + var instanceA = CreateInstance(fabric, "pod-a"); + var instanceB = await CreateSubscribedInstanceAsync(fabric, "pod-b"); + + Assert.True(instanceB.GetNetworkConfiguration().EnableRemoteAccess); + + var updated = instanceA.GetNetworkConfiguration(); + updated.EnableRemoteAccess = false; + instanceA.SaveConfiguration(NetworkConfigurationStore.StoreKey, updated); + + Assert.False(instanceB.GetNetworkConfiguration().EnableRemoteAccess); + } + + /// + /// A replica that cannot reach the bus keeps serving: the admin's save still lands on the shared + /// directory, and the only loss is that the other replicas are not told about it. + /// + [Fact] + public void SaveConfiguration_WithUnreachableBus_DoesNotThrow() + { + using var multiplexer = ConnectionMultiplexer.Connect("127.0.0.1:1,abortConnect=false,connectTimeout=200,connectRetry=1,syncTimeout=200"); + var bus = new RedisConfigurationInvalidationBus(multiplexer, NullLogger.Instance); + + bus.Subscribe(_ => throw new InvalidOperationException("Nothing can be delivered by an unreachable bus.")); + + var instance = CreateInstance(new FakeInvalidationBusFabric(), "pod-a"); + instance.InvalidationBus = bus; + + instance.Configuration.QuickConnectAvailable = false; + instance.SaveConfiguration(); + instance.SaveConfiguration(NetworkConfigurationStore.StoreKey, instance.GetNetworkConfiguration()); + + Assert.False(instance.Configuration.QuickConnectAvailable); + } + + private async Task CreateSubscribedInstanceAsync(FakeInvalidationBusFabric fabric, string originId) + { + var instance = CreateInstance(fabric, originId); + var subscriber = new ConfigurationInvalidationSubscriber( + instance.InvalidationBus, + instance, + NullLogger.Instance); + + await subscriber.StartAsync(CancellationToken.None); + return instance; + } + + private ServerConfigurationManager CreateInstance(FakeInvalidationBusFabric fabric, string originId) + { + // Every instance has its own paths object, all of them pointing at the one shared directory. + var paths = new ServerApplicationPaths( + Ensure("data"), + Ensure("log"), + Ensure("config"), + Ensure("cache"), + Ensure("web")); + + var manager = new ServerConfigurationManager(paths, NullLoggerFactory.Instance, new MyXmlSerializer()) + { + InvalidationBus = fabric.Connect(originId) + }; + + manager.AddParts([new NetworkConfigurationFactory()]); + return manager; + } + + private string Ensure(string name) + { + var path = Path.Combine(_root, name); + Directory.CreateDirectory(path); + return path; + } +}