From d3e6079d38723c60ea49a77f621cb6660bb6b768 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 5 May 2026 17:43:37 +0200 Subject: [PATCH] Move MusicBrainz Query client to plugin instance --- .../MusicBrainz/MusicBrainzAlbumProvider.cs | 83 +++----------- .../MusicBrainz/MusicBrainzArtistProvider.cs | 69 +----------- .../Plugins/MusicBrainz/Plugin.cs | 102 +++++++++++++++++- 3 files changed, 116 insertions(+), 138 deletions(-) diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs index 88c8e4f7c9..715bdd9da4 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs @@ -9,81 +9,41 @@ using Jellyfin.Extensions; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; -using MediaBrowser.Model.Plugins; using MediaBrowser.Model.Providers; using MediaBrowser.Providers.Music; -using MediaBrowser.Providers.Plugins.MusicBrainz.Configuration; using MetaBrainz.MusicBrainz; using MetaBrainz.MusicBrainz.Interfaces.Entities; using MetaBrainz.MusicBrainz.Interfaces.Searches; -using Microsoft.Extensions.Logging; namespace MediaBrowser.Providers.Plugins.MusicBrainz; /// /// Music album metadata provider for MusicBrainz. /// -public class MusicBrainzAlbumProvider : IRemoteMetadataProvider, IHasOrder, IDisposable +public class MusicBrainzAlbumProvider : IRemoteMetadataProvider, IHasOrder { - private readonly ILogger _logger; - private Query _musicBrainzQuery; - - /// - /// Initializes a new instance of the class. - /// - /// The logger. - public MusicBrainzAlbumProvider(ILogger logger) - { - _logger = logger; - _musicBrainzQuery = new Query(); - ReloadConfig(null, MusicBrainz.Plugin.Instance!.Configuration); - MusicBrainz.Plugin.Instance!.ConfigurationChanged += ReloadConfig; - } - /// public string Name => "MusicBrainz"; /// public int Order => 0; - private void ReloadConfig(object? sender, BasePluginConfiguration e) - { - var configuration = (PluginConfiguration)e; - if (Uri.TryCreate(configuration.Server, UriKind.Absolute, out var server)) - { - Query.DefaultServer = server.DnsSafeHost; - Query.DefaultPort = server.Port; - Query.DefaultUrlScheme = server.Scheme; - } - else - { - // Fallback to official server - _logger.LogWarning("Invalid MusicBrainz server specified, falling back to official server"); - var defaultServer = new Uri(PluginConfiguration.DefaultServer); - Query.DefaultServer = defaultServer.Host; - Query.DefaultPort = defaultServer.Port; - Query.DefaultUrlScheme = defaultServer.Scheme; - } - - Query.DelayBetweenRequests = configuration.RateLimit; - _musicBrainzQuery = new Query(); - } - /// public async Task> GetSearchResults(AlbumInfo searchInfo, CancellationToken cancellationToken) { + var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; var releaseId = searchInfo.GetReleaseId(); var releaseGroupId = searchInfo.GetReleaseGroupId(); if (!string.IsNullOrEmpty(releaseId)) { - var releaseResult = await _musicBrainzQuery.LookupReleaseAsync(new Guid(releaseId), Include.Artists | Include.ReleaseGroups, cancellationToken).ConfigureAwait(false); + var releaseResult = await query.LookupReleaseAsync(new Guid(releaseId), Include.Artists | Include.ReleaseGroups, cancellationToken).ConfigureAwait(false); return GetReleaseResult(releaseResult).SingleItemAsEnumerable(); } if (!string.IsNullOrEmpty(releaseGroupId)) { - var releaseGroupResult = await _musicBrainzQuery.LookupReleaseGroupAsync(new Guid(releaseGroupId), Include.Releases, null, cancellationToken).ConfigureAwait(false); + var releaseGroupResult = await query.LookupReleaseGroupAsync(new Guid(releaseGroupId), Include.Releases, null, cancellationToken).ConfigureAwait(false); // No need to pass the cancellation token to GetReleaseGroupResultAsync as we're already passing it to ToBlockingEnumerable return GetReleaseGroupResultAsync(releaseGroupResult.Releases, CancellationToken.None).ToBlockingEnumerable(cancellationToken); @@ -93,7 +53,7 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider 0) @@ -106,7 +66,7 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider 0) @@ -138,10 +98,11 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider> GetMetadata(AlbumInfo info, CancellationToken cancellationToken) { // TODO: This sets essentially nothing. As-is, it's mostly useless. Make it actually pull metadata and use it. + var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; var releaseId = info.GetReleaseId(); var releaseGroupId = info.GetReleaseGroupId(); @@ -207,7 +169,7 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider 0 ? releaseGroup.Releases[0] : null; if (release is not null) { @@ -224,13 +186,13 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider 0 ? releaseSearchResults.Results[0].Item : null; } else if (!string.IsNullOrEmpty(info.GetAlbumArtist())) { - var releaseSearchResults = await _musicBrainzQuery.FindReleasesAsync($"\"{info.Name}\" AND artist:{info.GetAlbumArtist()}", null, null, false, cancellationToken) + var releaseSearchResults = await query.FindReleasesAsync($"\"{info.Name}\" AND artist:{info.GetAlbumArtist()}", null, null, false, cancellationToken) .ConfigureAwait(false); releaseResult = releaseSearchResults.Results.Count > 0 ? releaseSearchResults.Results[0].Item : null; } @@ -253,7 +215,7 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - /// - /// Dispose all resources. - /// - /// Whether to dispose. - protected virtual void Dispose(bool disposing) - { - if (disposing) - { - _musicBrainzQuery.Dispose(); - } - } } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs index 9df21596c5..0fe4e6bb16 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs @@ -8,37 +8,19 @@ using Jellyfin.Extensions; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; -using MediaBrowser.Model.Plugins; using MediaBrowser.Model.Providers; using MediaBrowser.Providers.Music; -using MediaBrowser.Providers.Plugins.MusicBrainz.Configuration; using MetaBrainz.MusicBrainz; using MetaBrainz.MusicBrainz.Interfaces.Entities; using MetaBrainz.MusicBrainz.Interfaces.Searches; -using Microsoft.Extensions.Logging; namespace MediaBrowser.Providers.Plugins.MusicBrainz; /// /// MusicBrainz artist provider. /// -public class MusicBrainzArtistProvider : IRemoteMetadataProvider, IDisposable, IHasOrder +public class MusicBrainzArtistProvider : IRemoteMetadataProvider, IHasOrder { - private readonly ILogger _logger; - private Query _musicBrainzQuery; - - /// - /// Initializes a new instance of the class. - /// - /// The logger. - public MusicBrainzArtistProvider(ILogger logger) - { - _logger = logger; - _musicBrainzQuery = new Query(); - ReloadConfig(null, MusicBrainz.Plugin.Instance!.Configuration); - MusicBrainz.Plugin.Instance!.ConfigurationChanged += ReloadConfig; - } - /// public string Name => "MusicBrainz"; @@ -46,41 +28,19 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider 0; - private void ReloadConfig(object? sender, BasePluginConfiguration e) - { - var configuration = (PluginConfiguration)e; - if (Uri.TryCreate(configuration.Server, UriKind.Absolute, out var server)) - { - Query.DefaultServer = server.DnsSafeHost; - Query.DefaultPort = server.Port; - Query.DefaultUrlScheme = server.Scheme; - } - else - { - // Fallback to official server - _logger.LogWarning("Invalid MusicBrainz server specified, falling back to official server"); - var defaultServer = new Uri(PluginConfiguration.DefaultServer); - Query.DefaultServer = defaultServer.Host; - Query.DefaultPort = defaultServer.Port; - Query.DefaultUrlScheme = defaultServer.Scheme; - } - - Query.DelayBetweenRequests = configuration.RateLimit; - _musicBrainzQuery = new Query(); - } - /// public async Task> GetSearchResults(ArtistInfo searchInfo, CancellationToken cancellationToken) { + var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; var artistId = searchInfo.GetMusicBrainzArtistId(); if (!string.IsNullOrWhiteSpace(artistId)) { - var artistResult = await _musicBrainzQuery.LookupArtistAsync(new Guid(artistId), Include.Aliases, null, null, cancellationToken).ConfigureAwait(false); + var artistResult = await query.LookupArtistAsync(new Guid(artistId), Include.Aliases, null, null, cancellationToken).ConfigureAwait(false); return GetResultFromResponse(artistResult).SingleItemAsEnumerable(); } - var artistSearchResults = await _musicBrainzQuery.FindArtistsAsync($"\"{searchInfo.Name}\"", null, null, false, cancellationToken) + var artistSearchResults = await query.FindArtistsAsync($"\"{searchInfo.Name}\"", null, null, false, cancellationToken) .ConfigureAwait(false); if (artistSearchResults.Results.Count > 0) { @@ -90,7 +50,7 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider 0) { @@ -168,23 +128,4 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - /// - /// Dispose all resources. - /// - /// Whether to dispose. - protected virtual void Dispose(bool disposing) - { - if (disposing) - { - _musicBrainzQuery.Dispose(); - } - } } diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs index 39cfd727f3..69225d0b95 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/Plugin.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Net.Http.Headers; +using System.Threading; using MediaBrowser.Common; using MediaBrowser.Common.Configuration; using MediaBrowser.Common.Plugins; @@ -8,30 +10,42 @@ using MediaBrowser.Model.Plugins; using MediaBrowser.Model.Serialization; using MediaBrowser.Providers.Plugins.MusicBrainz.Configuration; using MetaBrainz.MusicBrainz; +using Microsoft.Extensions.Logging; namespace MediaBrowser.Providers.Plugins.MusicBrainz; /// /// Plugin instance. /// -public class Plugin : BasePlugin, IHasWebPages +public class Plugin : BasePlugin, IHasWebPages, IDisposable { + private readonly ILogger _logger; + private readonly Lock _queryLock = new(); + private Query _musicBrainzQuery; + private bool _disposed; + /// /// Initializes a new instance of the class. /// /// Instance of the interface. /// Instance of the interface. /// Instance of the interface. - public Plugin(IApplicationPaths applicationPaths, IXmlSerializer xmlSerializer, IApplicationHost applicationHost) + /// Instance of the interface. + public Plugin(IApplicationPaths applicationPaths, IXmlSerializer xmlSerializer, IApplicationHost applicationHost, ILogger logger) : base(applicationPaths, xmlSerializer) { Instance = this; + _logger = logger; // TODO: Change this to "JellyfinMusicBrainzPlugin" once we take it out of the server repo. Query.DefaultUserAgent.Add(new ProductInfoHeaderValue(applicationHost.Name.Replace(' ', '-'), applicationHost.ApplicationVersionString)); Query.DefaultUserAgent.Add(new ProductInfoHeaderValue($"({applicationHost.ApplicationUserAgentAddress})")); - Query.DelayBetweenRequests = Instance.Configuration.RateLimit; - Query.DefaultServer = Instance.Configuration.Server; + + ApplyServerConfig(Configuration); + Query.DelayBetweenRequests = Configuration.RateLimit; + _musicBrainzQuery = new Query(); + + ConfigurationChanged += OnConfigurationChanged; } /// @@ -52,6 +66,25 @@ public class Plugin : BasePlugin, IHasWebPages // TODO remove when plugin removed from server. public override string ConfigurationFileName => "Jellyfin.Plugin.MusicBrainz.xml"; + /// + /// Gets the current MusicBrainz query client. + /// + /// + /// Always read this property anew before each request — the underlying instance is + /// replaced when the server URL changes. Old instances are intentionally left alive + /// so in-flight requests can finish; their unmanaged resources leak until GC. + /// + public Query MusicBrainzQuery + { + get + { + lock (_queryLock) + { + return _musicBrainzQuery; + } + } + } + /// public IEnumerable GetPages() { @@ -61,4 +94,65 @@ public class Plugin : BasePlugin, IHasWebPages EmbeddedResourcePath = GetType().Namespace + ".Configuration.config.html" }; } + + /// + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Releases unmanaged and managed resources. + /// + /// Whether to dispose managed resources. + protected virtual void Dispose(bool disposing) + { + if (_disposed) + { + return; + } + + if (disposing) + { + ConfigurationChanged -= OnConfigurationChanged; + lock (_queryLock) + { + _musicBrainzQuery.Dispose(); + } + } + + _disposed = true; + } + + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP003:Dispose previous before re-assigning", Justification = "The previous Query may still be in use by in-flight async requests; disposing it would cause ObjectDisposedException. The orphan is intentionally left for GC.")] + private void OnConfigurationChanged(object? sender, BasePluginConfiguration e) + { + var configuration = (PluginConfiguration)e; + ApplyServerConfig(configuration); + Query.DelayBetweenRequests = configuration.RateLimit; + + lock (_queryLock) + { + _musicBrainzQuery = new Query(); + } + } + + private void ApplyServerConfig(PluginConfiguration configuration) + { + if (Uri.TryCreate(configuration.Server, UriKind.Absolute, out var server)) + { + Query.DefaultServer = server.DnsSafeHost; + Query.DefaultPort = server.Port; + Query.DefaultUrlScheme = server.Scheme; + } + else + { + _logger.LogWarning("Invalid MusicBrainz server specified, falling back to official server"); + var defaultServer = new Uri(PluginConfiguration.DefaultServer); + Query.DefaultServer = defaultServer.Host; + Query.DefaultPort = defaultServer.Port; + Query.DefaultUrlScheme = defaultServer.Scheme; + } + } }