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 1/6] 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 From 0fbd8d85c825b2871ea38e5c7c1d61baca0772c9 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Mon, 1 Nov 2021 00:16:11 +0100 Subject: [PATCH 2/6] Validate multi-images, lazy-delete bg on refresh Fix failing test: Invalid background images not purged by validate Fixes #6310: Background images only delete when using "Replace existing images" when new image(s) is found to replace them --- .../Manager/ItemImageProvider.cs | 68 +++++++++++-------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index 49b7a5d6b8..c80407bcb4 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -103,14 +103,16 @@ namespace MediaBrowser.Providers.Manager ImageRefreshOptions refreshOptions, CancellationToken cancellationToken) { + List oldBackdropImages = new List(); if (refreshOptions.IsReplacingImage(ImageType.Backdrop)) { - ClearImages(item, ImageType.Backdrop); + oldBackdropImages = item.GetImages(ImageType.Backdrop).ToList(); } + List oldScreenshotImages = new List(); if (refreshOptions.IsReplacingImage(ImageType.Screenshot)) { - ClearImages(item, ImageType.Screenshot); + oldScreenshotImages = item.GetImages(ImageType.Screenshot).ToList(); } var result = new RefreshResult { UpdateType = ItemUpdateType.None }; @@ -118,9 +120,9 @@ namespace MediaBrowser.Providers.Manager var typeName = item.GetType().Name; var typeOptions = libraryOptions.GetTypeOptions(typeName) ?? new TypeOptions { Type = typeName }; - // In order to avoid duplicates, only download these if there are none already - var backdropLimit = typeOptions.GetLimit(ImageType.Backdrop); - var screenshotLimit = typeOptions.GetLimit(ImageType.Screenshot); + // track library limits, adding buffer to allow lazy replacing of current images + var backdropLimit = typeOptions.GetLimit(ImageType.Backdrop) + oldBackdropImages.Count; + var screenshotLimit = typeOptions.GetLimit(ImageType.Screenshot) + oldScreenshotImages.Count; var downloadedImages = new List(); foreach (var provider in providers) @@ -137,6 +139,17 @@ namespace MediaBrowser.Providers.Manager } } + // only delete existing multi-images if new ones were added + if (oldBackdropImages.Count > 0 && oldBackdropImages.Count < item.GetImages(ImageType.Backdrop).Count()) + { + PruneImages(item, oldBackdropImages); + } + + if (oldScreenshotImages.Count > 0 && oldScreenshotImages.Count < item.GetImages(ImageType.Screenshot).Count()) + { + PruneImages(item, oldScreenshotImages); + } + return result; } @@ -176,13 +189,14 @@ namespace MediaBrowser.Providers.Manager if (response.Protocol == MediaProtocol.Http) { _logger.LogDebug("Setting image url into item {0}", item.Id); + var index = item.AllowsMultipleImages(imageType) ? item.GetImages(imageType).Count() : 0; item.SetImage( new ItemImageInfo { Path = response.Path, Type = imageType }, - 0); + index); } else { @@ -352,35 +366,25 @@ namespace MediaBrowser.Providers.Manager return options.IsEnabled(type); } - private void ClearImages(BaseItem item, ImageType type) + private void PruneImages(BaseItem item, List images) { - var deleted = false; - var deletedImages = new List(); - - foreach (var image in item.GetImages(type)) + for (var i = 0; i < images.Count; i++) { - if (!image.IsLocalFile) - { - deletedImages.Add(image); - continue; - } + var image = images[i]; - try - { - _fileSystem.DeleteFile(image.Path); - deleted = true; - } - catch (FileNotFoundException) + if (image.IsLocalFile) { + try + { + _fileSystem.DeleteFile(image.Path); + } + catch (FileNotFoundException) + { + } } } - item.RemoveImages(deletedImages); - - if (deleted) - { - item.ValidateImages(new DirectoryService(_fileSystem)); - } + item.RemoveImages(images); } /// @@ -476,6 +480,14 @@ namespace MediaBrowser.Providers.Manager { var changed = false; + var deletedImages = item.GetImages(type).Where(i => i.IsLocalFile && !File.Exists(i.Path)).ToList(); + + if (deletedImages.Count > 0) + { + item.RemoveImages(deletedImages); + changed = true; + } + var newImageFileInfos = images .Where(i => i.Type == type) .Select(i => i.FileInfo) From b478b115e3194aa383f86d7d6fbf07e0f2bfadea Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Mon, 1 Nov 2021 02:38:12 +0100 Subject: [PATCH 3/6] Refactor to validate all images up front --- MediaBrowser.Controller/Entities/BaseItem.cs | 19 ++-------- .../Manager/ItemImageProvider.cs | 29 ++------------- .../Manager/ItemImageProviderTests.cs | 35 ++++++++++++------- 3 files changed, 27 insertions(+), 56 deletions(-) diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs index 838a9f2f8d..7dd8e310ed 100644 --- a/MediaBrowser.Controller/Entities/BaseItem.cs +++ b/MediaBrowser.Controller/Entities/BaseItem.cs @@ -2495,11 +2495,11 @@ namespace MediaBrowser.Controller.Entities } /// - /// Adds the images. + /// Adds the images, updating metadata if they already are part of this item. /// /// Type of the image. /// The images. - /// true if XXXX, false otherwise. + /// true if images were added or updated, false otherwise. /// Cannot call AddImages with chapter images. public bool AddImages(ImageType imageType, List images) { @@ -2512,7 +2512,6 @@ namespace MediaBrowser.Controller.Entities .ToList(); var newImageList = new List(); - var imageAdded = false; var imageUpdated = false; foreach (var newImage in images) @@ -2528,7 +2527,6 @@ namespace MediaBrowser.Controller.Entities if (existing == null) { newImageList.Add(newImage); - imageAdded = true; } else { @@ -2549,19 +2547,6 @@ namespace MediaBrowser.Controller.Entities } } - if (imageAdded || images.Count != existingImages.Count) - { - var newImagePaths = images.Select(i => i.FullName).ToList(); - - var deleted = existingImages - .FindAll(i => i.IsLocalFile && !newImagePaths.Contains(i.Path.AsSpan(), StringComparison.OrdinalIgnoreCase) && !File.Exists(i.Path)); - - if (deleted.Count > 0) - { - ImageInfos = ImageInfos.Except(deleted).ToArray(); - } - } - if (newImageList.Count > 0) { ImageInfos = ImageInfos.Concat(newImageList.Select(i => GetImageInfo(i, imageType))).ToArray(); diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index c80407bcb4..f60fce11b8 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -395,7 +395,7 @@ namespace MediaBrowser.Providers.Manager /// true if changes were made to the item; otherwise false. public bool MergeImages(BaseItem item, IReadOnlyList images) { - var changed = false; + var changed = item.ValidateImages(new DirectoryService(_fileSystem)); for (var i = 0; i < _singularImages.Length; i++) { @@ -431,18 +431,6 @@ namespace MediaBrowser.Providers.Manager currentImage.DateModified = newDateModified; } } - else - { - var existing = item.GetImageInfo(type, 0); - if (existing != null) - { - if (existing.IsLocalFile && !File.Exists(existing.Path)) - { - item.RemoveImage(existing); - changed = true; - } - } - } } if (UpdateMultiImages(item, images, ImageType.Backdrop)) @@ -450,12 +438,9 @@ namespace MediaBrowser.Providers.Manager changed = true; } - if (item is IHasScreenshots) + if (item is IHasScreenshots && UpdateMultiImages(item, images, ImageType.Screenshot)) { - if (UpdateMultiImages(item, images, ImageType.Screenshot)) - { - changed = true; - } + changed = true; } return changed; @@ -480,14 +465,6 @@ namespace MediaBrowser.Providers.Manager { var changed = false; - var deletedImages = item.GetImages(type).Where(i => i.IsLocalFile && !File.Exists(i.Path)).ToList(); - - if (deletedImages.Count > 0) - { - item.RemoveImages(deletedImages); - changed = true; - } - var newImageFileInfos = images .Where(i => i.Type == type) .Select(i => i.FileInfo) diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs index 253bcb7cad..b5efd8f013 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs @@ -183,7 +183,7 @@ namespace Jellyfin.Providers.Tests.Manager var images = GetImages(imageType, imageCount, true); - var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); + var itemImageProvider = GetItemImageProvider(null, fileSystem); var changed = itemImageProvider.MergeImages(item, images); Assert.False(changed); @@ -213,7 +213,7 @@ namespace Jellyfin.Providers.Tests.Manager var images = GetImages(imageType, imageCount, true); - var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); + var itemImageProvider = GetItemImageProvider(null, fileSystem); var changed = itemImageProvider.MergeImages(item, images); Assert.True(changed); @@ -363,7 +363,7 @@ namespace Jellyfin.Providers.Tests.Manager ReplaceAllImages = true }; - var itemImageProvider = GetItemImageProvider(null, Mock.Of()); + var itemImageProvider = GetItemImageProvider(null, new Mock()); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); @@ -401,7 +401,7 @@ namespace Jellyfin.Providers.Tests.Manager 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 itemImageProvider = GetItemImageProvider(providerManager.Object, null); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); @@ -459,7 +459,7 @@ namespace Jellyfin.Providers.Tests.Manager var fileSystem = new Mock(); fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny())) .Returns(new FileSystemMetadata { Length = 1 }); - var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem.Object); + var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); @@ -496,7 +496,7 @@ namespace Jellyfin.Providers.Tests.Manager 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 itemImageProvider = GetItemImageProvider(providerManager.Object, null); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); @@ -505,7 +505,7 @@ namespace Jellyfin.Providers.Tests.Manager // 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); + var index = int.Parse(Regex.Match(image.Path, @"[0-9]+").Value, NumberStyles.Integer, CultureInfo.InvariantCulture); Assert.True(index < imageCount); } } @@ -543,14 +543,14 @@ namespace Jellyfin.Providers.Tests.Manager 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 itemImageProvider = GetItemImageProvider(providerManager.Object, new Mock()); 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); + Assert.Matches(@"image url [0-9]", image.Path); } } @@ -573,19 +573,28 @@ namespace Jellyfin.Providers.Tests.Manager ReplaceAllImages = true }; - var itemImageProvider = GetItemImageProvider(Mock.Of(), Mock.Of()); + var itemImageProvider = GetItemImageProvider(Mock.Of(), null); 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) + private static ItemImageProvider GetItemImageProvider(IProviderManager? providerManager, Mock? mockFileSystem) { // 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); + + // BaseItem.ValidateImages depends on the directory service being able to list directory contents, give it the expected valid file paths + mockFileSystem ??= new Mock(MockBehavior.Strict); + mockFileSystem.Setup(fs => fs.GetFilePaths(It.IsAny(), It.IsAny())) + .Returns(new[] + { + string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 0), + string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 1) + }); + + return new ItemImageProvider(new NullLogger(), providerManager, mockFileSystem.Object); } private static BaseItem GetItemWithImages(ImageType type, int count, bool validPaths) From 7da6bd905ad23c486da31aa122021e7b8a07d7d7 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Tue, 2 Nov 2021 00:31:59 +0100 Subject: [PATCH 4/6] Fix edge case in multi-image replacing --- MediaBrowser.Providers/Manager/ItemImageProvider.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index f60fce11b8..2c7d43c86d 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -342,12 +342,12 @@ namespace MediaBrowser.Providers.Manager } minWidth = savedOptions.GetMinWidth(ImageType.Backdrop); - await DownloadMultiImages(item, ImageType.Backdrop, backdropLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); + await DownloadMultiImages(item, ImageType.Backdrop, refreshOptions, backdropLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); if (item is IHasScreenshots) { minWidth = savedOptions.GetMinWidth(ImageType.Screenshot); - await DownloadMultiImages(item, ImageType.Screenshot, screenshotLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); + await DownloadMultiImages(item, ImageType.Screenshot, refreshOptions, screenshotLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false); } } catch (OperationCanceledException) @@ -586,7 +586,7 @@ namespace MediaBrowser.Providers.Manager newIndex); } - private async Task DownloadMultiImages(BaseItem item, ImageType imageType, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable images, int minWidth, CancellationToken cancellationToken) + private async Task DownloadMultiImages(BaseItem item, ImageType imageType, ImageRefreshOptions refreshOptions, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable images, int minWidth, CancellationToken cancellationToken) { foreach (var image in images.Where(i => i.Type == imageType)) { @@ -626,8 +626,8 @@ namespace MediaBrowser.Providers.Manager break; } - // If there's already an image of the same file size, skip it - if (response.Content.Headers.ContentLength.HasValue) + // If there's already an image of the same file size, skip it unless doing a full refresh + if (response.Content.Headers.ContentLength.HasValue && !refreshOptions.IsReplacingImage(imageType)) { try { From 7fcf01235c2360ec64cad685df7f155ef3dee69a Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Tue, 2 Nov 2021 16:16:06 +0100 Subject: [PATCH 5/6] Change RemoveImages to array, improve download test --- MediaBrowser.Controller/Entities/BaseItem.cs | 2 +- .../Manager/ItemImageProvider.cs | 20 +++++----- .../Manager/ItemImageProviderTests.cs | 39 ++++++++++++------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs index 7dd8e310ed..02ee97b23f 100644 --- a/MediaBrowser.Controller/Entities/BaseItem.cs +++ b/MediaBrowser.Controller/Entities/BaseItem.cs @@ -2345,7 +2345,7 @@ namespace MediaBrowser.Controller.Entities RemoveImages(new List { image }); } - public void RemoveImages(List deletedImages) + public void RemoveImages(IEnumerable deletedImages) { ImageInfos = ImageInfos.Except(deletedImages).ToArray(); } diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index 2c7d43c86d..8d5795f8e1 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -103,16 +103,16 @@ namespace MediaBrowser.Providers.Manager ImageRefreshOptions refreshOptions, CancellationToken cancellationToken) { - List oldBackdropImages = new List(); + var oldBackdropImages = Array.Empty(); if (refreshOptions.IsReplacingImage(ImageType.Backdrop)) { - oldBackdropImages = item.GetImages(ImageType.Backdrop).ToList(); + oldBackdropImages = item.GetImages(ImageType.Backdrop).ToArray(); } - List oldScreenshotImages = new List(); + var oldScreenshotImages = Array.Empty(); if (refreshOptions.IsReplacingImage(ImageType.Screenshot)) { - oldScreenshotImages = item.GetImages(ImageType.Screenshot).ToList(); + oldScreenshotImages = item.GetImages(ImageType.Screenshot).ToArray(); } var result = new RefreshResult { UpdateType = ItemUpdateType.None }; @@ -121,8 +121,8 @@ namespace MediaBrowser.Providers.Manager var typeOptions = libraryOptions.GetTypeOptions(typeName) ?? new TypeOptions { Type = typeName }; // track library limits, adding buffer to allow lazy replacing of current images - var backdropLimit = typeOptions.GetLimit(ImageType.Backdrop) + oldBackdropImages.Count; - var screenshotLimit = typeOptions.GetLimit(ImageType.Screenshot) + oldScreenshotImages.Count; + var backdropLimit = typeOptions.GetLimit(ImageType.Backdrop) + oldBackdropImages.Length; + var screenshotLimit = typeOptions.GetLimit(ImageType.Screenshot) + oldScreenshotImages.Length; var downloadedImages = new List(); foreach (var provider in providers) @@ -140,12 +140,12 @@ namespace MediaBrowser.Providers.Manager } // only delete existing multi-images if new ones were added - if (oldBackdropImages.Count > 0 && oldBackdropImages.Count < item.GetImages(ImageType.Backdrop).Count()) + if (oldBackdropImages.Length > 0 && oldBackdropImages.Length < item.GetImages(ImageType.Backdrop).Count()) { PruneImages(item, oldBackdropImages); } - if (oldScreenshotImages.Count > 0 && oldScreenshotImages.Count < item.GetImages(ImageType.Screenshot).Count()) + if (oldScreenshotImages.Length > 0 && oldScreenshotImages.Length < item.GetImages(ImageType.Screenshot).Count()) { PruneImages(item, oldScreenshotImages); } @@ -366,9 +366,9 @@ namespace MediaBrowser.Providers.Manager return options.IsEnabled(type); } - private void PruneImages(BaseItem item, List images) + private void PruneImages(BaseItem item, ItemImageInfo[] images) { - for (var i = 0; i < images.Count; i++) + for (var i = 0; i < images.Length; i++) { var image = images[i]; diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs index b5efd8f013..54f2cb71bf 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs @@ -409,21 +409,23 @@ namespace Jellyfin.Providers.Tests.Manager } [Theory] - [MemberData(nameof(GetImageTypesWithCount))] - public async void RefreshImages_EmptyNonStubItemPopulatedProviderRemote_DownloadsImages(ImageType imageType, int imageCount) + [InlineData(ImageType.Primary, 0, false)] // singular type only fetches if type is missing from item, no caching + [InlineData(ImageType.Backdrop, 0, false)] // empty item, no cache to check + [InlineData(ImageType.Backdrop, 1, false)] // populated item, cached so no download + [InlineData(ImageType.Backdrop, 1, true)] // populated item, forced to download + public async void RefreshImages_NonStubItemPopulatedProviderRemote_DownloadsIfNecessary(ImageType imageType, int initialImageCount, bool fullRefresh) { - // Has to exist for querying DateModified time on file, results stored but not checked so not populating - BaseItem.FileSystem ??= Mock.Of(); + var targetImageCount = 1; // Set path and media source manager so images will be downloaded (EnableImageStub will return false) - var item = new MovieWithScreenshots - { - Path = "non-empty path" - }; + var item = GetItemWithImages(imageType, initialImageCount, false); + item.Path = "non-empty path"; BaseItem.MediaSourceManager = Mock.Of(); - var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + // seek 2 so it won't short-circuit out of downloading when populated + var libraryOptions = GetLibraryOptions(item, imageType, 2); + var content = "Content"; var remoteProvider = new Mock(MockBehavior.Strict); remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider"); remoteProvider.Setup(rp => rp.GetSupportedImages(item)) @@ -433,13 +435,19 @@ namespace Jellyfin.Providers.Tests.Manager { ReasonPhrase = url, StatusCode = HttpStatusCode.OK, - Content = new StringContent("Content", Encoding.UTF8, "image/jpeg") + Content = new StringContent(content, Encoding.UTF8, "image/jpeg") }); - var refreshOptions = new ImageRefreshOptions(null); + var refreshOptions = fullRefresh + ? new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, + ReplaceAllImages = true + } + : new ImageRefreshOptions(null); var remoteInfo = new List(); - for (int i = 0; i < imageCount; i++) + for (int i = 0; i < targetImageCount; i++) { remoteInfo.Add(new RemoteImageInfo { @@ -457,13 +465,14 @@ namespace Jellyfin.Providers.Tests.Manager callbackItem.SetImagePath(callbackType, callbackItem.AllowsMultipleImages(callbackType) ? callbackItem.GetImages(callbackType).Count() : 0, new FileSystemMetadata())) .Returns(Task.CompletedTask); var fileSystem = new Mock(); + // match reported file size to image content length - condition for skipping already downloaded multi-images fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny())) - .Returns(new FileSystemMetadata { Length = 1 }); + .Returns(new FileSystemMetadata { Length = content.Length }); var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem); 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()); + Assert.Equal(initialImageCount == 0 || fullRefresh, result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(targetImageCount, item.GetImages(imageType).Count()); } [Theory] From 149c77d9b180d2a64f4d9acf392e26611129f82d Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Tue, 2 Nov 2021 22:46:53 +0100 Subject: [PATCH 6/6] Remove commented theory data, merge tests --- .../Manager/ItemImageProviderTests.cs | 240 ++++++------------ 1 file changed, 77 insertions(+), 163 deletions(-) diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs index 54f2cb71bf..6d65ba2d7a 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs @@ -50,35 +50,14 @@ namespace Jellyfin.Providers.Tests.Manager 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); + var theoryTypes = new TheoryData + { + // minimal test cases that hit different handling + { ImageType.Primary, 1 }, + { ImageType.Backdrop, 1 }, + { ImageType.Backdrop, 2 }, + { ImageType.Screenshot, 1 } + }; return theoryTypes; } @@ -228,49 +207,24 @@ namespace Jellyfin.Providers.Tests.Manager } [Theory] - [MemberData(nameof(GetImageTypesWithCount))] - public async void RefreshImages_PopulatedItemPopulatedProviderDynamic_NoChange(ImageType imageType, int imageCount) + [InlineData(ImageType.Primary, 1, false)] + [InlineData(ImageType.Backdrop, 2, false)] + [InlineData(ImageType.Screenshot, 2, false)] + [InlineData(ImageType.Primary, 1, true)] + [InlineData(ImageType.Backdrop, 2, true)] + [InlineData(ImageType.Screenshot, 2, true)] + public async void RefreshImages_PopulatedItemPopulatedProviderDynamic_UpdatesImagesIfForced(ImageType imageType, int imageCount, bool forceRefresh) { - 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 item = GetItemWithImages(imageType, imageCount, false); 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 + Path = "url path", + Protocol = MediaProtocol.Http }; var dynamicProvider = new Mock(MockBehavior.Strict); @@ -280,23 +234,37 @@ namespace Jellyfin.Providers.Tests.Manager dynamicProvider.Setup(rp => rp.GetImage(item, imageType, It.IsAny())) .ReturnsAsync(imageResponse); - var refreshOptions = new ImageRefreshOptions(null); + var refreshOptions = forceRefresh + ? new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true + } + : 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 itemImageProvider = GetItemImageProvider(null, new Mock()); 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(forceRefresh, result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + if (forceRefresh) + { + // replaces multi-types + Assert.Single(item.GetImages(imageType)); + } + else + { + // adds to multi-types if room + Assert.Equal(imageCount, item.GetImages(imageType).Count()); + } } [Theory] - [MemberData(nameof(GetImageTypesWithCount))] - public async void RefreshImages_EmptyItemPopulatedProviderDynamicWithoutPath_AddsImages(ImageType imageType, int imageCount) + [InlineData(ImageType.Primary, 1, true, MediaProtocol.Http)] + [InlineData(ImageType.Backdrop, 2, true, MediaProtocol.Http)] + [InlineData(ImageType.Primary, 1, true, MediaProtocol.File)] + [InlineData(ImageType.Backdrop, 2, true, MediaProtocol.File)] + [InlineData(ImageType.Primary, 1, false, MediaProtocol.File)] + [InlineData(ImageType.Backdrop, 2, false, MediaProtocol.File)] + public async void RefreshImages_EmptyItemPopulatedProviderDynamic_AddsImages(ImageType imageType, int imageCount, bool responseHasPath, MediaProtocol protocol) { // Has to exist for querying DateModified time on file, results stored but not checked so not populating BaseItem.FileSystem = Mock.Of(); @@ -305,11 +273,13 @@ namespace Jellyfin.Providers.Tests.Manager var libraryOptions = GetLibraryOptions(item, imageType, imageCount); + // Path must exist if set: is read in as a stream by AsyncFile.OpenRead var imageResponse = new DynamicImageResponse { HasImage = true, Format = ImageFormat.Jpg, - Protocol = MediaProtocol.File + Path = responseHasPath ? string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 0) : null, + Protocol = protocol }; var dynamicProvider = new Mock(MockBehavior.Strict); @@ -331,50 +301,22 @@ namespace Jellyfin.Providers.Tests.Manager 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) + if (protocol == MediaProtocol.Http) { - ImageRefreshMode = MetadataRefreshMode.FullRefresh, - ReplaceAllImages = true - }; - - var itemImageProvider = GetItemImageProvider(null, new Mock()); - 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)); + Assert.Equal(imageResponse.Path, item.GetImagePath(imageType, 0)); + } } [Theory] - [MemberData(nameof(GetImageTypesWithCount))] - public async void RefreshImages_PopulatedItemPopulatedProviderRemote_NoChange(ImageType imageType, int imageCount) + [InlineData(ImageType.Primary, 1, false)] + [InlineData(ImageType.Backdrop, 1, false)] + [InlineData(ImageType.Backdrop, 2, false)] + [InlineData(ImageType.Screenshot, 2, false)] + [InlineData(ImageType.Primary, 1, true)] + [InlineData(ImageType.Backdrop, 1, true)] + [InlineData(ImageType.Backdrop, 2, true)] + [InlineData(ImageType.Screenshot, 2, true)] + public async void RefreshImages_PopulatedItemPopulatedProviderRemote_UpdatesImagesIfForced(ImageType imageType, int imageCount, bool forceRefresh) { var item = GetItemWithImages(imageType, imageCount, false); @@ -385,7 +327,12 @@ namespace Jellyfin.Providers.Tests.Manager remoteProvider.Setup(rp => rp.GetSupportedImages(item)) .Returns(new[] { imageType }); - var refreshOptions = new ImageRefreshOptions(null); + var refreshOptions = forceRefresh + ? new ImageRefreshOptions(null) + { + ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true + } + : new ImageRefreshOptions(null); var remoteInfo = new List(); for (int i = 0; i < imageCount; i++) @@ -401,11 +348,22 @@ namespace Jellyfin.Providers.Tests.Manager var providerManager = new Mock(MockBehavior.Strict); providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(remoteInfo); - var itemImageProvider = GetItemImageProvider(providerManager.Object, null); + var itemImageProvider = GetItemImageProvider(providerManager.Object, new Mock()); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { remoteProvider.Object }, refreshOptions, CancellationToken.None); - Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); + Assert.Equal(forceRefresh, result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.Equal(imageCount, item.GetImages(imageType).Count()); + foreach (var image in item.GetImages(imageType)) + { + if (forceRefresh) + { + Assert.Matches(@"image url [0-9]", image.Path); + } + else + { + Assert.DoesNotMatch(@"image url [0-9]", image.Path); + } + } } [Theory] @@ -519,50 +477,6 @@ namespace Jellyfin.Providers.Tests.Manager } } - [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, new Mock()); - 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 [0-9]", image.Path); - } - } - [Theory] [MemberData(nameof(GetImageTypesWithCount))] public async void RefreshImages_PopulatedItemEmptyProviderRemoteFullRefresh_DoesntClearImages(ImageType imageType, int imageCount)