diff options
3 files changed, 150 insertions, 35 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,56 +177,51 @@ 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 + // 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) .Where(b => !b.Images!.Any(i => i.ImageType == ImageInfoImageType.Primary) || string.IsNullOrEmpty(b.Overview)) - .CountAsync(cancellationToken) + .OrderBy(b => b.Id) + .Select(b => b.Id) + .ToListAsync(cancellationToken) .ConfigureAwait(false); + } - _logger.LogDebug("Found {Count} people needing image/overview refresh", numPeople); + _logger.LogDebug("Found {Count} people needing image/overview refresh", peopleIds.Count); - if (numPeople == 0) - { - progress.Report(100); - return; - } + if (peopleIds.Count == 0) + { + progress.Report(100); + return; + } - var numComplete = 0; - var numRefreshed = 0; + var numComplete = 0; + var numRefreshed = 0; - await foreach (var entry in 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)) - .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++; - } + foreach (var personId in peopleIds) + { + cancellationToken.ThrowIfCancellationRequested(); - numComplete++; - progress.Report(100.0 * numComplete / numPeople); + 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) 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<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) { 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<IRemoteMetadataProvider<Person, PersonLookupInfo>>(MockBehavior.Loose); + provider.Setup(p => p.Name).Returns("Provider"); + provider.Setup(p => p.GetMetadata(It.IsAny<PersonLookupInfo>(), It.IsAny<CancellationToken>())) + .ReturnsAsync(new MetadataResult<Person> { HasMetadata = false }); + + var libraryOptions = new LibraryOptions(); + + var libraryManager = new Mock<ILibraryManager>(MockBehavior.Loose); + libraryManager.Setup(l => l.GetLibraryOptions(It.IsAny<BaseItem>())).Returns(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<Person>(It.IsAny<BaseItem>(), It.IsAny<LibraryOptions>())) + .Returns(new[] { (IMetadataProvider<Person>)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 applicationPaths = new Mock<IServerApplicationPaths>(MockBehavior.Loose); + applicationPaths.Setup(a => a.PeoplePath).Returns(peoplePath); + var configurationManager = new Mock<IServerConfigurationManager>(MockBehavior.Loose); + configurationManager.Setup(c => c.ApplicationPaths).Returns(applicationPaths.Object); + configurationManager.Setup(c => c.Configuration).Returns(new ServerConfiguration()); + + var fileSystem = new Mock<IFileSystem>(MockBehavior.Loose); + fileSystem.Setup(f => f.GetFileSystemInfo(It.IsAny<string>())).Returns(new FileSystemMetadata { Exists = false }); + fileSystem.Setup(f => f.GetValidFilename(It.IsAny<string>())).Returns<string>(name => name); + + var mediaSourceManager = new Mock<IMediaSourceManager>(MockBehavior.Loose); + mediaSourceManager.Setup(m => m.GetPathProtocol(It.IsAny<string>())).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<IDirectoryService>()) + { + 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<BaseItem>(), It.IsAny<ItemUpdateType>(), It.IsAny<CancellationToken>()), + expectSaved ? Times.Once() : Times.Never()); + + if (expectSaved) + { + Assert.True(item.DateLastRefreshed > stampBefore); + } + } + private sealed class TestMetadataService : MetadataService<Movie, MovieInfo> { public TestMetadataService() @@ -249,5 +346,20 @@ namespace Jellyfin.Providers.Tests.Manager ICollection<IMetadataProvider> providers) => RefreshWithProviders(metadata, id, options, providers, ImageProvider, false, CancellationToken.None); } + + private sealed class TestPersonMetadataService : MetadataService<Person, PersonLookupInfo> + { + public TestPersonMetadataService(ILibraryManager libraryManager, IProviderManager providerManager, IItemRepository itemRepository, IFileSystem fileSystem) + : base( + Mock.Of<IServerConfigurationManager>(), + NullLogger<MetadataService<Person, PersonLookupInfo>>.Instance, + providerManager, + fileSystem, + libraryManager, + Mock.Of<IExternalDataManager>(), + itemRepository) + { + } + } } } |