diff options
7 files changed, 285 insertions, 59 deletions
diff --git a/Jellyfin.Api/Controllers/ItemLookupController.cs b/Jellyfin.Api/Controllers/ItemLookupController.cs index d009f80a96..39ba5ab186 100644 --- a/Jellyfin.Api/Controllers/ItemLookupController.cs +++ b/Jellyfin.Api/Controllers/ItemLookupController.cs @@ -13,6 +13,7 @@ using MediaBrowser.Controller.Entities.Movies; using MediaBrowser.Controller.Entities.TV; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Providers; +using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; using MediaBrowser.Model.Providers; using Microsoft.AspNetCore.Authorization; @@ -263,7 +264,7 @@ public class ItemLookupController : BaseJellyfinApiController searchResult.ProviderIds); // Since the refresh process won't erase provider Ids, we need to set this explicitly now. - item.ProviderIds = searchResult.ProviderIds; + item.SetProviderIds(searchResult.ProviderIds); await _providerManager.RefreshFullItem( item, new MetadataRefreshOptions(new DirectoryService(_fileSystem)) diff --git a/Jellyfin.Api/Controllers/ItemUpdateController.cs b/Jellyfin.Api/Controllers/ItemUpdateController.cs index 36c82cf461..65fffc4181 100644 --- a/Jellyfin.Api/Controllers/ItemUpdateController.cs +++ b/Jellyfin.Api/Controllers/ItemUpdateController.cs @@ -428,15 +428,7 @@ public class ItemUpdateController : BaseJellyfinApiController if (request.ProviderIds is not null) { - foreach (var pair in request.ProviderIds.ToList()) - { - if (string.IsNullOrEmpty(pair.Value)) - { - request.ProviderIds.Remove(pair.Key); - } - } - - item.ProviderIds = request.ProviderIds; + item.SetProviderIds(request.ProviderIds); } if (item is Video video) diff --git a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs index 27d7a4654b..09eba92d9e 100644 --- a/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs +++ b/MediaBrowser.Model/Entities/ProviderIdsExtensions.cs @@ -159,8 +159,15 @@ public static partial class ProviderIdsExtensions // When name contains a '=' it can't be deserialized from the database if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(value) - || name.Contains('=', StringComparison.Ordinal) - || !IsValidProviderId(name, value)) + || name.Contains('=', StringComparison.Ordinal)) + { + return false; + } + + name = name.Trim(); + value = value.Trim(); + + if (!IsValidProviderId(name, value)) { return false; } @@ -197,7 +204,6 @@ public static partial class ProviderIdsExtensions /// <param name="instance">The instance.</param> /// <param name="name">The name, this should not contain a '=' character.</param> /// <param name="value">The value.</param> - /// <remarks>Due to how deserialization from the database works the name cannot contain '='.</remarks> public static void SetProviderId(this IHasProviderIds instance, string name, string value) { ArgumentNullException.ThrowIfNull(instance); @@ -210,17 +216,27 @@ public static partial class ProviderIdsExtensions throw new ArgumentException("Provider id name cannot contain '='", nameof(name)); } - // Ensure it exists - instance.ProviderIds ??= new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); + instance.TrySetProviderId(name, value); + } - // Match on internal MetadataProvider enum string values before adding arbitrary providers - if (_metadataProviderEnumDictionary.TryGetValue(name, out var enumValue)) + /// <summary> + /// Replaces all provider ids, dropping the ones that cannot belong to the provider they are filed under. + /// </summary> + /// <param name="instance">The instance.</param> + /// <param name="providerIds">The provider ids to set.</param> + public static void SetProviderIds(this IHasProviderIds instance, IReadOnlyDictionary<string, string>? providerIds) + { + ArgumentNullException.ThrowIfNull(instance); + + instance.ProviderIds = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); + if (providerIds is null) { - instance.ProviderIds[enumValue] = value; + return; } - else + + foreach (var (name, value) in providerIds) { - instance.ProviderIds[name] = value; + instance.TrySetProviderId(name, value); } } @@ -259,7 +275,7 @@ public static partial class ProviderIdsExtensions } private static bool IsPositiveNumber(string value) - => long.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out var id) && id > 0; + => int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out var id) && id > 0; private static bool IsGuid(string value) => Guid.TryParse(value, CultureInfo.InvariantCulture, out _); diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index c6c15198be..d11db8f531 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -260,21 +260,40 @@ namespace MediaBrowser.Providers.Manager switch (lookupInfo) { case EpisodeInfo episodeInfo: - episodeInfo.SeriesProviderIds = result.ProviderIds; + episodeInfo.SeriesProviderIds = GetValidProviderIds(result.ProviderIds); episodeInfo.ProviderIds.Clear(); break; case SeasonInfo seasonInfo: - seasonInfo.SeriesProviderIds = result.ProviderIds; + seasonInfo.SeriesProviderIds = GetValidProviderIds(result.ProviderIds); seasonInfo.ProviderIds.Clear(); break; default: - lookupInfo.ProviderIds = result.ProviderIds; + lookupInfo.SetProviderIds(result.ProviderIds); lookupInfo.Name = result.Name; lookupInfo.Year = result.ProductionYear; break; } } + private static Dictionary<string, string> GetValidProviderIds(IReadOnlyDictionary<string, string> providerIds) + { + var validProviderIds = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); + if (providerIds is null) + { + return validProviderIds; + } + + foreach (var (name, value) in providerIds) + { + if (ProviderIdsExtensions.IsValidProviderId(name, value)) + { + validProviderIds[name] = value; + } + } + + return validProviderIds; + } + protected async Task SaveItemAsync(MetadataResult<TItemType> result, ItemUpdateType reason, bool reattachUserData, CancellationToken cancellationToken) { await result.Item.UpdateToRepositoryAsync(reason, cancellationToken).ConfigureAwait(false); @@ -835,6 +854,7 @@ namespace MediaBrowser.Providers.Manager } } + var hasRemoteMetadata = false; var isLocalLocked = temp.Item.IsLocked; if (!isLocalLocked && (options.ReplaceAllMetadata || options.MetadataRefreshMode > MetadataRefreshMode.ValidationOnly)) { @@ -849,6 +869,7 @@ namespace MediaBrowser.Providers.Manager var remoteResult = await ExecuteRemoteProviders(temp, logName, false, id, remoteProviders, cancellationToken).ConfigureAwait(false); + hasRemoteMetadata = remoteResult.UpdateType.HasFlag(ItemUpdateType.MetadataDownload); refreshResult.UpdateType |= remoteResult.UpdateType; refreshResult.ErrorMessage = remoteResult.ErrorMessage; refreshResult.Failures += remoteResult.Failures; @@ -858,10 +879,12 @@ namespace MediaBrowser.Providers.Manager { if (refreshResult.UpdateType > ItemUpdateType.None) { - // A provider that failed contributed nothing, so the result is not the complete - // replacement the caller asked for. Keeping the existing values stops a provider being - // temporarily unreachable, or choking on a bad id, from deleting the data it owns. - if (!options.RemoveOldMetadata || refreshResult.Failures > 0) + // Erasing the old values is only safe when a remote provider returned something to + // replace them with. If every one of them failed there is no replacement, and wiping the + // item would turn a provider being temporarily unreachable into permanent data loss. + // A single failure is not enough: Identify asks for the erasure precisely because the + // previous match was wrong, and an unrelated provider throwing must not undo that. + if (!options.RemoveOldMetadata || (refreshResult.Failures > 0 && !hasRemoteMetadata)) { // Add existing metadata to provider result if it does not exist there MergeData(metadata, temp, [], false, false); @@ -935,7 +958,7 @@ namespace MediaBrowser.Providers.Manager { result.Provider = provider.Name; - LogInvalidProviderIds(result.Item, providerName, logName); + LogInvalidProviderIds(result, providerName, logName); MergeData(result, temp, [], replaceData, false); MergeNewData(temp.Item, id); @@ -969,19 +992,48 @@ namespace MediaBrowser.Providers.Manager /// The ids are dropped when merging, this names the provider that produced them so the source of a /// recurring bad id can be found. /// </remarks> - private void LogInvalidProviderIds(TItemType item, string providerName, string logName) + private void LogInvalidProviderIds(MetadataResult<TItemType> result, string providerName, string logName) + { + if (!Logger.IsEnabled(LogLevel.Debug)) + { + return; + } + + LogInvalidProviderIds(result.Item?.ProviderIds, providerName, logName, null); + + if (result.People is null) + { + return; + } + + foreach (var person in result.People) + { + LogInvalidProviderIds(person.ProviderIds, providerName, logName, person.Name); + } + } + + private void LogInvalidProviderIds(IReadOnlyDictionary<string, string> providerIds, string providerName, string logName, string personName) { - if (item?.ProviderIds is null || !Logger.IsEnabled(LogLevel.Debug)) + if (providerIds is null) { return; } - foreach (var (key, value) in item.ProviderIds) + foreach (var (key, value) in providerIds) { - if (!ProviderIdsExtensions.IsValidProviderId(key, value)) + if (ProviderIdsExtensions.IsValidProviderId(key, value)) + { + continue; + } + + if (personName is null) { Logger.LogDebug("Discarding {Key} id '{Value}' returned by {Provider} for {Item}", key, value, providerName, logName); } + else + { + Logger.LogDebug("Discarding {Key} id '{Value}' returned by {Provider} for {Person} of {Item}", key, value, providerName, personName, logName); + } } } @@ -997,8 +1049,13 @@ namespace MediaBrowser.Providers.Manager continue; } - // Don't replace existing Id's. - lookupInfo.ProviderIds.TryAdd(key, providerId.Value); + // Don't replace existing Id's, unless the one already there is unusable - handing that + // one to the providers that have yet to run is what makes them fail. + if (!lookupInfo.ProviderIds.TryGetValue(key, out var existingId) + || !ProviderIdsExtensions.IsValidProviderId(key, existingId)) + { + lookupInfo.ProviderIds[key] = providerId.Value; + } } } @@ -1138,6 +1195,7 @@ namespace MediaBrowser.Providers.Manager if (!lockedFields.Contains(MetadataField.Cast)) { RemoveInvalidProviderIds(sourceResult.People); + RemoveInvalidProviderIds(targetResult.People); if (replaceData || targetResult.People is null || targetResult.People.Count == 0) { @@ -1217,15 +1275,24 @@ namespace MediaBrowser.Providers.Manager continue; } - // Don't replace existing Id's. - if (replaceData) + // Don't replace existing Id's, unless the stored one is unusable - that one is the bad + // match the refresh is meant to repair. + if (replaceData + || !target.ProviderIds.TryGetValue(key, out var existingId) + || !ProviderIdsExtensions.IsValidProviderId(key, existingId)) { target.ProviderIds[key] = id.Value; } - else - { - target.ProviderIds.TryAdd(key, id.Value); - } + } + + // A bad id no provider offered a replacement for still has to go, otherwise the item keeps + // failing the same way on every refresh. + foreach (var key in target.ProviderIds + .Where(id => !ProviderIdsExtensions.IsValidProviderId(id.Key, id.Value)) + .Select(id => id.Key) + .ToArray()) + { + target.ProviderIds.Remove(key); } if (replaceData || !target.CriticRating.HasValue) diff --git a/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs b/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs index d50e2c6c11..2923dd3290 100644 --- a/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs +++ b/MediaBrowser.Providers/Music/AlbumInfoExtensions.cs @@ -1,7 +1,5 @@ #pragma warning disable CS1591 -using System; -using System.Globalization; using System.Linq; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; @@ -25,11 +23,11 @@ namespace MediaBrowser.Providers.Music public static string? GetReleaseGroupId(this AlbumInfo info) { - var id = MusicBrainzId(info.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup)); + var id = MusicBrainzId(MetadataProvider.MusicBrainzReleaseGroup, info.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup)); if (string.IsNullOrEmpty(id)) { - return info.SongInfos.Select(i => MusicBrainzId(i.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup))) + return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzReleaseGroup, i.GetProviderId(MetadataProvider.MusicBrainzReleaseGroup))) .FirstOrDefault(i => !string.IsNullOrEmpty(i)); } @@ -38,11 +36,11 @@ namespace MediaBrowser.Providers.Music public static string? GetReleaseId(this AlbumInfo info) { - var id = MusicBrainzId(info.GetProviderId(MetadataProvider.MusicBrainzAlbum)); + var id = MusicBrainzId(MetadataProvider.MusicBrainzAlbum, info.GetProviderId(MetadataProvider.MusicBrainzAlbum)); if (string.IsNullOrEmpty(id)) { - return info.SongInfos.Select(i => MusicBrainzId(i.GetProviderId(MetadataProvider.MusicBrainzAlbum))) + return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbum, i.GetProviderId(MetadataProvider.MusicBrainzAlbum))) .FirstOrDefault(i => !string.IsNullOrEmpty(i)); } @@ -52,17 +50,17 @@ namespace MediaBrowser.Providers.Music public static string? GetMusicBrainzArtistId(this AlbumInfo info) { info.ProviderIds.TryGetValue(MetadataProvider.MusicBrainzAlbumArtist.ToString(), out string? id); - id = MusicBrainzId(id); + id = MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, id); if (string.IsNullOrEmpty(id)) { info.ArtistProviderIds.TryGetValue(MetadataProvider.MusicBrainzArtist.ToString(), out id); - id = MusicBrainzId(id); + id = MusicBrainzId(MetadataProvider.MusicBrainzArtist, id); } if (string.IsNullOrEmpty(id)) { - return info.SongInfos.Select(i => MusicBrainzId(i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))) + return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))) .FirstOrDefault(i => !string.IsNullOrEmpty(i)); } @@ -72,11 +70,11 @@ namespace MediaBrowser.Providers.Music public static string? GetMusicBrainzArtistId(this ArtistInfo info) { info.ProviderIds.TryGetValue(MetadataProvider.MusicBrainzArtist.ToString(), out var id); - id = MusicBrainzId(id); + id = MusicBrainzId(MetadataProvider.MusicBrainzArtist, id); if (string.IsNullOrEmpty(id)) { - return info.SongInfos.Select(i => MusicBrainzId(i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))) + return info.SongInfos.Select(i => MusicBrainzId(MetadataProvider.MusicBrainzAlbumArtist, i.GetProviderId(MetadataProvider.MusicBrainzAlbumArtist))) .FirstOrDefault(i => !string.IsNullOrEmpty(i)); } @@ -84,9 +82,9 @@ namespace MediaBrowser.Providers.Music } /// <summary> - /// Returns the id if it can be a MusicBrainz id, otherwise <c>null</c>. + /// Returns the id if it can be an id of the given provider, otherwise <c>null</c>. /// </summary> - private static string? MusicBrainzId(string? id) - => Guid.TryParse(id, CultureInfo.InvariantCulture, out _) ? id : null; + private static string? MusicBrainzId(MetadataProvider provider, string? id) + => ProviderIdsExtensions.IsValidProviderId(provider.ToString(), id) ? id : null; } } diff --git a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs index 0fae58fe67..2347c08961 100644 --- a/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs +++ b/tests/Jellyfin.Model.Tests/Entities/ProviderIdsExtensionsTests.cs @@ -229,6 +229,66 @@ namespace Jellyfin.Model.Tests.Entities Assert.Equal("11", provider.GetProviderId(MetadataProvider.Tmdb)); } + [Theory] + [InlineData(nameof(MetadataProvider.Imdb), " tt0113375 ")] + [InlineData(" Imdb", ExampleImdbId)] + public void TrySetProviderId_SurroundingWhitespace_Trimmed(string name, string value) + { + var provider = new ProviderIdsExtensionsTestsObject(); + + Assert.True(provider.TrySetProviderId(name, value)); + Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb)); + } + + [Fact] + public void SetProviderIds_ReplacesAll() + { + var provider = new ProviderIdsExtensionsTestsObject(); + provider.ProviderIds[MetadataProvider.Tvdb.ToString()] = "12345"; + + provider.SetProviderIds(new Dictionary<string, string> + { + [MetadataProvider.Imdb.ToString()] = ExampleImdbId + }); + + Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb)); + Assert.False(provider.HasProviderId(MetadataProvider.Tvdb)); + } + + [Fact] + public void SetProviderIds_ForeignId_Dropped() + { + var provider = new ProviderIdsExtensionsTestsObject(); + + provider.SetProviderIds(new Dictionary<string, string> + { + [MetadataProvider.Tmdb.ToString()] = "nm0000123", + [MetadataProvider.Imdb.ToString()] = ExampleImdbId, + [MetadataProvider.Tvdb.ToString()] = string.Empty + }); + + Assert.False(provider.HasProviderId(MetadataProvider.Tmdb)); + Assert.False(provider.HasProviderId(MetadataProvider.Tvdb)); + Assert.Equal(ExampleImdbId, provider.GetProviderId(MetadataProvider.Imdb)); + } + + [Fact] + public void SetProviderIds_Null_Clears() + { + var provider = new ProviderIdsExtensionsTestsObject(); + provider.ProviderIds[MetadataProvider.Imdb.ToString()] = ExampleImdbId; + + provider.SetProviderIds(null); + + Assert.Empty(provider.ProviderIds); + } + + [Fact] + public void SetProviderIds_NullInstance_ThrowsArgumentNullException() + { + Assert.Throws<ArgumentNullException>(() => ProviderIdsExtensions.SetProviderIds(null!, new Dictionary<string, string>())); + } + [Fact] public void RemoveProviderId_Null_Remove() { diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs index cbc8a65577..1d2fb2e760 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Jellyfin.Data.Enums; @@ -22,9 +23,13 @@ namespace Jellyfin.Providers.Tests.Manager public class MetadataServiceRefreshTests { [Theory] - [InlineData(false, "existing overview")] - [InlineData(true, null)] - public async Task RefreshWithProviders_ReplaceAllMetadata_KeepsExistingDataOnProviderFailure(bool allProvidersSucceed, string? expectedOverview) + // RemoveOldMetadata is only ever set by an explicit user action - a refresh with "replace all + // metadata", or Identify. A provider failing must not silently downgrade that to a merge: the + // providers that did answer supplied the replacement, and the old values are the wrong match + // the user asked to get rid of. + [InlineData(false)] + [InlineData(true)] + public async Task RefreshWithProviders_ReplaceAllMetadata_ErasesOldDataWhenAProviderAnswers(bool allProvidersSucceed) { var item = new Movie { @@ -63,7 +68,51 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal(allProvidersSucceed ? 0 : 1, result.Failures); Assert.Equal("new tagline", item.Tagline); - Assert.Equal(expectedOverview, item.Overview); + Assert.Null(item.Overview); + } + + [Fact] + public async Task RefreshWithProviders_ReplaceAllMetadata_KeepsExistingDataWhenEveryRemoteProviderFails() + { + var item = new Movie + { + Name = "Test Movie", + Overview = "existing overview" + }; + + // Something has to contribute for the merge to run at all, otherwise the item is never touched + // and the case is moot. The local provider is the replacement the remote ones did not deliver. + var local = new Mock<ILocalMetadataProvider<Movie>>(MockBehavior.Loose); + local.Setup(p => p.Name).Returns("Local"); + local.Setup(p => p.GetMetadata(It.IsAny<ItemInfo>(), It.IsAny<IDirectoryService>(), It.IsAny<CancellationToken>())) + .ReturnsAsync(new MetadataResult<Movie> + { + HasMetadata = true, + Item = new Movie { Name = "Test Movie", Tagline = "new tagline" } + }); + + var remote = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose); + remote.Setup(p => p.Name).Returns("Failing"); + remote.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>())) + .Returns(Task.FromException<MetadataResult<Movie>>(new HttpRequestException("unreachable"))); + + var service = new TestMetadataService(); + var result = await service.RefreshWithProvidersInternal( + new MetadataResult<Movie> { Item = item }, + new MovieInfo { Name = item.Name }, + new MetadataRefreshOptions(Mock.Of<IDirectoryService>()) + { + MetadataRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllMetadata = true, + RemoveOldMetadata = true + }, + [local.Object, remote.Object]).ConfigureAwait(true); + + Assert.Equal(1, result.Failures); + Assert.Equal("new tagline", item.Tagline); + + // No remote provider answered, so erasing the overview would lose it for good. + Assert.Equal("existing overview", item.Overview); } [Fact] @@ -97,6 +146,49 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal("tt0113375", item.GetProviderId(MetadataProvider.Imdb)); } + [Fact] + public async Task RefreshWithProviders_ForeignProviderId_ReplacedInLookupInfo() + { + var item = new Movie { Name = "Test Movie" }; + var lookupInfo = new MovieInfo { Name = item.Name }; + lookupInfo.ProviderIds[MetadataProvider.Tmdb.ToString()] = "nm0000123"; + + var answering = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose); + answering.Setup(p => p.Name).Returns("Answering"); + answering.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>())) + .ReturnsAsync(() => + { + var found = new Movie { Name = "Test Movie" }; + found.ProviderIds[MetadataProvider.Tmdb.ToString()] = "12345"; + return new MetadataResult<Movie> { HasMetadata = true, Item = found }; + }); + + string? tmdbIdSeenBySecondProvider = null; + var following = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose); + following.Setup(p => p.Name).Returns("Following"); + following.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>())) + .ReturnsAsync((MovieInfo info, CancellationToken _) => + { + tmdbIdSeenBySecondProvider = info.GetProviderId(MetadataProvider.Tmdb); + return new MetadataResult<Movie> { HasMetadata = false }; + }); + + var service = new TestMetadataService(); + await service.RefreshWithProvidersInternal( + new MetadataResult<Movie> { Item = item }, + lookupInfo, + new MetadataRefreshOptions(Mock.Of<IDirectoryService>()) + { + MetadataRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllMetadata = true + }, + [answering.Object, following.Object]).ConfigureAwait(true); + + // The stored id cannot be a TMDb one, so the provider that still has to run must get the id + // that was just found instead of failing on the same bad one. + Assert.Equal("12345", tmdbIdSeenBySecondProvider); + } + [Theory] [InlineData(true)] [InlineData(false)] |