From 080b02cc4c9879d92de725a763527fb7285cb181 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Sun, 31 Oct 2021 02:40:15 +0200 Subject: [PATCH] Add comments, minor cleanup, add tests --- .../Manager/ItemImageProvider.cs | 52 +- .../Jellyfin.Providers.Tests.csproj | 6 + .../Manager/ItemImageProviderTests.cs | 674 ++++++++++++++++++ .../Test Data/Images/blank0.jpg | 0 .../Test Data/Images/blank1.jpg | 0 5 files changed, 720 insertions(+), 12 deletions(-) create mode 100644 tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs create mode 100644 tests/Jellyfin.Providers.Tests/Test Data/Images/blank0.jpg create mode 100644 tests/Jellyfin.Providers.Tests/Test Data/Images/blank1.jpg diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index 39372acb94..49b7a5d6b8 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -1,7 +1,5 @@ #nullable disable -#pragma warning disable CA1002, CS1591 - using System; using System.Collections.Generic; using System.IO; @@ -25,6 +23,9 @@ using Microsoft.Extensions.Logging; namespace MediaBrowser.Providers.Manager { + /// + /// Utilities for managing images attached to items. + /// public class ItemImageProvider { private readonly ILogger _logger; @@ -47,6 +48,12 @@ namespace MediaBrowser.Providers.Manager ImageType.Thumb }; + /// + /// Initializes a new instance of the class. + /// + /// The logger. + /// The provider manager for interacting with provider image references. + /// The filesystem. public ItemImageProvider(ILogger logger, IProviderManager providerManager, IFileSystem fileSystem) { _logger = logger; @@ -54,6 +61,13 @@ namespace MediaBrowser.Providers.Manager _fileSystem = fileSystem; } + /// + /// Verifies existing images have valid paths and adds any new local images provided. + /// + /// The to validate images for. + /// The providers to use, must include (s) for local scanning. + /// The directory service for s to use. + /// true if changes were made to the item; otherwise false. public bool ValidateImages(BaseItem item, IEnumerable providers, IDirectoryService directoryService) { var hasChanges = false; @@ -73,6 +87,15 @@ namespace MediaBrowser.Providers.Manager return hasChanges; } + /// + /// Refreshes from the providers according to the given options. + /// + /// The to gather images for. + /// The library options. + /// The providers to query for images. + /// The refresh options. + /// The cancellation token. + /// The refresh result. public async Task RefreshImages( BaseItem item, LibraryOptions libraryOptions, @@ -118,7 +141,7 @@ namespace MediaBrowser.Providers.Manager } /// - /// Refreshes from provider. + /// Refreshes from a dynamic provider. /// private async Task RefreshFromProvider( BaseItem item, @@ -234,7 +257,7 @@ namespace MediaBrowser.Providers.Manager } /// - /// Refreshes from provider. + /// Refreshes from a remote provider. /// /// The item. /// The provider. @@ -305,12 +328,12 @@ namespace MediaBrowser.Providers.Manager } minWidth = savedOptions.GetMinWidth(ImageType.Backdrop); - await DownloadBackdrops(item, ImageType.Backdrop, backdropLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); + await DownloadMultiImages(item, ImageType.Backdrop, backdropLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); - if (item is IHasScreenshots hasScreenshots) + if (item is IHasScreenshots) { minWidth = savedOptions.GetMinWidth(ImageType.Screenshot); - await DownloadBackdrops(item, ImageType.Screenshot, screenshotLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); + await DownloadMultiImages(item, ImageType.Screenshot, screenshotLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); } } catch (OperationCanceledException) @@ -360,6 +383,12 @@ namespace MediaBrowser.Providers.Manager } } + /// + /// Merges a list of images into the provided item, validating existing images and replacing them or adding new images as necessary. + /// + /// The to modify. + /// The new images to place in item. + /// true if changes were made to the item; otherwise false. public bool MergeImages(BaseItem item, IReadOnlyList images) { var changed = false; @@ -417,8 +446,7 @@ namespace MediaBrowser.Providers.Manager changed = true; } - var hasScreenshots = item as IHasScreenshots; - if (hasScreenshots != null) + if (item is IHasScreenshots) { if (UpdateMultiImages(item, images, ImageType.Screenshot)) { @@ -536,7 +564,7 @@ namespace MediaBrowser.Providers.Manager return true; } - if (item is IItemByName && item is not MusicArtist) + if (item is IItemByName and not MusicArtist) { var hasDualAccess = item as IHasDualAccess; if (hasDualAccess == null || hasDualAccess.IsAccessedByName) @@ -569,7 +597,7 @@ namespace MediaBrowser.Providers.Manager newIndex); } - private async Task DownloadBackdrops(BaseItem item, ImageType imageType, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable images, int minWidth, CancellationToken cancellationToken) + private async Task DownloadMultiImages(BaseItem item, ImageType imageType, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable images, int minWidth, CancellationToken cancellationToken) { foreach (var image in images.Where(i => i.Type == imageType)) { @@ -609,7 +637,7 @@ namespace MediaBrowser.Providers.Manager break; } - // If there's already an image of the same size, skip it + // If there's already an image of the same file size, skip it if (response.Content.Headers.ContentLength.HasValue) { try diff --git a/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj b/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj index 0b2db64b0b..9fb1a4364e 100644 --- a/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj +++ b/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj @@ -6,6 +6,12 @@ ../jellyfin-tests.ruleset + + + PreserveNewest + + + diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs new file mode 100644 index 0000000000..253bcb7cad --- /dev/null +++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs @@ -0,0 +1,674 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Entities.Movies; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Providers; +using MediaBrowser.Model.Configuration; +using MediaBrowser.Model.Drawing; +using MediaBrowser.Model.Entities; +using MediaBrowser.Model.IO; +using MediaBrowser.Model.MediaInfo; +using MediaBrowser.Model.Providers; +using MediaBrowser.Providers.Manager; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Providers.Tests.Manager +{ + public class ItemImageProviderTests + { + private static readonly string TestDataImagePath = "Test Data/Images/blank{0}.jpg"; + + [Fact] + public void ValidateImages_PhotoEmptyProviders_NoChange() + { + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.ValidateImages(new Photo(), new List(), null); + + Assert.False(changed); + } + + [Fact] + public void ValidateImages_EmptyItemEmptyProviders_NoChange() + { + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.ValidateImages(new MovieWithScreenshots(), new List(), null); + + Assert.False(changed); + } + + private static TheoryData GetImageTypesWithCount() + { + var theoryTypes = new TheoryData(); + + // shotgun approach; overkill for frequent runs + // foreach (var imageType in (ImageType[])Enum.GetValues(typeof(ImageType))) + // { + // switch (imageType) + // { + // case ImageType.Chapter: + // case ImageType.Profile: + // // skip types that can't be set using BaseItem.SetImagePath or otherwise don't apply to BaseItem + // break; + // case ImageType.Backdrop: + // case ImageType.Screenshot: + // // for types that support multiple test with 1 and with more than 1 + // theoryTypes.Add(imageType, 1); + // theoryTypes.Add(imageType, 2); + // break; + // default: + // // for singular types just test with 1 + // theoryTypes.Add(imageType, 1); + // break; + // } + // } + + // specific test cases that hit different handling + theoryTypes.Add(ImageType.Primary, 1); + theoryTypes.Add(ImageType.Backdrop, 1); + theoryTypes.Add(ImageType.Backdrop, 2); + theoryTypes.Add(ImageType.Screenshot, 1); + + return theoryTypes; + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void ValidateImages_EmptyItemAndPopulatedProviders_AddsImages(ImageType imageType, int imageCount) + { + // Has to exist for querying DateModified time on file, results stored but not checked so not populating + BaseItem.FileSystem = Mock.Of(); + + var item = new MovieWithScreenshots(); + var imageProvider = GetImageProvider(imageType, imageCount, true); + + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.ValidateImages(item, new List { imageProvider }, null); + + Assert.True(changed); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void ValidateImages_PopulatedItemWithGoodPathsAndEmptyProviders_NoChange(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, true); + + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.ValidateImages(item, new List(), null); + + Assert.False(changed); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void ValidateImages_PopulatedItemWithBadPathsAndEmptyProviders_RemovesImage(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, false); + + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.ValidateImages(item, new List(), null); + + Assert.True(changed); + Assert.Empty(item.GetImages(imageType)); + } + + [Fact] + public void MergeImages_EmptyItemNewImagesEmpty_NoChange() + { + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.MergeImages(new MovieWithScreenshots(), new List()); + + Assert.False(changed); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void MergeImages_PopulatedItemWithGoodPathsAndPopulatedNewImages_AddsUpdatesImages(ImageType imageType, int imageCount) + { + // valid and not valid paths - should replace the valid paths with the invalid ones + var item = GetItemWithImages(imageType, imageCount, true); + var images = GetImages(imageType, imageCount, false); + + var itemImageProvider = GetItemImageProvider(null, null); + var changed = itemImageProvider.MergeImages(item, images); + + Assert.True(changed); + // adds for types that allow multiple, replaces singular type images + if (item.AllowsMultipleImages(imageType)) + { + Assert.Equal(imageCount * 2, item.GetImages(imageType).Count()); + } + else + { + Assert.Single(item.GetImages(imageType)); + Assert.Same(images[0].FileInfo.FullName, item.GetImages(imageType).First().Path); + } + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void MergeImages_PopulatedItemWithGoodPathsAndSameNewImages_NoChange(ImageType imageType, int imageCount) + { + var oldTime = new DateTime(1970, 1, 1); + + // match update time with time added to item images (unix epoch) + var fileSystem = new Mock(); + fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny())) + .Returns(oldTime); + BaseItem.FileSystem = fileSystem.Object; + + // all valid paths - matching for strictly updating + var item = GetItemWithImages(imageType, imageCount, true); + // set size to non-zero to allow for updates to occur + foreach (var image in item.GetImages(imageType)) + { + image.DateModified = oldTime; + image.Height = 1; + image.Width = 1; + } + + var images = GetImages(imageType, imageCount, true); + + var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); + var changed = itemImageProvider.MergeImages(item, images); + + Assert.False(changed); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public void MergeImages_PopulatedItemWithGoodPathsAndSameNewImagesWithNewTimestamps_ResetsImageSizes(ImageType imageType, int imageCount) + { + var oldTime = new DateTime(1970, 1, 1); + var updatedTime = new DateTime(2021, 1, 1); + + var fileSystem = new Mock(); + fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny())) + .Returns(updatedTime); + BaseItem.FileSystem = fileSystem.Object; + + // all valid paths - matching for strictly updating + var item = GetItemWithImages(imageType, imageCount, true); + // set size to non-zero to allow for image size reset to occur + foreach (var image in item.GetImages(imageType)) + { + image.DateModified = oldTime; + image.Height = 1; + image.Width = 1; + } + + var images = GetImages(imageType, imageCount, true); + + var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); + var changed = itemImageProvider.MergeImages(item, images); + + Assert.True(changed); + // before and after paths are the same, verify updated by size reset to 0 + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + foreach (var image in item.GetImages(imageType)) + { + Assert.Equal(updatedTime, image.DateModified); + Assert.Equal(0, image.Height); + Assert.Equal(0, image.Width); + } + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_PopulatedItemPopulatedProviderDynamic_NoChange(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, true); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var dynamicProvider = new Mock(MockBehavior.Strict); + dynamicProvider.Setup(rp => rp.Name).Returns("MockDynamicProvider"); + dynamicProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + + var refreshOptions = new ImageRefreshOptions(null); + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, It.IsAny())) + .Callback((callbackItem, _, _, callbackType, _, _) => callbackItem.SetImagePath(callbackType, 0, new FileSystemMetadata())) + .Returns(Task.CompletedTask); + var itemImageProvider = GetItemImageProvider(providerManager.Object, null); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_EmptyItemPopulatedProviderDynamicWithPath_AddsImages(ImageType imageType, int imageCount) + { + // Has to exist for querying DateModified time on file, results stored but not checked so not populating + BaseItem.FileSystem = Mock.Of(); + + var item = new MovieWithScreenshots(); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + // Path must exist: is read in as a stream by AsyncFile.OpenRead + var imageResponse = new DynamicImageResponse + { + HasImage = true, + Format = ImageFormat.Jpg, + Path = string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 0), + Protocol = MediaProtocol.File + }; + + var dynamicProvider = new Mock(MockBehavior.Strict); + dynamicProvider.Setup(rp => rp.Name).Returns("MockDynamicProvider"); + dynamicProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + dynamicProvider.Setup(rp => rp.GetImage(item, imageType, It.IsAny())) + .ReturnsAsync(imageResponse); + + var refreshOptions = new ImageRefreshOptions(null); + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, It.IsAny())) + .Callback((callbackItem, _, _, callbackType, _, _) => callbackItem.SetImagePath(callbackType, 0, new FileSystemMetadata())) + .Returns(Task.CompletedTask); + var itemImageProvider = GetItemImageProvider(providerManager.Object, null); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + // dynamic provider unable to return multiple images + Assert.Single(item.GetImages(imageType)); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_EmptyItemPopulatedProviderDynamicWithoutPath_AddsImages(ImageType imageType, int imageCount) + { + // Has to exist for querying DateModified time on file, results stored but not checked so not populating + BaseItem.FileSystem = Mock.Of(); + + var item = new MovieWithScreenshots(); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var imageResponse = new DynamicImageResponse + { + HasImage = true, + Format = ImageFormat.Jpg, + Protocol = MediaProtocol.File + }; + + var dynamicProvider = new Mock(MockBehavior.Strict); + dynamicProvider.Setup(rp => rp.Name).Returns("MockDynamicProvider"); + dynamicProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + dynamicProvider.Setup(rp => rp.GetImage(item, imageType, It.IsAny())) + .ReturnsAsync(imageResponse); + + var refreshOptions = new ImageRefreshOptions(null); + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, It.IsAny())) + .Callback((callbackItem, _, _, callbackType, _, _) => callbackItem.SetImagePath(callbackType, 0, new FileSystemMetadata())) + .Returns(Task.CompletedTask); + var itemImageProvider = GetItemImageProvider(providerManager.Object, null); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + // dynamic provider unable to return multiple images + Assert.Single(item.GetImages(imageType)); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_PopulatedItemPopulatedProviderDynamicFullRefresh_UpdatesImages(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, false); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var expectedPath = "dynamic response path url"; + var imageResponse = new DynamicImageResponse + { + HasImage = true, + Format = ImageFormat.Jpg, + Path = expectedPath, + Protocol = MediaProtocol.Http + }; + + var dynamicProvider = new Mock(MockBehavior.Strict); + dynamicProvider.Setup(rp => rp.Name).Returns("MockDynamicProvider"); + dynamicProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + dynamicProvider.Setup(rp => rp.GetImage(item, imageType, It.IsAny())) + .ReturnsAsync(imageResponse); + + var refreshOptions = new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllImages = true + }; + + var itemImageProvider = GetItemImageProvider(null, Mock.Of()); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + // dynamic provider unable to return multiple images + Assert.Single(item.GetImages(imageType)); + Assert.Equal(expectedPath, item.GetImagePath(imageType, 0)); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_PopulatedItemPopulatedProviderRemote_NoChange(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, false); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var remoteProvider = new Mock(MockBehavior.Strict); + remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); + remoteProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + + var refreshOptions = new ImageRefreshOptions(null); + + var remoteInfo = new List(); + for (int i = 0; i < imageCount; i++) + { + remoteInfo.Add(new RemoteImageInfo + { + Type = imageType, + Url = "image url " + i, + Width = 1 // min width is set to 0, this will always pass + }); + } + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(remoteInfo); + var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of()); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_EmptyNonStubItemPopulatedProviderRemote_DownloadsImages(ImageType imageType, int imageCount) + { + // Has to exist for querying DateModified time on file, results stored but not checked so not populating + BaseItem.FileSystem ??= Mock.Of(); + + // Set path and media source manager so images will be downloaded (EnableImageStub will return false) + var item = new MovieWithScreenshots + { + Path = "non-empty path" + }; + BaseItem.MediaSourceManager = Mock.Of(); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var remoteProvider = new Mock(MockBehavior.Strict); + remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); + remoteProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + remoteProvider.Setup(rp => rp.GetImageResponse(It.IsAny(), It.IsAny())) + .ReturnsAsync((string url, CancellationToken _) => new HttpResponseMessage + { + ReasonPhrase = url, + StatusCode = HttpStatusCode.OK, + Content = new StringContent("Content", Encoding.UTF8, "image/jpeg") + }); + + var refreshOptions = new ImageRefreshOptions(null); + + var remoteInfo = new List(); + for (int i = 0; i < imageCount; i++) + { + remoteInfo.Add(new RemoteImageInfo + { + Type = imageType, + Url = "image url " + i, + Width = 1 // min width is set to 0, this will always pass + }); + } + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(remoteInfo); + providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, It.IsAny())) + .Callback((callbackItem, _, _, callbackType, _, _) => + callbackItem.SetImagePath(callbackType, callbackItem.AllowsMultipleImages(callbackType) ? callbackItem.GetImages(callbackType).Count() : 0, new FileSystemMetadata())) + .Returns(Task.CompletedTask); + var fileSystem = new Mock(); + fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny())) + .Returns(new FileSystemMetadata { Length = 1 }); + var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem.Object); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_EmptyItemPopulatedProviderRemoteExtras_LimitsImages(ImageType imageType, int imageCount) + { + var item = new MovieWithScreenshots(); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var remoteProvider = new Mock(MockBehavior.Strict); + remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); + remoteProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + + var refreshOptions = new ImageRefreshOptions(null); + + // populate remote with double the required images to verify count is trimmed to the library option count + var remoteInfo = new List(); + for (int i = 0; i < imageCount * 2; i++) + { + remoteInfo.Add(new RemoteImageInfo + { + Type = imageType, + Url = "image url " + i, + Width = 1 // min width is set to 0, this will always pass + }); + } + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(remoteInfo); + var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of()); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + var actualImages = item.GetImages(imageType).ToList(); + Assert.Equal(imageCount, actualImages.Count); + // images from the provider manager are sorted by preference (earlier images are higher priority) so we can verify that low url numbers are chosen + foreach (var image in actualImages) + { + var index = int.Parse(Regex.Match(image.Path, @"\d+").Value, NumberStyles.Integer, CultureInfo.InvariantCulture); + Assert.True(index < imageCount); + } + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_PopulatedItemPopulatedProviderRemoteFullRefresh_UpdatesImages(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, false); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var remoteProvider = new Mock(MockBehavior.Strict); + remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); + remoteProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + + var refreshOptions = new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllImages = true + }; + + var remoteInfo = new List(); + for (int i = 0; i < imageCount; i++) + { + remoteInfo.Add(new RemoteImageInfo + { + Type = imageType, + Url = "image url " + i, + Width = 1 // min width is set to 0, this will always pass + }); + } + + var providerManager = new Mock(MockBehavior.Strict); + providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(remoteInfo); + var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of()); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + foreach (var image in item.GetImages(imageType)) + { + Assert.Matches(@"image url \d", image.Path); + } + } + + [Theory] + [MemberData(nameof(GetImageTypesWithCount))] + public async void RefreshImages_PopulatedItemEmptyProviderRemoteFullRefresh_DoesntClearImages(ImageType imageType, int imageCount) + { + var item = GetItemWithImages(imageType, imageCount, false); + + var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + + var remoteProvider = new Mock(MockBehavior.Strict); + remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); + remoteProvider.Setup(rp => rp.GetSupportedImages(item)) + .Returns(new[] { imageType }); + + var refreshOptions = new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllImages = true + }; + + var itemImageProvider = GetItemImageProvider(Mock.Of(), Mock.Of()); + var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); + + Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } + + private static ItemImageProvider GetItemImageProvider(IProviderManager? providerManager, IFileSystem? fileSystem) + { + // strict to ensure this isn't accidentally used where a prepared mock is intended + providerManager ??= Mock.Of(MockBehavior.Strict); + fileSystem ??= Mock.Of(MockBehavior.Strict); + return new ItemImageProvider(new NullLogger(), providerManager, fileSystem); + } + + private static BaseItem GetItemWithImages(ImageType type, int count, bool validPaths) + { + // Has to exist for querying DateModified time on file, results stored but not checked so not populating + BaseItem.FileSystem ??= Mock.Of(); + + var item = new MovieWithScreenshots(); + + var path = validPaths ? TestDataImagePath : "invalid path {0}"; + for (int i = 0; i < count; i++) + { + item.SetImagePath(type, i, new FileSystemMetadata + { + FullName = string.Format(CultureInfo.InvariantCulture, path, i), + }); + } + + return item; + } + + private static ILocalImageProvider GetImageProvider(ImageType type, int count, bool validPaths) + { + var images = GetImages(type, count, validPaths); + + var imageProvider = new Mock(); + imageProvider.Setup(ip => ip.GetImages(It.IsAny(), It.IsAny())) + .Returns(images); + return imageProvider.Object; + } + + /// + /// Creates a list of references of the specified type and size, optionally pointing to files that exist. + /// + private static List GetImages(ImageType type, int count, bool validPaths) + { + var path = validPaths ? TestDataImagePath : "invalid path {0}"; + var images = new List(count); + for (int i = 0; i < count; i++) + { + images.Add(new LocalImageInfo + { + Type = type, + FileInfo = new FileSystemMetadata + { + FullName = string.Format(CultureInfo.InvariantCulture, path, i) + } + }); + } + + return images; + } + + /// + /// Generates a object that will allow for the requested number of images for the target type. + /// + private static LibraryOptions GetLibraryOptions(BaseItem item, ImageType type, int count) + { + return new LibraryOptions + { + TypeOptions = new[] + { + new TypeOptions + { + Type = item.GetType().Name, + ImageOptions = new[] + { + new ImageOption + { + Type = type, + Limit = count, + MinWidth = 0 + } + } + } + } + }; + } + + // Create a class that implements IHasScreenshots for testing since no BaseItem class is also IHasScreenshots + private class MovieWithScreenshots : Movie, IHasScreenshots + { + // No contents + } + } +} diff --git a/tests/Jellyfin.Providers.Tests/Test Data/Images/blank0.jpg b/tests/Jellyfin.Providers.Tests/Test Data/Images/blank0.jpg new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/Jellyfin.Providers.Tests/Test Data/Images/blank1.jpg b/tests/Jellyfin.Providers.Tests/Test Data/Images/blank1.jpg new file mode 100644 index 0000000000..e69de29bb2