Apply review suggestions

2 files changed, 69 insertions, 0 deletions

diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs
index fb1781accc..c6c15198be 100644
--- a/MediaBrowser.Providers/Manager/MetadataService.cs
+++ b/MediaBrowser.Providers/Manager/MetadataService.cs
@@ -1137,6 +1137,8 @@ namespace MediaBrowser.Providers.Manager
 
             if (!lockedFields.Contains(MetadataField.Cast))
             {
+                RemoveInvalidProviderIds(sourceResult.People);
+
                 if (replaceData || targetResult.People is null || targetResult.People.Count == 0)
                 {
                     targetResult.People = sourceResult.People;
@@ -1291,6 +1293,32 @@ namespace MediaBrowser.Providers.Manager
             }
         }
 
+        private static void RemoveInvalidProviderIds(IReadOnlyList<PersonInfo> people)
+        {
+            if (people is null)
+            {
+                return;
+            }
+
+            foreach (var person in people)
+            {
+                if (person.ProviderIds is null || person.ProviderIds.Count == 0)
+                {
+                    continue;
+                }
+
+                var invalidKeys = person.ProviderIds
+                    .Where(id => !ProviderIdsExtensions.IsValidProviderId(id.Key, id.Value))
+                    .Select(id => id.Key)
+                    .ToArray();
+
+                foreach (var key in invalidKeys)
+                {
+                    person.ProviderIds.Remove(key);
+                }
+            }
+        }
+
         private static void MergePeople(IReadOnlyList<PersonInfo> source, IReadOnlyList<PersonInfo> target)
         {
             var sourceByName = source.ToLookup(p => p.Name.RemoveDiacritics(), StringComparer.OrdinalIgnoreCase);
diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs
index 449abb2e6a..cbc8a65577 100644
--- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs
+++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs
@@ -2,7 +2,9 @@ using System;
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using Jellyfin.Data.Enums;
 using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Entities.Movies;
 using MediaBrowser.Controller.IO;
 using MediaBrowser.Controller.Library;
@@ -95,6 +97,45 @@ namespace Jellyfin.Providers.Tests.Manager
             Assert.Equal("tt0113375", item.GetProviderId(MetadataProvider.Imdb));
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task RefreshWithProviders_ForeignPersonProviderId_NotStored(bool replaceAllMetadata)
+        {
+            var item = new Movie { Name = "Test Movie" };
+            var existing = new MetadataResult<Movie> { Item = item };
+            existing.AddPerson(new PersonInfo { Name = "Some Actor", Type = PersonKind.Actor });
+
+            var provider = new Mock<IRemoteMetadataProvider<Movie, MovieInfo>>(MockBehavior.Loose);
+            provider.Setup(p => p.Name).Returns("Provider");
+            provider.Setup(p => p.GetMetadata(It.IsAny<MovieInfo>(), It.IsAny<CancellationToken>()))
+                .ReturnsAsync(() =>
+                {
+                    var person = new PersonInfo { Name = "Some Actor", Type = PersonKind.Actor };
+                    person.ProviderIds[MetadataProvider.Tmdb.ToString()] = "nm0000123";
+                    person.ProviderIds[MetadataProvider.Imdb.ToString()] = "nm0000123";
+
+                    var found = new MetadataResult<Movie> { HasMetadata = true, Item = new Movie { Name = "Test Movie" } };
+                    found.AddPerson(person);
+                    return found;
+                });
+
+            var service = new TestMetadataService();
+            await service.RefreshWithProvidersInternal(
+                existing,
+                new MovieInfo { Name = item.Name },
+                new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
+                {
+                    MetadataRefreshMode = MetadataRefreshMode.FullRefresh,
+                    ReplaceAllMetadata = replaceAllMetadata
+                },
+                [provider.Object]).ConfigureAwait(true);
+
+            var mergedPerson = Assert.Single(existing.People);
+            Assert.False(mergedPerson.HasProviderId(MetadataProvider.Tmdb));
+            Assert.Equal("nm0000123", mergedPerson.GetProviderId(MetadataProvider.Imdb));
+        }
+
         private sealed class TestMetadataService : MetadataService<Movie, MovieInfo>
         {
             public TestMetadataService()