Additional fixes

Co-Authored-By: Cody Robibero <cody@robibe.ro>

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)]