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]