From 9cc47c4fd6c289118d2c4df0d4866faff083cba2 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 24 Aug 2026 21:52:35 +0200 Subject: [PATCH 1/2] Persist the refresh stamp so the people task stops redoing its work --- .../Tasks/PeopleValidationTask.cs | 67 +++++------ .../Manager/MetadataService.cs | 14 ++- .../Manager/MetadataServiceRefreshTests.cs | 112 ++++++++++++++++++ 3 files changed, 154 insertions(+), 39 deletions(-) diff --git a/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs b/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs index bd73f63aa7..afb27ddf9e 100644 --- a/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs +++ b/Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs @@ -177,33 +177,14 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask var thirtyDaysAgo = DateTime.UtcNow.AddDays(-30); var personTypeName = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Person]; + List peopleIds; + var context = await _dbContextFactory.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); await using (context.ConfigureAwait(false)) { - const int PartitionSize = 100; - - var numPeople = await context.BaseItems - .AsNoTracking() - .Where(b => b.Type == personTypeName) - .Where(b => b.DateLastRefreshed == null || b.DateLastRefreshed < thirtyDaysAgo) - .Where(b => - !b.Images!.Any(i => i.ImageType == ImageInfoImageType.Primary) || - string.IsNullOrEmpty(b.Overview)) - .CountAsync(cancellationToken) - .ConfigureAwait(false); - - _logger.LogDebug("Found {Count} people needing image/overview refresh", numPeople); - - if (numPeople == 0) - { - progress.Report(100); - return; - } - - var numComplete = 0; - var numRefreshed = 0; - - await foreach (var entry in context.BaseItems + // Read the candidates in one go rather than paging them. A refresh stamps the person and takes + // it out of this set, so a growing offset over a shrinking set walks past people it never visits. + peopleIds = await context.BaseItems .AsNoTracking() .Where(b => b.Type == personTypeName) .Where(b => b.DateLastRefreshed == null || b.DateLastRefreshed < thirtyDaysAgo) @@ -211,22 +192,36 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask !b.Images!.Any(i => i.ImageType == ImageInfoImageType.Primary) || string.IsNullOrEmpty(b.Overview)) .OrderBy(b => b.Id) - .WithPartitionProgress(partition => _logger.LogDebug("Processing people partition {Partition}", partition)) - .PartitionEagerAsync(PartitionSize, cancellationToken) - .WithCancellation(cancellationToken) - .ConfigureAwait(false)) - { - if (await RefreshPersonAsync(entry.Id, cancellationToken).ConfigureAwait(false)) - { - numRefreshed++; - } + .Select(b => b.Id) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + } - numComplete++; - progress.Report(100.0 * numComplete / numPeople); + _logger.LogDebug("Found {Count} people needing image/overview refresh", peopleIds.Count); + + if (peopleIds.Count == 0) + { + progress.Report(100); + return; + } + + var numComplete = 0; + var numRefreshed = 0; + + foreach (var personId in peopleIds) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (await RefreshPersonAsync(personId, cancellationToken).ConfigureAwait(false)) + { + numRefreshed++; } - _logger.LogInformation("Refreshed metadata for {Count} people missing images or overview", numRefreshed); + numComplete++; + progress.Report(100.0 * numComplete / peopleIds.Count); } + + _logger.LogInformation("Refreshed metadata for {Count} people missing images or overview", numRefreshed); } private async Task RefreshPersonAsync(Guid personId, CancellationToken cancellationToken) diff --git a/MediaBrowser.Providers/Manager/MetadataService.cs b/MediaBrowser.Providers/Manager/MetadataService.cs index 26dc8f9930..fe5285bf65 100644 --- a/MediaBrowser.Providers/Manager/MetadataService.cs +++ b/MediaBrowser.Providers/Manager/MetadataService.cs @@ -212,22 +212,30 @@ namespace MediaBrowser.Providers.Manager var attemptedFetch = refreshOptions.MetadataRefreshMode > MetadataRefreshMode.ValidationOnly || refreshOptions.ImageRefreshMode > MetadataRefreshMode.ValidationOnly; + var refreshStampNeedsSaving = false; + if (hasRefreshedMetadata && hasRefreshedImages && attemptedFetch) { item.DateLastRefreshed = DateTime.UtcNow; updateType |= item.OnMetadataChanged(); + + // A full refresh queries every provider whether or not anything looks stale. When they all + // come back empty the stamp is the only thing that changed, and without it nothing records + // that the lookup happened, so the next pass repeats the same fruitless queries forever. + refreshStampNeedsSaving = refreshOptions.MetadataRefreshMode == MetadataRefreshMode.FullRefresh + || refreshOptions.ImageRefreshMode == MetadataRefreshMode.FullRefresh; } - updateType = await SaveInternal(item, refreshOptions, updateType, isFirstRefresh, requiresRefresh, metadataResult, cancellationToken).ConfigureAwait(false); + updateType = await SaveInternal(item, refreshOptions, updateType, isFirstRefresh, requiresRefresh, refreshStampNeedsSaving, metadataResult, cancellationToken).ConfigureAwait(false); await AfterMetadataRefresh(itemOfType, refreshOptions, cancellationToken).ConfigureAwait(false); return updateType; - async Task SaveInternal(BaseItem item, MetadataRefreshOptions refreshOptions, ItemUpdateType updateType, bool isFirstRefresh, bool requiresRefresh, MetadataResult metadataResult, CancellationToken cancellationToken) + async Task SaveInternal(BaseItem item, MetadataRefreshOptions refreshOptions, ItemUpdateType updateType, bool isFirstRefresh, bool requiresRefresh, bool refreshStampNeedsSaving, MetadataResult metadataResult, CancellationToken cancellationToken) { // Save if changes were made, or it's never been saved before - if (refreshOptions.ForceSave || updateType > ItemUpdateType.None || isFirstRefresh || refreshOptions.ReplaceAllMetadata || requiresRefresh) + if (refreshOptions.ForceSave || updateType > ItemUpdateType.None || isFirstRefresh || refreshOptions.ReplaceAllMetadata || requiresRefresh || refreshStampNeedsSaving) { if (item.IsFileProtocol) { diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs index 1d2fb2e760..465a032328 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs @@ -4,6 +4,7 @@ using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Jellyfin.Data.Enums; +using MediaBrowser.Controller; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; @@ -11,8 +12,10 @@ using MediaBrowser.Controller.IO; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Providers; +using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; +using MediaBrowser.Model.MediaInfo; using MediaBrowser.Providers.Manager; using Microsoft.Extensions.Logging.Abstractions; using Moq; @@ -228,6 +231,100 @@ namespace Jellyfin.Providers.Tests.Manager Assert.Equal("nm0000123", mergedPerson.GetProviderId(MetadataProvider.Imdb)); } + [Theory] + [InlineData(MetadataRefreshMode.FullRefresh, true)] + [InlineData(MetadataRefreshMode.Default, false)] + public async Task RefreshMetadata_ProvidersFoundNothing_PersistsRefreshDateOnFullRefresh(MetadataRefreshMode mode, bool expectSaved) + { + var peoplePath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "people"); + + var item = new Person + { + Id = Guid.NewGuid(), + Name = "Test Person", + Path = System.IO.Path.Combine(peoplePath, "T", "Test Person"), + PreferredMetadataLanguage = "en", + PreferredMetadataCountryCode = "US", + DateLastRefreshed = DateTime.UtcNow.AddDays(-60), + DateLastSaved = DateTime.UtcNow.AddDays(-60) + }; + item.PresentationUniqueKey = item.CreatePresentationUniqueKey(); + + var stampBefore = item.DateLastRefreshed; + + var provider = new Mock>(MockBehavior.Loose); + provider.Setup(p => p.Name).Returns("Provider"); + provider.Setup(p => p.GetMetadata(It.IsAny(), It.IsAny())) + .ReturnsAsync(new MetadataResult { HasMetadata = false }); + + var libraryOptions = new LibraryOptions(); + + var libraryManager = new Mock(MockBehavior.Loose); + libraryManager.Setup(l => l.GetLibraryOptions(It.IsAny())).Returns(libraryOptions); + + var providerManager = new Mock(MockBehavior.Loose); + providerManager.Setup(p => p.GetImageProviders(It.IsAny(), It.IsAny())) + .Returns(Array.Empty()); + providerManager.Setup(p => p.GetMetadataProviders(It.IsAny(), It.IsAny())) + .Returns(new[] { (IMetadataProvider)provider.Object }); + providerManager.Setup(p => p.GetMetadataSavers(It.IsAny(), It.IsAny())) + .Returns(Array.Empty()); + + var itemRepository = new Mock(MockBehavior.Loose); + itemRepository.Setup(r => r.ItemExistsAsync(It.IsAny())).ReturnsAsync(true); + + var applicationPaths = new Mock(MockBehavior.Loose); + applicationPaths.Setup(a => a.PeoplePath).Returns(peoplePath); + var configurationManager = new Mock(MockBehavior.Loose); + configurationManager.Setup(c => c.ApplicationPaths).Returns(applicationPaths.Object); + configurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + + var fileSystem = new Mock(MockBehavior.Loose); + fileSystem.Setup(f => f.GetFileSystemInfo(It.IsAny())).Returns(new FileSystemMetadata { Exists = false }); + fileSystem.Setup(f => f.GetValidFilename(It.IsAny())).Returns(name => name); + + var mediaSourceManager = new Mock(MockBehavior.Loose); + mediaSourceManager.Setup(m => m.GetPathProtocol(It.IsAny())).Returns(MediaProtocol.File); + + var previousLibraryManager = BaseItem.LibraryManager; + var previousConfigurationManager = BaseItem.ConfigurationManager; + var previousFileSystem = BaseItem.FileSystem; + var previousMediaSourceManager = BaseItem.MediaSourceManager; + BaseItem.LibraryManager = libraryManager.Object; + BaseItem.ConfigurationManager = configurationManager.Object; + BaseItem.FileSystem = fileSystem.Object; + BaseItem.MediaSourceManager = mediaSourceManager.Object; + try + { + var service = new TestPersonMetadataService(libraryManager.Object, providerManager.Object, itemRepository.Object, fileSystem.Object); + + await service.RefreshMetadata( + item, + new MetadataRefreshOptions(Mock.Of()) + { + MetadataRefreshMode = mode, + ImageRefreshMode = mode + }, + CancellationToken.None).ConfigureAwait(true); + } + finally + { + BaseItem.LibraryManager = previousLibraryManager; + BaseItem.ConfigurationManager = previousConfigurationManager; + BaseItem.FileSystem = previousFileSystem; + BaseItem.MediaSourceManager = previousMediaSourceManager; + } + + libraryManager.Verify( + l => l.UpdateItemAsync(item, It.IsAny(), It.IsAny(), It.IsAny()), + expectSaved ? Times.Once() : Times.Never()); + + if (expectSaved) + { + Assert.True(item.DateLastRefreshed > stampBefore); + } + } + private sealed class TestMetadataService : MetadataService { public TestMetadataService() @@ -249,5 +346,20 @@ namespace Jellyfin.Providers.Tests.Manager ICollection providers) => RefreshWithProviders(metadata, id, options, providers, ImageProvider, false, CancellationToken.None); } + + private sealed class TestPersonMetadataService : MetadataService + { + public TestPersonMetadataService(ILibraryManager libraryManager, IProviderManager providerManager, IItemRepository itemRepository, IFileSystem fileSystem) + : base( + Mock.Of(), + NullLogger>.Instance, + providerManager, + fileSystem, + libraryManager, + Mock.Of(), + itemRepository) + { + } + } } } From 38093e2952f634ddbdeaede12c30b0566c64a464 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 24 Aug 2026 22:27:11 +0200 Subject: [PATCH 2/2] Fix test --- .../Manager/MetadataServiceRefreshTests.cs | 111 ++++++++---------- 1 file changed, 49 insertions(+), 62 deletions(-) diff --git a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs index 465a032328..3b4d6fc9bb 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/MetadataServiceRefreshTests.cs @@ -1,10 +1,10 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Jellyfin.Data.Enums; -using MediaBrowser.Controller; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; @@ -15,7 +15,6 @@ using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; -using MediaBrowser.Model.MediaInfo; using MediaBrowser.Providers.Manager; using Microsoft.Extensions.Logging.Abstractions; using Moq; @@ -236,13 +235,10 @@ namespace Jellyfin.Providers.Tests.Manager [InlineData(MetadataRefreshMode.Default, false)] public async Task RefreshMetadata_ProvidersFoundNothing_PersistsRefreshDateOnFullRefresh(MetadataRefreshMode mode, bool expectSaved) { - var peoplePath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "people"); - - var item = new Person + var item = new TestItem { Id = Guid.NewGuid(), - Name = "Test Person", - Path = System.IO.Path.Combine(peoplePath, "T", "Test Person"), + Name = "Test Item", PreferredMetadataLanguage = "en", PreferredMetadataCountryCode = "US", DateLastRefreshed = DateTime.UtcNow.AddDays(-60), @@ -252,72 +248,38 @@ namespace Jellyfin.Providers.Tests.Manager var stampBefore = item.DateLastRefreshed; - var provider = new Mock>(MockBehavior.Loose); + var provider = new Mock>(MockBehavior.Loose); provider.Setup(p => p.Name).Returns("Provider"); - provider.Setup(p => p.GetMetadata(It.IsAny(), It.IsAny())) - .ReturnsAsync(new MetadataResult { HasMetadata = false }); - - var libraryOptions = new LibraryOptions(); + provider.Setup(p => p.GetMetadata(It.IsAny(), It.IsAny())) + .ReturnsAsync(new MetadataResult { HasMetadata = false }); var libraryManager = new Mock(MockBehavior.Loose); - libraryManager.Setup(l => l.GetLibraryOptions(It.IsAny())).Returns(libraryOptions); + libraryManager.Setup(l => l.GetLibraryOptions(It.IsAny())).Returns(new LibraryOptions()); var providerManager = new Mock(MockBehavior.Loose); providerManager.Setup(p => p.GetImageProviders(It.IsAny(), It.IsAny())) .Returns(Array.Empty()); - providerManager.Setup(p => p.GetMetadataProviders(It.IsAny(), It.IsAny())) - .Returns(new[] { (IMetadataProvider)provider.Object }); + providerManager.Setup(p => p.GetMetadataProviders(It.IsAny(), It.IsAny())) + .Returns(new[] { (IMetadataProvider)provider.Object }); providerManager.Setup(p => p.GetMetadataSavers(It.IsAny(), It.IsAny())) .Returns(Array.Empty()); var itemRepository = new Mock(MockBehavior.Loose); itemRepository.Setup(r => r.ItemExistsAsync(It.IsAny())).ReturnsAsync(true); - var applicationPaths = new Mock(MockBehavior.Loose); - applicationPaths.Setup(a => a.PeoplePath).Returns(peoplePath); - var configurationManager = new Mock(MockBehavior.Loose); - configurationManager.Setup(c => c.ApplicationPaths).Returns(applicationPaths.Object); - configurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + var service = new TestItemMetadataService(libraryManager.Object, providerManager.Object, itemRepository.Object); - var fileSystem = new Mock(MockBehavior.Loose); - fileSystem.Setup(f => f.GetFileSystemInfo(It.IsAny())).Returns(new FileSystemMetadata { Exists = false }); - fileSystem.Setup(f => f.GetValidFilename(It.IsAny())).Returns(name => name); + await service.RefreshMetadata( + item, + new MetadataRefreshOptions(Mock.Of()) + { + MetadataRefreshMode = mode, + ImageRefreshMode = mode + }, + CancellationToken.None).ConfigureAwait(true); - var mediaSourceManager = new Mock(MockBehavior.Loose); - mediaSourceManager.Setup(m => m.GetPathProtocol(It.IsAny())).Returns(MediaProtocol.File); - - var previousLibraryManager = BaseItem.LibraryManager; - var previousConfigurationManager = BaseItem.ConfigurationManager; - var previousFileSystem = BaseItem.FileSystem; - var previousMediaSourceManager = BaseItem.MediaSourceManager; - BaseItem.LibraryManager = libraryManager.Object; - BaseItem.ConfigurationManager = configurationManager.Object; - BaseItem.FileSystem = fileSystem.Object; - BaseItem.MediaSourceManager = mediaSourceManager.Object; - try - { - var service = new TestPersonMetadataService(libraryManager.Object, providerManager.Object, itemRepository.Object, fileSystem.Object); - - await service.RefreshMetadata( - item, - new MetadataRefreshOptions(Mock.Of()) - { - MetadataRefreshMode = mode, - ImageRefreshMode = mode - }, - CancellationToken.None).ConfigureAwait(true); - } - finally - { - BaseItem.LibraryManager = previousLibraryManager; - BaseItem.ConfigurationManager = previousConfigurationManager; - BaseItem.FileSystem = previousFileSystem; - BaseItem.MediaSourceManager = previousMediaSourceManager; - } - - libraryManager.Verify( - l => l.UpdateItemAsync(item, It.IsAny(), It.IsAny(), It.IsAny()), - expectSaved ? Times.Once() : Times.Never()); + // Nothing was found, so on a full refresh the advanced stamp is the only reason to write the row. + Assert.Equal(expectSaved, item.Saved); if (expectSaved) { @@ -325,6 +287,31 @@ namespace Jellyfin.Providers.Tests.Manager } } + /// + /// Stands in for a real item so the refresh stays off the shared BaseItem statics, which other + /// test classes in this assembly overwrite while xUnit runs them in parallel. + /// + internal sealed class TestItem : BaseItem + { + public bool Saved { get; private set; } + + public override bool RequiresRefresh() => false; + + public override bool IsSaveLocalMetadataEnabled() => false; + + public override string CreatePresentationUniqueKey() => Id.ToString("N", CultureInfo.InvariantCulture); + + public override ItemUpdateType OnMetadataChanged() => ItemUpdateType.None; + + public override bool BeforeMetadataRefresh(bool replaceAllMetadata) => false; + + public override Task UpdateToRepositoryAsync(ItemUpdateType updateReason, CancellationToken cancellationToken) + { + Saved = true; + return Task.CompletedTask; + } + } + private sealed class TestMetadataService : MetadataService { public TestMetadataService() @@ -347,14 +334,14 @@ namespace Jellyfin.Providers.Tests.Manager => RefreshWithProviders(metadata, id, options, providers, ImageProvider, false, CancellationToken.None); } - private sealed class TestPersonMetadataService : MetadataService + private sealed class TestItemMetadataService : MetadataService { - public TestPersonMetadataService(ILibraryManager libraryManager, IProviderManager providerManager, IItemRepository itemRepository, IFileSystem fileSystem) + public TestItemMetadataService(ILibraryManager libraryManager, IProviderManager providerManager, IItemRepository itemRepository) : base( Mock.Of(), - NullLogger>.Instance, + NullLogger>.Instance, providerManager, - fileSystem, + Mock.Of(), libraryManager, Mock.Of(), itemRepository)