aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorClaus Vium <cvium@users.noreply.github.com>2021-11-03 13:01:23 +0100
committerGitHub <noreply@github.com>2021-11-03 13:01:23 +0100
commit869d537aaa4666db1e1ab6f28123d0a825af3e03 (patch)
tree5006140c590c1ff9d48d8ce6dacdf243a50ae026
parent2c42d75288a57c2961f257a7619e286702f4ac79 (diff)
parent149c77d9b180d2a64f4d9acf392e26611129f82d (diff)
Merge pull request #6752 from 1337joe/fix-metadata-refresh-deletes-backgrounds
-rw-r--r--MediaBrowser.Controller/Entities/BaseItem.cs21
-rw-r--r--MediaBrowser.Providers/Manager/ItemImageProvider.cs133
-rw-r--r--tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj6
-rw-r--r--tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs606
-rw-r--r--tests/Jellyfin.Providers.Tests/Test Data/Images/blank0.jpg0
-rw-r--r--tests/Jellyfin.Providers.Tests/Test Data/Images/blank1.jpg0
6 files changed, 690 insertions, 76 deletions
diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs
index 838a9f2f8..02ee97b23 100644
--- a/MediaBrowser.Controller/Entities/BaseItem.cs
+++ b/MediaBrowser.Controller/Entities/BaseItem.cs
@@ -2345,7 +2345,7 @@ namespace MediaBrowser.Controller.Entities
RemoveImages(new List<ItemImageInfo> { image });
}
- public void RemoveImages(List<ItemImageInfo> deletedImages)
+ public void RemoveImages(IEnumerable<ItemImageInfo> deletedImages)
{
ImageInfos = ImageInfos.Except(deletedImages).ToArray();
}
@@ -2495,11 +2495,11 @@ namespace MediaBrowser.Controller.Entities
}
/// <summary>
- /// Adds the images.
+ /// Adds the images, updating metadata if they already are part of this item.
/// </summary>
/// <param name="imageType">Type of the image.</param>
/// <param name="images">The images.</param>
- /// <returns><c>true</c> if XXXX, <c>false</c> otherwise.</returns>
+ /// <returns><c>true</c> if images were added or updated, <c>false</c> otherwise.</returns>
/// <exception cref="ArgumentException">Cannot call AddImages with chapter images.</exception>
public bool AddImages(ImageType imageType, List<FileSystemMetadata> images)
{
@@ -2512,7 +2512,6 @@ namespace MediaBrowser.Controller.Entities
.ToList();
var newImageList = new List<FileSystemMetadata>();
- 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 39372acb9..8d5795f8e 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
{
+ /// <summary>
+ /// Utilities for managing images attached to items.
+ /// </summary>
public class ItemImageProvider
{
private readonly ILogger _logger;
@@ -47,6 +48,12 @@ namespace MediaBrowser.Providers.Manager
ImageType.Thumb
};
+ /// <summary>
+ /// Initializes a new instance of the <see cref="ItemImageProvider"/> class.
+ /// </summary>
+ /// <param name="logger">The logger.</param>
+ /// <param name="providerManager">The provider manager for interacting with provider image references.</param>
+ /// <param name="fileSystem">The filesystem.</param>
public ItemImageProvider(ILogger logger, IProviderManager providerManager, IFileSystem fileSystem)
{
_logger = logger;
@@ -54,6 +61,13 @@ namespace MediaBrowser.Providers.Manager
_fileSystem = fileSystem;
}
+ /// <summary>
+ /// Verifies existing images have valid paths and adds any new local images provided.
+ /// </summary>
+ /// <param name="item">The <see cref="BaseItem"/> to validate images for.</param>
+ /// <param name="providers">The providers to use, must include <see cref="ILocalImageProvider"/>(s) for local scanning.</param>
+ /// <param name="directoryService">The directory service for <see cref="ILocalImageProvider"/>s to use.</param>
+ /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
public bool ValidateImages(BaseItem item, IEnumerable<IImageProvider> providers, IDirectoryService directoryService)
{
var hasChanges = false;
@@ -73,6 +87,15 @@ namespace MediaBrowser.Providers.Manager
return hasChanges;
}
+ /// <summary>
+ /// Refreshes from the providers according to the given options.
+ /// </summary>
+ /// <param name="item">The <see cref="BaseItem"/> to gather images for.</param>
+ /// <param name="libraryOptions">The library options.</param>
+ /// <param name="providers">The providers to query for images.</param>
+ /// <param name="refreshOptions">The refresh options.</param>
+ /// <param name="cancellationToken">The cancellation token.</param>
+ /// <returns>The refresh result.</returns>
public async Task<RefreshResult> RefreshImages(
BaseItem item,
LibraryOptions libraryOptions,
@@ -80,14 +103,16 @@ namespace MediaBrowser.Providers.Manager
ImageRefreshOptions refreshOptions,
CancellationToken cancellationToken)
{
+ var oldBackdropImages = Array.Empty<ItemImageInfo>();
if (refreshOptions.IsReplacingImage(ImageType.Backdrop))
{
- ClearImages(item, ImageType.Backdrop);
+ oldBackdropImages = item.GetImages(ImageType.Backdrop).ToArray();
}
+ var oldScreenshotImages = Array.Empty<ItemImageInfo>();
if (refreshOptions.IsReplacingImage(ImageType.Screenshot))
{
- ClearImages(item, ImageType.Screenshot);
+ oldScreenshotImages = item.GetImages(ImageType.Screenshot).ToArray();
}
var result = new RefreshResult { UpdateType = ItemUpdateType.None };
@@ -95,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.Length;
+ var screenshotLimit = typeOptions.GetLimit(ImageType.Screenshot) + oldScreenshotImages.Length;
var downloadedImages = new List<ImageType>();
foreach (var provider in providers)
@@ -114,11 +139,22 @@ namespace MediaBrowser.Providers.Manager
}
}
+ // only delete existing multi-images if new ones were added
+ if (oldBackdropImages.Length > 0 && oldBackdropImages.Length < item.GetImages(ImageType.Backdrop).Count())
+ {
+ PruneImages(item, oldBackdropImages);
+ }
+
+ if (oldScreenshotImages.Length > 0 && oldScreenshotImages.Length < item.GetImages(ImageType.Screenshot).Count())
+ {
+ PruneImages(item, oldScreenshotImages);
+ }
+
return result;
}
/// <summary>
- /// Refreshes from provider.
+ /// Refreshes from a dynamic provider.
/// </summary>
private async Task RefreshFromProvider(
BaseItem item,
@@ -153,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
{
@@ -234,7 +271,7 @@ namespace MediaBrowser.Providers.Manager
}
/// <summary>
- /// Refreshes from provider.
+ /// Refreshes from a remote provider.
/// </summary>
/// <param name="item">The item.</param>
/// <param name="provider">The provider.</param>
@@ -305,12 +342,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, refreshOptions, 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, refreshOptions, screenshotLimit, provider, result, list, minWidth, cancellationToken).ConfigureAwait(false);
}
}
catch (OperationCanceledException)
@@ -329,40 +366,36 @@ namespace MediaBrowser.Providers.Manager
return options.IsEnabled(type);
}
- private void ClearImages(BaseItem item, ImageType type)
+ private void PruneImages(BaseItem item, ItemImageInfo[] images)
{
- var deleted = false;
- var deletedImages = new List<ItemImageInfo>();
-
- foreach (var image in item.GetImages(type))
+ for (var i = 0; i < images.Length; 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);
}
+ /// <summary>
+ /// Merges a list of images into the provided item, validating existing images and replacing them or adding new images as necessary.
+ /// </summary>
+ /// <param name="item">The <see cref="BaseItem"/> to modify.</param>
+ /// <param name="images">The new images to place in <c>item</c>.</param>
+ /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
public bool MergeImages(BaseItem item, IReadOnlyList<LocalImageInfo> images)
{
- var changed = false;
+ var changed = item.ValidateImages(new DirectoryService(_fileSystem));
for (var i = 0; i < _singularImages.Length; i++)
{
@@ -398,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))
@@ -417,13 +438,9 @@ namespace MediaBrowser.Providers.Manager
changed = true;
}
- var hasScreenshots = item as IHasScreenshots;
- if (hasScreenshots != null)
+ if (item is IHasScreenshots && UpdateMultiImages(item, images, ImageType.Screenshot))
{
- if (UpdateMultiImages(item, images, ImageType.Screenshot))
- {
- changed = true;
- }
+ changed = true;
}
return changed;
@@ -536,7 +553,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 +586,7 @@ namespace MediaBrowser.Providers.Manager
newIndex);
}
- private async Task DownloadBackdrops(BaseItem item, ImageType imageType, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable<RemoteImageInfo> images, int minWidth, CancellationToken cancellationToken)
+ private async Task DownloadMultiImages(BaseItem item, ImageType imageType, ImageRefreshOptions refreshOptions, int limit, IRemoteImageProvider provider, RefreshResult result, IEnumerable<RemoteImageInfo> images, int minWidth, CancellationToken cancellationToken)
{
foreach (var image in images.Where(i => i.Type == imageType))
{
@@ -609,8 +626,8 @@ namespace MediaBrowser.Providers.Manager
break;
}
- // If there's already an image of the same 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
{
diff --git a/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj b/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj
index bb88ec6a1..10767ae23 100644
--- a/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj
+++ b/tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj
@@ -7,6 +7,12 @@
</PropertyGroup>
<ItemGroup>
+ <None Include="Test Data\**\*.*">
+ <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+ </None>
+ </ItemGroup>
+
+ <ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
<PackageReference Include="Moq" Version="4.16.1" />
<PackageReference Include="xunit" Version="2.4.1" />
diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs
new file mode 100644
index 000000000..6d65ba2d7
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs
@@ -0,0 +1,606 @@
+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<ILocalImageProvider>(), null);
+
+ Assert.False(changed);
+ }
+
+ [Fact]
+ public void ValidateImages_EmptyItemEmptyProviders_NoChange()
+ {
+ var itemImageProvider = GetItemImageProvider(null, null);
+ var changed = itemImageProvider.ValidateImages(new MovieWithScreenshots(), new List<ILocalImageProvider>(), null);
+
+ Assert.False(changed);
+ }
+
+ private static TheoryData<ImageType, int> GetImageTypesWithCount()
+ {
+ var theoryTypes = new TheoryData<ImageType, int>
+ {
+ // minimal test cases that hit different handling
+ { ImageType.Primary, 1 },
+ { ImageType.Backdrop, 1 },
+ { ImageType.Backdrop, 2 },
+ { 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<IFileSystem>();
+
+ var item = new MovieWithScreenshots();
+ var imageProvider = GetImageProvider(imageType, imageCount, true);
+
+ var itemImageProvider = GetItemImageProvider(null, null);
+ var changed = itemImageProvider.ValidateImages(item, new List<ILocalImageProvider> { 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<ILocalImageProvider>(), 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<ILocalImageProvider>(), 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<LocalImageInfo>());
+
+ 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<IFileSystem>();
+ fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
+ .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);
+ 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<IFileSystem>();
+ fileSystem.Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
+ .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);
+ 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]
+ [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, false);
+
+ var libraryOptions = GetLibraryOptions(item, imageType, imageCount);
+
+ var imageResponse = new DynamicImageResponse
+ {
+ HasImage = true,
+ Format = ImageFormat.Jpg,
+ Path = "url path",
+ Protocol = MediaProtocol.Http
+ };
+
+ var dynamicProvider = new Mock<IDynamicImageProvider>(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<CancellationToken>()))
+ .ReturnsAsync(imageResponse);
+
+ var refreshOptions = forceRefresh
+ ? new ImageRefreshOptions(null)
+ {
+ ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true
+ }
+ : new ImageRefreshOptions(null);
+
+ var itemImageProvider = GetItemImageProvider(null, new Mock<IFileSystem>());
+ var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { dynamicProvider.Object }, refreshOptions, CancellationToken.None);
+
+ 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]
+ [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<IFileSystem>();
+
+ var item = new MovieWithScreenshots();
+
+ 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,
+ Path = responseHasPath ? string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 0) : null,
+ Protocol = protocol
+ };
+
+ var dynamicProvider = new Mock<IDynamicImageProvider>(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<CancellationToken>()))
+ .ReturnsAsync(imageResponse);
+
+ var refreshOptions = new ImageRefreshOptions(null);
+
+ var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
+ providerManager.Setup(pm => pm.SaveImage(item, It.IsAny<Stream>(), It.IsAny<string>(), imageType, null, It.IsAny<CancellationToken>()))
+ .Callback<BaseItem, Stream, string, ImageType, int?, CancellationToken>((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<IImageProvider> { dynamicProvider.Object }, refreshOptions, CancellationToken.None);
+
+ Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
+ // dynamic provider unable to return multiple images
+ Assert.Single(item.GetImages(imageType));
+ if (protocol == MediaProtocol.Http)
+ {
+ Assert.Equal(imageResponse.Path, item.GetImagePath(imageType, 0));
+ }
+ }
+
+ [Theory]
+ [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);
+
+ var libraryOptions = GetLibraryOptions(item, imageType, imageCount);
+
+ var remoteProvider = new Mock<IRemoteImageProvider>(MockBehavior.Strict);
+ remoteProvider.Setup(rp => rp.Name).Returns("MockRemoteProvider");
+ remoteProvider.Setup(rp => rp.GetSupportedImages(item))
+ .Returns(new[] { imageType });
+
+ var refreshOptions = forceRefresh
+ ? new ImageRefreshOptions(null)
+ {
+ ImageRefreshMode = MetadataRefreshMode.FullRefresh, ReplaceAllImages = true
+ }
+ : new ImageRefreshOptions(null);
+
+ var remoteInfo = new List<RemoteImageInfo>();
+ 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<IProviderManager>(MockBehavior.Strict);
+ providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(remoteInfo);
+ var itemImageProvider = GetItemImageProvider(providerManager.Object, new Mock<IFileSystem>());
+ var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
+
+ 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]
+ [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)
+ {
+ var targetImageCount = 1;
+
+ // Set path and media source manager so images will be downloaded (EnableImageStub will return false)
+ var item = GetItemWithImages(imageType, initialImageCount, false);
+ item.Path = "non-empty path";
+ BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
+
+ // 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<IRemoteImageProvider>(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<string>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync((string url, CancellationToken _) => new HttpResponseMessage
+ {
+ ReasonPhrase = url,
+ StatusCode = HttpStatusCode.OK,
+ Content = new StringContent(content, Encoding.UTF8, "image/jpeg")
+ });
+
+ var refreshOptions = fullRefresh
+ ? new ImageRefreshOptions(null)
+ {
+ ImageRefreshMode = MetadataRefreshMode.FullRefresh,
+ ReplaceAllImages = true
+ }
+ : new ImageRefreshOptions(null);
+
+ var remoteInfo = new List<RemoteImageInfo>();
+ for (int i = 0; i < targetImageCount; 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<IProviderManager>(MockBehavior.Strict);
+ providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(remoteInfo);
+ providerManager.Setup(pm => pm.SaveImage(item, It.IsAny<Stream>(), It.IsAny<string>(), imageType, null, It.IsAny<CancellationToken>()))
+ .Callback<BaseItem, Stream, string, ImageType, int?, CancellationToken>((callbackItem, _, _, callbackType, _, _) =>
+ callbackItem.SetImagePath(callbackType, callbackItem.AllowsMultipleImages(callbackType) ? callbackItem.GetImages(callbackType).Count() : 0, new FileSystemMetadata()))
+ .Returns(Task.CompletedTask);
+ var fileSystem = new Mock<IFileSystem>();
+ // match reported file size to image content length - condition for skipping already downloaded multi-images
+ fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny<string>()))
+ .Returns(new FileSystemMetadata { Length = content.Length });
+ var itemImageProvider = GetItemImageProvider(providerManager.Object, fileSystem);
+ var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
+
+ Assert.Equal(initialImageCount == 0 || fullRefresh, result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
+ Assert.Equal(targetImageCount, 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<IRemoteImageProvider>(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<RemoteImageInfo>();
+ 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<IProviderManager>(MockBehavior.Strict);
+ providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
+ .ReturnsAsync(remoteInfo);
+ var itemImageProvider = GetItemImageProvider(providerManager.Object, null);
+ var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { 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, @"[0-9]+").Value, NumberStyles.Integer, CultureInfo.InvariantCulture);
+ Assert.True(index < imageCount);
+ }
+ }
+
+ [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<IRemoteImageProvider>(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<IProviderManager>(), null);
+ var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { 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, Mock<IFileSystem>? mockFileSystem)
+ {
+ // strict to ensure this isn't accidentally used where a prepared mock is intended
+ providerManager ??= Mock.Of<IProviderManager>(MockBehavior.Strict);
+
+ // BaseItem.ValidateImages depends on the directory service being able to list directory contents, give it the expected valid file paths
+ mockFileSystem ??= new Mock<IFileSystem>(MockBehavior.Strict);
+ mockFileSystem.Setup(fs => fs.GetFilePaths(It.IsAny<string>(), It.IsAny<bool>()))
+ .Returns(new[]
+ {
+ string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 0),
+ string.Format(CultureInfo.InvariantCulture, TestDataImagePath, 1)
+ });
+
+ return new ItemImageProvider(new NullLogger<ItemImageProvider>(), providerManager, mockFileSystem.Object);
+ }
+
+ 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<IFileSystem>();
+
+ 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<ILocalImageProvider>();
+ imageProvider.Setup(ip => ip.GetImages(It.IsAny<BaseItem>(), It.IsAny<IDirectoryService>()))
+ .Returns(images);
+ return imageProvider.Object;
+ }
+
+ /// <summary>
+ /// Creates a list of <see cref="LocalImageInfo"/> references of the specified type and size, optionally pointing to files that exist.
+ /// </summary>
+ private static List<LocalImageInfo> GetImages(ImageType type, int count, bool validPaths)
+ {
+ var path = validPaths ? TestDataImagePath : "invalid path {0}";
+ var images = new List<LocalImageInfo>(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;
+ }
+
+ /// <summary>
+ /// Generates a <see cref="LibraryOptions"/> object that will allow for the requested number of images for the target type.
+ /// </summary>
+ 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 000000000..e69de29bb
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Test Data/Images/blank0.jpg
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 000000000..e69de29bb
--- /dev/null
+++ b/tests/Jellyfin.Providers.Tests/Test Data/Images/blank1.jpg