diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 672a86eb8a..48b61b78a3 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -1203,6 +1203,12 @@ namespace Emby.Server.Implementations.Library
.FirstOrDefault();
}
+ ///
+ public Guid GetPersonId(string name)
+ {
+ return GetItemByNameId(Person.GetPath(name));
+ }
+
///
public Person? GetPerson(string name)
{
diff --git a/Emby.Server.Implementations/Library/Validators/ArtistsValidator.cs b/Emby.Server.Implementations/Library/Validators/ArtistsValidator.cs
index fa7112eb90..690466be70 100644
--- a/Emby.Server.Implementations/Library/Validators/ArtistsValidator.cs
+++ b/Emby.Server.Implementations/Library/Validators/ArtistsValidator.cs
@@ -1,4 +1,5 @@
using System;
+using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading;
@@ -61,6 +62,9 @@ public class ArtistsValidator
var count = names.Count;
var refreshed = 0;
+ var liveIds = new HashSet();
+ var unresolved = 0;
+
foreach (var name in names)
{
try
@@ -73,13 +77,20 @@ public class ArtistsValidator
// Fall back to GetArtist if not found (creates new item if needed)
item ??= _libraryManager.GetArtist(name);
- var isNew = !existingArtistIds.Contains(item.Id);
- var neverRefreshed = item.DateLastRefreshed == default;
- if (isNew || neverRefreshed)
+ // A name with no item is nothing to refresh, and nothing to keep alive either.
+ if (item is not null)
{
- await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
- refreshed++;
+ liveIds.Add(item.Id);
+
+ var isNew = !existingArtistIds.Contains(item.Id);
+ var neverRefreshed = item.DateLastRefreshed == default;
+
+ if (isNew || neverRefreshed)
+ {
+ await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
+ refreshed++;
+ }
}
}
catch (OperationCanceledException)
@@ -88,6 +99,7 @@ public class ArtistsValidator
}
catch (Exception ex)
{
+ unresolved++;
_logger.LogError(ex, "Error refreshing {ArtistName}", name);
}
@@ -101,13 +113,26 @@ public class ArtistsValidator
_logger.LogInformation("Refreshed metadata for {RefreshedCount} new artists out of {TotalCount} total", refreshed, count);
+ // Every name that threw is a name whose artist is missing from the live set, and deleting against
+ // a live set with holes in it deletes artists the library still refers to. Leave the sweep to a
+ // run that got a clean read of them.
+ if (unresolved > 0)
+ {
+ _logger.LogWarning(
+ "Not removing dead artists: {Count} of {TotalCount} names could not be resolved this run",
+ unresolved,
+ count);
+
+ progress.Report(100);
+ return;
+ }
+
var deadEntities = _libraryManager.GetItemList(new InternalItemsQuery
{
IncludeItemTypes = [BaseItemKind.MusicArtist],
- IsDeadArtist = true,
IsLocked = false
- }).Cast()
- .Where(item => item.IsAccessedByName)
+ }).OfType()
+ .Where(item => item.IsAccessedByName && !liveIds.Contains(item.Id))
.ToList();
foreach (var item in deadEntities)
diff --git a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
index 3c8806d549..7d53f40ce7 100644
--- a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
+++ b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
@@ -1,4 +1,5 @@
using System;
+using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Threading;
@@ -58,6 +59,8 @@ public class PeopleValidator
IncludeItemTypes = [BaseItemKind.Person]
}).ToHashSet();
+ var (newNames, deadIds) = PartitionCreditsByPersonId(names, _libraryManager.GetPersonId, existingPersonIds);
+
var numComplete = 0;
var count = names.Count;
var refreshed = 0;
@@ -96,14 +99,18 @@ public class PeopleValidator
progress.Report(percent);
}
- _logger.LogInformation("Refreshed metadata for {RefreshedCount} new people out of {TotalCount} total", refreshed, count);
+ _logger.LogInformation(
+ "Refreshed metadata for {RefreshedCount} people out of {TotalCount} total, {NewCount} of which had no item yet",
+ refreshed,
+ count,
+ newNames.Count);
- var deadEntities = _libraryManager.GetItemList(new InternalItemsQuery
- {
- IncludeItemTypes = [BaseItemKind.Person],
- IsDeadPerson = true,
- IsLocked = false
- });
+ // A person somebody locked is theirs, not ours, however little the library still credits them.
+ var deadEntities = deadIds
+ .Select(_libraryManager.GetItemById)
+ .OfType()
+ .Where(item => !item.IsLocked)
+ .ToList();
foreach (var item in deadEntities)
{
@@ -114,4 +121,39 @@ public class PeopleValidator
progress.Report(100);
}
+
+ ///
+ /// Splits the person items into the ones a credit still calls for and the ones nothing does.
+ ///
+ /// Every name credited on an item, from the people table.
+ /// Maps a credit name to the id its person item has.
+ /// The ids of the person items that exist.
+ /// The credits needing an item, and the ids of the items nothing credits.
+ internal static (List NewNames, List DeadIds) PartitionCreditsByPersonId(
+ IReadOnlyList creditNames,
+ Func getPersonId,
+ IReadOnlySet existingPersonIds)
+ {
+ ArgumentNullException.ThrowIfNull(creditNames);
+ ArgumentNullException.ThrowIfNull(getPersonId);
+ ArgumentNullException.ThrowIfNull(existingPersonIds);
+
+ var newNames = new List();
+ var liveIds = new HashSet();
+
+ foreach (var name in creditNames)
+ {
+ var personId = getPersonId(name);
+
+ // Distinct credit names can normalize onto one id; only the first of them needs an item.
+ if (liveIds.Add(personId) && !existingPersonIds.Contains(personId))
+ {
+ newNames.Add(name);
+ }
+ }
+
+ var deadIds = existingPersonIds.Where(id => !liveIds.Contains(id)).ToList();
+
+ return (newNames, deadIds);
+ }
}
diff --git a/MediaBrowser.Controller/Entities/InternalItemsQuery.cs b/MediaBrowser.Controller/Entities/InternalItemsQuery.cs
index 0e5a5047cd..eb2a3676ac 100644
--- a/MediaBrowser.Controller/Entities/InternalItemsQuery.cs
+++ b/MediaBrowser.Controller/Entities/InternalItemsQuery.cs
@@ -432,12 +432,18 @@ namespace MediaBrowser.Controller.Entities
public string? HasNoSubtitleTrackWithLanguage { get; set; }
+ ///
+ /// Gets or sets a value indicating whether to return only items nothing names any more.
+ ///
public bool? IsDeadArtist { get; set; }
public bool? IsDeadStudio { get; set; }
public bool? IsDeadGenre { get; set; }
+ ///
+ /// Gets or sets a value indicating whether to return only items nothing names any more.
+ ///
public bool? IsDeadPerson { get; set; }
///
diff --git a/MediaBrowser.Controller/Library/ILibraryManager.cs b/MediaBrowser.Controller/Library/ILibraryManager.cs
index 71b3f054ff..9028b0d6b8 100644
--- a/MediaBrowser.Controller/Library/ILibraryManager.cs
+++ b/MediaBrowser.Controller/Library/ILibraryManager.cs
@@ -707,6 +707,14 @@ namespace MediaBrowser.Controller.Library
/// true if ignored, false otherwise.
bool IgnoreFile(FileSystemMetadata file, BaseItem parent);
+ ///
+ /// Gets the id a item for the name would have, without looking it up
+ /// or creating it.
+ ///
+ /// The name of the person.
+ /// The item id for the name.
+ Guid GetPersonId(string name);
+
Guid GetStudioId(string name);
Guid GetGenreId(string name);
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Library/PeopleValidatorPartitionTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Library/PeopleValidatorPartitionTests.cs
new file mode 100644
index 0000000000..30f7bed208
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Library/PeopleValidatorPartitionTests.cs
@@ -0,0 +1,117 @@
+using System;
+using System.Collections.Generic;
+using Emby.Server.Implementations.Library.Validators;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Library;
+
+///
+/// Tests for how the people validator decides which credits need a person item and which person items
+/// nothing credits any more. Keying either half on the item's name rather than its id put the two halves
+/// in a loop that created, refreshed and deleted the same people on every run, so these pin the id.
+///
+public class PeopleValidatorPartitionTests
+{
+ // Stands in for the real item-by-name id: derived from the credit name, case-insensitively, and
+ // from nothing else. The property that matters is that it does not depend on the item's own name.
+ private static Guid PersonId(string creditName)
+ {
+#pragma warning disable CA5351 // Do Not Use Broken Cryptographic Algorithms
+ var hash = System.Security.Cryptography.MD5.HashData(
+ System.Text.Encoding.Unicode.GetBytes(creditName.ToLowerInvariant()));
+#pragma warning restore CA5351 // Do Not Use Broken Cryptographic Algorithms
+ return new Guid(hash);
+ }
+
+ [Fact]
+ public void PartitionCreditsByPersonId_ProviderRenamedThePerson_KeepsThemAndCreatesNothing()
+ {
+ // The credit still says "AURORA"; the item it made has been renamed to "Aurora" by the provider
+ // that refreshed it. Nothing about the library changed, so nothing should be created or deleted.
+ var credits = new[] { "AURORA" };
+ var existing = new HashSet { PersonId("AURORA") };
+
+ var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+
+ Assert.Empty(newNames);
+ Assert.Empty(deadIds);
+ }
+
+ [Theory]
+ // Every shape of rename seen in the wild on a real library.
+ [InlineData("AURORA")]
+ [InlineData("Amir AboulEla")]
+ [InlineData("Miguel Ángel Fuentes")]
+ [InlineData("a‐ha")]
+ [InlineData("윤현민")]
+ public void PartitionCreditsByPersonId_CreditWithAnItem_IsNeverBothCreatedAndDeleted(string creditName)
+ {
+ var existing = new HashSet { PersonId(creditName) };
+
+ var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId([creditName], PersonId, existing);
+
+ Assert.Empty(newNames);
+ Assert.Empty(deadIds);
+ }
+
+ [Fact]
+ public void PartitionCreditsByPersonId_CreditWithNoItem_IsCreated()
+ {
+ var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(
+ ["Wanted Person"],
+ PersonId,
+ new HashSet());
+
+ Assert.Equal(["Wanted Person"], newNames);
+ Assert.Empty(deadIds);
+ }
+
+ [Fact]
+ public void PartitionCreditsByPersonId_ItemNoCreditNames_IsDead()
+ {
+ var orphan = PersonId("Nobody Credits Me");
+ var existing = new HashSet { PersonId("Credited"), orphan };
+
+ var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(["Credited"], PersonId, existing);
+
+ Assert.Empty(newNames);
+ Assert.Equal([orphan], deadIds);
+ }
+
+ [Fact]
+ public void PartitionCreditsByPersonId_CreditsNormalizingOntoOneId_CreateOneItem()
+ {
+ // "AURORA" and "Aurora" are one person as far as the item-by-name id is concerned, so exactly
+ // one of them should create the item and neither should end up dead.
+ var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(
+ ["AURORA", "Aurora", "aurora"],
+ PersonId,
+ new HashSet());
+
+ Assert.Single(newNames);
+ Assert.Empty(deadIds);
+ }
+
+ [Fact]
+ public void PartitionCreditsByPersonId_SecondRunAfterCreating_AsksForNothingFurther()
+ {
+ // The churn showed up as a run that never settled, so drive two rounds: whatever round one
+ // created must leave round two with nothing to do.
+ string[] credits = ["AURORA", "Amir AboulEla", "Miguel Ángel Fuentes"];
+ var existing = new HashSet();
+
+ var (firstNames, firstDead) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+ Assert.Equal(3, firstNames.Count);
+ Assert.Empty(firstDead);
+
+ foreach (var created in firstNames)
+ {
+ existing.Add(PersonId(created));
+ }
+
+ var (secondNames, secondDead) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+
+ Assert.Empty(secondNames);
+ Assert.Empty(secondDead);
+ }
+}