From 43cdbe856a7d49f44299e7bdc8efb054fc623dd1 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 18 Aug 2026 11:07:31 +0200 Subject: More resilient MusicBrainz lookup --- .../MusicBrainz/MusicBrainzAlbumProvider.cs | 94 ++++++++++------- .../MusicBrainz/MusicBrainzArtistProvider.cs | 33 ++++-- .../MusicBrainz/MusicBrainzQueryExtensions.cs | 112 +++++++++++++++++++++ 3 files changed, 196 insertions(+), 43 deletions(-) create mode 100644 MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzQueryExtensions.cs diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs index 397c916a4f..7dee5fd31d 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzAlbumProvider.cs @@ -14,6 +14,7 @@ using MediaBrowser.Providers.Music; using MetaBrainz.MusicBrainz; using MetaBrainz.MusicBrainz.Interfaces.Entities; using MetaBrainz.MusicBrainz.Interfaces.Searches; +using Microsoft.Extensions.Logging; namespace MediaBrowser.Providers.Plugins.MusicBrainz; @@ -22,6 +23,17 @@ namespace MediaBrowser.Providers.Plugins.MusicBrainz; /// public class MusicBrainzAlbumProvider : IRemoteMetadataProvider, IHasOrder { + private readonly ILogger _logger; + + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public MusicBrainzAlbumProvider(ILogger logger) + { + _logger = logger; + } + /// public string Name => "MusicBrainz"; @@ -32,21 +44,26 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider> GetSearchResults(AlbumInfo searchInfo, CancellationToken cancellationToken) { var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; - var releaseId = searchInfo.GetReleaseId(); - var releaseGroupId = searchInfo.GetReleaseGroupId(); + var releaseId = MusicBrainzQueryExtensions.ParseMusicBrainzId(searchInfo.GetReleaseId(), "release", _logger); + var releaseGroupId = MusicBrainzQueryExtensions.ParseMusicBrainzId(searchInfo.GetReleaseGroupId(), "release group", _logger); - if (!string.IsNullOrEmpty(releaseId)) + if (releaseId is not null) { - var releaseResult = await query.LookupReleaseAsync(new Guid(releaseId), Include.Artists | Include.ReleaseGroups, cancellationToken).ConfigureAwait(false); - return GetReleaseResult(releaseResult).SingleItemAsEnumerable(); + var releaseResult = await query.LookupReleaseOrNullAsync(releaseId.Value, Include.Artists | Include.ReleaseGroups, _logger, cancellationToken).ConfigureAwait(false); + if (releaseResult is not null) + { + return GetReleaseResult(releaseResult).SingleItemAsEnumerable(); + } } - if (!string.IsNullOrEmpty(releaseGroupId)) + if (releaseGroupId is not null) { - 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); + var releaseGroupResult = await query.LookupReleaseGroupOrNullAsync(releaseGroupId.Value, Include.Releases, _logger, cancellationToken).ConfigureAwait(false); + if (releaseGroupResult is not null) + { + // 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); + } } var artistMusicBrainzId = searchInfo.GetMusicBrainzArtistId(); @@ -102,8 +119,11 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider> GetMetadata(AlbumInfo info, CancellationToken cancellationToken) { var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; - var releaseId = info.GetReleaseId(); - var releaseGroupId = info.GetReleaseGroupId(); + var releaseId = MusicBrainzQueryExtensions.ParseMusicBrainzId(info.GetReleaseId(), "release", _logger); + var releaseGroupId = MusicBrainzQueryExtensions.ParseMusicBrainzId(info.GetReleaseGroupId(), "release group", _logger); var result = new MetadataResult { @@ -165,15 +185,15 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider 0 ? releaseGroupLookup.Releases[0].Id.ToString() : null; + var releaseGroupLookup = await query.LookupReleaseGroupOrNullAsync(releaseGroupId.Value, Include.None, _logger, cancellationToken).ConfigureAwait(false); + releaseId = releaseGroupLookup?.Releases?.Count > 0 ? releaseGroupLookup.Releases[0].Id : null; } // If there is no release ID, lookup a release with the info we have - if (string.IsNullOrWhiteSpace(releaseId)) + if (releaseId is null) { var artistMusicBrainzId = info.GetMusicBrainzArtistId(); IRelease? releaseResult = null; @@ -193,55 +213,61 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider public class MusicBrainzArtistProvider : IRemoteMetadataProvider, IHasOrder { + private readonly ILogger _logger; + + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public MusicBrainzArtistProvider(ILogger logger) + { + _logger = logger; + } + /// public string Name => "MusicBrainz"; @@ -32,12 +44,15 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider> GetSearchResults(ArtistInfo searchInfo, CancellationToken cancellationToken) { var query = MusicBrainz.Plugin.Instance!.MusicBrainzQuery; - var artistId = searchInfo.GetMusicBrainzArtistId(); + var artistId = MusicBrainzQueryExtensions.ParseMusicBrainzId(searchInfo.GetMusicBrainzArtistId(), "artist", _logger); - if (!string.IsNullOrWhiteSpace(artistId)) + if (artistId is not null) { - var artistResult = await query.LookupArtistAsync(new Guid(artistId), Include.Aliases, null, null, cancellationToken).ConfigureAwait(false); - return GetResultFromResponse(artistResult).SingleItemAsEnumerable(); + var artistResult = await query.LookupArtistOrNullAsync(artistId.Value, Include.Aliases, _logger, cancellationToken).ConfigureAwait(false); + if (artistResult is not null) + { + return GetResultFromResponse(artistResult).SingleItemAsEnumerable(); + } } if (string.IsNullOrWhiteSpace(searchInfo.Name)) @@ -99,22 +114,22 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider { Item = new MusicArtist() }; - var musicBrainzId = info.GetMusicBrainzArtistId(); + var musicBrainzId = MusicBrainzQueryExtensions.ParseMusicBrainzId(info.GetMusicBrainzArtistId(), "artist", _logger); // If we don't have an id yet, resolve one by name so we can look the artist up. - if (string.IsNullOrWhiteSpace(musicBrainzId)) + if (musicBrainzId is null) { var searchResults = await GetSearchResults(info, cancellationToken).ConfigureAwait(false); - musicBrainzId = searchResults.FirstOrDefault()?.GetProviderId(MetadataProvider.MusicBrainzArtist); + musicBrainzId = MusicBrainzQueryExtensions.ParseMusicBrainzId(searchResults.FirstOrDefault()?.GetProviderId(MetadataProvider.MusicBrainzArtist), "artist", _logger); } - if (string.IsNullOrWhiteSpace(musicBrainzId)) + if (musicBrainzId is null) { return result; } var query = Plugin.Instance!.MusicBrainzQuery; - var artist = await query.LookupArtistAsync(new Guid(musicBrainzId), Include.Genres | Include.Tags, null, null, cancellationToken).ConfigureAwait(false); + var artist = await query.LookupArtistOrNullAsync(musicBrainzId.Value, Include.Genres | Include.Tags, _logger, cancellationToken).ConfigureAwait(false); if (artist is null) { diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzQueryExtensions.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzQueryExtensions.cs new file mode 100644 index 0000000000..f3df41e942 --- /dev/null +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzQueryExtensions.cs @@ -0,0 +1,112 @@ +using System; +using System.Net; +using System.Threading; +using System.Threading.Tasks; +using MetaBrainz.Common; +using MetaBrainz.MusicBrainz; +using MetaBrainz.MusicBrainz.Interfaces.Entities; +using Microsoft.Extensions.Logging; + +namespace MediaBrowser.Providers.Plugins.MusicBrainz; + +/// +/// Helpers for talking to MusicBrainz with identifiers that are not guaranteed to be valid. +/// +internal static class MusicBrainzQueryExtensions +{ + /// + /// Parses a MusicBrainz identifier, which may come from user-supplied tags or NFO files and is therefore not + /// guaranteed to be a valid GUID. + /// + /// The identifier to parse. + /// The type of entity the identifier refers to, used for logging. + /// The logger. + /// The parsed identifier, or if it is missing or malformed. + public static Guid? ParseMusicBrainzId(string? id, string entityType, ILogger logger) + { + if (string.IsNullOrWhiteSpace(id)) + { + return null; + } + + if (!Guid.TryParse(id, out var parsedId)) + { + logger.LogDebug("Ignoring malformed MusicBrainz {EntityType} id {Id}", entityType, id); + return null; + } + + return parsedId; + } + + /// + /// Looks up a release, treating an unknown identifier as missing data rather than an error. + /// + /// The MusicBrainz query client. + /// The release identifier. + /// The additional data to include in the lookup. + /// The logger. + /// The cancellation token. + /// The release, or if MusicBrainz does not have it. + public static Task LookupReleaseOrNullAsync(this Query query, Guid releaseId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupReleaseAsync(releaseId, include, cancellationToken), + "release", + releaseId, + logger); + + /// + /// Looks up a release group, treating an unknown identifier as missing data rather than an error. + /// + /// The MusicBrainz query client. + /// The release group identifier. + /// The additional data to include in the lookup. + /// The logger. + /// The cancellation token. + /// The release group, or if MusicBrainz does not have it. + public static Task LookupReleaseGroupOrNullAsync(this Query query, Guid releaseGroupId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupReleaseGroupAsync(releaseGroupId, include, null, cancellationToken), + "release group", + releaseGroupId, + logger); + + /// + /// Looks up an artist, treating an unknown identifier as missing data rather than an error. + /// + /// The MusicBrainz query client. + /// The artist identifier. + /// The additional data to include in the lookup. + /// The logger. + /// The cancellation token. + /// The artist, or if MusicBrainz does not have it. + public static Task LookupArtistOrNullAsync(this Query query, Guid artistId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupArtistAsync(artistId, include, null, null, cancellationToken), + "artist", + artistId, + logger); + + /// + /// Runs a lookup, mapping a "not found" response to . Identifiers stored on a library item + /// can refer to entities that no longer exist in MusicBrainz, which is not an error worth failing a refresh over. + /// + /// The type of entity being looked up. + /// The lookup to run. + /// The type of entity being looked up, used for logging. + /// The identifier being looked up, used for logging. + /// The logger. + /// The entity, or if MusicBrainz does not have it. + private static async Task NotFoundAsNullAsync(Func> lookup, string entityType, Guid id, ILogger logger) + where T : class + { + try + { + return await lookup().ConfigureAwait(false); + } + catch (HttpError ex) when (ex.Status == HttpStatusCode.NotFound) + { + logger.LogDebug("MusicBrainz has no {EntityType} with id {Id}", entityType, id); + return null; + } + } +} -- cgit v1.2.3