Fix people validator not creating missing people

4 files changed, 77 insertions, 89 deletions

diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index 3db8265f6e..672a86eb8a 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -15,7 +15,6 @@ using Emby.Naming.Common;
 using Emby.Naming.TV;
 using Emby.Naming.Video;
 using Emby.Server.Implementations.Library.Resolvers;
-using Emby.Server.Implementations.Library.Validators;
 using Emby.Server.Implementations.Playlists;
 using Emby.Server.Implementations.ScheduledTasks.Tasks;
 using Emby.Server.Implementations.Sorting;
@@ -35,7 +34,6 @@ using MediaBrowser.Controller.Entities.Movies;
 using MediaBrowser.Controller.IO;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.LiveTv;
-using MediaBrowser.Controller.MediaEncoding;
 using MediaBrowser.Controller.Persistence;
 using MediaBrowser.Controller.Playlists;
 using MediaBrowser.Controller.Providers;
@@ -75,7 +73,6 @@ namespace Emby.Server.Implementations.Library
         private readonly Lazy<IProviderManager> _providerManagerFactory;
         private readonly Lazy<IUserViewManager> _userViewManagerFactory;
         private readonly IServerApplicationHost _appHost;
-        private readonly IMediaEncoder _mediaEncoder;
         private readonly IFileSystem _fileSystem;
         private readonly IItemRepository _itemRepository;
         private readonly IItemPersistenceService _persistenceService;
@@ -122,7 +119,6 @@ namespace Emby.Server.Implementations.Library
         /// <param name="fileSystem">The file system.</param>
         /// <param name="providerManagerFactory">The provider manager.</param>
         /// <param name="userViewManagerFactory">The user view manager.</param>
-        /// <param name="mediaEncoder">The media encoder.</param>
         /// <param name="itemRepository">The item repository.</param>
         /// <param name="persistenceService">The item persistence service.</param>
         /// <param name="nextUpService">The next up service.</param>
@@ -148,7 +144,6 @@ namespace Emby.Server.Implementations.Library
             IFileSystem fileSystem,
             Lazy<IProviderManager> providerManagerFactory,
             Lazy<IUserViewManager> userViewManagerFactory,
-            IMediaEncoder mediaEncoder,
             IItemRepository itemRepository,
             IItemPersistenceService persistenceService,
             INextUpService nextUpService,
@@ -174,7 +169,6 @@ namespace Emby.Server.Implementations.Library
             _fileSystem = fileSystem;
             _providerManagerFactory = providerManagerFactory;
             _userViewManagerFactory = userViewManagerFactory;
-            _mediaEncoder = mediaEncoder;
             _itemRepository = itemRepository;
             _persistenceService = persistenceService;
             _nextUpService = nextUpService;
@@ -1222,6 +1216,33 @@ namespace Emby.Server.Implementations.Library
             return null;
         }
 
+        /// <inheritdoc />
+        public Person GetOrCreatePerson(string name)
+        {
+            var existing = GetPerson(name);
+            if (existing is not null)
+            {
+                return existing;
+            }
+
+            var path = Person.GetPath(name);
+            var info = Directory.CreateDirectory(path);
+            var item = new Person
+            {
+                Name = name,
+                Id = GetItemByNameId<Person>(path),
+                DateCreated = info.CreationTimeUtc,
+                DateModified = info.LastWriteTimeUtc,
+                Path = path
+            };
+
+            item.PresentationUniqueKey = item.CreatePresentationUniqueKey();
+
+            CreateItem(item, null);
+
+            return item;
+        }
+
         /// <summary>
         /// Gets the studio.
         /// </summary>
@@ -1354,15 +1375,6 @@ namespace Emby.Server.Implementations.Library
             return GetNewItemIdInternal(path, typeof(T), forceCaseInsensitiveId);
         }
 
-        /// <inheritdoc />
-        public Task ValidatePeopleAsync(IProgress<double> progress, CancellationToken cancellationToken)
-        {
-            // Ensure the location is available.
-            Directory.CreateDirectory(_configurationManager.ApplicationPaths.PeoplePath);
-
-            return new PeopleValidator(this, _logger, _fileSystem).ValidatePeople(cancellationToken, progress);
-        }
-
         /// <summary>
         /// Reloads the root media folder.
         /// </summary>
@@ -3746,27 +3758,14 @@ namespace Emby.Server.Implementations.Library
 
                 var itemUpdateType = ItemUpdateType.MetadataDownload;
                 var saveEntity = false;
