Delete a credit once no item maps to it any more

6 files changed, 132 insertions, 1 deletions

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 2bba659a23..6cf9f33e6e 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -3559,6 +3559,12 @@ namespace Emby.Server.Implementations.Library
         }
 
         /// <inheritdoc/>
+        public int DeleteOrphanedCredits()
+        {
+            return _peopleRepository.DeleteOrphanedCredits();
+        }
+
+        /// <inheritdoc/>
         public IReadOnlyDictionary<Guid, IReadOnlyList<string>> GetPeopleNamesByItems(IReadOnlyList<Guid> itemIds, IReadOnlyList<string> personTypes)
         {
             return _peopleRepository.GetPeopleNamesByItems(itemIds, personTypes);
diff --git a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
index dacef102dd..078a0b921d 100644
--- a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
+++ b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
@@ -49,6 +49,14 @@ public class PeopleValidator
     /// <returns>Task.</returns>
     public async Task ValidatePeople(CancellationToken cancellationToken, IProgress<double> progress)
     {
+        // Before the refresh below walks them: a credit no item maps to any more stands for nothing,
+        // and while it is there the person it names cannot reach the dead-person sweep either.
+        var numOrphaned = _libraryManager.DeleteOrphanedCredits();
+        if (numOrphaned > 0)
+        {
+            _logger.LogDebug("Deleted {Amount} credits no item maps to", numOrphaned);
+        }
+
         var people = _libraryManager.GetPeopleNames(new InternalPeopleQuery());
 
         var numComplete = 0;
@@ -115,6 +123,6 @@ public class PeopleValidator
 
         progress.Report(100);
 
-        _logger.LogInformation("People validation complete");
+        _logger.LogInformation("People validation complete, deleted {Orphaned} orphaned credits", numOrphaned);
     }
 }
diff --git a/Jellyfin.Server.Implementations/Item/PeopleRepository.cs b/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
index aaa363b046..da2ad033ec 100644
--- a/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
+++ b/Jellyfin.Server.Implementations/Item/PeopleRepository.cs
@@ -194,13 +194,45 @@ public class PeopleRepository(IDbContextFactory<JellyfinDbContext> dbProvider, I
             listOrder++;
         }
 
+        var droppedCredits = existingMaps.Select(e => e.PeopleId).Distinct().ToArray();
         context.PeopleBaseItemMap.RemoveRange(existingMaps);
 
         context.SaveChanges();
+
+        // Nothing else ever deletes a credit row, so one left without a single mapping outlives the
+        // credit it stood for: it keeps a person of that name off the dead-person sweep, which only
+        // sees items no credit names, and keeps the name in every by-name list. That is how a credit
+        // a provider dropped, or one a broken provider result invented, becomes impossible to clean up.
+        DeleteCreditsWithoutMapping(context, droppedCredits);
+
+        context.SaveChanges();
         transaction.Commit();
     }
 
     /// <inheritdoc/>
