diff options
3 files changed, 196 insertions, 43 deletions
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; /// </summary> public class MusicBrainzAlbumProvider : IRemoteMetadataProvider<MusicAlbum, AlbumInfo>, IHasOrder { + private readonly ILogger<MusicBrainzAlbumProvider> _logger; + + /// <summary> + /// Initializes a new instance of the <see cref="MusicBrainzAlbumProvider"/> class. + /// </summary> + /// <param name="logger">The logger.</param> + public MusicBrainzAlbumProvider(ILogger<MusicBrainzAlbumProvider> logger) + { + _logger = logger; + } + /// <inheritdoc /> public string Name => "MusicBrainz"; @@ -32,21 +44,26 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider<MusicAlbum, Albu public async Task<IEnumerable<RemoteSearchResult>> 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<MusicAlbum, Albu foreach (var result in releaseSearchResults) { // Fetch full release info, otherwise artists are missing - var fullResult = await query.LookupReleaseAsync(result.Id, Include.Artists | Include.ReleaseGroups, cancellationToken).ConfigureAwait(false); - yield return GetReleaseResult(fullResult); + var fullResult = await query.LookupReleaseOrNullAsync(result.Id, Include.Artists | Include.ReleaseGroups, _logger, cancellationToken).ConfigureAwait(false); + if (fullResult is not null) + { + yield return GetReleaseResult(fullResult); + } } } @@ -156,8 +176,8 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider<MusicAlbum, Albu public async Task<MetadataResult<MusicAlbum>> 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<MusicAlbum> { @@ -165,15 +185,15 @@ public class MusicBrainzAlbumProvider : IRemoteMetadataProvider<MusicAlbum, Albu }; // If there is a release group, but no release ID, try to match the release - if (string.IsNullOrWhiteSpace(releaseId) && !string.IsNullOrWhiteSpace(releaseGroupId)) + if (releaseId is null && releaseGroupId is not null) { // TODO: Actually try to match the release. Simply taking the first result is stupid. - var releaseGroupLookup = await query.LookupReleaseGroupAsync(new Guid(releaseGroupId), Include.None, null, cancellationToken).ConfigureAwait(false); - releaseId = releaseGroupLookup.Releases?.Count > 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<MusicAlbum, Albu if (releaseResult is not null) { - releaseId = releaseResult.Id.ToString(); + releaseId = releaseResult.Id; if (releaseResult.ReleaseGroup?.Id is not null) { - releaseGroupId = releaseResult.ReleaseGroup.Id.ToString(); + releaseGroupId = releaseResult.ReleaseGroup.Id; } } } - if (string.IsNullOrWhiteSpace(releaseId) && string.IsNullOrWhiteSpace(releaseGroupId)) + if (releaseId is null && releaseGroupId is null) { return result; } // Fetch the full release (and its release group) so we can populate everything MusicBrainz returns. IRelease? release = null; - if (!string.IsNullOrWhiteSpace(releaseId)) + if (releaseId is not null) { - release = await query.LookupReleaseAsync( - new Guid(releaseId), + release = await query.LookupReleaseOrNullAsync( + releaseId.Value, Include.Artists | Include.ReleaseGroups | Include.Labels | Include.Genres | Include.Tags, + _logger, cancellationToken).ConfigureAwait(false); - if (string.IsNullOrWhiteSpace(releaseGroupId) && release?.ReleaseGroup?.Id is not null) + if (releaseGroupId is null && release?.ReleaseGroup?.Id is not null) { - releaseGroupId = release.ReleaseGroup.Id.ToString(); + releaseGroupId = release.ReleaseGroup.Id; } } IReleaseGroup? releaseGroup = null; - if (!string.IsNullOrWhiteSpace(releaseGroupId)) + if (releaseGroupId is not null) { - releaseGroup = await query.LookupReleaseGroupAsync( - new Guid(releaseGroupId), + releaseGroup = await query.LookupReleaseGroupOrNullAsync( + releaseGroupId.Value, Include.Artists | Include.Genres | Include.Tags, - null, + _logger, cancellationToken).ConfigureAwait(false); } + if (release is null && releaseGroup is null) + { + return result; + } + result.HasMetadata = true; - if (!string.IsNullOrEmpty(releaseId)) + if (releaseId is not null) { - result.Item.SetProviderId(MetadataProvider.MusicBrainzAlbum, releaseId); + result.Item.SetProviderId(MetadataProvider.MusicBrainzAlbum, releaseId.Value.ToString()); } - if (!string.IsNullOrEmpty(releaseGroupId)) + if (releaseGroupId is not null) { - result.Item.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, releaseGroupId); + result.Item.SetProviderId(MetadataProvider.MusicBrainzReleaseGroup, releaseGroupId.Value.ToString()); } Populate(result.Item, release, releaseGroup); diff --git a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs index a9e950fb64..c3d13ed42c 100644 --- a/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs +++ b/MediaBrowser.Providers/Plugins/MusicBrainz/MusicBrainzArtistProvider.cs @@ -13,6 +13,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; @@ -21,6 +22,17 @@ namespace MediaBrowser.Providers.Plugins.MusicBrainz; /// </summary> public class MusicBrainzArtistProvider : IRemoteMetadataProvider<MusicArtist, ArtistInfo>, IHasOrder { + private readonly ILogger<MusicBrainzArtistProvider> _logger; + + /// <summary> + /// Initializes a new instance of the <see cref="MusicBrainzArtistProvider"/> class. + /// </summary> + /// <param name="logger">The logger.</param> + public MusicBrainzArtistProvider(ILogger<MusicBrainzArtistProvider> logger) + { + _logger = logger; + } + /// <inheritdoc /> public string Name => "MusicBrainz"; @@ -32,12 +44,15 @@ public class MusicBrainzArtistProvider : IRemoteMetadataProvider<MusicArtist, Ar public async Task<IEnumerable<RemoteSearchResult>> 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<MusicArtist, Ar { var result = new MetadataResult<MusicArtist> { 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; + +/// <summary> +/// Helpers for talking to MusicBrainz with identifiers that are not guaranteed to be valid. +/// </summary> +internal static class MusicBrainzQueryExtensions +{ + /// <summary> + /// Parses a MusicBrainz identifier, which may come from user-supplied tags or NFO files and is therefore not + /// guaranteed to be a valid GUID. + /// </summary> + /// <param name="id">The identifier to parse.</param> + /// <param name="entityType">The type of entity the identifier refers to, used for logging.</param> + /// <param name="logger">The logger.</param> + /// <returns>The parsed identifier, or <see langword="null"/> if it is missing or malformed.</returns> + 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; + } + + /// <summary> + /// Looks up a release, treating an unknown identifier as missing data rather than an error. + /// </summary> + /// <param name="query">The MusicBrainz query client.</param> + /// <param name="releaseId">The release identifier.</param> + /// <param name="include">The additional data to include in the lookup.</param> + /// <param name="logger">The logger.</param> + /// <param name="cancellationToken">The cancellation token.</param> + /// <returns>The release, or <see langword="null"/> if MusicBrainz does not have it.</returns> + public static Task<IRelease?> LookupReleaseOrNullAsync(this Query query, Guid releaseId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupReleaseAsync(releaseId, include, cancellationToken), + "release", + releaseId, + logger); + + /// <summary> + /// Looks up a release group, treating an unknown identifier as missing data rather than an error. + /// </summary> + /// <param name="query">The MusicBrainz query client.</param> + /// <param name="releaseGroupId">The release group identifier.</param> + /// <param name="include">The additional data to include in the lookup.</param> + /// <param name="logger">The logger.</param> + /// <param name="cancellationToken">The cancellation token.</param> + /// <returns>The release group, or <see langword="null"/> if MusicBrainz does not have it.</returns> + public static Task<IReleaseGroup?> LookupReleaseGroupOrNullAsync(this Query query, Guid releaseGroupId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupReleaseGroupAsync(releaseGroupId, include, null, cancellationToken), + "release group", + releaseGroupId, + logger); + + /// <summary> + /// Looks up an artist, treating an unknown identifier as missing data rather than an error. + /// </summary> + /// <param name="query">The MusicBrainz query client.</param> + /// <param name="artistId">The artist identifier.</param> + /// <param name="include">The additional data to include in the lookup.</param> + /// <param name="logger">The logger.</param> + /// <param name="cancellationToken">The cancellation token.</param> + /// <returns>The artist, or <see langword="null"/> if MusicBrainz does not have it.</returns> + public static Task<IArtist?> LookupArtistOrNullAsync(this Query query, Guid artistId, Include include, ILogger logger, CancellationToken cancellationToken) + => NotFoundAsNullAsync( + () => query.LookupArtistAsync(artistId, include, null, null, cancellationToken), + "artist", + artistId, + logger); + + /// <summary> + /// Runs a lookup, mapping a "not found" response to <see langword="null"/>. 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. + /// </summary> + /// <typeparam name="T">The type of entity being looked up.</typeparam> + /// <param name="lookup">The lookup to run.</param> + /// <param name="entityType">The type of entity being looked up, used for logging.</param> + /// <param name="id">The identifier being looked up, used for logging.</param> + /// <param name="logger">The logger.</param> + /// <returns>The entity, or <see langword="null"/> if MusicBrainz does not have it.</returns> + private static async Task<T?> NotFoundAsNullAsync<T>(Func<Task<T>> 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; + } + } +} |