-                var createEntity = false;
                 var personEntity = GetPerson(person.Name);
 
                 if (personEntity is null)
                 {
                     try
                     {
-                        var path = Person.GetPath(person.Name);
-                        var info = Directory.CreateDirectory(path);
-                        personEntity = new Person()
-                        {
-                            Name = person.Name,
-                            Id = GetItemByNameId<Person>(path),
-                            DateCreated = info.CreationTimeUtc,
-                            DateModified = info.LastWriteTimeUtc,
-                            Path = path
-                        };
-
-                        personEntity.PresentationUniqueKey = personEntity.CreatePresentationUniqueKey();
+                        personEntity = GetOrCreatePerson(person.Name);
                         saveEntity = true;
-                        createEntity = true;
                     }
                     catch (Exception ex)
                     {
@@ -3800,11 +3799,6 @@ namespace Emby.Server.Implementations.Library
 
                 if (saveEntity)
                 {
-                    if (createEntity)
-                    {
-                        CreateItems([personEntity], null, CancellationToken.None);
-                    }
-
                     await RunMetadataSavers(personEntity, itemUpdateType).ConfigureAwait(false);
                     personEntity.DateLastSaved = DateTime.UtcNow;
 
diff --git a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
index 078a0b921d..3c8806d549 100644
--- a/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
+++ b/Emby.Server.Implementations/Library/Validators/PeopleValidator.cs
@@ -1,12 +1,11 @@
 using System;
+using System.Globalization;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Jellyfin.Data.Enums;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
-using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.IO;
 using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Validators;
@@ -17,94 +16,88 @@ namespace Emby.Server.Implementations.Library.Validators;
 public class PeopleValidator
 {
     /// <summary>
-    /// The _library manager.
+    /// The library manager.
     /// </summary>
     private readonly ILibraryManager _libraryManager;
 
     /// <summary>
-    /// The _logger.
+    /// The logger.
     /// </summary>
-    private readonly ILogger _logger;
-
-    private readonly IFileSystem _fileSystem;
+    private readonly ILogger<PeopleValidator> _logger;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="PeopleValidator" /> class.
     /// </summary>
     /// <param name="libraryManager">The library manager.</param>
     /// <param name="logger">The logger.</param>
-    /// <param name="fileSystem">The file system.</param>
-    public PeopleValidator(ILibraryManager libraryManager, ILogger logger, IFileSystem fileSystem)
+    public PeopleValidator(ILibraryManager libraryManager, ILogger<PeopleValidator> logger)
     {
         _libraryManager = libraryManager;
         _logger = logger;
-        _fileSystem = fileSystem;
     }
 
     /// <summary>
     /// Validates the people.
     /// </summary>
-    /// <param name="cancellationToken">The cancellation token.</param>
     /// <param name="progress">The progress.</param>
+    /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>Task.</returns>
-    public async Task ValidatePeople(CancellationToken cancellationToken, IProgress<double> progress)
+    public async Task Run(IProgress<double> progress, CancellationToken cancellationToken)
     {
         // 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);
+            _logger.LogInformation("Deleted {Amount} credits no item maps to", numOrphaned);
         }
 
-        var people = _libraryManager.GetPeopleNames(new InternalPeopleQuery());
+        var names = _libraryManager.GetPeopleNames(new InternalPeopleQuery());
+        var existingPersonIds = _libraryManager.GetItemIds(new InternalItemsQuery
+        {
+            IncludeItemTypes = [BaseItemKind.Person]
+        }).ToHashSet();
 
         var numComplete = 0;
+        var count = names.Count;
+        var refreshed = 0;
 
-        var numPeople = people.Count;
-
-        IProgress<double> subProgress = new Progress<double>((val) => progress.Report(val / 2));
-
-        _logger.LogDebug("Will refresh {Amount} people", numPeople);
-
-        foreach (var person in people)
+        foreach (var name in names)
         {
             cancellationToken.ThrowIfCancellationRequested();
 
             try
             {
-                var item = _libraryManager.GetPerson(person);
-                if (item is null)
-                {
-                    _logger.LogWarning("Failed to get person: {Name}", person);
-                    continue;
-                }
+                var item = _libraryManager.GetOrCreatePerson(name);
+                var isNew = !existingPersonIds.Contains(item.Id);
+                var neverRefreshed = item.DateLastRefreshed == default;
 
-                var options = new MetadataRefreshOptions(new DirectoryService(_fileSystem))
+                if (isNew || neverRefreshed)
                 {
-                    ImageRefreshMode = MetadataRefreshMode.ValidationOnly,
-                    MetadataRefreshMode = MetadataRefreshMode.ValidationOnly
-                };
-
-                await item.RefreshMetadata(options, cancellationToken).ConfigureAwait(false);
+                    await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
+                    refreshed++;
+                }
             }
             catch (OperationCanceledException)
             {
+                // Don't clutter the log
                 throw;
             }
             catch (Exception ex)
             {
-                _logger.LogError(ex, "Error validating IBN entry {Person}", person);
+                _logger.LogError(ex, "Error refreshing {PersonName}", name);
             }
 
-            // Update progress
             numComplete++;
             double percent = numComplete;
-            percent /= numPeople;
+            percent /= count;
+            percent *= 100;
 
-            subProgress.Report(100 * percent);
+            progress.Report(percent);
         }
 
+        _logger.LogInformation("Refreshed metadata for {RefreshedCount} new people out of {TotalCount} total", refreshed, count);
+
         var deadEntities = _libraryManager.GetItemList(new InternalItemsQuery
         {
             IncludeItemTypes = [BaseItemKind.Person],
@@ -112,17 +105,13 @@ public class PeopleValidator
             IsLocked = false
         });
 
-        subProgress = new Progress<double>((val) => progress.Report((val / 2) + 50));
-
-        var i = 0;
-        foreach (var item in deadEntities.Chunk(500))
+        foreach (var item in deadEntities)
         {
-            _libraryManager.DeleteItemsUnsafeFast(item, true);
-            subProgress.Report(100f / deadEntities.Count * (i++ * 100));
+            _logger.LogInformation("Deleting dead {ItemType} {ItemId} {ItemName}", item.GetType().Name, item.Id.ToString("N", CultureInfo.InvariantCulture), item.Name);
         }
 
-        progress.Report(100);
+        _libraryManager.DeleteItemsUnsafeFast(deadEntities, deleteSourceFiles: true);
 
-        _logger.LogInformation("People validation complete, deleted {Orphaned} orphaned credits", numOrphaned);
+        progress.Report(100);
     }
 }
diff --git a/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs b/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs
index 42835d7ad0..092a621bfc 100644
--- a/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs
+++ b/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs
@@ -4,6 +4,7 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Emby.Server.Implementations.Library.Validators;
 using Jellyfin.Data.Enums;
 using Jellyfin.Database.Implementations;
 using Jellyfin.Database.Implementations.Entities;
@@ -29,6 +30,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
     private readonly IDbContextFactory<JellyfinDbContext> _dbContextFactory;
     private readonly IFileSystem _fileSystem;
     private readonly ILogger<PeopleValidationTask> _logger;
+    private readonly ILogger<PeopleValidator> _validatorLogger;
     private readonly IItemTypeLookup _itemTypeLookup;
 
     /// <summary>
@@ -39,6 +41,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
     /// <param name="dbContextFactory">Instance of the <see cref="IDbContextFactory{TContext}"/> interface.</param>
     /// <param name="fileSystem">Instance of the <see cref="IFileSystem"/> interface.</param>
     /// <param name="logger">Instance of the <see cref="ILogger{PeopleValidationTask}"/> interface.</param>
+    /// <param name="validatorLogger">Instance of the <see cref="ILogger{PeopleValidator}"/> interface.</param>
     /// <param name="itemTypeLookup">Instance of the <see cref="IItemTypeLookup"/> interface.</param>
     public PeopleValidationTask(
         ILibraryManager libraryManager,
@@ -46,6 +49,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         IDbContextFactory<JellyfinDbContext> dbContextFactory,
         IFileSystem fileSystem,
         ILogger<PeopleValidationTask> logger,
+        ILogger<PeopleValidator> validatorLogger,
         IItemTypeLookup itemTypeLookup)
     {
         _libraryManager = libraryManager;
@@ -53,6 +57,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         _dbContextFactory = dbContextFactory;
         _fileSystem = fileSystem;
         _logger = logger;
+        _validatorLogger = validatorLogger;
         _itemTypeLookup = itemTypeLookup;
     }
 
@@ -165,7 +170,9 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         // Phase 2: Validate people (33-66%). Runs after orphaned PeopleBaseItemMap entries are
         // cleaned up above, so dead people are removed in a single pass instead of requiring a second run.
         IProgress<double> validateProgress = new Progress<double>((val) => progress.Report((val / 3) + 33));
-        await _libraryManager.ValidatePeopleAsync(validateProgress, cancellationToken).ConfigureAwait(false);
+        await new PeopleValidator(_libraryManager, _validatorLogger)
+            .Run(validateProgress, cancellationToken)
+            .ConfigureAwait(false);
 
         // Phase 3: Refresh images for people missing them (66-100%)
         IProgress<double> refreshProgress = new Progress<double>((val) => progress.Report((val / 3) + 66));
diff --git a/MediaBrowser.Controller/Library/ILibraryManager.cs b/MediaBrowser.Controller/Library/ILibraryManager.cs
index c8cca1fa93..71b3f054ff 100644
--- a/MediaBrowser.Controller/Library/ILibraryManager.cs
+++ b/MediaBrowser.Controller/Library/ILibraryManager.cs
@@ -107,6 +107,13 @@ namespace MediaBrowser.Controller.Library
         Person? GetPerson(string name);
 
         /// <summary>
+        /// Gets a Person, creating and persisting it if no item exists for the name yet.
+        /// </summary>
+        /// <param name="name">The name of the person.</param>
+        /// <returns>The person.</returns>
+        Person GetOrCreatePerson(string name);
+
+        /// <summary>
         /// Finds the by path.
         /// </summary>
         /// <param name="path">The path.</param>
@@ -153,15 +160,6 @@ namespace MediaBrowser.Controller.Library
         Year GetYear(int value);
 
         /// <summary>
-        /// Validate and refresh the People sub-set of the IBN.
-        /// The items are stored in the db but not loaded into memory until actually requested by an operation.
-        /// </summary>
-        /// <param name="progress">The progress.</param>
-        /// <param name="cancellationToken">The cancellation token.</param>
-        /// <returns>Task.</returns>
-        Task ValidatePeopleAsync(IProgress<double> progress, CancellationToken cancellationToken);
-
-        /// <summary>
         /// Reloads the root media folder.
         /// </summary>
         /// <param name="progress">The progress.</param>