Merge pull request #17709 from Shadowghost/fix-people-task-perist

Persist the refresh stamp so the people task stops redoing its work
This commit is contained in:
Cody Robibero
2026-08-25 19:10:44 -04:00
committed by GitHub
3 changed files with 141 additions and 39 deletions
@@ -177,33 +177,14 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
var thirtyDaysAgo = DateTime.UtcNow.AddDays(-30);
var personTypeName = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Person];
List<Guid> 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<bool> RefreshPersonAsync(Guid personId, CancellationToken cancellationToken)
@@ -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<ItemUpdateType> SaveInternal(BaseItem item, MetadataRefreshOptions refreshOptions, ItemUpdateType updateType, bool isFirstRefresh, bool requiresRefresh, MetadataResult<TItemType> metadataResult, CancellationToken cancellationToken)
async Task<ItemUpdateType> SaveInternal(BaseItem item, MetadataRefreshOptions refreshOptions, ItemUpdateType updateType, bool isFirstRefresh, bool requiresRefresh, bool refreshStampNeedsSaving, MetadataResult<TItemType> 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)
{
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
@@ -11,6 +12,7 @@ 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.Providers.Manager;
@@ -228,6 +230,88 @@ 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 item = new TestItem
{
Id = Guid.NewGuid(),
Name = "Test Item",
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<IRemoteMetadataProvider<TestItem, ItemLookupInfo>>(MockBehavior.Loose);
provider.Setup(p => p.Name).Returns("Provider");
provider.Setup(p => p.GetMetadata(It.IsAny<ItemLookupInfo>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(new MetadataResult<TestItem> { HasMetadata = false });
var libraryManager = new Mock<ILibraryManager>(MockBehavior.Loose);
libraryManager.Setup(l => l.GetLibraryOptions(It.IsAny<BaseItem>())).Returns(new LibraryOptions());
var providerManager = new Mock<IProviderManager>(MockBehavior.Loose);
providerManager.Setup(p => p.GetImageProviders(It.IsAny<BaseItem>(), It.IsAny<ImageRefreshOptions>()))
.Returns(Array.Empty<IImageProvider>());
providerManager.Setup(p => p.GetMetadataProviders<TestItem>(It.IsAny<BaseItem>(), It.IsAny<LibraryOptions>()))
.Returns(new[] { (IMetadataProvider<TestItem>)provider.Object });
providerManager.Setup(p => p.GetMetadataSavers(It.IsAny<BaseItem>(), It.IsAny<LibraryOptions>()))
.Returns(Array.Empty<IMetadataSaver>());
var itemRepository = new Mock<IItemRepository>(MockBehavior.Loose);
itemRepository.Setup(r => r.ItemExistsAsync(It.IsAny<Guid>())).ReturnsAsync(true);
var service = new TestItemMetadataService(libraryManager.Object, providerManager.Object, itemRepository.Object);
await service.RefreshMetadata(
item,
new MetadataRefreshOptions(Mock.Of<IDirectoryService>())
{
MetadataRefreshMode = mode,
ImageRefreshMode = mode
},
CancellationToken.None).ConfigureAwait(true);
// 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)
{
Assert.True(item.DateLastRefreshed > stampBefore);
}
}
/// <summary>
/// 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.
/// </summary>
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<Movie, MovieInfo>
{
public TestMetadataService()
@@ -249,5 +333,20 @@ namespace Jellyfin.Providers.Tests.Manager
ICollection<IMetadataProvider> providers)
=> RefreshWithProviders(metadata, id, options, providers, ImageProvider, false, CancellationToken.None);
}
private sealed class TestItemMetadataService : MetadataService<TestItem, ItemLookupInfo>
{
public TestItemMetadataService(ILibraryManager libraryManager, IProviderManager providerManager, IItemRepository itemRepository)
: base(
Mock.Of<IServerConfigurationManager>(),
NullLogger<MetadataService<TestItem, ItemLookupInfo>>.Instance,
providerManager,
Mock.Of<IFileSystem>(),
libraryManager,
Mock.Of<IExternalDataManager>(),
itemRepository)
{
}
}
}
}