+    public int DeleteOrphanedCredits()
+    {
+        using var context = _dbProvider.CreateDbContext();
+
+        return DeleteCreditsWithoutMapping(context, null);
+    }
+
+    // A null candidate list sweeps every credit, anything else only the ones just unmapped.
+    private int DeleteCreditsWithoutMapping(JellyfinDbContext context, IReadOnlyList<Guid>? candidates)
+    {
+        if (candidates is not null && candidates.Count == 0)
+        {
+            return 0;
+        }
+
+        var credits = candidates is null
+            ? context.Peoples.AsQueryable()
+            : context.Peoples.WhereOneOrMany(candidates, e => e.Id);
+
+        return credits.Where(e => !context.PeopleBaseItemMap.Any(f => f.PeopleId == e.Id)).ExecuteDelete();
+    }
+
+    /// <inheritdoc/>
     public IReadOnlyDictionary<Guid, IReadOnlyList<string>> GetPeopleNamesByItems(IReadOnlyList<Guid> itemIds, IReadOnlyList<string> personTypes)
     {
         using var context = _dbProvider.CreateDbContext();
diff --git a/MediaBrowser.Controller/Library/ILibraryManager.cs b/MediaBrowser.Controller/Library/ILibraryManager.cs
index ca686fbd9d..2a6ea214b8 100644
--- a/MediaBrowser.Controller/Library/ILibraryManager.cs
+++ b/MediaBrowser.Controller/Library/ILibraryManager.cs
@@ -606,6 +606,12 @@ namespace MediaBrowser.Controller.Library
         IReadOnlyList<string> GetPeopleNames(InternalPeopleQuery query);
 
         /// <summary>
+        /// Deletes every credit that no item maps to any more.
+        /// </summary>
+        /// <returns>The number of credits that were deleted.</returns>
+        int DeleteOrphanedCredits();
+
+        /// <summary>
         /// Gets the distinct people names per item for multiple items.
         /// </summary>
         /// <param name="itemIds">The item IDs.</param>
diff --git a/MediaBrowser.Controller/Persistence/IPeopleRepository.cs b/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
index 9811241d31..15183a8806 100644
--- a/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
+++ b/MediaBrowser.Controller/Persistence/IPeopleRepository.cs
@@ -34,6 +34,12 @@ public interface IPeopleRepository
     IReadOnlyList<string> GetPeopleNames(InternalPeopleQuery filter);
 
     /// <summary>
+    /// Deletes every credit that no item maps to any more.
+    /// </summary>
+    /// <returns>The number of credits that were deleted.</returns>
+    int DeleteOrphanedCredits();
+
+    /// <summary>
     /// Gets the distinct people names per item for multiple items efficiently by querying from the mapping table.
     /// </summary>
     /// <param name="itemIds">The item IDs to get people for.</param>
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleRepositoryUpdatePeopleTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleRepositoryUpdatePeopleTests.cs
index 54565c5787..649458f733 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleRepositoryUpdatePeopleTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleRepositoryUpdatePeopleTests.cs
@@ -142,6 +142,79 @@ public sealed class PeopleRepositoryUpdatePeopleTests : SqliteDbTestFixture
         Assert.Equal("Hero", map.Role);
     }
 
+    [Fact]
+    public void UpdatePeople_CreditDroppedByTheProvider_LeavesNoCreditRowBehind()
+    {
+        _repository.UpdatePeople(_itemId, [
+            CreatePerson("Person A", PersonKind.Actor, "Hero"),
+            CreatePerson("Person B", PersonKind.Actor, "Villain")
+        ]);
+
+        _repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
+
+        using var ctx = CreateDbContext();
+        Assert.Equal(["Person A"], ctx.Peoples.Select(e => e.Name).ToArray());
+    }
+
+    [Fact]
+    public void UpdatePeople_CreditStillHeldByAnotherItem_IsKept()
+    {
+        _repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
+        _repository.UpdatePeople(AddMovie("Other Movie"), [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
+
+        _repository.UpdatePeople(_itemId, []);
+
+        using var after = CreateDbContext();
+        Assert.Single(after.Peoples);
+        Assert.Single(after.PeopleBaseItemMap);
+    }
+
+    [Fact]
+    public void DeleteOrphanedCredits_CreditNoItemMapsTo_IsDeleted()
+    {
+        _repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
+        using (var ctx = CreateDbContext())
+        {
+            // The state a credit was left in before UpdatePeople cleaned up after itself.
+            ctx.PeopleBaseItemMap.RemoveRange(ctx.PeopleBaseItemMap);
+            ctx.SaveChanges();
+        }
+
+        Assert.Equal(1, _repository.DeleteOrphanedCredits());
+
+        using var after = CreateDbContext();
+        Assert.Empty(after.Peoples);
+    }
+
+    [Fact]
+    public void DeleteOrphanedCredits_CreditAnItemMapsTo_IsKept()
+    {
+        _repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
+
+        Assert.Equal(0, _repository.DeleteOrphanedCredits());
+
+        using var after = CreateDbContext();
+        Assert.Single(after.Peoples);
+    }
+
+    private Guid AddMovie(string name)
+    {
+        var id = Guid.NewGuid();
+        using var ctx = CreateDbContext();
+        ctx.BaseItems.Add(new BaseItemEntity
+        {
+            Id = id,
+            Type = new ItemTypeLookup().BaseItemKindNames[BaseItemKind.Movie],
+            Name = name,
+            MediaType = "Video",
+            IsMovie = true,
+            IsFolder = false,
+            IsVirtualItem = false
+        });
+        ctx.SaveChanges();
+        return id;
+    }
+
     private static PersonInfo CreatePerson(string name, PersonKind type, string role)
     {
         return new PersonInfo