Cutlery/jellyfin
1
0
Fork 0
mirror of https://github.com/jellyfin/jellyfin.git synced 2025-07-09 03:04:24 -04:00
Code Issues Packages Projects Releases Wiki Activity

Refactor to validate all images up front

Browse Source
This commit is contained in:
Joe Rogers 2021-11-01 02:38:12 +01:00
parent 0fbd8d85c8
commit b478b115e3
No known key found for this signature in database
GPG Key ID: 0074AD57B8FDBBB4
3 changed files with 27 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
MediaBrowser.Controller/Entities/BaseItem.cs
View File

@ -2495,11 +2495,11 @@ namespace MediaBrowser.Controller.Entities
} }
/// <summary> /// <summary>
/// Adds the images. /// Adds the images, updating metadata if they already are part of this item.
/// </summary> /// </summary>
/// <param name="imageType">Type of the image.</param> /// <param name="imageType">Type of the image.</param>
/// <param name="images">The images.</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> /// <exception cref="ArgumentException">Cannot call AddImages with chapter images.</exception>
public bool AddImages(ImageType imageType, List<FileSystemMetadata> images) public bool AddImages(ImageType imageType, List<FileSystemMetadata> images)
{ {
@ -2512,7 +2512,6 @@ namespace MediaBrowser.Controller.Entities
.ToList(); .ToList();
var newImageList = new List<FileSystemMetadata>(); var newImageList = new List<FileSystemMetadata>();
var imageAdded = false;
var imageUpdated = false; var imageUpdated = false;
foreach (var newImage in images) foreach (var newImage in images)
@ -2528,7 +2527,6 @@ namespace MediaBrowser.Controller.Entities
if (existing == null) if (existing == null)
{ {
newImageList.Add(newImage); newImageList.Add(newImage);
imageAdded = true;
} }
else 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) if (newImageList.Count > 0)
{ {
ImageInfos = ImageInfos.Concat(newImageList.Select(i => GetImageInfo(i, imageType))).ToArray(); ImageInfos = ImageInfos.Concat(newImageList.Select(i => GetImageInfo(i, imageType))).ToArray();

29
MediaBrowser.Providers/Manager/ItemImageProvider.cs
View File

@ -395,7 +395,7 @@ namespace MediaBrowser.Providers.Manager
/// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns> /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
public bool MergeImages(BaseItem item, IReadOnlyList<LocalImageInfo> images) 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++) for (var i = 0; i < _singularImages.Length; i++)
{ {
@ -431,18 +431,6 @@ namespace MediaBrowser.Providers.Manager
currentImage.DateModified = newDateModified; 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)) if (UpdateMultiImages(item, images, ImageType.Backdrop))
@ -450,12 +438,9 @@ namespace MediaBrowser.Providers.Manager
changed = true; 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; return changed;
@ -480,14 +465,6 @@ namespace MediaBrowser.Providers.Manager
{ {
var changed = false; 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 var newImageFileInfos = images
.Where(i => i.Type == type) .Where(i => i.Type == type)
.Select(i => i.FileInfo) .Select(i => i.FileInfo)

35
tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs
View File

@ -183,7 +183,7 @@ namespace Jellyfin.Providers.Tests.Manager
var images = GetImages(imageType, imageCount, true); var images = GetImages(imageType, imageCount, true);
var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); var itemImageProvider = GetItemImageProvider(null, fileSystem);
var changed = itemImageProvider.MergeImages(item, images); var changed = itemImageProvider.MergeImages(item, images);
Assert.False(changed); Assert.False(changed);
@ -213,7 +213,7 @@ namespace Jellyfin.Providers.Tests.Manager
var images = GetImages(imageType, imageCount, true); var images = GetImages(imageType, imageCount, true);
var itemImageProvider = GetItemImageProvider(null, fileSystem.Object); var itemImageProvider = GetItemImageProvider(null, fileSystem);
var changed = itemImageProvider.MergeImages(item, images); var changed = itemImageProvider.MergeImages(item, images);
Assert.True(changed); Assert.True(changed);
@ -363,7 +363,7 @@ namespace Jellyfin.Providers.Tests.Manager
ReplaceAllImages = true ReplaceAllImages = true
}; };
var itemImageProvider = GetItemImageProvider(null, Mock.Of<IFileSystem>()); var itemImageProvider = GetItemImageProvider(null, new Mock<IFileSystem>());
var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { dynamicProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { dynamicProvider.Object }, refreshOptions, CancellationToken.None);
Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
@ -401,7 +401,7 @@ namespace Jellyfin.Providers.Tests.Manager
var providerManager = new Mock<IProviderManager>(MockBehavior.Strict); var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>())) providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(remoteInfo); .ReturnsAsync(remoteInfo);
var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of<IFileSystem>()); var itemImageProvider = GetItemImageProvider(providerManager.Object, null);
var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
@ -459,7 +459,7 @@ namespace Jellyfin.Providers.Tests.Manager
var fileSystem = new Mock<IFileSystem>(); var fileSystem = new Mock<IFileSystem>();
fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny<string>())) fileSystem.Setup(fs => fs.GetFileInfo(It.IsAny<string>()))
.Returns(new FileSystemMetadata { Length = 1 }); .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<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
@ -496,7 +496,7 @@ namespace Jellyfin.Providers.Tests.Manager
var providerManager = new Mock<IProviderManager>(MockBehavior.Strict); var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>())) providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(remoteInfo); .ReturnsAsync(remoteInfo);
var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of<IFileSystem>()); var itemImageProvider = GetItemImageProvider(providerManager.Object, null);
var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); 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 // 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) 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); Assert.True(index < imageCount);
} }
} }
@ -543,14 +543,14 @@ namespace Jellyfin.Providers.Tests.Manager
var providerManager = new Mock<IProviderManager>(MockBehavior.Strict); var providerManager = new Mock<IProviderManager>(MockBehavior.Strict);
providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>())) providerManager.Setup(pm => pm.GetAvailableRemoteImages(It.IsAny<BaseItem>(), It.IsAny<RemoteImageQuery>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(remoteInfo); .ReturnsAsync(remoteInfo);
var itemImageProvider = GetItemImageProvider(providerManager.Object, Mock.Of<IFileSystem>()); var itemImageProvider = GetItemImageProvider(providerManager.Object, new Mock<IFileSystem>());
var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.True(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
Assert.Equal(imageCount, item.GetImages(imageType).Count()); Assert.Equal(imageCount, item.GetImages(imageType).Count());
foreach (var image in item.GetImages(imageType)) 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 ReplaceAllImages = true
}; };
var itemImageProvider = GetItemImageProvider(Mock.Of<IProviderManager>(), Mock.Of<IFileSystem>()); var itemImageProvider = GetItemImageProvider(Mock.Of<IProviderManager>(), null);
var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List<IImageProvider> { remoteProvider.Object }, refreshOptions, CancellationToken.None);
Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate)); Assert.False(result.UpdateType.HasFlag(ItemUpdateType.ImageUpdate));
Assert.Equal(imageCount, item.GetImages(imageType).Count()); Assert.Equal(imageCount, item.GetImages(imageType).Count());
} }
private static ItemImageProvider GetItemImageProvider(IProviderManager? providerManager, IFileSystem? fileSystem) private static ItemImageProvider GetItemImageProvider(IProviderManager? providerManager, Mock<IFileSystem>? mockFileSystem)
{ {
// strict to ensure this isn't accidentally used where a prepared mock is intended // strict to ensure this isn't accidentally used where a prepared mock is intended
providerManager ??= Mock.Of<IProviderManager>(MockBehavior.Strict); providerManager ??= Mock.Of<IProviderManager>(MockBehavior.Strict);
fileSystem ??= Mock.Of<IFileSystem>(MockBehavior.Strict);
return new ItemImageProvider(new NullLogger<ItemImageProvider>(), 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<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) private static BaseItem GetItemWithImages(ImageType type, int count, bool validPaths)