Merge pull request #17897 from nintwentydo/fix/orphaned-people-cleanup

Clean up orphaned people when deleting items

Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs

@@ -145,13 +145,13 @@ public class ItemPersistenceService : IItemPersistenceService
         context.ItemValuesMap.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
         context.LinkedChildren.WhereOneOrMany(relatedItems, e => e.ParentId).ExecuteDelete();
         context.LinkedChildren.WhereOneOrMany(relatedItems, e => e.ChildId).ExecuteDelete();
+        var peopleIds = context.PeopleBaseItemMap.WhereOneOrMany(relatedItems, e => e.ItemId).Select(f => f.PeopleId).Distinct().ToArray();
         context.BaseItems.WhereOneOrMany(relatedItems, e => e.Id).ExecuteDelete();
         context.KeyframeData.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
         context.MediaSegments.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
         context.MediaStreamInfos.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
-        var query = context.PeopleBaseItemMap.WhereOneOrMany(relatedItems, e => e.ItemId).Select(f => f.PeopleId).Distinct().ToArray();
         context.PeopleBaseItemMap.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
-        context.Peoples.WhereOneOrMany(query, e => e.Id).Where(e => e.BaseItems!.Count == 0).ExecuteDelete();
+        context.Peoples.WhereOneOrMany(peopleIds, e => e.Id).Where(e => !e.BaseItems!.Any()).ExecuteDelete();
         context.TrickplayInfos.WhereOneOrMany(relatedItems, e => e.ItemId).ExecuteDelete();
         context.SaveChanges();
         transaction.Commit();

tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistencePeopleCleanupTests.cs

@@ -0,0 +1,85 @@
+using System;
+using System.Linq;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Server.Implementations.Item;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Entities;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Item;
+
+public sealed class ItemPersistencePeopleCleanupTests : SqliteDbTestFixture
+{
+    private readonly ItemPersistenceService _service;
+
+    public ItemPersistencePeopleCleanupTests()
+    {
+        _service = new ItemPersistenceService(
+            CreateDbContextFactory(),
+            Mock.Of<IServerApplicationHost>(),
+            NullLogger<ItemPersistenceService>.Instance);
+    }
+
+    [Fact]
+    public void DeleteItem_RemovesUnusedPeopleForItemsDescendantsAndExtras()
+    {
+        var parent = CreateItem(isFolder: true);
+        var child = CreateItem();
+        child.ParentId = parent.Id;
+        var extra = CreateItem();
+        extra.OwnerId = child.Id;
+        var survivor = CreateItem();
+        var shared = CreatePerson("Shared person");
+        var unrelatedOrphan = CreatePerson("Unrelated orphan");
+        using (var context = CreateDbContext())
+        {
+            context.PeopleBaseItemMap.AddRange(
+                Map(parent, CreatePerson("Parent credit")),
+                Map(child, CreatePerson("Child credit")),
+                Map(extra, CreatePerson("Extra credit")),
+                Map(child, shared),
+                Map(survivor, shared));
+            context.Peoples.Add(unrelatedOrphan);
+            context.AncestorIds.Add(new AncestorId
+            {
+                ItemId = child.Id,
+                Item = child,
+                ParentItemId = parent.Id,
+                ParentItem = parent
+            });
+            context.SaveChanges();
+        }
+
+        _service.DeleteItem([parent.Id]);
+
+        using var after = CreateDbContext();
+        Assert.Equal(survivor.Id, Assert.Single(after.BaseItems.Where(e => !e.Id.Equals(BaseItemRepository.PlaceholderId))).Id);
+        Assert.Equal(survivor.Id, Assert.Single(after.PeopleBaseItemMap).ItemId);
+        Assert.Equal(new[] { shared.Id, unrelatedOrphan.Id }.Order(), after.Peoples.Select(e => e.Id).Order());
+    }
+
+    private static BaseItemEntity CreateItem(bool isFolder = false) => new()
+    {
+        Id = Guid.NewGuid(),
+        Type = isFolder ? typeof(Folder).FullName! : typeof(Book).FullName!,
+        IsFolder = isFolder
+    };
+
+    private static People CreatePerson(string name) => new()
+    {
+        Id = Guid.NewGuid(),
+        Name = name,
+        PersonType = "Actor"
+    };
+
+    private static PeopleBaseItemMap Map(BaseItemEntity item, People person) => new()
+    {
+        ItemId = item.Id,
+        Item = item,
+        PeopleId = person.Id,
+        People = person,
+        Role = string.Empty
+    };
+}