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)